service/dap: detect and report unloaded variables (#2353)

* Detect and report unloaded variables

* Make DeepSource happy

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
This commit is contained in:
polinasok 2021-02-23 08:29:06 -08:00 committed by GitHub
parent 946f9d7bd4
commit 129a9fe46c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 210 additions and 52 deletions

@ -357,6 +357,9 @@ func main() {
fmt.Println(amb1)
}
longarr := [100]int{}
longslice := make([]int, 100, 100)
runtime.Breakpoint()
fmt.Println(i1, i2, i3, p1, pp1, amb1, s1, s3, a0, a1, p2, p3, s2, as1, str1, f1, fn1, fn2, nilslice, nilptr, ch1, chnil, m1, mnil, m2, m3, m4, upnil, up1, i4, i5, i6, err1, err2, errnil, iface1, iface2, ifacenil, arr1, parr, cpx1, const1, iface3, iface4, recursive1, recursive1.x, iface5, iface2fn1, iface2fn2, bencharr, benchparr, mapinf, mainMenu, b, b2, sd, anonstruct1, anonstruct2, anoniface1, anonfunc, mapanonstruct1, ifacearr, efacearr, ni8, ni16, ni32, ni64, pinf, ninf, nan, zsvmap, zsslice, zsvar, tm, errtypednil, emptyslice, emptymap, byteslice, runeslice, bytearray, runearray, longstr, nilstruct, as2, as2.NonPointerRecieverMethod, s4, iface2map, issue1578, ll, unread, w2, w3, w4, w5)
fmt.Println(i1, i2, i3, p1, pp1, amb1, s1, s3, a0, a1, p2, p3, s2, as1, str1, f1, fn1, fn2, nilslice, nilptr, ch1, chnil, m1, mnil, m2, m3, m4, upnil, up1, i4, i5, i6, err1, err2, errnil, iface1, iface2, ifacenil, arr1, parr, cpx1, const1, iface3, iface4, recursive1, recursive1.x, iface5, iface2fn1, iface2fn2, bencharr, benchparr, mapinf, mainMenu, b, b2, sd, anonstruct1, anonstruct2, anoniface1, anonfunc, mapanonstruct1, ifacearr, efacearr, ni8, ni16, ni32, ni64, pinf, ninf, nan, zsvmap, zsslice, zsvar, tm, errtypednil, emptyslice, emptymap, byteslice, runeslice, bytearray, runearray, longstr, nilstruct, as2, as2.NonPointerRecieverMethod, s4, iface2map, issue1578, ll, unread, w2, w3, w4, w5, longarr, longslice)
}

