From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 102801 invoked by alias); 24 Mar 2015 05:38:51 -0000 Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org Received: (qmail 102790 invoked by uid 89); 24 Mar 2015 05:38:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD autolearn=unavailable version=3.3.2 X-HELO: e28smtp07.in.ibm.com Received: from e28smtp07.in.ibm.com (HELO e28smtp07.in.ibm.com) (122.248.162.7) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 24 Mar 2015 05:38:48 +0000 Received: from /spool/local by e28smtp07.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 24 Mar 2015 11:08:41 +0530 Received: from d28dlp01.in.ibm.com (9.184.220.126) by e28smtp07.in.ibm.com (192.168.1.137) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 24 Mar 2015 11:08:39 +0530 Received: from d28relay01.in.ibm.com (d28relay01.in.ibm.com [9.184.220.58]) by d28dlp01.in.ibm.com (Postfix) with ESMTP id A7E86E0054 for ; Tue, 24 Mar 2015 11:10:50 +0530 (IST) Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay01.in.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t2O5cbol20185112 for ; Tue, 24 Mar 2015 11:08:37 +0530 Received: from d28av03.in.ibm.com (localhost [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t2O5VdUF010251 for ; Tue, 24 Mar 2015 11:01:40 +0530 Received: from localhost.localdomain ([9.79.177.93]) by d28av03.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id t2O5Vc1k010200; Tue, 24 Mar 2015 11:01:38 +0530 Message-ID: <5510F85A.6030503@linux.vnet.ibm.com> Date: Tue, 24 Mar 2015 05:38:00 -0000 From: Hemant Kumar User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Mark Wielaard CC: systemtap@sourceware.org, uweigand@gcc.gnu.org, ulrich.weigand@de.ibm.com, fche@redhat.com, anton@samba.org, naveen.n.rao@linux.vnet.ibm.com Subject: Re: [PATCH v2] Fix PPC64 ELF ABI v2 symbol address retrieval References: <20150206063347.8998.37062.stgit@hemant-fedora> <1423854830.7128.14.camel@bordewijk.wildebeest.org> <1427117123.3077.33.camel@bordewijk.wildebeest.org> In-Reply-To: <1427117123.3077.33.camel@bordewijk.wildebeest.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15032405-0025-0000-0000-000003ED18EF X-IsSubscribed: yes X-SW-Source: 2015-q1/txt/msg00254.txt.bz2 On 03/23/2015 06:55 PM, Mark Wielaard wrote: > It seems important to get this cleaned up. > Does the review below make sense? I replied to this mail sometime back! Probably my mail client's fault. Working on your review comments. See my replies below. > On Fri, 2015-02-13 at 20:13 +0100, Mark Wielaard wrote: >> On Fri, 2015-02-06 at 12:04 +0530, Hemant Kumar wrote: >>> @@ -2099,7 +2106,19 @@ query_dwarf_func (Dwarf_Die * func, dwarf_query * q) >>> q->dw.function_line (&func.decl_line); >>> >>> Dwarf_Addr entrypc; >>> - if (q->dw.function_entrypc (&entrypc)) >>> + func.entrypc = 0; >>> + /* Giving priority to sym_table */ >>> + if (q->dw.mod_info->sym_table) >>> + { >>> + func_info * fi; >>> + fi = q->dw.mod_info->sym_table->lookup_symbol(func.name); >>> + if (fi) >>> + { >>> + func.entrypc = fi->addr; >>> + q->filtered_functions.push_back(func); >>> + } >>> + } >>> + if (!func.entrypc && q->dw.function_entrypc (&entrypc)) >>> { >>> func.entrypc = entrypc; >>> q->filtered_functions.push_back (func); >> I am still concerned about this lookup overriding the normal >> function_entrypc. It would be good to guard this with a check to see if >> it is necessary (only for a ppc64le target) so no extra lookups are >> necessary in the normal case. But I believe in general it is wrong since >> it only does the name lookup, and doesn't try to see if the address >> matched. Take for example this program: >> >> :::::::::::::: >> /tmp/baz.c >> :::::::::::::: >> static int >> foo (int v) >> { >> return v + 1; >> } >> >> int >> bar (int i) >> { >> return foo (i - 1); >> } >> >> :::::::::::::: >> /tmp/main.c >> :::::::::::::: >> int foo (int v) >> { >> return bar (v - 1); >> } >> >> int >> main (int argc, char **argv) >> { >> return foo (argc); >> } >> >> gcc -g -c baz.c >> gcc -g -c main.c >> gcc -g -o prog baz.o main.o >> >> This will have two function symbols called "foo" in the symbol table. >> Since you only match on the name, you will likely get one of them wrong. >> You could check before/after your patch with a simple script like: >> >> stap -e 'process.function("foo") { printf ("%s: %x\n", pp(), uaddr()) }' >> -c ./prog You are right! So, will change the map of name to address mapping to a map of lists where each name can have multiple func_info's. And we can probe on multiple addresses for that name. >>> @@ -8154,6 +8173,25 @@ symbol_table::get_from_elf() >>> reject = reject_section(section); >>> #endif >>> >>> + /* >>> + * For ELF ABI v2 on PPC64 LE, we need to adjust sym.st_value corresponding >>> + * to the bits of sym.st_other. These bits will tell us what's the offset >>> + * of the local entry point from the global entry point. >>> + */ >>> + Dwarf_Addr bias; >>> + Elf* elf = (dwarf_getelf (dwfl_module_getdwarf (mod, &bias)) >>> + ?: dwfl_module_getelf (mod, &bias)); >>> + >>> + GElf_Ehdr ehdr_mem; >>> + GElf_Ehdr* em = gelf_getehdr (elf, &ehdr_mem); >>> + if (em == 0) { DWFL_ASSERT ("dwfl_getehdr", dwfl_errno()); } >>> + assert(em); >> This sanity check seems to confuse a couple of things. >> There are 3 ways this could end up as NULL. Either dwarf_getelf or >> dwfl_module_getelf returned NULL, then gelf_getehdr would return NULL >> also. Or gelf_getehdr returns NULL because it got passed a faulty Elf. >> All cases are very unlikely, because we wouldn't have arrived at this if >> any of those cases were true. DWFL_ASSERT will print only the dwfl_errno >> when set, but not the elf_errno or dwarf_errno. There also isn't >> anything called dwfl_getehdr. Normally DWFL_ASSERT would throw on error, >> so the assert is never reached. I would simply only assert (em). Or if >> you really want to throw an error do: >> if (em == NULL) throw SEMANTIC_ERROR (_("Couldn't get elf header")); >> >> Also please move this out of the loop. Just fetch the ehdr once at the >> start. Sure. Will made the changes. >>> + /* st_other is currently only used with ABIv2 on ppc64 */ >>> + if ((em->e_machine == EM_PPC64) && (GELF_ST_TYPE(sym.st_info) == STT_FUNC) >>> + && sym.st_other) >>> + addr = addr + PPC64_LOCAL_ENTRY_OFFSET(sym.st_other); >>> + >>> if (name && GELF_ST_TYPE(sym.st_info) == STT_FUNC) >>> add_symbol(name, (GELF_ST_BIND(sym.st_info) == STB_WEAK), >>> reject, addr, &high_addr); >> Thanks, >> >> Mark Testing the v3 and will send it out asap. -- Thanks, Hemant Kumar