[4/4,gdb/symtab] Use unrelocated addresses in call_site

Message ID 20211001123328.22314-4-tdevries@suse.de
State New
Headers show
Series
  • [1/4,gdb/symtab] Fix htab_find_slot call in read_call_site_scope
Related show

Commit Message

Lancelot SIX via Gdb-patches Oct. 1, 2021, 12:33 p.m.
From: Simon Marchi <simon.marchi@polymtl.ca>


Consider test-case gdb.trace/entry-values.exp with target board
unix/-fPIE/-pie.

Using this command we have an abbreviated version, and can see the correct
@entry values for foo:
...
$ gdb -q -batch outputs/gdb.trace/entry-values/entry-values \
  -ex start \
  -ex "break foo" \
  -ex "set print entry-values both" \
  -ex continue
Temporary breakpoint 1 at 0x679

Temporary breakpoint 1, 0x0000555555554679 in main ()
Breakpoint 2 at 0x55555555463e

Breakpoint 2, 0x000055555555463e in foo (i=0, i@entry=2, j=2, j@entry=3)
...

Now, let's try the same again, but run directly to foo rather than stopping at
main:
...
$ gdb -q -batch outputs/gdb.trace/entry-values/entry-values \
  -ex "break foo" \
  -ex "set print entry-values both" \
  -ex run
Breakpoint 1 at 0x63e

Breakpoint 1, 0x000055555555463e in foo (i=0, i@entry=<optimized out>, \
  j=2, j@entry=<optimized out>)
...

So, what explains the difference?  Noteworthy, this is a dwarf assembly
test-case, with debug info for foo and bar, but not for main.

In the first case:
- we run to main
- this does not trigger expanding debug info, because there's none for main
- we set a breakpoint at foo
- this triggers expanding debug info.  Relocated addresses are used in
  call_site info (because the exec is started)
- we continue to foo, and manage to find the call_site info

In the second case:
- we set a breakpoint at foo
- this triggers expanding debug info.  Unrelocated addresses are used in
  call_site info (because the exec is not started)
- we run to foo
- this triggers objfile_relocate1, but it doesn't update the call_site
  info addresses
- we don't manage to find the call_site info

We could fix this by adding the missing call_site relocation in
objfile_relocate1.

This solution however is counter-trend in the sense that we're trying to
work towards the situation where when starting two instances of an executable,
we need only one instance of debug information, implying the use of
unrelocated addresses.

So, fix this instead by using unrelocated addresses in call_site info.

Tested on x86_64-linux.

This fixes all remaining unix/-fno-PIE/-no-pie vs unix/-fPIE/-pie
regressions, like f.i. PR24892.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24892

Co-Authored-By: Tom de Vries <tdevries@suse.de>
---
 gdb/dwarf2/loc.c  | 9 ++++++++-
 gdb/dwarf2/read.c | 6 ++++--
 gdb/gdbtypes.c    | 5 ++++-
 gdb/gdbtypes.h    | 6 +++---
 gdb/symtab.c      | 6 +++++-
 5 files changed, 24 insertions(+), 8 deletions(-)

-- 
2.26.2

Comments

Lancelot SIX via Gdb-patches Oct. 1, 2021, 8:56 p.m. | #1
On 2021-10-01 08:33, Tom de Vries via Gdb-patches wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>

> 

> Consider test-case gdb.trace/entry-values.exp with target board

> unix/-fPIE/-pie.

> 

> Using this command we have an abbreviated version, and can see the 

> correct

> @entry values for foo:

> ...

> $ gdb -q -batch outputs/gdb.trace/entry-values/entry-values \

>   -ex start \

>   -ex "break foo" \

>   -ex "set print entry-values both" \

>   -ex continue

> Temporary breakpoint 1 at 0x679

> 

> Temporary breakpoint 1, 0x0000555555554679 in main ()

> Breakpoint 2 at 0x55555555463e

> 

