[v2,3/4] Fix handling of DW_AT_entry_pc of inlined subroutines

Message ID AM8PR10MB4708ED8173B75A4AF5C1F965E4209@AM8PR10MB4708.EURPRD10.PROD.OUTLOOK.COM
State Superseded
Headers show
Series
  • [v2,1/4] Introduce a new line table flag is_weak
Related show

Commit Message

Bernd Edlinger May 30, 2021, 1:58 p.m.
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.

2021-01-01  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* dwarf2/read.c (dwarf2_record_block_ranges): Move range beginning
	at DW_AT_entry_pc to the first block range.


Thanks,
Bernd.

Comments

Tom de Vries via Gdb-patches June 2, 2021, 3:54 p.m. | #1
Hi Bernd,

I have a question; see below...

On Sun, 30 May 2021 15:58:51 +0200
Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:

> 2021-01-01  Bernd Edlinger  <bernd.edlinger@hotmail.de>

> 

> 	* dwarf2/read.c (dwarf2_record_block_ranges): Move range beginning

> 	at DW_AT_entry_pc to the first block range.

> ---

>  gdb/dwarf2/read.c | 14 ++++++++++++++

>  1 file changed, 14 insertions(+)

> 

> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c

> index 25d9d96..6168d5e 100644

> --- a/gdb/dwarf2/read.c

> +++ b/gdb/dwarf2/read.c

> @@ -14177,6 +14177,18 @@ class process_die_scope

>        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)


Is there any reason to restrict the entry_pc handling that you
introduce here to DW_TAG_inline_subroutine?

If it can used for other types of DIEs too, then the comment preceding
the definition of BLOCK_ENTRY_PC in block.h should be updated.

> +	{

> +	  attr = dwarf2_attr (die, DW_AT_entry_pc, cu);

> +	  if (attr != nullptr)

> +	    {

> +	      entry_pc = attr->as_address ();

> +	      entry_pc += baseaddr;

> +	      entry_pc = gdbarch_adjust_dwarf2_addr (gdbarch, entry_pc);

> +	    }

> +	}

> +

>        std::vector<blockrange> blockvec;

>        dwarf2_ranges_process (ranges_offset, cu, die->tag,

>  	[&] (CORE_ADDR start, CORE_ADDR end)

> @@ -14187,6 +14199,8 @@ class process_die_scope

>  	  end = gdbarch_adjust_dwarf2_addr (gdbarch, end);

>  	  cu->get_builder ()->record_block_range (block, start, end - 1);

>  	  blockvec.emplace_back (start, end);

> +	  if (entry_pc == start && blockvec.size () > 1)

> +	    std::swap (blockvec[0], blockvec[blockvec.size () - 1]);

>  	});

>  

>        BLOCK_RANGES(block) = make_blockranges (objfile, blockvec);
Bernd Edlinger June 2, 2021, 6:24 p.m. | #2
Hi Kevin,

On 6/2/21 5:54 PM, Kevin Buettner wrote:
> Hi Bernd,

> 

> I have a question; see below...

> 

> On Sun, 30 May 2021 15:58:51 +0200

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

> 

>> 2021-01-01  Bernd Edlinger  <bernd.edlinger@hotmail.de>

>>

>> 	* dwarf2/read.c (dwarf2_record_block_ranges): Move range beginning

>> 	at DW_AT_entry_pc to the first block range.

>> ---

>>  gdb/dwarf2/read.c | 14 ++++++++++++++

>>  1 file changed, 14 insertions(+)

>>

>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c

>> index 25d9d96..6168d5e 100644

>> --- a/gdb/dwarf2/read.c

>> +++ b/gdb/dwarf2/read.c

>> @@ -14177,6 +14177,18 @@ class process_die_scope

>>        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)

> 

> Is there any reason to restrict the entry_pc handling that you

> introduce here to DW_TAG_inline_subroutine?

> 


No, but I have so far never seen an entry_pc for a static function.

The entry pc for a static function should be the symbol of the same
name, or one of the name.part.x labels, usually there is more than
one PC where a function can be entered.

But OTOH for inline functions most of the time, the entry_pc is the start
of the first subrange, but there are some cases, where the entry_pc is the
start of the second subrange, and of course also cases where there is no
entry_pc for whatever reason, and really rare cases, where the entry pc
is the start of an empty subrange.  Nevertheless I think that in some cases
the debug info allows us to examine something interesting like an input
parameter for this inlined subroutine, when we allow the program to stop
at the entry_pc wherever that may be.


Bernd.

> If it can used for other types of DIEs too, then the comment preceding

> the definition of BLOCK_ENTRY_PC in block.h should be updated.

> 

>> +	{

>> +	  attr = dwarf2_attr (die, DW_AT_entry_pc, cu);

>> +	  if (attr != nullptr)

>> +	    {

>> +	      entry_pc = attr->as_address ();

>> +	      entry_pc += baseaddr;

>> +	      entry_pc = gdbarch_adjust_dwarf2_addr (gdbarch, entry_pc);

>> +	    }

>> +	}

>> +

>>        std::vector<blockrange> blockvec;

>>        dwarf2_ranges_process (ranges_offset, cu, die->tag,

>>  	[&] (CORE_ADDR start, CORE_ADDR end)

>> @@ -14187,6 +14199,8 @@ class process_die_scope

>>  	  end = gdbarch_adjust_dwarf2_addr (gdbarch, end);

>>  	  cu->get_builder ()->record_block_range (block, start, end - 1);

>>  	  blockvec.emplace_back (start, end);

>> +	  if (entry_pc == start && blockvec.size () > 1)

>> +	    std::swap (blockvec[0], blockvec[blockvec.size () - 1]);

>>  	});

>>  

>>        BLOCK_RANGES(block) = make_blockranges (objfile, blockvec);

>

Patch

From e654a4ac797636580167d9745dcf29532d5c90de Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Tue, 3 Nov 2020 18:41:43 +0100
Subject: [PATCH v2 3/4] 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.

2021-01-01  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* dwarf2/read.c (dwarf2_record_block_ranges): Move range beginning
	at DW_AT_entry_pc to the first block range.
---
 gdb/dwarf2/read.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 25d9d96..6168d5e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14177,6 +14177,18 @@  class process_die_scope
       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 = attr->as_address ();
+	      entry_pc += baseaddr;
+	      entry_pc = gdbarch_adjust_dwarf2_addr (gdbarch, entry_pc);
+	    }
+	}
+
       std::vector<blockrange> blockvec;
       dwarf2_ranges_process (ranges_offset, cu, die->tag,
 	[&] (CORE_ADDR start, CORE_ADDR end)
@@ -14187,6 +14199,8 @@  class process_die_scope
 	  end = gdbarch_adjust_dwarf2_addr (gdbarch, end);
 	  cu->get_builder ()->record_block_range (block, start, end - 1);
 	  blockvec.emplace_back (start, end);
+	  if (entry_pc == start && blockvec.size () > 1)
+	    std::swap (blockvec[0], blockvec[blockvec.size () - 1]);
 	});
 
       BLOCK_RANGES(block) = make_blockranges (objfile, blockvec);
-- 
1.9.1