yinghuitan created this revision.
yinghuitan added reviewers: clayborg, wallace.
Herald added a subscriber: jfb.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
VScode now sends a "scopes" DAP request immediately after any expression
evaluation.
This scopes request would clear and invalidate any non-scoped expandable
variables in g_vsc.variables, causing later "variables" request to return empty
result.
The symptom is that any expandable variables in VScode watch window/debug
console UI to return empty content.
This diff fixes this issue by only clearing the expandable variables at process
continue time. To achieve this, we have to repopulate all scoped variables
during context switch for each "scopes" request without clearing global
expandable variables.
So the PR puts the scoped variables into its own locals/globals/registers; and
all expandable variables into separate "expandableVariables" list.
Also, instead of using the variable index for "variableReference", it generates
a new variableReference id each time as the key of "expandableVariables".
As a further new feature, this PR adds a new "expandablePermanentVariables"
which has the lifetime of debug session. Any expandable variables from debug
console
are added into this list. This enables users to snapshot expanable old variable
in debug console and compare with new variables if desire.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D105166
Files:
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,25 @@
enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
+// Debuggee will continue from stopped state.
+void willContinue() {
+ g_vsc.variableHolder.clear();
+}
+
+lldb::SBValueList* getScopeRef(int64_t variablesReference) {
+ assert(VARREF_IS_SCOPE(variablesReference));
+ switch (variablesReference) {
+ case VARREF_LOCALS:
+ return &g_vsc.variableHolder.locals;
+ case VARREF_GLOBALS:
+ return &g_vsc.variableHolder.globals;
+ case VARREF_REGS:
+ return &g_vsc.variableHolder.registers;
+ default:
+ return nullptr;
+ }
+}
+
SOCKET AcceptConnection(int portno) {
// Accept a socket connection from any host on "portno".
SOCKET newsockfd = -1;
@@ -727,6 +746,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);
+ willContinue();
lldb::SBError error = process.Continue();
llvm::json::Object body;
body.try_emplace("allThreadsContinued", true);
@@ -1215,9 +1235,8 @@
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.variableHolder.insertNewExpandableVariable(value, /*isPermanent*/context == "repl");
+ body.try_emplace("variablesReference", variableReference);
} else {
body.try_emplace("variablesReference", (int64_t)0);
}
@@ -1769,6 +1788,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();
+ willContinue();
thread.StepOver();
} else {
response["success"] = llvm::json::Value(false);
@@ -1895,20 +1915,15 @@
frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
frame.GetThread().SetSelectedFrame(frame.GetFrameID());
}
- g_vsc.variables.Clear();
- g_vsc.variables.Append(frame.GetVariables(true, // arguments
+ g_vsc.variableHolder.locals = 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
+ true); // in_scope_only
+ g_vsc.variableHolder.globals = 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);
+ true); // in_scope_only
+ g_vsc.variableHolder.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 +2538,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();
+ willContinue();
thread.StepInto();
} else {
response["success"] = llvm::json::Value(false);
@@ -2575,6 +2591,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();
+ willContinue();
thread.StepOut();
} else {
response["success"] = llvm::json::Value(false);
@@ -2750,37 +2767,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);
+ variable = g_vsc.variableHolder.getVariableFromVariableReference(id_value);
} else if (VARREF_IS_SCOPE(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;
- }
+ lldb::SBValueList *pScopeRef = getScopeRef(variablesReference);
for (int64_t i = end_idx - 1; i >= start_idx; --i) {
- lldb::SBValue curr_variable = g_vsc.variables.GetValueAtIndex(i);
+ lldb::SBValue curr_variable = pScopeRef->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.variableHolder.getNewVariableRefence(true);
break;
}
}
@@ -2790,8 +2792,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.variableHolder.getVariableFromVariableReference(variablesReference);
variable = container.GetChildMemberWithName(name.data());
if (!variable.IsValid()) {
if (name.startswith("[")) {
@@ -2807,8 +2808,8 @@
// 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 isPermanent = g_vsc.variableHolder.isPermanentVariableReference(variablesReference);
+ g_vsc.variableHolder.insertNewExpandableVariable(variable, isPermanent);
}
}
}
@@ -2924,28 +2925,16 @@
// 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;
- }
+ lldb::SBValueList *pScopeRef = getScopeRef(variablesReference);
+ assert(pScopeRef != nullptr);
+ num_children = pScopeRef->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 = pScopeRef->GetValueAtIndex(i);
if (!variable.IsValid())
break;
variable_name_counts[GetNonNullVariableName(variable)]++;
@@ -2953,19 +2942,23 @@
// 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 = pScopeRef->GetValueAtIndex(i);
if (!variable.IsValid())
break;
- variables.emplace_back(CreateVariable(variable, VARIDX_TO_VARREF(i), i,
+
+ int64_t variableReference = 0;
+ if (variable.MightHaveChildren()) {
+ variableReference = g_vsc.variableHolder.insertNewExpandableVariable(variable, /*isPermanent*/false);
+ }
+ variables.emplace_back(CreateVariable(variable, variableReference, variableReference != 0? variableReference: 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.variableHolder.getVariableFromVariableReference(variablesReference);
if (variable.IsValid()) {
const auto num_children = variable.GetNumChildren();
const int64_t end_idx = start + ((count == 0) ? num_children : count);
@@ -2974,11 +2967,10 @@
if (!child.IsValid())
break;
if (child.MightHaveChildren()) {
- const int64_t var_idx = g_vsc.variables.GetSize();
- auto childVariablesReferences = VARIDX_TO_VARREF(var_idx);
+ auto isPermanent = g_vsc.variableHolder.isPermanentVariableReference(variablesReference);
+ auto childVariablesReferences = g_vsc.variableHolder.insertNewExpandableVariable(child, isPermanent);
variables.emplace_back(
- CreateVariable(child, childVariablesReferences, var_idx, hex));
- g_vsc.variables.Append(child);
+ 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
@@ -59,8 +59,6 @@
#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 +81,43 @@
JSONNotObject
};
+struct VariablesHolder {
+ // Bit tag bit to tell if a variableReference is inside expandablePermanentVariables or not.
+ static constexpr int PermanentVariableReferenceTagBit = 32;
+
+ lldb::SBValueList locals;
+ lldb::SBValueList globals;
+ lldb::SBValueList registers;
+
+ int64_t nextVariableReferenceSeed{VARREF_FIRST_VAR_IDX};
+ llvm::DenseMap<int64_t, lldb::SBValue> expandableVariables;
+ llvm::DenseMap<int64_t, lldb::SBValue> expandablePermanentVariables;
+
+ // Check if \p variableReference points to variable in a expandablePermanentVariables.
+ bool isPermanentVariableReference(int64_t variableReference);
+
+ // \return a new variableReference. If isPermanent is true the returned
+ // value will have PermanentVariableReferenceTagBit set.
+ int64_t getNewVariableRefence(bool isPermanent);
+
+ // \return the expandable variable corresponding with variableReference value of \p value.
+ lldb::SBValue getVariableFromVariableReference(int64_t value);
+
+ // Insert a new \p expandableVariable.
+ int64_t insertNewExpandableVariable(lldb::SBValue expandableVariable, bool isPermanent);
+
+ // 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;
+ VariablesHolder variableHolder;
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;
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"), log(),
exception_breakpoints(
{{"cpp_catch", "C++ Catch", lldb::eLanguageTypeC_plus_plus},
{"cpp_throw", "C++ Throw", lldb::eLanguageTypeC_plus_plus},
@@ -382,10 +381,10 @@
llvm::json::Value VSCode::CreateTopLevelScopes() {
llvm::json::Array scopes;
- scopes.emplace_back(CreateScope("Locals", VARREF_LOCALS, num_locals, false));
+ scopes.emplace_back(CreateScope("Locals", VARREF_LOCALS, g_vsc.variableHolder.locals.GetSize(), false));
scopes.emplace_back(
- CreateScope("Globals", VARREF_GLOBALS, num_globals, false));
- scopes.emplace_back(CreateScope("Registers", VARREF_REGS, num_regs, false));
+ CreateScope("Globals", VARREF_GLOBALS, g_vsc.variableHolder.globals.GetSize(), false));
+ scopes.emplace_back(CreateScope("Registers", VARREF_REGS, g_vsc.variableHolder.registers.GetSize(), false));
return llvm::json::Value(std::move(scopes));
}
@@ -527,4 +526,42 @@
request_handlers[request] = callback;
}
+void VariablesHolder::clear() {
+ this->locals.Clear();
+ this->globals.Clear();
+ this->registers.Clear();
+ this->expandableVariables.clear();
+}
+
+int64_t VariablesHolder::getNewVariableRefence(bool isPermanent) {
+ const int64_t newVariableReference = isPermanent ? ((1ll << PermanentVariableReferenceTagBit) | nextVariableReferenceSeed) : nextVariableReferenceSeed;
+ ++nextVariableReferenceSeed;
+ return newVariableReference;
+}
+
+bool VariablesHolder::isPermanentVariableReference(int64_t variableReference) {
+ return (variableReference & (1ll << PermanentVariableReferenceTagBit)) != 0;
+}
+
+lldb::SBValue VariablesHolder::getVariableFromVariableReference(int64_t value) {
+ lldb::SBValue variable;
+ auto isPermanent = isPermanentVariableReference(value);
+ if (isPermanent) {
+ variable = expandablePermanentVariables.find(value)->second;
+ } else {
+ variable = expandableVariables.find(value)->second;
+ }
+ return variable;
+}
+
+int64_t VariablesHolder::insertNewExpandableVariable(lldb::SBValue expandableVariable, bool isPermanent) {
+ auto newVariablesReferences = getNewVariableRefence(isPermanent);
+ if (isPermanent) {
+ expandablePermanentVariables.insert(std::make_pair(newVariablesReferences, expandableVariable));
+ } else {
+ expandableVariables.insert(std::make_pair(newVariablesReferences, expandableVariable));
+ }
+ return newVariablesReferences;
+}
+
} // namespace lldb_vscode
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
