[PATCH][gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH][gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

Tom de Vries
Hi,

When running gdb.dwarf2/amd64-entry-value-param.exp with target board
unix/-fPIE/-pie, we get:
...
FAIL: gdb.arch/amd64-entry-value-param.exp: call 1: p y
...

The problem is that read_call_site_scope attempts to put relocated addresses
in cu->call_site_htab, for both the pc field:
...
  baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
  ...
  pc = attr_value_as_address (attr) + baseaddr;
  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
  ...
  call_site->pc = pc;
...
and the target field:
...
             lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
             SET_FIELD_PHYSADDR (call_site->target, lowpc);
...
but fails to do so because it is called before objfile_relocate, making
baseaddr 0.

Fix this by eliminating baseaddr from read_call_site_scope, and handling the
relocation offset at the use sites in call_site_for_pc and
call_site_to_target_addr.

Tested on x86_64-linux, both with and without -fPIE/-pie.

OK for trunk?

Thanks,
- Tom

[gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

gdb/ChangeLog:

2019-08-09  Tom de Vries  <[hidden email]>

        PR gdb/24892
        * block.c (call_site_for_pc): Substract relocation offset before
        finding pc in COMPUNIT_CALL_SITE_HTAB.
        * dwarf2loc.c (call_site_to_target_addr): Add relocation offset to
        FIELD_STATIC_PHYSADDR (call_site->target).
        * dwarf2read.c (read_call_site_scope): Eliminate baseaddr.

---
 gdb/block.c      | 17 ++++++++++++++++-
 gdb/dwarf2loc.c  | 12 +++++++++++-
 gdb/dwarf2read.c | 12 +++---------
 3 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/gdb/block.c b/gdb/block.c
index 5c6faa8504..eece1c5171 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -226,7 +226,22 @@ call_site_for_pc (struct gdbarch *gdbarch, CORE_ADDR pc)
   cust = find_pc_compunit_symtab (pc - 1);
 
   if (cust != NULL && COMPUNIT_CALL_SITE_HTAB (cust) != NULL)
-    slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust), &pc, NO_INSERT);
+    {
+      struct obj_section *sec;
+      sec = find_pc_section (pc);
+      if (sec != NULL)
+ {
+  struct objfile *objfile = sec->objfile;
+  CORE_ADDR baseaddr
+    = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+  CORE_ADDR pc_unrelocated
+    = gdbarch_adjust_dwarf2_addr (gdbarch, pc - baseaddr);
+
+  slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust),
+ &pc_unrelocated, NO_INSERT);
+ }
+    }
+
 
   if (slot == NULL)
     {
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 63643cb45d..885412f7ef 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -855,7 +855,17 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch,
       }
 
     case FIELD_LOC_KIND_PHYSADDR:
-      return FIELD_STATIC_PHYSADDR (call_site->target);
+      {
+ CORE_ADDR addr = FIELD_STATIC_PHYSADDR (call_site->target);
+ struct dwarf2_per_objfile *dwarf2_per_objfile
+  = call_site->per_cu->dwarf2_per_objfile;
+ struct objfile *objfile = dwarf2_per_objfile->objfile;
+ CORE_ADDR baseaddr
+  = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+ addr
+  = gdbarch_adjust_dwarf2_addr (call_site_gdbarch, addr + baseaddr);
+ return addr;
+      }
 
     default:
       internal_error (__FILE__, __LINE__, _("invalid call site target kind"));
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index de9755f6ce..a3ae7c93e2 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -13844,15 +13844,13 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
 {
   struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
-  CORE_ADDR pc, baseaddr;
+  CORE_ADDR pc;
   struct attribute *attr;
   struct call_site *call_site, call_site_local;
   void **slot;
   int nparams;
   struct die_info *child_die;
 
-  baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
-
   attr = dwarf2_attr (die, DW_AT_call_return_pc, cu);
   if (attr == NULL)
     {
@@ -13867,8 +13865,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
  sect_offset_str (die->sect_off), objfile_name (objfile));
       return;
     }
-  pc = attr_value_as_address (attr) + baseaddr;
-  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
+  pc = attr_value_as_address (attr);
 
   if (cu->call_site_htab == NULL)
     cu->call_site_htab = htab_create_alloc_ex (16, core_addr_hash, core_addr_eq,
@@ -14019,10 +14016,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
          "low pc, for referencing DIE %s [in module %s]"),
        sect_offset_str (die->sect_off), objfile_name (objfile));
   else
-    {
-      lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
-      SET_FIELD_PHYSADDR (call_site->target, lowpc);
-    }
+    SET_FIELD_PHYSADDR (call_site->target, lowpc);
  }
     }
   else
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

Jan Kratochvil-2
On Fri, 09 Aug 2019 09:54:26 +0200, Tom de Vries wrote:
> Fix this by eliminating baseaddr from read_call_site_scope, and handling the
> relocation offset at the use sites in call_site_for_pc and
> call_site_to_target_addr.

It sure looks correct to me that way, thanks.  This is not a commit approval.


Thanks,
Jan Kratochvil
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

Tom Tromey-2
In reply to this post by Tom de Vries
>>>>> "Tom" == Tom de Vries <[hidden email]> writes:

Tom> Fix this by eliminating baseaddr from read_call_site_scope, and handling the
Tom> relocation offset at the use sites in call_site_for_pc and
Tom> call_site_to_target_addr.

I like this approach, since it represents some small progress on the
objfile splitting project.

Tom> +    {
Tom> +      struct obj_section *sec;
Tom> +      sec = find_pc_section (pc);
Tom> +      if (sec != NULL)
Tom> + {
Tom> +  struct objfile *objfile = sec->objfile;
Tom> +  CORE_ADDR baseaddr
Tom> +    = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));

Why SECT_OFF_TEXT rather than the section "sec"?

Tom> +  CORE_ADDR pc_unrelocated
Tom> +    = gdbarch_adjust_dwarf2_addr (gdbarch, pc - baseaddr);

I am not sure gdbarch_adjust_dwarf2_addr can be used "bidirectionally"
like this.  It is probably fine in practice but I wonder about the
documented contract.

Tom> + CORE_ADDR baseaddr
Tom> +  = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));

