service/dap: Add support for debug and test modes (#1901)

* service/dap: Add support for debug and test modes

* Address code review comments

* Remove //dap comment

* OptFlags() => optfalgs()

* If mode => switch mode

_fixtures/buildtest/main_test.go

@@ -0,0 +1,10 @@
+package main
+
+import (
+	"os"
+	"testing"
+)
+
+func TestMain(m *testing.M) {
+	os.Exit(m.Run())
+}

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()
-}

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()
+}

service/dap/daptest/client.go

@@ -136,18 +136,27 @@ func (c *Client) InitializeRequest() {
 	c.send(request)
 }
 
-// LaunchRequest sends a 'launch' request.
+// LaunchRequest sends a 'launch' request with the specified args.
-func (c *Client) LaunchRequest(program string, stopOnEntry bool) {
+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")}

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,

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)
+		seqCnt := 0
-
+		expectFailedToLaunch := func(response *dap.ErrorResponse) {
-		expectFailedToLaunch := func(response *dap.ErrorResponse, seq int) {
 			t.Helper()
-			if response.RequestSeq != seq {
+			if response.RequestSeq != seqCnt {
-				t.Errorf("RequestSeq got %d, want %d", seq, response.RequestSeq)
+				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)
+		expectFailedToLaunchWithMessage := func(response *dap.ErrorResponse, errmsg string) {
-		expectFailedToLaunch(resp, 0)
+			t.Helper()
-		// Test for the DAP-specific detailed error message.
+			expectFailedToLaunch(response)
-		wantErrorFormat := "Failed to launch: The program attribute is missing in debug configuration."
+			if response.Body.Error.Format != errmsg {
-		if resp.Body.Error.Format != wantErrorFormat {
+				t.Errorf("\ngot  %q\nwant %q", response.Body.Error.Format, errmsg)
-			t.Errorf("got %q, want %q", resp.Body.Error.Format, wantErrorFormat)
 			}
+		}
+
+		// 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)
+		client.LaunchRequest("exec", fixture.Path+"_does_not_exist", stopOnEntry)
-		expectFailedToLaunch(client.ExpectErrorResponse(t), 1)
+		expectFailedToLaunch(client.ExpectErrorResponse(t))
 
-		client.LaunchRequest(fixture.Source, true) // Not an executable
+		client.LaunchRequest("debug", fixture.Path+"_does_not_exist", stopOnEntry)
-		expectFailedToLaunch(client.ExpectErrorResponse(t), 2)
+		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 {
+		if dresp.RequestSeq != seqCnt {
-			t.Errorf("got %#v, want RequestSeq=3", dresp)
+			t.Errorf("got %#v, want RequestSeq=%d", dresp, seqCnt)
 		}
 	})
 }