Fix gdb.arch/amd64-tailcall-*.exp with -fPIE/-pie
Commit Message
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
Comments
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
[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(-)
@@ -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;
}