yinghuitan updated this revision to Diff 355942.
yinghuitan marked 13 inline comments as done.
yinghuitan added a comment.
Address review comments and add testcase.
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;
@@ -727,6 +740,7 @@
// Remember the thread ID that caused the resume so we can set the
// "threadCausedFocus" boolean value in the "stopped" events.
g_vsc.focus_tid = GetUnsigned(arguments, "threadId", LLDB_INVALID_THREAD_ID);
+ g_vsc.WillContinue();
lldb::SBError error = process.Continue();
llvm::json::Object body;
body.try_emplace("allThreadsContinued", true);
@@ -1215,9 +1229,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.InsertNewExpandableVariable(
+ value, /*is_permanent=*/context == "repl");
+ body.try_emplace("variablesReference", variableReference);
} else {
body.try_emplace("variablesReference", (int64_t)0);
}
@@ -1769,6 +1783,7 @@
// Remember the thread ID that caused the resume so we can set the
// "threadCausedFocus" boolean value in the "stopped" events.
g_vsc.focus_tid = thread.GetThreadID();
+ g_vsc.WillContinue();
thread.StepOver();
} else {
response["success"] = llvm::json::Value(false);
@@ -1895,20 +1910,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)));
@@ -2523,6 +2533,7 @@
// Remember the thread ID that caused the resume so we can set the
// "threadCausedFocus" boolean value in the "stopped" events.
g_vsc.focus_tid = thread.GetThreadID();
+ g_vsc.WillContinue();
thread.StepInto();
} else {
response["success"] = llvm::json::Value(false);
@@ -2575,6 +2586,7 @@
// Remember the thread ID that caused the resume so we can set the
// "threadCausedFocus" boolean value in the "stopped" events.
g_vsc.focus_tid = thread.GetThreadID();
+ g_vsc.WillContinue();
thread.StepOut();
} else {
response["success"] = llvm::json::Value(false);
@@ -2750,37 +2762,22 @@
// 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;
+ newVariablesReference = g_vsc.variables.GetNewVariableRefence(true);
break;
}
}
@@ -2790,8 +2787,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("[")) {
@@ -2807,8 +2803,9 @@
// 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);
+ auto is_permanent =
+ g_vsc.variables.IsPermanentVariableReference(variablesReference);
+ g_vsc.variables.InsertNewExpandableVariable(variable, is_permanent);
}
}
}
@@ -2919,33 +2916,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 +2936,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.InsertNewExpandableVariable(
+ 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 +2962,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.InsertNewExpandableVariable(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,48 @@
JSONNotObject
};
+struct Variables {
+ // Bit mask to tell if a variableReference is inside
+ // expandable_permanent_variables or not.
+ static constexpr int64_t PermanentVariableBitMask = (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{VARREF_FIRST_VAR_IDX};
+ llvm::DenseMap<int64_t, lldb::SBValue> expandable_variables;
+ llvm::DenseMap<int64_t, lldb::SBValue> expandable_permanent_variables;
+
+ /// Check if \p var_ref points to variable in a
+ /// expandable_permanent_variables.
+ static bool IsPermanentVariableReference(int64_t var_ref);
+
+ /// \return a new variableReference. If is_permanent is true the returned
+ /// value will have the PermanentVariableBitMask bit set.
+ int64_t GetNewVariableRefence(bool is_permanent);
+
+ /// \return the expandable variable corresponding with variableReference value
+ /// of \p value.
+ lldb::SBValue GetVariable(int64_t var_ref) const;
+
+ /// Insert a new \p variable.
+ int64_t InsertNewExpandableVariable(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 +234,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();
+ next_temporary_var_ref = VARREF_FIRST_VAR_IDX;
+}
+
+int64_t Variables::GetNewVariableRefence(bool is_permanent) {
+ return is_permanent ? (PermanentVariableBitMask | next_permanent_var_ref++)
+ : (next_temporary_var_ref++);
+}
+
+bool Variables::IsPermanentVariableReference(int64_t var_ref) {
+ return (var_ref & PermanentVariableBitMask) != 0;
+}
+
+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::InsertNewExpandableVariable(lldb::SBValue variable,
+ bool is_permanent) {
+ auto 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:
@@ -50,7 +50,8 @@
# by using the evaluate name.
varRef = actual['variablesReference']
if varRef != 0 and varref_dict is not None:
- varref_dict[actual['evaluateName']] = varRef
+ evaluateName = expression if expression is not None else actual['evaluateName']
+ varref_dict[evaluateName] = varRef
if ('hasVariablesReference' in verify_dict and
verify_dict['hasVariablesReference']):
self.assertTrue(hasVariablesReference,
@@ -282,3 +283,107 @@
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
+ '''
+ 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)
+
+ # 1. 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)
+
+ # 2. Evluate expandable expression twice: once permanent (from repl) the other temporary (from other UI).
+ expandable_expression = {
+ 'pt': {
+ 'equals': {'type': 'PointType'},
+ 'startswith': {'result': 'PointType @ 0x'},
+ 'hasVariablesReference': True
+ },
+ }
+
+ 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}
+ }
+ }
+
+
+ # 2.1. 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'])
+
+ # 2.2. 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'])
+
+ # 3. Evaluate locals again.
+ locals = self.vscode.get_local_variables()
+ self.verify_variables(verify_locals, locals)
+
+ # 4. Verify the evaluated expressions before second locals evaluation can be expanded.
+ var_ref = temporary_expr_varref_dict[expandable_expression['name']]
+ print(f"request_variables called: {expandable_expression['name']} {var_ref}")
+ response = self.vscode.request_variables(var_ref)
+ self.verify_variables(expandable_expression['children'],
+ response['body']['variables'])
+
+
+ # 5. 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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
