yinghuitan updated this revision to Diff 361815.
yinghuitan marked 4 inline comments as done.
yinghuitan added a comment.
Fix the issue in setVariable request.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105166/new/
https://reviews.llvm.org/D105166
Files:
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
lldb/tools/lldb-vscode/VSCode.cpp
lldb/tools/lldb-vscode/VSCode.h
lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===================================================================
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -107,6 +107,19 @@
enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
+lldb::SBValueList *GetTopLevelScope(int64_t variablesReference) {
+ switch (variablesReference) {
+ case VARREF_LOCALS:
+ return &g_vsc.variables.locals;
+ case VARREF_GLOBALS:
+ return &g_vsc.variables.globals;
+ case VARREF_REGS:
+ return &g_vsc.variables.registers;
+ default:
+ return nullptr;
+ }
+}
+
SOCKET AcceptConnection(int portno) {
// Accept a socket connection from any host on "portno".
SOCKET newsockfd = -1;
@@ -439,6 +452,7 @@
}
break;
case lldb::eStateRunning:
+ g_vsc.WillContinue();
break;
case lldb::eStateExited: {
// Run any exit LLDB commands the user specified in the
@@ -1196,6 +1210,10 @@
lldb::SBValue value = frame.GetValueForVariablePath(
expression.data(), lldb::eDynamicDontRunTarget);
+ // Freeze dry the value in case users expand it later in the debug console
+ if (value.GetError().Success() && context == "repl")
+ value = value.Persist();
+
if (value.GetError().Fail() && context != "hover")
value = frame.EvaluateExpression(expression.data());
@@ -1215,9 +1233,9 @@
EmplaceSafeString(body, "type",
value_typename ? value_typename : NO_TYPENAME);
if (value.MightHaveChildren()) {
- auto variablesReference = VARIDX_TO_VARREF(g_vsc.variables.GetSize());
- g_vsc.variables.Append(value);
- body.try_emplace("variablesReference", variablesReference);
+ auto variableReference = g_vsc.variables.InsertExpandableVariable(
+ value, /*is_permanent=*/context == "repl");
+ body.try_emplace("variablesReference", variableReference);
} else {
body.try_emplace("variablesReference", (int64_t)0);
}
@@ -1895,20 +1913,15 @@
frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
frame.GetThread().SetSelectedFrame(frame.GetFrameID());
}
- g_vsc.variables.Clear();
- g_vsc.variables.Append(frame.GetVariables(true, // arguments
- true, // locals
- false, // statics
- true)); // in_scope_only
- g_vsc.num_locals = g_vsc.variables.GetSize();
- g_vsc.variables.Append(frame.GetVariables(false, // arguments
- false, // locals
- true, // statics
- true)); // in_scope_only
- g_vsc.num_globals = g_vsc.variables.GetSize() - (g_vsc.num_locals);
- g_vsc.variables.Append(frame.GetRegisters());
- g_vsc.num_regs =
- g_vsc.variables.GetSize() - (g_vsc.num_locals + g_vsc.num_globals);
+ g_vsc.variables.locals = frame.GetVariables(/*arguments=*/true,
+ /*locals=*/true,
+ /*statics=*/false,
+ /*in_scope_only=*/true);
+ g_vsc.variables.globals = frame.GetVariables(/*arguments=*/false,
+ /*locals=*/false,
+ /*statics=*/true,
+ /*in_scope_only=*/true);
+ g_vsc.variables.registers = frame.GetRegisters();
body.try_emplace("scopes", g_vsc.CreateTopLevelScopes());
response.try_emplace("body", std::move(body));
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
@@ -2750,37 +2763,20 @@
// of the variable within the g_vsc.variables list.
const auto id_value = GetUnsigned(arguments, "id", UINT64_MAX);
if (id_value != UINT64_MAX) {
- variable = g_vsc.variables.GetValueAtIndex(id_value);
- } else if (VARREF_IS_SCOPE(variablesReference)) {
+ variable = g_vsc.variables.GetVariable(id_value);
+ } else if (lldb::SBValueList *top_scope =
+ GetTopLevelScope(variablesReference)) {
// variablesReference is one of our scopes, not an actual variable it is
// asking for a variable in locals or globals or registers
- int64_t start_idx = 0;
- int64_t end_idx = 0;
- switch (variablesReference) {
- case VARREF_LOCALS:
- start_idx = 0;
- end_idx = start_idx + g_vsc.num_locals;
- break;
- case VARREF_GLOBALS:
- start_idx = g_vsc.num_locals;
- end_idx = start_idx + g_vsc.num_globals;
- break;
- case VARREF_REGS:
- start_idx = g_vsc.num_locals + g_vsc.num_globals;
- end_idx = start_idx + g_vsc.num_regs;
- break;
- default:
- break;
- }
-
- for (int64_t i = end_idx - 1; i >= start_idx; --i) {
- lldb::SBValue curr_variable = g_vsc.variables.GetValueAtIndex(i);
+ int64_t end_idx = top_scope->GetSize();
+ // Searching backward so that we choose the variable in closest scope
+ // among variables of the same name.
+ for (int64_t i = end_idx - 1; i >= 0; --i) {
+ lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
std::string variable_name = CreateUniqueVariableNameForDisplay(
curr_variable, is_duplicated_variable_name);
if (variable_name == name) {
variable = curr_variable;
- if (curr_variable.MightHaveChildren())
- newVariablesReference = i;
break;
}
}
@@ -2790,8 +2786,7 @@
// We have a named item within an actual variable so we need to find it
// withing the container variable by name.
- const int64_t var_idx = VARREF_TO_VARIDX(variablesReference);
- lldb::SBValue container = g_vsc.variables.GetValueAtIndex(var_idx);
+ lldb::SBValue container = g_vsc.variables.GetVariable(variablesReference);
variable = container.GetChildMemberWithName(name.data());
if (!variable.IsValid()) {
if (name.startswith("[")) {
@@ -2803,14 +2798,6 @@
}
}
}
-
- // We don't know the index of the variable in our g_vsc.variables
- if (variable.IsValid()) {
- if (variable.MightHaveChildren()) {
- newVariablesReference = VARIDX_TO_VARREF(g_vsc.variables.GetSize());
- g_vsc.variables.Append(variable);
- }
- }
}
if (variable.IsValid()) {
@@ -2819,6 +2806,15 @@
if (success) {
SetValueForKey(variable, body, "value");
EmplaceSafeString(body, "type", variable.GetType().GetDisplayTypeName());
+
+ // We don't know the index of the variable in our g_vsc.variables
+ // so always insert a new one to get its variablesReference.
+ // is_permanent is false because debug console does not support
+ // setVariable request.
+ if (variable.MightHaveChildren())
+ newVariablesReference = g_vsc.variables.InsertExpandableVariable(
+ variable, /*is_permanent=*/false);
+
body.try_emplace("variablesReference", newVariablesReference);
} else {
EmplaceSafeString(body, "message", std::string(error.GetCString()));
@@ -2919,33 +2915,19 @@
if (format)
hex = GetBoolean(format, "hex", false);
- if (VARREF_IS_SCOPE(variablesReference)) {
+ if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
// variablesReference is one of our scopes, not an actual variable it is
// asking for the list of args, locals or globals.
int64_t start_idx = 0;
int64_t num_children = 0;
- switch (variablesReference) {
- case VARREF_LOCALS:
- start_idx = start;
- num_children = g_vsc.num_locals;
- break;
- case VARREF_GLOBALS:
- start_idx = start + g_vsc.num_locals + start;
- num_children = g_vsc.num_globals;
- break;
- case VARREF_REGS:
- start_idx = start + g_vsc.num_locals + g_vsc.num_globals;
- num_children = g_vsc.num_regs;
- break;
- default:
- break;
- }
+
+ num_children = top_scope->GetSize();
const int64_t end_idx = start_idx + ((count == 0) ? num_children : count);
// We first find out which variable names are duplicated
std::map<std::string, int> variable_name_counts;
for (auto i = start_idx; i < end_idx; ++i) {
- lldb::SBValue variable = g_vsc.variables.GetValueAtIndex(i);
+ lldb::SBValue variable = top_scope->GetValueAtIndex(i);
if (!variable.IsValid())
break;
variable_name_counts[GetNonNullVariableName(variable)]++;
@@ -2953,19 +2935,24 @@
// Now we construct the result with unique display variable names
for (auto i = start_idx; i < end_idx; ++i) {
- lldb::SBValue variable = g_vsc.variables.GetValueAtIndex(i);
+ lldb::SBValue variable = top_scope->GetValueAtIndex(i);
if (!variable.IsValid())
break;
- variables.emplace_back(CreateVariable(variable, VARIDX_TO_VARREF(i), i,
- hex,
+
+ int64_t var_ref = 0;
+ if (variable.MightHaveChildren()) {
+ var_ref = g_vsc.variables.InsertExpandableVariable(
+ variable, /*is_permanent=*/false);
+ }
+ variables.emplace_back(CreateVariable(
+ variable, var_ref, var_ref != 0 ? var_ref : UINT64_MAX, hex,
variable_name_counts[GetNonNullVariableName(variable)] > 1));
}
} else {
// We are expanding a variable that has children, so we will return its
// children.
- const int64_t var_idx = VARREF_TO_VARIDX(variablesReference);
- lldb::SBValue variable = g_vsc.variables.GetValueAtIndex(var_idx);
+ lldb::SBValue variable = g_vsc.variables.GetVariable(variablesReference);
if (variable.IsValid()) {
const auto num_children = variable.GetNumChildren();
const int64_t end_idx = start + ((count == 0) ? num_children : count);
@@ -2974,11 +2961,12 @@
if (!child.IsValid())
break;
if (child.MightHaveChildren()) {
- const int64_t var_idx = g_vsc.variables.GetSize();
- auto childVariablesReferences = VARIDX_TO_VARREF(var_idx);
- variables.emplace_back(
- CreateVariable(child, childVariablesReferences, var_idx, hex));
- g_vsc.variables.Append(child);
+ auto is_permanent =
+ g_vsc.variables.IsPermanentVariableReference(variablesReference);
+ auto childVariablesReferences =
+ g_vsc.variables.InsertExpandableVariable(child, is_permanent);
+ variables.emplace_back(CreateVariable(child, childVariablesReferences,
+ childVariablesReferences, hex));
} else {
variables.emplace_back(CreateVariable(child, 0, INT64_MAX, hex));
}
Index: lldb/tools/lldb-vscode/VSCode.h
===================================================================
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -58,9 +58,6 @@
#define VARREF_GLOBALS (int64_t)2
#define VARREF_REGS (int64_t)3
#define VARREF_FIRST_VAR_IDX (int64_t)4
-#define VARREF_IS_SCOPE(v) (VARREF_LOCALS <= 1 && v < VARREF_FIRST_VAR_IDX)
-#define VARIDX_TO_VARREF(i) ((i) + VARREF_FIRST_VAR_IDX)
-#define VARREF_TO_VARIDX(v) ((v)-VARREF_FIRST_VAR_IDX)
#define NO_TYPENAME "<no-type>"
namespace lldb_vscode {
@@ -83,17 +80,54 @@
JSONNotObject
};
+struct Variables {
+ /// Variable_reference start index of permanent expandable variable.
+ static constexpr int64_t PermanentVariableStartIndex = (1ll << 32);
+
+ lldb::SBValueList locals;
+ lldb::SBValueList globals;
+ lldb::SBValueList registers;
+
+ int64_t next_temporary_var_ref{VARREF_FIRST_VAR_IDX};
+ int64_t next_permanent_var_ref{PermanentVariableStartIndex};
+
+ /// Expandable variables that are alive in this stop state.
+ /// Will be cleared when debuggee resumes.
+ llvm::DenseMap<int64_t, lldb::SBValue> expandable_variables;
+ /// Expandable variables that persist across entire debug session.
+ /// These are the variables evaluated from debug console REPL.
+ llvm::DenseMap<int64_t, lldb::SBValue> expandable_permanent_variables;
+
+ /// Check if \p var_ref points to a variable that should persist for the
+ /// entire duration of the debug session, e.g. repl expandable variables
+ static bool IsPermanentVariableReference(int64_t var_ref);
+
+ /// \return a new variableReference.
+ /// Specify is_permanent as true for variable that should persist entire
+ /// debug session.
+ int64_t GetNewVariableRefence(bool is_permanent);
+
+ /// \return the expandable variable corresponding with variableReference
+ /// value of \p value.
+ /// If \p var_ref is invalid an empty SBValue is returned.
+ lldb::SBValue GetVariable(int64_t var_ref) const;
+
+ /// Insert a new \p variable.
+ /// \return variableReference assigned to this expandable variable.
+ int64_t InsertExpandableVariable(lldb::SBValue variable, bool is_permanent);
+
+ /// Clear all scope variables and non-permanent expandable variables.
+ void Clear();
+};
+
struct VSCode {
std::string debug_adaptor_path;
InputStream input;
OutputStream output;
lldb::SBDebugger debugger;
lldb::SBTarget target;
- lldb::SBValueList variables;
+ Variables variables;
lldb::SBBroadcaster broadcaster;
- int64_t num_regs;
- int64_t num_locals;
- int64_t num_globals;
std::thread event_thread;
std::thread progress_event_thread;
std::unique_ptr<std::ofstream> log;
@@ -206,6 +240,9 @@
/// IDE.
void RegisterRequestCallback(std::string request, RequestCallback callback);
+ /// Debuggee will continue from stopped state.
+ void WillContinue() { variables.Clear(); }
+
private:
// Send the JSON in "json_str" to the "out" stream. Correctly send the
// "Content-Length:" field followed by the length, followed by the raw
Index: lldb/tools/lldb-vscode/VSCode.cpp
===================================================================
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -30,8 +30,7 @@
VSCode g_vsc;
VSCode::VSCode()
- : variables(), broadcaster("lldb-vscode"), num_regs(0), num_locals(0),
- num_globals(0), log(),
+ : broadcaster("lldb-vscode"),
exception_breakpoints(
{{"cpp_catch", "C++ Catch", lldb::eLanguageTypeC_plus_plus},
{"cpp_throw", "C++ Throw", lldb::eLanguageTypeC_plus_plus},
@@ -382,10 +381,12 @@
llvm::json::Value VSCode::CreateTopLevelScopes() {
llvm::json::Array scopes;
- scopes.emplace_back(CreateScope("Locals", VARREF_LOCALS, num_locals, false));
- scopes.emplace_back(
- CreateScope("Globals", VARREF_GLOBALS, num_globals, false));
- scopes.emplace_back(CreateScope("Registers", VARREF_REGS, num_regs, false));
+ scopes.emplace_back(CreateScope("Locals", VARREF_LOCALS,
+ g_vsc.variables.locals.GetSize(), false));
+ scopes.emplace_back(CreateScope("Globals", VARREF_GLOBALS,
+ g_vsc.variables.globals.GetSize(), false));
+ scopes.emplace_back(CreateScope("Registers", VARREF_REGS,
+ g_vsc.variables.registers.GetSize(), false));
return llvm::json::Value(std::move(scopes));
}
@@ -527,4 +528,44 @@
request_handlers[request] = callback;
}
+void Variables::Clear() {
+ locals.Clear();
+ globals.Clear();
+ registers.Clear();
+ expandable_variables.clear();
+}
+
+int64_t Variables::GetNewVariableRefence(bool is_permanent) {
+ if (is_permanent)
+ return next_permanent_var_ref++;
+ return next_temporary_var_ref++;
+}
+
+bool Variables::IsPermanentVariableReference(int64_t var_ref) {
+ return var_ref >= PermanentVariableStartIndex;
+}
+
+lldb::SBValue Variables::GetVariable(int64_t var_ref) const {
+ if (IsPermanentVariableReference(var_ref)) {
+ auto pos = expandable_permanent_variables.find(var_ref);
+ if (pos != expandable_permanent_variables.end())
+ return pos->second;
+ } else {
+ auto pos = expandable_variables.find(var_ref);
+ if (pos != expandable_variables.end())
+ return pos->second;
+ }
+ return lldb::SBValue();
+}
+
+int64_t Variables::InsertExpandableVariable(lldb::SBValue variable,
+ bool is_permanent) {
+ int64_t var_ref = GetNewVariableRefence(is_permanent);
+ if (is_permanent)
+ expandable_permanent_variables.insert(std::make_pair(var_ref, variable));
+ else
+ expandable_variables.insert(std::make_pair(var_ref, variable));
+ return var_ref;
+}
+
} // namespace lldb_vscode
Index: lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
===================================================================
--- lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -23,7 +23,7 @@
mydir = TestBase.compute_mydir(__file__)
- def verify_values(self, verify_dict, actual, varref_dict=None):
+ def verify_values(self, verify_dict, actual, varref_dict=None, expression=None):
if 'equals' in verify_dict:
verify = verify_dict['equals']
for key in verify:
@@ -48,9 +48,13 @@
if hasVariablesReference:
# Remember variable references in case we want to test further
# by using the evaluate name.
- varRef = actual['variablesReference']
+ varRef = actual["variablesReference"]
if varRef != 0 and varref_dict is not None:
- varref_dict[actual['evaluateName']] = varRef
+ if expression is None:
+ evaluateName = actual["evaluateName"]
+ else:
+ evaluateName = expression
+ varref_dict[evaluateName] = varRef
if ('hasVariablesReference' in verify_dict and
verify_dict['hasVariablesReference']):
self.assertTrue(hasVariablesReference,
@@ -282,3 +286,111 @@
self.assertNotIn('x @ main.cpp:21', names)
self.verify_variables(verify_locals, locals)
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_scopes_and_evaluate_expansion(self):
+ """
+ Tests the evaluated expression expands successfully after "scopes" packets
+ and permanent expressions persist.
+ """
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ source = "main.cpp"
+ breakpoint1_line = line_number(source, "// breakpoint 1")
+ lines = [breakpoint1_line]
+ # Set breakpoint in the thread function so we can step the threads
+ breakpoint_ids = self.set_source_breakpoints(source, lines)
+ self.assertEqual(
+ len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
+ )
+ self.continue_to_breakpoints(breakpoint_ids)
+
+ # Verify locals
+ locals = self.vscode.get_local_variables()
+ buffer_children = make_buffer_verify_dict(0, 32)
+ verify_locals = {
+ "argc": {"equals": {"type": "int", "value": "1"}},
+ "argv": {
+ "equals": {"type": "const char **"},
+ "startswith": {"value": "0x"},
+ "hasVariablesReference": True,
+ },
+ "pt": {
+ "equals": {"type": "PointType"},
+ "hasVariablesReference": True,
+ "children": {
+ "x": {"equals": {"type": "int", "value": "11"}},
+ "y": {"equals": {"type": "int", "value": "22"}},
+ "buffer": {"children": buffer_children},
+ },
+ },
+ "x": {"equals": {"type": "int"}},
+ }
+ self.verify_variables(verify_locals, locals)
+
+ # Evaluate expandable expression twice: once permanent (from repl)
+ # the other temporary (from other UI).
+ expandable_expression = {
+ "name": "pt",
+ "response": {
+ "equals": {"type": "PointType"},
+ "startswith": {"result": "PointType @ 0x"},
+ "hasVariablesReference": True,
+ },
+ "children": {
+ "x": {"equals": {"type": "int", "value": "11"}},
+ "y": {"equals": {"type": "int", "value": "22"}},
+ "buffer": {"children": buffer_children},
+ },
+ }
+
+ # Evaluate from permanent UI.
+ permanent_expr_varref_dict = {}
+ response = self.vscode.request_evaluate(
+ expandable_expression["name"], frameIndex=0, threadId=None, context="repl"
+ )
+ self.verify_values(
+ expandable_expression["response"],
+ response["body"],
+ permanent_expr_varref_dict,
+ expandable_expression["name"],
+ )
+
+ # Evaluate from temporary UI.
+ temporary_expr_varref_dict = {}
+ response = self.vscode.request_evaluate(expandable_expression["name"])
+ self.verify_values(
+ expandable_expression["response"],
+ response["body"],
+ temporary_expr_varref_dict,
+ expandable_expression["name"],
+ )
+
+ # Evaluate locals again.
+ locals = self.vscode.get_local_variables()
+ self.verify_variables(verify_locals, locals)
+
+ # Verify the evaluated expressions before second locals evaluation
+ # can be expanded.
+ var_ref = temporary_expr_varref_dict[expandable_expression["name"]]
+ response = self.vscode.request_variables(var_ref)
+ self.verify_variables(
+ expandable_expression["children"], response["body"]["variables"]
+ )
+
+ # Continue to breakpoint 3, permanent variable should still exist
+ # after resume.
+ breakpoint3_line = line_number(source, "// breakpoint 3")
+ lines = [breakpoint3_line]
+ breakpoint_ids = self.set_source_breakpoints(source, lines)
+ self.assertEqual(
+ len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
+ )
+ self.continue_to_breakpoints(breakpoint_ids)
+
+ var_ref = permanent_expr_varref_dict[expandable_expression["name"]]
+ response = self.vscode.request_variables(var_ref)
+ self.verify_variables(
+ expandable_expression["children"], response["body"]["variables"]
+ )
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
