Repository: go-delve/delve
Pull request #2494 (2021-05-18 16:59:36)

dap: add sameuser check
On linux, delve RPC server allows only connections from the same user
if --only-same-user is set (true, by default). Do the same for DAP
server.

Moved the sameuser check logic to service/internal/sameuser.
Considered importing service/rpccommon from the dap server,
but when we eventually migrate to multiplex rpc and dap from one
port, I am afraid that can cause cyclic imports.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6b08b9ef..0b4e6e73 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,41 @@
 All notable changes to this project will be documented in this file.
 This project adheres to Semantic Versioning.
 
+## [1.6.1] 2021-05-18
+
+### Added
+
+- Dump command: generate core dumps from within Delve (@aarzilli)
+- Toggle command: toggle breakpoints on or off (@alexsaezm)
+- DAP server improvements (@polinasok, @hyangah, @suzmue)
+- Delve now parses and uses the .eh_frame section when available (@aarzilli)
+- Add linespec argument to 'continue' command (@icholy)
+- Add optional format argument to 'print' and 'display' commands (@aarzilli)
+
+### Fixed
+
+- Fixed file reference handling with DWARF5 compilation units (@thanm)
+- Fix embedded field searching (@aarzilli)
+- Fix off by one error reading core files (@aarzilli)
+- Correctly read G address on linux/arm64
+- Avoid double removal of temp built binary (@polinasok)
+- Fix temp binary deletion race in DAP server (@polinasok)
+- Fix shutdown related bugs in RPC server (@aarzilli)
+- Fix crashes induced by RequestManualStop (@aarzilli)
+- Fix handling of DW_OP_piece (@aarzilli)
+- Correctly truncate the result of binary operations on integers (@aarzilli)
+
+### Changed
+
+- Dropped Go 1.13 support (@aarzilli)
+- Improved documentation (@ChrisHines, @aarzilli, @becheran, @hedenface, @andreimatei, @ehershey , @hyangah)
+- Allow examinememory to use an expression (@felixge)
+- Improve gdb server check on newer ARM based macs (@oxisto)
+- CPU registers can now be used in expressions (@aarzilli)
+- DAP: Add type information to variables (@suzmue)
+- DAP: Support setting breakpoints while target is running (@polinasok)
+- DAP: Implement function breakpoints (@suzmue)
+
 ## [1.6.0] 2021-01-28
 
 ### Added
diff --git a/cmd/dlv/cmds/commands.go b/cmd/dlv/cmds/commands.go
index 3d097b26..1f0c8a8e 100644
--- a/cmd/dlv/cmds/commands.go
+++ b/cmd/dlv/cmds/commands.go
@@ -452,6 +452,7 @@ func dapCmd(cmd *cobra.Command, args []string) {
 				CheckGoVersion:       checkGoVersion,
 				TTY:                  tty,
 			},
+			CheckLocalConnUser: checkLocalConnUser,
 		})
 		defer server.Stop()
 
diff --git a/pkg/version/version.go b/pkg/version/version.go
index 8cc97519..114218c2 100644
--- a/pkg/version/version.go
+++ b/pkg/version/version.go
@@ -14,7 +14,7 @@ type Version struct {
 var (
 	// DelveVersion is the current version of Delve.
 	DelveVersion = Version{
-		Major: "1", Minor: "6", Patch: "0", Metadata: "",
+		Major: "1", Minor: "6", Patch: "1", Metadata: "",
 		Build: "$Id$",
 	}
 )
diff --git a/service/dap/server.go b/service/dap/server.go
index 2e108cd8..085d0db9 100644
--- a/service/dap/server.go
+++ b/service/dap/server.go
@@ -34,6 +34,7 @@ import (
 	"github.com/go-delve/delve/service"
 	"github.com/go-delve/delve/service/api"
 	"github.com/go-delve/delve/service/debugger"
+	"github.com/go-delve/delve/service/internal/sameuser"
 	"github.com/google/go-dap"
 	"github.com/sirupsen/logrus"
 )
@@ -320,6 +321,13 @@ func (s *Server) Run() {
 			}
 			return
 		}
+		if s.config.CheckLocalConnUser {
+			if !sameuser.CanAccept(s.listener.Addr(), conn.RemoteAddr()) {
+				s.log.Error("Error accepting client connection: Only connections from the same user that started this instance of Delve are allowed to connect. See --only-same-user.")
+				s.triggerServerStop()
+				return
+			}
+		}
 		s.mu.Lock()
 		s.conn = conn // closed in Stop()
 		s.mu.Unlock()
