Re: [PATCH] gdb: handle DW_AT_entry_pc pointing at an empty sub-range

[Andrew Burgess <aburgess@redhat.com>]

Bernd Edlinger <bernd.edlinger@hotmail.de> writes:

> Hmm, sorry, but I think this goes in the wrong direction.
>
> On 11/20/24 16:01, Andrew Burgess wrote:
>> The test gdb.cp/step-and-next-inline.exp creates a test binary called
>> step-and-next-inline-no-header.  This test includes a function
>> `tree_check` which is inlined 3 times.
>> 
>> When testing with some older versions of gcc (I've tried 8.4.0, 9.3.1)
>> we see the following DWARF representing one of the inline instances of
>> tree_check:
>> 
>>  <2><8d9>: Abbrev Number: 38 (DW_TAG_inlined_subroutine)
>>     <8da>   DW_AT_abstract_origin: <0x9ee>
>>     <8de>   DW_AT_entry_pc    : 0x401165
>>     <8e6>   DW_AT_GNU_entry_view: 0
>>     <8e7>   DW_AT_ranges      : 0x30
>>     <8eb>   DW_AT_call_file   : 1
>>     <8ec>   DW_AT_call_line   : 52
>>     <8ed>   DW_AT_call_column : 10
>>     <8ee>   DW_AT_sibling     : <0x92d>
>> 
>>  ...
>> 
>>  <1><9ee>: Abbrev Number: 46 (DW_TAG_subprogram)
>>     <9ef>   DW_AT_external    : 1
>>     <9ef>   DW_AT_name        : (indirect string, offset: 0xe8): tree_check
>>     <9f3>   DW_AT_decl_file   : 1
>>     <9f4>   DW_AT_decl_line   : 38
>>     <9f5>   DW_AT_decl_column : 1
>>     <9f6>   DW_AT_linkage_name: (indirect string, offset: 0x2f2): _Z10tree_checkP4treei
>>     <9fa>   DW_AT_type        : <0x9e8>
>>     <9fe>   DW_AT_inline      : 3       (declared as inline and inlined)
>>     <9ff>   DW_AT_sibling     : <0xa22>
>> 
>>  ...
>> 
>>  Contents of the .debug_ranges section:
>> 
>>     Offset   Begin    End
>>     ...
>>     00000030 0000000000401165 0000000000401165 (start == end)
>>     00000030 0000000000401169 0000000000401173
>>     00000030 0000000000401040 0000000000401045
>>     00000030 <End of list>
>>     ...
>> 
>> Notice that one of the sub-ranges of tree-check is empty, this is the
>> line marked 'start == end'.  As the end address is the first address
>> after the range, this range cover absolutely no code.
>> 
>> But notice too that the DW_AT_entry_pc for the inline instance points
>> at this empty range.
>> 
>> Further, notice that despite the ordering of the sub-ranges, the empty
>> range is actually in the middle of the region defined by the lowest
>> address to the highest address.  The ordering is not a problem, the
>> DWARF spec doesn't require that ranges be in any particular order.
>> 
>> However, this empty range is causing issues with GDB newly acquire
>> DW_AT_entry_pc support.
>> 
>> GDB already rejects, and has done for a long time, empty sub-ranges,
>> after all, the DWARF spec is clear that such a range covers no code.
>> 
>> The recent DW_AT_entry_pc patch also had GDB reject an entry-pc which
>> was outside of the low/high bounds of a block.
>> 
>> But in this case, the entry-pc value is within the bounds of a block,
>> it's just not within any useful sub-range.  As a consequence, GDB is
>> storing the entry-pc value, and making use of it, but when GDB stops,
>> and tries to work out which block the inferior is in, it fails to spot
>> that the inferior is within tree_check, and instead reports the
>> function into which tree_check was inlined.
>> 
>> I've tested with newer versions of gcc (12.2.0 and 14.2.0) and with
>> these versions gcc is still generating the empty sub-range, but now
>> this empty sub-range is no longer the entry point.  Here's the
>> corresponding ranges table from gcc 14.2.0:
>> 
>
> Yeah, maybe not in this test case, but that is not true in general,
> a quick check with gcc-15 shows that there still a number of such
> empty range table entries in the gdb executable itself.
>
> Note that ignoring these entry_pc values is completely wrong,
> and my patch series handles exactly these empty subranges, by not
> ignoring them in the dwarf reader, and the debug experience is
> completely normal when this happens.
>
> Furthermore, I think that having a break point at these PC values has
> some benefit, because it is the earliest point in time, when all
> the input parameter values of the inline function are available,
> and can be inspected by gdb.

I hear and understand your frustration.  I'm absolutely not ignoring the
work you've done.  But I see this as a journey of small steps to get
where you want to be.

I agree with you 100% that dealing better with the empty sub-ranges is
the right way to go, and indeed, I have just finished pulling the empty
sub-range work from your series, and I plan to post those patches
hopefully over the weekend, I'm just running final tests now.

But, this patch does more than just deal with the case of entry-pc
pointing at the empty sub-range.  I believe that having a sanity check
that the entry-pc is within any sub-range is the right thing for GDB to
do, regardless of what we ultimately end up doing with empty ranges.

I'm hoping I can convince you that fixing this first is not going to
prevent us merging your empty sub-range handling code next.

> I think now it is time to consider merging the rest of my
> patch.

My top priority between now and year end is to merge either your
patches, or equivalent functionality, into GDB.  But, as you can tell
from what I've posted, I think your series covers a lot of fixes which
should be broken into separate commits.

I think the work you have done is absolutely amazing, and makes huge
improvements to GDB's handling of optimised code, I just don't want to
rush things, but I'm certain we will get there.

Thanks,
Andrew