[Lldb-commits] [PATCH] D89925: [lldb] Fix a regression introduced by D75730

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, jingham.
Herald added a subscriber: pengfei.
Herald added a project: LLDB.
JDevlieghere requested review of this revision.

In a new Range class was introduced to simplify and the Disassembler API and reduce duplication. It unintentionally broke the `SBFrame::Disassemble` functionality because it unconditionally converts the number of instructions to a `Range{Limit::Instructions, num_instructions}`. This is subtly different from the previous behavior, where now we're passing a Range and assume it's valid in the callee, the original code would propagate `num_instructions` and the callee would compare the value and decided between disassembling instructions or bytes.

Unfortunately the existing tests was not particularly strict:

  disassembly = frame.Disassemble()
  self.assertNotEqual(len(disassembly), 0, "Disassembly was empty.")

This would pass because without this patch we'd disassemble zero instructions, resulting in an error:

  ./bin/lldb /tmp/a.out -o 'b main' -o r -o 'script print(lldb.frame.Disassemble())'
  (lldb) target create "/tmp/a.out"
  Current executable set to '/tmp/a.out' (x86_64).
  (lldb) b main
  Breakpoint 1: where = a.out`main + 11 at foo.c:2:3, address = 0x0000000100003fab
  (lldb) r
  Process 22141 stopped
  * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
      frame #0: 0x0000000100003fab a.out`main at foo.c:2:3
     1    int main() {
  -> 2      return 10;
     3    }
  
  Process 22141 launched: '/tmp/a.out' (x86_64)
  (lldb) script print(lldb.frame.Disassemble())
  error: error reading data from section __text


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89925

Files:
  lldb/source/Core/Disassembler.cpp
  lldb/test/API/commands/disassemble/basic/TestFrameDisassemble.py
  lldb/test/API/commands/disassemble/basic/main.cpp


Index: lldb/test/API/commands/disassemble/basic/main.cpp
===================================================================
--- lldb/test/API/commands/disassemble/basic/main.cpp
+++ lldb/test/API/commands/disassemble/basic/main.cpp
@@ -2,6 +2,7 @@
 sum (int a, int b)
 {
     int result = a + b; // Set a breakpoint here
+    asm("nop");
     return result;
 }
 
Index: lldb/test/API/commands/disassemble/basic/TestFrameDisassemble.py
===================================================================
--- lldb/test/API/commands/disassemble/basic/TestFrameDisassemble.py
+++ lldb/test/API/commands/disassemble/basic/TestFrameDisassemble.py
@@ -57,4 +57,6 @@
 
         frame = threads[0].GetFrameAtIndex(0)
         disassembly = frame.Disassemble()
-        self.assertNotEqual(len(disassembly), 0, "Disassembly was empty.")
+        self.assertNotEqual(disassembly, "")
+        self.assertNotIn("error", disassembly)
+        self.assertIn(": nop", disassembly)
Index: lldb/source/Core/Disassembler.cpp
===================================================================
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -564,10 +564,16 @@
       range.SetByteSize(DEFAULT_DISASM_BYTE_SIZE);
   }
 
-  return Disassemble(
-      debugger, arch, plugin_name, flavor, exe_ctx, range.GetBaseAddress(),
-      {Limit::Instructions, num_instructions}, mixed_source_and_assembly,
-      num_mixed_context_lines, options, strm);
+  Disassembler::Limit limit = {Limit::Instructions, num_instructions};
+  if (num_instructions == 0) {
+    limit = {Disassembler::Limit::Bytes, range.GetByteSize()};
+    if (limit.value == 0)
+      limit.value = DEFAULT_DISASM_BYTE_SIZE;
+  }
+
+  return Disassemble(debugger, arch, plugin_name, flavor, exe_ctx,
+                     range.GetBaseAddress(), limit, mixed_source_and_assembly,
+                     num_mixed_context_lines, options, strm);
 }
 
 Instruction::Instruction(const Address &address, AddressClass addr_class)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89925.299853.patch
Type: text/x-patch
Size: 1996 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20201022/511ac351/attachment.bin>