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:

> On 11/22/24 17:53, Andrew Burgess wrote:
>> 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
>> 
>
> Okay, I just wanted to point out that in my opinion the debug info which
> points at the end of a sub-range is not incorrect, just maybe on a border
> line, where the dwarf spec is unclear.  So you should not say:
> "after all, the DWARF spec is clear that such a range covers no code."
>
> But there are obviously not only cases where the entry_pc points at
> an empty sub-range, but also in very rare cases the entry_pc points at
> the end of a non-empty sub-range.
> So could you please change the check in dwarf2_addr_in_block_ranges
> from addr >= start && addr < end to addr >= start && addr <= end.

Could you expand on why you believe that the DWARF spec is unclear in
this regard.  I came to my conclusion based on this text within the
DWARF-5 specification, section 2.17.3 Non-Contiguous Address Ranges:

  Bounded range. This kind of entry defines an address range that is
  included in the range list. The starting address is the lowest address
  of the address range. The ending address is the address of the first
  location past the highest address of the address range

This seems pretty clear (to me) that the end address is not part of the
region covered by a range.

Additionally, if we start to accept 'addr == end' then this is going to
cause problems elsewhere.  GDB will place a b/p at the 'end' address,
but when GDB then performs block lookup, GDB will not return the block
we expect, and so GDB will not report the inferior as having stopped in
the scope that the user expects.

Thanks,
Andrew