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

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

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
...

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.

Thanks,
- Tom