[PATCH v2 0/7] [gdb/tui] Some section hole handling fixes

[PATCH v2 0/7] [gdb/tui] Some section hole handling fixes

[Tom de Vries]

This series consists of 7 patches.

The first two fix minor issues in tui disassembly handling.

The rest of the patches:
- fix a corner case in tui_find_backward_disassembly_start_address.
- refactor find_pc_section.
- add prev/next parameters in find_pc_section.
- improve handling of section holes when forward disassembling.
- improve handling of section holes when backward disassembling, fixing
  PR tui/34399.

Changes in v2:
- add patch fixing tui_disassemble comment after review comment
- drop restrictive fix for PR tui/34399 after review comments
- add patches implementing an enabling fix for PR tui/34399.

Versions:
- v1 https://sourceware.org/pipermail/gdb-patches/2026-August/229584.html

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34399

Tom de Vries (7):
  [gdb/tui] Improve tui_disassemble comment
  [gdb/tui] Fix underflow in tui_find_backward_disassembly_start_address
  [gdb/tui] Improve section handling in
    tui_find_backward_disassembly_start_address
  [gdb] replace bsearch with std::lower_bound in find_pc_section
  [gdb] Add prev/next params to find_pc_section
  [gdb/tui] Handle section holes when forward disassembling
  [gdb/tui] Handle section holes when backward disassembling

 gdb/objfiles.c                                |  72 +++++++-----
 gdb/objfiles.h                                |   4 +-
 .../gdb.tui/tui-layout-asm-short-prog.exp     |  25 +++++
 gdb/tui/tui-disasm.c                          | 105 ++++++++++++++----
 4 files changed, 161 insertions(+), 45 deletions(-)


base-commit: 48ad7db89036813e93f6669e424617a3a8860988
-- 
2.51.0

[PATCH v2 1/7] [gdb/tui] Improve tui_disassemble comment

[Tom de Vries]