I guess this assumes the text section - but then can the call to
find_pc_section give anything else?  Maybe it's just something to
comment and move on.

Tom> -  pc = attr_value_as_address (attr) + baseaddr;
Tom> -  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
Tom> +  pc = attr_value_as_address (attr);

The approach taken elsewhere in dwarf2read.c is to bias, adjust, then
unbias:

      CORE_ADDR low
        = (gdbarch_adjust_dwarf2_addr (gdbarch, best_lowpc + baseaddr)
           - baseaddr);

Maybe this would be better here as well, or at least consistent.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

Tom de Vries
On 09-08-19 20:16, Tom Tromey wrote:

>>>>>> "Tom" == Tom de Vries <[hidden email]> writes:
>
> Tom> Fix this by eliminating baseaddr from read_call_site_scope, and handling the
> Tom> relocation offset at the use sites in call_site_for_pc and
> Tom> call_site_to_target_addr.
>
> I like this approach, since it represents some small progress on the
> objfile splitting project.
>
> Tom> +    {
> Tom> +      struct obj_section *sec;
> Tom> +      sec = find_pc_section (pc);
> Tom> +      if (sec != NULL)
> Tom> + {
> Tom> +  struct objfile *objfile = sec->objfile;
> Tom> +  CORE_ADDR baseaddr
> Tom> +    = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>
> Why SECT_OFF_TEXT rather than the section "sec"?
>
Fixed by using obj_section_offset:
...
        CORE_ADDR pc_unrelocated = pc - obj_section_offset (sec);
...

> Tom> +  CORE_ADDR pc_unrelocated
> Tom> +    = gdbarch_adjust_dwarf2_addr (gdbarch, pc - baseaddr);
>
> I am not sure gdbarch_adjust_dwarf2_addr can be used "bidirectionally"
> like this.  It is probably fine in practice but I wonder about the
> documented contract.
>

Thanks for pointing that out. Hmm, it seems there is no documented
contract (as you mentioned here:
https://sourceware.org/ml/gdb-patches/2018-05/msg00074.html ).

But, I guess we assume relocated addresses as arg to
gdbarch_adjust_dwarf2_addr because mips_adjust_dwarf2_addr uses
mips_pc_is_mips which uses lookup_minimal_symbol_by_pc which uses
lookup_minimal_symbol_by_pc_section which uses frob_address to
de-relocate the used address before comparing it to the unrelocated
address in MSYMBOL_VALUE_RAW_ADDRESS.

> Tom> + CORE_ADDR baseaddr
> Tom> +  = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>
> I guess this assumes the text section - but then can the call to
> find_pc_section give anything else?  Maybe it's just something to
> comment and move on.
>

I suppose that find_pc_section also can return .init or .fini., but I
imagine these wil have the same sections offsets as .text.

> Tom> -  pc = attr_value_as_address (attr) + baseaddr;
> Tom> -  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
> Tom> +  pc = attr_value_as_address (attr);
>
> The approach taken elsewhere in dwarf2read.c is to bias, adjust, then
> unbias:
>
>       CORE_ADDR low
> = (gdbarch_adjust_dwarf2_addr (gdbarch, best_lowpc + baseaddr)
>   - baseaddr);
>
> Maybe this would be better here as well, or at least consistent.
Done. Though as pointed out in the rationale, baseaddr can be 0 here, so
effectively we pass at least sometimes unrelocated addresses into
gdbarch_adjust_dwarf2_addr. I'm not sure if frob_address is guaranteed
to be the identity function for unrelocated addresses.

Also, I've copied relocate_text_addr from
https://sourceware.org/ml/gdb-patches/2019-08/msg00241.html to use it in
call_site_to_target_addr.

OK like this?

Thanks,
- Tom

[gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

When running gdb.dwarf2/amd64-entry-value-param.exp with target board
unix/-fPIE/-pie, we get:
...
FAIL: gdb.arch/amd64-entry-value-param.exp: call 1: p y
...

The problem is that read_call_site_scope attempts to put relocated addresses
in cu->call_site_htab, for both the pc field:
...
  baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
  ...
  pc = attr_value_as_address (attr) + baseaddr;
  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
  ...
  call_site->pc = pc;
...
and the target field:
...
             lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
             SET_FIELD_PHYSADDR (call_site->target, lowpc);
...
but fails to do so because it is called before objfile_relocate, making
baseaddr 0.

Fix this by eliminating baseaddr from read_call_site_scope, and handling the
relocation offset at the use sites in call_site_for_pc and
call_site_to_target_addr.

Tested on x86_64-linux, both with and without -fPIE/-pie.

gdb/ChangeLog:

2019-08-09  Tom de Vries  <[hidden email]>

        PR gdb/24892
        * objfiles.c (relocate_text_addr): New function.
        * objfiles.h (relocate_text_addr): Declare.
        * block.c (call_site_for_pc): Substract relocation offset before
        finding pc in COMPUNIT_CALL_SITE_HTAB.
        * dwarf2loc.c (call_site_to_target_addr): Add relocation offset to
        FIELD_STATIC_PHYSADDR (call_site->target).
        * dwarf2read.c (read_call_site_scope): Eliminate baseaddr.

---
 gdb/block.c      | 12 +++++++++++-
 gdb/dwarf2loc.c  |  7 ++++++-
 gdb/dwarf2read.c |  7 ++-----
 gdb/objfiles.c   | 11 +++++++++++
 gdb/objfiles.h   |  5 +++++
 5 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/gdb/block.c b/gdb/block.c
index 5c6faa8504..3113980259 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -226,7 +226,17 @@ call_site_for_pc (struct gdbarch *gdbarch, CORE_ADDR pc)
   cust = find_pc_compunit_symtab (pc - 1);
 
   if (cust != NULL && COMPUNIT_CALL_SITE_HTAB (cust) != NULL)
-    slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust), &pc, NO_INSERT);
+    {
+      struct obj_section *sec;
+      sec = find_pc_section (pc);
+      if (sec != NULL)
+ {
+  CORE_ADDR pc_unrelocated = pc - obj_section_offset (sec);
+  slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust),
+ &pc_unrelocated, NO_INSERT);
+ }
+    }
+
 
   if (slot == NULL)
     {
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 63643cb45d..de8573f6d9 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -855,7 +855,12 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch,
       }
 
     case FIELD_LOC_KIND_PHYSADDR:
