diff --git a/_fixtures/fncall.go b/_fixtures/fncall.go index ba001be4..949e4010 100644 --- a/_fixtures/fncall.go +++ b/_fixtures/fncall.go @@ -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) } diff --git a/_fixtures/testvariables2.go b/_fixtures/testvariables2.go index d350262b..d59cbebc 100644 --- a/_fixtures/testvariables2.go +++ b/_fixtures/testvariables2.go @@ -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) } diff --git a/service/dap/server.go b/service/dap/server.go index 13b4bf8e..d61ae55e 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -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 } diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 233b7769..90fa38b6 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -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)