Skip to content

Commit 0d0da96

Browse files
committed
[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.
1 parent 1a933e9 commit 0d0da96

File tree

7 files changed

+175
-38
lines changed

7 files changed

+175
-38
lines changed
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
C_SOURCES := main.c
1+
CXX_SOURCES := main.cpp
22

33
include Makefile.rules

lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def test_locations(self):
1818
"""
1919
program = self.getBuildArtifact("a.out")
2020
self.build_and_launch(program)
21-
source = "main.c"
21+
source = "main.cpp"
2222
self.source_path = os.path.join(os.getcwd(), source)
2323
self.set_source_breakpoints(
2424
source,
@@ -35,5 +35,46 @@ def test_locations(self):
3535
locals["var1"]["declarationLocationReference"]
3636
)
3737
self.assertTrue(loc_var1["success"])
38-
self.assertTrue(loc_var1["body"]["source"]["path"].endswith("main.c"))
39-
self.assertEqual(loc_var1["body"]["line"], 2)
38+
self.assertTrue(loc_var1["body"]["source"]["path"].endswith("main.cpp"))
39+
self.assertEqual(loc_var1["body"]["line"], 6)
40+
41+
# func_ptr has both a declaration and a valueLocation
42+
self.assertIn("declarationLocationReference", locals["func_ptr"].keys())
43+
self.assertIn("valueLocationReference", locals["func_ptr"].keys())
44+
decl_loc_func_ptr = self.dap_server.request_locations(
45+
locals["func_ptr"]["declarationLocationReference"]
46+
)
47+
self.assertTrue(decl_loc_func_ptr["success"])
48+
self.assertTrue(
49+
decl_loc_func_ptr["body"]["source"]["path"].endswith("main.cpp")
50+
)
51+
self.assertEqual(decl_loc_func_ptr["body"]["line"], 7)
52+
val_loc_func_ptr = self.dap_server.request_locations(
53+
locals["func_ptr"]["valueLocationReference"]
54+
)
55+
self.assertTrue(val_loc_func_ptr["success"])
56+
self.assertTrue(val_loc_func_ptr["body"]["source"]["path"].endswith("main.cpp"))
57+
self.assertEqual(val_loc_func_ptr["body"]["line"], 3)
58+
59+
# func_ref has both a declaration and a valueLocation
60+
self.assertIn("declarationLocationReference", locals["func_ref"].keys())
61+
self.assertIn("valueLocationReference", locals["func_ref"].keys())
62+
decl_loc_func_ref = self.dap_server.request_locations(
63+
locals["func_ref"]["declarationLocationReference"]
64+
)
65+
self.assertTrue(decl_loc_func_ref["success"])
66+
self.assertTrue(
67+
decl_loc_func_ref["body"]["source"]["path"].endswith("main.cpp")
68+
)
69+
self.assertEqual(decl_loc_func_ref["body"]["line"], 8)
70+
val_loc_func_ref = self.dap_server.request_locations(
71+
locals["func_ref"]["valueLocationReference"]
72+
)
73+
self.assertTrue(val_loc_func_ref["success"])
74+
self.assertTrue(val_loc_func_ref["body"]["source"]["path"].endswith("main.cpp"))
75+
self.assertEqual(val_loc_func_ref["body"]["line"], 3)
76+
77+
# `evaluate` responses for function pointers also have locations associated
78+
eval_res = self.dap_server.request_evaluate("greet")
79+
self.assertTrue(eval_res["success"])
80+
self.assertIn("valueLocationReference", eval_res["body"].keys())

lldb/test/API/tools/lldb-dap/locations/main.c

Lines changed: 0 additions & 5 deletions
This file was deleted.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#include <cstdio>
2+
3+
void greet() { printf("Hello"); }
4+
5+
int main(void) {
6+
int var1 = 1;
7+
void (*func_ptr)() = &greet;
8+
void (&func_ref)() = greet;
9+
// BREAK HERE
10+
return 0;
11+
}

lldb/tools/lldb-dap/JSONUtils.cpp

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,6 +1091,18 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
10911091
return description.trim().str();
10921092
}
10931093

1094+
bool HasValueLocation(lldb::SBValue v) {
1095+
if (!v.GetType().IsPointerType() && !v.GetType().IsReferenceType()) {
1096+
return false;
1097+
}
1098+
1099+
lldb::addr_t addr = v.GetValueAsAddress();
1100+
lldb::SBLineEntry line_entry =
1101+
g_dap.target.ResolveLoadAddress(addr).GetLineEntry();
1102+
1103+
return line_entry.IsValid();
1104+
}
1105+
10941106
// "Variable": {
10951107
// "type": "object",
10961108
// "description": "A Variable is a name/value pair. Optionally a variable
@@ -1160,6 +1172,18 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
11601172
// Object References' in the Overview section for
11611173
// details."
11621174
// },
1175+
// "valueLocationReference": {
1176+
// "type": "integer",
1177+
// "description": "A reference that allows the client to request the
1178+
// location where the variable's value is declared. For
1179+
// example, if the variable contains a function pointer,
1180+
// the adapter may be able to look up the function's
1181+
// location. This should be present only if the adapter
1182+
// is likely to be able to resolve the location.\n\nThis
1183+
// reference shares the same lifetime as the
1184+
// `variablesReference`. See 'Lifetime of Object
1185+
// References' in the Overview section for details."
1186+
// },
11631187
//
11641188
// "$__lldb_extensions": {
11651189
// "description": "Unofficial extensions to the protocol",
@@ -1273,7 +1297,10 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
12731297
object.try_emplace("variablesReference", 0);
12741298

12751299
if (v.GetDeclaration().IsValid())
1276-
object.try_emplace("declarationLocationReference", var_ref);
1300+
object.try_emplace("declarationLocationReference", var_ref << 1);
1301+
1302+
if (HasValueLocation(v))
1303+
object.try_emplace("valueLocationReference", (var_ref << 1) | 1);
12771304

12781305
object.try_emplace("$__lldb_extensions", desc.GetVariableExtensionsJSON());
12791306
return llvm::json::Value(std::move(object));
@@ -1296,8 +1323,8 @@ CreateRunInTerminalReverseRequest(const llvm::json::Object &launch_request,
12961323
llvm::StringRef comm_file,
12971324
lldb::pid_t debugger_pid) {
12981325
llvm::json::Object run_in_terminal_args;
1299-
// This indicates the IDE to open an embedded terminal, instead of opening the
1300-
// terminal in a new window.
1326+
// This indicates the IDE to open an embedded terminal, instead of opening
1327+
// the terminal in a new window.
13011328
run_in_terminal_args.try_emplace("kind", "integrated");
13021329

13031330
auto launch_request_arguments = launch_request.getObject("arguments");

lldb/tools/lldb-dap/JSONUtils.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,9 @@ struct VariableDescription {
421421
std::string GetResult(llvm::StringRef context);
422422
};
423423

424+
/// Does the given variable have an associated value location?
425+
bool HasValueLocation(lldb::SBValue v);
426+
424427
/// Create a "Variable" object for a LLDB thread object.
425428
///
426429
/// This function will fill in the following keys in the returned

lldb/tools/lldb-dap/lldb-dap.cpp

Lines changed: 86 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1346,6 +1346,19 @@ void request_completions(const llvm::json::Object &request) {
13461346
// client can use this optional information to
13471347
// present the variables in a paged UI and fetch
13481348
// them in chunks."
1349+
// },
1350+
// "valueLocationReference": {
1351+
// "type": "integer",
1352+
// "description": "A reference that allows the client to request
1353+
// the location where the returned value is
1354+
// declared. For example, if a function pointer is
1355+
// returned, the adapter may be able to look up the
1356+
// function's location. This should be present only
1357+
// if the adapter is likely to be able to resolve
1358+
// the location.\n\nThis reference shares the same
1359+
// lifetime as the `variablesReference`. See
1360+
// 'Lifetime of Object References' in the
1361+
// Overview section for details."
13491362
// }
13501363
// },
13511364
// "required": [ "result", "variablesReference" ]
@@ -1405,13 +1418,14 @@ void request_evaluate(const llvm::json::Object &request) {
14051418
VariableDescription desc(value);
14061419
EmplaceSafeString(body, "result", desc.GetResult(context));
14071420
EmplaceSafeString(body, "type", desc.display_type_name);
1408-
if (value.MightHaveChildren()) {
1409-
auto variableReference = g_dap.variables.InsertVariable(
1410-
value, /*is_permanent=*/context == "repl");
1411-
body.try_emplace("variablesReference", variableReference);
1412-
} else {
1421+
auto var_ref = g_dap.variables.InsertVariable(
1422+
value, /*is_permanent=*/context == "repl");
1423+
if (value.MightHaveChildren())
1424+
body.try_emplace("variablesReference", var_ref);
1425+
else
14131426
body.try_emplace("variablesReference", (int64_t)0);
1414-
}
1427+
if (HasValueLocation(value))
1428+
body.try_emplace("valueLocationReference", var_ref);
14151429
}
14161430
}
14171431
response.try_emplace("body", std::move(body));
@@ -3573,6 +3587,17 @@ void request_threads(const llvm::json::Object &request) {
35733587
// "description": "The number of indexed child variables. The client
35743588
// can use this optional information to present the variables in a
35753589
// paged UI and fetch them in chunks."
3590+
// },
3591+
// "valueLocationReference": {
3592+
// "type": "integer",
3593+
// "description": "A reference that allows the client to request the
3594+
// location where the new value is declared. For example, if the new
3595+
// value is function pointer, the adapter may be able to look up the
3596+
// function's location. This should be present only if the adapter
3597+
// is likely to be able to resolve the location.\n\nThis reference
3598+
// shares the same lifetime as the `variablesReference`. See
3599+
// 'Lifetime of Object References' in the Overview section for
3600+
// details."
35763601
// }
35773602
// },
35783603
// "required": [ "value" ]
@@ -3597,7 +3622,6 @@ void request_setVariable(const llvm::json::Object &request) {
35973622
response.try_emplace("success", false);
35983623

35993624
lldb::SBValue variable;
3600-
int64_t newVariablesReference = 0;
36013625

36023626
// The "id" is the unique integer ID that is unique within the enclosing
36033627
// variablesReference. It is optionally added to any "interface Variable"
@@ -3627,11 +3651,15 @@ void request_setVariable(const llvm::json::Object &request) {
36273651
// so always insert a new one to get its variablesReference.
36283652
// is_permanent is false because debug console does not support
36293653
// setVariable request.
3630-
if (variable.MightHaveChildren())
3631-
newVariablesReference =
3632-
g_dap.variables.InsertVariable(variable, /*is_permanent=*/false);
3654+
int64_t new_var_ref =
3655+
g_dap.variables.InsertVariable(variable, /*is_permanent=*/false);
36333656

3634-
body.try_emplace("variablesReference", newVariablesReference);
3657+
if (variable.MightHaveChildren())
3658+
body.try_emplace("variablesReference", new_var_ref);
3659+
else
3660+
body.try_emplace("variablesReference", 0);
3661+
if (HasValueLocation(variable))
3662+
body.try_emplace("valueLocationReference", new_var_ref);
36353663
} else {
36363664
EmplaceSafeString(body, "message", std::string(error.GetCString()));
36373665
}
@@ -3925,29 +3953,61 @@ void request_locations(const llvm::json::Object &request) {
39253953
auto arguments = request.getObject("arguments");
39263954

39273955
uint64_t reference_id = GetUnsigned(arguments, "locationReference", 0);
3928-
lldb::SBValue variable = g_dap.variables.GetVariable(reference_id);
3956+
// We use the lowest bit to distinguish between value location and declaration
3957+
// location
3958+
bool isValueLocation = reference_id & 1;
3959+
lldb::SBValue variable = g_dap.variables.GetVariable(reference_id >> 1);
39293960
if (!variable.IsValid()) {
39303961
response["success"] = false;
39313962
response["message"] = "Invalid variable reference";
39323963
g_dap.SendJSON(llvm::json::Value(std::move(response)));
39333964
return;
39343965
}
39353966

3936-
// Get the declaration location
3937-
lldb::SBDeclaration decl = variable.GetDeclaration();
3938-
if (!decl.IsValid()) {
3939-
response["success"] = false;
3940-
response["message"] = "No declaration location available";
3941-
g_dap.SendJSON(llvm::json::Value(std::move(response)));
3942-
return;
3943-
}
3944-
39453967
llvm::json::Object body;
3946-
body.try_emplace("source", CreateSource(decl.GetFileSpec()));
3947-
if (int line = decl.GetLine())
3948-
body.try_emplace("line", line);
3949-
if (int column = decl.GetColumn())
3950-
body.try_emplace("column", column);
3968+
if (isValueLocation) {
3969+
// Get the value location
3970+
if (!variable.GetType().IsPointerType() &&
3971+
!variable.GetType().IsReferenceType()) {
3972+
response["success"] = false;
3973+
response["message"] =
3974+
"Value locations are only available for pointers and references";
3975+
g_dap.SendJSON(llvm::json::Value(std::move(response)));
3976+
return;
3977+
}
3978+
3979+
lldb::addr_t addr = variable.GetValueAsAddress();
3980+
lldb::SBLineEntry line_entry =
3981+
g_dap.target.ResolveLoadAddress(addr).GetLineEntry();
3982+
3983+
if (!line_entry.IsValid()) {
3984+
response["success"] = false;
3985+
response["message"] = "Failed to resolve line entry for location";
3986+
g_dap.SendJSON(llvm::json::Value(std::move(response)));
3987+
return;
3988+
}
3989+
3990+
body.try_emplace("source", CreateSource(line_entry.GetFileSpec()));
3991+
if (int line = line_entry.GetLine())
3992+
body.try_emplace("line", line);
3993+
if (int column = line_entry.GetColumn())
3994+
body.try_emplace("column", column);
3995+
} else {
3996+
// Get the declaration location
3997+
lldb::SBDeclaration decl = variable.GetDeclaration();
3998+
if (!decl.IsValid()) {
3999+
response["success"] = false;
4000+
response["message"] = "No declaration location available";
4001+
g_dap.SendJSON(llvm::json::Value(std::move(response)));
4002+
return;
4003+
}
4004+
4005+
body.try_emplace("source", CreateSource(decl.GetFileSpec()));
4006+
if (int line = decl.GetLine())
4007+
body.try_emplace("line", line);
4008+
if (int column = decl.GetColumn())
4009+
body.try_emplace("column", column);
4010+
}
39514011

39524012
response.try_emplace("body", std::move(body));
39534013
g_dap.SendJSON(llvm::json::Value(std::move(response)));

0 commit comments

Comments
 (0)