> Breakpoint 2, 0x000055555555463e in foo (i=0, i@entry=2, j=2, 

> j@entry=3)

> ...

> 

> Now, let's try the same again, but run directly to foo rather than 

> stopping at

> main:

> ...

> $ gdb -q -batch outputs/gdb.trace/entry-values/entry-values \

>   -ex "break foo" \

>   -ex "set print entry-values both" \

>   -ex run

> Breakpoint 1 at 0x63e

> 

> Breakpoint 1, 0x000055555555463e in foo (i=0, i@entry=<optimized out>, 

> \

>   j=2, j@entry=<optimized out>)

> ...

> 

> So, what explains the difference?  Noteworthy, this is a dwarf assembly

> test-case, with debug info for foo and bar, but not for main.

> 

> In the first case:

> - we run to main

> - this does not trigger expanding debug info, because there's none for 

> main

> - we set a breakpoint at foo

> - this triggers expanding debug info.  Relocated addresses are used in

>   call_site info (because the exec is started)

> - we continue to foo, and manage to find the call_site info

> 

> In the second case:

> - we set a breakpoint at foo

> - this triggers expanding debug info.  Unrelocated addresses are used 

> in

>   call_site info (because the exec is not started)

> - we run to foo

> - this triggers objfile_relocate1, but it doesn't update the call_site

>   info addresses

> - we don't manage to find the call_site info


Thanks for this explanation, I had not realized the difference in 
behavior here.

The patch LGTM.

Simon
Lancelot SIX via Gdb-patches Oct. 4, 2021, 4:47 p.m. | #2
On 10/1/21 10:56 PM, Simon Marchi wrote:
> On 2021-10-01 08:33, Tom de Vries via Gdb-patches wrote:

>> From: Simon Marchi <simon.marchi@polymtl.ca>

>>

>> Consider test-case gdb.trace/entry-values.exp with target board

>> unix/-fPIE/-pie.

>>

>> Using this command we have an abbreviated version, and can see the

>> correct

>> @entry values for foo:

>> ...

>> $ gdb -q -batch outputs/gdb.trace/entry-values/entry-values \

>>   -ex start \

>>   -ex "break foo" \

>>   -ex "set print entry-values both" \

>>   -ex continue

>> Temporary breakpoint 1 at 0x679

>>

>> Temporary breakpoint 1, 0x0000555555554679 in main ()

>> Breakpoint 2 at 0x55555555463e

>>

>> Breakpoint 2, 0x000055555555463e in foo (i=0, i@entry=2, j=2, j@entry=3)

>> ...

>>

>> Now, let's try the same again, but run directly to foo rather than

>> stopping at

>> main:

>> ...

>> $ gdb -q -batch outputs/gdb.trace/entry-values/entry-values \

>>   -ex "break foo" \

>>   -ex "set print entry-values both" \

>>   -ex run

>> Breakpoint 1 at 0x63e

>>

>> Breakpoint 1, 0x000055555555463e in foo (i=0, i@entry=<optimized out>, \

>>   j=2, j@entry=<optimized out>)

>> ...

>>

>> So, what explains the difference?  Noteworthy, this is a dwarf assembly

>> test-case, with debug info for foo and bar, but not for main.

>>

>> In the first case:

>> - we run to main

>> - this does not trigger expanding debug info, because there's none for

>> main

>> - we set a breakpoint at foo

>> - this triggers expanding debug info.  Relocated addresses are used in

>>   call_site info (because the exec is started)

>> - we continue to foo, and manage to find the call_site info

>>

>> In the second case:

>> - we set a breakpoint at foo

>> - this triggers expanding debug info.  Unrelocated addresses are used in

>>   call_site info (because the exec is not started)

>> - we run to foo

>> - this triggers objfile_relocate1, but it doesn't update the call_site

>>   info addresses

>> - we don't manage to find the call_site info

> 

> Thanks for this explanation, I had not realized the difference in

> behavior here.

> 

> The patch LGTM.


Thanks for the review.  Committed.

Thanks,
- Tom

Patch

diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 27a1c97682a..16e0be73d02 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -713,7 +713,14 @@  call_site_to_target_addr (struct gdbarch *call_site_gdbarch,
       }
 
     case FIELD_LOC_KIND_PHYSADDR:
-      return FIELD_STATIC_PHYSADDR (call_site->target);
+      {
+	dwarf2_per_objfile *per_objfile = call_site->per_objfile;
+	compunit_symtab *cust = per_objfile->get_symtab (call_site->per_cu);
+	int sect_idx = COMPUNIT_BLOCK_LINE_SECTION (cust);
+	CORE_ADDR delta = per_objfile->objfile->section_offsets[sect_idx];
+
+	return FIELD_STATIC_PHYSADDR (call_site->target) + delta;
+      }
 
     default:
       internal_error (__FILE__, __LINE__, _("invalid call site target kind"));
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index d0460674d10..fa775722afb 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -13364,6 +13364,7 @@  read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
     }
   pc = attr->as_address () + baseaddr;
   pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
+  pc -= baseaddr;
 
   if (cu->call_site_htab == NULL)
     cu->call_site_htab = htab_create_alloc_ex (16, core_addr_hash, core_addr_eq,
@@ -13405,7 +13406,7 @@  read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
 		      + (sizeof (*call_site->parameter) * (nparams - 1))));
   *slot = call_site;
   memset (call_site, 0, sizeof (*call_site) - sizeof (*call_site->parameter));
-  call_site->m_pc = pc;
+  call_site->m_unrelocated_pc = pc;
 
   if (dwarf2_flag_true_p (die, DW_AT_call_tail_call, cu)
       || dwarf2_flag_true_p (die, DW_AT_GNU_tail_call, cu))
@@ -13516,7 +13517,8 @@  read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
 		       sect_offset_str (die->sect_off), objfile_name (objfile));
 	  else
 	    {
-	      lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
+	      lowpc = (gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr)
+		       - baseaddr);
 	      SET_FIELD_PHYSADDR (call_site->target, lowpc);
 	    }
 	}
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 500488adece..54a99af9c2e 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -6315,7 +6315,10 @@  objfile_type (struct objfile *objfile)
 CORE_ADDR
 call_site::pc () const
 {
-  return m_pc;
+  compunit_symtab *cust = this->per_objfile->get_symtab (this->per_cu);
+  CORE_ADDR delta
+	= this->per_objfile->objfile->section_offsets[COMPUNIT_BLOCK_LINE_SECTION (cust)];
+  return m_unrelocated_pc + delta;
 }
 
 void _initialize_gdbtypes ();
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 7a17005229e..1d15916d4e5 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1783,13 +1783,13 @@  struct call_site_parameter
 
 struct call_site
   {
-    /* As m_pc.  */
+    /* As m_unrelocated_pc, but relocated.  */
 
     CORE_ADDR pc () const;
 
-    /* Address of the first instruction after this call.  */
+    /* Unrelocated address of the first instruction after this call.  */
 
-    CORE_ADDR m_pc;
+    CORE_ADDR m_unrelocated_pc;
 
     /* * List successor with head in FUNC_TYPE.TAIL_CALL_LIST.  */
 
diff --git a/gdb/symtab.c b/gdb/symtab.c
index b7a00709d00..65247869913 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -337,7 +337,11 @@  compunit_symtab::find_call_site (CORE_ADDR pc) const
   if (m_call_site_htab == nullptr)
     return nullptr;
 
-  void **slot = htab_find_slot (m_call_site_htab, &pc, NO_INSERT);
+  CORE_ADDR delta
+    = this->objfile->section_offsets[COMPUNIT_BLOCK_LINE_SECTION (this)];
+  CORE_ADDR unrelocated_pc = pc - delta;
+
+  void **slot = htab_find_slot (m_call_site_htab, &unrelocated_pc, NO_INSERT);
   if (slot == nullptr)
     return nullptr;