[llvm-branch-commits] [lldb] release/22.x: lldb-dap: Stop using replicated variable ids (#124232) (PR #180031)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Feb 5 12:55:49 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: None (llvmbot)
<details>
<summary>Changes</summary>
Backport 6ca8f79fc88971412e6e5692ea86d680cd438fe3
Requested by: @<!-- -->da-viper
---
Patch is 29.84 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/180031.diff
10 Files Affected:
- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+30-2)
- (modified) lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py (+3-1)
- (modified) lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py (+36)
- (modified) lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp (+3-56)
- (modified) lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp (+12-9)
- (modified) lldb/tools/lldb-dap/JSONUtils.cpp (-85)
- (modified) lldb/tools/lldb-dap/SourceBreakpoint.cpp (-1)
- (modified) lldb/tools/lldb-dap/Variables.cpp (+116-16)
- (modified) lldb/tools/lldb-dap/Variables.h (+72-10)
- (modified) lldb/unittests/DAP/VariablesTest.cpp (+47-10)
``````````diff
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index e4a3e1c786ffe..5dd2405a3389b 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -379,11 +379,39 @@ def set_variable(self, varRef, name, value, id=None):
def set_local(self, name, value, id=None):
"""Set a top level local variable only."""
- return self.set_variable(1, name, str(value), id=id)
+ # Get the locals scope reference dynamically
+ locals_ref = self.get_locals_scope_reference()
+ if locals_ref is None:
+ return None
+ return self.set_variable(locals_ref, name, str(value), id=id)
def set_global(self, name, value, id=None):
"""Set a top level global variable only."""
- return self.set_variable(2, name, str(value), id=id)
+ # Get the globals scope reference dynamically
+ stackFrame = self.dap_server.get_stackFrame()
+ if stackFrame is None:
+ return None
+ frameId = stackFrame["id"]
+ scopes_response = self.dap_server.request_scopes(frameId)
+ frame_scopes = scopes_response["body"]["scopes"]
+ for scope in frame_scopes:
+ if scope["name"] == "Globals":
+ varRef = scope["variablesReference"]
+ return self.set_variable(varRef, name, str(value), id=id)
+ return None
+
+ def get_locals_scope_reference(self):
+ """Get the variablesReference for the locals scope."""
+ stackFrame = self.dap_server.get_stackFrame()
+ if stackFrame is None:
+ return None
+ frameId = stackFrame["id"]
+ scopes_response = self.dap_server.request_scopes(frameId)
+ frame_scopes = scopes_response["body"]["scopes"]
+ for scope in frame_scopes:
+ if scope["name"] == "Locals":
+ return scope["variablesReference"]
+ return None
def stepIn(
self,
diff --git a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
index df029ca16d667..0c8ba4e5d07af 100644
--- a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
@@ -111,8 +111,10 @@ def test_functionality(self):
self.set_source_breakpoints(source, [first_loop_break_line])
self.continue_to_next_stop()
self.dap_server.get_local_variables()
+ locals_ref = self.get_locals_scope_reference()
+ self.assertIsNotNone(locals_ref, "Failed to get locals scope reference")
# Test write watchpoints on x, arr[2]
- response_x = self.dap_server.request_dataBreakpointInfo(1, "x")
+ response_x = self.dap_server.request_dataBreakpointInfo(locals_ref, "x")
arr = self.dap_server.get_local_variable("arr")
response_arr_2 = self.dap_server.request_dataBreakpointInfo(
arr["variablesReference"], "[2]"
diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
index 05445af40aea5..1dbb0143e7a55 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -834,3 +834,39 @@ def test_value_format(self):
self.assertEqual(var_pt_x["value"], "11")
var_pt_y = self.dap_server.get_local_variable_child("pt", "y", is_hex=is_hex)
self.assertEqual(var_pt_y["value"], "22")
+
+ @skipIfWindows
+ def test_variable_id_uniqueness_simple(self):
+ """
+ Simple regression test for variable ID uniqueness across frames.
+ Ensures variable IDs are not reused between different scopes/frames.
+ """
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ source = "main.cpp"
+
+ bp_line = line_number(source, "// breakpoint 3")
+ self.set_source_breakpoints(source, [bp_line])
+ self.continue_to_next_stop()
+
+ frames = self.get_stackFrames()
+ self.assertGreaterEqual(len(frames), 2, "Need at least 2 frames")
+
+ all_refs = set()
+
+ for i in range(min(3, len(frames))):
+ frame_id = frames[i]["id"]
+ scopes = self.dap_server.request_scopes(frame_id)["body"]["scopes"]
+
+ for scope in scopes:
+ ref = scope["variablesReference"]
+ if ref != 0:
+ self.assertNotIn(
+ ref, all_refs, f"Variable reference {ref} was reused!"
+ )
+ all_refs.add(ref)
+
+ self.assertGreater(len(all_refs), 0, "Should have found variable references")
+ for ref in all_refs:
+ response = self.dap_server.request_variables(ref)
+ self.assertTrue(response["success"], f"Failed to access reference {ref}")
diff --git a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp
index aaad0e20f9c21..251711ca62406 100644
--- a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp
@@ -8,51 +8,11 @@
#include "DAP.h"
#include "RequestHandler.h"
+#include "Variables.h"
using namespace lldb_dap::protocol;
namespace lldb_dap {
-/// Creates a `protocol::Scope` struct.
-///
-///
-/// \param[in] name
-/// The value to place into the "name" key
-///
-/// \param[in] variablesReference
-/// The value to place into the "variablesReference" key
-///
-/// \param[in] namedVariables
-/// The value to place into the "namedVariables" key
-///
-/// \param[in] expensive
-/// The value to place into the "expensive" key
-///
-/// \return
-/// A `protocol::Scope`
-static Scope CreateScope(const llvm::StringRef name, int64_t variablesReference,
- int64_t namedVariables, bool expensive) {
- Scope scope;
- scope.name = name;
-
- // TODO: Support "arguments" and "return value" scope.
- // At the moment lldb-dap includes the arguments and return_value into the
- // "locals" scope.
- // vscode only expands the first non-expensive scope, this causes friction
- // if we add the arguments above the local scope as the locals scope will not
- // be expanded if we enter a function with arguments. It becomes more
- // annoying when the scope has arguments, return_value and locals.
- if (variablesReference == VARREF_LOCALS)
- scope.presentationHint = Scope::eScopePresentationHintLocals;
- else if (variablesReference == VARREF_REGS)
- scope.presentationHint = Scope::eScopePresentationHintRegisters;
-
- scope.variablesReference = variablesReference;
- scope.namedVariables = namedVariables;
- scope.expensive = expensive;
-
- return scope;
-}
-
llvm::Expected<ScopesResponseBody>
ScopesRequestHandler::Run(const ScopesArguments &args) const {
lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId);
@@ -75,22 +35,9 @@ ScopesRequestHandler::Run(const ScopesArguments &args) const {
frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
frame.GetThread().SetSelectedFrame(frame.GetFrameID());
}
- dap.variables.locals = frame.GetVariables(/*arguments=*/true,
- /*locals=*/true,
- /*statics=*/false,
- /*in_scope_only=*/true);
- dap.variables.globals = frame.GetVariables(/*arguments=*/false,
- /*locals=*/false,
- /*statics=*/true,
- /*in_scope_only=*/true);
- dap.variables.registers = frame.GetRegisters();
- std::vector scopes = {CreateScope("Locals", VARREF_LOCALS,
- dap.variables.locals.GetSize(), false),
- CreateScope("Globals", VARREF_GLOBALS,
- dap.variables.globals.GetSize(), false),
- CreateScope("Registers", VARREF_REGS,
- dap.variables.registers.GetSize(), false)};
+ std::vector<protocol::Scope> scopes =
+ dap.variables.CreateScopes(args.frameId, frame);
return ScopesResponseBody{std::move(scopes)};
}
diff --git a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp
index 0177df8b754ab..ee7cb525d9c29 100644
--- a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp
@@ -11,6 +11,7 @@
#include "Handler/RequestHandler.h"
#include "JSONUtils.h"
#include "ProtocolUtils.h"
+#include "Variables.h"
using namespace llvm;
using namespace lldb_dap::protocol;
@@ -30,20 +31,22 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const {
std::vector<Variable> variables;
- if (lldb::SBValueList *top_scope = dap.variables.GetTopLevelScope(var_ref)) {
+ std::optional<ScopeData> scope_data = dap.variables.GetTopLevelScope(var_ref);
+ if (scope_data) {
// 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;
- if (var_ref == VARREF_REGS) {
+ if (scope_data->kind == eScopeKindRegisters) {
+
// Change the default format of any pointer sized registers in the first
// register set to be the lldb::eFormatAddressInfo so we show the pointer
// and resolve what the pointer resolves to. Only change the format if the
// format was set to the default format or if it was hex as some registers
// have formats set for them.
const uint32_t addr_size = dap.target.GetProcess().GetAddressByteSize();
- lldb::SBValue reg_set = dap.variables.registers.GetValueAtIndex(0);
+ lldb::SBValue reg_set = scope_data->scope.GetValueAtIndex(0);
const uint32_t num_regs = reg_set.GetNumChildren();
for (uint32_t reg_idx = 0; reg_idx < num_regs; ++reg_idx) {
lldb::SBValue reg = reg_set.GetChildAtIndex(reg_idx);
@@ -55,15 +58,15 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const {
}
}
- num_children = top_scope->GetSize();
- if (num_children == 0 && var_ref == VARREF_LOCALS) {
+ num_children = scope_data->scope.GetSize();
+ if (num_children == 0 && scope_data->kind == eScopeKindLocals) {
// Check for an error in the SBValueList that might explain why we don't
// have locals. If we have an error display it as the sole value in the
// the locals.
// "error" owns the error string so we must keep it alive as long as we
// want to use the returns "const char *"
- lldb::SBError error = top_scope->GetError();
+ lldb::SBError error = scope_data->scope.GetError();
const char *var_err = error.GetCString();
if (var_err) {
// Create a fake variable named "error" to explain why variables were
@@ -85,14 +88,14 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const {
// We first find out which variable names are duplicated
std::map<llvm::StringRef, int> variable_name_counts;
for (auto i = start_idx; i < end_idx; ++i) {
- lldb::SBValue variable = top_scope->GetValueAtIndex(i);
+ lldb::SBValue variable = scope_data->scope.GetValueAtIndex(i);
if (!variable.IsValid())
break;
variable_name_counts[GetNonNullVariableName(variable)]++;
}
// Show return value if there is any ( in the local top frame )
- if (var_ref == VARREF_LOCALS) {
+ if (scope_data && scope_data->kind == eScopeKindLocals) {
auto process = dap.target.GetProcess();
auto selected_thread = process.GetSelectedThread();
lldb::SBValue stop_return_value = selected_thread.GetStopReturnValue();
@@ -116,7 +119,7 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const {
// Now we construct the result with unique display variable names
for (auto i = start_idx; i < end_idx; ++i) {
- lldb::SBValue variable = top_scope->GetValueAtIndex(i);
+ lldb::SBValue variable = scope_data->scope.GetValueAtIndex(i);
if (!variable.IsValid())
break;
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 8851af65d1c4f..530927a4a9ef5 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -299,91 +299,6 @@ void FillResponse(const llvm::json::Object &request,
response.try_emplace("success", true);
}
-// "Scope": {
-// "type": "object",
-// "description": "A Scope is a named container for variables. Optionally
-// a scope can map to a source or a range within a source.",
-// "properties": {
-// "name": {
-// "type": "string",
-// "description": "Name of the scope such as 'Arguments', 'Locals'."
-// },
-// "presentationHint": {
-// "type": "string",
-// "description": "An optional hint for how to present this scope in the
-// UI. If this attribute is missing, the scope is shown
-// with a generic UI.",
-// "_enum": [ "arguments", "locals", "registers" ],
-// },
-// "variablesReference": {
-// "type": "integer",
-// "description": "The variables of this scope can be retrieved by
-// passing the value of variablesReference to the
-// VariablesRequest."
-// },
-// "namedVariables": {
-// "type": "integer",
-// "description": "The number of named variables in this scope. The
-// client can use this optional information to present
-// the variables in a paged UI and fetch them in chunks."
-// },
-// "indexedVariables": {
-// "type": "integer",
-// "description": "The number of indexed variables in this scope. The
-// client can use this optional information to present
-// the variables in a paged UI and fetch them in chunks."
-// },
-// "expensive": {
-// "type": "boolean",
-// "description": "If true, the number of variables in this scope is
-// large or expensive to retrieve."
-// },
-// "source": {
-// "$ref": "#/definitions/Source",
-// "description": "Optional source for this scope."
-// },
-// "line": {
-// "type": "integer",
-// "description": "Optional start line of the range covered by this
-// scope."
-// },
-// "column": {
-// "type": "integer",
-// "description": "Optional start column of the range covered by this
-// scope."
-// },
-// "endLine": {
-// "type": "integer",
-// "description": "Optional end line of the range covered by this scope."
-// },
-// "endColumn": {
-// "type": "integer",
-// "description": "Optional end column of the range covered by this
-// scope."
-// }
-// },
-// "required": [ "name", "variablesReference", "expensive" ]
-// }
-llvm::json::Value CreateScope(const llvm::StringRef name,
- int64_t variablesReference,
- int64_t namedVariables, bool expensive) {
- llvm::json::Object object;
- EmplaceSafeString(object, "name", name.str());
-
- // TODO: Support "arguments" scope. At the moment lldb-dap includes the
- // arguments into the "locals" scope.
- if (variablesReference == VARREF_LOCALS) {
- object.try_emplace("presentationHint", "locals");
- } else if (variablesReference == VARREF_REGS) {
- object.try_emplace("presentationHint", "registers");
- }
-
- object.try_emplace("variablesReference", variablesReference);
- object.try_emplace("expensive", expensive);
- object.try_emplace("namedVariables", namedVariables);
- return llvm::json::Value(std::move(object));
-}
-
// "Event": {
// "allOf": [ { "$ref": "#/definitions/ProtocolMessage" }, {
// "type": "object",
diff --git a/lldb/tools/lldb-dap/SourceBreakpoint.cpp b/lldb/tools/lldb-dap/SourceBreakpoint.cpp
index 843a5eb09c7ae..6f776f4d5f429 100644
--- a/lldb/tools/lldb-dap/SourceBreakpoint.cpp
+++ b/lldb/tools/lldb-dap/SourceBreakpoint.cpp
@@ -10,7 +10,6 @@
#include "BreakpointBase.h"
#include "DAP.h"
#include "JSONUtils.h"
-#include "ProtocolUtils.h"
#include "lldb/API/SBBreakpoint.h"
#include "lldb/API/SBFileSpec.h"
#include "lldb/API/SBFileSpecList.h"
diff --git a/lldb/tools/lldb-dap/Variables.cpp b/lldb/tools/lldb-dap/Variables.cpp
index 777e3183d8c0d..6d49113243799 100644
--- a/lldb/tools/lldb-dap/Variables.cpp
+++ b/lldb/tools/lldb-dap/Variables.cpp
@@ -8,27 +8,77 @@
#include "Variables.h"
#include "JSONUtils.h"
+#include "Protocol/ProtocolTypes.h"
+#include "lldb/API/SBFrame.h"
+#include "lldb/API/SBValue.h"
+#include "lldb/API/SBValueList.h"
+#include <cstdint>
+#include <optional>
+#include <vector>
using namespace lldb_dap;
-lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) {
- switch (variablesReference) {
- case VARREF_LOCALS:
- return &locals;
- case VARREF_GLOBALS:
- return &globals;
- case VARREF_REGS:
- return ®isters;
- default:
- return nullptr;
+namespace lldb_dap {
+
+protocol::Scope CreateScope(const ScopeKind kind, int64_t variablesReference,
+ int64_t namedVariables, bool expensive) {
+ protocol::Scope scope;
+ scope.variablesReference = variablesReference;
+ scope.namedVariables = namedVariables;
+ scope.expensive = expensive;
+
+ // TODO: Support "arguments" and "return value" scope.
+ // At the moment lldb-dap includes the arguments and return_value into the
+ // "locals" scope.
+ // VS Code only expands the first non-expensive scope. This causes friction
+ // if we add the arguments above the local scope, as the locals scope will not
+ // be expanded if we enter a function with arguments. It becomes more
+ // annoying when the scope has arguments, return_value and locals.
+ switch (kind) {
+ case eScopeKindLocals:
+ scope.presentationHint = protocol::Scope::eScopePresentationHintLocals;
+ scope.name = "Locals";
+ break;
+ case eScopeKindGlobals:
+ scope.name = "Globals";
+ break;
+ case eScopeKindRegisters:
+ scope.presentationHint = protocol::Scope::eScopePresentationHintRegisters;
+ scope.name = "Registers";
+ break;
}
+
+ return scope;
+}
+
+std::optional<ScopeData>
+Variables::GetTopLevelScope(int64_t variablesReference) {
+ auto scope_kind_iter = m_scope_kinds.find(variablesReference);
+ if (scope_kind_iter == m_scope_kinds.end())
+ return std::nullopt;
+
+ ScopeKind scope_kind = scope_kind_iter->second.first;
+ uint64_t dap_frame_id = scope_kind_iter->second.second;
+
+ auto frame_iter = m_frames.find(dap_frame_id);
+ if (frame_iter == m_frames.end())
+ return std::nullopt;
+
+ lldb::SBValueList *scope = frame_iter->second.GetScope(scope_kind);
+ if (scope == nullptr)
+ return std::nullopt;
+
+ ScopeData scope_data;
+ scope_data.kind = scope_kind;
+ scope_data.scope = *scope;
+ return scope_data;
}
void Variables::Clear() {
- locals.Clear();
- globals.Clear();
- registers.Clear();
m_referencedvariables.clear();
+ m_scope_kinds.clear();
+ m_frames.clear();
+ m_next_temporary_var_ref = TemporaryVariableStartIndex;
}
int64_t Variables::GetNewVariableReference(bool is_permanent) {
@@ -66,15 +116,16 @@ int64_t Variables::InsertVariable(lldb::SBValue variable, bool is_permanent) {
lldb::SBValue Variables::FindVariable(uint64_t variablesReference,
llvm::StringRef name) {
lldb::SBValue variable;
- if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
+ if (std::optional<ScopeData> scope_data =
+ GetTopLevelScope(variablesReference)) {
bool is_duplicated_variable_name = name.contains(" @");
// variablesReference is one ...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/180031
More information about the llvm-branch-commits
mailing list