From b072f61bf853fb8a22e6d09b1c89e209358831fc Mon Sep 17 00:00:00 2001 From: Derek Parker Date: Fri, 28 Oct 2022 05:59:09 -0700 Subject: [PATCH] service/debugger: Assume current dir for exec (#3167) This patch modifies the behavior of the exec subcommand such that you don't necessarily have to write the "./" prefix when trying to debug a precompiled binary in your working directory. For example (given foo.test in working dir), before this change: dlv exec foo.test Would result in an error, forcing the user to type: dlv exec ./foo.test This just makes things a bit more convenient. --- pkg/proc/native/proc_windows.go | 6 +---- service/debugger/debugger.go | 44 +++++++++++++++---------------- service/debugger/debugger_test.go | 33 +++++++++++++++++++++++ 3 files changed, 56 insertions(+), 27 deletions(-) diff --git a/pkg/proc/native/proc_windows.go b/pkg/proc/native/proc_windows.go index 5c5ffcf25f..d37249ed0d 100644 --- a/pkg/proc/native/proc_windows.go +++ b/pkg/proc/native/proc_windows.go @@ -3,7 +3,6 @@ package native import ( "fmt" "os" - "path/filepath" "syscall" "unsafe" @@ -25,10 +24,7 @@ func (os *osProcessDetails) Close() {} // Launch creates and begins debugging a new process. func Launch(cmd []string, wd string, flags proc.LaunchFlags, _ []string, _ string, redirects [3]string) (*proc.Target, error) { - argv0Go, err := filepath.Abs(cmd[0]) - if err != nil { - return nil, err - } + argv0Go := cmd[0] env := proc.DisableAsyncPreemptEnv() diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index cabba03bfb..091f46294e 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -9,6 +9,7 @@ import ( "fmt" "go/parser" "go/token" + "io" "os" "os/exec" "path/filepath" @@ -246,9 +247,11 @@ func (d *Debugger) TargetGoVersion() string { // Launch will start a process with the given args and working directory. func (d *Debugger) Launch(processArgs []string, wd string) (*proc.Target, error) { - if err := verifyBinaryFormat(processArgs[0]); err != nil { + fullpath, err := verifyBinaryFormat(processArgs[0]) + if err != nil { return nil, err } + processArgs[0] = fullpath launchFlags := proc.LaunchFlags(0) if d.config.Foreground { @@ -2259,34 +2262,31 @@ func noDebugErrorWarning(err error) error { return err } -func verifyBinaryFormat(exePath string) error { - f, err := os.Open(exePath) +func verifyBinaryFormat(exePath string) (string, error) { + fullpath, err := filepath.Abs(exePath) if err != nil { - return err + return "", err + } + + f, err := os.Open(fullpath) + if err != nil { + return "", err } defer f.Close() - switch runtime.GOOS { - case "windows": - // Make sure the binary exists and is an executable file - if filepath.Base(exePath) == exePath { - if _, err := exec.LookPath(exePath); err != nil { - return err - } - } - default: - fi, err := f.Stat() + // Skip this check on Windows. + // TODO(derekparker) exec.LookPath looks for valid Windows extensions. + // We don't create our binaries with valid extensions, even though we should. + // Skip this check for now. + if runtime.GOOS != "windows" { + _, err = exec.LookPath(fullpath) if err != nil { - return err - } - if (fi.Mode() & 0111) == 0 { - return api.ErrNotExecutable + return "", api.ErrNotExecutable } } // check that the binary format is what we expect for the host system - var exe interface{ Close() error } - + var exe io.Closer switch runtime.GOOS { case "darwin": exe, err = macho.NewFile(f) @@ -2299,10 +2299,10 @@ func verifyBinaryFormat(exePath string) error { } if err != nil { - return api.ErrNotExecutable + return "", api.ErrNotExecutable } exe.Close() - return nil + return fullpath, nil } var attachErrorMessage = attachErrorMessageDefault diff --git a/service/debugger/debugger_test.go b/service/debugger/debugger_test.go index 275e1a5cab..b8b05ece55 100644 --- a/service/debugger/debugger_test.go +++ b/service/debugger/debugger_test.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" "github.com/go-delve/delve/pkg/gobuild" @@ -68,3 +69,35 @@ func TestDebugger_LaunchInvalidFormat(t *testing.T) { t.Fatalf("expected error \"%s\" got \"%v\"", api.ErrNotExecutable, err) } } + +func TestDebugger_LaunchCurrentDir(t *testing.T) { + fixturesDir := protest.FindFixturesDir() + testDir := filepath.Join(fixturesDir, "buildtest") + debugname := "debug" + exepath := filepath.Join(testDir, debugname) + originalPath, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + defer os.Chdir(originalPath) + defer func() { + if err := os.Remove(exepath); err != nil { + t.Fatalf("error removing executable %v", err) + } + }() + if err := gobuild.GoBuild(debugname, []string{testDir}, fmt.Sprintf("-o %s", exepath)); err != nil { + t.Fatalf("go build error %v", err) + } + + os.Chdir(testDir) + + d := new(Debugger) + d.config = &Config{} + _, err = d.Launch([]string{debugname}, ".") + if err == nil { + t.Fatal("expected error but none was generated") + } + if err != nil && !strings.Contains(err.Error(), "unknown backend") { + t.Fatal(err) + } +}