@@ -667,13 +675,13 @@ func (s *Server) onInitializeRequest(request *dap.InitializeRequest) {
 	response.Body.SupportsConditionalBreakpoints = true
 	response.Body.SupportsDelayedStackTraceLoading = true
 	response.Body.SupportTerminateDebuggee = true
+	response.Body.SupportsFunctionBreakpoints = true
 	response.Body.SupportsExceptionInfoRequest = true
 	// TODO(polina): support this to match vscode-go functionality
 	response.Body.SupportsSetVariable = false
 	// TODO(polina): support these requests in addition to vscode-go feature parity
 	response.Body.SupportsTerminateRequest = false
 	response.Body.SupportsRestartRequest = false
-	response.Body.SupportsFunctionBreakpoints = false
 	response.Body.SupportsStepBack = false
 	response.Body.SupportsSetExpression = false
 	response.Body.SupportsLoadedSourcesRequest = false
@@ -1020,23 +1028,10 @@ func (s *Server) onSetBreakpointsRequest(request *dap.SetBreakpointsRequest) {
 	// -- exists and in request => AmendBreakpoint
 	// -- doesn't exist and in request => SetBreakpoint
 
-	// Clear all existing breakpoints in the file.
-	existing := s.debugger.Breakpoints()
-	for _, bp := range existing {
-		// Skip special breakpoints such as for panic.
-		if bp.ID < 0 {
-			continue
-		}
-		// Skip other source files.
-		// TODO(polina): should this be normalized because of different OSes?
-		if bp.File != serverPath {
-			continue
-		}
-		_, err := s.debugger.ClearBreakpoint(bp)
-		if err != nil {
-			s.sendErrorResponse(request.Request, UnableToSetBreakpoints, "Unable to set or clear breakpoints", err.Error())
-			return
-		}
+	// Clear all existing breakpoints in the file that are not function breakpoints.
+	if err := s.clearMatchingBreakpoints(func(bp *api.Breakpoint) bool { return bp.File == serverPath && bp.Name == "" }); err != nil {
+		s.sendErrorResponse(request.Request, UnableToSetBreakpoints, "Unable to set or clear breakpoints", err.Error())
+		return
 	}
 
 	// Set all requested breakpoints.
@@ -1883,10 +1878,101 @@ func (s *Server) onRestartRequest(request *dap.RestartRequest) {
 	s.sendNotYetImplementedErrorResponse(request.Request)
 }
 
-// onSetFunctionBreakpointsRequest sends a not-yet-implemented error response.
-// Capability 'supportsFunctionBreakpoints' is not set 'initialize' response.
+// functionBpPrefix is the prefix of bp.Name for every breakpoint bp set
+// in this request.
+const functionBpPrefix = "functionBreakpoint"
+
 func (s *Server) onSetFunctionBreakpointsRequest(request *dap.SetFunctionBreakpointsRequest) {
-	s.sendNotYetImplementedErrorResponse(request.Request)
+	if s.noDebugProcess != nil {
+		s.sendErrorResponse(request.Request, UnableToSetBreakpoints, "Unable to set or clear breakpoints", "running in noDebug mode")
+		return
+	}
+	// TODO(polina): handle this while running by halting first.
+
+	// According to the spec, setFunctionBreakpoints "replaces all existing function
+	// breakpoints with new function breakpoints." The simplest way is
+	// to clear all and then set all.
+
+	// Clear all existing function breakpoints in the file.
+	// Function breakpoints are set with the Name field set to be the
+	// function name. We cannot use FunctionName to determine this, because
+	// the debugger sets the function name for all breakpoints.
+	if err := s.clearMatchingBreakpoints(func(bp *api.Breakpoint) bool { return strings.HasPrefix(bp.Name, functionBpPrefix) }); err != nil {
+		s.sendErrorResponse(request.Request, UnableToSetBreakpoints, "Unable to set or clear breakpoints", err.Error())
+		return
+	}
+
+	// Set all requested breakpoints.
+	response := &dap.SetFunctionBreakpointsResponse{Response: *newResponse(request.Request)}
+	response.Body.Breakpoints = make([]dap.Breakpoint, len(request.Arguments.Breakpoints))
+	for i, want := range request.Arguments.Breakpoints {
+		spec, err := locspec.Parse(want.Name)
+		if err != nil {
+			response.Body.Breakpoints[i].Message = err.Error()
+			continue
+		}
+		if loc, ok := spec.(*locspec.NormalLocationSpec); !ok || loc.FuncBase == nil {
+			// Other locations do not make sense in the context of function breakpoints.
+			// Regex locations are likely to resolve to multiple places and offset locations
+			// are only meaningful at the time the breakpoint was created.
+			response.Body.Breakpoints[i].Message = fmt.Sprintf("breakpoint name %q could not be parsed as a function. name must be in the format 'funcName', 'funcName:line' or 'fileName:line'.", want.Name)
+			continue
+		}
+
+		if want.Name[0] == '.' {
+			response.Body.Breakpoints[i].Message = "breakpoint names that are relative paths are not supported."
+			continue
+		}
+		// Find the location of the function name. CreateBreakpoint requires the name to include the base
+		// (e.g. main.functionName is supported but not functionName).
+		// We first find the location of the function, and then set breakpoints for that location.
+		locs, err := s.debugger.FindLocationSpec(-1, 0, 0, want.Name, spec, true, s.args.substitutePathClientToServer)
+		if err != nil {
+			response.Body.Breakpoints[i].Message = err.Error()
+			continue
+		}
+		if len(locs) == 0 {
+			response.Body.Breakpoints[i].Message = fmt.Sprintf("no location found for %q", want.Name)
+			continue
+		}
+		if len(locs) > 0 {
+			s.log.Debugf("multiple locations found for %s", want.Name)
+			response.Body.Breakpoints[i].Message = fmt.Sprintf("multiple locations found for %s, function breakpoint is only set for the first location", want.Name)
+		}
+
+		// Set breakpoint using the PCs that were found.
+		loc := locs[0]
+		got, err := s.debugger.CreateBreakpoint(
+			&api.Breakpoint{Name: fmt.Sprintf("%s%d", functionBpPrefix, i), Addr: loc.PC, Addrs: loc.PCs, Cond: want.Condition})
+		response.Body.Breakpoints[i].Verified = (err == nil)
+		if err != nil {
+			response.Body.Breakpoints[i].Message = err.Error()
+		} else {
+			response.Body.Breakpoints[i].Id = got.ID
+			response.Body.Breakpoints[i].Line = got.Line
+			response.Body.Breakpoints[i].Source = dap.Source{Name: filepath.Base(got.File), Path: got.File}
+		}
+	}
+	s.send(response)
+}
+
+func (s *Server) clearMatchingBreakpoints(matchCondition func(*api.Breakpoint) bool) error {
+	existing := s.debugger.Breakpoints()
+	for _, bp := range existing {
+		// Skip special breakpoints such as for panic.
+		if bp.ID < 0 {
+			continue
+		}
+		// Skip breakpoints that do not meet the condition.
+		if !matchCondition(bp) {
+			continue
+		}
+		_, err := s.debugger.ClearBreakpoint(bp)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 // onStepBackRequest sends a not-yet-implemented error response.
@@ -2144,6 +2230,9 @@ func (s *Server) doRunCommand(command string, asyncSetupDone chan struct{}) {
 				stopped.Body.Reason = "exception"
 				stopped.Body.Description = "Paused on panic"
 			}
+			if strings.HasPrefix(state.CurrentThread.Breakpoint.Name, functionBpPrefix) {
+				stopped.Body.Reason = "function breakpoint"
+			}
 		}
 	} else {
 		s.exceptionErr = err
diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go
index f58dc337..0aa4de22 100644
--- a/service/debugger/debugger.go
+++ b/service/debugger/debugger.go
@@ -1585,9 +1585,28 @@ func (d *Debugger) FindLocation(goid, frame, deferredCall int, locStr string, in
 		return nil, err
 	}
 
+	return d.findLocation(goid, frame, deferredCall, locStr, loc, includeNonExecutableLines, substitutePathRules)
+}
+
+// FindLocationSpec will find the location specified by 'locStr' and 'locSpec'.
+// 'locSpec' should be the result of calling 'locspec.Parse(locStr)'. 'locStr'
+// is also passed, because it made be used to broaden the search criteria, if
+// the parsed result did not find anything.
+func (d *Debugger) FindLocationSpec(goid, frame, deferredCall int, locStr string, locSpec locspec.LocationSpec, includeNonExecutableLines bool, substitutePathRules [][2]string) ([]api.Location, error) {
+	d.targetMutex.Lock()
+	defer d.targetMutex.Unlock()
+
+	if _, err := d.target.Valid(); err != nil {
+		return nil, err
+	}
+
+	return d.findLocation(goid, frame, deferredCall, locStr, locSpec, includeNonExecutableLines, substitutePathRules)
+}
+
+func (d *Debugger) findLocation(goid, frame, deferredCall int, locStr string, locSpec locspec.LocationSpec, includeNonExecutableLines bool, substitutePathRules [][2]string) ([]api.Location, error) {
 	s, _ := proc.ConvertEvalScope(d.target, goid, frame, deferredCall)
 
-	locs, err := loc.Find(d.target, d.processArgs, s, locStr, includeNonExecutableLines, substitutePathRules)
+	locs, err := locSpec.Find(d.target, d.processArgs, s, locStr, includeNonExecutableLines, substitutePathRules)
 	for i := range locs {
 		if locs[i].PC == 0 {
 			continue
diff --git a/service/internal/sameuser/doc.go b/service/internal/sameuser/doc.go
new file mode 100644
index 00000000..b6fc8fd1
--- /dev/null
+++ b/service/internal/sameuser/doc.go
@@ -0,0 +1,3 @@
+// Package sameuser provides utilities for checking users of a local connection.
+// Only works in Linux.
+package sameuser
diff --git a/service/internal/sameuser/sameuser.go b/service/internal/sameuser/sameuser.go
new file mode 100644
index 00000000..8ca5ef8b
--- /dev/null
+++ b/service/internal/sameuser/sameuser.go
@@ -0,0 +1,9 @@
+//+build !linux
+
+package sameuser
+
+import "net"
+
+func CanAccept(_, _ net.Addr) bool {
+	return true
+}
diff --git a/service/rpccommon/sameuser_linux.go b/service/internal/sameuser/sameuser_linux.go
similarity index 98%
rename from service/rpccommon/sameuser_linux.go
rename to service/internal/sameuser/sameuser_linux.go
index 4321d192..28e4755c 100644
--- a/service/rpccommon/sameuser_linux.go
+++ b/service/internal/sameuser/sameuser_linux.go
@@ -1,6 +1,6 @@
 //+build linux
 
-package rpccommon
+package sameuser
 
 import (
 	"bytes"
@@ -96,7 +96,7 @@ func sameUserForRemoteAddr(remoteAddr *net.TCPAddr) (bool, error) {
 	return sameUserForRemoteAddr4(remoteAddr)
 }
 
-func canAccept(listenAddr, remoteAddr net.Addr) bool {
+func CanAccept(listenAddr, remoteAddr net.Addr) bool {
 	laddr, ok := listenAddr.(*net.TCPAddr)
 	if !ok || !laddr.IP.IsLoopback() {
 		return true
diff --git a/service/rpccommon/sameuser.go b/service/rpccommon/sameuser.go
deleted file mode 100644
index b2d6e1cc..00000000
--- a/service/rpccommon/sameuser.go
+++ /dev/null
@@ -1,9 +0,0 @@
-//+build !linux
-
-package rpccommon
-
-import "net"
-
-func canAccept(_, _ net.Addr) bool {
-	return true
-}
diff --git a/service/rpccommon/server.go b/service/rpccommon/server.go
index affdd114..a9d8f777 100644
--- a/service/rpccommon/server.go
+++ b/service/rpccommon/server.go
@@ -20,6 +20,7 @@ import (
 	"github.com/go-delve/delve/service"
 	"github.com/go-delve/delve/service/api"
 	"github.com/go-delve/delve/service/debugger"
+	"github.com/go-delve/delve/service/internal/sameuser"
 	"github.com/go-delve/delve/service/rpc1"
 	"github.com/go-delve/delve/service/rpc2"
 	"github.com/sirupsen/logrus"
@@ -145,7 +146,7 @@ func (s *ServerImpl) Run() error {
 			}
 
 			if s.config.CheckLocalConnUser {
-				if !canAccept(s.listener.Addr(), c.RemoteAddr()) {
+				if !sameuser.CanAccept(s.listener.Addr(), c.RemoteAddr()) {
 					c.Close()
 					continue
 				}


Test patch:
diff --git a/service/dap/daptest/client.go b/service/dap/daptest/client.go
index 52221adc..fc3d7dcc 100644
--- a/service/dap/daptest/client.go
+++ b/service/dap/daptest/client.go
@@ -101,6 +101,7 @@ func (c *Client) ExpectInitializeResponseAndCapabilities(t *testing.T) *dap.Init
 		SupportsDelayedStackTraceLoading: true,
 		SupportTerminateDebuggee:         true,
 		SupportsExceptionInfoRequest:     true,
+		SupportsFunctionBreakpoints:      true,
 	}
 	if !reflect.DeepEqual(initResp.Body, wantCapabilities) {
 		t.Errorf("capabilities in initializeResponse: got %+v, want %v", pretty(initResp.Body), pretty(wantCapabilities))
@@ -331,8 +332,13 @@ func (c *Client) RestartRequest() {
 }
 
 // SetFunctionBreakpointsRequest sends a 'setFunctionBreakpoints' request.
-func (c *Client) SetFunctionBreakpointsRequest() {
-	c.send(&dap.SetFunctionBreakpointsRequest{Request: *c.newRequest("setFunctionBreakpoints")})
+func (c *Client) SetFunctionBreakpointsRequest(breakpoints []dap.FunctionBreakpoint) {
+	c.send(&dap.SetFunctionBreakpointsRequest{
+		Request: *c.newRequest("setFunctionBreakpoints"),
+		Arguments: dap.SetFunctionBreakpointsArguments{
+			Breakpoints: breakpoints,
+		},
+	})
 }
 
 // StepBackRequest sends a 'stepBack' request.
diff --git a/service/dap/server_test.go b/service/dap/server_test.go
index 6175a0af..dc66c2c8 100644
--- a/service/dap/server_test.go
+++ b/service/dap/server_test.go
@@ -1949,6 +1949,259 @@ func TestSetBreakpoint(t *testing.T) {
 	})
 }
 
+// TestSetFunctionBreakpoints is inspired by service/test.TestClientServer_FindLocations.
+func TestSetFunctionBreakpoints(t *testing.T) {
+	runTest(t, "locationsprog", func(client *daptest.Client, fixture protest.Fixture) {
+		runDebugSessionWithBPs(t, client, "launch",
+			// Launch
+			func() {
+				client.LaunchRequest("exec", fixture.Path, !stopOnEntry)
+			},
+			// Set breakpoints
+			fixture.Source, []int{30}, // b main.main
+			[]onBreakpoint{{
+				execute: func() {
+					handleStop(t, client, 1, "main.main", 30)
+
+					type Breakpoint struct {
+						line       int
+						sourceName string
+						verified   bool
+						errMsg     string
+					}
+					expectSetFunctionBreakpointsResponse := func(bps []Breakpoint) {
+						t.Helper()
+						got := client.ExpectSetFunctionBreakpointsResponse(t)
+						if len(got.Body.Breakpoints) != len(bps) {
+							t.Errorf("got %#v,\nwant len(Breakpoints)=%d", got, len(bps))
+							return
+						}
+						for i, bp := range got.Body.Breakpoints {
+							if bps[i].line < 0 {
+								if bp.Verified != bps[i].verified || !strings.Contains(bp.Message, bps[i].errMsg) {
+									t.Errorf("got breakpoints[%d] = %#v, \nwant %#v", i, bp, bps[i])
+								}
+								continue
+							}
+							// Some function breakpoints may be in packages that have been imported and we do not control, so
+							// we do not always want to check breakpoint lines.
+							if (bps[i].line >= 0 && bp.Line != bps[i].line) || bp.Verified != bps[i].verified || bp.Source.Name != bps[i].sourceName {
+								t.Errorf("got breakpoints[%d] = %#v, \nwant %#v", i, bp, bps[i])
+							}
+						}
+					}
+
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "anotherFunction"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{26, filepath.Base(fixture.Source), true, ""}})
+
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "main.anotherFunction"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{26, filepath.Base(fixture.Source), true, ""}})
+
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "SomeType.String"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{14, filepath.Base(fixture.Source), true, ""}})
+
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "(*SomeType).String"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{14, filepath.Base(fixture.Source), true, ""}})
+
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "main.SomeType.String"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{14, filepath.Base(fixture.Source), true, ""}})
+
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "main.(*SomeType).String"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{14, filepath.Base(fixture.Source), true, ""}})
+
+					// Test line offsets
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "main.anotherFunction:1"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{27, filepath.Base(fixture.Source), true, ""}})
+
+					// Test function names in imported package.
+					// Issue #275
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "io/ioutil.ReadFile"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{-1, "ioutil.go", true, ""}})
+
+					// Issue #296
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "/io/ioutil.ReadFile"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{-1, "ioutil.go", true, ""}})
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "ioutil.ReadFile"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{-1, "ioutil.go", true, ""}})
+
+					// Function Breakpoint name also accepts breakpoints that are specified as file:line.
+					// TODO(suzmue): We could return an error, but it probably is not necessary since breakpoints,
+					// and function breakpoints come in with different requests.
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: fmt.Sprintf("%s:14", fixture.Source)},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{14, filepath.Base(fixture.Source), true, ""}})
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: fmt.Sprintf("%s:14", filepath.Base(fixture.Source))},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{14, filepath.Base(fixture.Source), true, ""}})
+
+					// Expect error for ambiguous function name.
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "String"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{-1, "", false, "Location \"String\" ambiguous"}})
+
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "main.String"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{-1, "", false, "Location \"main.String\" ambiguous"}})
+
+					// Expect error for function that does not exist.
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "fakeFunction"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{-1, "", false, "location \"fakeFunction\" not found"}})
+
+					// Expect error for negative line number.
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "main.anotherFunction:-1"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{-1, "", false, "line offset negative or not a number"}})
+
+					// Expect error when function name is regex.
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: `/^.*String.*$/`},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{-1, "", false, "breakpoint name \"/^.*String.*$/\" could not be parsed as a function"}})
+
+					// Expect error when function name is an offset.
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "+1"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{-1, filepath.Base(fixture.Source), false, "breakpoint name \"+1\" could not be parsed as a function"}})
+
+					// Expect error when function name is a line number.
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "14"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{-1, filepath.Base(fixture.Source), false, "breakpoint name \"14\" could not be parsed as a function"}})
+
+					// Expect error when function name is an address.
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "*b"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{-1, filepath.Base(fixture.Source), false, "breakpoint name \"*b\" could not be parsed as a function"}})
+
+					// Expect error when function name is a relative path.
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: fmt.Sprintf(".%s%s:14", string(filepath.Separator), filepath.Base(fixture.Source))},
+					})
+					// This relative path could also be caught by the parser, so we should not match the error message.
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{-1, filepath.Base(fixture.Source), false, ""}})
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: fmt.Sprintf("..%s%s:14", string(filepath.Separator), filepath.Base(fixture.Source))},
+					})
+					// This relative path could also be caught by the parser, so we should not match the error message.
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{-1, filepath.Base(fixture.Source), false, ""}})
+
+					// Test multiple function breakpoints.
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "SomeType.String"}, {Name: "anotherFunction"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{14, filepath.Base(fixture.Source), true, ""}, {26, filepath.Base(fixture.Source), true, ""}})
+
+					// Test multiple breakpoints to the same location.
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "SomeType.String"}, {Name: "(*SomeType).String"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{14, filepath.Base(fixture.Source), true, ""}, {-1, "", false, "Breakpoint exists"}})
+
+					// Set two breakpoints at SomeType.String and SomeType.SomeFunction.
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "SomeType.String"}, {Name: "SomeType.SomeFunction"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{14, filepath.Base(fixture.Source), true, ""}, {22, filepath.Base(fixture.Source), true, ""}})
+
+					// Clear SomeType.String, reset SomeType.SomeFunction (SomeType.String is called before SomeType.SomeFunction).
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "SomeType.SomeFunction"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{22, filepath.Base(fixture.Source), true, ""}})
+
+					// Expect the next breakpoint to be at SomeType.SomeFunction.
+					client.ContinueRequest(1)
+					client.ExpectContinueResponse(t)
+
+					if se := client.ExpectStoppedEvent(t); se.Body.Reason != "function breakpoint" || se.Body.ThreadId != 1 {
+						t.Errorf("got %#v, want Reason=\"function breakpoint\", ThreadId=1", se)
+					}
+					handleStop(t, client, 1, "main.(*SomeType).SomeFunction", 22)
+
+					// Set a breakpoint at the next line in the program.
+					client.SetBreakpointsRequest(fixture.Source, []int{23})
+					got := client.ExpectSetBreakpointsResponse(t)
+					if len(got.Body.Breakpoints) != 1 {
+						t.Errorf("got %#v,\nwant len(Breakpoints)=%d", got, 1)
+						return
+					}
+					bp := got.Body.Breakpoints[0]
+					if bp.Line != 23 || bp.Verified != true || bp.Source.Path != fixture.Source {
+						t.Errorf("got breakpoints[0] = %#v, \nwant Line=23 Verified=true Source.Path=%q", bp, fixture.Source)
+					}
+
+					// Set a function breakpoint, this should not clear the breakpoint that was set in the previous setBreakpoints request.
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{
+						{Name: "anotherFunction"},
+					})
+					expectSetFunctionBreakpointsResponse([]Breakpoint{{26, filepath.Base(fixture.Source), true, ""}})
+
+					// Expect the next breakpoint to be at line 23.
+					client.ContinueRequest(1)
+					client.ExpectContinueResponse(t)
+
+					if se := client.ExpectStoppedEvent(t); se.Body.Reason != "breakpoint" || se.Body.ThreadId != 1 {
+						t.Errorf("got %#v, want Reason=\"breakpoint\", ThreadId=1", se)
+					}
+					handleStop(t, client, 1, "main.(*SomeType).SomeFunction", 23)
+
+					// Set a breakpoint, this should not clear the breakpoint that was set in the previous setFunctionBreakpoints request.
+					client.SetBreakpointsRequest(fixture.Source, []int{37})
+					got = client.ExpectSetBreakpointsResponse(t)
+					if len(got.Body.Breakpoints) != 1 {
+						t.Errorf("got %#v,\nwant len(Breakpoints)=%d", got, 1)
+						return
+					}
+					bp = got.Body.Breakpoints[0]
+					if bp.Line != 37 || bp.Verified != true || bp.Source.Path != fixture.Source {
+						t.Errorf("got breakpoints[0] = %#v, \nwant Line=23 Verified=true Source.Path=%q", bp, fixture.Source)
+					}
+
+					// Expect the next breakpoint to be at line anotherFunction.
+					client.ContinueRequest(1)
+					client.ExpectContinueResponse(t)
+
+					if se := client.ExpectStoppedEvent(t); se.Body.Reason != "function breakpoint" || se.Body.ThreadId != 1 {
+						t.Errorf("got %#v, want Reason=\"function breakpoint\", ThreadId=1", se)
+					}
+					handleStop(t, client, 1, "main.anotherFunction", 26)
+
+				},
+				disconnect: true,
+			}})
+	})
+}
+
 func expectSetBreakpointsResponseAndStoppedEvent(t *testing.T, client *daptest.Client) (se *dap.StoppedEvent, br *dap.SetBreakpointsResponse) {
 	for i := 0; i < 2; i++ {
 		switch m := client.ExpectMessage(t).(type) {
@@ -2499,7 +2752,7 @@ func TestNextAndStep(t *testing.T) {
 					client.ExpectNextResponse(t)
 					client.ExpectContinuedEvent(t)
 					if se := client.ExpectStoppedEvent(t); se.Body.Reason != "error" || se.Body.Text != "unknown goroutine -1000" {
-						t.Errorf("got %#v, want Reaspon=\"error\", Text=\"unknown goroutine -1000\"", se)
+						t.Errorf("got %#v, want Reason=\"error\", Text=\"unknown goroutine -1000\"", se)
 					}
 					handleStop(t, client, 1, "main.inlineThis", 5)
 				},
@@ -3287,9 +3540,6 @@ func TestOptionalNotYetImplementedResponses(t *testing.T) {
 		client.RestartRequest()
 		expectNotYetImplemented("restart")
 
-		client.SetFunctionBreakpointsRequest()
-		expectNotYetImplemented("setFunctionBreakpoints")
-
 		client.StepBackRequest()
 		expectNotYetImplemented("stepBack")
 
diff --git a/service/rpccommon/sameuser_linux_test.go b/service/internal/sameuser/sameuser_linux_test.go
similarity index 99%
rename from service/rpccommon/sameuser_linux_test.go
rename to service/internal/sameuser/sameuser_linux_test.go
index 9fd2bd4a..8cb185a7 100644
--- a/service/rpccommon/sameuser_linux_test.go
+++ b/service/internal/sameuser/sameuser_linux_test.go
@@ -1,6 +1,6 @@
 //+build linux
 
-package rpccommon
+package sameuser
 
 import (
 	"net"