[Lldb-commits] [lldb] [lldb-dap] Fix disassembled ranges for `disassemble` request (PR #105446)

Adrian Vogelsgesang via lldb-commits lldb-commits at lists.llvm.org
Tue Aug 20 16:05:45 PDT 2024


https://github.com/vogelsgesang created https://github.com/llvm/llvm-project/pull/105446

This is a first draft PR which fixes #103021

The main issue was that the `instructionOffset` was handled as a byte offset and not as an instruction offset.

This commit also incorporates previous feedback from https://reviews.llvm.org/D140358: With this change, we are using symbols and DWARF debug information to find function boundaries and correctly start disassembling at this address.

TODO:
* Update test case
* Check if we can also support disassembling across functions



>From b809f570dd8055e5b899c337ec9ff5110ab94c6e Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Mon, 19 Aug 2024 15:15:22 +0000
Subject: [PATCH] [lldb-dap] Fix disassembled ranges for `disassemble` request

TODO:
* Update test case
* Check if we can also support disassembling across functions

Incorporated feedback from https://reviews.llvm.org/D140358
---
 .../test/tools/lldb-dap/dap_server.py         |  4 +-
 .../disassemble/TestDAP_disassemble.py        |  8 +++
 lldb/tools/lldb-dap/lldb-dap.cpp              | 68 +++++++++++++++++--
 3 files changed, 71 insertions(+), 9 deletions(-)

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..75731ebfde6723 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
@@ -674,11 +674,11 @@ def request_disconnect(self, terminateDebuggee=None):
         return self.send_recv(command_dict)
 
     def request_disassemble(
-        self, memoryReference, offset=-50, instructionCount=200, resolveSymbols=True
+        self, memoryReference, instructionOffset=0, instructionCount=10, resolveSymbols=True
     ):
         args_dict = {
             "memoryReference": memoryReference,
-            "offset": offset,
+            "instructionOffset": instructionOffset,
             "instructionCount": instructionCount,
             "resolveSymbols": resolveSymbols,
         }
diff --git a/lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py b/lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py
index 9e8ef5b289f2e8..ab72eb2d13d729 100644
--- a/lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py
+++ b/lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py
@@ -29,6 +29,14 @@ def test_disassemble(self):
         )
         self.continue_to_next_stop()
 
+        stackFrames = self.get_stackFrames(
+            threadId=threadId, startFrame=frameIndex, levels=1
+        )
+        self.assertIsNotNone(stackFrames)
+
+        # XXX finish updating test case
+        memoryReference = stackFrames[0]["instructionPointerReference"]
+
         pc_assembly = self.disassemble(frameIndex=0)
         self.assertIn("location", pc_assembly, "Source location missing.")
         self.assertIn("instruction", pc_assembly, "Assembly instruction missing.")
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index ea84f31aec3a6c..5d9d28b59ea805 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -3910,8 +3910,8 @@ void request_disassemble(const llvm::json::Object &request) {
     return;
   }
 
-  addr_ptr += GetSigned(arguments, "instructionOffset", 0);
-  lldb::SBAddress addr(addr_ptr, g_dap.target);
+  addr_ptr += GetSigned(arguments, "offset", 0);
+  lldb::SBAddress addr = g_dap.target.ResolveLoadAddress(addr_ptr);
   if (!addr.IsValid()) {
     response["success"] = false;
     response["message"] = "Memory reference not found in the current binary.";
@@ -3919,9 +3919,27 @@ void request_disassemble(const llvm::json::Object &request) {
     return;
   }
 
-  const auto inst_count = GetUnsigned(arguments, "instructionCount", 0);
-  lldb::SBInstructionList insts =
-      g_dap.target.ReadInstructions(addr, inst_count);
+  int64_t inst_offset = GetSigned(arguments, "instructionOffset", 0);
+  const int64_t inst_count = GetSigned(arguments, "instructionCount", 0);
+  if (inst_count < 0) {
+    response["success"] = false;
+    response["message"] = "Negative instruction count.";
+    g_dap.SendJSON(llvm::json::Value(std::move(response)));
+    return;
+  }
+
+  lldb::SBInstructionList insts;
+  if (lldb::SBFunction func = addr.GetFunction()) {
+    // First try to identify the function boundaries through debug information
+    insts = func.GetInstructions(g_dap.target);
+  } else if (lldb::SBSymbol sym = addr.GetSymbol()) {
+    // Try to identify the function boundaries through the symbol table
+    insts = sym.GetInstructions(g_dap.target);
+  } else {
+    // We could not identify the function. Just disassemble starting from the
+    // provided address.
+    insts = g_dap.target.ReadInstructions(addr, inst_offset + inst_count);
+  }
 
   if (!insts.IsValid()) {
     response["success"] = false;
@@ -3930,10 +3948,46 @@ void request_disassemble(const llvm::json::Object &request) {
     return;
   }
 
+  // Find the start offset in the disassembled function
+  ssize_t offset = 0;
+  while (insts.GetInstructionAtIndex(offset).GetAddress().GetLoadAddress(
+             g_dap.target) < addr.GetLoadAddress(g_dap.target))
+    ++offset;
+  offset += inst_offset;
+
+  // Format the disassembled instructions
   const bool resolveSymbols = GetBoolean(arguments, "resolveSymbols", false);
   llvm::json::Array instructions;
-  const auto num_insts = insts.GetSize();
-  for (size_t i = 0; i < num_insts; ++i) {
+  for (ssize_t i = offset; i < inst_count + offset; ++i) {
+    // Before the range of disassembled instructions?
+    if (i < 0) {
+      auto fake_addr =
+          insts.GetInstructionAtIndex(0).GetAddress().GetLoadAddress(
+              g_dap.target) -
+          offset + i;
+      llvm::json::Object disassembled_inst{
+          {"address", "0x" + llvm::utohexstr(fake_addr)},
+          {"instruction", "Instruction before disassembled address range"},
+          {"presentationHint", "invalid"},
+      };
+      instructions.emplace_back(std::move(disassembled_inst));
+      continue;
+    }
+    // Past the range of disassembled instructions?
+    if (static_cast<uint64_t>(i) >= insts.GetSize()) {
+      auto fake_addr = insts.GetInstructionAtIndex(insts.GetSize() - 1)
+                           .GetAddress()
+                           .GetLoadAddress(g_dap.target) -
+                       offset + i - insts.GetSize();
+      llvm::json::Object disassembled_inst{
+          {"address", "0x" + llvm::utohexstr(fake_addr)},
+          {"instruction", "Instruction after disassembled address range"},
+          {"presentationHint", "invalid"},
+      };
+      instructions.emplace_back(std::move(disassembled_inst));
+      continue;
+    }
+
     lldb::SBInstruction inst = insts.GetInstructionAtIndex(i);
     auto addr = inst.GetAddress();
     const auto inst_addr = addr.GetLoadAddress(g_dap.target);



More information about the lldb-commits mailing list