[Lldb-commits] [lldb] [lldb-dap] Implement value locations for function pointers (PR #104589)
Adrian Vogelsgesang via lldb-commits
lldb-commits at lists.llvm.org
Mon Sep 16 23:49:52 PDT 2024
https://github.com/vogelsgesang updated https://github.com/llvm/llvm-project/pull/104589
>From 79684ccb2b6312b0938de73641e89d7cd29ce1a8 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] [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 | 49 +++++++-
lldb/test/API/tools/lldb-dap/locations/main.c | 5 -
.../API/tools/lldb-dap/locations/main.cpp | 11 ++
lldb/tools/lldb-dap/JSONUtils.cpp | 33 ++++-
lldb/tools/lldb-dap/JSONUtils.h | 3 +
lldb/tools/lldb-dap/lldb-dap.cpp | 113 +++++++++++++-----
7 files changed, 176 insertions(+), 40 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..45f836a2fa3c39 100644
--- a/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
+++ b/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
@@ -19,11 +19,11 @@ 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,
- [line_number(source, "// BREAK HERE")],
+ [line_number(source, "break here")],
)
self.continue_to_next_stop()
@@ -36,5 +36,46 @@ 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"], 6)
+
+ # 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"], 7)
+ 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"], 8)
+ 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(eval_res["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..e3b2e73f9837f8
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/locations/main.cpp
@@ -0,0 +1,11 @@
+#include <cstdio>
+
+void greet() { printf("Hello"); }
+
+int main(void) {
+ int var1 = 1;
+ void (*func_ptr)() = &greet;
+ void (&func_ref)() = greet;
+ __builtin_printf("break here");
+ return 0;
+}
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 4f9c9c01cf4b6b..1d3fc110791d2f 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -1198,6 +1198,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
@@ -1277,6 +1289,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",
@@ -1390,7 +1414,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);
if (lldb::addr_t addr = v.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS)
object.try_emplace("memoryReference", EncodeMemoryReference(addr));
@@ -1416,8 +1443,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 13018458ffe0ad..0e00c0e6bcb8c5 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -458,6 +458,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 2fb86f675b4516..caf2d31bc92b1d 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1542,6 +1542,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."
// }
// "memoryReference": {
// "type": "string",
@@ -1611,16 +1624,17 @@ 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 (lldb::addr_t addr = value.GetLoadAddress();
addr != LLDB_INVALID_ADDRESS)
body.try_emplace("memoryReference", EncodeMemoryReference(addr));
+ if (HasValueLocation(value))
+ body.try_emplace("valueLocationReference", var_ref);
}
}
response.try_emplace("body", std::move(body));
@@ -3735,6 +3749,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" ]
@@ -3759,7 +3784,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"
@@ -3789,14 +3813,17 @@ 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.
+ int64_t new_var_ref =
+ g_dap.variables.InsertVariable(variable, /*is_permanent=*/false);
if (variable.MightHaveChildren())
- newVariablesReference =
- g_dap.variables.InsertVariable(variable, /*is_permanent=*/false);
- body.try_emplace("variablesReference", newVariablesReference);
-
+ body.try_emplace("variablesReference", new_var_ref);
+ else
+ body.try_emplace("variablesReference", 0);
if (lldb::addr_t addr = variable.GetLoadAddress();
addr != LLDB_INVALID_ADDRESS)
body.try_emplace("memoryReference", EncodeMemoryReference(addr));
+ if (HasValueLocation(variable))
+ body.try_emplace("valueLocationReference", new_var_ref);
} else {
EmplaceSafeString(body, "message", std::string(error.GetCString()));
}
@@ -4087,10 +4114,13 @@ void request_variables(const llvm::json::Object &request) {
void request_locations(const llvm::json::Object &request) {
llvm::json::Object response;
FillResponse(request, response);
- auto arguments = request.getObject("arguments");
+ 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";
@@ -4098,21 +4128,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