Skip to content

Commit

Permalink
cmd/dlv,service/dap: use randomized name as default output binary
Browse files Browse the repository at this point in the history
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
  • Loading branch information
aarzilli committed May 16, 2023
1 parent e95ae9c commit b899f8d
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 16 deletions.
2 changes: 1 addition & 1 deletion Documentation/usage/dlv_debug.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```

Expand Down
2 changes: 1 addition & 1 deletion Documentation/usage/dlv_test.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Documentation/usage/dlv_trace.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
28 changes: 21 additions & 7 deletions cmd/dlv/cmds/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand All @@ -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{
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
38 changes: 37 additions & 1 deletion cmd/dlv/dlv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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!"
Expand Down Expand Up @@ -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")
}
}
28 changes: 28 additions & 0 deletions pkg/gobuild/defaultexe.go
Original file line number Diff line number Diff line change
@@ -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
}
14 changes: 9 additions & 5 deletions service/dap/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand All @@ -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{
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit b899f8d

Please sign in to comment.