[v3,1/3] Fix handling of DW_AT_entry_pc of inlined subroutines

  It may happen that the inline entry point is not the
start address of the first sub-range of an inline
function.

But the PC for a breakpoint on an inlined subroutine
is always the start address of the first sub-range.

This patch moves the sub-range starting at the entry
point to the first position of the block list.

Therefore the breakpoint on an inlined function
changes in rare cases from the start address of
the first sub-range to the real entry point.

There should always be a subrange that starts at the
entry point, even if that is an empty sub-range.
---
 gdb/dwarf2/read.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
  

On 7/5/24 6:16 AM, Bernd Edlinger wrote:
> It may happen that the inline entry point is not the
> start address of the first sub-range of an inline
> function.
>
> But the PC for a breakpoint on an inlined subroutine
> is always the start address of the first sub-range.
>
> This patch moves the sub-range starting at the entry
> point to the first position of the block list.
>
> Therefore the breakpoint on an inlined function
> changes in rare cases from the start address of
> the first sub-range to the real entry point.
>
> There should always be a subrange that starts at the
> entry point, even if that is an empty sub-range.
> ---
>   gdb/dwarf2/read.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 71237d0fba8..60fd8b45eb5 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -11246,6 +11246,16 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>         if (die->tag != DW_TAG_compile_unit)
>   	ranges_offset += cu->gnu_ranges_base;
>   
> +      CORE_ADDR entry_pc = (CORE_ADDR) -1;
> +      if (die->tag == DW_TAG_inlined_subroutine)
> +	{
> +	  attr = dwarf2_attr (die, DW_AT_entry_pc, cu);
> +	  if (attr != nullptr)
> +	    {
> +	      entry_pc = per_objfile->relocate (attr->as_address ());

According to the DWARF 5 spec -section 2.18, DW_AT_entry_pc can be 
either an address (which you are correctly handling) or a constant, 
which would be an offset from the base address of the function. I think 
the second form should also be handled, but we should at the very least 
warn that it happened so we don't get assert or worse, cryptic bugs.

> +	    }
> +	}
> +
>         std::vector<blockrange> blockvec;
>         dwarf2_ranges_process (ranges_offset, cu, die->tag,
>   	[&] (unrelocated_addr start, unrelocated_addr end)
> @@ -11255,6 +11265,8 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>   	  cu->get_builder ()->record_block_range (block, abs_start,
>   						  abs_end - 1);
>   	  blockvec.emplace_back (abs_start, abs_end);
> +	  if (entry_pc == abs_start && blockvec.size () > 1)
> +	    std::swap (blockvec[0], blockvec[blockvec.size () - 1]);

I'm confused about this. If the entry program counter is the start of 
the given block, we swap blocks?

Could you explain what's going on here?

>   	});
>   
>         block->set_ranges (make_blockranges (objfile, blockvec));
  

Guinevere Larsen <blarsen@redhat.com> writes:

