[PATCH v3 4/5] [gdb] Handle sections when disassembling

[Tom de Vries <tdevries@suse.de>]

Consider the following session:
...
$ gdb -q -batch a.out -ex start -ex "disassemble _init,0x0000000000401030"
  ...
Dump of assembler code from 0x401000 to 0x401030:
   ...
   0x000000000040101a <_init+26>:	ret
   0x000000000040101b:	add    %al,(%rax)
   0x000000000040101d:	add    %al,(%rax)
   0x000000000040101f:	add    %bh,%bh
   0x0000000000401021:	xor    $0x2fca,%eax
   0x0000000000401026:	jmp    *0x2fcc(%rip)
   0x000000000040102c:	nopl   0x0(%rax)
End of assembler dump.
...

If disassemble with objdump instead, we get:
...
Disassembly of section .init:

0000000000401000 <_init>:
  ...
  40101a:       c3                      ret

Disassembly of section .plt:

0000000000401020 <puts@plt-0x10>:
  401020:       ff 35 ca 2f 00 00       push   0x2fca(%rip)
  401026:       ff 25 cc 2f 00 00       jmp    *0x2fcc(%rip)
  40102c:       0f 1f 40 00             nopl   0x0(%rax)
...

Note the xor vs push.

So gdb:
- first disassembles the .init section
- then disassembles the section hole in between .init and .plt
- interprets the first byte of .plt as the last byte of an instruction
  starting in the section hole
- continues disassembling at .plt+1 instead of .plt+0

Fix this in gdb_disassembly by making it section-aware, and not disassembling
across a section change, getting us instead:
...
   0x000000000040101a <_init+26>:	ret
   0x000000000040101b:	add    %al,(%rax)
   0x000000000040101d:	add    %al,(%rax)
   0x000000000040101f:	add    %bh,%bh
   0x0000000000401020:	push   0x2fca(%rip)        # 0x403ff0
   0x0000000000401026:	jmp    *0x2fcc(%rip)        # 0x403ff8
   0x000000000040102c:	nopl   0x0(%rax)
...

Likewise in tui_disassemble.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34617
---
 gdb/disasm.c         | 72 ++++++++++++++++++++++++++++++-------
 gdb/tui/tui-disasm.c | 85 ++++++++++++++++++++++++++++++++++----------
 2 files changed, 125 insertions(+), 32 deletions(-)

diff --git a/gdb/disasm.c b/gdb/disasm.c
index 01361fe4323..9e8334b065e 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -34,6 +34,7 @@
 #include "cli/cli-style.h"
 #include "objfiles.h"
 #include "inferior.h"
+#include "addrmap.h"
 
 /* Disassemble functions.
    FIXME: We should get rid of all the duplicate code in gdb that does
@@ -524,7 +525,7 @@ dump_insns (struct gdbarch *gdbarch,
 
    N.B. This view is deprecated.  */
 
-static void
+static int
 do_mixed_source_and_assembly_deprecated
   (struct gdbarch *gdbarch, struct ui_out *uiout,
    struct symtab *symtab,
@@ -669,6 +670,8 @@ do_mixed_source_and_assembly_deprecated
       if (how_many >= 0 && num_displayed >= how_many)
 	break;
     }
+
+  return num_displayed;
 }
 
 /* The idea here is to present a source-O-centric view of a
@@ -676,7 +679,7 @@ do_mixed_source_and_assembly_deprecated
    in source order, with (possibly) out of order assembly
    immediately following.  */
 
-static void
+static int
 do_mixed_source_and_assembly (struct gdbarch *gdbarch,
 			      struct ui_out *uiout,
 			      struct symtab *main_symtab,
@@ -906,16 +909,18 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
       last_symtab = sal.symtab;
       last_line = sal.line;
     }
+
+  return num_displayed;
 }
 
