[PATCH] gdb: change type of `general_symbol_info::m_section` to int

[Tom de Vries]

On 9/12/24 14:27, Simon Marchi wrote:
> The binary provided with bug 32165 [1] has 36139 ELF sections.  GDB
> crashes on it with (note that my GDB is build with -D_GLIBCXX_DEBUG=1:
> 
>      $ ./gdb  -nx -q --data-directory=data-directory ./vmlinux
>      Reading symbols from ./vmlinux...
>      (No debugging symbols found in ./vmlinux)
>      (gdb) info func
>      /usr/include/c++/14.2.1/debug/vector:508:
>      In function:
>          std::debug::vector<_Tp, _Allocator>::reference std::debug::vector<_Tp,
>          _Allocator>::operator[](size_type) [with _Tp = long unsigned int;
>          _Allocator = std::allocator<long unsigned int>; reference = long
>          unsigned int&; size_type = long unsigned int]
> 
>      Error: attempt to subscript container with out-of-bounds index -29445, but
>      container only holds 36110 elements.
> 
>      Objects involved in the operation:
>          sequence "this" @ 0x514000007340 {
>            type = std::debug::vector<unsigned long, std::allocator<unsigned long> >;
>          }
> 
> The crash occurs here:
> 
>      #3  0x00007ffff5e334c3 in __GI_abort () at abort.c:79
>      #4  0x00007ffff689afc4 in __gnu_debug::_Error_formatter::_M_error (this=<optimized out>) at /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/debug.cc:1320
>      #5  0x0000555561119a16 in std::__debug::vector<unsigned long, std::allocator<unsigned long> >::operator[] (this=0x514000007340, __n=18446744073709522171)
>          at /usr/include/c++/14.2.1/debug/vector:508
>      #6  0x0000555562e288e8 in minimal_symbol::value_address (this=0x5190000bb698, objfile=0x514000007240) at /home/smarchi/src/binutils-gdb/gdb/symtab.c:517
>      #7  0x0000555562e5a131 in global_symbol_searcher::expand_symtabs (this=0x7ffff0f5c340, objfile=0x514000007240, preg=std::optional [no contained value])
>          at /home/smarchi/src/binutils-gdb/gdb/symtab.c:4983
>      #8  0x0000555562e5d2ed in global_symbol_searcher::search (this=0x7ffff0f5c340) at /home/smarchi/src/binutils-gdb/gdb/symtab.c:5189
>      #9  0x0000555562e5ffa4 in symtab_symbol_info (quiet=false, exclude_minsyms=false, regexp=0x0, kind=FUNCTION_DOMAIN, t_regexp=0x0, from_tty=1)
>          at /home/smarchi/src/binutils-gdb/gdb/symtab.c:5361
>      #10 0x0000555562e6131b in info_functions_command (args=0x0, from_tty=1) at /home/smarchi/src/binutils-gdb/gdb/symtab.c:5525
> 
> That is, at this line of `minimal_symbol::value_address`, where
> `objfile->section_offsets` is an `std::vector`:
> 
>      return (CORE_ADDR (this->unrelocated_address ())
> 	    + objfile->section_offsets[this->section_index ()]);
> 
> A section index of -29445 is suspicious.  The minimal_symbol at play
> here is:
> 
>      (top-gdb) p m_name
>      $1 = 0x521001de10af "_sinittext"
> 
> So I restarted debugging, breaking on:
> 
>     (top-gdb) b general_symbol_info::set_section_index if $_streq("_sinittext", m_name)
> 
> And I see that weird -29445 value:
> 
>      (top-gdb) frame
>      #0  general_symbol_info::set_section_index (this=0x525000082390, idx=-29445) at /home/smarchi/src/binutils-gdb/gdb/symtab.h:611
>      611       { m_section = idx; }
> 
> But going up one frame, the section index is 36091:
> 
>      (top-gdb) frame
>      #1  0x0000555562426526 in minimal_symbol_reader::record_full (this=0x7ffff0ead560, name="_sinittext", copy_name=false,
>          address=-2111475712, ms_type=mst_text, section=36091) at /home/smarchi/src/binutils-gdb/gdb/minsyms.c:1228
>      1228      msymbol->set_section_index (section);
> 
> It seems like the problem is just that the type used for the section
> index (short) is not big enough.  Change from short to int.  If somebody
> insists, we could even go long long / int64_t, but I doubt it's
> necessary.
> 
> With that fixed, I get:
> 
>      (gdb) info func
>      All defined functions:
> 
>      Non-debugging symbols:
>      0xffffffff81000000  _stext
>      0xffffffff82257000  _sinittext
>      0xffffffff822b4ebb  _einittext
> 

Hi Simon,

LGTM.

I wonder if using auto instead of int in set_section_index and 
section_index would be cleaner.

Reviewed-By: Tom de Vries <tdevries@suse.de>

Thanks,
- Tom

> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=32165
> 
> Change-Id: Icb1c3de9474ff5adef7e0bbbf5e0b67b279dee04
> ---
>   gdb/symtab.h | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index d615fdf1e52b..a273f32ddd0d 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -601,20 +601,20 @@ struct general_symbol_info
>        section_offsets for this objfile.  Negative means that the symbol
>        does not get relocated relative to a section.  */
>   
> -  short m_section;
> +  int m_section;
>   
>     /* Set the index into the obj_section list (within the containing
>        objfile) for the section that contains this symbol.  See M_SECTION
>        for more details.  */
>   
> -  void set_section_index (short idx)
> +  void set_section_index (int idx)
>     { m_section = idx; }
>   
>     /* Return the index into the obj_section list (within the containing
>        objfile) for the section that contains this symbol.  See M_SECTION
>        for more details.  */
>   
> -  short section_index () const
> +  int section_index () const
>     { return m_section; }
>   
>     /* Return the obj_section from OBJFILE for this symbol.  The symbol
> 
> base-commit: 4290b2c07e2d9bc1e1661a4ad5e343e3eb307770