> On 7/5/24 6:16 AM, Bernd Edlinger wrote:
>> It may happen that the inline entry point is not the
>> start address of the first sub-range of an inline
>> function.
>>
>> But the PC for a breakpoint on an inlined subroutine
>> is always the start address of the first sub-range.
>>
>> This patch moves the sub-range starting at the entry
>> point to the first position of the block list.
>>
>> Therefore the breakpoint on an inlined function
>> changes in rare cases from the start address of
>> the first sub-range to the real entry point.
>>
>> There should always be a subrange that starts at the
>> entry point, even if that is an empty sub-range.
>> ---
>>   gdb/dwarf2/read.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>> index 71237d0fba8..60fd8b45eb5 100644
>> --- a/gdb/dwarf2/read.c
>> +++ b/gdb/dwarf2/read.c
>> @@ -11246,6 +11246,16 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>>         if (die->tag != DW_TAG_compile_unit)
>>   	ranges_offset += cu->gnu_ranges_base;
>>   
>> +      CORE_ADDR entry_pc = (CORE_ADDR) -1;
>> +      if (die->tag == DW_TAG_inlined_subroutine)
>> +	{
>> +	  attr = dwarf2_attr (die, DW_AT_entry_pc, cu);
>> +	  if (attr != nullptr)
>> +	    {
>> +	      entry_pc = per_objfile->relocate (attr->as_address ());
>
> According to the DWARF 5 spec -section 2.18, DW_AT_entry_pc can be 
> either an address (which you are correctly handling) or a constant, 
> which would be an offset from the base address of the function. I think 
> the second form should also be handled, but we should at the very least 
> warn that it happened so we don't get assert or worse, cryptic bugs.

I think the constant case was only added in DWARF5.  We have other
places in dwarf/read.c where we do things like:

  if (cu->header.version >= 4 && attr_high->form_is_constant ())
    ...

which is probably what we need here.

>
>> +	    }
>> +	}
>> +
>>         std::vector<blockrange> blockvec;
>>         dwarf2_ranges_process (ranges_offset, cu, die->tag,
>>   	[&] (unrelocated_addr start, unrelocated_addr end)
>> @@ -11255,6 +11265,8 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>>   	  cu->get_builder ()->record_block_range (block, abs_start,
>>   						  abs_end - 1);
>>   	  blockvec.emplace_back (abs_start, abs_end);
>> +	  if (entry_pc == abs_start && blockvec.size () > 1)
>> +	    std::swap (blockvec[0], blockvec[blockvec.size () - 1]);
>
> I'm confused about this. If the entry program counter is the start of 
> the given block, we swap blocks?

If the entry $pc is the start of the block which we just added, but the
block isn't the first block (due to the .size() > 1 check) then we swap
this block with the first block.

This ensures that blockvec[0] is the one that starts at entry_pc.

If we do go with this approach then I do think a comment here wouldn't
hurt.

Thanks,
Andrew

>
> Could you explain what's going on here?
>
>>   	});
>>   
>>         block->set_ranges (make_blockranges (objfile, blockvec));
>
>
> -- 
> Cheers,
> Guinevere Larsen
> She/Her/Hers
  

Bernd,

Thanks for picking up these changes again.  Any improvements to
debugging optimised code will be a huge improvement.

I noticed that your patches arrived to the mailing list without
threading, which makes it harder to track the related emails.  If you
are using 'git format-patch' or 'get send-email' to send your patches
then you should consider adding the '--thread' option.

Alternatively, I have this in my .gitconfig file:

  [format]
          useAutoBase = true
          coverLetter = auto
          thread = shallow

specifically, the 'thread' option here means I don't need to pass the
--thread flag.

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

> It may happen that the inline entry point is not the
> start address of the first sub-range of an inline
> function.
>
> But the PC for a breakpoint on an inlined subroutine
> is always the start address of the first sub-range.
>
> This patch moves the sub-range starting at the entry
> point to the first position of the block list.
>
> Therefore the breakpoint on an inlined function
> changes in rare cases from the start address of
> the first sub-range to the real entry point.
>
> There should always be a subrange that starts at the
> entry point, even if that is an empty sub-range.
> ---
>  gdb/dwarf2/read.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

I notice there's no test added for this change.

I tried applying all 3 patches in this series and saw that all the new
tests passed.  Then I backed out this patch and all of the new tests
continued to pass, so I suspect that this change is untested.

This might be that this functionality is only hit for some compiler
versions maybe?

I think we should consider adding some tests into gdb.dwarf2/ that make
use of GDB's DWARF compiler to ensure this new functionality is always
being tested.

>
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 71237d0fba8..60fd8b45eb5 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -11246,6 +11246,16 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>        if (die->tag != DW_TAG_compile_unit)
>  	ranges_offset += cu->gnu_ranges_base;
>  
> +      CORE_ADDR entry_pc = (CORE_ADDR) -1;
> +      if (die->tag == DW_TAG_inlined_subroutine)

I understand from reading earlier versions of this thread that the
DW_TAG_inlined_subroutine check is because you've only seen the
DW_AT_entry_pc for this case, but I wonder if there would be any harm
handling it for other cases?

Alternatively, if that makes you uncomfortable, maybe we should check
for the DW_AT_entry_pc and give a warning in other cases that the
entry-pc has been ignored?  At least then users might report this and
we'd know that we should cover more cases.

Personally, I'd be happy to just remove the DW_TAG_inlined_subroutine
check.

