From patchwork Sat Aug 10 05:44:06 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 34052 Received: (qmail 108852 invoked by alias); 10 Aug 2019 05:44:12 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 108843 invoked by uid 89); 10 Aug 2019 05:44:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 10 Aug 2019 05:44:10 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 55E60B02A; Sat, 10 Aug 2019 05:44:07 +0000 (UTC) Subject: Re: [PATCH][gdb] Fix gdb.arch/amd64-tailcall-*.exp with -fPIE/-pie To: Pedro Alves , gdb-patches@sourceware.org Cc: Jan Kratochvil References: <20190809104848.GA6563@delia> <57ada901-a8d8-b632-f7d8-e42283314b5a@redhat.com> <38825791-ad92-3f7e-d3ae-2ac123dd6422@suse.de> From: Tom de Vries Openpgp: preference=signencrypt Message-ID: Date: Sat, 10 Aug 2019 07:44:06 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: X-IsSubscribed: yes 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=) +#1 0x0000000000000672 in ?? () -#2 0x00000000004003d5 in main (argc=, argv=) +#2 0x0000555555554535 in main (argc=, argv=) -(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 * 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) */