@ -87,6 +87,17 @@ var defaultArgs = launchAttachArgs{
showGlobalVariables: false,
}
// DefaultLoadConfig controls how variables are loaded from the target's memory, borrowing the
// default value from the original vscode-go debug adapter and rpc server.
// TODO(polina): Support setting config via launch/attach args or only rely on on-demand loading?
var DefaultLoadConfig = proc.LoadConfig{
FollowPointers: true,
MaxVariableRecurse: 1,
MaxStringLen: 64,
MaxArrayValues: 64,
MaxStructFields: -1,
}
// NewServer creates a new DAP Server. It takes an opened Listener
// via config and assumes its ownership. config.disconnectChan has to be set;
// it will be closed by the server when the client disconnects or requests
@ -825,11 +836,9 @@ func (s *Server) onScopesRequest(request *dap.ScopesRequest) {
goid := sf.(stackFrame).goroutineID
frame := sf.(stackFrame).frameIndex
// TODO(polina): Support setting config via launch/attach args
cfg := proc.LoadConfig{FollowPointers: true, MaxVariableRecurse: 1, MaxStringLen: 64, MaxArrayValues: 64, MaxStructFields: -1}
// Retrieve arguments
args, err := s.debugger.FunctionArguments(goid, frame, 0, cfg)
args, err := s.debugger.FunctionArguments(goid, frame, 0, DefaultLoadConfig)
if err != nil {
s.sendErrorResponse(request.Request, UnableToListArgs, "Unable to list args", err.Error())
return
@ -837,7 +846,7 @@ func (s *Server) onScopesRequest(request *dap.ScopesRequest) {
argScope := &fullyQualifiedVariable{&proc.Variable{Name: "Arguments", Children: slicePtrVarToSliceVar(args)}, "", true}
// Retrieve local variables
locals, err := s.debugger.LocalVariables(goid, frame, 0, cfg)
locals, err := s.debugger.LocalVariables(goid, frame, 0, DefaultLoadConfig)
if err != nil {
s.sendErrorResponse(request.Request, UnableToListLocals, "Unable to list locals", err.Error())
return
@ -866,7 +875,7 @@ func (s *Server) onScopesRequest(request *dap.ScopesRequest) {
return
}
currPkgFilter := fmt.Sprintf("^%s\\.", currPkg)
globals, err := s.debugger.PackageVariables(currPkgFilter, cfg)
globals, err := s.debugger.PackageVariables(currPkgFilter, DefaultLoadConfig)
if err != nil {
s.sendErrorResponse(request.Request, UnableToListGlobals, "Unable to list globals", err.Error())
return
@ -908,8 +917,6 @@ func (s *Server) onVariablesRequest(request *dap.VariablesRequest) {
return
}
children := make([]dap.Variable, 0)
// TODO(polina): check and handle if variable loaded incompletely
// https://github.com/go-delve/delve/blob/master/Documentation/api/ClientHowto.md#looking-into-variables
switch v.Kind {
case reflect.Map:
@ -1045,6 +1052,11 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s
return
}
typeName := api.PrettyTypeName(v.DwarfType)
// As per https://github.com/go-delve/delve/blob/master/Documentation/api/ClientHowto.md#looking-into-variables,
// some of the types might be fully or partially not loaded based on LoadConfig. For now, clearly
// communicate when values are not fully loaded. TODO(polina): look into loading on demand.
switch v.Kind {
case reflect.UnsafePointer:
if len(v.Children) == 0 {
@ -1061,10 +1073,20 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s
value = "void"
} else {
value = fmt.Sprintf("<%s>(%#x)", typeName, v.Children[0].Addr)
variablesReference = maybeCreateVariableHandle(v)
if v.Children[0].OnlyAddr { // Not fully loaded
value += " (value not loaded)"
// Since child is not fully loaded, don't provide a reference to view it.
} else {
value = fmt.Sprintf("<%s>(%#x)", typeName, v.Children[0].Addr)
variablesReference = maybeCreateVariableHandle(v)
}
}
case reflect.Array:
value = "<" + typeName + ">"
if v.Len > int64(len(v.Children)) { // Not fully loaded
value = fmt.Sprintf("<%s> (length: %d, loaded: %d)", typeName, v.Len, len(v.Children))
} else {
value = fmt.Sprintf("<%s>", typeName)
}
if len(v.Children) > 0 {
variablesReference = maybeCreateVariableHandle(v)
}
@ -1072,7 +1094,11 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s
if v.Base == 0 {
value = "nil <" + typeName + ">"
} else {
value = fmt.Sprintf("<%s> (length: %d, cap: %d)", typeName, v.Len, v.Cap)
if v.Len > int64(len(v.Children)) { // Not fully loaded
value = fmt.Sprintf("<%s> (length: %d, cap: %d, loaded: %d)", typeName, v.Len, v.Cap, len(v.Children))
} else {
value = fmt.Sprintf("<%s> (length: %d, cap: %d)", typeName, v.Len, v.Cap)
}
if len(v.Children) > 0 {
variablesReference = maybeCreateVariableHandle(v)
}
@ -1081,7 +1107,11 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s
if v.Base == 0 {
value = "nil <" + typeName + ">"
} else {
value = fmt.Sprintf("<%s> (length: %d)", typeName, v.Len)
if v.Len > int64(len(v.Children)/2) { // Not fully loaded
value = fmt.Sprintf("<%s> (length: %d, loaded: %d)", typeName, v.Len, len(v.Children)/2)
} else {
value = fmt.Sprintf("<%s> (length: %d)", typeName, v.Len)
}
if len(v.Children) > 0 {
variablesReference = maybeCreateVariableHandle(v)
}
@ -1089,7 +1119,7 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s
case reflect.String:
vvalue := constant.StringVal(v.Value)
lenNotLoaded := v.Len - int64(len(vvalue))
if lenNotLoaded > 0 {
if lenNotLoaded > 0 { // Not fully loaded
vvalue += fmt.Sprintf("...+%d more", lenNotLoaded)
}
value = fmt.Sprintf("%q", vvalue)
@ -1111,21 +1141,35 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s
} else if len(v.Children) == 0 || v.Children[0].Kind == reflect.Invalid && v.Children[0].Addr == 0 {
value = "nil <" + typeName + ">"
} else {
value = "<" + typeName + "(" + v.Children[0].TypeString() + ")" + ">"
// TODO(polina): should we remove one level of indirection and skip "data"?
// Then we will have:
// Before:
// i: <interface{}(int)>
// data: 123
// After:
// i: <interface{}(int)> 123
// Before:
// i: <interface{}(main.MyStruct)>
// data: <main.MyStruct>
// field1: ...
// After:
// i: <interface{}(main.MyStruct)>
// field1: ...
if v.Children[0].OnlyAddr { // Not fully loaded
value = "<" + typeName + "(" + v.Children[0].TypeString() + ")" + ">" + " (data not loaded)"
// Since child is not fully loaded, don't provide a reference to view it.
} else {
value = "<" + typeName + "(" + v.Children[0].TypeString() + ")" + ">"
// TODO(polina): should we remove one level of indirection and skip "data"?
// Then we will have:
// Before:
// i: <interface{}(int)>
// data: 123
// After:
// i: <interface{}(int)> 123
// Before:
// i: <interface{}(main.MyStruct)>
// data: <main.MyStruct>
// field1: ...
// After:
// i: <interface{}(main.MyStruct)>
// field1: ...
variablesReference = maybeCreateVariableHandle(v)
}
}
case reflect.Struct:
if v.Len > int64(len(v.Children)) { // Not fully loaded
value = fmt.Sprintf("<%s> (fields: %d, loaded: %d)", typeName, v.Len, len(v.Children))
} else {
value = fmt.Sprintf("<%s>", typeName)
}
if len(v.Children) > 0 {
variablesReference = maybeCreateVariableHandle(v)
}
case reflect.Complex64, reflect.Complex128:
@ -1142,7 +1186,7 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s
v.Children[1].Kind = reflect.Float64
}
fallthrough
default: // Struct, complex, scalar
default: // complex, scalar
vvalue := api.VariableValueAsString(v)
if vvalue != "" {
value = vvalue
@ -1177,9 +1221,6 @@ func (s *Server) onEvaluateRequest(request *dap.EvaluateRequest) {
goid = sf.(stackFrame).goroutineID
frame = sf.(stackFrame).frameIndex
}
// TODO(polina): Support config settings via launch/attach args vs auto-loading?
apiCfg := &api.LoadConfig{FollowPointers: true, MaxVariableRecurse: 1, MaxStringLen: 64, MaxArrayValues: 64, MaxStructFields: -1}
prcCfg := proc.LoadConfig{FollowPointers: true, MaxVariableRecurse: 1, MaxStringLen: 64, MaxArrayValues: 64, MaxStructFields: -1}
response := &dap.EvaluateResponse{Response: *newResponse(request.Request)}
isCall, err := regexp.MatchString(`^\s*call\s+\S+`, request.Arguments.Expression)
@ -1201,7 +1242,7 @@ func (s *Server) onEvaluateRequest(request *dap.EvaluateRequest) {
}
state, err := s.debugger.Command(&api.DebuggerCommand{
Name: api.Call,
ReturnInfoLoadConfig: apiCfg,
ReturnInfoLoadConfig: api.LoadConfigFromProc(&DefaultLoadConfig),
Expr: strings.Replace(request.Arguments.Expression, "call ", "", 1),
UnsafeCall: false,
GoroutineID: goid,
@ -1224,7 +1265,7 @@ func (s *Server) onEvaluateRequest(request *dap.EvaluateRequest) {
if t.GoroutineID == stateBeforeCall.SelectedGoroutine.ID &&
t.Line == stateBeforeCall.SelectedGoroutine.CurrentLoc.Line && t.CallReturn {
// The call completed. Get the return values.
retVars, err = s.debugger.FindThreadReturnValues(t.ID, prcCfg)
retVars, err = s.debugger.FindThreadReturnValues(t.ID, DefaultLoadConfig)
if err != nil {
s.sendErrorResponseWithOpts(request.Request, UnableToEvaluateExpression, "Unable to evaluate expression", err.Error(), showErrorToUser)
return
@ -1264,7 +1305,7 @@ func (s *Server) onEvaluateRequest(request *dap.EvaluateRequest) {
}
}
} else { // {expression}
exprVar, err := s.debugger.EvalVariableInScope(goid, frame, 0, request.Arguments.Expression, prcCfg)
exprVar, err := s.debugger.EvalVariableInScope(goid, frame, 0, request.Arguments.Expression, DefaultLoadConfig)
if err != nil {
s.sendErrorResponseWithOpts(request.Request, UnableToEvaluateExpression, "Unable to evaluate expression", err.Error(), showErrorToUser)
return

@ -1022,7 +1022,6 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
// Breakpoints are set within the program
fixture.Source, []int{},
[]onBreakpoint{{
// Stop at line 317
execute: func() {
client.StackTraceRequest(1, 0, 20)
stack := client.ExpectStackTraceResponse(t)
@ -1035,7 +1034,6 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
},
disconnect: false,
}, {
// Stop at line 322
execute: func() {
client.StackTraceRequest(1, 0, 20)
stack := client.ExpectStackTraceResponse(t)
@ -1275,15 +1273,77 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
expectVarRegex(t, val, 0, "^$", `\(\*unread\)`, "unreadable <.+>", noChildren)
validateEvaluateName(t, client, val, 0)
}
},
disconnect: true,
}})
})
}
// Test that variables are not yet loaded completely.
ref = expectVarExact(t, locals, -1, "m1", "m1", "<map[string]main.astruct> (length: 66)", hasChildren)
// TestVariablesLoading exposes test cases where variables might be partiall or
// fully unloaded.
func TestVariablesLoading(t *testing.T) {
runTest(t, "testvariables2", func(client *daptest.Client, fixture protest.Fixture) {
runDebugSessionWithBPs(t, client, "launch",
// Launch
func() {
client.LaunchRequest("exec", fixture.Path, !stopOnEntry)
},
// Breakpoints are set within the program
fixture.Source, []int{},
[]onBreakpoint{{
execute: func() {},
disconnect: false,
}, {
execute: func() {
// Change default config values to trigger certain unloaded corner cases
DefaultLoadConfig.MaxStructFields = 5
defer func() { DefaultLoadConfig.MaxStructFields = -1 }()
client.StackTraceRequest(1, 0, 0)
client.ExpectStackTraceResponse(t)
client.ScopesRequest(1000)
client.ExpectScopesResponse(t)
client.VariablesRequest(1001) // Locals
locals := client.ExpectVariablesResponse(t)
// String partially missing based on LoadConfig.MaxStringLen
expectVarExact(t, locals, -1, "longstr", "longstr", "\"very long string 0123456789a0123456789b0123456789c0123456789d012...+73 more\"", noChildren)
// Array partially missing based on LoadConfig.MaxArrayValues
ref := expectVarExact(t, locals, -1, "longarr", "longarr", "<[100]int> (length: 100, loaded: 64)", hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
longarr := client.ExpectVariablesResponse(t)
expectChildren(t, longarr, "longarr", 64)
}
// Slice partially missing based on LoadConfig.MaxArrayValues
ref = expectVarExact(t, locals, -1, "longslice", "longslice", "<[]int> (length: 100, cap: 100, loaded: 64)", hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
longarr := client.ExpectVariablesResponse(t)
expectChildren(t, longarr, "longslice", 64)
}
// Map partially missing based on LoadConfig.MaxArrayValues
ref = expectVarExact(t, locals, -1, "m1", "m1", "<map[string]main.astruct> (length: 66, loaded: 64)", hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
m1 := client.ExpectVariablesResponse(t)
expectChildren(t, m1, "m1", 64) // TODO(polina): should be 66.
expectChildren(t, m1, "m1", 64)
}
// Struct partially missing based on LoadConfig.MaxStructFields
ref = expectVarExact(t, locals, -1, "sd", "sd", "<main.D> (fields: 6, loaded: 5)", hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
sd := client.ExpectVariablesResponse(t)
expectChildren(t, sd, "sd", 5)
}
// Struct fully missing due to hitting LoadConfig.MaxVariableRecurse (also tests evaluateName)
ref = expectVarExact(t, locals, -1, "c1", "c1", "<main.cstruct>", hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
@ -1299,12 +1359,13 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
client.VariablesRequest(ref)
c1sa0 := client.ExpectVariablesResponse(t)
expectChildren(t, c1sa0, "c1.sa[0]", 1)
// TODO(polina): there should be a child here once we support auto loading
expectVarExact(t, c1sa0, 0, "", "(*c1.sa[0])", "<main.astruct>", noChildren)
// TODO(polina): there should be children here once we support auto loading
expectVarExact(t, c1sa0, 0, "", "(*c1.sa[0])", "<main.astruct> (fields: 2, loaded: 0)", noChildren)
}
}
}
// Struct fully missing due to hitting LoadConfig.MaxVariableRecurse (also tests evaluteName)
ref = expectVarExact(t, locals, -1, "aas", "aas", "<[]main.a> (length: 1, cap: 1)", hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
@ -1321,11 +1382,12 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
aas0aas := client.ExpectVariablesResponse(t)
expectChildren(t, aas0aas, "aas[0].aas", 1)
// TODO(polina): there should be a child here once we support auto loading - test for "aas[0].aas[0].aas"
expectVarExact(t, aas0aas, 0, "[0]", "aas[0].aas[0]", "<main.a>", noChildren)
expectVarExact(t, aas0aas, 0, "[0]", "aas[0].aas[0]", "<main.a> (fields: 1, loaded: 0)", noChildren)
}
}
}
// Map fully missing due to hitting LoadConfig.MaxVariableRecurse (also tests evaluateName)
ref = expectVarExact(t, locals, -1, "tm", "tm", "<main.truncatedMap>", hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
@ -1337,9 +1399,62 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
tm_v := client.ExpectVariablesResponse(t)
expectChildren(t, tm_v, "tm.v", 1)
// TODO(polina): there should be children here once we support auto loading - test for "tm.v[0]["gutters"]"
expectVarExact(t, tm_v, 0, "[0]", "tm.v[0]", "<map[string]main.astruct> (length: 66)", noChildren)
expectVarExact(t, tm_v, 0, "[0]", "tm.v[0]", "<map[string]main.astruct> (length: 66, loaded: 0)", noChildren)
}
}
// Interface fully missing due to hitting LoadConfig.MaxVariableRecurse
ref = expectVarRegex(t, locals, -1, "ptrinf", "ptrinf", `<\*interface {}>\(0x[0-9a-f]+\)`, hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
ptrinf := client.ExpectVariablesResponse(t)
ref = expectVarExact(t, ptrinf, 0, "", "(*ptrinf)", "<interface {}(**interface {})>", hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
ptrinfVal := client.ExpectVariablesResponse(t)
ref = expectVarRegex(t, ptrinfVal, 0, "", `\(\*ptrinf\)\.\(data\)`, `<\*\*interface {}>\(0x[0-9a-f]+\)`, hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
ptrinfValData := client.ExpectVariablesResponse(t)
ref = expectVarRegex(t, ptrinfValData, 0, "", `\(\*\(\*ptrinf\)\.\(data\)\)`, `<\*interface {}>\(0x[0-9a-f]+\)`, hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
iface6dataValVal := client.ExpectVariablesResponse(t)
expectVarExact(t, iface6dataValVal, 0, "", "(*(*(*ptrinf).(data)))", "<interface {}(**interface {})> (data not loaded)", noChildren)
}
}
}
}
// Pointer fully missing - see below
},
disconnect: true,
}})
})
runTest(t, "testvariables2", func(client *daptest.Client, fixture protest.Fixture) {
runDebugSessionWithBPs(t, client, "launch",
// Launch
func() {
client.LaunchRequest("exec", fixture.Path, !stopOnEntry)
},
// Breakpoints are set within the program
fixture.Source, []int{},
[]onBreakpoint{{
execute: func() {
DefaultLoadConfig.FollowPointers = false
defer func() { DefaultLoadConfig.FollowPointers = true }()
client.StackTraceRequest(1, 0, 0)
client.ExpectStackTraceResponse(t)
client.ScopesRequest(1000)
client.ExpectScopesResponse(t)
client.VariablesRequest(1001) // Locals
locals := client.ExpectVariablesResponse(t)
// Pointer value not loaded
expectVarRegex(t, locals, -1, "ptrinf", "ptrinf", `<\*interface {}>\(0x[0-9a-f]+\) \(value not loaded\)`, noChildren)
},
disconnect: true,
}})
@ -1580,6 +1695,11 @@ func TestEvaluateRequest(t *testing.T) {
validateEvaluateName(t, client, a5, 4)
}
// Variable lookup that's not fully loaded
client.EvaluateRequest("ba", 1000, "this context will be ignored")
got = client.ExpectEvaluateResponse(t)
expectEval(t, got, "<[]int> (length: 200, cap: 200, loaded: 64)", hasChildren)
// All (binary and unary) on basic types except <-, ++ and --
client.EvaluateRequest("1+1", 1000, "this context will be ignored")
got = client.ExpectEvaluateResponse(t)
@ -1598,14 +1718,8 @@ func TestEvaluateRequest(t *testing.T) {
// Type casts of integer constants into any pointer type and vice versa
client.EvaluateRequest("(*int)(2)", 1000, "this context will be ignored")
got = client.ExpectEvaluateResponse(t)
ref = expectEval(t, got, "<*int>(0x2)", hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
expr := client.ExpectVariablesResponse(t)
expectChildren(t, expr, "(*int)(2)", 1)
// TODO(polina): should this be printed as (unknown int) instead?
expectVarExact(t, expr, 0, "", "(*((*int)(2)))", "<int>", noChildren)
}
expectEval(t, got, "<*int>(0x2) (value not loaded)", noChildren)
// Type casts between string, []byte and []rune
client.EvaluateRequest("[]byte(\"ABC€\")", 1000, "this context will be ignored")
got = client.ExpectEvaluateResponse(t)