PenaDevops/delve
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

service/dap: Add support for package globals to scopes/variables requests (#2160)

Browse Source 
* Support global variables

* Respond to review comments

* Clarify comment

* Add more details to test error messages

* Remove flaky main..inittask checks

* Rename globals flag to match vscode-go

* Normalize filepath with slash separator

* Improve handling for unknown package

* Tweak error message

* More refactoring, normalization and error details to deal with Win test failures

* Clean up optional launch args processing

* Add CurrentPackage to debugger and use instead of ListPackagesBuildInfo

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
This commit is contained in:
polinasok 2020-09-15 13:14:55 -07:00 committed by GitHub
parent 37d1e0100a
commit 4980fff8ce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 176 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
service/dap/server.go
View File

@ -18,6 +18,7 @@ import (
"os"
"path/filepath"
"reflect"
"strings"
"github.com/go-delve/delve/pkg/gobuild"
"github.com/go-delve/delve/pkg/logflags"
@ -72,12 +73,15 @@ type launchAttachArgs struct {
stopOnEntry bool
// stackTraceDepth is the maximum length of the returned list of stack frames.
stackTraceDepth int
// showGlobalVariables indicates if global package variables should be loaded.
showGlobalVariables bool
}
// defaultArgs borrows the defaults for the arguments from the original vscode-go adapter.
var defaultArgs = launchAttachArgs{
stopOnEntry: false,
stackTraceDepth: 50,
stopOnEntry: false,
stackTraceDepth: 50,
showGlobalVariables: false,
}
// NewServer creates a new DAP Server. It takes an opened Listener
@ -457,13 +461,19 @@ func (s *Server) onLaunchRequest(request *dap.LaunchRequest) {
return
}
stop, ok := request.Arguments["stopOnEntry"]
s.args.stopOnEntry = ok && stop == true
// If user-specified, overwrite the defaults for optional args.
stop, ok := request.Arguments["stopOnEntry"].(bool)
if ok {
s.args.stopOnEntry = stop
}
depth, ok := request.Arguments["stackTraceDepth"].(float64)
if ok && depth > 0 {
s.args.stackTraceDepth = int(depth)
}
globals, ok := request.Arguments["showGlobalVariables"].(bool)
if ok {
s.args.showGlobalVariables = globals
}
var targetArgs []string
args, ok := request.Arguments["args"]
@ -732,18 +742,51 @@ func (s *Server) onScopesRequest(request *dap.ScopesRequest) {
// Retrieve local variables
locals, err := s.debugger.LocalVariables(goid, frame, 0, cfg)
if err != nil {
s.sendErrorResponse(request.Request, UnableToListLocals, "Unable to list local vars", err.Error())
s.sendErrorResponse(request.Request, UnableToListLocals, "Unable to list locals", err.Error())
return
}
locScope := &proc.Variable{Name: "Locals", Children: slicePtrVarToSliceVar(locals)}
// TODO(polina): Annotate shadowed variables
// TODO(polina): Retrieve global variables
scopeArgs := dap.Scope{Name: argScope.Name, VariablesReference: s.variableHandles.create(argScope)}
scopeLocals := dap.Scope{Name: locScope.Name, VariablesReference: s.variableHandles.create(locScope)}
scopes := []dap.Scope{scopeArgs, scopeLocals}
if s.args.showGlobalVariables {
// Limit what global variables we will return to the current package only.
// TODO(polina): This is how vscode-go currently does it to make
// the amount of the returned data manageable. In fact, this is
// considered so expensive even with the package filter, that
// the default for showGlobalVariables was recently flipped to
// not showing. If we delay loading of the globals until the corresponding
// scope is expanded, generating an explicit variable request,
// should we consider making all globals accessible with a scope per package?
// Or users can just rely on watch variables.
currPkg, err := s.debugger.CurrentPackage()
if err != nil {
s.sendErrorResponse(request.Request, UnableToListGlobals, "Unable to list globals", err.Error())
return
}
currPkgFilter := fmt.Sprintf("^%s\\.", currPkg)
globals, err := s.debugger.PackageVariables(s.debugger.CurrentThread().ThreadID(), currPkgFilter, cfg)
if err != nil {
s.sendErrorResponse(request.Request, UnableToListGlobals, "Unable to list globals", err.Error())
return
}
// Remove package prefix from the fully-qualified variable names.
// We will include the package info once in the name of the scope instead.
for i, g := range globals {
globals[i].Name = strings.TrimPrefix(g.Name, currPkg+".")
}
globScope := &proc.Variable{
Name: fmt.Sprintf("Globals (package %s)", currPkg),
Children: slicePtrVarToSliceVar(globals),
}
scopeGlobals := dap.Scope{Name: globScope.Name, VariablesReference: s.variableHandles.create(globScope)}
scopes = append(scopes, scopeGlobals)
}
response := &dap.ScopesResponse{
Response: *newResponse(request.Request),
Body: dap.ScopesResponseBody{Scopes: scopes},

111
service/dap/server_test.go
View File

@ -341,6 +341,9 @@ func TestSetBreakpoint(t *testing.T) {
client.ScopesRequest(1000)
scopes := client.ExpectScopesResponse(t)
if len(scopes.Body.Scopes) > 2 {
t.Errorf("\ngot %#v\nwant len(Scopes)=2 (Arguments & Locals)", scopes)
}
expectScope(t, scopes, 0, "Arguments", 1000)
expectScope(t, scopes, 1, "Locals", 1001)
@ -413,12 +416,12 @@ func expectScope(t *testing.T, got *dap.ScopesResponse, i int, name string, varR
}
// expectChildren is a helper for verifying the number of variables within a VariablesResponse.
// parentName - name of the enclosing variable or scope
// parentName - pseudoname of the enclosing variable or scope (used for error message only)
// numChildren - number of variables/fields/elements of this variable
func expectChildren(t *testing.T, got *dap.VariablesResponse, parentName string, numChildren int) {
t.Helper()
if len(got.Body.Variables) != numChildren {
t.Errorf("\ngot len(%s)=%d\nwant %d", parentName, len(got.Body.Variables), numChildren)
t.Errorf("\ngot len(%s)=%d (children=%#v)\nwant len=%d", parentName, len(got.Body.Variables), got.Body.Variables, numChildren)
}
}
@ -432,7 +435,7 @@ func expectChildren(t *testing.T, got *dap.VariablesResponse, parentName string,
func expectVar(t *testing.T, got *dap.VariablesResponse, i int, name, value string, useExactMatch, hasRef bool) (ref int) {
t.Helper()
if len(got.Body.Variables) <= i {
t.Errorf("\ngot len=%d\nwant len>%d", len(got.Body.Variables), i)
t.Errorf("\ngot len=%d (children=%#v)\nwant len>%d", len(got.Body.Variables), got.Body.Variables, i)
return
}
if i < 0 {
@ -556,7 +559,9 @@ func TestScopesAndVariablesRequests(t *testing.T) {
runDebugSessionWithBPs(t, client,
// Launch
func() {
client.LaunchRequest("exec", fixture.Path, !stopOnEntry)
client.LaunchRequestWithArgs(map[string]interface{}{
"mode": "exec", "program": fixture.Path, "showGlobalVariables": true,
})
},
// Breakpoints are set within the program
fixture.Source, []int{},
@ -580,6 +585,7 @@ func TestScopesAndVariablesRequests(t *testing.T) {
scopes := client.ExpectScopesResponse(t)
expectScope(t, scopes, 0, "Arguments", 1000)
expectScope(t, scopes, 1, "Locals", 1001)
expectScope(t, scopes, 2, "Globals (package main)", 1002)
// Arguments
@ -596,6 +602,12 @@ func TestScopesAndVariablesRequests(t *testing.T) {
expectVarExact(t, bar, 1, "Bur", `"lorem"`, noChildren)
}
// Globals
client.VariablesRequest(1002)
globals := client.ExpectVariablesResponse(t)
expectVarExact(t, globals, 0, "p1", "10", noChildren)
// Locals
client.VariablesRequest(1001)
@ -775,6 +787,7 @@ func TestScopesAndVariablesRequests(t *testing.T) {
scopes := client.ExpectScopesResponse(t)
expectScope(t, scopes, 0, "Arguments", 1000)
expectScope(t, scopes, 1, "Locals", 1001)
expectScope(t, scopes, 2, "Globals (package main)", 1002)
client.ScopesRequest(1111)
erres := client.ExpectErrorResponse(t)
@ -791,6 +804,10 @@ func TestScopesAndVariablesRequests(t *testing.T) {
expectChildren(t, locals, "Locals", 1)
expectVarExact(t, locals, -1, "a1", `"bur"`, noChildren)
client.VariablesRequest(1002) // Globals
globals := client.ExpectVariablesResponse(t)
expectVarExact(t, globals, 0, "p1", "10", noChildren)
client.VariablesRequest(7777)
erres = client.ExpectErrorResponse(t)
if erres.Body.Error.Format != "Unable to lookup variable: unknown reference 7777" {
@ -835,6 +852,9 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
client.ScopesRequest(1000)
scopes := client.ExpectScopesResponse(t)
if len(scopes.Body.Scopes) > 2 {
t.Errorf("\ngot %#v\nwant len(scopes)=2 (Argumes & Locals)", scopes)
}
expectScope(t, scopes, 0, "Arguments", 1000)
expectScope(t, scopes, 1, "Locals", 1001)
@ -964,18 +984,18 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
ref = expectVarExact(t, m4, 2, "[key 1]", "<main.astruct>", hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
m4_key1 := client.ExpectVariablesResponse(t)
expectChildren(t, m4_key1, "m4_key1", 2)
expectVarExact(t, m4_key1, 0, "A", "2", noChildren)
expectVarExact(t, m4_key1, 1, "B", "2", noChildren)
m4Key1 := client.ExpectVariablesResponse(t)
expectChildren(t, m4Key1, "m4Key1", 2)
expectVarExact(t, m4Key1, 0, "A", "2", noChildren)
expectVarExact(t, m4Key1, 1, "B", "2", noChildren)
}
ref = expectVarExact(t, m4, 3, "[val 1]", "<main.astruct>", hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
m4_val1 := client.ExpectVariablesResponse(t)
expectChildren(t, m4_val1, "m4_val1", 2)
expectVarExact(t, m4_val1, 0, "A", "22", noChildren)
expectVarExact(t, m4_val1, 1, "B", "22", noChildren)
m4Val1 := client.ExpectVariablesResponse(t)
expectChildren(t, m4Val1, "m4Val1", 2)
expectVarExact(t, m4Val1, 0, "A", "22", noChildren)
expectVarExact(t, m4Val1, 1, "B", "22", noChildren)
}
}
expectVarExact(t, locals, -1, "emptymap", "<map[string]string> (length: 0)", noChildren)
@ -1020,6 +1040,73 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
})
}
// TestGlobalScopeAndVariables launches the program with showGlobalVariables
// arg set, executes to a breakpoint in the main package and tests that global
// package main variables got loaded. It then steps into a function
// in another package and tests that globals scope got updated to those vars.
func TestGlobalScopeAndVariables(t *testing.T) {
runTest(t, "consts", func(client *daptest.Client, fixture protest.Fixture) {
runDebugSessionWithBPs(t, client,
// Launch
func() {
client.LaunchRequestWithArgs(map[string]interface{}{
"mode": "exec", "program": fixture.Path, "showGlobalVariables": true,
})
},
// Breakpoints are set within the program
fixture.Source, []int{},
[]onBreakpoint{{
// Stop at line 36
execute: func() {
client.StackTraceRequest(1, 0, 20)
stack := client.ExpectStackTraceResponse(t)
expectStackFrames(t, stack, 36, 1000, 3, 3)
client.ScopesRequest(1000)
scopes := client.ExpectScopesResponse(t)
expectScope(t, scopes, 0, "Arguments", 1000)
expectScope(t, scopes, 1, "Locals", 1001)
expectScope(t, scopes, 2, "Globals (package main)", 1002)
client.VariablesRequest(1002)
client.ExpectVariablesResponse(t)
// The program has no user-defined globals.
// Depending on the Go version, there might
// be some runtime globals (e.g. main..inittask)
// so testing for the total number is too fragile.
// Step into pkg.AnotherMethod()
client.StepInRequest(1)
client.ExpectStepInResponse(t)
client.ExpectStoppedEvent(t)
client.StackTraceRequest(1, 0, 20)
stack = client.ExpectStackTraceResponse(t)
expectStackFrames(t, stack, 14, 1000, 4, 4)
client.ScopesRequest(1000)
scopes = client.ExpectScopesResponse(t)
expectScope(t, scopes, 0, "Arguments", 1000)
expectScope(t, scopes, 1, "Locals", 1001)
expectScope(t, scopes, 2, "Globals (package github.com/go-delve/delve/_fixtures/internal/dir0/pkg)", 1002)
client.VariablesRequest(1002)
globals := client.ExpectVariablesResponse(t)
expectChildren(t, globals, "Globals", 1)
ref := expectVarExact(t, globals, 0, "SomeVar", "<github.com/go-delve/delve/_fixtures/internal/dir0/pkg.SomeType>", hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
somevar := client.ExpectVariablesResponse(t)
expectChildren(t, somevar, "SomeVar", 1)
expectVarExact(t, somevar, 0, "X", "0", noChildren)
}
},
disconnect: false,
}})
})
}
// Tests that 'stackTraceDepth' from LaunchRequest is parsed and passed to
// stacktrace requests handlers.
func TestLaunchRequestWithStackTraceDepth(t *testing.T) {

27
service/debugger/debugger.go
View File

@ -1415,6 +1415,26 @@ func (d *Debugger) convertDefers(defers []*proc.Defer) []api.Defer {
return r
}
// CurrentPackage returns the fully qualified name of the
// package corresponding to the function location of the
// current thread.
func (d *Debugger) CurrentPackage() (string, error) {
d.targetMutex.Lock()
defer d.targetMutex.Unlock()
if _, err := d.target.Valid(); err != nil {
return "", err
}
loc, err := d.target.CurrentThread().Location()
if err != nil {
return "", err
}
if loc.Fn == nil {
return "", fmt.Errorf("unable to determine current package due to unspecified function location")
}
return loc.Fn.PackageName(), nil
}
// FindLocation will find the location specified by 'locStr'.
func (d *Debugger) FindLocation(goid, frame, deferredCall int, locStr string, includeNonExecutableLines bool) ([]api.Location, error) {
d.targetMutex.Lock()
@ -1490,6 +1510,13 @@ func (d *Debugger) Recorded() (recorded bool, tracedir string) {
return d.target.Recorded()
}
// CurrentThread returns the current thread.
func (d *Debugger) CurrentThread() proc.Thread {
d.targetMutex.Lock()
defer d.targetMutex.Unlock()
return d.target.CurrentThread()
}
// Checkpoint will set a checkpoint specified by the locspec.
func (d *Debugger) Checkpoint(where string) (int, error) {
d.targetMutex.Lock()