Re: [PATCH v2 4/5] [gdb/symtab] Improve invalid range check in create_addrmap_from_gdb_index

[Simon Marchi <simark@simark.ca>]

On 8/21/25 9:31 AM, Tom de Vries wrote:
> When running test-case gdb.tui/tui-missing-src.exp with target board
> gold-gdb-index (and likewise fission and fission-dwp) on aarch64-linux, I run
> into:
> ...
> FAIL: gdb.tui/tui-missing-src.exp: checking if inside f2 ()
> ...
> 
> Looking at the gold-gdb-index case, the problem is caused by the address table
> of the .gdb_index section:
> ...
> Address table:
> 000000000040066c 0000000000400694 0
> 000000000040053f 0000000000400563 1
> ...
> 
> The address range for f2 is [0x400694, 0x4006b8), but the address table says
> it's [0x40053f, 0x400563).
> 
> The address 0x40053f is not even in a section:
> ...
>   [Nr] Name    Type            Address          Off    Size   ES Flg Lk Inf Al
>   ...
>   [12] .plt    PROGBITS        00000000004004b8 0004b8 000050 10  AX  0   0  8
>   [13] .text   PROGBITS        0000000000400540 000540 000178 00  AX  0   0 64
> ...
> but part of the hole [0x400508, 0x400540) in between .plt and .text.
> 
> Detect this in the invalid range check in create_addrmap_from_gdb_index.
> 
> Tested on aarch64-linux.
> ---
>  gdb/dwarf2/read-gdb-index.c | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
> index 79d19a3abaa..df20b20e081 100644
> --- a/gdb/dwarf2/read-gdb-index.c
> +++ b/gdb/dwarf2/read-gdb-index.c
> @@ -1420,14 +1420,33 @@ create_addrmap_from_gdb_index (dwarf2_per_objfile *per_objfile,
>        cu_index = extract_unsigned_integer (iter, 4, BFD_ENDIAN_LITTLE);
>        iter += 4;
>  
> -      if (lo >= hi)
> +      bool valid_range_p = lo < hi;
> +      bool valid_index_p = cu_index < index->units.size ();

For readability, I would prefer kepeing each check independent.  There
will be less conditionals below.  To avoid duplicating the warning, you
can use a lambda or free function:

  if (lo >= hi)
    return invalid_range ();

with, for instance:

  auto invalid_range = [&] ()
    {
      complaint (_(".gdb_index address table has invalid range (%s - %s)"),
		 hex_string (lo), hex_string (hi));
      return false;
    };

> +
> +      /* Variable hi is the exclusive upper bound, get the inclusive one.  */
> +      CORE_ADDR hi_m1 = (valid_range_p
> +			 ? hi - 1
> +			 : 0);
> +
> +      if (valid_range_p)
> +	{
> +	  CORE_ADDR relocated_lo
> +	    = per_objfile->relocate (unrelocated_addr (lo));
> +	  CORE_ADDR relocated_hi_m1
> +	    = per_objfile->relocate (unrelocated_addr (hi_m1));
> +	  struct obj_section *lo_sect = find_pc_section (relocated_lo);
> +	  struct obj_section *hi_sect = find_pc_section (relocated_hi_m1);
> +	  valid_range_p = lo_sect != nullptr && hi_sect != nullptr;

It only concerns MIPS, so perhaps not super important (I guess MIPS
programs tend to be not too big), but per_objfile->relocate can be
somewhat heavy, as mips_adjust_dwarf2_addr does one minimal symbol
lookup for each address.  It might not be a problem in practice, but it
would seem much more logical to me if this work was done completely in
the unrelocated domain.  It's an analysis that can be done with the
object file in isolation, independent of whether or where it's loaded in
the inferior.

Simon