The tui_disassemble comment states:
...
/* 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 of the COUNT'th instruction after pc.
...

In fact, the address after the last disassembled instruction is returned.

In case less than COUNT instructions are disassembled, the comment is
incorrect.

Fix the comment to use "address after last disassembled instruction".

Suggested-By: Guinevere Larsen <guinevere@redhat.com>
---
 gdb/tui/tui-disasm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 68a5c425934..923d4373e8f 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -82,7 +82,7 @@ len_without_escapes (const std::string &str)
 
 /* 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 of the COUNT'th instruction after pc.
+   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
-- 
2.51.0

[PATCH v2 2/7] [gdb/tui] Fix underflow in tui_find_backward_disassembly_start_address

[Tom de Vries]

In the tui-disasm.c selftests, we do:
...
      SELF_CHECK (tui_find_disassembly_address (gdbarch, 0, -1) == 0);
...

This causes tui_find_backward_disassembly_start_address to be called with
addr == 0, causing underflow in:
...
  bound_minimal_symbol msym
    = lookup_minimal_symbol_by_pc_section (addr - 1, nullptr,
                                           lookup_msym_prefer::TEXT,
                                           &msym_prev);
...

Fix this by handling addr == 0 in tui_find_backward_disassembly_start_address.
---
 gdb/tui/tui-disasm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 923d4373e8f..7dcb59e6264 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -161,6 +161,12 @@ tui_disassemble (struct gdbarch *gdbarch,
 static CORE_ADDR
 tui_find_backward_disassembly_start_address (CORE_ADDR addr)
 {
+  if (addr == 0)
+    {
+      /* We cannot go backwards from zero.  */
+      return addr;
+    }
+
   bound_minimal_symbol msym_prev;
   bound_minimal_symbol msym
     = lookup_minimal_symbol_by_pc_section (addr - 1, nullptr,
-- 
2.51.0

Re: [PATCH v2 2/7] [gdb/tui] Fix underflow in tui_find_backward_disassembly_start_address

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> In the tui-disasm.c selftests, we do:
Tom> ...
Tom>       SELF_CHECK (tui_find_disassembly_address (gdbarch, 0, -1) == 0);
Tom> ...

This test seems nonsensical to me.

Tom> +  if (addr == 0)
Tom> +    {
Tom> +      /* We cannot go backwards from zero.  */
Tom> +      return addr;
Tom> +    }

Like even if there is a section at 0, would there ever be code at 0?

Tom

Re: [PATCH v2 2/7] [gdb/tui] Fix underflow in tui_find_backward_disassembly_start_address

[Tom Tromey]

Tom> Like even if there is a section at 0, would there ever be code at 0?

I just totally spaced, and this probably does happen.

I think this patch is ok.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

[PATCH v2 3/7] [gdb/tui] Improve section handling in tui_find_backward_disassembly_start_address

[Tom de Vries]

Consider a hello world:
...
$ gcc hello.c -g
...
and gdb setup like this:
...
$ gdb a.out -ex "layout asm" -ex "disassemble _start"
...
which gives us the following asm window:
...
┌─────────────────────────────────────────────────────────────────────┐
│   0x401040 <_start>       endbr64                                   │
│   0x401044 <_start+4>     xor    %ebp,%ebp                          │
│   0x401046 <_start+6>     mov    %rdx,%r9                           │
│   0x401049 <_start+9>     pop    %rsi                               │
│   0x40104a <_start+10>    mov    %rsp,%rdx                          │
│   0x40104d <_start+13>    and    $0xfffffffffffffff0,%rsp           │
│   0x401051 <_start+17>    push   %rax                               │
│   0x401052 <_start+18>    push   %rsp                               │
│   0x401053 <_start+19>    xor    %r8d,%r8d                          │
│   0x401056 <_start+22>    xor    %ecx,%ecx                          │
└─────────────────────────────────────────────────────────────────────┘
...

The _start function sits at the start of the text section:
...
Disassembly of section .plt:

   ...

0000000000401030 <puts@plt>:
  401030:       ff 25 ca 2f 00 00       jmp    *0x2fca(%rip)
  401036:       68 00 00 00 00          push   $0x0
  40103b:       e9 e0 ff ff ff          jmp    401020 <_init+0x20>

Disassembly of section .text:

0000000000401040 <_start>:
...
and using <Up Arrow> we can scroll to the last instruction of the plt section,
at 0x40103b.

To make this happen:
- tui_find_backward_disassembly_start_address is called with addr == 0x401040
- lookup_minimal_symbol_by_pc_section is called with addr - 1
- minimal symbol puts@plt is found
- its address is returned and used as starting point for disassembly

However, tui_find_backward_disassembly_start_address has two types of
information it uses: minimal symbols, and sections.

If we disable the minimal symbol handling in
tui_find_backward_disassembly_start_address, we're no longer able to scroll to
the instruction at 0x40103b.

The section handling code:
....
   /* Find the section that ADDR is in, and look for the start of the
      section.  */
   struct obj_section *section = find_pc_section (addr);
   if (section != NULL)
     return section->addr ();
...
looks up the section for 0x401040, which is .text, and it returns its start
address, which is also 0x401040, signalling that we cannot scroll to before
that address.

Fix this by detecting this case and using the previous section instead:
...
  if (section != nullptr && section->addr () == addr)
    section = find_pc_section (addr - 1);
...

This happens to work because there's no section hole between sections plt and
text.
---
 gdb/tui/tui-disasm.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 7dcb59e6264..3e525000efd 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -177,9 +177,22 @@ tui_find_backward_disassembly_start_address (CORE_ADDR addr)
   else if (msym_prev.minsym != nullptr)
     return msym_prev.value_address ();
 
-  /* Find the section that ADDR is in, and look for the start of the
-     section.  */
-  struct obj_section *section = find_pc_section (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;
+  for (int offset = 0; offset <= 1; ++offset)
+    {
+      section = find_pc_section (addr - offset);
+      if (offset == 0 && section != nullptr && section->addr () == addr)
+	{
+	  /* If ADDR is the start of its section, use ADDR - 1.  */
+	  continue;
+	}
+
+      break;
+    }
+
   if (section != NULL)
     return section->addr ();
 
-- 
2.51.0

Re: [PATCH v2 3/7] [gdb/tui] Improve section handling in tui_find_backward_disassembly_start_address

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> -  /* Find the section that ADDR is in, and look for the start of the
Tom> -     section.  */
Tom> -  struct obj_section *section = find_pc_section (addr);
Tom> +  /* Find the first section with start address before ADDR, and use its start
Tom> +     address.  The found section may be the one containing ADDR, or the one
Tom> +     before it.  */
Tom> +  struct obj_section *section;
Tom> +  for (int offset = 0; offset <= 1; ++offset)
Tom> +    {
Tom> +      section = find_pc_section (addr - offset);
Tom> +      if (offset == 0 && section != nullptr && section->addr () == addr)
Tom> +	{
Tom> +	  /* If ADDR is the start of its section, use ADDR - 1.  */
Tom> +	  continue;
Tom> +	}
Tom> +
Tom> +      break;
Tom> +    }

I was puzzling about this.  It seems like it would be ok to always check
"addr-1" here?  Since if addr is not at the start of the section, it
will return that same section.  And if addr is at the start of the
section, then that's the answer you want?

Tom

Re: [PATCH v2 3/7] [gdb/tui] Improve section handling in tui_find_backward_disassembly_start_address

[Tom de Vries]

On 9/11/26 8:26 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> -  /* Find the section that ADDR is in, and look for the start of the
> Tom> -     section.  */
> Tom> -  struct obj_section *section = find_pc_section (addr);
> Tom> +  /* Find the first section with start address before ADDR, and use its start
> Tom> +     address.  The found section may be the one containing ADDR, or the one
> Tom> +     before it.  */
> Tom> +  struct obj_section *section;
> Tom> +  for (int offset = 0; offset <= 1; ++offset)
> Tom> +    {
> Tom> +      section = find_pc_section (addr - offset);
> Tom> +      if (offset == 0 && section != nullptr && section->addr () == addr)
> Tom> +	{
> Tom> +	  /* If ADDR is the start of its section, use ADDR - 1.  */
> Tom> +	  continue;
> Tom> +	}
> Tom> +
> Tom> +      break;
> Tom> +    }
> 
> I was puzzling about this.  It seems like it would be ok to always check
> "addr-1" here?  Since if addr is not at the start of the section, it
> will return that same section.  And if addr is at the start of the
> section, then that's the answer you want?
> 

Thanks for that observation, that greatly simplifies things.

Thanks,
- Tom

> Tom

[PATCH v2 4/7] [gdb] replace bsearch with std::lower_bound in find_pc_section

[Tom de Vries]

In find_pc_section, we use bsearch to find a matching section.  This works
fine, but in case the pc matches a section hole, it returns nullptr, with no
information about where the section hole is.

Instead, use std::lower_bound.

No functional changes.
---
 gdb/objfiles.c | 40 +++++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 408ddf1ff83..066d1af9b9e 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -912,28 +912,13 @@ update_section_map (struct program_space *pspace,
   *pmap_size = map_size;
 }
 
-/* Bsearch comparison function.  */
-
-static int
-bsearch_cmp (const void *key, const void *elt)
-{
-  const CORE_ADDR pc = *(CORE_ADDR *) key;
-  const struct obj_section *section = *(const struct obj_section **) elt;
-
-  if (pc < section->addr ())
-    return -1;
-  if (pc < section->endaddr ())
-    return 0;
-  return 1;
-}
-
 /* Returns a section whose range includes PC or NULL if none found.   */
 
 struct obj_section *
 find_pc_section (CORE_ADDR pc)
 {
   struct objfile_pspace_info *pspace_info;
-  struct obj_section *s, **sp;
+  struct obj_section *s;
 
   /* Check for mapped overlay section first.  */
   s = find_pc_mapped_section (pc);
@@ -963,13 +948,22 @@ find_pc_section (CORE_ADDR pc)
       return NULL;
     }
 
-  sp = (struct obj_section **) bsearch (&pc,
-					pspace_info->sections,
-					pspace_info->num_sections,
-					sizeof (*pspace_info->sections),
-					bsearch_cmp);
-  if (sp != NULL)
-    return *sp;
+  gdb::array_view<obj_section *> data
+    = gdb::make_array_view (pspace_info->sections, pspace_info->num_sections);
+  auto section_before_pc
+    = [] (obj_section *sect, CORE_ADDR pc_) -> bool
+      {
+	/* Return true if SECT is ordered before PC.  */
+	return sect->endaddr () <= pc_;
+      };
+  auto it
+    = std::lower_bound (data.cbegin (), data.cend (), pc, section_before_pc);
+  /* The std::lower_bound call returns the first section which is not ordered
+     before pc.  That section may or may not contain pc.  */
+  if (it != data.cend ()
+      && (*it)->addr () <= pc && pc < (*it)->endaddr ())
+    return *it;
+
   return NULL;
 }
 
-- 
2.51.0

Re: [PATCH v2 4/7] [gdb] replace bsearch with std::lower_bound in find_pc_section

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> In find_pc_section, we use bsearch to find a matching section.  This works
Tom> fine, but in case the pc matches a section hole, it returns nullptr, with no
Tom> information about where the section hole is.

Tom> Instead, use std::lower_bound.

Tom> +  /* The std::lower_bound call returns the first section which is not ordered
Tom> +     before pc.  That section may or may not contain pc.  */
Tom> +  if (it != data.cend ()
Tom> +      && (*it)->addr () <= pc && pc < (*it)->endaddr ())

Could this use (*it)->contains?

Tom

Re: [PATCH v2 4/7] [gdb] replace bsearch with std::lower_bound in find_pc_section

[Tom de Vries]

On 9/11/26 8:19 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> In find_pc_section, we use bsearch to find a matching section.  This works
> Tom> fine, but in case the pc matches a section hole, it returns nullptr, with no
> Tom> information about where the section hole is.
> 
> Tom> Instead, use std::lower_bound.
> 
> Tom> +  /* The std::lower_bound call returns the first section which is not ordered
> Tom> +     before pc.  That section may or may not contain pc.  */
> Tom> +  if (it != data.cend ()
> Tom> +      && (*it)->addr () <= pc && pc < (*it)->endaddr ())
> 
> Could this use (*it)->contains?

Yes, that works.

Thanks,
- Tom

> 
> Tom

[PATCH v2 5/7] [gdb] Add prev/next params to find_pc_section

[Tom de Vries]

Now that we're using std::lower_bound to find sections in find_pc_section,
extend find_pc_section to return the previous and next section in case it
returns nullptr.
---
 gdb/objfiles.c | 32 ++++++++++++++++++++++++++++++--
 gdb/objfiles.h |  4 +++-
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 066d1af9b9e..3871ef1670f 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -912,14 +912,22 @@ update_section_map (struct program_space *pspace,
   *pmap_size = map_size;
 }
 
-/* Returns a section whose range includes PC or NULL if none found.   */
+/* Returns a section whose range includes PC or nullptr if none found.
+   On returning nullptr, set PREV_PTR and NEXT_PTR to point to the sections
+   before and after PC, if any.  */
 
 struct obj_section *
-find_pc_section (CORE_ADDR pc)
+find_pc_section (CORE_ADDR pc, struct obj_section **prev_ptr,
+		 struct obj_section **next_ptr)
 {
   struct objfile_pspace_info *pspace_info;
   struct obj_section *s;
 
+  if (prev_ptr != nullptr)
+    *prev_ptr = nullptr;
+  if (next_ptr != nullptr)
+    *next_ptr = nullptr;
+
   /* Check for mapped overlay section first.  */
   s = find_pc_mapped_section (pc);
   if (s)
@@ -964,6 +972,26 @@ find_pc_section (CORE_ADDR pc)
       && (*it)->addr () <= pc && pc < (*it)->endaddr ())
     return *it;
 
+  struct obj_section *prev = nullptr, *next = nullptr;
+  auto prev_it = (it == data.cbegin ()
+		  ? data.cend ()
+		  : it - 1);
+  if (prev_it != data.cend ())
+    {
+      prev = *prev_it;
+      gdb_assert (prev->endaddr () <= pc);
+    }
+  if (it != data.cend ())
+    {
+      next = *it;
+      gdb_assert (pc < next->addr ());
+    }
+
+  if (prev_ptr != nullptr)
+    *prev_ptr = prev;
+  if (next_ptr != nullptr)
+    *next_ptr = next;
+
   return NULL;
 }
 
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index fa265f83b79..d23e2b3046e 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -954,7 +954,9 @@ extern void objfile_purge_solibs (program_space *pspace);
 /* Functions for dealing with the minimal symbol table, really a misc
    address<->symbol mapping for things we don't have debug symbols for.  */
 
