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

dap: use larger string type variable load limits in 'repl', 'variables' context (#2418)

Browse Source 
* dap: use larger variable load limits in 'repl', 'variables' context

When evaluate requests are triggered in the context of 'repl'
(DEBUG CONSOLE in VSCode) or 'variables' (copy values from VARIABLES
section in VSCode), they are the result of human action and have
more rooms to display. So it is not too bad to apply longer limits.

Variable auto-loading for strings or arrays is nice but currently
it's unclear to me how this should be integrated in the DEBUG
CONSOLE or with the Copy Value feature. Until we have better ideas
and tools, let's go with these larger limits.

Unfortunately, the "Copy Value" from WATCH section triggers evaluate
requests with "watch" context and we don't want to load large data
automatically for "watch". So, users who want to query a large value
should first copy the expression to DEBUG CONSOLE and evaluate it.
Not ideal but not the end of the world either.

Updates golang/vscode-go#1318

* dap: apply large limit only to the string type result

* dap: move string reload logic to convertVariable* where other reload logic is

Currently we are thinking string reload for evaluation as a temporary
workaround until we figure out an intutitive way to present long strings.
So, I hope moving this logic near other reload logic may be better.

And, use the address based expression when reloading - when handling the
function return values, we may not have an expression to use.

* dap: make deep source check happy

* dap: move string reevaluation logic back to onEvaluateRequest

Reloading string variables is tricky if they are in registers.
We don't attempt to reload them but for clarity, move this up
to the onEvaluateRequest handler.

For function call, use a generous limit for string load
since the results are volatile.

* dap: check variable isn't affected by evaluate in other context
This commit is contained in:
Hyang-Ah Hana Kim 2021-05-25 13:23:41 -04:00 committed by GitHub
parent 63985d1d9e
commit 5e12091da2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 92 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
_fixtures/fncall.go
View File

@ -178,7 +178,7 @@ func main() {
a2 := a2struct{Y: 7}
var pa2 *astruct
var str string = "old string value"
longstrs := []string{"very long string 0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g012345678h90123456789i0123456789j0123456789"}
var vable_a VRcvrable = a
var vable_pa VRcvrable = pa
var pable_pa PRcvrable = pa
@ -200,5 +200,5 @@ func main() {
d.Method()
d.Base.Method()
x.CallMe()
fmt.Println(one, two, zero, call, call0, call2, callexit, callpanic, callbreak, callstacktrace, stringsJoin, intslice, stringslice, comma, a.VRcvr, a.PRcvr, pa, vable_a, vable_pa, pable_pa, fn2clos, fn2glob, fn2valmeth, fn2ptrmeth, fn2nil, ga, escapeArg, a2, square, intcallpanic, onetwothree, curriedAdd, getAStruct, getAStructPtr, getVRcvrableFromAStruct, getPRcvrableFromAStructPtr, getVRcvrableFromAStructPtr, pa2, noreturncall, str, d, x, x2.CallMe(5))
fmt.Println(one, two, zero, call, call0, call2, callexit, callpanic, callbreak, callstacktrace, stringsJoin, intslice, stringslice, comma, a.VRcvr, a.PRcvr, pa, vable_a, vable_pa, pable_pa, fn2clos, fn2glob, fn2valmeth, fn2ptrmeth, fn2nil, ga, escapeArg, a2, square, intcallpanic, onetwothree, curriedAdd, getAStruct, getAStructPtr, getVRcvrableFromAStruct, getPRcvrableFromAStructPtr, getVRcvrableFromAStructPtr, pa2, noreturncall, str, d, x, x2.CallMe(5), longstrs)
}

4
_fixtures/testvariables2.go
View File

@ -328,7 +328,7 @@ func main() {
longstr := "very long string 0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g012345678h90123456789i0123456789j0123456789"
m5 := map[C]int{{longstr}: 1}
m6 := C{s: longstr}
var nilstruct *astruct = nil
val := A{val: 1} // val vs val.val
@ -363,5 +363,5 @@ func main() {
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, m5, 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, rettm, errtypednil, emptyslice, emptymap, byteslice, runeslice, bytearray, runearray, longstr, nilstruct, as2, as2.NonPointerRecieverMethod, s4, iface2map, issue1578, ll, unread, w2, w3, w4, w5, longarr, longslice, val)
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, m5, 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, rettm, errtypednil, emptyslice, emptymap, byteslice, runeslice, bytearray, runearray, longstr, nilstruct, as2, as2.NonPointerRecieverMethod, s4, iface2map, issue1578, ll, unread, w2, w3, w4, w5, longarr, longslice, val, m6)
}

35
service/dap/server.go
View File

@ -1632,7 +1632,7 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s
}
value = api.ConvertVar(v).SinglelineString()
if v.Unreadable != nil {
return
return value, variablesReference
}
// Some of the types might be fully or partially not loaded based on LoadConfig.
@ -1721,7 +1721,7 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s
variablesReference = maybeCreateVariableHandle(v)
}
case reflect.String:
// TODO(polina): implement auto-loading here
// 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 loaded
@ -1761,7 +1761,7 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s
variablesReference = maybeCreateVariableHandle(v)
}
}
return
return value, variablesReference
}
// onEvaluateRequest handles 'evalute' requests.
@ -1817,8 +1817,21 @@ func (s *Server) onEvaluateRequest(request *dap.EvaluateRequest) {
s.sendErrorResponseWithOpts(request.Request, UnableToEvaluateExpression, "Unable to evaluate expression", err.Error(), showErrorToUser)
return
}
if exprVar.Kind == reflect.String && (request.Arguments.Context == "repl" || request.Arguments.Context == "variables") {
if strVal := constant.StringVal(exprVar.Value); exprVar.Len > int64(len(strVal)) {
// reload string value with a bigger limit.
loadCfg := DefaultLoadConfig
loadCfg.MaxStringLen = 4 << 10
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)
} else {
exprVar = v
}
}
}
// TODO(polina): as far as I can tell, evaluateName is ignored by vscode for expression variables.
// Should it be skipped alltogether for all levels?
// Should it be skipped altogether for all levels?
exprVal, exprRef := s.convertVariable(exprVar, fmt.Sprintf("(%s)", request.Arguments.Expression))
response.Body = dap.EvaluateResponseBody{Result: exprVal, VariablesReference: exprRef}
}
@ -1839,12 +1852,22 @@ func (s *Server) doCall(goid, frame int, expr string) (*api.DebuggerState, []*pr
if err != nil {
return nil, nil, err
}
// The return values of injected function calls are volatile.
// 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,
// 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?
loadCfg := DefaultLoadConfig
loadCfg.MaxStringLen = 1 << 10
// TODO(polina): since call will resume execution of all goroutines,
// we should do this asynchronously and send a continued event to the
// editor, followed by a stop event when the call completes.
state, err := s.debugger.Command(&api.DebuggerCommand{
Name: api.Call,
ReturnInfoLoadConfig: api.LoadConfigFromProc(&DefaultLoadConfig),
ReturnInfoLoadConfig: api.LoadConfigFromProc(&loadCfg),
Expr: expr,
UnsafeCall: false,
GoroutineID: goid,
@ -1869,7 +1892,7 @@ func (s *Server) doCall(goid, frame int, expr string) (*api.DebuggerState, []*pr
t.Line == stateBeforeCall.SelectedGoroutine.CurrentLoc.Line && t.CallReturn {
found = true
// The call completed. Get the return values.
retVars, err = s.debugger.FindThreadReturnValues(t.ID, DefaultLoadConfig)
retVars, err = s.debugger.FindThreadReturnValues(t.ID, loadCfg)
if err != nil {
return nil, nil, err
}

61
service/dap/server_test.go
View File

@ -1368,7 +1368,7 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
expectVarExact(t, locals, -1, "emptyslice", "emptyslice", "[]string len: 0, cap: 0, []", "[]string", noChildren)
expectVarExact(t, locals, -1, "nilslice", "nilslice", "[]int len: 0, cap: 0, nil", "[]int", noChildren)
// reflect.Kind == String
expectVarExact(t, locals, -1, "longstr", "longstr", "\"very long string 0123456789a0123456789b0123456789c0123456789d012...+73 more\"", "string", noChildren)
expectVarExact(t, locals, -1, "longstr", "longstr", `"very long string 0123456789a0123456789b0123456789c0123456789d012...+73 more"`, "string", noChildren)
// reflect.Kind == Struct
expectVarExact(t, locals, -1, "zsvar", "zsvar", "struct {} {}", "struct {}", noChildren)
// reflect.Kind == UnsafePointer
@ -1448,7 +1448,7 @@ func TestScopesRequestsOptimized(t *testing.T) {
protest.EnableOptimization)
}
// TestVariablesLoading exposes test cases where variables might be partiall or
// TestVariablesLoading exposes test cases where variables might be partially or
// fully unloaded.
func TestVariablesLoading(t *testing.T) {
runTest(t, "testvariables2", func(client *daptest.Client, fixture protest.Fixture) {
@ -2666,6 +2666,55 @@ func TestEvaluateRequest(t *testing.T) {
})
}
func TestEvaluateRequestLongStr(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() {
longstr := `"very long string 0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g012345678h90123456789i0123456789j0123456789"`
longstrTruncated := `"very long string 0123456789a0123456789b0123456789c0123456789d012...+73 more"`
handleStop(t, client, 1, "main.main", -1)
client.VariablesRequest(1001) // Locals
locals := client.ExpectVariablesResponse(t)
// reflect.Kind == String, load with longer load limit if evaluated in repl/variables context.
for _, evalContext := range []string{"", "watch", "repl", "variables", "somethingelse"} {
t.Run(evalContext, func(t *testing.T) {
// long string
client.EvaluateRequest("longstr", 0, evalContext)
got := client.ExpectEvaluateResponse(t)
want := longstrTruncated
if evalContext == "repl" || evalContext == "variables" {
want = longstr
}
expectEval(t, got, want, false)
// long string as a struct field
client.EvaluateRequest("(m6).s", 0, evalContext)
got2 := client.ExpectEvaluateResponse(t)
expectEval(t, got2, want, false)
// variables are not affected.
expectVarExact(t, locals, -1, "longstr", "longstr", longstrTruncated, "string", noChildren)
expectVarExact(t, locals, -1, "m6", "m6", `main.C {s: `+longstrTruncated+`}`, "main.C", hasChildren)
})
}
},
disconnect: true,
}})
})
}
func TestEvaluateCallRequest(t *testing.T) {
protest.MustSupportFunctionCalls(t, testBackend)
runTest(t, "fncall", func(client *daptest.Client, fixture protest.Fixture) {
@ -2796,6 +2845,14 @@ func TestEvaluateCallRequest(t *testing.T) {
client.EvaluateRequest("call ", 1000, "watch")
got = client.ExpectEvaluateResponse(t)
expectEval(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)
expectEval(t, got, `"very long string 0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g012345678h90123456789i0123456789j0123456789"`, hasChildren)
client.EvaluateRequest(`call stringsJoin(longstrs, ",")`, 1000, "watch") // full string
got = client.ExpectEvaluateResponse(t)
expectEval(t, got, `"very long string 0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g012345678h90123456789i0123456789j0123456789"`, hasChildren)
// Call error
client.EvaluateRequest("call call1(one)", 1000, "watch")
erres := client.ExpectInvisibleErrorResponse(t)