-static void
+static int
 do_assembly_only (struct gdbarch *gdbarch, struct ui_out *uiout,
 		  CORE_ADDR low, CORE_ADDR high,
 		  int how_many, gdb_disassembly_flags flags)
 {
   ui_out_emit_list list_emitter (uiout, "asm_insns");
 
-  dump_insns (gdbarch, uiout, low, high, how_many, flags, NULL);
+  return dump_insns (gdbarch, uiout, low, high, how_many, flags, NULL);
 }
 
 /* Combine implicit and user disassembler options and return them
@@ -1141,10 +1146,12 @@ gdb_disassembler::print_insn (CORE_ADDR memaddr,
   return length;
 }
 
-void
-gdb_disassembly (struct gdbarch *gdbarch, struct ui_out *uiout,
-		 gdb_disassembly_flags flags, int how_many,
-		 CORE_ADDR low, CORE_ADDR high)
+/* Helper function for gdb_disassembly.  */
+
+static int
+gdb_disassembly_1 (struct gdbarch *gdbarch, struct ui_out *uiout,
+		   gdb_disassembly_flags flags, int how_many,
+		   CORE_ADDR low, CORE_ADDR high)
 {
   struct symtab *symtab;
   int nlines = -1;
@@ -1155,19 +1162,58 @@ gdb_disassembly (struct gdbarch *gdbarch, struct ui_out *uiout,
   if (symtab != NULL && symtab->linetable () != NULL)
     nlines = symtab->linetable ()->nitems;
 
+  int num_displayed = 0;
   if (!(flags & (DISASSEMBLY_SOURCE_DEPRECATED | DISASSEMBLY_SOURCE))
       || nlines <= 0)
-    do_assembly_only (gdbarch, uiout, low, high, how_many, flags);
+    num_displayed
+      = do_assembly_only (gdbarch, uiout, low, high, how_many, flags);
 
   else if (flags & DISASSEMBLY_SOURCE)
-    do_mixed_source_and_assembly (gdbarch, uiout, symtab, low, high,
-				  how_many, flags);
+    num_displayed
+      = do_mixed_source_and_assembly (gdbarch, uiout, symtab, low, high,
+				      how_many, flags);
 
   else if (flags & DISASSEMBLY_SOURCE_DEPRECATED)
-    do_mixed_source_and_assembly_deprecated (gdbarch, uiout, symtab,
-					     low, high, how_many, flags);
+    num_displayed
+      = do_mixed_source_and_assembly_deprecated (gdbarch, uiout, symtab,
+						 low, high, how_many, flags);
 
   gdb_flush (gdb_stdout);
+  return num_displayed;
+}
+
+void
+gdb_disassembly (struct gdbarch *gdbarch, struct ui_out *uiout,
+		 gdb_disassembly_flags flags, int how_many,
+		 CORE_ADDR low, CORE_ADDR high)
+{
+  std::unique_ptr<addrmap_mutable> map = section_addrmap ();
+  bool update_how_many = how_many != -1;
+
+  while (low < high)
+    {
+      CORE_ADDR tmp_high = high;
+
+      CORE_ADDR range_high;
+      map->find (low, nullptr, &range_high);
+
+      /* Don't disassemble past a section change.  */
+      if (range_high != (CORE_ADDR)-1)
+	tmp_high = std::min (tmp_high, range_high + 1);
+
+      int res
+	= gdb_disassembly_1 (gdbarch, uiout, flags, how_many, low,
+			     tmp_high);
+
+      if (update_how_many)
+	{
+	  how_many -= res;
+	  if (how_many <= 0)
+	    break;
+	}
+
+      low = tmp_high;
+    }
 }
 
 /* Print the instruction at address MEMADDR in debugged memory,
diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 1a1866003dd..26afc4bd978 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -38,6 +38,7 @@
 #include "tui/tui-location.h"
 #include "gdbsupport/selftest.h"
 #include "inferior.h"
+#include "addrmap.h"
 
 struct tui_asm_line
 {
@@ -80,23 +81,13 @@ len_without_escapes (const std::string &str)
   return len;
 }
 
-/* Function to disassemble up to COUNT instructions starting from address
-   PC into the ASM_LINES vector (which will be emptied of any previous
-   contents).  Return the address after the last disassembled instruction.
-   When ADDR_SIZE is non-null then place the maximum size of an address and
-   label into the value pointed to by ADDR_SIZE, and set the addr_size
-   field on each item in ASM_LINES, otherwise the addr_size fields within
-   ASM_LINES are undefined.
+/* Helper function for tui_disassemble.  */
 
-   It is worth noting that ASM_LINES might not have COUNT entries when this
-   function returns.  If the disassembly is truncated for some other
-   reason, for example, we hit invalid memory, then ASM_LINES can have
-   fewer entries than requested.  */
 static CORE_ADDR
-tui_disassemble (struct gdbarch *gdbarch,
-		 std::vector<tui_asm_line> &asm_lines,
-		 CORE_ADDR pc, int count,
-		 size_t *addr_size = nullptr)
+tui_disassemble_1 (struct gdbarch *gdbarch,
+		   std::vector<tui_asm_line> &asm_lines, CORE_ADDR pc,
+		   std::optional<CORE_ADDR> high_pc, int count,
+		   size_t *addr_size)
 {
   bool term_out = disassembler_styling && gdb_stdout->can_emit_style_escape ();
   string_file gdb_dis_out (term_out);
@@ -104,20 +95,29 @@ tui_disassemble (struct gdbarch *gdbarch,
 			    ? (decltype (stream))&null_stream
 			    : (decltype (stream))&gdb_dis_out);
 
-  /* Must start with an empty list.  */
-  asm_lines.clear ();
-
   /* Now construct each line.  */
   for (int i = 0; i < count; ++i)
     {
       tui_asm_line tal;
 
+      if (high_pc.has_value () && pc >= *high_pc)
+	break;
+
       /* Save the instruction address.  */
       tal.addr = pc;
 
       try
 	{
-	  pc += gdb_print_insn (gdbarch, pc, stream, NULL);
+	  int len = gdb_print_insn (gdbarch, pc, stream, NULL);
+	  CORE_ADDR next_pc = pc + len;
+	  if (high_pc.has_value () && next_pc > *high_pc)
+	    {
+	      /* Instruction spans high_pc.  */
+	      tal.insn = "(bad)";
+	      next_pc = *high_pc;
+	    }
+
+	  pc = next_pc;
 	}
       catch (const gdb_exception_error &except)
 	{
@@ -153,6 +153,53 @@ tui_disassemble (struct gdbarch *gdbarch,
   return pc;
 }
 
+/* Function to disassemble up to COUNT instructions starting from address
+   PC into the ASM_LINES vector (which will be emptied of any previous
+   contents).  Return the address after the last disassembled instruction.
+   When ADDR_SIZE is non-null then place the maximum size of an address and
+   label into the value pointed to by ADDR_SIZE, and set the addr_size
+   field on each item in ASM_LINES, otherwise the addr_size fields within
+   ASM_LINES are undefined.
+
+   It is worth noting that ASM_LINES might not have COUNT entries when this
+   function returns.  If the disassembly is truncated for some other
+   reason, for example, we hit invalid memory, then ASM_LINES can have
+   fewer entries than requested.  */
+
+static CORE_ADDR
+tui_disassemble (struct gdbarch *gdbarch,
+		 std::vector<tui_asm_line> &asm_lines,
+		 CORE_ADDR pc, int count,
+		 size_t *addr_size = nullptr)
+{
+  std::unique_ptr<addrmap_mutable> map = section_addrmap ();
+
+  /* Must start with an empty list.  */
+  asm_lines.clear ();
+
+  while (count > 0)
+    {
+      CORE_ADDR range_high;
+      map->find (pc, nullptr, &range_high);
+
+      /* Don't disassemble past a section change.  */
+      std::optional<CORE_ADDR> high_pc;
+      if (range_high != (CORE_ADDR)-1)
+	high_pc = range_high + 1;
+
+      int prev_nr_lines = asm_lines.size ();
+      pc = tui_disassemble_1 (gdbarch, asm_lines, pc, high_pc, count,
+			      addr_size);
+      int nr_lines = asm_lines.size () - prev_nr_lines;
+      if (nr_lines == 0)
+	break;
+
+      count -= nr_lines;
+    }
+
+  return pc;
+}
+
 /* Look backward from ADDR for an address from which we can start
    disassembling, this needs to be something we can be reasonably
    confident will fall on an instruction boundary.  We use msymbol
-- 
2.51.0