From patchwork Mon Aug 12 13:10:29 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: 34057 Received: (qmail 75322 invoked by alias); 12 Aug 2019 13:10:35 -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 75308 invoked by uid 89); 12 Aug 2019 13:10:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_STOCKGEN, SPF_PASS autolearn=ham version=3.3.1 spammy=wil, practice, gdbpatches, gdb-patches 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; Mon, 12 Aug 2019 13:10:33 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 52B9DB0CC; Mon, 12 Aug 2019 13:10:31 +0000 (UTC) Subject: Re: [PATCH][gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie To: Tom Tromey Cc: gdb-patches@sourceware.org, Jan Kratochvil References: <20190809075424.GA15972@delia> <87h86q4324.fsf@tromey.com> From: Tom de Vries Openpgp: preference=signencrypt Message-ID: Date: Mon, 12 Aug 2019 15:10:29 +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: <87h86q4324.fsf@tromey.com> X-IsSubscribed: yes On 09-08-19 20:16, Tom Tromey wrote: >>>>>> "Tom" == Tom de Vries writes: > > Tom> Fix this by eliminating baseaddr from read_call_site_scope, and handling the > Tom> relocation offset at the use sites in call_site_for_pc and > Tom> call_site_to_target_addr. > > I like this approach, since it represents some small progress on the > objfile splitting project. > > Tom> + { > Tom> + struct obj_section *sec; > Tom> + sec = find_pc_section (pc); > Tom> + if (sec != NULL) > Tom> + { > Tom> + struct objfile *objfile = sec->objfile; > Tom> + CORE_ADDR baseaddr > Tom> + = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)); > > Why SECT_OFF_TEXT rather than the section "sec"? > Fixed by using obj_section_offset: ... CORE_ADDR pc_unrelocated = pc - obj_section_offset (sec); ... > Tom> + CORE_ADDR pc_unrelocated > Tom> + = gdbarch_adjust_dwarf2_addr (gdbarch, pc - baseaddr); > > I am not sure gdbarch_adjust_dwarf2_addr can be used "bidirectionally" > like this. It is probably fine in practice but I wonder about the > documented contract. > Thanks for pointing that out. Hmm, it seems there is no documented contract (as you mentioned here: https://sourceware.org/ml/gdb-patches/2018-05/msg00074.html ). But, I guess we assume relocated addresses as arg to gdbarch_adjust_dwarf2_addr because mips_adjust_dwarf2_addr uses mips_pc_is_mips which uses lookup_minimal_symbol_by_pc which uses lookup_minimal_symbol_by_pc_section which uses frob_address to de-relocate the used address before comparing it to the unrelocated address in MSYMBOL_VALUE_RAW_ADDRESS. > Tom> + CORE_ADDR baseaddr > Tom> + = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)); > > I guess this assumes the text section - but then can the call to > find_pc_section give anything else? Maybe it's just something to > comment and move on. > I suppose that find_pc_section also can return .init or .fini., but I imagine these wil have the same sections offsets as .text. > Tom> - pc = attr_value_as_address (attr) + baseaddr; > Tom> - pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc); > Tom> + pc = attr_value_as_address (attr); > > The approach taken elsewhere in dwarf2read.c is to bias, adjust, then > unbias: > > CORE_ADDR low > = (gdbarch_adjust_dwarf2_addr (gdbarch, best_lowpc + baseaddr) > - baseaddr); > > Maybe this would be better here as well, or at least consistent. Done. Though as pointed out in the rationale, baseaddr can be 0 here, so effectively we pass at least sometimes unrelocated addresses into gdbarch_adjust_dwarf2_addr. I'm not sure if frob_address is guaranteed to be the identity function for unrelocated addresses. Also, I've copied relocate_text_addr from https://sourceware.org/ml/gdb-patches/2019-08/msg00241.html to use it in call_site_to_target_addr. OK like this? Thanks, - Tom [gdb] Fix gdb.dwarf2/amd64-entry-value-param.exp with -fPIE/-pie When running gdb.dwarf2/amd64-entry-value-param.exp with target board unix/-fPIE/-pie, we get: ... FAIL: gdb.arch/amd64-entry-value-param.exp: call 1: p y ... The problem is that read_call_site_scope attempts to put relocated addresses in cu->call_site_htab, for both the pc field: ... baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)); ... pc = attr_value_as_address (attr) + baseaddr; pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc); ... call_site->pc = pc; ... and the target field: ... lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr); SET_FIELD_PHYSADDR (call_site->target, lowpc); ... but fails to do so because it is called before objfile_relocate, making baseaddr 0. Fix this by eliminating baseaddr from read_call_site_scope, and handling the relocation offset at the use sites in call_site_for_pc and call_site_to_target_addr. Tested on x86_64-linux, both with and without -fPIE/-pie. gdb/ChangeLog: 2019-08-09 Tom de Vries PR gdb/24892 * objfiles.c (relocate_text_addr): New function. * objfiles.h (relocate_text_addr): Declare. * block.c (call_site_for_pc): Substract relocation offset before finding pc in COMPUNIT_CALL_SITE_HTAB. * dwarf2loc.c (call_site_to_target_addr): Add relocation offset to FIELD_STATIC_PHYSADDR (call_site->target). * dwarf2read.c (read_call_site_scope): Eliminate baseaddr. --- gdb/block.c | 12 +++++++++++- gdb/dwarf2loc.c | 7 ++++++- gdb/dwarf2read.c | 7 ++----- gdb/objfiles.c | 11 +++++++++++ gdb/objfiles.h | 5 +++++ 5 files changed, 35 insertions(+), 7 deletions(-) diff --git a/gdb/block.c b/gdb/block.c index 5c6faa8504..3113980259 100644 --- a/gdb/block.c +++ b/gdb/block.c @@ -226,7 +226,17 @@ call_site_for_pc (struct gdbarch *gdbarch, CORE_ADDR pc) cust = find_pc_compunit_symtab (pc - 1); if (cust != NULL && COMPUNIT_CALL_SITE_HTAB (cust) != NULL) - slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust), &pc, NO_INSERT); + { + struct obj_section *sec; + sec = find_pc_section (pc); + if (sec != NULL) + { + CORE_ADDR pc_unrelocated = pc - obj_section_offset (sec); + slot = htab_find_slot (COMPUNIT_CALL_SITE_HTAB (cust), + &pc_unrelocated, NO_INSERT); + } + } + if (slot == NULL) { diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c index 63643cb45d..de8573f6d9 100644 --- a/gdb/dwarf2loc.c +++ b/gdb/dwarf2loc.c @@ -855,7 +855,12 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch, } case FIELD_LOC_KIND_PHYSADDR: - return FIELD_STATIC_PHYSADDR (call_site->target); + { + CORE_ADDR addr = FIELD_STATIC_PHYSADDR (call_site->target); + struct objfile *objfile + = call_site->per_cu->dwarf2_per_objfile->objfile; + return relocate_text_addr (addr, objfile); + } default: internal_error (__FILE__, __LINE__, _("invalid call site target kind")); diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index de9755f6ce..6a8218dc61 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -13868,7 +13868,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu) return; } pc = attr_value_as_address (attr) + baseaddr; - pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc); + pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc) - baseaddr; if (cu->call_site_htab == NULL) cu->call_site_htab = htab_create_alloc_ex (16, core_addr_hash, core_addr_eq, @@ -14019,10 +14019,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu) "low pc, for referencing DIE %s [in module %s]"), sect_offset_str (die->sect_off), objfile_name (objfile)); else - { - lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr); - SET_FIELD_PHYSADDR (call_site->target, lowpc); - } + SET_FIELD_PHYSADDR (call_site->target, lowpc); } } else 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) */