[lldb-dap] fix disassembly request instruction offset handling (#140486)

Fix the handling of the `instructionOffset` parameter, which resulted in
always returning the wrong disassembly because VSCode always uses
`instructionOffset = -50` and expects 50 instructions before the given
address, instead of 50 bytes before

Author: eronnen

lldb/include/lldb/API/SBTarget.h

@@ -349,6 +349,18 @@ class LLDB_API SBTarget {
 
   SBError SetLabel(const char *label);
 
+  /// Architecture opcode byte size width accessor
+  ///
+  /// \return
+  /// The minimum size in 8-bit (host) bytes of an opcode.
+  uint32_t GetMinimumOpcodeByteSize() const;
+
+  /// Architecture opcode byte size width accessor
+  ///
+  /// \return
+  /// The maximum size in 8-bit (host) bytes of an opcode.
+  uint32_t GetMaximumOpcodeByteSize() const;
+
   /// Architecture data byte width accessor
   ///
   /// \return

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py

@@ -136,7 +136,6 @@ def __init__(
         self.initialized = False
         self.frame_scopes = {}
         self.init_commands = init_commands
-        self.disassembled_instructions = {}
 
     @classmethod
     def encode_content(cls, s: str) -> bytes:
@@ -735,11 +734,15 @@ 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=-50,
+        instructionCount=200,
+        resolveSymbols=True,
     ):
         args_dict = {
             "memoryReference": memoryReference,
-            "offset": offset,
+            "instructionOffset": instructionOffset,
             "instructionCount": instructionCount,
             "resolveSymbols": resolveSymbols,
         }
@@ -748,9 +751,7 @@ def request_disassemble(
             "type": "request",
             "arguments": args_dict,
         }
-        instructions = self.send_recv(command_dict)["body"]["instructions"]
-        for inst in instructions:
-            self.disassembled_instructions[inst["address"]] = inst
+        return self.send_recv(command_dict)["body"]["instructions"]
 
     def request_readMemory(self, memoryReference, offset, count):
         args_dict = {

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py

@@ -346,10 +346,14 @@ def disassemble(self, threadId=None, frameIndex=None):
         memoryReference = stackFrames[0]["instructionPointerReference"]
         self.assertIsNotNone(memoryReference)
 
-        if memoryReference not in self.dap_server.disassembled_instructions:
-            self.dap_server.request_disassemble(memoryReference=memoryReference)
+        instructions = self.dap_server.request_disassemble(
+            memoryReference=memoryReference
+        )
+        disassembled_instructions = {}
+        for inst in instructions:
+            disassembled_instructions[inst["address"]] = inst
 
-        return self.dap_server.disassembled_instructions[memoryReference]
+        return disassembled_instructions, disassembled_instructions[memoryReference]
 
     def attach(
         self,

lldb/source/API/SBTarget.cpp

@@ -1668,6 +1668,26 @@ SBError SBTarget::SetLabel(const char *label) {
   return Status::FromError(target_sp->SetLabel(label));
 }
 
+uint32_t SBTarget::GetMinimumOpcodeByteSize() const {
+  LLDB_INSTRUMENT_VA(this);
+
+  TargetSP target_sp(GetSP());
+  if (target_sp)
+    return target_sp->GetArchitecture().GetMinimumOpcodeByteSize();
+
+  return 0;
+}
+
+uint32_t SBTarget::GetMaximumOpcodeByteSize() const {
+  LLDB_INSTRUMENT_VA(this);
+
+  TargetSP target_sp(GetSP());
+  if (target_sp)
+    return target_sp->GetArchitecture().GetMaximumOpcodeByteSize();
+
+  return 0;
+}
+
 uint32_t SBTarget::GetDataByteSize() {
   LLDB_INSTRUMENT_VA(this);
 

lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py

@@ -20,21 +20,56 @@ def test_disassemble(self):
         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, "// breakpoint 1"),
-            ],
-        )
+        self.set_source_breakpoints(source, [line_number(source, "// breakpoint 1")])
         self.continue_to_next_stop()
 
-        pc_assembly = self.disassemble(frameIndex=0)
+        _, pc_assembly = self.disassemble(frameIndex=0)
         self.assertIn("location", pc_assembly, "Source location missing.")
         self.assertIn("instruction", pc_assembly, "Assembly instruction missing.")
 
         # The calling frame (qsort) is coming from a system library, as a result
         # we should not have a source location.
-        qsort_assembly = self.disassemble(frameIndex=1)
+        _, qsort_assembly = self.disassemble(frameIndex=1)
         self.assertNotIn("location", qsort_assembly, "Source location not expected.")
         self.assertIn("instruction", pc_assembly, "Assembly instruction missing.")
+
+    def test_disassemble_backwards(self):
+        """
+        Tests the 'disassemble' request with a backwards disassembly range.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.c"
+        self.set_source_breakpoints(source, [line_number(source, "// breakpoint 1")])
+        self.continue_to_next_stop()
+
+        instruction_pointer_reference = self.get_stackFrames()[1][
+            "instructionPointerReference"
+        ]
+        backwards_instructions = 200
+        instructions_count = 400
+        instructions = self.dap_server.request_disassemble(
+            memoryReference=instruction_pointer_reference,
+            instructionOffset=-backwards_instructions,
+            instructionCount=instructions_count,
+        )
+
+        self.assertEqual(
+            len(instructions),
+            instructions_count,
+            "Disassemble request should return the exact requested number of instructions.",
+        )
+
+        frame_instruction_index = next(
+            (
+                i
+                for i, instruction in enumerate(instructions)
+                if instruction["address"] == instruction_pointer_reference
+            ),
+            -1,
+        )
+        self.assertEqual(
+            frame_instruction_index,
+            backwards_instructions,
+            f"requested instruction should be preceeded by {backwards_instructions} instructions. Actual index: {frame_instruction_index}",
+        )

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

@@ -27,4 +27,4 @@ int main(void) {
 
   printf("\n");
   return 0;
-}
+}

lldb/test/API/tools/lldb-dap/instruction-breakpoint/TestDAP_instruction_breakpoint.py

@@ -66,16 +66,15 @@ def instruction_breakpoint_test(self):
         )
 
         # Check disassembly view
-        instruction = self.disassemble(frameIndex=0)
+        disassembled_instructions, instruction = self.disassemble(frameIndex=0)
         self.assertEqual(
             instruction["address"],
             intstructionPointerReference[0],
             "current breakpoint reference is not in the disaasembly view",
         )
 
         # Get next instruction address to set instruction breakpoint
-        disassembled_instruction_list = self.dap_server.disassembled_instructions
-        instruction_addr_list = list(disassembled_instruction_list.keys())
+        instruction_addr_list = list(disassembled_instructions.keys())
         index = instruction_addr_list.index(intstructionPointerReference[0])
         if len(instruction_addr_list) >= (index + 1):
             next_inst_addr = instruction_addr_list[index + 1]