-      return FIELD_STATIC_PHYSADDR (call_site->target);
+      {
+ CORE_ADDR addr = FIELD_STATIC_PHYSADDR (call_site->target);
+ struct objfile *objfile
+  = call_site->per_cu->dwarf2_per_objfile->objfile;
+ return relocate_text_addr (addr, objfile);
+      }
 
     default:
       internal_error (__FILE__, __LINE__, _("invalid call site target kind"));
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index de9755f6ce..6a8218dc61 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -13868,7 +13868,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
       return;
     }
   pc = attr_value_as_address (attr) + baseaddr;
-  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
+  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc) - baseaddr;
 
   if (cu->call_site_htab == NULL)
     cu->call_site_htab = htab_create_alloc_ex (16, core_addr_hash, core_addr_eq,
@@ -14019,10 +14019,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
          "low pc, for referencing DIE %s [in module %s]"),
        sect_offset_str (die->sect_off), objfile_name (objfile));
   else
-    {
-      lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
-      SET_FIELD_PHYSADDR (call_site->target, lowpc);
-    }
+    SET_FIELD_PHYSADDR (call_site->target, lowpc);
  }
     }
   else
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 7cbcbbd01b..18d9540849 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -1493,3 +1493,14 @@ objfile_flavour_name (struct objfile *objfile)
     return bfd_flavour_name (bfd_get_flavour (objfile->obfd));
   return NULL;
 }
+
+/* See objfiles.h.  */
+
+CORE_ADDR
+relocate_text_addr (CORE_ADDR addr, struct objfile *objfile)
+{
+  CORE_ADDR baseaddr
+    = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+  struct gdbarch *gdbarch = get_objfile_arch (objfile);
+  return gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
+}
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 239aba2c2a..a86bf3b5f9 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -789,4 +789,9 @@ extern void objfile_register_static_link
 extern const struct dynamic_prop *objfile_lookup_static_link
   (struct objfile *objfile, const struct block *block);
 
+/* Given an unrelocated address ADDR belonging to the text section of OBJFILE,
+   return the relocated address.  */
+
+extern CORE_ADDR relocate_text_addr (CORE_ADDR addr, struct objfile *objfile);
+
 #endif /* !defined (OBJFILES_H) */
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

Pedro Alves-7
On 8/12/19 2:10 PM, Tom de Vries wrote:

>> Tom> + CORE_ADDR baseaddr
>> Tom> +  = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>>
>> I guess this assumes the text section - but then can the call to
>> find_pc_section give anything else?  Maybe it's just something to
>> comment and move on.
>>
> I suppose that find_pc_section also can return .init or .fini., but I
> imagine these wil have the same sections offsets as .text.
>

Hmm.  That'll usually be the case on GNU/Linux and other standard
operating systems, where you have a single text segment containing all sections.
But they might well not have the same offsets if you're debugging a relocatable
object, for example.  Some targets' shared libraries are relocatable objects
instead of fully linked binaries.  See "Library List Format" in the manual:

 ~~~
 For the common case of libraries that are fully linked binaries, the
 library should have a list of segments.  If the target supports
 dynamic linking of a relocatable object file, its library XML element
 should instead include a list of allocated sections.  The segment or
 section bases are start addresses, not relocation offsets; they do not
 depend on the library's link-time base addresses.
 ~~~

Linux kernel modules would be something like that too, I think.

If easy, it seems better to look up the section.

Thanks,
Pedro Alves
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

Tom de Vries
On 16-08-19 20:33, Pedro Alves wrote:

> On 8/12/19 2:10 PM, Tom de Vries wrote:
>>> Tom> + CORE_ADDR baseaddr
>>> Tom> +  = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>>>
>>> I guess this assumes the text section - but then can the call to
>>> find_pc_section give anything else?  Maybe it's just something to
>>> comment and move on.
>>>
>> I suppose that find_pc_section also can return .init or .fini., but I
>> imagine these wil have the same sections offsets as .text.
>>
>
> Hmm.  That'll usually be the case on GNU/Linux and other standard
> operating systems, where you have a single text segment containing all sections.
> But they might well not have the same offsets if you're debugging a relocatable
> object, for example.  Some targets' shared libraries are relocatable objects
> instead of fully linked binaries.  See "Library List Format" in the manual:
>
>  ~~~
>  For the common case of libraries that are fully linked binaries, the
>  library should have a list of segments.  If the target supports
>  dynamic linking of a relocatable object file, its library XML element
>  should instead include a list of allocated sections.  The segment or
>  section bases are start addresses, not relocation offsets; they do not
>  depend on the library's link-time base addresses.
>  ~~~
>
> Linux kernel modules would be something like that too, I think.
>
> If easy, it seems better to look up the section.
Done.

Updated patch OK for trunk?

Thanks,
- Tom


[gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

When running gdb.dwarf2/amd64-entry-value-param.exp with target board
unix/-fPIE/-pie, we get:
...
FAIL: gdb.arch/amd64-entry-value-param.exp: call 1: p y
...

The problem is that read_call_site_scope attempts to put relocated addresses
in cu->call_site_htab, for both the pc field:
...
  baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
  ...
  pc = attr_value_as_address (attr) + baseaddr;
  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
  ...
  call_site->pc = pc;
...
and the target field:
...
             lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
             SET_FIELD_PHYSADDR (call_site->target, lowpc);
...
but fails to do so because it is called before objfile_relocate, making
baseaddr 0.

Fix this by eliminating baseaddr from read_call_site_scope, and handling the
relocation offset at the use sites in call_site_for_pc and
call_site_to_target_addr.

Tested on x86_64-linux, both with and without -fPIE/-pie.

gdb/ChangeLog:

2019-08-09  Tom de Vries  <[hidden email]>

        PR gdb/24892
        * objfiles.c (find_objfile_section_for_unrelocated_addr)
        (relocate_pc_addr): New function.
        * objfiles.h (relocate_pc_addr): Declare.
        * block.c (call_site_for_pc): Substract relocation offset before
        finding pc in COMPUNIT_CALL_SITE_HTAB.
        * dwarf2loc.c (call_site_to_target_addr): Add relocation offset to
        FIELD_STATIC_PHYSADDR (call_site->target).
        * dwarf2read.c (read_call_site_scope): Eliminate baseaddr.

---
 gdb/block.c      | 12 +++++++++++-
 gdb/dwarf2loc.c  |  7 ++++++-
 gdb/dwarf2read.c |  7 ++-----
 gdb/objfiles.c   | 33 +++++++++++++++++++++++++++++++++
 gdb/objfiles.h   |  5 +++++
 5 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/gdb/block.c b/gdb/block.c
index 5c6faa8504..3113980259 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -226,7 +226,17 @@ call_site_for_pc (struct gdbarch *gdbarch, CORE_ADDR pc)
   cust = find_pc_compunit_symtab (pc - 1);
 
   if (cust != NULL && COMPUNIT_CALL_SITE_HTAB (cust) != NULL)
-    slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust), &pc, NO_INSERT);
+    {
+      struct obj_section *sec;
+      sec = find_pc_section (pc);
+      if (sec != NULL)
+ {
+  CORE_ADDR pc_unrelocated = pc - obj_section_offset (sec);
+  slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust),
+ &pc_unrelocated, NO_INSERT);
+ }
+    }
+
 
   if (slot == NULL)
     {
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 63643cb45d..c936630e0c 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -855,7 +855,12 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch,
       }
 
     case FIELD_LOC_KIND_PHYSADDR:
-      return FIELD_STATIC_PHYSADDR (call_site->target);
+      {
+ CORE_ADDR addr = FIELD_STATIC_PHYSADDR (call_site->target);
+ struct objfile *objfile
+  = call_site->per_cu->dwarf2_per_objfile->objfile;
+ return relocate_pc_addr (addr, objfile);
+      }
 
     default:
       internal_error (__FILE__, __LINE__, _("invalid call site target kind"));
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index de9755f6ce..6a8218dc61 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -13868,7 +13868,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
       return;
     }
   pc = attr_value_as_address (attr) + baseaddr;
-  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
+  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc) - baseaddr;
 
   if (cu->call_site_htab == NULL)
     cu->call_site_htab = htab_create_alloc_ex (16, core_addr_hash, core_addr_eq,
@@ -14019,10 +14019,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
          "low pc, for referencing DIE %s [in module %s]"),
        sect_offset_str (die->sect_off), objfile_name (objfile));
   else
-    {
-      lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
-      SET_FIELD_PHYSADDR (call_site->target, lowpc);
-    }
+    SET_FIELD_PHYSADDR (call_site->target, lowpc);
  }
     }
   else
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 7cbcbbd01b..5e1c49c658 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -1493,3 +1493,36 @@ objfile_flavour_name (struct objfile *objfile)
     return bfd_flavour_name (bfd_get_flavour (objfile->obfd));
   return NULL;
 }
+
+/* Return section of OBJFILE that contains unrelocated address ADDR.  */
+
+static struct obj_section *
+find_objfile_section_for_unrelocated_addr (struct objfile *objfile,
+   CORE_ADDR addr)
+{
+  struct obj_section *osect;
+
+  if (objfile == NULL)
+    return NULL;
+
+  ALL_OBJFILE_OSECTIONS (objfile, osect)
+    {
+      if (obj_section_addr (osect) - obj_section_offset (osect) <= addr
+  && addr < obj_section_endaddr (osect) - obj_section_offset (osect))
+ return osect;
+    }
+
+  return NULL;
+}
+
+/* See objfiles.h.  */
+
+CORE_ADDR
+relocate_pc_addr (CORE_ADDR addr, struct objfile *objfile)
+{
+  struct obj_section *sec
+    = find_objfile_section_for_unrelocated_addr (objfile, addr);
+  CORE_ADDR baseaddr = obj_section_offset (sec);
+  struct gdbarch *gdbarch = get_objfile_arch (objfile);
+  return gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
+}
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 239aba2c2a..d4a9726b73 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -789,4 +789,9 @@ extern void objfile_register_static_link
 extern const struct dynamic_prop *objfile_lookup_static_link
   (struct objfile *objfile, const struct block *block);
 
+/* Given an unrelocated pc address ADDR belonging to a section of OBJFILE,
+   return the relocated address.  */
+
+extern CORE_ADDR relocate_pc_addr (CORE_ADDR addr, struct objfile *objfile);
+
 #endif /* !defined (OBJFILES_H) */
Reply | Threaded
Open this post in threaded view
|

