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

service/dap: increase default string loading limit and refactor (#2546)

Browse Source 
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
This commit is contained in:
polinasok 2021-06-22 08:14:47 -07:00 committed by GitHub
parent 544a803a80
commit 0a19c78021
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 125 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
_fixtures/longstrings.go Normal file
View File

@ -0,0 +1,22 @@
package main
import (
"runtime"
)
func buildString(length int) string {
s := ""
for i := 0; i < length; i++ {
s = s + "x"
}
return s
}
func main() {
s513 := buildString(513)
s1025 := buildString(1025)
s4097 := buildString(4097)
nested := map[int]string{513: s513, 1025: s1025, 4097: s4097}
runtime.Breakpoint()
_ = nested
}

2
_fixtures/testvariables2.go
View File

@ -329,7 +329,7 @@ func main() {
longstr := "very long string 0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g012345678h90123456789i0123456789j0123456789" longstr := "very long string 0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g012345678h90123456789i0123456789j0123456789"
m5 := map[C]int{{longstr}: 1} m5 := map[C]int{{longstr}: 1}
m6 := map[string]int{longstr: 123} m6 := map[string]int{longstr: 123}
m7 := map[C]C{{longstr}: {longstr}} m7 := map[C]C{{longstr}: {"hello"}}
cl := C{s: longstr} cl := C{s: longstr}
var nilstruct *astruct = nil var nilstruct *astruct = nil

48
service/dap/server.go
View File

@ -165,18 +165,31 @@ type dapClientCapabilites struct {
supportsProgressReporting bool supportsProgressReporting bool
} }
// DefaultLoadConfig controls how variables are loaded from the target's memory, borrowing the // DefaultLoadConfig controls how variables are loaded from the target's memory.
// default value from the original vscode-go debug adapter and rpc server. // These limits are conservative to minimize performace overhead for bulk loading.
// With dlv-dap, users currently do not have a way to adjust these. // With dlv-dap, users do not have a way to adjust these.
// TODO(polina): Support setting config via launch/attach args or only rely on on-demand loading? // Instead we are focusing in interacive loading with nested reloads, array/map
// paging and context-specific string limits.
var DefaultLoadConfig = proc.LoadConfig{ var DefaultLoadConfig = proc.LoadConfig{
FollowPointers: true, FollowPointers: true,
MaxVariableRecurse: 1, MaxVariableRecurse: 1,
MaxStringLen: 64, // TODO(polina): consider 1024 limit instead:
// - vscode+C appears to use 1024 as the load limit
// - vscode viewlet hover truncates at 1023 characters
MaxStringLen: 512,
MaxArrayValues: 64, MaxArrayValues: 64,
MaxStructFields: -1, MaxStructFields: -1,
} }
const (
// When a user examines a single string, we can relax the loading limit.
maxSingleStringLen = 4 << 10 // 4096
// Results of a call are single-use and transient. We need to maximize
// what is presented. A common use case of a call injection is to
// stringify complex data conveniently.
maxStringLenInCallRetVars = 1 << 10 // 1024
)
// NewServer creates a new DAP Server. It takes an opened Listener // NewServer creates a new DAP Server. It takes an opened Listener
// via config and assumes its ownership. config.DisconnectChan has to be set; // via config and assumes its ownership. config.DisconnectChan has to be set;
// it will be closed by the server when the client fails to connect, // it will be closed by the server when the client fails to connect,
@ -1794,9 +1807,9 @@ func (s *Server) childrenToDAPVariables(v *fullyQualifiedVariable) ([]dap.Variab
Value: val, Value: val,
} }
if keyref != 0 { // key is a type to be expanded if keyref != 0 { // key is a type to be expanded
if len(key) > DefaultLoadConfig.MaxStringLen { if len(key) > maxMapKeyValueLen {
// Truncate and make unique // Truncate and make unique
kvvar.Name = fmt.Sprintf("%s... @ %#x", key[0:DefaultLoadConfig.MaxStringLen], keyv.Addr) kvvar.Name = fmt.Sprintf("%s... @ %#x", key[0:maxMapKeyValueLen], keyv.Addr)
} }
kvvar.VariablesReference = keyref kvvar.VariablesReference = keyref
kvvar.IndexedVariables = getIndexedVariableCount(keyv) kvvar.IndexedVariables = getIndexedVariableCount(keyv)
@ -1890,9 +1903,12 @@ func (s *Server) convertVariableToString(v *proc.Variable) string {
return val return val
} }
// defaultMaxValueLen is the max length of a string representation of a compound or reference const (
// type variable. // Limit the length of a string representation of a compound or reference type variable.
const defaultMaxValueLen = 1 << 8 // 256 maxVarValueLen = 1 << 8 // 256
// Limit the length of an inlined map key.
maxMapKeyValueLen = 64
)
// Flags for convertVariableWithOpts option. // Flags for convertVariableWithOpts option.
type convertVariableFlags uint8 type convertVariableFlags uint8
@ -2048,8 +2064,8 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s
// By default, only values of variables that have children can be truncated. // By default, only values of variables that have children can be truncated.
// If showFullValue is set, then all value strings are not truncated. // If showFullValue is set, then all value strings are not truncated.
canTruncateValue := showFullValue&opts == 0 canTruncateValue := showFullValue&opts == 0
if len(value) > defaultMaxValueLen && canTruncateValue && canHaveRef { if len(value) > maxVarValueLen && canTruncateValue && canHaveRef {
value = value[:defaultMaxValueLen] + "..." value = value[:maxVarValueLen] + "..."
} }
return value, variablesReference return value, variablesReference
} }
@ -2113,9 +2129,9 @@ func (s *Server) onEvaluateRequest(request *dap.EvaluateRequest) {
case "repl", "variables", "hover", "clipboard": case "repl", "variables", "hover", "clipboard":
if exprVar.Kind == reflect.String { if exprVar.Kind == reflect.String {
if strVal := constant.StringVal(exprVar.Value); exprVar.Len > int64(len(strVal)) { if strVal := constant.StringVal(exprVar.Value); exprVar.Len > int64(len(strVal)) {
// reload string value with a bigger limit. // Reload the string value with a bigger limit.
loadCfg := DefaultLoadConfig loadCfg := DefaultLoadConfig
loadCfg.MaxStringLen = 4 << 10 loadCfg.MaxStringLen = maxSingleStringLen
if v, err := s.debugger.EvalVariableInScope(goid, frame, 0, request.Arguments.Expression, loadCfg); err != nil { if v, err := s.debugger.EvalVariableInScope(goid, frame, 0, request.Arguments.Expression, loadCfg); err != nil {
s.log.Debugf("Failed to load more for %v: %v", request.Arguments.Expression, err) s.log.Debugf("Failed to load more for %v: %v", request.Arguments.Expression, err)
} else { } else {
@ -2152,13 +2168,11 @@ func (s *Server) doCall(goid, frame int, expr string) (*api.DebuggerState, []*pr
} }
// The return values of injected function calls are volatile. // The return values of injected function calls are volatile.
// Load as much useful data as possible. // Load as much useful data as possible.
// - String: a common use case of call injection is to stringify complex
// data conveniently. That can easily exceed the default limit, 64.
// TODO: investigate whether we need to increase other limits. For example, // TODO: investigate whether we need to increase other limits. For example,
// the return value is a pointer to a temporary object, which can become // the return value is a pointer to a temporary object, which can become
// invalid by other injected function calls. Do we care about such use cases? // invalid by other injected function calls. Do we care about such use cases?
loadCfg := DefaultLoadConfig loadCfg := DefaultLoadConfig
loadCfg.MaxStringLen = 1 << 10 loadCfg.MaxStringLen = maxStringLenInCallRetVars
// TODO(polina): since call will resume execution of all goroutines, // TODO(polina): since call will resume execution of all goroutines,
// we should do this asynchronously and send a continued event to the // we should do this asynchronously and send a continued event to the

111
service/dap/server_test.go
View File

@ -682,7 +682,7 @@ func checkVar(t *testing.T, got *dap.VariablesResponse, i int, name, evalName, v
} }
} }
if i < 0 { if i < 0 {
t.Errorf("\ngot %#v\nwant Variables[i].Name=%q", got, name) t.Errorf("\ngot %#v\nwant Variables[i].Name=%q (not found)", got, name)
return 0 return 0
} }
@ -1379,7 +1379,7 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
checkVarExact(t, locals, -1, "emptyslice", "emptyslice", "[]string len: 0, cap: 0, []", "[]string", noChildren) checkVarExact(t, locals, -1, "emptyslice", "emptyslice", "[]string len: 0, cap: 0, []", "[]string", noChildren)
checkVarExact(t, locals, -1, "nilslice", "nilslice", "[]int len: 0, cap: 0, nil", "[]int", noChildren) checkVarExact(t, locals, -1, "nilslice", "nilslice", "[]int len: 0, cap: 0, nil", "[]int", noChildren)
// reflect.Kind == String // reflect.Kind == String
checkVarExact(t, locals, -1, "longstr", "longstr", `"very long string 0123456789a0123456789b0123456789c0123456789d012...+73 more"`, "string", noChildren) checkVarExact(t, locals, -1, "longstr", "longstr", longstr, "string", noChildren)
// reflect.Kind == Struct // reflect.Kind == Struct
checkVarExact(t, locals, -1, "zsvar", "zsvar", "struct {} {}", "struct {}", noChildren) checkVarExact(t, locals, -1, "zsvar", "zsvar", "struct {} {}", "struct {}", noChildren)
// reflect.Kind == UnsafePointer // reflect.Kind == UnsafePointer
@ -1476,8 +1476,12 @@ func TestVariablesLoading(t *testing.T) {
}, { }, {
execute: func() { execute: func() {
// Change default config values to trigger certain unloaded corner cases // Change default config values to trigger certain unloaded corner cases
saveDefaultConfig := DefaultLoadConfig
DefaultLoadConfig.MaxStructFields = 5 DefaultLoadConfig.MaxStructFields = 5
defer func() { DefaultLoadConfig.MaxStructFields = -1 }() DefaultLoadConfig.MaxStringLen = 64
defer func() {
DefaultLoadConfig = saveDefaultConfig
}()
client.StackTraceRequest(1, 0, 0) client.StackTraceRequest(1, 0, 0)
client.ExpectStackTraceResponse(t) client.ExpectStackTraceResponse(t)
@ -1489,7 +1493,8 @@ func TestVariablesLoading(t *testing.T) {
locals := client.ExpectVariablesResponse(t) locals := client.ExpectVariablesResponse(t)
// String partially missing based on LoadConfig.MaxStringLen // String partially missing based on LoadConfig.MaxStringLen
checkVarExact(t, locals, -1, "longstr", "longstr", "\"very long string 0123456789a0123456789b0123456789c0123456789d012...+73 more\"", "string", noChildren) // See also TestVariableLoadingOfLongStrings
checkVarExact(t, locals, -1, "longstr", "longstr", longstrLoaded64, "string", noChildren)
checkArrayChildren := func(t *testing.T, longarr *dap.VariablesResponse, parentName string, start int) { checkArrayChildren := func(t *testing.T, longarr *dap.VariablesResponse, parentName string, start int) {
t.Helper() t.Helper()
@ -2827,7 +2832,7 @@ func TestEvaluateRequest(t *testing.T) {
// From testvariables2 fixture // From testvariables2 fixture
const ( const (
// As defined in the code // As defined in the code
longstrFull = `"very long string 0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g012345678h90123456789i0123456789j0123456789"` longstr = `"very long string 0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g012345678h90123456789i0123456789j0123456789"`
// Loaded with MaxStringLen=64 // Loaded with MaxStringLen=64
longstrLoaded64 = `"very long string 0123456789a0123456789b0123456789c0123456789d012...+73 more"` longstrLoaded64 = `"very long string 0123456789a0123456789b0123456789c0123456789d012...+73 more"`
longstrLoaded64re = `\"very long string 0123456789a0123456789b0123456789c0123456789d012\.\.\.\+73 more\"` longstrLoaded64re = `\"very long string 0123456789a0123456789b0123456789c0123456789d012\.\.\.\+73 more\"`
@ -2880,28 +2885,27 @@ func TestVariableValueTruncation(t *testing.T) {
// Compound map keys may be truncated even further // Compound map keys may be truncated even further
// As the keys are always inside of a map container, // As the keys are always inside of a map container,
// this applies to variable requests. // this applies to variables requests only, not evalute requests.
// key - compound, value - scalar (inlined key:value display) => truncate key if too long // key - compound, value - scalar (inlined key:value display) => truncate key if too long
ref := checkVarExact(t, locals, -1, "m5", "m5", "map[main.C]int [{s: "+longstrLoaded64+"}: 1, ]", "map[main.C]int", hasChildren) ref := checkVarExact(t, locals, -1, "m5", "m5", "map[main.C]int [{s: "+longstr+"}: 1, ]", "map[main.C]int", hasChildren)
if ref > 0 { if ref > 0 {
client.VariablesRequest(ref) client.VariablesRequest(ref)
// Key format: <truncated>... @<address> // Key format: <truncated>... @<address>
checkVarRegex(t, client.ExpectVariablesResponse(t), 0, `main\.C {s: "very long string 0123456789.+\.\.\. @ 0x[0-9a-f]+`, `m5\[\(\*\(\*"main\.C"\)\(0x[0-9a-f]+\)\)\]`, "1", `int`, hasChildren) checkVarRegex(t, client.ExpectVariablesResponse(t), 0, `main\.C {s: "very long string 0123456789.+\.\.\. @ 0x[0-9a-f]+`, `m5\[\(\*\(\*"main\.C"\)\(0x[0-9a-f]+\)\)\]`, "1", `int`, hasChildren)
} }
// key - scalar, value - scalar (inlined key:value display) => not truncated // key - scalar, value - scalar (inlined key:value display) => key not truncated
ref = checkVarExact(t, locals, -1, "m6", "m6", "map[string]int ["+longstrLoaded64+": 123, ]", "map[string]int", hasChildren) ref = checkVarExact(t, locals, -1, "m6", "m6", "map[string]int ["+longstr+": 123, ]", "map[string]int", hasChildren)
if ref > 0 { if ref > 0 {
client.VariablesRequest(ref) client.VariablesRequest(ref)
checkVarRegex(t, client.ExpectVariablesResponse(t), 0, longstrLoaded64re, `m6[\(\*\(\*\"string\"\)\(0x[0-9a-f]+\)\)]`, "123", "string: int", noChildren) checkVarExact(t, client.ExpectVariablesResponse(t), 0, longstr, `m6[`+longstr+`]`, "123", "string: int", noChildren)
} }
// key - compound, value - compound (array-like display) => not truncated // key - compound, value - compound (array-like display) => key not truncated
ref = checkVarExact(t, locals, -1, "m7", "m7", "map[main.C]main.C [{s: "+longstrLoaded64+"}: {s: "+longstrLoaded64+"}, ]", "map[main.C]main.C", hasChildren) ref = checkVarExact(t, locals, -1, "m7", "m7", "map[main.C]main.C [{s: "+longstr+"}: {s: \"hello\"}, ]", "map[main.C]main.C", hasChildren)
if ref > 0 { if ref > 0 {
client.VariablesRequest(ref) client.VariablesRequest(ref)
m7 := client.ExpectVariablesResponse(t) m7 := client.ExpectVariablesResponse(t)
checkVarRegex(t, m7, 0, "[key 0]", `\(\*\(\*\"main\.C\"\)\(0x[0-9a-f]+\)\)`, `main\.C {s: `+longstrLoaded64re+`}`, `main\.C`, hasChildren) checkVarRegex(t, m7, 0, "[key 0]", `\(\*\(\*\"main\.C\"\)\(0x[0-9a-f]+\)\)`, `main\.C {s: `+longstr+`}`, `main\.C`, hasChildren)
checkVarRegex(t, m7, 1, "[val 0]", `\(\*\(\*\"main\.C\"\)\(0x[0-9a-f]+\)\)`, `main\.C {s: `+longstrLoaded64re+`}`, `main\.C`, hasChildren)
} }
}, },
disconnect: true, disconnect: true,
@ -2909,16 +2913,17 @@ func TestVariableValueTruncation(t *testing.T) {
}) })
} }
// TestVariableLoadingOfLargeStrings tests that different string loading limits // TestVariableLoadingOfLongStrings tests that different string loading limits
// apply that depending on the context. // apply that depending on the context.
func TestVariableLoadingOfLargeStrings(t *testing.T) { func TestVariableLoadingOfLongStrings(t *testing.T) {
runTest(t, "testvariables2", func(client *daptest.Client, fixture protest.Fixture) { protest.MustSupportFunctionCalls(t, testBackend)
runTest(t, "longstrings", func(client *daptest.Client, fixture protest.Fixture) {
runDebugSessionWithBPs(t, client, "launch", runDebugSessionWithBPs(t, client, "launch",
// Launch // Launch
func() { func() {
client.LaunchRequest("exec", fixture.Path, !stopOnEntry) client.LaunchRequest("exec", fixture.Path, !stopOnEntry)
}, },
// Breakpoints are set within the program // Breakpoint set within the program
fixture.Source, []int{}, fixture.Source, []int{},
[]onBreakpoint{{ []onBreakpoint{{
execute: func() { execute: func() {
@ -2927,29 +2932,56 @@ func TestVariableLoadingOfLargeStrings(t *testing.T) {
client.VariablesRequest(1001) // Locals client.VariablesRequest(1001) // Locals
locals := client.ExpectVariablesResponse(t) locals := client.ExpectVariablesResponse(t)
// Limits vary for evaluate requests // Limits vary for evaluate requests with different contexts
for _, evalContext := range []string{"", "watch", "repl", "variables", "hover", "clipboard", "somethingelse"} { tests := []struct {
t.Run(evalContext, func(t *testing.T) { context string
// long string by itself (limits vary) limit int
client.EvaluateRequest("longstr", 0, evalContext) }{
got := client.ExpectEvaluateResponse(t) {"", DefaultLoadConfig.MaxStringLen},
want := longstrLoaded64 {"watch", DefaultLoadConfig.MaxStringLen},
switch evalContext { {"repl", maxSingleStringLen},
case "repl", "variables", "hover", "clipboard": {"hover", maxSingleStringLen},
want = longstrFull {"variables", maxSingleStringLen},
{"clipboard", maxSingleStringLen},
{"somethingelse", DefaultLoadConfig.MaxStringLen},
} }
checkEval(t, got, want, false) for _, tc := range tests {
t.Run(tc.context, func(t *testing.T) {
// Long string by itself (limits vary)
client.EvaluateRequest("s4097", 0, tc.context)
want := fmt.Sprintf(`"x+\.\.\.\+%d more"`, 4097-tc.limit)
checkEvalRegex(t, client.ExpectEvaluateResponse(t), want, noChildren)
// long string as a child (same limits) // Evaluated container variables return values with minimally loaded
client.EvaluateRequest("(cl).s", 0, evalContext) // strings, which are further truncated for displaying, so we
got2 := client.ExpectEvaluateResponse(t) // can't test for loading limit except in contexts where an untruncated
checkEval(t, got2, want, false) // value is returned.
client.EvaluateRequest("&s4097", 0, tc.context)
switch tc.context {
case "variables", "clipboard":
want = fmt.Sprintf(`\*"x+\.\.\.\+%d more`, 4097-DefaultLoadConfig.MaxStringLen)
default:
want = fmt.Sprintf(`\*"x{%d}\.\.\.`, maxVarValueLen-2)
}
checkEvalRegex(t, client.ExpectEvaluateResponse(t), want, hasChildren)
}) })
} }
// Variables requests are not affected. // Long strings returned from calls are subject to a different limit,
checkVarExact(t, locals, -1, "longstr", "longstr", longstrLoaded64, "string", noChildren) // same limit regardless of context
checkVarExact(t, locals, -1, "cl", "cl", `main.C {s: `+longstrLoaded64+`}`, "main.C", hasChildren) for _, context := range []string{"", "watch", "repl", "variables", "hover", "clipboard", "somethingelse"} {
t.Run(context, func(t *testing.T) {
client.EvaluateRequest(`call buildString(4097)`, 1000, context)
want := fmt.Sprintf(`"x+\.\.\.\+%d more"`, 4097-maxStringLenInCallRetVars)
got := client.ExpectEvaluateResponse(t)
checkEvalRegex(t, got, want, hasChildren)
})
}
// Variables requests use the most conservative loading limit
checkVarRegex(t, locals, -1, "s513", "s513", `"x{512}\.\.\.\+1 more"`, "string", noChildren)
// Container variables are subject to additional stricter value truncation that drops +more part
checkVarRegex(t, locals, -1, "nested", "nested", `map\[int\]string \[513: \"x+\.\.\.`, "string", hasChildren)
}, },
disconnect: true, disconnect: true,
}}) }})
@ -3086,13 +3118,6 @@ func TestEvaluateCallRequest(t *testing.T) {
client.EvaluateRequest("call ", 1000, "watch") client.EvaluateRequest("call ", 1000, "watch")
got = client.ExpectEvaluateResponse(t) got = client.ExpectEvaluateResponse(t)
checkEval(t, got, "\"this is a variable named `call`\"", noChildren) checkEval(t, got, "\"this is a variable named `call`\"", noChildren)
// Long string as a return value
client.EvaluateRequest(`call stringsJoin(longstrs, ",")`, 1000, "variables") // full string
got = client.ExpectEvaluateResponse(t)
checkEval(t, got, `"very long string 0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g012345678h90123456789i0123456789j0123456789"`, hasChildren)
client.EvaluateRequest(`call stringsJoin(longstrs, ",")`, 1000, "watch") // full string
got = client.ExpectEvaluateResponse(t)
checkEval(t, got, `"very long string 0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g012345678h90123456789i0123456789j0123456789"`, hasChildren)
// Call error // Call error
client.EvaluateRequest("call call1(one)", 1000, "watch") client.EvaluateRequest("call call1(one)", 1000, "watch")