Re: [PATCH][gdb/build] Fix build with g++-4.8

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-09-27 05:29, Tom de Vries wrote:
> On 9/26/21 9:33 PM, Simon Marchi wrote:
>>> [ I considered just removing the initialization, with the idea that access
>>> should be guarded by has_pc_info, but I ran into one failure in the testsuite,
>>> for gdb.base/check-psymtab.exp due to add_partial_symbol using lowpc without
>>> checking has_pc_info. ]
>>
>> Does that mean you've found a bug in add_partial_symbol?
> 
> Good question, I'm not sure.
> 
> Basically, we're processing this DIE:
> ...
>  <1><134>: Abbrev Number: 5 (DW_TAG_subprogram)
>     <135>   DW_AT_name        : foo
>     <139>   DW_AT_decl_file   : 1
>     <13a>   DW_AT_decl_line   : 19
>     <13b>   DW_AT_prototyped  : 1
>     <13b>   DW_AT_type        : <0x12d>
>     <13f>   DW_AT_inline      : 3       (declared as inline and inlined)
> ...
> which indeed does not have low_pc/high_pc or ranges, so has_pc_info
> remains false, and lowpc remains set to the initialization value of 0,
> end we end up with foo at addresss 0:
> ...
> (gdb) maint print psymbols^M
>   ...
>   Global partial symbols:^M
>     `main', function, 0x4004a7^M
>   Static partial symbols:^M
>     `int', type, 0x0^M
>     `foo', function, 0x0^M
> ...

That looks a little odd to pretend this function has code at address
0.  But if that doesn't have direct unwanted consequences, ok.

> There actually is an entry:
> ...
>  <2><115>: Abbrev Number: 3 (DW_TAG_inlined_subroutine)
>     <116>   DW_AT_abstract_origin: <0x134>
>     <11a>   DW_AT_low_pc      : 0x4004ab
>     <122>   DW_AT_high_pc     : 0x5
>     <12a>   DW_AT_call_file   : 1
>     <12b>   DW_AT_call_line   : 27
> ...
> with low_pc/high_pc, but that one is ignored because of being a child of
> DW_TAG_subprogram main rather than a top-level DIE.
> 
> So, does foo having address 0 cause problems?  I suspect not.  It's
> inlined into main, so the address range is covered there.
> 
> Then, is it a bug to access pdi->low_pc without has_pc_info == true?  If
> so, it's fixed by doing:
> ...
>      case DW_TAG_subprogram:
> -      addr = ... pdi->lowpc ... ;
> +      if (pdi->has_pc_info)
> +        addr = ... pdi->lowpc ... ;
> ...
> with as only effect that the initialization could be removed.
> 
> I think addressing this in a more consistent way is to add accessor
> functions that can assert when used incorrectly, and perhaps adding an
> enum { none, high_low, high_low_non_contiguous, range_offset } to more
> precisely encode what kind of information we have.

Yes, that would make sense.

Simon