[PING][PATCH][gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

Tom de Vries
On 19-08-19 12:48, Tom de Vries wrote:

> On 16-08-19 20:33, Pedro Alves wrote:
>> On 8/12/19 2:10 PM, Tom de Vries wrote:
>>>> Tom> + CORE_ADDR baseaddr
>>>> Tom> +  = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>>>>
>>>> I guess this assumes the text section - but then can the call to
>>>> find_pc_section give anything else?  Maybe it's just something to
>>>> comment and move on.
>>>>
>>> I suppose that find_pc_section also can return .init or .fini., but I
>>> imagine these wil have the same sections offsets as .text.
>>>
>> Hmm.  That'll usually be the case on GNU/Linux and other standard
>> operating systems, where you have a single text segment containing all sections.
>> But they might well not have the same offsets if you're debugging a relocatable
>> object, for example.  Some targets' shared libraries are relocatable objects
>> instead of fully linked binaries.  See "Library List Format" in the manual:
>>
>>  ~~~
>>  For the common case of libraries that are fully linked binaries, the
>>  library should have a list of segments.  If the target supports
>>  dynamic linking of a relocatable object file, its library XML element
>>  should instead include a list of allocated sections.  The segment or
>>  section bases are start addresses, not relocation offsets; they do not
>>  depend on the library's link-time base addresses.
>>  ~~~
>>
>> Linux kernel modules would be something like that too, I think.
>>
>> If easy, it seems better to look up the section.
> Done.
>
> Updated patch OK for trunk?
>

Ping.

Thanks,
- Tom

> 0001-gdb-Fix-gdb.dwarf2-amd64-entry-value-param.exp-with-fPIE-pie.patch
>
> [gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie
>
> When running gdb.dwarf2/amd64-entry-value-param.exp with target board
> unix/-fPIE/-pie, we get:
> ...
> FAIL: gdb.arch/amd64-entry-value-param.exp: call 1: p y
> ...
>
> The problem is that read_call_site_scope attempts to put relocated addresses
> in cu->call_site_htab, for both the pc field:
> ...
>   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>   ...
>   pc = attr_value_as_address (attr) + baseaddr;
>   pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
>   ...
>   call_site->pc = pc;
> ...
> and the target field:
> ...
>              lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
>              SET_FIELD_PHYSADDR (call_site->target, lowpc);
> ...
> but fails to do so because it is called before objfile_relocate, making
> baseaddr 0.
>
> Fix this by eliminating baseaddr from read_call_site_scope, and handling the
> relocation offset at the use sites in call_site_for_pc and
> call_site_to_target_addr.
>
> Tested on x86_64-linux, both with and without -fPIE/-pie.
>
> gdb/ChangeLog:
>
> 2019-08-09  Tom de Vries  <[hidden email]>
>
> PR gdb/24892
> * objfiles.c (find_objfile_section_for_unrelocated_addr)
> (relocate_pc_addr): New function.
> * objfiles.h (relocate_pc_addr): Declare.
> * block.c (call_site_for_pc): Substract relocation offset before
> finding pc in COMPUNIT_CALL_SITE_HTAB.
> * dwarf2loc.c (call_site_to_target_addr): Add relocation offset to
> FIELD_STATIC_PHYSADDR (call_site->target).
> * dwarf2read.c (read_call_site_scope): Eliminate baseaddr.
>
> ---
>  gdb/block.c      | 12 +++++++++++-
>  gdb/dwarf2loc.c  |  7 ++++++-
>  gdb/dwarf2read.c |  7 ++-----
>  gdb/objfiles.c   | 33 +++++++++++++++++++++++++++++++++
>  gdb/objfiles.h   |  5 +++++
>  5 files changed, 57 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/block.c b/gdb/block.c
> index 5c6faa8504..3113980259 100644
> --- a/gdb/block.c
> +++ b/gdb/block.c
> @@ -226,7 +226,17 @@ call_site_for_pc (struct gdbarch *gdbarch, CORE_ADDR pc)
>    cust = find_pc_compunit_symtab (pc - 1);
>  
>    if (cust != NULL && COMPUNIT_CALL_SITE_HTAB (cust) != NULL)
> -    slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust), &pc, NO_INSERT);
> +    {
> +      struct obj_section *sec;
> +      sec = find_pc_section (pc);
> +      if (sec != NULL)
> + {
> +  CORE_ADDR pc_unrelocated = pc - obj_section_offset (sec);
> +  slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust),
> + &pc_unrelocated, NO_INSERT);
> + }
> +    }
> +
>  
>    if (slot == NULL)
>      {
> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index 63643cb45d..c936630e0c 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
> @@ -855,7 +855,12 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch,
>        }
>  
>      case FIELD_LOC_KIND_PHYSADDR:
> -      return FIELD_STATIC_PHYSADDR (call_site->target);
> +      {
> + CORE_ADDR addr = FIELD_STATIC_PHYSADDR (call_site->target);
> + struct objfile *objfile
> +  = call_site->per_cu->dwarf2_per_objfile->objfile;
> + return relocate_pc_addr (addr, objfile);
> +      }
>  
>      default:
>        internal_error (__FILE__, __LINE__, _("invalid call site target kind"));
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index de9755f6ce..6a8218dc61 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -13868,7 +13868,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
>        return;
>      }
>    pc = attr_value_as_address (attr) + baseaddr;
> -  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
> +  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc) - baseaddr;
>  
>    if (cu->call_site_htab == NULL)
>      cu->call_site_htab = htab_create_alloc_ex (16, core_addr_hash, core_addr_eq,
> @@ -14019,10 +14019,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
>           "low pc, for referencing DIE %s [in module %s]"),
>         sect_offset_str (die->sect_off), objfile_name (objfile));
>    else
> -    {
> -      lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
> -      SET_FIELD_PHYSADDR (call_site->target, lowpc);
> -    }
> +    SET_FIELD_PHYSADDR (call_site->target, lowpc);
>   }
>      }
>    else
> diff --git a/gdb/objfiles.c b/gdb/objfiles.c
> index 7cbcbbd01b..5e1c49c658 100644
> --- a/gdb/objfiles.c
> +++ b/gdb/objfiles.c
> @@ -1493,3 +1493,36 @@ objfile_flavour_name (struct objfile *objfile)
>      return bfd_flavour_name (bfd_get_flavour (objfile->obfd));
>    return NULL;
>  }
> +
> +/* Return section of OBJFILE that contains unrelocated address ADDR.  */
> +
> +static struct obj_section *
> +find_objfile_section_for_unrelocated_addr (struct objfile *objfile,
> +   CORE_ADDR addr)
> +{
> +  struct obj_section *osect;
> +
> +  if (objfile == NULL)
> +    return NULL;
> +
> +  ALL_OBJFILE_OSECTIONS (objfile, osect)
> +    {
> +      if (obj_section_addr (osect) - obj_section_offset (osect) <= addr
> +  && addr < obj_section_endaddr (osect) - obj_section_offset (osect))
> + return osect;
> +    }
> +
> +  return NULL;
> +}
> +
> +/* See objfiles.h.  */
> +
> +CORE_ADDR
> +relocate_pc_addr (CORE_ADDR addr, struct objfile *objfile)
> +{
> +  struct obj_section *sec
> +    = find_objfile_section_for_unrelocated_addr (objfile, addr);
> +  CORE_ADDR baseaddr = obj_section_offset (sec);
> +  struct gdbarch *gdbarch = get_objfile_arch (objfile);
> +  return gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
> +}
> diff --git a/gdb/objfiles.h b/gdb/objfiles.h
> index 239aba2c2a..d4a9726b73 100644
> --- a/gdb/objfiles.h
> +++ b/gdb/objfiles.h
> @@ -789,4 +789,9 @@ extern void objfile_register_static_link
>  extern const struct dynamic_prop *objfile_lookup_static_link
>    (struct objfile *objfile, const struct block *block);
>  
> +/* Given an unrelocated pc address ADDR belonging to a section of OBJFILE,
> +   return the relocated address.  */
> +
> +extern CORE_ADDR relocate_pc_addr (CORE_ADDR addr, struct objfile *objfile);
> +
>  #endif /* !defined (OBJFILES_H) */
>
Reply | Threaded
Open this post in threaded view
|

