This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][gdb] Fix gdb.arch/amd64-tailcall-*.exp with -fPIE/-pie


On 09-08-19 19:38, Pedro Alves wrote:
> On 8/9/19 4:03 PM, Tom de Vries wrote:
> 
>>  
>> +/* Given an unrelocated address ADDR belonging to the text section of OBJFILE,
>> +   return the relocated address.  */
>> +
>> +static 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);
>> +  addr = gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
>> +  return addr;
> 
> I'd write:
> 
>  return gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
> 

Updated accordingly.

>> +}
>> +
>>  /* Find PC to be unwound from THIS_FRAME.  THIS_FRAME must be a part of
>>     CACHE.  */
>>  
>> @@ -240,14 +255,25 @@ pretend_pc (struct frame_info *this_frame, struct tailcall_cache *cache)
>>    gdb_assert (next_levels >= 0);
>>  
>>    if (next_levels < chain->callees)
>> -    return chain->call_site[chain->length - next_levels - 1]->pc;
>> +    {
>> +      struct call_site *call_site
>> +	= chain->call_site[chain->length - next_levels - 1];
>> +      struct objfile *objfile = call_site->per_cu->dwarf2_per_objfile->objfile;
>> +      return relocate_text_addr (call_site->pc, objfile);
>> +    }
>>    next_levels -= chain->callees;
>>  
>>    /* Otherwise CHAIN->CALLEES are already covered by CHAIN->CALLERS.  */
>>    if (chain->callees != chain->length)
>>      {
>>        if (next_levels < chain->callers)
>> -	return chain->call_site[chain->callers - next_levels - 1]->pc;
>> +	{
>> +	  struct call_site *call_site
>> +	    = chain->call_site[chain->callers - next_levels - 1];
>> +	  struct objfile *objfile
>> +	    = call_site->per_cu->dwarf2_per_objfile->objfile;
>> +	  return relocate_text_addr (call_site->pc, objfile);
>> +	}
> 
> That seems fine, but it seems you could have factored out more, like:
> 
> static CORE_ADDR
> call_site_relocated_pc (struct call_site *call_site)
> {
>   struct objfile *objfile
>     = call_site->per_cu->dwarf2_per_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, call_size->pc + baseaddr);
> }
> 
> Then the other hunks would look like:
> 
>   struct call_site *call_site
>     = chain->call_site[chain->length - next_levels - 1];
>   return call_site_relocated_pc (call_site);
> 
> ...
> 
>   struct call_site *call_site
>     = chain->call_site[chain->callers - next_levels - 1];
>   return call_site_relocated_pc (call_site);
> 
> call_site_relocated_pc could even be a method of struct call_site, I guess,
> so you'd write:
> 
>   return call_site->relocated_pc ();
> 
> Any reason you didn't do something like that?

I went for a utility function that can be maximally reused, as opposed
to a function that maximally factors out the code in this particular
patch. But we can do both, so this patch adds:
- relocate_text_addr (I've moved it to objfiles.h/objfiles.c)
- call_site::relocated_pc

[ BTW, I should mention that this fix is dependent on the patch "[gdb]
Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie" (
https://sourceware.org/ml/gdb-patches/2019-08/msg00215.html ). ]

OK like this?

Thanks,
- Tom
[gdb] Fix gdb.arch/amd64-tailcall-*.exp with -fPIE/-pie

When running gdb.arch/amd64-tailcall-*.exp with target board
unix/-fPIE/-pie, we get:
...
FAIL: gdb.arch/amd64-tailcall-cxx.exp: bt
FAIL: gdb.arch/amd64-tailcall-noret.exp: bt
FAIL: gdb.arch/amd64-tailcall-self.exp: bt
...

The first FAIL in more detail, compared to a nopie run:
...
(gdb) bt
 #0  b ()
-#1  0x00000000004004df in a (q=<optimized out>)
+#1  0x0000000000000672 in ?? ()
-#2  0x00000000004003d5 in main (argc=<optimized out>, argv=<optimized out>)
+#2  0x0000555555554535 in main (argc=<optimized out>, argv=<optimized out>)
-(gdb) PASS: gdb.arch/amd64-tailcall-self.exp: bt
+(gdb) FAIL: gdb.arch/amd64-tailcall-self.exp: bt
...
shows an unrelocated address for function a.

The problem is that pretend_pc uses the pc field from a struct call_site
without adding the missing relocation offset.

Fix this by adding the appropriate offset.

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

gdb/ChangeLog:

2019-08-09  Tom de Vries  <tdevries@suse.de>

	* gdbtypes.c (call_site::relocated_pc): New function.
	* gdbtypes.h (call_site::relocated_pc): Declare.
	* objfiles.c (relocate_text_addr): New function.
	* objfiles.h (relocate_text_addr): Declare.
	* dwarf2-frame-tailcall.c (pretend_pc): Add relocation offset to pc
	field of struct call_site.

---
 gdb/dwarf2-frame-tailcall.c | 12 ++++++++++--
 gdb/gdbtypes.c              |  8 ++++++++
 gdb/gdbtypes.h              |  4 ++++
 gdb/objfiles.c              | 11 +++++++++++
 gdb/objfiles.h              |  5 +++++
 5 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2-frame-tailcall.c b/gdb/dwarf2-frame-tailcall.c
index e8f5aaf9c7..d2c9a813f2 100644
--- a/gdb/dwarf2-frame-tailcall.c
+++ b/gdb/dwarf2-frame-tailcall.c
@@ -240,14 +240,22 @@ pretend_pc (struct frame_info *this_frame, struct tailcall_cache *cache)
   gdb_assert (next_levels >= 0);
 
   if (next_levels < chain->callees)
-    return chain->call_site[chain->length - next_levels - 1]->pc;
+    {
+      struct call_site *call_site
+	= chain->call_site[chain->length - next_levels - 1];
+      return call_site->relocated_pc ();
+    }
   next_levels -= chain->callees;
 
   /* Otherwise CHAIN->CALLEES are already covered by CHAIN->CALLERS.  */
   if (chain->callees != chain->length)
     {
       if (next_levels < chain->callers)
-	return chain->call_site[chain->callers - next_levels - 1]->pc;
+	{
+	  struct call_site *call_site
+	    = chain->call_site[chain->callers - next_levels - 1];
+	  return call_site->relocated_pc ();
+	}
       next_levels -= chain->callers;
     }
 
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 177455e612..7466300572 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -39,6 +39,7 @@
 #include "dwarf2loc.h"
 #include "gdbcore.h"
 #include "floatformat.h"
+#include "dwarf2read.h"
 
 /* Initialize BADNESS constants.  */
 
@@ -5614,3 +5615,10 @@ _initialize_gdbtypes (void)
 			   show_strict_type_checking,
 			   &setchecklist, &showchecklist);
 }
+
+/* See gdbtypes.h.  */
+
+CORE_ADDR call_site::relocated_pc ()
+{
+  return relocate_text_addr (pc, per_cu->dwarf2_per_objfile->objfile);
+}
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 7268d3e4aa..d25398e7c4 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1238,6 +1238,10 @@ struct call_site
     /* * Describe DW_TAG_call_site's DW_TAG_formal_parameter.  */
 
     struct call_site_parameter parameter[1];
+
+    /* Return a relocated call_site->pc.  */
+
+    CORE_ADDR relocated_pc ();
   };
 
 /* * The default value of TYPE_CPLUS_SPECIFIC(T) points to this shared
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) */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]