> +	{
> +	  attr = dwarf2_attr (die, DW_AT_entry_pc, cu);
> +	  if (attr != nullptr)
> +	    {
> +	      entry_pc = per_objfile->relocate (attr->as_address ());
> +	    }
> +	}

I agree with Guinevere that we should try to cover the DWARF5 constant
case here too.

Also the { ... } around the 'entry_pc = ....' line should be removed
please (GDB coding standard).

> +
>        std::vector<blockrange> blockvec;
>        dwarf2_ranges_process (ranges_offset, cu, die->tag,
>  	[&] (unrelocated_addr start, unrelocated_addr end)
> @@ -11255,6 +11265,8 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>  	  cu->get_builder ()->record_block_range (block, abs_start,
>  						  abs_end - 1);
>  	  blockvec.emplace_back (abs_start, abs_end);
> +	  if (entry_pc == abs_start && blockvec.size () > 1)
> +	    std::swap (blockvec[0], blockvec[blockvec.size () - 1]);

I think it is worth reading the comment in block.h on
block::entry_pc().  There's some interesting commentary on how the
entry-pc should be handled.

The thinking there is that we should add a specific 'entry_pc' field to
(probably) the block class.  And I think there's something to be said
for this idea.

My reading of the DWARF spec is that there's no specific reason why the
entry-pc has to be the first address of any particular blockrange, or
that the DW_AT_entry_pc can only be used when a block has DW_AT_ranges,
though I suspect this is all you've encountered, and I can see, from an
implementation perspective, why that is likely the case.

I wonder if it would be better to have the DWARF reader figure out the
entry-pc and then tell the block what that should be.  Then the block
class could drop the conditional logic from ::entry_pc() and instead
just return the entry-pc it was told.

I think we should consider splitting the DW_AT_entry_pc check from the
DW_AT_ranges check, and having dwarf2_record_block_ranges become
dwarf2_record_block_ranges_and_entry_pc.  In
dwarf2_record_block_ranges_and_entry_pc we'd then call a new
block::set_entry_pc(...) method, and block::entry_pc() could drop the
current logic and just report the recorded entry-pc.

Thanks,
Andrew


>  	});
>  
>        block->set_ranges (make_blockranges (objfile, blockvec));
> -- 
> 2.39.2
  