-extern struct obj_section *find_pc_section (CORE_ADDR pc);
+extern struct obj_section *find_pc_section (CORE_ADDR pc,
+					    struct obj_section **prev = nullptr,
+					    struct obj_section **next = nullptr);
 
 /* Return true if PC is in a section called NAME.  */
 extern bool pc_in_section (CORE_ADDR, const char *);
-- 
2.51.0

Re: [PATCH v2 5/7] [gdb] Add prev/next params to find_pc_section

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Now that we're using std::lower_bound to find sections in find_pc_section,
Tom> extend find_pc_section to return the previous and next section in case it
Tom> returns nullptr.

See the comments on patch 6.
You may want some other API here, not find_pc_section.

Tom

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

[Tom de Vries]

Consider a hello world:
...
$ gcc hello.c -g
...
and gdb setup like this:
...
$ gdb -q a.out -ex "layout asm" -ex "disassemble _init"
...
which gives us the following asm window:
...
┌─────────────────────────────────────────────────────────────────────┐
│   0x401000 <_init>        endbr64                                   │
│   0x401004 <_init+4>      sub    $0x8,%rsp                          │
│   0x401008 <_init+8>      mov    0x2fd1(%rip),%rax        # 0x403fe0│
│   0x40100f <_init+15>     test   %rax,%rax                          │
│   0x401012 <_init+18>     je     0x401016 <_init+22>                │
│   0x401014 <_init+20>     call   *%rax                              │
│   0x401016 <_init+22>     add    $0x8,%rsp                          │
│   0x40101a <_init+26>     ret                                       │
│                                                                     │
│                                                                     │
└─────────────────────────────────────────────────────────────────────┘
...

