*: Better error launching invalid binary format

Fixes #1310

pkg/proc/gdbserial/gdbserver.go

@@ -322,14 +322,8 @@ func getLdEnvVars() []string {
 // it to launch the specified target program with the specified arguments
 // (cmd) on the specified directory wd.
 func LLDBLaunch(cmd []string, wd string, foreground bool, debugInfoDirs []string) (*proc.Target, error) {
-	switch runtime.GOOS {
-	case "windows":
+	if runtime.GOOS == "windows" {
 		return nil, ErrUnsupportedOS
-	default:
-		// check that the argument to Launch is an executable file
-		if fi, staterr := os.Stat(cmd[0]); staterr == nil && (fi.Mode()&0111) == 0 {
-			return nil, proc.ErrNotExecutable
-		}
 	}
 
 	if foreground {

pkg/proc/native/proc_darwin.go

@@ -38,10 +38,6 @@ type osProcessDetails struct {
 // PT_SIGEXC on Darwin which will turn Unix signals into
 // Mach exceptions.
 func Launch(cmd []string, wd string, foreground bool, _ []string) (*proc.Target, error) {
-	// check that the argument to Launch is an executable file
-	if fi, staterr := os.Stat(cmd[0]); staterr == nil && (fi.Mode()&0111) == 0 {
-		return nil, proc.ErrNotExecutable
-	}
 	argv0Go, err := filepath.Abs(cmd[0])
 	if err != nil {
 		return nil, err

pkg/proc/native/proc_freebsd.go

@@ -48,10 +48,6 @@ func Launch(cmd []string, wd string, foreground bool, debugInfoDirs []string) (*
 		process *exec.Cmd
 		err     error
 	)
-	// check that the argument to Launch is an executable file
-	if fi, staterr := os.Stat(cmd[0]); staterr == nil && (fi.Mode()&0111) == 0 {
-		return nil, proc.ErrNotExecutable
-	}
 
 	if !isatty.IsTerminal(os.Stdin.Fd()) {
 		// exec.(*Process).Start will fail if we try to send a process to

pkg/proc/native/proc_linux.go

@@ -54,10 +54,6 @@ func Launch(cmd []string, wd string, foreground bool, debugInfoDirs []string) (*
 		process *exec.Cmd
 		err     error
 	)
-	// check that the argument to Launch is an executable file
-	if fi, staterr := os.Stat(cmd[0]); staterr == nil && (fi.Mode()&0111) == 0 {
-		return nil, proc.ErrNotExecutable
-	}
 
 	if !isatty.IsTerminal(os.Stdin.Fd()) {
 		// exec.(*Process).Start will fail if we try to send a process to

pkg/proc/native/proc_windows.go

@@ -1,11 +1,8 @@
 package native
 
 import (
-	"debug/pe"
 	"fmt"
-	"io"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"syscall"
 	"unsafe"
@@ -22,19 +19,6 @@ type osProcessDetails struct {
 	entryPoint  uint64
 }
 
-func openExecutablePathPE(path string) (*pe.File, io.Closer, error) {
-	f, err := os.OpenFile(path, 0, os.ModePerm)
-	if err != nil {
-		return nil, nil, err
-	}
-	peFile, err := pe.NewFile(f)
-	if err != nil {
-		f.Close()
-		return nil, nil, err
-	}
-	return peFile, f, nil
-}
-
 // Launch creates and begins debugging a new process.
 func Launch(cmd []string, wd string, foreground bool, _ []string) (*proc.Target, error) {
 	argv0Go, err := filepath.Abs(cmd[0])
@@ -42,19 +26,6 @@ func Launch(cmd []string, wd string, foreground bool, _ []string) (*proc.Target,
 		return nil, err
 	}
 
-	// Make sure the binary exists and is an executable file
-	if filepath.Base(cmd[0]) == cmd[0] {
-		if _, err := exec.LookPath(cmd[0]); err != nil {
-			return nil, err
-		}
-	}
-
-	_, closer, err := openExecutablePathPE(argv0Go)
-	if err != nil {
-		return nil, proc.ErrNotExecutable
-	}
-	closer.Close()
-
 	env := proc.DisableAsyncPreemptEnv()
 
 	var p *os.Process

pkg/proc/proc_test.go

@@ -2037,32 +2037,6 @@ func TestStepParked(t *testing.T) {
 	})
 }
 
-func TestIssue509(t *testing.T) {
-	fixturesDir := protest.FindFixturesDir()
-	nomaindir := filepath.Join(fixturesDir, "nomaindir")
-	cmd := exec.Command("go", "build", "-gcflags=-N -l", "-o", "debug")
-	cmd.Dir = nomaindir
-	assertNoError(cmd.Run(), t, "go build")
-	exepath := filepath.Join(nomaindir, "debug")
-	defer os.Remove(exepath)
-	var err error
-
-	switch testBackend {
-	case "native":
-		_, err = native.Launch([]string{exepath}, ".", false, []string{})
-	case "lldb":
-		_, err = gdbserial.LLDBLaunch([]string{exepath}, ".", false, []string{})
-	default:
-		t.Skip("test not valid for this backend")
-	}
-	if err == nil {
-		t.Fatalf("expected error but none was generated")
-	}
-	if err != proc.ErrNotExecutable {
-		t.Fatalf("expected error \"%v\" got \"%v\"", proc.ErrNotExecutable, err)
-	}
-}
-
 func TestUnsupportedArch(t *testing.T) {
 	ver, _ := goversion.Parse(runtime.Version())
 	if ver.Major < 0 || !ver.AfterOrEqual(goversion.GoVersion{Major: 1, Minor: 6, Rev: -1}) || ver.AfterOrEqual(goversion.GoVersion{Major: 1, Minor: 7, Rev: -1}) {

pkg/proc/target.go

@@ -11,10 +11,6 @@ import (
 )
 
 var (
-	// ErrNotExecutable is returned after attempting to execute a non-executable file
-	// to begin a debug session.
-	ErrNotExecutable = errors.New("not an executable file")
-
 	// ErrNotRecorded is returned when an action is requested that is
 	// only possible on recorded (traced) programs.
 	ErrNotRecorded = errors.New("not a recording")

service/api/types.go

@@ -13,7 +13,7 @@ import (
 
 // ErrNotExecutable is an error returned when trying
 // to debug a non-executable file.
-var ErrNotExecutable = proc.ErrNotExecutable
+var ErrNotExecutable = errors.New("not an executable file")
 
 // DebuggerState represents the current context of the debugger.
 type DebuggerState struct {
@@ -212,7 +212,7 @@ const (
 	// that may outlive the stack frame are allocated on the heap instead and
 	// only the address is recorded on the stack. These variables will be
 	// marked with this flag.
-	VariableEscaped = (1 << iota)
+	VariableEscaped = 1 << iota
 
 	// VariableShadowed is set for local variables that are shadowed by a
 	// variable with the same name in another scope

service/debugger/debugger.go

@@ -26,6 +26,17 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+var (
+	// ErrCanNotRestart is returned when the target cannot be restarted.
+	// This is returned for targets that have been attached to, or when
+	// debugging core files.
+	ErrCanNotRestart = errors.New("can not restart this target")
+
+	// ErrNotRecording is returned when StopRecording is called while the
+	// debugger is not recording the target.
+	ErrNotRecording = errors.New("debugger is not recording")
+)
+
 // Debugger service.
 //
 // Debugger provides a higher level of
@@ -179,6 +190,9 @@ func (d *Debugger) checkGoVersion() error {
 
 // 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 {
+		return nil, err
+	}
 	switch d.config.Backend {
 	case "native":
 		return native.Launch(processArgs, wd, d.config.Foreground, d.config.DebugInfoDirectories)
@@ -260,11 +274,6 @@ func (d *Debugger) recordingRun(run func() (string, error)) (*proc.Target, error
 	return gdbserial.Replay(tracedir, false, true, d.config.DebugInfoDirectories)
 }
 
-// ErrNoAttachPath is the error returned when the client tries to attach to
-// a process on macOS using the lldb backend without specifying the path to
-// the target's executable.
-var ErrNoAttachPath = errors.New("must specify executable path on macOS")
-
 // Attach will attach to the process specified by 'pid'.
 func (d *Debugger) Attach(pid int, path string) (*proc.Target, error) {
 	switch d.config.Backend {
@@ -369,12 +378,6 @@ func (d *Debugger) detach(kill bool) error {
 	return d.target.Detach(kill)
 }
 
-var ErrCanNotRestart = errors.New("can not restart this target")
-
-// ErrNotRecording is returned when StopRecording is called while the
-// debugger is not recording the target.
-var ErrNotRecording = errors.New("debugger is not recording")
-
 // Restart will restart the target process, first killing
 // and then exec'ing it again.
 // If the target process is a recording it will restart it from the given

service/debugger/debugger_test.go

@@ -0,0 +1,70 @@
+package debugger
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+
+	"github.com/go-delve/delve/pkg/gobuild"
+	protest "github.com/go-delve/delve/pkg/proc/test"
+	"github.com/go-delve/delve/service/api"
+)
+
+func TestDebugger_LaunchNoMain(t *testing.T) {
+	fixturesDir := protest.FindFixturesDir()
+	nomaindir := filepath.Join(fixturesDir, "nomaindir")
+	debugname := "debug"
+	exepath := filepath.Join(nomaindir, debugname)
+	defer os.Remove(exepath)
+	if err := gobuild.GoBuild(debugname, []string{nomaindir}, fmt.Sprintf("-o %s", exepath)); err != nil {
+		t.Fatalf("go build error %v", err)
+	}
+
+	d := new(Debugger)
+	_, err := d.Launch([]string{exepath}, ".")
+	if err == nil {
+		t.Fatalf("expected error but none was generated")
+	}
+	if err != api.ErrNotExecutable {
+		t.Fatalf("expected error \"%v\" got \"%v\"", api.ErrNotExecutable, err)
+	}
+}
+
+func TestDebugger_LaunchInvalidFormat(t *testing.T) {
+	goos := os.Getenv("GOOS")
+	goarch := os.Getenv("GOARCH")
+	defer func() {
+		// restore environment values
+		os.Setenv("GOOS", goos)
+		os.Setenv("GOARCH", goarch)
+	}()
+	fixturesDir := protest.FindFixturesDir()
+	buildtestdir := filepath.Join(fixturesDir, "buildtest")
+	debugname := "debug"
+	switchOS := map[string]string{
+		"darwin":  "linux",
+		"windows": "linux",
+		"freebsd": "windows",
+		"linux":   "windows",
+	}
+	if runtime.GOARCH == "arm64" && runtime.GOOS == "linux" {
+		os.Setenv("GOARCH", "amd64")
+	}
+	os.Setenv("GOOS", switchOS[runtime.GOOS])
+	exepath := filepath.Join(buildtestdir, debugname)
+	if err := gobuild.GoBuild(debugname, []string{buildtestdir}, fmt.Sprintf("-o %s", exepath)); err != nil {
+		t.Fatalf("go build error %v", err)
+	}
+	defer os.Remove(exepath)
+
+	d := new(Debugger)
+	_, err := d.Launch([]string{exepath}, ".")
+	if err == nil {
+		t.Fatalf("expected error but none was generated")
+	}
+	if err != api.ErrNotExecutable {
+		t.Fatalf("expected error \"%s\" got \"%v\"", api.ErrNotExecutable, err)
+	}
+}

service/debugger/debugger_unix.go

@@ -0,0 +1,42 @@
+// +build !windows
+
+package debugger
+
+import (
+	"debug/elf"
+	"debug/macho"
+	"os"
+	"runtime"
+
+	"github.com/go-delve/delve/service/api"
+)
+
+func verifyBinaryFormat(exePath string) error {
+	f, err := os.Open(exePath)
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+
+	fi, err := f.Stat()
+	if err != nil {
+		return err
+	}
+	if (fi.Mode() & 0111) == 0 {
+		return api.ErrNotExecutable
+	}
+
+	// check that the binary format is what we expect for the host system
+	switch runtime.GOOS {
+	case "darwin":
+		_, err = macho.NewFile(f)
+	case "linux", "freebsd":
+		_, err = elf.NewFile(f)
+	default:
+		panic("attempting to open file Delve cannot parse")
+	}
+	if err != nil {
+		return api.ErrNotExecutable
+	}
+	return nil
+}

service/debugger/debugger_unix_test.go

@@ -0,0 +1,47 @@
+// +build !windows
+
+package debugger
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+
+	"github.com/go-delve/delve/pkg/gobuild"
+	protest "github.com/go-delve/delve/pkg/proc/test"
+	"github.com/go-delve/delve/service/api"
+)
+
+func TestDebugger_LaunchNoExecutablePerm(t *testing.T) {
+	fixturesDir := protest.FindFixturesDir()
+	buildtestdir := filepath.Join(fixturesDir, "buildtest")
+	debugname := "debug"
+	switchOS := map[string]string{
+		"darwin":  "linux",
+		"windows": "linux",
+		"freebsd": "windows",
+		"linux":   "windows",
+	}
+	if runtime.GOARCH == "arm64" && runtime.GOOS == "linux" {
+		os.Setenv("GOARCH", "amd64")
+	}
+	os.Setenv("GOOS", switchOS[runtime.GOOS])
+	exepath := filepath.Join(buildtestdir, debugname)
+	if err := gobuild.GoBuild(debugname, []string{buildtestdir}, fmt.Sprintf("-o %s", exepath)); err != nil {
+		t.Fatalf("go build error %v", err)
+	}
+	defer os.Remove(exepath)
+	if err := os.Chmod(exepath, 0644); err != nil {
+		t.Fatal(err)
+	}
+	d := new(Debugger)
+	_, err := d.Launch([]string{exepath}, ".")
+	if err == nil {
+		t.Fatalf("expected error but none was generated")
+	}
+	if err != api.ErrNotExecutable {
+		t.Fatalf("expected error \"%s\" got \"%v\"", api.ErrNotExecutable, err)
+	}
+}

service/debugger/debugger_windows.go

@@ -1,7 +1,13 @@
 package debugger
 
 import (
+	"debug/pe"
 	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+
+	"github.com/go-delve/delve/service/api"
 )
 
 func attachErrorMessage(pid int, err error) error {
@@ -14,3 +20,23 @@ func stopProcess(pid int) error {
 	// the process.
 	return nil
 }
+
+func verifyBinaryFormat(exePath string) error {
+	f, err := os.Open(exePath)
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+
+	// Make sure the binary exists and is an executable file
+	if filepath.Base(exePath) == exePath {
+		if _, err := exec.LookPath(exePath); err != nil {
+			return err
+		}
+	}
+
+	if _, err = pe.NewFile(f); err != nil {
+		return api.ErrNotExecutable
+	}
+	return nil
+}