On 7/15/24 17:36, Andrew Burgess wrote:
> Guinevere Larsen <blarsen@redhat.com> writes:
> 
>> On 7/5/24 6:16 AM, Bernd Edlinger wrote:
>>> It may happen that the inline entry point is not the
>>> start address of the first sub-range of an inline
>>> function.
>>>
>>> But the PC for a breakpoint on an inlined subroutine
>>> is always the start address of the first sub-range.
>>>
>>> This patch moves the sub-range starting at the entry
>>> point to the first position of the block list.
>>>
>>> Therefore the breakpoint on an inlined function
>>> changes in rare cases from the start address of
>>> the first sub-range to the real entry point.
>>>
>>> There should always be a subrange that starts at the
>>> entry point, even if that is an empty sub-range.
>>> ---
>>>   gdb/dwarf2/read.c | 12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>>> index 71237d0fba8..60fd8b45eb5 100644
>>> --- a/gdb/dwarf2/read.c
>>> +++ b/gdb/dwarf2/read.c
>>> @@ -11246,6 +11246,16 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>>>         if (die->tag != DW_TAG_compile_unit)
>>>   	ranges_offset += cu->gnu_ranges_base;
>>>   
>>> +      CORE_ADDR entry_pc = (CORE_ADDR) -1;
>>> +      if (die->tag == DW_TAG_inlined_subroutine)
>>> +	{
>>> +	  attr = dwarf2_attr (die, DW_AT_entry_pc, cu);
>>> +	  if (attr != nullptr)
>>> +	    {
>>> +	      entry_pc = per_objfile->relocate (attr->as_address ());
>>
>> According to the DWARF 5 spec -section 2.18, DW_AT_entry_pc can be 
>> either an address (which you are correctly handling) or a constant, 
>> which would be an offset from the base address of the function. I think 
>> the second form should also be handled, but we should at the very least 
>> warn that it happened so we don't get assert or worse, cryptic bugs.
> 
> I think the constant case was only added in DWARF5.  We have other
> places in dwarf/read.c where we do things like:
> 
>   if (cu->header.version >= 4 && attr_high->form_is_constant ())
>     ...
> 
> which is probably what we need here.
> 

Ok, I see,

I have tried to find such constant forms, and fortunately there is no such
thing.  I was trying with gcc-9, gcc-12 and gcc-14.
But of course that may change in the future.

Therefore I would change this part to:

  attr = dwarf2_attr (die, DW_AT_entry_pc, cu);
  if (attr != nullptr)
  if (attr != nullptr && !attr->form_is_constant ())
    entry_pc = per_objfile->relocate (attr->as_address ());

to be on the safe side.

>>
>>> +	    }
>>> +	}
>>> +
>>>         std::vector<blockrange> blockvec;
>>>         dwarf2_ranges_process (ranges_offset, cu, die->tag,
>>>   	[&] (unrelocated_addr start, unrelocated_addr end)
>>> @@ -11255,6 +11265,8 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>>>   	  cu->get_builder ()->record_block_range (block, abs_start,
>>>   						  abs_end - 1);
>>>   	  blockvec.emplace_back (abs_start, abs_end);
>>> +	  if (entry_pc == abs_start && blockvec.size () > 1)
>>> +	    std::swap (blockvec[0], blockvec[blockvec.size () - 1]);
>>
>> I'm confused about this. If the entry program counter is the start of 
>> the given block, we swap blocks?
> 
> If the entry $pc is the start of the block which we just added, but the
> block isn't the first block (due to the .size() > 1 check) then we swap
> this block with the first block.
> 
> This ensures that blockvec[0] is the one that starts at entry_pc.
> 
> If we do go with this approach then I do think a comment here wouldn't
> hurt.
> 

Yes, I can add a comment, that refers to block::entry_pc () and says
this makes it do the right thing.


Thanks
Bernd.

> Thanks,
> Andrew
> 
>>
>> Could you explain what's going on here?
>>
>>>   	});
>>>   
>>>         block->set_ranges (make_blockranges (objfile, blockvec));
>>
>>
>> -- 
>> Cheers,
>> Guinevere Larsen
>> She/Her/Hers
>
  

On 7/16/24 11:41, Andrew Burgess wrote:
> 
> Bernd,
> 
> Thanks for picking up these changes again.  Any improvements to
> debugging optimised code will be a huge improvement.
> 
> I noticed that your patches arrived to the mailing list without
> threading, which makes it harder to track the related emails.  If you
> are using 'git format-patch' or 'get send-email' to send your patches
> then you should consider adding the '--thread' option.
> 
> Alternatively, I have this in my .gitconfig file:
> 
>   [format]
>           useAutoBase = true
>           coverLetter = auto
>           thread = shallow
> 
> specifically, the 'thread' option here means I don't need to pass the
> --thread flag.
> 
> Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
> 
>> It may happen that the inline entry point is not the
>> start address of the first sub-range of an inline
>> function.
>>
>> But the PC for a breakpoint on an inlined subroutine
>> is always the start address of the first sub-range.
>>
>> This patch moves the sub-range starting at the entry
>> point to the first position of the block list.
>>
>> Therefore the breakpoint on an inlined function
>> changes in rare cases from the start address of
>> the first sub-range to the real entry point.
>>
>> There should always be a subrange that starts at the
>> entry point, even if that is an empty sub-range.
>> ---
>>  gdb/dwarf2/read.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
> 
> I notice there's no test added for this change.
> 
> I tried applying all 3 patches in this series and saw that all the new
> tests passed.  Then I backed out this patch and all of the new tests
> continued to pass, so I suspect that this change is untested.
> 
> This might be that this functionality is only hit for some compiler
> versions maybe?
> 
> I think we should consider adding some tests into gdb.dwarf2/ that make
> use of GDB's DWARF compiler to ensure this new functionality is always
> being tested.
> 

Well, yes.  I tried to find examples of real issues by analyzing the
debug info of gdb and other binutils programs.  And the results are:

- there are almost always entry_pc but only in inline_blocks,
- there was not a single entry_pc in a non-inline block.
- no entry_pc had the constant form.
- around 50% of entry_pc point to the beginning of first subrange
- around 50% of entry_pc point to the beginning of a subsequent subrange
- empty subranges exist, they are currently ignored, but part 3 will change that
- rarely an entry_pc points to an empty subrange.
- very rarely an entry_pc points into the middle of a subrange
- and extremely rarely an entry_pc points to the end_pc of a subrange
- no entry_pc point completely outside of any subrange
- subrange[0] starts not always at lowest address

Using this data I tried to find a debug code that exhibits the
problem that I described in the commit message, but unfortunately I could
not locate that anymore.  It is possible that this was only in a daily
gcc snapshot, and fixed later.  Especially that subranges are not sorted
by address, is completely new to me.

Finally I found an anomaly withing binutils/strip.c
where we have this:

static bool
is_specified_symbol (const char *name, htab_t htab)
{
  if (wildcard)
    {
      struct is_specified_symbol_predicate_data data;

      data.name = name;
      data.found = false;

      htab_traverse (htab, is_specified_symbol_predicate, &data);

      return data.found;
    }

  return htab_find (htab, name) != NULL;
}

[...]

          for (w_relpp = relpp, i = 0; i < relcount; i++)
            /* PR 17512: file: 9e907e0c.  */
            if (relpp[i]->sym_ptr_ptr
                /* PR 20096 */
                && *relpp[i]->sym_ptr_ptr
                && is_specified_symbol (bfd_asymbol_name (*relpp[i]->sym_ptr_ptr),
                                        keep_specific_htab))
              *w_relpp++ = relpp[i];
          relcount = w_relpp - relpp;
          *w_relpp = 0;


 <5><1a3e6>: Abbrev Number: 18 (DW_TAG_inlined_subroutine)
    <1a3e7>   DW_AT_abstract_origin: <0x181b7>
    <1a3eb>   DW_AT_entry_pc    : 0x408514
    <1a3f3>   DW_AT_GNU_entry_view: 1
    <1a3f5>   DW_AT_ranges      : 0x37f
    <1a3f9>   DW_AT_call_file   : 1
    <1a3f9>   DW_AT_call_line   : 4579
    <1a3fb>   DW_AT_call_column : 6
    <1a3fc>   DW_AT_sibling     : <0x1a4aa>

    0000037f 00000000004084bd (base address)
    00000388 00000000004084bd 00000000004084c2 
    0000038b 00000000004084fd 0000000000408504 
    0000038e 0000000000408514 000000000040852f 

and the assembly at the first subrange is:

  return htab_find (htab, name) != NULL;
  4084bd:       e8 ee 77 0d 00          call   4dfcb0 <htab_find>

but this is the last statement of the is_specified_symbol (),
and it is not called in every case. the correct entry point is at

  if (wildcard)
  408514:       74 a7                   je     4084bd <copy_relocations_in_section+0x13d>

I have seen this anomaly with the same function in gcc-12 and gcc-15
builds.  However real issues like this is a very rarely thing, although
it is a rather common thing that the entry_pc points to a subsequent range start.
But usually there are no conditional jumps involved, unlike in the example above.

I would probably try to create a realistic test case out of this, and if that
is not possible, maybe create a test case in gdb.dwarf2 form, but I have never
tried that before.