We cannot scroll from the init section to the plt section, because there's a
section hole in between:
...
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)
...
and the section hole is causing MEMORY_ERRORs in tui_disassemble.

However, that all changes if instead we do:
...
$ gdb -q a.out -ex start -ex "layout asm" -ex "disassemble _init"
...
and get:
...
┌─────────────────────────────────────────────────────────────────────┐
│   0x401000 <_init>        endbr64                                   │
│   0x401004 <_init+4>      sub    $0x8,%rsp                          │
│   0x401008 <_init+8>      mov    0x2fd1(%rip),%rax        # 0x403fe0│
│   0x40100f <_init+15>     test   %rax,%rax                          │
│   0x401012 <_init+18>     je     0x401016 <_init+22>                │
│   0x401014 <_init+20>     call   *%rax                              │
│   0x401016 <_init+22>     add    $0x8,%rsp                          │
│   0x40101a <_init+26>     ret                                       │
│   0x40101b                add    %al,(%rax)                         │
│   0x40101d                add    %al,(%rax)                         │
└─────────────────────────────────────────────────────────────────────┘
...

The section hole is no longer causing MEMORY_ERRORs, and we can scroll past it
into the plt section.

Achieve something similar in the initial setup (without start) by skipping
over the section hole, getting us:
...
┌─────────────────────────────────────────────────────────────────────┐
│   0x401000 <_init>        endbr64                                   │
│   0x401004 <_init+4>      sub    $0x8,%rsp                          │
│   0x401008 <_init+8>      mov    0x2fd1(%rip),%rax        # 0x403fe0│
│   0x40100f <_init+15>     test   %rax,%rax                          │
│   0x401012 <_init+18>     je     0x401016 <_init+22>                │
│   0x401014 <_init+20>     call   *%rax                              │
│   0x401016 <_init+22>     add    $0x8,%rsp                          │
│   0x40101a <_init+26>     ret                                       │
│   0x401020                push   0x2fca(%rip)        # 0x403ff0     │
│   0x401026                jmp    *0x2fcc(%rip)        # 0x403ff8    │
└─────────────────────────────────────────────────────────────────────┘
...
allowing to scroll (further) into the plt section.
---
 gdb/tui/tui-disasm.c | 51 +++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 15 deletions(-)

diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 3e525000efd..d96048dc1de 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -112,22 +112,43 @@ tui_disassemble (struct gdbarch *gdbarch,
     {
       tui_asm_line tal;
 
-      /* Save the instruction address.  */
-      tal.addr = pc;
-
-      try
-	{
-	  pc += gdb_print_insn (gdbarch, pc, stream, NULL);
-	}
-      catch (const gdb_exception_error &except)
+      bool retry;
+      do
 	{
-	  /* If PC points to an invalid address then we'll catch a
-	     MEMORY_ERROR here, this should stop the disassembly, but
-	     otherwise is fine.  */
-	  if (except.error != MEMORY_ERROR)
-	    throw;
-	  return pc;
+	  retry = false;
+
+	  /* Save the instruction address.  */
+	  tal.addr = pc;
+
+	  try
+	    {
+	      pc += gdb_print_insn (gdbarch, pc, stream, NULL);
+	    }
+	  catch (const gdb_exception_error &except)
+	    {
+	      /* If PC points to an invalid address then we'll catch a
+		 MEMORY_ERROR here, this should stop the disassembly, but
+		 otherwise is fine.  */
+	      if (except.error != MEMORY_ERROR)
+		throw;
+
+	      struct obj_section *next;
+	      struct obj_section *section
+		= find_pc_section (pc, nullptr, &next);
+	      if (section == nullptr
+		  && next != nullptr
+		  && ((bfd_section_flags (next->the_bfd_section) & SEC_ALLOC)
+		      != 0))
+		{
+		  /* Skip over section hole.  */
+		  pc = next->addr ();
+		  retry = true;
+		}
+	      else
+		return pc;
+	    }
 	}
+      while (retry);
 
       /* If that's all we need, continue.  */
       if (addr_size == nullptr)
@@ -563,7 +584,7 @@ run_tests ()
 
       /* Check that tui_find_disassembly_address robustly handles the case of
 	 being passed a PC for which gdb_print_insn throws a MEMORY_ERROR.  */
-      SELF_CHECK (tui_find_disassembly_address (gdbarch, 0, 1) == 0);
+      tui_find_disassembly_address (gdbarch, 0, 1);
       SELF_CHECK (tui_find_disassembly_address (gdbarch, 0, -1) == 0);
     }
 }
