From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92533 invoked by alias); 9 Aug 2019 17:38:35 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 92505 invoked by uid 89); 9 Aug 2019 17:38:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy= X-HELO: mail-wm1-f67.google.com Received: from mail-wm1-f67.google.com (HELO mail-wm1-f67.google.com) (209.85.128.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Aug 2019 17:38:31 +0000 Received: by mail-wm1-f67.google.com with SMTP id v15so6493559wml.0 for ; Fri, 09 Aug 2019 10:38:31 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id y2sm86247404wrl.4.2019.08.09.10.38.29 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Fri, 09 Aug 2019 10:38:29 -0700 (PDT) Subject: Re: [PATCH][gdb] Fix gdb.arch/amd64-tailcall-*.exp with -fPIE/-pie To: Tom de Vries , gdb-patches@sourceware.org References: <20190809104848.GA6563@delia> <57ada901-a8d8-b632-f7d8-e42283314b5a@redhat.com> <38825791-ad92-3f7e-d3ae-2ac123dd6422@suse.de> Cc: Jan Kratochvil From: Pedro Alves Message-ID: Date: Fri, 09 Aug 2019 17:38:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <38825791-ad92-3f7e-d3ae-2ac123dd6422@suse.de> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-08/txt/msg00233.txt.bz2 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