This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
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) */