From a2a9918a8524f3f7675297b75daa2a80bc0790ff Mon Sep 17 00:00:00 2001 From: Walter Erquinigo Date: Mon, 11 Sep 2023 17:00:01 -0400 Subject: [PATCH] [lldb-vscode] Make descriptive summaries and raw child for synthetics configurable (#65687) "descriptive summaries" should only be used for small to medium binaries because of the performance penalty the cause when completing types. I'm defaulting it to false. Besides that, the "raw child" for synthetics should be optional as well. I'm defaulting it to false. Both options can be set via a launch or attach config, following the pattern of most settings. javascript extension wrappers can set these settings on their own as well. --- .../tools/lldb-vscode/lldbvscode_testcase.py | 15 +++- .../test/tools/lldb-vscode/vscode.py | 6 +- .../evaluate/TestVSCode_evaluate.py | 24 ++++-- .../variables/TestVSCode_variables.py | 81 +++++++++++++++---- lldb/tools/lldb-vscode/JSONUtils.cpp | 12 ++- lldb/tools/lldb-vscode/VSCode.cpp | 2 + lldb/tools/lldb-vscode/VSCode.h | 2 + lldb/tools/lldb-vscode/lldb-vscode.cpp | 11 ++- lldb/tools/lldb-vscode/package.json | 20 +++++ 9 files changed, 144 insertions(+), 29 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py index a6c370ccd203..8cd4e8454c89 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py @@ -1,8 +1,9 @@ -from lldbsuite.test.lldbtest import * import os -import vscode import time +import vscode +from lldbsuite.test.lldbtest import * + class VSCodeTestCaseBase(TestBase): NO_DEBUG_INFO_TESTCASE = True @@ -267,7 +268,7 @@ class VSCodeTestCaseBase(TestBase): if memoryReference not in self.vscode.disassembled_instructions: self.vscode.request_disassemble(memoryReference=memoryReference) - + return self.vscode.disassembled_instructions[memoryReference] def attach( @@ -348,6 +349,8 @@ class VSCodeTestCaseBase(TestBase): runInTerminal=False, expectFailure=False, postRunCommands=None, + enableAutoVariableSummaries=False, + enableSyntheticChildDebugging=False, ): """Sending launch request to vscode""" @@ -384,6 +387,8 @@ class VSCodeTestCaseBase(TestBase): sourceMap=sourceMap, runInTerminal=runInTerminal, postRunCommands=postRunCommands, + enableAutoVariableSummaries=enableAutoVariableSummaries, + enableSyntheticChildDebugging=enableSyntheticChildDebugging, ) if expectFailure: @@ -418,6 +423,8 @@ class VSCodeTestCaseBase(TestBase): disconnectAutomatically=True, postRunCommands=None, lldbVSCodeEnv=None, + enableAutoVariableSummaries=False, + enableSyntheticChildDebugging=False, ): """Build the default Makefile target, create the VSCode debug adaptor, and launch the process. @@ -446,4 +453,6 @@ class VSCodeTestCaseBase(TestBase): runInTerminal=runInTerminal, disconnectAutomatically=disconnectAutomatically, postRunCommands=postRunCommands, + enableAutoVariableSummaries=enableAutoVariableSummaries, + enableSyntheticChildDebugging=enableSyntheticChildDebugging, ) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py index 14f0bf0a2d4e..b30443e2e2ac 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py @@ -648,7 +648,7 @@ class DebugCommunication(object): "arguments": args_dict, } return self.send_recv(command_dict) - + def request_disassemble(self, memoryReference, offset=-50, instructionCount=200, resolveSymbols=True): args_dict = { "memoryReference": memoryReference, @@ -727,6 +727,8 @@ class DebugCommunication(object): sourceMap=None, runInTerminal=False, postRunCommands=None, + enableAutoVariableSummaries=False, + enableSyntheticChildDebugging=False, ): args_dict = {"program": program} if args: @@ -768,6 +770,8 @@ class DebugCommunication(object): args_dict["runInTerminal"] = runInTerminal if postRunCommands: args_dict["postRunCommands"] = postRunCommands + args_dict["enableAutoVariableSummaries"] = enableAutoVariableSummaries + args_dict["enableSyntheticChildDebugging"] = enableSyntheticChildDebugging command_dict = {"command": "launch", "type": "request", "arguments": args_dict} response = self.send_recv(command_dict) diff --git a/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py b/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py index 85bd23d9abfd..d0172ef9160f 100644 --- a/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py +++ b/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py @@ -28,13 +28,17 @@ class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase): def isExpressionParsedExpected(self): return self.context != "hover" - def run_test_evaluate_expressions(self, context=None): + def run_test_evaluate_expressions( + self, context=None, enableAutoVariableSummaries=False + ): """ Tests the evaluate expression request at different breakpoints """ self.context = context program = self.getBuildArtifact("a.out") - self.build_and_launch(program) + self.build_and_launch( + program, enableAutoVariableSummaries=enableAutoVariableSummaries + ) source = "main.cpp" self.set_source_breakpoints( source, @@ -55,7 +59,9 @@ class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase): self.assertEvaluate("var2", "21") self.assertEvaluate("static_int", "42") self.assertEvaluate("non_static_int", "43") - self.assertEvaluate("struct1", "{foo:15}") + self.assertEvaluate( + "struct1", "{foo:15}" if enableAutoVariableSummaries else "my_struct @ 0x" + ) self.assertEvaluate("struct1.foo", "15") self.assertEvaluate("struct2->foo", "16") @@ -85,7 +91,9 @@ class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase): self.assertEvaluate( "non_static_int", "10" ) # different variable with the same name - self.assertEvaluate("struct1", "{foo:15}") + self.assertEvaluate( + "struct1", "{foo:15}" if enableAutoVariableSummaries else "my_struct @ 0x" + ) self.assertEvaluate("struct1.foo", "15") self.assertEvaluate("struct2->foo", "16") @@ -146,22 +154,22 @@ class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase): @skipIfRemote def test_generic_evaluate_expressions(self): # Tests context-less expression evaluations - self.run_test_evaluate_expressions() + self.run_test_evaluate_expressions(enableAutoVariableSummaries=False) @skipIfWindows @skipIfRemote def test_repl_evaluate_expressions(self): # Tests expression evaluations that are triggered from the Debug Console - self.run_test_evaluate_expressions("repl") + self.run_test_evaluate_expressions("repl", enableAutoVariableSummaries=True) @skipIfWindows @skipIfRemote def test_watch_evaluate_expressions(self): # Tests expression evaluations that are triggered from a watch expression - self.run_test_evaluate_expressions("watch") + self.run_test_evaluate_expressions("watch", enableAutoVariableSummaries=False) @skipIfWindows @skipIfRemote def test_hover_evaluate_expressions(self): # Tests expression evaluations that are triggered when hovering on the editor - self.run_test_evaluate_expressions("hover") + self.run_test_evaluate_expressions("hover", enableAutoVariableSummaries=True) diff --git a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py index e9596ac56e0e..fc24b3b34e70 100644 --- a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py +++ b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py @@ -127,15 +127,17 @@ class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase): varref_dict = {} self.verify_variables(verify_locals, locals, varref_dict) - @skipIfWindows - @skipIfRemote - def test_scopes_variables_setVariable_evaluate(self): + def do_test_scopes_variables_setVariable_evaluate( + self, enableAutoVariableSummaries: bool + ): """ Tests the "scopes", "variables", "setVariable", and "evaluate" packets. """ program = self.getBuildArtifact("a.out") - self.build_and_launch(program) + self.build_and_launch( + program, enableAutoVariableSummaries=enableAutoVariableSummaries + ) source = "main.cpp" breakpoint1_line = line_number(source, "// breakpoint 1") lines = [breakpoint1_line] @@ -219,12 +221,20 @@ class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase): }, "pt": { "equals": {"type": "PointType"}, - "startswith": {"result": "{x:11, y:22}"}, + "startswith": { + "result": "{x:11, y:22}" + if enableAutoVariableSummaries + else "PointType @ 0x" + }, "hasVariablesReference": True, }, "pt.buffer": { "equals": {"type": "int[32]"}, - "startswith": {"result": "{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...}"}, + "startswith": { + "result": "{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...}" + if enableAutoVariableSummaries + else "int[32] @ 0x" + }, "hasVariablesReference": True, }, "argv": { @@ -347,13 +357,27 @@ class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase): @skipIfWindows @skipIfRemote - def test_scopes_and_evaluate_expansion(self): + def test_scopes_variables_setVariable_evaluate(self): + self.do_test_scopes_variables_setVariable_evaluate( + enableAutoVariableSummaries=False + ) + + @skipIfWindows + @skipIfRemote + def test_scopes_variables_setVariable_evaluate_with_descriptive_summaries(self): + self.do_test_scopes_variables_setVariable_evaluate( + enableAutoVariableSummaries=True + ) + + def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: bool): """ Tests the evaluated expression expands successfully after "scopes" packets and permanent expressions persist. """ program = self.getBuildArtifact("a.out") - self.build_and_launch(program) + self.build_and_launch( + program, enableAutoVariableSummaries=enableAutoVariableSummaries + ) source = "main.cpp" breakpoint1_line = line_number(source, "// breakpoint 1") lines = [breakpoint1_line] @@ -410,7 +434,11 @@ class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase): "name": "pt", "response": { "equals": {"type": "PointType"}, - "startswith": {"result": "{x:11, y:22}"}, + "startswith": { + "result": "{x:11, y:22}" + if enableAutoVariableSummaries + else "PointType @ 0x" + }, "missing": ["indexedVariables"], "hasVariablesReference": True, }, @@ -487,14 +515,24 @@ class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase): @skipIfWindows @skipIfRemote - def test_indexedVariables(self): + def test_scopes_and_evaluate_expansion(self): + self.do_test_scopes_and_evaluate_expansion(enableAutoVariableSummaries=False) + + @skipIfWindows + @skipIfRemote + def test_scopes_and_evaluate_expansion_with_descriptive_summaries(self): + self.do_test_scopes_and_evaluate_expansion(enableAutoVariableSummaries=True) + + def do_test_indexedVariables(self, enableSyntheticChildDebugging: bool): """ Tests that arrays and lldb.SBValue objects that have synthetic child providers have "indexedVariables" key/value pairs. This helps the IDE not to fetch too many children all at once. """ program = self.getBuildArtifact("a.out") - self.build_and_launch(program) + self.build_and_launch( + program, enableSyntheticChildDebugging=enableSyntheticChildDebugging + ) source = "main.cpp" breakpoint1_line = line_number(source, "// breakpoint 4") lines = [breakpoint1_line] @@ -507,13 +545,14 @@ class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase): # Verify locals locals = self.vscode.get_local_variables() - # The vector variables will have one additional entry from the fake + # The vector variables might have one additional entry from the fake # "[raw]" child. + raw_child_count = 1 if enableSyntheticChildDebugging else 0 verify_locals = { "small_array": {"equals": {"indexedVariables": 5}}, "large_array": {"equals": {"indexedVariables": 200}}, - "small_vector": {"equals": {"indexedVariables": 6}}, - "large_vector": {"equals": {"indexedVariables": 201}}, + "small_vector": {"equals": {"indexedVariables": 5 + raw_child_count}}, + "large_vector": {"equals": {"indexedVariables": 200 + raw_child_count}}, "pt": {"missing": ["indexedVariables"]}, } self.verify_variables(verify_locals, locals) @@ -526,13 +565,25 @@ class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase): "[2]": {"equals": {"type": "int", "value": "0"}}, "[3]": {"equals": {"type": "int", "value": "0"}}, "[4]": {"equals": {"type": "int", "value": "0"}}, - "[raw]": {"contains": {"type": ["vector"]}}, } + if enableSyntheticChildDebugging: + verify_children["[raw]"] = ({"contains": {"type": ["vector"]}},) + children = self.vscode.request_variables(locals[2]["variablesReference"])[ "body" ]["variables"] self.verify_variables(verify_children, children) + @skipIfWindows + @skipIfRemote + def test_indexedVariables(self): + self.do_test_indexedVariables(enableSyntheticChildDebugging=False) + + @skipIfWindows + @skipIfRemote + def test_indexedVariables_with_raw_child_for_synthetics(self): + self.do_test_indexedVariables(enableSyntheticChildDebugging=True) + @skipIfWindows @skipIfRemote def test_registers(self): diff --git a/lldb/tools/lldb-vscode/JSONUtils.cpp b/lldb/tools/lldb-vscode/JSONUtils.cpp index e34448c9b7f2..0d149ff27fd3 100644 --- a/lldb/tools/lldb-vscode/JSONUtils.cpp +++ b/lldb/tools/lldb-vscode/JSONUtils.cpp @@ -137,6 +137,12 @@ std::vector GetStrings(const llvm::json::Object *obj, /// glance. static std::optional GetSyntheticSummaryForContainer(lldb::SBValue &v) { + // We gate this feature because it performs GetNumChildren(), which can + // cause performance issues because LLDB needs to complete possibly huge + // types. + if (!g_vsc.enable_auto_variable_summaries) + return std::nullopt; + if (v.TypeIsPointerType() || !v.MightHaveChildren()) return std::nullopt; /// As this operation can be potentially slow, we limit the total time spent @@ -191,6 +197,9 @@ GetSyntheticSummaryForContainer(lldb::SBValue &v) { /// Return whether we should dereference an SBValue in order to generate a more /// meaningful summary string. static bool ShouldBeDereferencedForSummary(lldb::SBValue &v) { + if (!g_vsc.enable_auto_variable_summaries) + return false; + if (!v.GetType().IsPointerType() && !v.GetType().IsReferenceType()) return false; @@ -1137,7 +1146,8 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference, // We create a "[raw]" fake child for each synthetic type, so we have to // account for it when returning indexed variables. We don't need to do this // for non-indexed ones. - int actual_num_children = num_children + (is_synthetic ? 1 : 0); + bool has_raw_child = is_synthetic && g_vsc.enable_synthetic_child_debugging; + int actual_num_children = num_children + (has_raw_child ? 1 : 0); if (is_array) { object.try_emplace("indexedVariables", actual_num_children); } else if (num_children > 0) { diff --git a/lldb/tools/lldb-vscode/VSCode.cpp b/lldb/tools/lldb-vscode/VSCode.cpp index 85e1f4b4ac06..1384604c4837 100644 --- a/lldb/tools/lldb-vscode/VSCode.cpp +++ b/lldb/tools/lldb-vscode/VSCode.cpp @@ -41,6 +41,8 @@ VSCode::VSCode() {"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}), focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false), stop_at_entry(false), is_attach(false), + enable_auto_variable_summaries(false), + enable_synthetic_child_debugging(false), restarting_process_id(LLDB_INVALID_PROCESS_ID), configuration_done_sent(false), waiting_for_run_in_terminal(false), progress_event_reporter( diff --git a/lldb/tools/lldb-vscode/VSCode.h b/lldb/tools/lldb-vscode/VSCode.h index 730223046b3c..59bb11c71e67 100644 --- a/lldb/tools/lldb-vscode/VSCode.h +++ b/lldb/tools/lldb-vscode/VSCode.h @@ -167,6 +167,8 @@ struct VSCode { std::atomic sent_terminated_event; bool stop_at_entry; bool is_attach; + bool enable_auto_variable_summaries; + bool enable_synthetic_child_debugging; // The process event thread normally responds to process exited events by // shutting down the entire adapter. When we're restarting, we keep the id of // the old process here so we can detect this case and keep running. diff --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp b/lldb/tools/lldb-vscode/lldb-vscode.cpp index 5480edd74f24..3904d430c49b 100644 --- a/lldb/tools/lldb-vscode/lldb-vscode.cpp +++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp @@ -647,6 +647,10 @@ void request_attach(const llvm::json::Object &request) { std::vector postRunCommands = GetStrings(arguments, "postRunCommands"); const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot"); + g_vsc.enable_auto_variable_summaries = + GetBoolean(arguments, "enableAutoVariableSummaries", false); + g_vsc.enable_synthetic_child_debugging = + GetBoolean(arguments, "enableSyntheticChildDebugging", false); // This is a hack for loading DWARF in .o files on Mac where the .o files // in the debug map of the main executable have relative paths which require @@ -1794,6 +1798,10 @@ void request_launch(const llvm::json::Object &request) { GetStrings(arguments, "postRunCommands"); g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false); const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot"); + g_vsc.enable_auto_variable_summaries = + GetBoolean(arguments, "enableAutoVariableSummaries", false); + g_vsc.enable_synthetic_child_debugging = + GetBoolean(arguments, "enableSyntheticChildDebugging", false); // This is a hack for loading DWARF in .o files on Mac where the .o files // in the debug map of the main executable have relative paths which require @@ -3292,7 +3300,8 @@ void request_variables(const llvm::json::Object &request) { // "[raw]" child that can be used to inspect the raw version of a // synthetic member. That eliminates the need for the user to go to the // debug console and type `frame var to get these values. - if (variable.IsSynthetic() && i == num_children) + if (g_vsc.enable_synthetic_child_debugging && variable.IsSynthetic() && + i == num_children) addChild(variable.GetNonSyntheticValue(), "[raw]"); } } diff --git a/lldb/tools/lldb-vscode/package.json b/lldb/tools/lldb-vscode/package.json index cc6df03097a7..1b3d452f92ab 100644 --- a/lldb/tools/lldb-vscode/package.json +++ b/lldb/tools/lldb-vscode/package.json @@ -240,6 +240,16 @@ "timeout": { "type": "string", "description": "The time in seconds to wait for a program to stop at entry point when launching with \"launchCommands\". Defaults to 30 seconds." + }, + "enableAutoVariableSummaries": { + "type": "boolean", + "description": "Enable auto generated summaries for variables when no summaries exist for a given type. This feature can cause performance delays in large projects when viewing variables.", + "default": false + }, + "enableSyntheticChildDebugging": { + "type": "boolean", + "description": "If a variable is displayed using a synthetic children, also display the actual contents of the variable at the end under a [raw] entry. This is useful when creating sythetic child plug-ins as it lets you see the actual contents of the variable.", + "default": false } } }, @@ -319,6 +329,16 @@ "timeout": { "type": "string", "description": "The time in seconds to wait for a program to stop when attaching using \"attachCommands\". Defaults to 30 seconds." + }, + "enableAutoVariableSummaries": { + "type": "boolean", + "description": "Enable auto generated summaries for variables when no summaries exist for a given type. This feature can cause performance delays in large projects when viewing variables.", + "default": false + }, + "enableSyntheticChildDebugging": { + "type": "boolean", + "description": "If a variable is displayed using a synthetic children, also display the actual contents of the variable at the end under a [raw] entry. This is useful when creating sythetic child plug-ins as it lets you see the actual contents of the variable.", + "default": false } } }