https://github.com/vogelsgesang updated https://github.com/llvm/llvm-project/pull/104589
>From 72c725469268d0bd39666d20f94404f81a0cb7ad Mon Sep 17 00:00:00 2001 From: Adrian Vogelsgesang <avogelsges...@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 | 47 +++++++- 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, 175 insertions(+), 39 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..bea660d92b7a60 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,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..a22483d0b2c3c1 --- /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; + // 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))); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
