From b899f8d476e3d6a96ec3e536ed228faed7ae2d4e Mon Sep 17 00:00:00 2001 From: aarzilli Date: Fri, 12 May 2023 15:25:49 +0200 Subject: [PATCH] cmd/dlv,service/dap: use randomized name as default output binary Using a fixed path as the default output binary means that executing Delve twice in the same directory will cause the second invocation to overwrite the output binary of the first instance of Delve, making the restart command not work correctly. Fixes #3345 --- Documentation/usage/dlv_debug.md | 2 +- Documentation/usage/dlv_test.md | 2 +- Documentation/usage/dlv_trace.md | 2 +- cmd/dlv/cmds/commands.go | 28 +++++++++++++++++------ cmd/dlv/dlv_test.go | 38 +++++++++++++++++++++++++++++++- pkg/gobuild/defaultexe.go | 28 +++++++++++++++++++++++ service/dap/server.go | 14 +++++++----- 7 files changed, 98 insertions(+), 16 deletions(-) create mode 100644 pkg/gobuild/defaultexe.go diff --git a/Documentation/usage/dlv_debug.md b/Documentation/usage/dlv_debug.md index 718ae476c6..23c75ea840 100644 --- a/Documentation/usage/dlv_debug.md +++ b/Documentation/usage/dlv_debug.md @@ -20,7 +20,7 @@ dlv debug [package] [flags] ``` --continue Continue the debugged process on start. -h, --help help for debug - --output string Output path for the binary. (default "./__debug_bin") + --output string Output path for the binary. --tty string TTY to use for the target program ``` diff --git a/Documentation/usage/dlv_test.md b/Documentation/usage/dlv_test.md index d7817541a2..37a9fe0ff0 100644 --- a/Documentation/usage/dlv_test.md +++ b/Documentation/usage/dlv_test.md @@ -23,7 +23,7 @@ dlv test [package] [flags] ``` -h, --help help for test - --output string Output path for the binary. (default "debug.test") + --output string Output path for the binary. ``` ### Options inherited from parent commands diff --git a/Documentation/usage/dlv_trace.md b/Documentation/usage/dlv_trace.md index 018a511429..a6428100b9 100644 --- a/Documentation/usage/dlv_trace.md +++ b/Documentation/usage/dlv_trace.md @@ -24,7 +24,7 @@ dlv trace [package] regexp [flags] --ebpf Trace using eBPF (experimental). -e, --exec string Binary file to exec and trace. -h, --help help for trace - --output string Output path for the binary. (default "debug") + --output string Output path for the binary. -p, --pid int Pid to attach to. -s, --stack int Show stack trace with given depth. (Ignored with --ebpf) -t, --test Trace a test binary. diff --git a/cmd/dlv/cmds/commands.go b/cmd/dlv/cmds/commands.go index 54b61ab939..ee4d696f15 100644 --- a/cmd/dlv/cmds/commands.go +++ b/cmd/dlv/cmds/commands.go @@ -230,7 +230,7 @@ package name and Delve will compile that package instead, and begin a new debug session.`, Run: debugCmd, } - debugCommand.Flags().String("output", "./__debug_bin", "Output path for the binary.") + debugCommand.Flags().String("output", "", "Output path for the binary.") debugCommand.Flags().BoolVar(&continueOnStart, "continue", false, "Continue the debugged process on start.") debugCommand.Flags().StringVar(&tty, "tty", "", "TTY to use for the target program") rootCommand.AddCommand(debugCommand) @@ -287,7 +287,7 @@ dlv test [package] -- -test.run TestSomething -test.v -other-argument See also: 'go help testflag'.`, Run: testCmd, } - testCommand.Flags().String("output", "debug.test", "Output path for the binary.") + testCommand.Flags().String("output", "", "Output path for the binary.") rootCommand.AddCommand(testCommand) // 'trace' subcommand. @@ -311,7 +311,7 @@ only see the output of the trace operations you can redirect stdout.`, traceCommand.Flags().BoolVarP(&traceUseEBPF, "ebpf", "", false, "Trace using eBPF (experimental).") traceCommand.Flags().BoolVarP(&traceShowTimestamp, "timestamp", "", false, "Show timestamp in the output") traceCommand.Flags().IntVarP(&traceStackDepth, "stack", "s", 0, "Show stack trace with given depth. (Ignored with --ebpf)") - traceCommand.Flags().String("output", "debug", "Output path for the binary.") + traceCommand.Flags().String("output", "", "Output path for the binary.") rootCommand.AddCommand(traceCommand) coreCommand := &cobra.Command{ @@ -528,10 +528,21 @@ func dapCmd(cmd *cobra.Command, args []string) { } func buildBinary(cmd *cobra.Command, args []string, isTest bool) (string, bool) { - debugname, err := filepath.Abs(cmd.Flag("output").Value.String()) - if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - return "", false + outputFlag := cmd.Flag("output").Value.String() + var debugname string + var err error + if outputFlag == "" { + if isTest { + debugname = gobuild.DefaultDebugBinaryPath("debug.test") + } else { + debugname = gobuild.DefaultDebugBinaryPath("__debug_bin") + } + } else { + debugname, err = filepath.Abs(outputFlag) + if err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + return "", false + } } if isTest { @@ -540,6 +551,9 @@ func buildBinary(cmd *cobra.Command, args []string, isTest bool) (string, bool) err = gobuild.GoBuild(debugname, args, buildFlags) } if err != nil { + if outputFlag == "" { + gobuild.Remove(debugname) + } fmt.Fprintf(os.Stderr, "%v\n", err) return "", false } diff --git a/cmd/dlv/dlv_test.go b/cmd/dlv/dlv_test.go index e1c3006528..06b8fd6020 100644 --- a/cmd/dlv/dlv_test.go +++ b/cmd/dlv/dlv_test.go @@ -239,7 +239,7 @@ func getDlvBinInternal(t *testing.T, goflags ...string) string { func TestOutput(t *testing.T) { dlvbin := getDlvBin(t) - for _, output := range []string{"", "myownname", filepath.Join(t.TempDir(), "absolute.path")} { + for _, output := range []string{"__debug_bin", "myownname", filepath.Join(t.TempDir(), "absolute.path")} { testOutput(t, dlvbin, output, []string{"exit"}) const hello = "hello world!" @@ -1327,3 +1327,39 @@ func TestStaticcheck(t *testing.T) { out, _ := cmd.CombinedOutput() checkAutogenDoc(t, "_scripts/staticcheck-out.txt", fmt.Sprintf("staticcheck %s > _scripts/staticcheck-out.txt", strings.Join(args, " ")), out) } + +func TestDefaultBinary(t *testing.T) { + // Check that when delve is run twice in the same directory simultaneously + // it will pick different default output binary paths. + dlvbin := getDlvBin(t) + fixture := filepath.Join(protest.FindFixturesDir(), "testargs.go") + + startOne := func() (io.WriteCloser, func() error, *bytes.Buffer) { + cmd := exec.Command(dlvbin, "debug", "--allow-non-terminal-interactive=true", fixture, "--", "test") + stdin, _ := cmd.StdinPipe() + stdoutBuf := new(bytes.Buffer) + cmd.Stdout = stdoutBuf + + assertNoError(cmd.Start(), t, "dlv debug") + return stdin, cmd.Wait, stdoutBuf + } + + stdin1, wait1, stdoutBuf1 := startOne() + defer stdin1.Close() + + stdin2, wait2, stdoutBuf2 := startOne() + defer stdin2.Close() + + fmt.Fprintf(stdin1, "continue\nquit\n") + fmt.Fprintf(stdin2, "continue\nquit\n") + + wait1() + wait2() + + out1, out2 := stdoutBuf1.String(), stdoutBuf2.String() + t.Logf("%q", out1) + t.Logf("%q", out2) + if out1 == out2 { + t.Errorf("outputs match") + } +} diff --git a/pkg/gobuild/defaultexe.go b/pkg/gobuild/defaultexe.go new file mode 100644 index 0000000000..14fb313fa0 --- /dev/null +++ b/pkg/gobuild/defaultexe.go @@ -0,0 +1,28 @@ +package gobuild + +import ( + "io/ioutil" + "runtime" + + "github.com/go-delve/delve/pkg/logflags" +) + +// DefaultDebugBinaryPath returns an unused file path in the current +// directory named 'name' followed by a random string +func DefaultDebugBinaryPath(name string) string { + pattern := name + if runtime.GOOS == "windows" { + pattern += "*.exe" + } + f, err := ioutil.TempFile(".", pattern) + if err != nil { + logflags.DebuggerLogger().Errorf("could not create temporary file for build output: %v", err) + if runtime.GOOS == "windows" { + return name + ".exe" + } + return name + } + r := f.Name() + f.Close() + return r +} diff --git a/service/dap/server.go b/service/dap/server.go index ecebb778d3..2b0532e222 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -859,10 +859,6 @@ func (s *Session) setClientCapabilities(args dap.InitializeRequestArguments) { s.clientCapabilities.supportsVariableType = args.SupportsVariableType } -// Default output file pathname for the compiled binary in debug or test modes. -// This is relative to the current working directory of the server. -const defaultDebugBinary string = "./__debug_bin" - func cleanExeName(name string) string { if runtime.GOOS == "windows" && filepath.Ext(name) != ".exe" { return name + ".exe" @@ -957,8 +953,10 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { // Prepare the debug executable filename, building it if necessary debugbinary := args.Program if args.Mode == "debug" || args.Mode == "test" { + deleteOnError := false if args.Output == "" { - args.Output = cleanExeName(defaultDebugBinary) + deleteOnError = true + args.Output = gobuild.DefaultDebugBinaryPath("__debug_bin") } else { args.Output = cleanExeName(args.Output) } @@ -981,6 +979,9 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { args.DlvCwd, _ = filepath.Abs(args.DlvCwd) s.config.log.Debugf("building from %q: [%s]", args.DlvCwd, cmd) if err != nil { + if deleteOnError { + gobuild.Remove(args.Output) + } s.send(&dap.OutputEvent{ Event: *newEvent("output"), Body: dap.OutputEventBody{ @@ -1049,6 +1050,9 @@ func (s *Session) onLaunchRequest(request *dap.LaunchRequest) { s.debugger, err = debugger.New(&s.config.Debugger, s.config.ProcessArgs) }() if err != nil { + if s.binaryToRemove != "" { + gobuild.Remove(s.binaryToRemove) + } s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", err.Error()) return }