[PATCH v2 7/7] [gdb/tui] Handle section holes when backward disassembling

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

Consider a hello world:
...
$ gcc hello.c -g
...
and gdb setup like this:
...
$ gdb -q a.out -ex "layout asm" -ex "disassemble 0x401020,0x401040"
...
which gives us the following asm window:
...
┌─────────────────────────────────────────────────────────────────────┐
│   0x401020                push   0x2fca(%rip)        # 0x403ff0     │
│   0x401026                jmp    *0x2fcc(%rip)        # 0x403ff8    │
│   0x40102c                nopl   0x0(%rax)                          │
│   0x401030 <puts@plt>     jmp    *0x2fca(%rip)        # 0x404000 <pu│
│   0x401036 <puts@plt+6>   push   $0x0                               │
│   0x40103b <puts@plt+11>  jmp    0x401020                           │
│   0x401040 <_start>       endbr64                                   │
│   0x401044 <_start+4>     xor    %ebp,%ebp                          │
│   0x401046 <_start+6>     mov    %rdx,%r9                           │
│   0x401049 <_start+9>     pop    %rsi                               │
└─────────────────────────────────────────────────────────────────────┘
...

We can't scroll back from the plt section into the init section, because
there's a section hole.

Unlike the previous commit, that doesn't improve if we use start.

The problem is that tui_find_backward_disassembly_start_address is not able
move past section holes.

Fix this by using the prev section returned by find_pc_section.

This also fixes the paradoxical situation reported in PR34399 where it's
possible to scroll forward into a section hole, but not back out of it.

This also break gdb.tui/tui-layout-asm-short-prog.exp test-case, because it
expects not being able to disassemble instructions before .text.

It's easy enough to fix that using:
...
-Term::command "- 15"
+Term::command "- 4"
...
but AFAIU, that breaks the purpose of the test-case.

So instead, I've used objcopy --remove-section=.note.* to strip the sections
before .text.

I fear this may be fragile though.

The patch caused a regression in this unit test:
...
      SELF_CHECK (tui_find_disassembly_address (gdbarch, 0, -1) == 0);
...
because this assert got triggered:
...
      /* When scrolling backward the addresses should move backward, or at
	 the very least stay the same if we are at the first address that
	 can be disassembled.  */
      gdb_assert (new_low <= pc);
...

I wrote two fixes for this in tui_find_disassembly_address.  Each fix on its
own fixes the regression, but ISTM both make sense, so I included both.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34399
---
 .../gdb.tui/tui-layout-asm-short-prog.exp     | 25 +++++++++++++++
 gdb/tui/tui-disasm.c                          | 31 +++++++++++++++++--
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.exp b/gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.exp
index b3dd72ce1ce..0fad6613e23 100644
--- a/gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.exp
+++ b/gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.exp
@@ -30,6 +30,31 @@ if { [build_executable "failed to prepare" $testfile $srcfile $opts] == -1 } {
     return
 }
 
+# This test-case relies on not being able to scroll to before _start.  That
+# used to be the case because there's a section hole in between
+# .note.gnu.build-id and .text:
+#
+# [Nr] Name               Type      Address  Off    Size   ES Flg Lk Inf Al
+# [ 0]                    NULL      00000000 000000 000000 00      0   0  0
+# [ 1] .note.gnu.property NOTE      00400190 000190 000030 00   A  0   0  8
+# [ 2] .note.gnu.build-id NOTE      004001c0 0001c0 000024 00   A  0   0  4
+# [ 3] .text              PROGBITS  00401000 001000 000005 00  AX  0   0  1
+#
+# Now that section holes no longer stop scrolling, strip the notes to get:
+#
+# [Nr] Name               Type      Address  Off    Size   ES Flg Lk Inf Al
+# [ 0]                    NULL      00000000 000000 000000 00      0   0  0
+# [ 1] .text              PROGBITS  00401000 001000 000005 00  AX  0   0  1
+
+set objcopy_program [gdb_find_objcopy]
+set res \
+    [remote_exec host \
+	 "$objcopy_program --remove-section=.note.* $binfile"]
+if {[lindex $res 0] != 0} {
+    unsupported "Couldn't strip notes"
+    return
+}
+
 Term::clean_restart 24 80 $testfile
 if {![Term::prepare_for_tui]} {
     unsupported "TUI not supported"
diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index d96048dc1de..bbb447fcf0f 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -201,10 +201,10 @@ tui_find_backward_disassembly_start_address (CORE_ADDR addr)
   /* Find the first section with start address before ADDR, and use its start
      address.  The found section may be the one containing ADDR, or the one
      before it.  */
-  struct obj_section *section;
+  struct obj_section *section, *prev;
   for (int offset = 0; offset <= 1; ++offset)
     {
-      section = find_pc_section (addr - offset);
+      section = find_pc_section (addr - offset, &prev);
       if (offset == 0 && section != nullptr && section->addr () == addr)
 	{
 	  /* If ADDR is the start of its section, use ADDR - 1.  */
@@ -217,6 +217,20 @@ tui_find_backward_disassembly_start_address (CORE_ADDR addr)
   if (section != NULL)
     return section->addr ();
 
+  if (prev != nullptr
+      && (bfd_section_flags (prev->the_bfd_section) & SEC_ALLOC) != 0)
+    {
+      /* Skip over section hole and use previous section.  */
+
+      /* If not causing infinite recursion, self-recurse to possibly use
+	 minimal symbols in the previous section.  */
+      if (prev->endaddr () < addr)
+	return tui_find_backward_disassembly_start_address (prev->endaddr ());
+
+      /* Fallback: simply use start of previous section.  */
+      return prev->addr ();
+    }
+
   return addr;
 }
 
@@ -282,6 +296,12 @@ tui_find_disassembly_address (struct gdbarch *gdbarch, CORE_ADDR pc, int from)
 	  /* Find an address from which we can start disassembling.  */
 	  prev_low = new_low;
 	  new_low = tui_find_backward_disassembly_start_address (new_low);
+	  if (new_low == prev_low)
+	    {
+	      /* No backward progress made, bail out.  */
+	      next_addr = new_low;
+	      break;
+	    }
 
 	  /* Disassemble forward.  */
 	  next_addr = tui_disassemble (gdbarch, asm_lines, new_low, max_lines);
@@ -321,6 +341,13 @@ tui_find_disassembly_address (struct gdbarch *gdbarch, CORE_ADDR pc, int from)
 	 MAX_LINES entries.  */
       gdb_assert (asm_lines.size () == max_lines);
 
+      if (next_addr > pc)
+	{
+	  /* We're about to scan forward starting at next_addr to reach pc.
+	     No need to do that if next_addr is already past pc.  */
+	  return new_low;
+	}
+
       /* Scan forward disassembling one instruction at a time until
 	 the last visible instruction of the window matches the pc.
 	 We keep the disassembled instructions in the 'lines' window
-- 
2.51.0