Patchwork Fix gdb.arch/amd64-tailcall-*.exp with -fPIE/-pie

login
register
mail settings
Submitter Tom de Vries
Date Aug. 9, 2019, 3:03 p.m.
Message ID <38825791-ad92-3f7e-d3ae-2ac123dd6422@suse.de>
Download mbox | patch
Permalink /patch/34017/
State New
Headers show

Comments

Tom de Vries - Aug. 9, 2019, 3:03 p.m.
On 09-08-19 15:57, Pedro Alves wrote:
> On 8/9/19 11:48 AM, Tom de Vries wrote:
>> @@ -240,14 +242,38 @@ 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];
>> +      CORE_ADDR addr = call_site->pc;
>> +      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));
>> +       struct gdbarch *gdbarch = get_objfile_arch (objfile);
>> +       addr = gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
>> +      return addr;
>> +    }
>>    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];
>> +	  CORE_ADDR addr = call_site->pc;
>> +	  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));
>> +	  struct gdbarch *gdbarch = get_objfile_arch (objfile);
>> +	  addr = gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
>> +	  return addr;
>> +	}
> 
> It seems like there's a repeated pattern here.  How about
> factoring out the common code to a function?

Indeed. How about this?

Thanks,
- Tom
Pedro Alves - Aug. 9, 2019, 5:38 p.m.
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);

> +}
> +
>  /* 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?

Thanks,
Pedro Alves

Patch

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

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

---
 gdb/dwarf2-frame-tailcall.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2-frame-tailcall.c b/gdb/dwarf2-frame-tailcall.c
index e8f5aaf9c7..2cd2d532f9 100644
--- a/gdb/dwarf2-frame-tailcall.c
+++ b/gdb/dwarf2-frame-tailcall.c
@@ -29,6 +29,8 @@ 
 #include "value.h"
 #include "dwarf2-frame.h"
 #include "gdbarch.h"
+#include "dwarf2read.h"
+#include "objfiles.h"
 
 /* Contains struct tailcall_cache indexed by next_bottom_frame.  */
 static htab_t cache_htab;
@@ -225,6 +227,19 @@  tailcall_frame_this_id (struct frame_info *this_frame, void **this_cache,
   gdb_assert ((*this_id).artificial_depth > 0);
 }
 
+/* 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;
+}
+
 /* 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);
+	}
       next_levels -= chain->callers;
     }