[PING^2][PATCH][gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

Tom de Vries
On 02-09-19 13:08, Tom de Vries wrote:

> On 19-08-19 12:48, Tom de Vries wrote:
>> On 16-08-19 20:33, Pedro Alves wrote:
>>> On 8/12/19 2:10 PM, Tom de Vries wrote:
>>>>> Tom> + CORE_ADDR baseaddr
>>>>> Tom> +  = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>>>>>
>>>>> I guess this assumes the text section - but then can the call to
>>>>> find_pc_section give anything else?  Maybe it's just something to
>>>>> comment and move on.
>>>>>
>>>> I suppose that find_pc_section also can return .init or .fini., but I
>>>> imagine these wil have the same sections offsets as .text.
>>>>
>>> Hmm.  That'll usually be the case on GNU/Linux and other standard
>>> operating systems, where you have a single text segment containing all sections.
>>> But they might well not have the same offsets if you're debugging a relocatable
>>> object, for example.  Some targets' shared libraries are relocatable objects
>>> instead of fully linked binaries.  See "Library List Format" in the manual:
>>>
>>>  ~~~
>>>  For the common case of libraries that are fully linked binaries, the
>>>  library should have a list of segments.  If the target supports
>>>  dynamic linking of a relocatable object file, its library XML element
>>>  should instead include a list of allocated sections.  The segment or
>>>  section bases are start addresses, not relocation offsets; they do not
>>>  depend on the library's link-time base addresses.
>>>  ~~~
>>>
>>> Linux kernel modules would be something like that too, I think.
>>>
>>> If easy, it seems better to look up the section.
>> Done.
>>
>> Updated patch OK for trunk?
>>
>

Ping^2.

Thanks,
- Tom

>> 0001-gdb-Fix-gdb.dwarf2-amd64-entry-value-param.exp-with-fPIE-pie.patch
>>
>> [gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie
>>
>> When running gdb.dwarf2/amd64-entry-value-param.exp with target board
>> unix/-fPIE/-pie, we get:
>> ...
>> FAIL: gdb.arch/amd64-entry-value-param.exp: call 1: p y
>> ...
>>
>> The problem is that read_call_site_scope attempts to put relocated addresses
>> in cu->call_site_htab, for both the pc field:
>> ...
>>   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>>   ...
>>   pc = attr_value_as_address (attr) + baseaddr;
>>   pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
>>   ...
>>   call_site->pc = pc;
>> ...
>> and the target field:
>> ...
>>              lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
>>              SET_FIELD_PHYSADDR (call_site->target, lowpc);
>> ...
>> but fails to do so because it is called before objfile_relocate, making
>> baseaddr 0.
>>
>> Fix this by eliminating baseaddr from read_call_site_scope, and handling the
>> relocation offset at the use sites in call_site_for_pc and
>> call_site_to_target_addr.
>>
>> Tested on x86_64-linux, both with and without -fPIE/-pie.
>>
>> gdb/ChangeLog:
>>
>> 2019-08-09  Tom de Vries  <[hidden email]>
>>
>> PR gdb/24892
>> * objfiles.c (find_objfile_section_for_unrelocated_addr)
>> (relocate_pc_addr): New function.
>> * objfiles.h (relocate_pc_addr): Declare.
>> * block.c (call_site_for_pc): Substract relocation offset before
>> finding pc in COMPUNIT_CALL_SITE_HTAB.
>> * dwarf2loc.c (call_site_to_target_addr): Add relocation offset to
>> FIELD_STATIC_PHYSADDR (call_site->target).
>> * dwarf2read.c (read_call_site_scope): Eliminate baseaddr.
>>
>> ---
>>  gdb/block.c      | 12 +++++++++++-
>>  gdb/dwarf2loc.c  |  7 ++++++-
>>  gdb/dwarf2read.c |  7 ++-----
>>  gdb/objfiles.c   | 33 +++++++++++++++++++++++++++++++++
>>  gdb/objfiles.h   |  5 +++++
>>  5 files changed, 57 insertions(+), 7 deletions(-)
>>
>> diff --git a/gdb/block.c b/gdb/block.c
>> index 5c6faa8504..3113980259 100644
>> --- a/gdb/block.c
>> +++ b/gdb/block.c
>> @@ -226,7 +226,17 @@ call_site_for_pc (struct gdbarch *gdbarch, CORE_ADDR pc)
>>    cust = find_pc_compunit_symtab (pc - 1);
>>  
>>    if (cust != NULL && COMPUNIT_CALL_SITE_HTAB (cust) != NULL)
>> -    slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust), &pc, NO_INSERT);
>> +    {
>> +      struct obj_section *sec;
>> +      sec = find_pc_section (pc);
>> +      if (sec != NULL)
>> + {
>> +  CORE_ADDR pc_unrelocated = pc - obj_section_offset (sec);
>> +  slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust),
>> + &pc_unrelocated, NO_INSERT);
>> + }
>> +    }
>> +
>>  
>>    if (slot == NULL)
>>      {
>> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
>> index 63643cb45d..c936630e0c 100644
>> --- a/gdb/dwarf2loc.c
>> +++ b/gdb/dwarf2loc.c
>> @@ -855,7 +855,12 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch,
>>        }
>>  
>>      case FIELD_LOC_KIND_PHYSADDR:
>> -      return FIELD_STATIC_PHYSADDR (call_site->target);
>> +      {
>> + CORE_ADDR addr = FIELD_STATIC_PHYSADDR (call_site->target);
>> + struct objfile *objfile
>> +  = call_site->per_cu->dwarf2_per_objfile->objfile;
>> + return relocate_pc_addr (addr, objfile);
>> +      }
>>  
>>      default:
>>        internal_error (__FILE__, __LINE__, _("invalid call site target kind"));
>> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
>> index de9755f6ce..6a8218dc61 100644
>> --- a/gdb/dwarf2read.c
>> +++ b/gdb/dwarf2read.c
>> @@ -13868,7 +13868,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
>>        return;
>>      }
>>    pc = attr_value_as_address (attr) + baseaddr;
>> -  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
>> +  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc) - baseaddr;
>>  
>>    if (cu->call_site_htab == NULL)
>>      cu->call_site_htab = htab_create_alloc_ex (16, core_addr_hash, core_addr_eq,
>> @@ -14019,10 +14019,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
>>           "low pc, for referencing DIE %s [in module %s]"),
>>         sect_offset_str (die->sect_off), objfile_name (objfile));
>>    else
>> -    {
>> -      lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
>> -      SET_FIELD_PHYSADDR (call_site->target, lowpc);
>> -    }
>> +    SET_FIELD_PHYSADDR (call_site->target, lowpc);
>>   }
>>      }
>>    else
>> diff --git a/gdb/objfiles.c b/gdb/objfiles.c
>> index 7cbcbbd01b..5e1c49c658 100644
>> --- a/gdb/objfiles.c
>> +++ b/gdb/objfiles.c
>> @@ -1493,3 +1493,36 @@ objfile_flavour_name (struct objfile *objfile)
>>      return bfd_flavour_name (bfd_get_flavour (objfile->obfd));
>>    return NULL;
>>  }
>> +
>> +/* Return section of OBJFILE that contains unrelocated address ADDR.  */
>> +
>> +static struct obj_section *
>> +find_objfile_section_for_unrelocated_addr (struct objfile *objfile,
>> +   CORE_ADDR addr)
>> +{
>> +  struct obj_section *osect;
>> +
>> +  if (objfile == NULL)
>> +    return NULL;
>> +
>> +  ALL_OBJFILE_OSECTIONS (objfile, osect)
>> +    {
>> +      if (obj_section_addr (osect) - obj_section_offset (osect) <= addr
>> +  && addr < obj_section_endaddr (osect) - obj_section_offset (osect))
>> + return osect;
>> +    }
>> +
>> +  return NULL;
>> +}
>> +
>> +/* See objfiles.h.  */
>> +
>> +CORE_ADDR
>> +relocate_pc_addr (CORE_ADDR addr, struct objfile *objfile)
>> +{
>> +  struct obj_section *sec
>> +    = find_objfile_section_for_unrelocated_addr (objfile, addr);
>> +  CORE_ADDR baseaddr = obj_section_offset (sec);
>> +  struct gdbarch *gdbarch = get_objfile_arch (objfile);
>> +  return gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
>> +}
>> diff --git a/gdb/objfiles.h b/gdb/objfiles.h
>> index 239aba2c2a..d4a9726b73 100644
>> --- a/gdb/objfiles.h
>> +++ b/gdb/objfiles.h
>> @@ -789,4 +789,9 @@ extern void objfile_register_static_link
>>  extern const struct dynamic_prop *objfile_lookup_static_link
>>    (struct objfile *objfile, const struct block *block);
>>  
>> +/* Given an unrelocated pc address ADDR belonging to a section of OBJFILE,
>> +   return the relocated address.  */
>> +
>> +extern CORE_ADDR relocate_pc_addr (CORE_ADDR addr, struct objfile *objfile);
>> +
>>  #endif /* !defined (OBJFILES_H) */
>>
Reply | Threaded
Open this post in threaded view
|