>>
>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>> index 71237d0fba8..60fd8b45eb5 100644
>> --- a/gdb/dwarf2/read.c
>> +++ b/gdb/dwarf2/read.c
>> @@ -11246,6 +11246,16 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>>        if (die->tag != DW_TAG_compile_unit)
>>  	ranges_offset += cu->gnu_ranges_base;
>>  
>> +      CORE_ADDR entry_pc = (CORE_ADDR) -1;
>> +      if (die->tag == DW_TAG_inlined_subroutine)
> 
> I understand from reading earlier versions of this thread that the
> DW_TAG_inlined_subroutine check is because you've only seen the
> DW_AT_entry_pc for this case, but I wonder if there would be any harm
> handling it for other cases?
> 
> Alternatively, if that makes you uncomfortable, maybe we should check
> for the DW_AT_entry_pc and give a warning in other cases that the
> entry-pc has been ignored?  At least then users might report this and
> we'd know that we should cover more cases.
> 
> Personally, I'd be happy to just remove the DW_TAG_inlined_subroutine
> check.
> 
>> +	{
>> +	  attr = dwarf2_attr (die, DW_AT_entry_pc, cu);
>> +	  if (attr != nullptr)
>> +	    {
>> +	      entry_pc = per_objfile->relocate (attr->as_address ());
>> +	    }
>> +	}
> 
> I agree with Guinevere that we should try to cover the DWARF5 constant
> case here too.
> 
> Also the { ... } around the 'entry_pc = ....' line should be removed
> please (GDB coding standard).
> 
>> +
>>        std::vector<blockrange> blockvec;
>>        dwarf2_ranges_process (ranges_offset, cu, die->tag,
>>  	[&] (unrelocated_addr start, unrelocated_addr end)
>> @@ -11255,6 +11265,8 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>>  	  cu->get_builder ()->record_block_range (block, abs_start,
>>  						  abs_end - 1);
>>  	  blockvec.emplace_back (abs_start, abs_end);
>> +	  if (entry_pc == abs_start && blockvec.size () > 1)
>> +	    std::swap (blockvec[0], blockvec[blockvec.size () - 1]);
> 
> I think it is worth reading the comment in block.h on
> block::entry_pc().  There's some interesting commentary on how the
> entry-pc should be handled.
> 
> The thinking there is that we should add a specific 'entry_pc' field to
> (probably) the block class.  And I think there's something to be said
> for this idea.
> 
> My reading of the DWARF spec is that there's no specific reason why the
> entry-pc has to be the first address of any particular blockrange, or
> that the DW_AT_entry_pc can only be used when a block has DW_AT_ranges,
> though I suspect this is all you've encountered, and I can see, from an
> implementation perspective, why that is likely the case.
> 
> I wonder if it would be better to have the DWARF reader figure out the
> entry-pc and then tell the block what that should be.  Then the block
> class could drop the conditional logic from ::entry_pc() and instead
> just return the entry-pc it was told.
> 
> I think we should consider splitting the DW_AT_entry_pc check from the
> DW_AT_ranges check, and having dwarf2_record_block_ranges become
> dwarf2_record_block_ranges_and_entry_pc.  In
> dwarf2_record_block_ranges_and_entry_pc we'd then call a new
> block::set_entry_pc(...) method, and block::entry_pc() could drop the
> current logic and just report the recorded entry-pc.
> 

Well, maybe that is something for a follow-up patch, but
I would prefer to do that step-wise, first address the rather common 50%
of all cases, where the entry_pc points to a subsequent range beginning,
and then address other cases.  While I am confident that this patch does
not create new issues, because we should be able to handle breakpoints at
empty subranges, and have always been able to handle breakpoints at
the start of a subrange, I am not sure, if breakpoints in the middle
of a subrange will work, and even less that breakpoints at the end
of a subrange will work properly, especially the callstack could be
very confusing.

So I would go with this simple approach first, and improve that later,
when I have examples where this improvement can be tested against.


Thanks
Bernd.

> Thanks,
> Andrew
> 
> 
>>  	});
>>  
>>        block->set_ranges (make_blockranges (objfile, blockvec));
>> -- 
>> 2.39.2
>
  

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Randomly reading old notes and found this:

>> According to the DWARF 5 spec -section 2.18, DW_AT_entry_pc can be 
>> either an address (which you are correctly handling) or a constant, 
>> which would be an offset from the base address of the function. I think 
>> the second form should also be handled, but we should at the very least 
>> warn that it happened so we don't get assert or worse, cryptic bugs.

Andrew> I think the constant case was only added in DWARF5.  We have other
Andrew> places in dwarf/read.c where we do things like:

Andrew>   if (cu->header.version >= 4 && attr_high->form_is_constant ())
Andrew>     ...

Andrew> which is probably what we need here.

If DWARF introduces a change that is backward-compatible, it seems fine
to simply accept it regardless of the version.  I'm sure gdb does this
in many places.

If the change isn't compatible, then a CU version check is needed.

Tom