Skip to content

Commit

Permalink
service/dap: auto-loading for fully missing pointers, structs, maps, …
Browse files Browse the repository at this point in the history 
…slices and arrays
  • Loading branch information
@polinasok
polinasok committed Apr 27, 2021
1 parent 6a85f34 commit cfb4114
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 35 deletions.
93 changes: 70 additions & 23 deletions service/dap/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ var defaultArgs = launchAttachArgs{

// 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.
// With dlv-dap, users currently do not have a way to adjust these.
// TODO(polina): Support setting config via launch/attach args or only rely on on-demand loading?
var DefaultLoadConfig = proc.LoadConfig{
FollowPointers: true,
Expand Down Expand Up @@ -1331,33 +1332,86 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s
if v.Unreadable != nil {
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.
// Some of the types might be fully or partially not loaded based on LoadConfig.
// Those that are fully missing (e.g. due to hitting MaxVariableRecurse), can be reloaded in place.
// Those that are partially missing (e.g. MaxArrayValues from a large array), need a more creative solution
// that is still to be determined. For now, clearly communicate when that happens with additional value labels.
// TODO(polina): look into layered/paged loading for truncated strings, arrays, maps and structs.
var reloadVariable = func(v *proc.Variable, qualifiedNameOrExpr string) (value string) {
// We might be loading variables from the frame that's not topmost, so use
// frame-independent address-based expression, not fully-qualified name as per
// https://github.com/go-delve/delve/blob/master/Documentation/api/ClientHowto.md#looking-into-variables.
value = api.ConvertVar(v).SinglelineString()
typeName := api.PrettyTypeName(v.DwarfType)
loadExpr := fmt.Sprintf("*(*%q)(%#x)", typeName, v.Addr)
s.log.Debugf("loading %s (type %s) with %s", qualifiedNameOrExpr, typeName, loadExpr)
// Make sure we can load the pointers directly, not by updating just the child
// This is not really necessary now because users have no way of setting FollowPointers to false.
config := DefaultLoadConfig
config.FollowPointers = true
vLoaded, err := s.debugger.EvalVariableInScope(-1, 0, 0, loadExpr, config)
if err != nil {
value += fmt.Sprintf(" - FAILED TO LOAD: %s", err)
} else {
v.Children = vLoaded.Children
value = api.ConvertVar(v).SinglelineString()
}
return value
}

switch v.Kind {
case reflect.UnsafePointer:
// Skip child reference
case reflect.Ptr:
if v.DwarfType != nil && len(v.Children) > 0 && v.Children[0].Addr != 0 && v.Children[0].Kind != reflect.Invalid {
if v.Children[0].OnlyAddr {
value = "(not loaded) " + value
} else {
if v.Children[0].OnlyAddr { // Not loaded
if v.Addr == 0 {
// This is equvalent to the following with the cli:
// (dlv) p &a7
// (**main.FooBar)(0xc0000a3918)
//
// TODO(polina): what is more appropriate?
// Option 1: leave it unloaded because it is a special case
// Option 2: load it, but then we have to load the child, not the parent, unlike all others
// TODO(polina): see if reloadVariable can be reused here
cTypeName := api.PrettyTypeName(v.Children[0].DwarfType)
cLoadExpr := fmt.Sprintf("*(*%q)(%#x)", cTypeName, v.Children[0].Addr)
s.log.Debugf("loading *(%s) (type %s) with %s", qualifiedNameOrExpr, cTypeName, cLoadExpr)
cLoaded, err := s.debugger.EvalVariableInScope(-1, 0, 0, cLoadExpr, DefaultLoadConfig)
if err != nil {
value += fmt.Sprintf(" - FAILED TO LOAD: %s", err)
} else {
cLoaded.Name = v.Children[0].Name // otherwise, this will be the pointer expression
v.Children = []proc.Variable{*cLoaded}
value = api.ConvertVar(v).SinglelineString()
}
} else {
value = reloadVariable(v, qualifiedNameOrExpr)
}
}
if !v.Children[0].OnlyAddr {
variablesReference = maybeCreateVariableHandle(v)
}
}
case reflect.Slice, reflect.Array:
if v.Len > int64(len(v.Children)) { // Not fully loaded
value = fmt.Sprintf("(loaded %d/%d) ", len(v.Children), v.Len) + value
if v.Base != 0 && len(v.Children) == 0 { // Fully missing
value = reloadVariable(v, qualifiedNameOrExpr)
} else { // Partially missing (TODO)
value = fmt.Sprintf("(loaded %d/%d) ", len(v.Children), v.Len) + value
}
}
if v.Base != 0 && len(v.Children) > 0 {
variablesReference = maybeCreateVariableHandle(v)
}
case reflect.Map:
if v.Len > int64(len(v.Children)/2) { // Not fully loaded
value = fmt.Sprintf("(loaded %d/%d) ", len(v.Children)/2, v.Len) + value
if len(v.Children) == 0 { // Fully missing
value = reloadVariable(v, qualifiedNameOrExpr)
} else { // Partially missing (TODO)
value = fmt.Sprintf("(loaded %d/%d) ", len(v.Children)/2, v.Len) + value
}
}
if v.Base != 0 && len(v.Children) > 0 {
variablesReference = maybeCreateVariableHandle(v)
Expand All @@ -1366,27 +1420,20 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s
// TODO(polina): implement auto-loading here
case reflect.Interface:
if v.Addr != 0 && len(v.Children) > 0 && v.Children[0].Kind != reflect.Invalid && v.Children[0].Addr != 0 {
if v.Children[0].OnlyAddr { // Not fully loaded
// We might be loading variables from the frame that's not topmost, so use
// frame-independent address-based expression, not fully-qualified name.
loadExpr := fmt.Sprintf("*(*%q)(%#x)", typeName, v.Addr)
s.log.Debugf("loading %s (type %s) with %s", qualifiedNameOrExpr, typeName, loadExpr)
vLoaded, err := s.debugger.EvalVariableInScope(-1, 0, 0, loadExpr, DefaultLoadConfig)
if err != nil {
value += fmt.Sprintf(" - FAILED TO LOAD: %s", err)
} else {
v.Children = vLoaded.Children
value = api.ConvertVar(v).SinglelineString()
}
if v.Children[0].OnlyAddr { // Not loaded
value = reloadVariable(v, qualifiedNameOrExpr)
}
// Provide a reference to the child only if it fully loaded
if !v.Children[0].OnlyAddr {
variablesReference = maybeCreateVariableHandle(v)
}
}
case reflect.Struct:
if v.Len > int64(len(v.Children)) { // Not fully loaded
value = fmt.Sprintf("(loaded %d/%d) ", len(v.Children), v.Len) + value
if len(v.Children) == 0 { // Fully missing
value = reloadVariable(v, qualifiedNameOrExpr)
} else { // Partially missing (TODO)
value = fmt.Sprintf("(loaded %d/%d) ", len(v.Children), v.Len) + value
}
}
if len(v.Children) > 0 {
variablesReference = maybeCreateVariableHandle(v)
Expand Down
64 changes: 52 additions & 12 deletions service/dap/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1374,7 +1374,7 @@ func TestVariablesLoading(t *testing.T) {
expectChildren(t, sd, "sd", 5)
}

// Struct fully missing due to hitting LoadConfig.MaxVariableRecurse (also tests evaluateName)
// Fully missing struct auto-loaded when reaching LoadConfig.MaxVariableRecurse (also tests evaluateName corner case)
ref = expectVarRegex(t, locals, -1, "c1", "c1", `main\.cstruct {pb: \*main\.bstruct {a: \(\*main\.astruct\)\(0x[0-9a-f]+\)}, sa: []\*main\.astruct len: 3, cap: 3, [\*\(\*main\.astruct\)\(0x[0-9a-f]+\),\*\(\*main\.astruct\)\(0x[0-9a-f]+\),\*\(\*main.astruct\)\(0x[0-9a-f]+\)]}`, hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
Expand All @@ -1387,16 +1387,16 @@ func TestVariablesLoading(t *testing.T) {
expectChildren(t, c1sa, "c1.sa", 3)
ref = expectVarRegex(t, c1sa, 0, `\[0\]`, `c1\.sa\[0\]`, `\*\(\*main\.astruct\)\(0x[0-9a-f]+\)`, hasChildren)
if ref > 0 {
// Auto-loading of fully missing struc children happens here
client.VariablesRequest(ref)
c1sa0 := client.ExpectVariablesResponse(t)
expectChildren(t, c1sa0, "c1.sa[0]", 1)
// TODO(polina): there should be children here once we support auto loading
expectVarRegex(t, c1sa0, 0, "", `\(\*c1\.sa\[0\]\)`, `\(loaded 0/2\) \(\*main\.astruct\)\(0x[0-9a-f]+\)`, noChildren)
expectVarExact(t, c1sa0, 0, "", "(*c1.sa[0])", "main.astruct {A: 1, B: 2}", hasChildren)
}
}
}

// Struct fully missing due to hitting LoadConfig.MaxVariableRecurse (also tests evaluteName)
// Fully missing struct auto-loaded when hitting LoadConfig.MaxVariableRecurse (also tests evaluteName corner case)
ref = expectVarRegex(t, locals, -1, "aas", "aas", `\[\]main\.a len: 1, cap: 1, \[{aas: \[\]main\.a len: 1, cap: 1, \[\(\*main\.a\)\(0x[0-9a-f]+\)\]}\]`, hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
Expand All @@ -1409,30 +1409,44 @@ func TestVariablesLoading(t *testing.T) {
expectChildren(t, aas0, "aas[0]", 1)
ref = expectVarRegex(t, aas0, 0, "aas", `aas\[0\]\.aas`, `\[\]main\.a len: 1, cap: 1, \[\(\*main\.a\)\(0x[0-9a-f]+\)\]`, hasChildren)
if ref > 0 {
// Auto-loading of fully missing struct children happens here
client.VariablesRequest(ref)
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"
expectVarRegex(t, aas0aas, 0, "[0]", `aas\[0\]\.aas\[0\]`, `\(loaded 0/1\) \(\*main\.a\)\(0x[0-9a-f]+\)`, noChildren)
ref = expectVarRegex(t, aas0aas, 0, "[0]", `aas\[0\]\.aas\[0\]`, `main\.a {aas: \[\]main\.a len: 1, cap: 1, \[\(\*main\.a\)\(0x[0-9a-f]+\)\]}`, hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
aas0aas0 := client.ExpectVariablesResponse(t)
expectChildren(t, aas0aas, "aas[0].aas[0]", 1)
expectVarRegex(t, aas0aas0, 0, "aas", `aas\[0\]\.aas\[0\]\.aas`, `\[\]main\.a len: 1, cap: 1, \[\(\*main\.a\)\(0x[0-9a-f]+\)\]`, hasChildren)
}
}
}
}

// Map fully missing due to hitting LoadConfig.MaxVariableRecurse (also tests evaluateName)
// Fully missing map auto-loaded when hitting LoadConfig.MaxVariableRecurse (also tests evaluateName corner case)
ref = expectVarExact(t, locals, -1, "tm", "tm", "main.truncatedMap {v: []map[string]main.astruct len: 1, cap: 1, [[...]]}", hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
tm := client.ExpectVariablesResponse(t)
expectChildren(t, tm, "tm", 1)
ref = expectVarExact(t, tm, 0, "v", "tm.v", "[]map[string]main.astruct len: 1, cap: 1, [[...]]", hasChildren)
if ref > 0 {
// Auto-loading of fully missing map chidlren happens here, but they get trancated at MaxArrayValuess
client.VariablesRequest(ref)
tmV := client.ExpectVariablesResponse(t)
expectChildren(t, tmV, "tm.v", 1)
// TODO(polina): there should be children here once we support auto loading - test for "tm.v[0]["gutters"]"
expectVarExact(t, tmV, 0, "[0]", "tm.v[0]", "(loaded 0/66) map[string]main.astruct [...]", noChildren)
ref = expectVarRegex(t, tmV, 0, `\[0\]`, `tm\.v\[0\]`, `map\[string\]main\.astruct \[.+\.\.\.\+2 more\]`, hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
tmV0 := client.ExpectVariablesResponse(t)
expectChildren(t, tmV0, "tm.v[0]", 64)
}
}
}

// TODO(polina): need fully missing array/slice test case
// TODO(polina): test that 0 size slices, structs with no fields, etc are not treated as fully missing
},
disconnect: true,
}})
Expand Down Expand Up @@ -1491,9 +1505,28 @@ func TestVariablesLoading(t *testing.T) {
}
}

// Pointer value not loaded with LoadConfig.FollowPointers=false
expectVarRegex(t, locals, -1, "a7", "a7", `\(not loaded\) \(\*main\.FooBar\)\(0x[0-9a-f]+\)`, noChildren)
// Pointer values loaded even with LoadConfig.FollowPointers=false

expectVarExact(t, locals, -1, "a7", "a7", "*main.FooBar {Baz: 5, Bur: \"strum\"}", hasChildren)

// Auto-loading works on results of evaluate expressions as well
client.EvaluateRequest("a7", frame, "repl")
expectEval(t, client.ExpectEvaluateResponse(t), "*main.FooBar {Baz: 5, Bur: \"strum\"}", hasChildren)

client.EvaluateRequest("&a7", frame, "repl")
pa7 := client.ExpectEvaluateResponse(t)
ref = expectEvalRegex(t, pa7, `\*\(\*main\.FooBar\)\(0x[0-9a-f]+\)`, hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
a7 := client.ExpectVariablesResponse(t)
ref = expectVarExact(t, a7, 0, "a7", "(*(&a7))", "*main.FooBar {Baz: 5, Bur: \"strum\"}", hasChildren)
}

// TODO(polina): add a test case for call result auto-loading
}

// Frame-independent loading expressions allow us to auto-load
// variables in any frame, not just topmost.
loadvars(1000 /*first topmost frame*/)
// step into another function
client.StepInRequest(1)
Expand Down Expand Up @@ -1938,7 +1971,14 @@ 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)
expectEval(t, got, "(not loaded) (*int)(0x2)", noChildren)
ref = expectEvalRegex(t, got, `\*\(unreadable .+\)`, hasChildren)
if ref > 0 {
client.VariablesRequest(ref)
val := client.ExpectVariablesResponse(t)
expectChildren(t, val, "*(*int)(2)", 1)
expectVarRegex(t, val, 0, "^$", `\(\*\(\(\*int\)\(2\)\)\)`, `\(unreadable .+\)`, noChildren)
validateEvaluateName(t, client, val, 0)
}

// Type casts between string, []byte and []rune
client.EvaluateRequest("[]byte(\"ABC€\")", 1000, "this context will be ignored")
Expand Down

0 comments on commit cfb4114

Please sign in to comment.