Re: [PATCHv6] gdb: fix handling of DW_AT_entry_pc of inlined subroutines

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

On 11/15/24 20:24, Andrew Burgess wrote:
> Tom de Vries <tdevries@suse.de> writes:
[...]
>> Regardless, LGTM.
>>
>> Approved-By: Tom de Vries <tdevries@suse.de>
> 
> I pushed the version of the patch below.
> 
> Thanks,
> Andrew
> 

Hmm Sorry Andrey, but I have noticed another difference in the patch
which unfortunately weakens it a bit.
I've overlooked it before, but my patch had a couple __builtin_expect
here in bar and main:

inline __attribute__((always_inline)) int
bar (int val)
{
  if (__builtin_expect(global == val, 0))
    return 1;
  foo (1);
  return 1;
}

int
main (void)
{
  if ((__builtin_expect(global, 0) && bar (1)) || bar (2))
    return 1;
  return 0;
}

but those are removed from your version, I have added them to make
the optimizer do some wrong decisions that what I wanted it to do
to get the bar function have an entry pc not at the first subrange.
Without them the test case still works in x86_64 but is weaker,
because the bar function has more subranges.

With the original test case x86_64 has this debug info:

 <2><6e>: Abbrev Number: 6 (DW_TAG_inlined_subroutine)
    <6f>   DW_AT_abstract_origin: <0xd2>
    <73>   DW_AT_entry_pc    : 0x1065
    <7b>   DW_AT_GNU_entry_view: 0
    <7c>   DW_AT_ranges      : 0xc

    0000000c 0000000000001048 (base address)
    00000015 0000000000001048 000000000000104e 
    00000018 0000000000001055 000000000000105f 
    0000001b 0000000000001065 000000000000106c 
    0000001e <End of list>

so three separate ranges, and the entry_pc is the
third one, while the test case now has only two subranges:

 <2><6e>: Abbrev Number: 6 (DW_TAG_inlined_subroutine)
    <6f>   DW_AT_abstract_origin: <0xd2>
    <73>   DW_AT_entry_pc    : 0x1050
    <7b>   DW_AT_GNU_entry_view: 0
    <7c>   DW_AT_ranges      : 0xc

    0000000c 0000000000001048 (base address)
    00000015 0000000000001048 000000000000104e 
    00000018 0000000000001050 000000000000105f 
    0000001b <End of list>

Yet the test case is still functional with x86_64.

But with powerpc64-unknown-linux-gcc-9 I have seen that the
the bar function does no longer have the entry_pc at the lowest
address:

This is the code from the current version of the test case:

0000000010000400 <.main>:
    10000400:   60 00 00 00     nop
    10000404:   81 22 81 68     lwz     r9,-32408(r2)
    10000408:   2c 09 00 00     cmpwi   r9,0
    1000040c:   41 82 00 38     beq     10000444 <.main+0x44>
    10000410:   81 22 81 68     lwz     r9,-32408(r2)
    10000414:   2c 09 00 01     cmpwi   r9,1
    10000418:   41 82 00 3c     beq     10000454 <.main+0x54>
    1000041c:   7c 08 02 a6     mflr    r0
    10000420:   38 60 00 01     li      r3,1
    10000424:   f8 01 00 10     std     r0,16(r1)
    10000428:   f8 21 ff 91     stdu    r1,-112(r1)
    1000042c:   48 00 02 35     bl      10000660 <.foo>

    <a6e>   DW_AT_abstract_origin: <0xad7>
    <a72>   DW_AT_entry_pc    : 0x10000410
    <a7a>   DW_AT_GNU_entry_view: 0
    <a7b>   DW_AT_ranges      : 0x40

    00000040 0000000010000410 000000001000041c 
    00000040 0000000010000420 0000000010000424 
    00000040 000000001000042c 0000000010000430 
    00000040 <End of list>

so this test cannot fail even before the fix.

While with the original version the code was:

0000000010000400 <.main>:
    10000400:   60 00 00 00     nop
    10000404:   81 22 81 68     lwz     r9,-32408(r2)
    10000408:   2c 09 00 00     cmpwi   r9,0
    1000040c:   60 00 00 00     nop
    10000410:   81 22 81 68     lwz     r9,-32408(r2)
    10000414:   40 82 00 34     bne     10000448 <.main+0x48>
    10000418:   2c 09 00 02     cmpwi   r9,2
    1000041c:   41 82 00 34     beq     10000450 <.main+0x50>
    10000420:   7c 08 02 a6     mflr    r0
    10000424:   38 60 00 01     li      r3,1
    10000428:   f8 01 00 10     std     r0,16(r1)
    1000042c:   f8 21 ff 91     stdu    r1,-112(r1)
    10000430:   48 00 02 21     bl      10000650 <.foo>

    10000448:   2c 09 00 01     cmpwi   r9,1
    1000044c:   40 82 ff d4     bne     10000420 <.main+0x20>
    10000450:   38 60 00 01     li      r3,1
    10000454:   4e 80 00 20     blr

 <2><a6d>: Abbrev Number: 6 (DW_TAG_inlined_subroutine)
    <a6e>   DW_AT_abstract_origin: <0xad7>
    <a72>   DW_AT_entry_pc    : 0x10000448
    <a7a>   DW_AT_GNU_entry_view: 0
    <a7b>   DW_AT_ranges      : 0x40

    00000040 000000001000040c 0000000010000414 
    00000040 0000000010000420 0000000010000420 (start == end)
    00000040 0000000010000424 0000000010000428 
    00000040 0000000010000430 0000000010000434 
    00000040 0000000010000448 0000000010000450 
    00000040 <End of list>

So the bar function has much more subranges and the entry_pc
should make the test case actually test the interesting case.

Therefore I would suggest to add the __builtin_expect again,
what do you think?


Thanks
Bernd.