[PING^2][PATCH][gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie

Tom de Vries
In reply to this post by Tom de Vries
On 02-09-19 13:08, Tom de Vries wrote:

> On 19-08-19 12:48, Tom de Vries wrote:
>> On 16-08-19 20:33, Pedro Alves wrote:
>>> On 8/12/19 2:10 PM, Tom de Vries wrote:
>>>>> Tom> + CORE_ADDR baseaddr
>>>>> Tom> +  = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>>>>>
>>>>> I guess this assumes the text section - but then can the call to
>>>>> find_pc_section give anything else?  Maybe it's just something to
>>>>> comment and move on.
>>>>>
>>>> I suppose that find_pc_section also can return .init or .fini., but I
>>>> imagine these wil have the same sections offsets as .text.
>>>>
>>> Hmm.  That'll usually be the case on GNU/Linux and other standard
>>> operating systems, where you have a single text segment containing all sections.
>>> But they might well not have the same offsets if you're debugging a relocatable
>>> object, for example.  Some targets' shared libraries are relocatable objects
>>> instead of fully linked binaries.  See "Library List Format" in the manual:
>>>
>>>  ~~~
>>>  For the common case of libraries that are fully linked binaries, the
>>>  library should have a list of segments.  If the target supports
>>>  dynamic linking of a relocatable object file, its library XML element
>>>  should instead include a list of allocated sections.  The segment or
>>>  section bases are start addresses, not relocation offsets; they do not
>>>  depend on the library's link-time base addresses.
>>>  ~~~
>>>
>>> Linux kernel modules would be something like that too, I think.
>>>
>>> If easy, it seems better to look up the section.
>> Done.
>>
>> Updated patch OK for trunk?
>>
>

Ping^2.

Thanks,
- Tom

>> 0001-gdb-Fix-gdb.dwarf2-amd64-entry-value-param.exp-with-fPIE-pie.patch
>>
>> [gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie
>>
>> When running gdb.dwarf2/amd64-entry-value-param.exp with target board
>> unix/-fPIE/-pie, we get:
>> ...
>> FAIL: gdb.arch/amd64-entry-value-param.exp: call 1: p y
>> ...
>>
>> The problem is that read_call_site_scope attempts to put relocated addresses
>> in cu->call_site_htab, for both the pc field:
>> ...
>>   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>>   ...
>>   pc = attr_value_as_address (attr) + baseaddr;
>>   pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
>>   ...
>>   call_site->pc = pc;
>> ...
>> and the target field:
>> ...
>>              lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
>>              SET_FIELD_PHYSADDR (call_site->target, lowpc);
>> ...
>> but fails to do so because it is called before objfile_relocate, making
>> baseaddr 0.
>>
>> Fix this by eliminating baseaddr from read_call_site_scope, and handling the
>> relocation offset at the use sites in call_site_for_pc and
>> call_site_to_target_addr.
>>
>> Tested on x86_64-linux, both with and without -fPIE/-pie.
>>
>> gdb/ChangeLog:
>>
>> 2019-08-09  Tom de Vries  <[hidden email]>
>>
>> PR gdb/24892
>> * objfiles.c (find_objfile_section_for_unrelocated_addr)
>> (relocate_pc_addr): New function.
>> * objfiles.h (relocate_pc_addr): Declare.
>> * block.c (call_site_for_pc): Substract relocation offset before
>> finding pc in COMPUNIT_CALL_SITE_HTAB.
>> * dwarf2loc.c (call_site_to_target_addr): Add relocation offset to
>> FIELD_STATIC_PHYSADDR (call_site->target).
>> * dwarf2read.c (read_call_site_scope): Eliminate baseaddr.
>>
>> ---
>>  gdb/block.c      | 12 +++++++++++-
>>  gdb/dwarf2loc.c  |  7 ++++++-
>>  gdb/dwarf2read.c |  7 ++-----
>>  gdb/objfiles.c   | 33 +++++++++++++++++++++++++++++++++
>>  gdb/objfiles.h   |  5 +++++
>>  5 files changed, 57 insertions(+), 7 deletions(-)
>>
>> diff --git a/gdb/block.c b/gdb/block.c
>> index 5c6faa8504..3113980259 100644
>> --- a/gdb/block.c
>> +++ b/gdb/block.c
>> @@ -226,7 +226,17 @@ call_site_for_pc (struct gdbarch *gdbarch, CORE_ADDR pc)
>>    cust = find_pc_compunit_symtab (pc - 1);
>>  
>>    if (cust != NULL && COMPUNIT_CALL_SITE_HTAB (cust) != NULL)
>> -    slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust), &pc, NO_INSERT);
>> +    {
>> +      struct obj_section *sec;
>> +      sec = find_pc_section (pc);
>> +      if (sec != NULL)
>> + {
>> +  CORE_ADDR pc_unrelocated = pc - obj_section_offset (sec);
>> +  slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust),
>> + &pc_unrelocated, NO_INSERT);
>> + }
>> +    }
>> +
>>  
>>    if (slot == NULL)
>>      {
>> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
>> index 63643cb45d..c936630e0c 100644
>> --- a/gdb/dwarf2loc.c
>> +++ b/gdb/dwarf2loc.c
>> @@ -855,7 +855,12 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch,
>>        }
>>  
>>      case FIELD_LOC_KIND_PHYSADDR:
>> -      return FIELD_STATIC_PHYSADDR (call_site->target);
>> +      {
>> + CORE_ADDR addr = FIELD_STATIC_PHYSADDR (call_site->target);
>> + struct objfile *objfile
>> +  = call_site->per_cu->dwarf2_per_objfile->objfile;
>> + return relocate_pc_addr (addr, objfile);
>> +      }
>>  
>>      default:
>>        internal_error (__FILE__, __LINE__, _("invalid call site target kind"));
>> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
>> index de9755f6ce..6a8218dc61 100644
>> --- a/gdb/dwarf2read.c
>> +++ b/gdb/dwarf2read.c
>> @@ -13868,7 +13868,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
>>        return;
>>      }
>>    pc = attr_value_as_address (attr) + baseaddr;
>> -  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
>> +  pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc) - baseaddr;
>>  
>>    if (cu->call_site_htab == NULL)
>>      cu->call_site_htab = htab_create_alloc_ex (16, core_addr_hash, core_addr_eq,
>> @@ -14019,10 +14019,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
>>           "low pc, for referencing DIE %s [in module %s]"),
>>         sect_offset_str (die->sect_off), objfile_name (objfile));
>>    else
>> -    {
>> -      lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
>> -      SET_FIELD_PHYSADDR (call_site->target, lowpc);
>> -    }
>> +    SET_FIELD_PHYSADDR (call_site->target, lowpc);
>>   }
>>      }
>>    else
>> diff --git a/gdb/objfiles.c b/gdb/objfiles.c
>> index 7cbcbbd01b..5e1c49c658 100644
>> --- a/gdb/objfiles.c
>> +++ b/gdb/objfiles.c
>> @@ -1493,3 +1493,36 @@ objfile_flavour_name (struct objfile *objfile)
>>      return bfd_flavour_name (bfd_get_flavour (objfile->obfd));
>>    return NULL;
>>  }
>> +
>> +/* Return section of OBJFILE that contains unrelocated address ADDR.  */
>> +
>> +static struct obj_section *
>> +find_objfile_section_for_unrelocated_addr (struct objfile *objfile,
>> +   CORE_ADDR addr)
>> +{
>> +  struct obj_section *osect;
>> +
>> +  if (objfile == NULL)
>> +    return NULL;
>> +
>> +  ALL_OBJFILE_OSECTIONS (objfile, osect)
>> +    {
>> +      if (obj_section_addr (osect) - obj_section_offset (osect) <= addr
>> +  && addr < obj_section_endaddr (osect) - obj_section_offset (osect))
>> + return osect;
>> +    }
>> +
>> +  return NULL;
>> +}
>> +
>> +/* See objfiles.h.  */
>> +
>> +CORE_ADDR
>> +relocate_pc_addr (CORE_ADDR addr, struct objfile *objfile)
>> +{
>> +  struct obj_section *sec
>> +    = find_objfile_section_for_unrelocated_addr (objfile, addr);
>> +  CORE_ADDR baseaddr = obj_section_offset (sec);
>> +  struct gdbarch *gdbarch = get_objfile_arch (objfile);
>> +  return gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
>> +}
>> diff --git a/gdb/objfiles.h b/gdb/objfiles.h
>> index 239aba2c2a..d4a9726b73 100644
>> --- a/gdb/objfiles.h
>> +++ b/gdb/objfiles.h
>> @@ -789,4 +789,9 @@ extern void objfile_register_static_link
>>  extern const struct dynamic_prop *objfile_lookup_static_link
>>    (struct objfile *objfile, const struct block *block);
>>  
>> +/* Given an unrelocated pc address ADDR belonging to a section of OBJFILE,
>> +   return the relocated address.  */
>> +
>> +extern CORE_ADDR relocate_pc_addr (CORE_ADDR addr, struct objfile *objfile);
>> +
>>  #endif /* !defined (OBJFILES_H) */
>>