-- 
2.51.0

Re: [PATCH v2 6/7] [gdb/tui] Handle section holes when forward disassembling

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> +	      struct obj_section *next;
Tom> +	      struct obj_section *section
Tom> +		= find_pc_section (pc, nullptr, &next);
Tom> +	      if (section == nullptr
Tom> +		  && next != nullptr
Tom> +		  && ((bfd_section_flags (next->the_bfd_section) & SEC_ALLOC)

I'm not sure this is the best test.  SEC_ALLOC can be set for data
sections as well; e.g. from 'objdump -h' on gdb:

 13 .rodata       00dc0acc  0000000001064d80  0000000001064d80  00c64d80  2**7
                  CONTENTS, ALLOC, LOAD, READONLY, DATA

Probably you want SEC_CODE or maybe SEC_ALLOC|SEC_CODE, though I don't
know if you can ever have code that isn't also 'alloc' (like, what would
be the point).

Then there's the issue that presumably you want to find the next code
section -- but if code and data are interleaved, find_pc_section won't
necessarily return this.

Tom

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

[Tom de Vries]

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

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

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

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

Tom> Fix this by using the prev section returned by find_pc_section.

Tom> +  if (prev != nullptr
Tom> +      && (bfd_section_flags (prev->the_bfd_section) & SEC_ALLOC) != 0)
Tom> +    {
Tom> +      /* Skip over section hole and use previous section.  */

I suspect this has the same problem as patch 6, where maybe SEC_CODE is
more desirable.

Tom

Re: [PATCH v2 0/7] [gdb/tui] Some section hole handling fixes

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> The rest of the patches:
Tom> - fix a corner case in tui_find_backward_disassembly_start_address.
Tom> - refactor find_pc_section.
Tom> - add prev/next parameters in find_pc_section.
Tom> - improve handling of section holes when forward disassembling.
Tom> - improve handling of section holes when backward disassembling, fixing
Tom>   PR tui/34399.

Thanks for doing this.

I'm hopeful that once this lands, maybe we can generalize it and/or
reuse parts to implement the DAP negative offsets feature:

https://sourceware.org/bugzilla/show_bug.cgi?id=31139

Tom