diff --git a/_fixtures/buildtest/main_test.go b/_fixtures/buildtest/main_test.go new file mode 100644 index 00000000..3b7d16cc --- /dev/null +++ b/_fixtures/buildtest/main_test.go @@ -0,0 +1,10 @@ +package main + +import ( + "os" + "testing" +) + +func TestMain(m *testing.M) { + os.Exit(m.Run()) +} diff --git a/cmd/dlv/cmds/commands.go b/cmd/dlv/cmds/commands.go index 39726f39..bc6a8d9f 100644 --- a/cmd/dlv/cmds/commands.go +++ b/cmd/dlv/cmds/commands.go @@ -13,6 +13,7 @@ import ( "syscall" "github.com/go-delve/delve/pkg/config" + "github.com/go-delve/delve/pkg/gobuild" "github.com/go-delve/delve/pkg/goversion" "github.com/go-delve/delve/pkg/logflags" "github.com/go-delve/delve/pkg/terminal" @@ -354,14 +355,6 @@ and dap modes. return RootCommand } -// Remove the file at path and issue a warning to stderr if this fails. -func remove(path string) { - err := os.Remove(path) - if err != nil { - fmt.Fprintf(os.Stderr, "could not remove %v: %v\n", path, err) - } -} - func dapCmd(cmd *cobra.Command, args []string) { status := func() int { if err := logflags.Setup(Log, LogOutput, LogDest); err != nil { @@ -428,12 +421,12 @@ func debugCmd(cmd *cobra.Command, args []string) { } dlvArgs, targetArgs := splitArgs(cmd, args) - err = gobuild(debugname, dlvArgs) + err = gobuild.GoBuild(debugname, dlvArgs, BuildFlags) if err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) return 1 } - defer remove(debugname) + defer gobuild.Remove(debugname) processArgs := append([]string{debugname}, targetArgs...) return execute(0, processArgs, conf, "", executingGeneratedFile) }() @@ -484,17 +477,17 @@ func traceCmd(cmd *cobra.Command, args []string) { return 1 } if traceTestBinary { - if err := gotestbuild(debugname, dlvArgs); err != nil { + if err := gobuild.GoTestBuild(debugname, dlvArgs, BuildFlags); err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) return 1 } } else { - if err := gobuild(debugname, dlvArgs); err != nil { + if err := gobuild.GoBuild(debugname, dlvArgs, BuildFlags); err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) return 1 } } - defer remove(debugname) + defer gobuild.Remove(debugname) } processArgs = append([]string{debugname}, targetArgs...) @@ -576,11 +569,11 @@ func testCmd(cmd *cobra.Command, args []string) { } dlvArgs, targetArgs := splitArgs(cmd, args) - err = gotestbuild(debugname, dlvArgs) + err = gobuild.GoTestBuild(debugname, dlvArgs, BuildFlags) if err != nil { return 1 } - defer remove(debugname) + defer gobuild.Remove(debugname) processArgs := append([]string{debugname}, targetArgs...) return execute(0, processArgs, conf, "", executingGeneratedTest) @@ -795,48 +788,3 @@ func execute(attachPid int, processArgs []string, conf *config.Config, coreFile return connect(listener.Addr().String(), clientConn, conf, kind) } - -func optflags(args []string) []string { - // after go1.9 building with -gcflags='-N -l' and -a simultaneously works. - // after go1.10 specifying -a is unnecessary because of the new caching strategy, but we should pass -gcflags=all=-N -l to have it applied to all packages - // see https://github.com/golang/go/commit/5993251c015dfa1e905bdf44bdb41572387edf90 - - ver, _ := goversion.Installed() - switch { - case ver.Major < 0 || ver.AfterOrEqual(goversion.GoVersion{1, 10, -1, 0, 0, ""}): - args = append(args, "-gcflags", "all=-N -l") - case ver.AfterOrEqual(goversion.GoVersion{1, 9, -1, 0, 0, ""}): - args = append(args, "-gcflags", "-N -l", "-a") - default: - args = append(args, "-gcflags", "-N -l") - } - return args -} - -func gobuild(debugname string, pkgs []string) error { - args := []string{"-o", debugname} - args = optflags(args) - if BuildFlags != "" { - args = append(args, config.SplitQuotedFields(BuildFlags, '\'')...) - } - args = append(args, pkgs...) - return gocommand("build", args...) -} - -func gotestbuild(debugname string, pkgs []string) error { - args := []string{"-c", "-o", debugname} - args = optflags(args) - if BuildFlags != "" { - args = append(args, config.SplitQuotedFields(BuildFlags, '\'')...) - } - args = append(args, pkgs...) - return gocommand("test", args...) -} - -func gocommand(command string, args ...string) error { - allargs := []string{command} - allargs = append(allargs, args...) - goBuild := exec.Command("go", allargs...) - goBuild.Stderr = os.Stderr - return goBuild.Run() -} diff --git a/pkg/gobuild/gobuild.go b/pkg/gobuild/gobuild.go new file mode 100644 index 00000000..ed59d1ac --- /dev/null +++ b/pkg/gobuild/gobuild.go @@ -0,0 +1,72 @@ +// Package gobuild provides utilities for building programs and tests +// for the debugging session. +package gobuild + +import ( + "fmt" + "os" + "os/exec" + + "github.com/go-delve/delve/pkg/config" + "github.com/go-delve/delve/pkg/goversion" +) + +// Remove the file at path and issue a warning to stderr if this fails. +// This can be used to remove the temporary binary generated for the session. +func Remove(path string) { + err := os.Remove(path) + if err != nil { + fmt.Fprintf(os.Stderr, "could not remove %v: %v\n", path, err) + } +} + +// optflags generates default build flags to turn off optimization and inlining. +func optflags(args []string) []string { + // after go1.9 building with -gcflags='-N -l' and -a simultaneously works. + // after go1.10 specifying -a is unnecessary because of the new caching strategy, + // but we should pass -gcflags=all=-N -l to have it applied to all packages + // see https://github.com/golang/go/commit/5993251c015dfa1e905bdf44bdb41572387edf90 + + ver, _ := goversion.Installed() + switch { + case ver.Major < 0 || ver.AfterOrEqual(goversion.GoVersion{1, 10, -1, 0, 0, ""}): + args = append(args, "-gcflags", "all=-N -l") + case ver.AfterOrEqual(goversion.GoVersion{1, 9, -1, 0, 0, ""}): + args = append(args, "-gcflags", "-N -l", "-a") + default: + args = append(args, "-gcflags", "-N -l") + } + return args +} + +// GoBuild builds non-test files in 'pkgs' with the specified 'buildflags' +// and writes the output at 'debugname'. +func GoBuild(debugname string, pkgs []string, buildflags string) error { + args := []string{"-o", debugname} + args = optflags(args) + if buildflags != "" { + args = append(args, config.SplitQuotedFields(buildflags, '\'')...) + } + args = append(args, pkgs...) + return gocommand("build", args...) +} + +// GoBuild builds test files 'pkgs' with the specified 'buildflags' +// and writes the output at 'debugname'. +func GoTestBuild(debugname string, pkgs []string, buildflags string) error { + args := []string{"-c", "-o", debugname} + args = optflags(args) + if buildflags != "" { + args = append(args, config.SplitQuotedFields(buildflags, '\'')...) + } + args = append(args, pkgs...) + return gocommand("test", args...) +} + +func gocommand(command string, args ...string) error { + allargs := []string{command} + allargs = append(allargs, args...) + goBuild := exec.Command("go", allargs...) + goBuild.Stderr = os.Stderr + return goBuild.Run() +} diff --git a/service/dap/daptest/client.go b/service/dap/daptest/client.go index 35d90b9f..7d607bc7 100644 --- a/service/dap/daptest/client.go +++ b/service/dap/daptest/client.go @@ -136,18 +136,27 @@ func (c *Client) InitializeRequest() { c.send(request) } -// LaunchRequest sends a 'launch' request. -func (c *Client) LaunchRequest(program string, stopOnEntry bool) { +// LaunchRequest sends a 'launch' request with the specified args. +func (c *Client) LaunchRequest(mode string, program string, stopOnEntry bool) { request := &dap.LaunchRequest{Request: *c.newRequest("launch")} request.Arguments = map[string]interface{}{ "request": "launch", - "mode": "exec", + "mode": mode, "program": program, "stopOnEntry": stopOnEntry, } c.send(request) } +// LaunchRequestWithArgs takes a map of untyped implementation-specific +// arguments to send a 'launch' request. This version can be used to +// test for values of unexpected types or unspecified values. +func (c *Client) LaunchRequestWithArgs(arguments map[string]interface{}) { + request := &dap.LaunchRequest{Request: *c.newRequest("launch")} + request.Arguments = arguments + c.send(request) +} + // DisconnectRequest sends a 'disconnect' request. func (c *Client) DisconnectRequest() { request := &dap.DisconnectRequest{Request: *c.newRequest("disconnect")} diff --git a/service/dap/server.go b/service/dap/server.go index fe916023..83b68e70 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -16,6 +16,7 @@ import ( "net" "path/filepath" + "github.com/go-delve/delve/pkg/gobuild" "github.com/go-delve/delve/pkg/logflags" "github.com/go-delve/delve/pkg/proc" "github.com/go-delve/delve/service" @@ -52,6 +53,8 @@ type Server struct { log *logrus.Entry // stopOnEntry is set to automatically stop the debugee after start. stopOnEntry bool + // binaryToRemove is the compiled binary to be removed on disconnect. + binaryToRemove string } // NewServer creates a new DAP Server. It takes an opened Listener @@ -115,6 +118,9 @@ func (s *Server) signalDisconnect() { close(s.config.DisconnectChan) s.config.DisconnectChan = nil } + if s.binaryToRemove != "" { + gobuild.Remove(s.binaryToRemove) + } } // Run launches a new goroutine where it accepts a client connection @@ -286,34 +292,72 @@ func (s *Server) onInitializeRequest(request *dap.InitializeRequest) { s.send(response) } +// Output path for the compiled binary in debug or test modes. +const debugBinary string = "./__debug_bin" + func (s *Server) onLaunchRequest(request *dap.LaunchRequest) { // TODO(polina): Respond with an error if debug session is in progress? - program, ok := request.Arguments["program"] + + program, ok := request.Arguments["program"].(string) if !ok || program == "" { s.sendErrorResponse(request.Request, FailedToContinue, "Failed to launch", "The program attribute is missing in debug configuration.") return } - s.config.ProcessArgs = []string{program.(string)} - s.config.WorkingDir = filepath.Dir(program.(string)) - // TODO: support program args - - stop, ok := request.Arguments["stopOnEntry"] - s.stopOnEntry = (ok && stop == true) mode, ok := request.Arguments["mode"] if !ok || mode == "" { mode = "debug" } - // TODO(polina): support "debug", "test" and "remote" modes - if mode != "exec" { + + if mode == "debug" || mode == "test" { + output, ok := request.Arguments["output"].(string) + if !ok || output == "" { + output = debugBinary + } + debugname, err := filepath.Abs(output) + if err != nil { + s.sendInternalErrorResponse(request.Seq, err.Error()) + return + } + + buildFlags, ok := request.Arguments["buildFlags"].(string) + if !ok { + buildFlags = "" + } + + switch mode { + case "debug": + err = gobuild.GoBuild(debugname, []string{program}, buildFlags) + case "test": + err = gobuild.GoTestBuild(debugname, []string{program}, buildFlags) + } + if err != nil { + s.sendErrorResponse(request.Request, + FailedToContinue, "Failed to launch", + fmt.Sprintf("Build error: %s", err.Error())) + return + } + program = debugname + s.binaryToRemove = debugname + } + + // TODO(polina): support "remote" mode + if mode != "exec" && mode != "debug" && mode != "test" { s.sendErrorResponse(request.Request, FailedToContinue, "Failed to launch", fmt.Sprintf("Unsupported 'mode' value %q in debug configuration.", mode)) return } + stop, ok := request.Arguments["stopOnEntry"] + s.stopOnEntry = (ok && stop == true) + + // TODO(polina): support target args + s.config.ProcessArgs = []string{program} + s.config.WorkingDir = filepath.Dir(program) + config := &debugger.Config{ WorkingDir: s.config.WorkingDir, AttachPid: 0, diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 6160fc92..5e1ceeef 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -5,6 +5,7 @@ import ( "io" "net" "os" + "path/filepath" "sync" "testing" "time" @@ -16,6 +17,8 @@ import ( "github.com/google/go-dap" ) +const stopOnEntry bool = true + func TestMain(m *testing.M) { var logOutput string flag.StringVar(&logOutput, "log-output", "", "configures log output") @@ -73,7 +76,7 @@ func TestStopOnEntry(t *testing.T) { t.Errorf("got %#v, want Seq=0, RequestSeq=0", initResp) } - client.LaunchRequest(fixture.Path, true /*stopOnEntry*/) + client.LaunchRequest("exec", fixture.Path, stopOnEntry) initEv := client.ExpectInitializedEvent(t) if initEv.Seq != 0 { t.Errorf("got %#v, want Seq=0", initEv) @@ -128,7 +131,7 @@ func TestSetBreakpoint(t *testing.T) { client.InitializeRequest() client.ExpectInitializeResponse(t) - client.LaunchRequest(fixture.Path, false /*stopOnEntry*/) + client.LaunchRequest("exec", fixture.Path, !stopOnEntry) client.ExpectInitializedEvent(t) launchResp := client.ExpectLaunchResponse(t) if launchResp.RequestSeq != 1 { @@ -172,14 +175,54 @@ func TestSetBreakpoint(t *testing.T) { }) } +// runDebugSesion is a helper for executing the standard init and shutdown +// sequences while specifying unique launch criteria via parameters. +func runDebugSession(t *testing.T, client *daptest.Client, launchRequest func()) { + client.InitializeRequest() + client.ExpectInitializeResponse(t) + + launchRequest() + client.ExpectInitializedEvent(t) + client.ExpectLaunchResponse(t) + + client.ConfigurationDoneRequest() + client.ExpectConfigurationDoneResponse(t) + + client.ExpectTerminatedEvent(t) + client.DisconnectRequest() + client.ExpectDisconnectResponse(t) +} + +func TestLaunchDebugRequest(t *testing.T) { + runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) { + // We reuse the harness that builds, but ignore the actual binary. + runDebugSession(t, client, func() { + // Use the default output directory. + client.LaunchRequestWithArgs(map[string]interface{}{ + "mode": "debug", "program": fixture.Source}) + }) + }) +} + +func TestLaunchTestRequest(t *testing.T) { + runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) { + runDebugSession(t, client, func() { + // We reuse the harness that builds, but ignore the actual binary. + fixtures := protest.FindFixturesDir() + testdir, _ := filepath.Abs(filepath.Join(fixtures, "buildtest")) + client.LaunchRequestWithArgs(map[string]interface{}{ + "mode": "test", "program": testdir, "output": "__mytestdir"}) + }) + }) +} + func TestBadLaunchRequests(t *testing.T) { runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) { - client.LaunchRequest("", true) - - expectFailedToLaunch := func(response *dap.ErrorResponse, seq int) { + seqCnt := 0 + expectFailedToLaunch := func(response *dap.ErrorResponse) { t.Helper() - if response.RequestSeq != seq { - t.Errorf("RequestSeq got %d, want %d", seq, response.RequestSeq) + if response.RequestSeq != seqCnt { + t.Errorf("RequestSeq got %d, want %d", seqCnt, response.RequestSeq) } if response.Command != "launch" { t.Errorf("Command got %q, want \"launch\"", response.Command) @@ -190,28 +233,61 @@ func TestBadLaunchRequests(t *testing.T) { if response.Body.Error.Id != 3000 { t.Errorf("Id got %d, want 3000", response.Body.Error.Id) } + seqCnt++ } - resp := client.ExpectErrorResponse(t) - expectFailedToLaunch(resp, 0) - // Test for the DAP-specific detailed error message. - wantErrorFormat := "Failed to launch: The program attribute is missing in debug configuration." - if resp.Body.Error.Format != wantErrorFormat { - t.Errorf("got %q, want %q", resp.Body.Error.Format, wantErrorFormat) + expectFailedToLaunchWithMessage := func(response *dap.ErrorResponse, errmsg string) { + t.Helper() + expectFailedToLaunch(response) + if response.Body.Error.Format != errmsg { + t.Errorf("\ngot %q\nwant %q", response.Body.Error.Format, errmsg) + } } + // Test for the DAP-specific detailed error message. + client.LaunchRequest("exec", "", stopOnEntry) + expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t), + "Failed to launch: The program attribute is missing in debug configuration.") + + client.LaunchRequestWithArgs(map[string]interface{}{"program": 12345}) + expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t), + "Failed to launch: The program attribute is missing in debug configuration.") + + client.LaunchRequestWithArgs(map[string]interface{}{"program": nil}) + expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t), + "Failed to launch: The program attribute is missing in debug configuration.") + + client.LaunchRequestWithArgs(map[string]interface{}{}) + expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t), + "Failed to launch: The program attribute is missing in debug configuration.") + + client.LaunchRequest("remote", fixture.Path, stopOnEntry) + expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t), + "Failed to launch: Unsupported 'mode' value \"remote\" in debug configuration.") + + client.LaunchRequest("notamode", fixture.Path, stopOnEntry) + expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t), + "Failed to launch: Unsupported 'mode' value \"notamode\" in debug configuration.") + + client.LaunchRequestWithArgs(map[string]interface{}{"mode": 12345, "program": fixture.Path}) + expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t), + "Failed to launch: Unsupported 'mode' value %!q(float64=12345) in debug configuration.") + // Skip detailed message checks for potentially different OS-specific errors. - client.LaunchRequest(fixture.Path+"_does_not_exist", false) - expectFailedToLaunch(client.ExpectErrorResponse(t), 1) + client.LaunchRequest("exec", fixture.Path+"_does_not_exist", stopOnEntry) + expectFailedToLaunch(client.ExpectErrorResponse(t)) - client.LaunchRequest(fixture.Source, true) // Not an executable - expectFailedToLaunch(client.ExpectErrorResponse(t), 2) + client.LaunchRequest("debug", fixture.Path+"_does_not_exist", stopOnEntry) + expectFailedToLaunch(client.ExpectErrorResponse(t)) // Build error + + client.LaunchRequest("exec", fixture.Source, stopOnEntry) + expectFailedToLaunch(client.ExpectErrorResponse(t)) // Not an executable // We failed to launch the program. Make sure shutdown still works. client.DisconnectRequest() dresp := client.ExpectDisconnectResponse(t) - if dresp.RequestSeq != 3 { - t.Errorf("got %#v, want RequestSeq=3", dresp) + if dresp.RequestSeq != seqCnt { + t.Errorf("got %#v, want RequestSeq=%d", dresp, seqCnt) } }) }