[Lldb-commits] [lldb] [lldb-dap] Implement value locations for function pointers (PR #104589)
Adrian Vogelsgesang via lldb-commits
lldb-commits at lists.llvm.org
Fri Aug 16 06:34:12 PDT 2024
https://github.com/vogelsgesang created https://github.com/llvm/llvm-project/pull/104589
Note to reviewers: This commit builds on top of the not-yet-merged PR #102928. When reviewing, ignore the first commit, it is part of the over PR. I will rebase and turn this into a non-draft PR after #102928 landed.
----
This commit adds `valueLocationReference` to function pointers and function references. Thereby, users can navigate directly to the pointed-to function from within the "variables" pane.
In general, it would be useful to also a add similar location references also to member function pointers, `std::source_location`, `std::function`, and many more. Doing so would require extending the formatters to provide such a source code location.
There were two RFCs about this a while ago:
https://discourse.llvm.org/t/rfc-extending-formatters-with-a-source-code-reference/68375
https://discourse.llvm.org/t/rfc-sbvalue-metadata-provider/68377/26
However, both RFCs ended without a conclusion. As such, this commit now solve the lowest-hanging fruit, i.e. function pointers. If people find it useful, I will revive the RFC afterwards.
>From 079def868f0216f31b78469f63034db5b350e250 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Sat, 10 Aug 2024 23:59:55 +0000
Subject: [PATCH 1/2] [lldb-dap] Implement declaration locations
This commit implements support for the "declaration location" recently
added by microsoft/debug-adapter-protocol#494 to the debug adapter
protocol.
For the `declarationLocationReference` we need a variable ID similar to
the the `variablesReference`. I decided to simply reuse the
`variablesReference` here and renamed `Variables::expandable_variables`
and friends accordingly. Given that almost all variables have a
declaration location, we now assign those variable ids to all variables.
While `declarationLocationReference` effectively supersedes
`$__lldb_extensions.declaration`, I did not remove this extension, yet,
since I assume that there are some closed-source extensions which rely
on it.
I tested this against VS-Code Insiders. However, VS-Code Insiders
currently only supports `valueLoctionReference` and not
`declarationLocationReference`, yet. Locally, I hence tried to publish
the declaration locations as value locations. However, it seems that
VS-Code still has issues with correctly resolving file paths, as
reported in https://github.com/microsoft/vscode/pull/225546#issuecomment-2292428591
---
.../test/tools/lldb-dap/dap_server.py | 11 ++
.../API/tools/lldb-dap/locations/Makefile | 3 +
.../lldb-dap/locations/TestDAP_locations.py | 40 +++++
lldb/test/API/tools/lldb-dap/locations/main.c | 5 +
lldb/tools/lldb-dap/DAP.cpp | 17 +-
lldb/tools/lldb-dap/DAP.h | 10 +-
lldb/tools/lldb-dap/JSONUtils.cpp | 125 ++++++++-------
lldb/tools/lldb-dap/JSONUtils.h | 31 ++--
lldb/tools/lldb-dap/lldb-dap.cpp | 146 +++++++++++++++---
9 files changed, 286 insertions(+), 102 deletions(-)
create mode 100644 lldb/test/API/tools/lldb-dap/locations/Makefile
create mode 100644 lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
create mode 100644 lldb/test/API/tools/lldb-dap/locations/main.c
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index a324af57b61df3..9879a34ed2020c 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -1079,6 +1079,17 @@ def request_setVariable(self, containingVarRef, name, value, id=None):
}
return self.send_recv(command_dict)
+ def request_locations(self, locationReference):
+ args_dict = {
+ "locationReference": locationReference,
+ }
+ command_dict = {
+ "command": "locations",
+ "type": "request",
+ "arguments": args_dict,
+ }
+ return self.send_recv(command_dict)
+
def request_testGetTargetBreakpoints(self):
"""A request packet used in the LLDB test suite to get all currently
set breakpoint infos for all breakpoints currently set in the
diff --git a/lldb/test/API/tools/lldb-dap/locations/Makefile b/lldb/test/API/tools/lldb-dap/locations/Makefile
new file mode 100644
index 00000000000000..10495940055b63
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/locations/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py b/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
new file mode 100644
index 00000000000000..76d938d3908492
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
@@ -0,0 +1,40 @@
+"""
+Test lldb-dap locations request
+"""
+
+
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbdap_testcase
+import os
+
+
+class TestDAP_locations(lldbdap_testcase.DAPTestCaseBase):
+ @skipIfWindows
+ def test_locations(self):
+ """
+ Tests the 'locations' request.
+ """
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ source = "main.c"
+ self.source_path = os.path.join(os.getcwd(), source)
+ self.set_source_breakpoints(
+ source,
+ [line_number(source, "// BREAK HERE")],
+ )
+ self.continue_to_next_stop()
+
+ locals = {l["name"]: l for l in self.dap_server.get_local_variables()}
+
+ # var1 has a declarationLocation but no valueLocation
+ self.assertIn("declarationLocationReference", locals["var1"].keys())
+ self.assertNotIn("valueLocationReference", locals["var1"].keys())
+ loc_var1 = self.dap_server.request_locations(
+ locals["var1"]["declarationLocationReference"]
+ )
+ self.assertTrue(loc_var1["success"])
+ self.assertTrue(loc_var1["body"]["source"]["path"].endswith("main.c"))
+ self.assertEqual(loc_var1["body"]["line"], 2)
diff --git a/lldb/test/API/tools/lldb-dap/locations/main.c b/lldb/test/API/tools/lldb-dap/locations/main.c
new file mode 100644
index 00000000000000..6a8c86d00cb562
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/locations/main.c
@@ -0,0 +1,5 @@
+int main(void) {
+ int var1 = 1;
+ // BREAK HERE
+ return 0;
+}
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index c3c70e9d739846..a773c2df28ed25 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -813,7 +813,7 @@ void Variables::Clear() {
locals.Clear();
globals.Clear();
registers.Clear();
- expandable_variables.clear();
+ referenced_variables.clear();
}
int64_t Variables::GetNewVariableReference(bool is_permanent) {
@@ -828,24 +828,23 @@ bool Variables::IsPermanentVariableReference(int64_t var_ref) {
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())
+ auto pos = referenced_permanent_variables.find(var_ref);
+ if (pos != referenced_permanent_variables.end())
return pos->second;
} else {
- auto pos = expandable_variables.find(var_ref);
- if (pos != expandable_variables.end())
+ auto pos = referenced_variables.find(var_ref);
+ if (pos != referenced_variables.end())
return pos->second;
}
return lldb::SBValue();
}
-int64_t Variables::InsertExpandableVariable(lldb::SBValue variable,
- bool is_permanent) {
+int64_t Variables::InsertVariable(lldb::SBValue variable, bool is_permanent) {
int64_t var_ref = GetNewVariableReference(is_permanent);
if (is_permanent)
- expandable_permanent_variables.insert(std::make_pair(var_ref, variable));
+ referenced_permanent_variables.insert(std::make_pair(var_ref, variable));
else
- expandable_variables.insert(std::make_pair(var_ref, variable));
+ referenced_variables.insert(std::make_pair(var_ref, variable));
return var_ref;
}
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 57562a14983519..848e69145da4f6 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -104,12 +104,12 @@ struct Variables {
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.
+ /// 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.
+ llvm::DenseMap<int64_t, lldb::SBValue> referenced_variables;
+ /// 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;
+ llvm::DenseMap<int64_t, lldb::SBValue> referenced_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
@@ -127,7 +127,7 @@ struct Variables {
/// Insert a new \p variable.
/// \return variableReference assigned to this expandable variable.
- int64_t InsertExpandableVariable(lldb::SBValue variable, bool is_permanent);
+ int64_t InsertVariable(lldb::SBValue variable, bool is_permanent);
/// Clear all scope variables and non-permanent expandable variables.
void Clear();
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index a8b85f55939e17..5738a3034990c1 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -614,9 +614,8 @@ CreateExceptionBreakpointFilter(const ExceptionBreakpoint &bp) {
// }
// }
// }
-llvm::json::Value CreateSource(lldb::SBLineEntry &line_entry) {
+llvm::json::Value CreateSource(const lldb::SBFileSpec &file) {
llvm::json::Object object;
- lldb::SBFileSpec file = line_entry.GetFileSpec();
if (file.IsValid()) {
const char *name = file.GetFilename();
if (name)
@@ -630,6 +629,10 @@ llvm::json::Value CreateSource(lldb::SBLineEntry &line_entry) {
return llvm::json::Value(std::move(object));
}
+llvm::json::Value CreateSource(const lldb::SBLineEntry &line_entry) {
+ return CreateSource(line_entry.GetFileSpec());
+}
+
llvm::json::Value CreateSource(llvm::StringRef source_path) {
llvm::json::Object source;
llvm::StringRef name = llvm::sys::path::filename(source_path);
@@ -1143,65 +1146,75 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
// "description": "The number of indexed child variables. The client
// can use this optional information to present the
// children in a paged UI and fetch them in chunks."
-// }
-//
+// },
+// "declarationLocationReference": {
+// "type": "integer",
+// "description": "A reference that allows the client to request the
+// location where the variable is declared. This should be
+// present only if the adapter is likely to be able to
+// resolve the location.\n\nThis reference shares the same
+// lifetime as the `variablesReference`. See 'Lifetime of
+// Object References' in the Overview section for
+// details."
+// },
//
// "$__lldb_extensions": {
// "description": "Unofficial extensions to the protocol",
// "properties": {
// "declaration": {
-// "type": "object",
-// "description": "The source location where the variable was declared.
-// This value won't be present if no declaration is
-// available.",
-// "properties": {
-// "path": {
-// "type": "string",
-// "description": "The source file path where the variable was
-// declared."
-// },
-// "line": {
-// "type": "number",
-// "description": "The 1-indexed source line where the variable was
-// declared."
-// },
-// "column": {
-// "type": "number",
-// "description": "The 1-indexed source column where the variable
-// was declared."
+// "type": "object",
+// "description": "The source location where the variable was
+// declared. This value won't be present if no
+// declaration is available.
+// Superseded by `declarationLocationReference`",
+// "properties": {
+// "path": {
+// "type": "string",
+// "description": "The source file path where the variable was
+// declared."
+// },
+// "line": {
+// "type": "number",
+// "description": "The 1-indexed source line where the variable
+// was declared."
+// },
+// "column": {
+// "type": "number",
+// "description": "The 1-indexed source column where the variable
+// was declared."
+// }
// }
+// },
+// "value": {
+// "type": "string",
+// "description": "The internal value of the variable as returned by
+// This is effectively SBValue.GetValue(). The other
+// `value` entry in the top-level variable response
+// is, on the other hand, just a display string for
+// the variable."
+// },
+// "summary": {
+// "type": "string",
+// "description": "The summary string of the variable. This is
+// effectively SBValue.GetSummary()."
+// },
+// "autoSummary": {
+// "type": "string",
+// "description": "The auto generated summary if using
+// `enableAutoVariableSummaries`."
+// },
+// "error": {
+// "type": "string",
+// "description": "An error message generated if LLDB couldn't inspect
+// the variable."
// }
-// },
-// "value":
-// "type": "string",
-// "description": "The internal value of the variable as returned by
-// This is effectively SBValue.GetValue(). The other
-// `value` entry in the top-level variable response is,
-// on the other hand, just a display string for the
-// variable."
-// },
-// "summary":
-// "type": "string",
-// "description": "The summary string of the variable. This is
-// effectively SBValue.GetSummary()."
-// },
-// "autoSummary":
-// "type": "string",
-// "description": "The auto generated summary if using
-// `enableAutoVariableSummaries`."
-// },
-// "error":
-// "type": "string",
-// "description": "An error message generated if LLDB couldn't inspect
-// the variable."
// }
// }
// },
// "required": [ "name", "value", "variablesReference" ]
// }
-llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
- int64_t varID, bool format_hex,
- bool is_name_duplicated,
+llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
+ bool format_hex, bool is_name_duplicated,
std::optional<std::string> custom_name) {
VariableDescription desc(v, format_hex, is_name_duplicated, custom_name);
llvm::json::Object object;
@@ -1246,12 +1259,18 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
}
}
EmplaceSafeString(object, "type", desc.display_type_name);
- if (varID != INT64_MAX)
- object.try_emplace("id", varID);
+
+ // A unique variable identifier to help in properly identifying variables with
+ // the same name. This is an extension to the VS protocol.
+ object.try_emplace("id", var_ref);
+
if (v.MightHaveChildren())
- object.try_emplace("variablesReference", variablesReference);
+ object.try_emplace("variablesReference", var_ref);
else
- object.try_emplace("variablesReference", (int64_t)0);
+ object.try_emplace("variablesReference", 0);
+
+ if (v.GetDeclaration().IsValid())
+ object.try_emplace("declarationLocationReference", var_ref);
object.try_emplace("$__lldb_extensions", desc.GetVariableExtensionsJSON());
return llvm::json::Value(std::move(object));
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 1515f5ba2e5f4d..0a88dd269bfbdf 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -282,6 +282,16 @@ llvm::json::Value CreateScope(const llvm::StringRef name,
int64_t variablesReference,
int64_t namedVariables, bool expensive);
+/// Create a "Source" JSON object as described in the debug adaptor definition.
+///
+/// \param[in] file
+/// The SBFileSpec to use when populating out the "Source" object
+///
+/// \return
+/// A "Source" JSON object that follows the formal JSON
+/// definition outlined by Microsoft.
+llvm::json::Value CreateSource(const lldb::SBFileSpec &file);
+
/// Create a "Source" JSON object as described in the debug adaptor definition.
///
/// \param[in] line_entry
@@ -289,9 +299,9 @@ llvm::json::Value CreateScope(const llvm::StringRef name,
/// object
///
/// \return
-/// A "Source" JSON object with that follows the formal JSON
+/// A "Source" JSON object that follows the formal JSON
/// definition outlined by Microsoft.
-llvm::json::Value CreateSource(lldb::SBLineEntry &line_entry);
+llvm::json::Value CreateSource(const lldb::SBLineEntry &line_entry);
/// Create a "Source" object for a given source path.
///
@@ -433,15 +443,10 @@ struct VariableDescription {
/// The LLDB value to use when populating out the "Variable"
/// object.
///
-/// \param[in] variablesReference
-/// The variable reference. Zero if this value isn't structured
-/// and has no children, non-zero if it does have children and
-/// might be asked to expand itself.
-///
-/// \param[in] varID
-/// A unique variable identifier to help in properly identifying
-/// variables with the same name. This is an extension to the
-/// VS protocol.
+/// \param[in] var_ref
+/// The variable reference. Used to identify the value, e.g.
+/// in the `variablesReference` or `declarationLocationReference`
+/// properties.
///
/// \param[in] format_hex
/// It set to true the variable will be formatted as hex in
@@ -462,8 +467,8 @@ struct VariableDescription {
/// \return
/// A "Variable" JSON object with that follows the formal JSON
/// definition outlined by Microsoft.
-llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
- int64_t varID, bool format_hex,
+llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
+ bool format_hex,
bool is_name_duplicated = false,
std::optional<std::string> custom_name = {});
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index ea84f31aec3a6c..ccf1ea61f9973e 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -8,6 +8,7 @@
#include "DAP.h"
#include "Watchpoint.h"
+#include "lldb/API/SBDeclaration.h"
#include "lldb/API/SBMemoryRegionInfo.h"
#include <cassert>
@@ -1405,7 +1406,7 @@ void request_evaluate(const llvm::json::Object &request) {
EmplaceSafeString(body, "result", desc.GetResult(context));
EmplaceSafeString(body, "type", desc.display_type_name);
if (value.MightHaveChildren()) {
- auto variableReference = g_dap.variables.InsertExpandableVariable(
+ auto variableReference = g_dap.variables.InsertVariable(
value, /*is_permanent=*/context == "repl");
body.try_emplace("variablesReference", variableReference);
} else {
@@ -3598,8 +3599,8 @@ void request_setVariable(const llvm::json::Object &request) {
// is_permanent is false because debug console does not support
// setVariable request.
if (variable.MightHaveChildren())
- newVariablesReference = g_dap.variables.InsertExpandableVariable(
- variable, /*is_permanent=*/false);
+ newVariablesReference =
+ g_dap.variables.InsertVariable(variable, /*is_permanent=*/false);
body.try_emplace("variablesReference", newVariablesReference);
} else {
@@ -3770,13 +3771,10 @@ void request_variables(const llvm::json::Object &request) {
if (!variable.IsValid())
break;
- int64_t var_ref = 0;
- if (variable.MightHaveChildren() || variable.IsSynthetic()) {
- var_ref = g_dap.variables.InsertExpandableVariable(
- variable, /*is_permanent=*/false);
- }
+ int64_t var_ref =
+ g_dap.variables.InsertVariable(variable, /*is_permanent=*/false);
variables.emplace_back(CreateVariable(
- variable, var_ref, var_ref != 0 ? var_ref : UINT64_MAX, hex,
+ variable, var_ref, hex,
variable_name_counts[GetNonNullVariableName(variable)] > 1));
}
} else {
@@ -3788,19 +3786,11 @@ void request_variables(const llvm::json::Object &request) {
std::optional<std::string> custom_name = {}) {
if (!child.IsValid())
return;
- if (child.MightHaveChildren()) {
- auto is_permanent =
- g_dap.variables.IsPermanentVariableReference(variablesReference);
- auto childVariablesReferences =
- g_dap.variables.InsertExpandableVariable(child, is_permanent);
- variables.emplace_back(CreateVariable(
- child, childVariablesReferences, childVariablesReferences, hex,
- /*is_name_duplicated=*/false, custom_name));
- } else {
- variables.emplace_back(CreateVariable(child, 0, INT64_MAX, hex,
- /*is_name_duplicated=*/false,
- custom_name));
- }
+ bool is_permanent =
+ g_dap.variables.IsPermanentVariableReference(variablesReference);
+ int64_t var_ref = g_dap.variables.InsertVariable(child, is_permanent);
+ variables.emplace_back(CreateVariable(
+ child, var_ref, hex, /*is_name_duplicated=*/false, custom_name));
};
const int64_t num_children = variable.GetNumChildren();
int64_t end_idx = start + ((count == 0) ? num_children : count);
@@ -3823,6 +3813,117 @@ void request_variables(const llvm::json::Object &request) {
g_dap.SendJSON(llvm::json::Value(std::move(response)));
}
+// "LocationsRequest": {
+// "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Looks up information about a location reference
+// previously returned by the debug adapter.",
+// "properties": {
+// "command": {
+// "type": "string",
+// "enum": [ "locations" ]
+// },
+// "arguments": {
+// "$ref": "#/definitions/LocationsArguments"
+// }
+// },
+// "required": [ "command", "arguments" ]
+// }]
+// },
+// "LocationsArguments": {
+// "type": "object",
+// "description": "Arguments for `locations` request.",
+// "properties": {
+// "locationReference": {
+// "type": "integer",
+// "description": "Location reference to resolve."
+// }
+// },
+// "required": [ "locationReference" ]
+// },
+// "LocationsResponse": {
+// "allOf": [ { "$ref": "#/definitions/Response" }, {
+// "type": "object",
+// "description": "Response to `locations` request.",
+// "properties": {
+// "body": {
+// "type": "object",
+// "properties": {
+// "source": {
+// "$ref": "#/definitions/Source",
+// "description": "The source containing the location; either
+// `source.path` or `source.sourceReference` must be
+// specified."
+// },
+// "line": {
+// "type": "integer",
+// "description": "The line number of the location. The client
+// capability `linesStartAt1` determines whether it
+// is 0- or 1-based."
+// },
+// "column": {
+// "type": "integer",
+// "description": "Position of the location within the `line`. It is
+// measured in UTF-16 code units and the client
+// capability `columnsStartAt1` determines whether
+// it is 0- or 1-based. If no column is given, the
+// first position in the start line is assumed."
+// },
+// "endLine": {
+// "type": "integer",
+// "description": "End line of the location, present if the location
+// refers to a range. The client capability
+// `linesStartAt1` determines whether it is 0- or
+// 1-based."
+// },
+// "endColumn": {
+// "type": "integer",
+// "description": "End position of the location within `endLine`,
+// present if the location refers to a range. It is
+// measured in UTF-16 code units and the client
+// capability `columnsStartAt1` determines whether
+// it is 0- or 1-based."
+// }
+// },
+// "required": [ "source", "line" ]
+// }
+// }
+// }]
+// },
+void request_locations(const llvm::json::Object &request) {
+ llvm::json::Object response;
+ FillResponse(request, response);
+ auto arguments = request.getObject("arguments");
+
+ uint64_t reference_id = GetUnsigned(arguments, "locationReference", 0);
+ lldb::SBValue variable = g_dap.variables.GetVariable(reference_id);
+ if (!variable.IsValid()) {
+ response["success"] = false;
+ response["message"] = "Invalid variable reference";
+ g_dap.SendJSON(llvm::json::Value(std::move(response)));
+ return;
+ }
+
+ // Get the declaration location
+ lldb::SBDeclaration decl = variable.GetDeclaration();
+ if (!decl.IsValid()) {
+ response["success"] = false;
+ response["message"] = "No declaration location available";
+ g_dap.SendJSON(llvm::json::Value(std::move(response)));
+ return;
+ }
+
+ llvm::json::Object body;
+ body.try_emplace("source", CreateSource(decl.GetFileSpec()));
+ if (int line = decl.GetLine())
+ body.try_emplace("line", line);
+ if (int column = decl.GetColumn())
+ body.try_emplace("column", column);
+
+ response.try_emplace("body", std::move(body));
+ g_dap.SendJSON(llvm::json::Value(std::move(response)));
+}
+
// "DisassembleRequest": {
// "allOf": [ { "$ref": "#/definitions/Request" }, {
// "type": "object",
@@ -4077,6 +4178,7 @@ void RegisterRequestCallbacks() {
g_dap.RegisterRequestCallback("stepOut", request_stepOut);
g_dap.RegisterRequestCallback("threads", request_threads);
g_dap.RegisterRequestCallback("variables", request_variables);
+ g_dap.RegisterRequestCallback("locations", request_locations);
g_dap.RegisterRequestCallback("disassemble", request_disassemble);
// Custom requests
g_dap.RegisterRequestCallback("compileUnits", request_compileUnits);
>From b0fee0c0c32001064aeab1ebca209fef02c6cb98 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Mon, 12 Aug 2024 14:53:31 +0000
Subject: [PATCH 2/2] [lldb-dap] Implement value locations for function
pointers
This commit adds `valueLocationReference` to function pointers and
function references. Thereby, users can navigate directly to the
pointed-to function from within the "variables" pane.
In general, it would be useful to also a similar location references
also to member function pointers, `std::source_location`,
`std::function`, and many more. Doing so would require extending the
formatters to provide such a source code location.
There were two RFCs about this a while ago:
https://discourse.llvm.org/t/rfc-extending-formatters-with-a-source-code-reference/68375
https://discourse.llvm.org/t/rfc-sbvalue-metadata-provider/68377/26
However, both RFCs ended without a conclusion. As such, this commit now
solve the lowest-hanging fruit, i.e. function pointers. If people find
it useful, I will revive the RFC afterwards.
---
.../API/tools/lldb-dap/locations/Makefile | 2 +-
.../lldb-dap/locations/TestDAP_locations.py | 43 ++++++-
lldb/test/API/tools/lldb-dap/locations/main.c | 5 -
.../API/tools/lldb-dap/locations/main.cpp | 13 ++
lldb/tools/lldb-dap/JSONUtils.cpp | 33 +++++-
lldb/tools/lldb-dap/JSONUtils.h | 3 +
lldb/tools/lldb-dap/lldb-dap.cpp | 112 ++++++++++++++----
7 files changed, 173 insertions(+), 38 deletions(-)
delete mode 100644 lldb/test/API/tools/lldb-dap/locations/main.c
create mode 100644 lldb/test/API/tools/lldb-dap/locations/main.cpp
diff --git a/lldb/test/API/tools/lldb-dap/locations/Makefile b/lldb/test/API/tools/lldb-dap/locations/Makefile
index 10495940055b63..99998b20bcb050 100644
--- a/lldb/test/API/tools/lldb-dap/locations/Makefile
+++ b/lldb/test/API/tools/lldb-dap/locations/Makefile
@@ -1,3 +1,3 @@
-C_SOURCES := main.c
+CXX_SOURCES := main.cpp
include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py b/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
index 76d938d3908492..5a755ee44d12c2 100644
--- a/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
+++ b/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
@@ -19,7 +19,7 @@ def test_locations(self):
"""
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)
- source = "main.c"
+ source = "main.cpp"
self.source_path = os.path.join(os.getcwd(), source)
self.set_source_breakpoints(
source,
@@ -36,5 +36,42 @@ def test_locations(self):
locals["var1"]["declarationLocationReference"]
)
self.assertTrue(loc_var1["success"])
- self.assertTrue(loc_var1["body"]["source"]["path"].endswith("main.c"))
- self.assertEqual(loc_var1["body"]["line"], 2)
+ self.assertTrue(loc_var1["body"]["source"]["path"].endswith("main.cpp"))
+ self.assertEqual(loc_var1["body"]["line"], 8)
+
+ # func_ptr has both a declaration and a valueLocation
+ self.assertIn("declarationLocationReference", locals["func_ptr"].keys())
+ self.assertIn("valueLocationReference", locals["func_ptr"].keys())
+ decl_loc_func_ptr = self.dap_server.request_locations(
+ locals["func_ptr"]["declarationLocationReference"]
+ )
+ self.assertTrue(decl_loc_func_ptr["success"])
+ self.assertTrue(decl_loc_func_ptr["body"]["source"]["path"].endswith("main.cpp"))
+ self.assertEqual(decl_loc_func_ptr["body"]["line"], 9)
+ val_loc_func_ptr = self.dap_server.request_locations(
+ locals["func_ptr"]["valueLocationReference"]
+ )
+ self.assertTrue(val_loc_func_ptr["success"])
+ self.assertTrue(val_loc_func_ptr["body"]["source"]["path"].endswith("main.cpp"))
+ self.assertEqual(val_loc_func_ptr["body"]["line"], 3)
+
+ # func_ref has both a declaration and a valueLocation
+ self.assertIn("declarationLocationReference", locals["func_ref"].keys())
+ self.assertIn("valueLocationReference", locals["func_ref"].keys())
+ decl_loc_func_ref = self.dap_server.request_locations(
+ locals["func_ref"]["declarationLocationReference"]
+ )
+ self.assertTrue(decl_loc_func_ref["success"])
+ self.assertTrue(decl_loc_func_ref["body"]["source"]["path"].endswith("main.cpp"))
+ self.assertEqual(decl_loc_func_ref["body"]["line"], 10)
+ val_loc_func_ref = self.dap_server.request_locations(
+ locals["func_ref"]["valueLocationReference"]
+ )
+ self.assertTrue(val_loc_func_ref["success"])
+ self.assertTrue(val_loc_func_ref["body"]["source"]["path"].endswith("main.cpp"))
+ self.assertEqual(val_loc_func_ref["body"]["line"], 3)
+
+ # `evaluate` responses for function pointers also have locations associated
+ eval_res = self.dap_server.request_evaluate("greet")
+ self.assertTrue(decl_loc_func_ref["success"])
+ self.assertIn("valueLocationReference", eval_res["body"].keys())
diff --git a/lldb/test/API/tools/lldb-dap/locations/main.c b/lldb/test/API/tools/lldb-dap/locations/main.c
deleted file mode 100644
index 6a8c86d00cb562..00000000000000
--- a/lldb/test/API/tools/lldb-dap/locations/main.c
+++ /dev/null
@@ -1,5 +0,0 @@
-int main(void) {
- int var1 = 1;
- // BREAK HERE
- return 0;
-}
diff --git a/lldb/test/API/tools/lldb-dap/locations/main.cpp b/lldb/test/API/tools/lldb-dap/locations/main.cpp
new file mode 100644
index 00000000000000..8eb5a7636d188c
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/locations/main.cpp
@@ -0,0 +1,13 @@
+#include <cstdio>
+
+void greet() {
+ printf("Hello");
+}
+
+int main(void) {
+ int var1 = 1;
+ void (*func_ptr)() = &greet;
+ void (&func_ref)() = greet;
+ // BREAK HERE
+ return 0;
+}
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 5738a3034990c1..b543355fa6fba8 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -1088,6 +1088,18 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
return description.trim().str();
}
+bool HasValueLocation(lldb::SBValue v) {
+ if (!v.GetType().IsPointerType() && !v.GetType().IsReferenceType()) {
+ return false;
+ }
+
+ lldb::addr_t addr = v.GetValueAsAddress();
+ lldb::SBLineEntry line_entry =
+ g_dap.target.ResolveLoadAddress(addr).GetLineEntry();
+
+ return line_entry.IsValid();
+}
+
// "Variable": {
// "type": "object",
// "description": "A Variable is a name/value pair. Optionally a variable
@@ -1157,6 +1169,18 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
// Object References' in the Overview section for
// details."
// },
+// "valueLocationReference": {
+// "type": "integer",
+// "description": "A reference that allows the client to request the
+// location where the variable's value is declared. For
+// example, if the variable contains a function pointer,
+// the adapter may be able to look up the function's
+// location. This should be present only if the adapter
+// is likely to be able to resolve the location.\n\nThis
+// reference shares the same lifetime as the
+// `variablesReference`. See 'Lifetime of Object
+// References' in the Overview section for details."
+// },
//
// "$__lldb_extensions": {
// "description": "Unofficial extensions to the protocol",
@@ -1270,7 +1294,10 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
object.try_emplace("variablesReference", 0);
if (v.GetDeclaration().IsValid())
- object.try_emplace("declarationLocationReference", var_ref);
+ object.try_emplace("declarationLocationReference", var_ref << 1);
+
+ if (HasValueLocation(v))
+ object.try_emplace("valueLocationReference", (var_ref << 1) | 1);
object.try_emplace("$__lldb_extensions", desc.GetVariableExtensionsJSON());
return llvm::json::Value(std::move(object));
@@ -1293,8 +1320,8 @@ CreateRunInTerminalReverseRequest(const llvm::json::Object &launch_request,
llvm::StringRef comm_file,
lldb::pid_t debugger_pid) {
llvm::json::Object run_in_terminal_args;
- // This indicates the IDE to open an embedded terminal, instead of opening the
- // terminal in a new window.
+ // This indicates the IDE to open an embedded terminal, instead of opening
+ // the terminal in a new window.
run_in_terminal_args.try_emplace("kind", "integrated");
auto launch_request_arguments = launch_request.getObject("arguments");
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 0a88dd269bfbdf..861951e488b6b8 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -421,6 +421,9 @@ struct VariableDescription {
std::string GetResult(llvm::StringRef context);
};
+/// Does the given variable have an associated value location?
+bool HasValueLocation(lldb::SBValue v);
+
/// Create a "Variable" object for a LLDB thread object.
///
/// This function will fill in the following keys in the returned
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index ccf1ea61f9973e..849ca43f925fde 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1346,6 +1346,19 @@ void request_completions(const llvm::json::Object &request) {
// client can use this optional information to
// present the variables in a paged UI and fetch
// them in chunks."
+// },
+// "valueLocationReference": {
+// "type": "integer",
+// "description": "A reference that allows the client to request
+// the location where the returned value is
+// declared. For example, if a function pointer is
+// returned, the adapter may be able to look up the
+// function's location. This should be present only
+// if the adapter is likely to be able to resolve
+// the location.\n\nThis reference shares the same
+// lifetime as the `variablesReference`. See
+// 'Lifetime of Object References' in the
+// Overview section for details."
// }
// },
// "required": [ "result", "variablesReference" ]
@@ -1405,13 +1418,14 @@ void request_evaluate(const llvm::json::Object &request) {
VariableDescription desc(value);
EmplaceSafeString(body, "result", desc.GetResult(context));
EmplaceSafeString(body, "type", desc.display_type_name);
- if (value.MightHaveChildren()) {
- auto variableReference = g_dap.variables.InsertVariable(
- value, /*is_permanent=*/context == "repl");
- body.try_emplace("variablesReference", variableReference);
- } else {
+ auto var_ref = g_dap.variables.InsertVariable(
+ value, /*is_permanent=*/context == "repl");
+ if (value.MightHaveChildren())
+ body.try_emplace("variablesReference", var_ref);
+ else
body.try_emplace("variablesReference", (int64_t)0);
- }
+ if (HasValueLocation(value))
+ body.try_emplace("valueLocationReference", var_ref);
}
}
response.try_emplace("body", std::move(body));
@@ -3544,6 +3558,17 @@ void request_threads(const llvm::json::Object &request) {
// "description": "The number of indexed child variables. The client
// can use this optional information to present the variables in a
// paged UI and fetch them in chunks."
+// },
+// "valueLocationReference": {
+// "type": "integer",
+// "description": "A reference that allows the client to request the
+// location where the new value is declared. For example, if the new
+// value is function pointer, the adapter may be able to look up the
+// function's location. This should be present only if the adapter
+// is likely to be able to resolve the location.\n\nThis reference
+// shares the same lifetime as the `variablesReference`. See
+// 'Lifetime of Object References' in the Overview section for
+// details."
// }
// },
// "required": [ "value" ]
@@ -3568,7 +3593,6 @@ void request_setVariable(const llvm::json::Object &request) {
response.try_emplace("success", false);
lldb::SBValue variable;
- int64_t newVariablesReference = 0;
// The "id" is the unique integer ID that is unique within the enclosing
// variablesReference. It is optionally added to any "interface Variable"
@@ -3598,11 +3622,15 @@ void request_setVariable(const llvm::json::Object &request) {
// 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_dap.variables.InsertVariable(variable, /*is_permanent=*/false);
+ int64_t new_var_ref =
+ g_dap.variables.InsertVariable(variable, /*is_permanent=*/false);
- body.try_emplace("variablesReference", newVariablesReference);
+ if (variable.MightHaveChildren())
+ body.try_emplace("variablesReference", new_var_ref);
+ else
+ body.try_emplace("variablesReference", 0);
+ if (HasValueLocation(variable))
+ body.try_emplace("valueLocationReference", new_var_ref);
} else {
EmplaceSafeString(body, "message", std::string(error.GetCString()));
}
@@ -3896,7 +3924,10 @@ void request_locations(const llvm::json::Object &request) {
auto arguments = request.getObject("arguments");
uint64_t reference_id = GetUnsigned(arguments, "locationReference", 0);
- lldb::SBValue variable = g_dap.variables.GetVariable(reference_id);
+ // We use the lowest bit to distinguish between value location and declaration
+ // location
+ bool isValueLocation = reference_id & 1;
+ lldb::SBValue variable = g_dap.variables.GetVariable(reference_id >> 1);
if (!variable.IsValid()) {
response["success"] = false;
response["message"] = "Invalid variable reference";
@@ -3904,21 +3935,50 @@ void request_locations(const llvm::json::Object &request) {
return;
}
- // Get the declaration location
- lldb::SBDeclaration decl = variable.GetDeclaration();
- if (!decl.IsValid()) {
- response["success"] = false;
- response["message"] = "No declaration location available";
- g_dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
-
llvm::json::Object body;
- body.try_emplace("source", CreateSource(decl.GetFileSpec()));
- if (int line = decl.GetLine())
- body.try_emplace("line", line);
- if (int column = decl.GetColumn())
- body.try_emplace("column", column);
+ if (isValueLocation) {
+ // Get the value location
+ if (!variable.GetType().IsPointerType() &&
+ !variable.GetType().IsReferenceType()) {
+ response["success"] = false;
+ response["message"] =
+ "Value locations are only available for pointers and references";
+ g_dap.SendJSON(llvm::json::Value(std::move(response)));
+ return;
+ }
+
+ lldb::addr_t addr = variable.GetValueAsAddress();
+ lldb::SBLineEntry line_entry =
+ g_dap.target.ResolveLoadAddress(addr).GetLineEntry();
+
+ if (!line_entry.IsValid()) {
+ response["success"] = false;
+ response["message"] = "Failed to resolve line entry for location";
+ g_dap.SendJSON(llvm::json::Value(std::move(response)));
+ return;
+ }
+
+ body.try_emplace("source", CreateSource(line_entry.GetFileSpec()));
+ if (int line = line_entry.GetLine())
+ body.try_emplace("line", line);
+ if (int column = line_entry.GetColumn())
+ body.try_emplace("column", column);
+ } else {
+ // Get the declaration location
+ lldb::SBDeclaration decl = variable.GetDeclaration();
+ if (!decl.IsValid()) {
+ response["success"] = false;
+ response["message"] = "No declaration location available";
+ g_dap.SendJSON(llvm::json::Value(std::move(response)));
+ return;
+ }
+
+ body.try_emplace("source", CreateSource(decl.GetFileSpec()));
+ if (int line = decl.GetLine())
+ body.try_emplace("line", line);
+ if (int column = decl.GetColumn())
+ body.try_emplace("column", column);
+ }
response.try_emplace("body", std::move(body));
g_dap.SendJSON(llvm::json::Value(std::move(response)));
More information about the lldb-commits
mailing list