Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
To: Andrew Burgess <aburgess@redhat.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH] gdb: handle DW_AT_entry_pc pointing at an empty sub-range
Date: Fri, 22 Nov 2024 23:57:55 +0100	[thread overview]
Message-ID: <DU2PR08MB1026331734B552BE1116A85D0E4232@DU2PR08MB10263.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <87y11bw6p3.fsf@redhat.com>



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.


Thanks
Bernd.

  reply	other threads:[~2024-11-22 22:57 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-20 15:01 Andrew Burgess
2024-11-20 21:00 ` Kevin Buettner
2024-11-21 13:21 ` Bernd Edlinger
2024-11-22 16:53   ` Andrew Burgess
2024-11-22 22:57     ` Bernd Edlinger [this message]
2024-11-25 14:30       ` Andrew Burgess
2024-11-26 12:35         ` Bernd Edlinger
2024-11-26 17:48           ` Andrew Burgess
2024-11-27 20:12             ` Bernd Edlinger
2024-11-28 10:10               ` Andrew Burgess
2024-11-28 17:44 ` [PATCHv2] " Andrew Burgess
2024-11-29 14:19   ` Bernd Edlinger
2024-12-02 10:53     ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DU2PR08MB1026331734B552BE1116A85D0E4232@DU2PR08MB10263.eurprd08.prod.outlook.com \
    --to=bernd.edlinger@hotmail.de \
    --cc=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox