From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19844 invoked by alias); 17 Mar 2011 13:04:19 -0000 Received: (qmail 19831 invoked by uid 22791); 17 Mar 2011 13:04:18 -0000 X-SWARE-Spam-Status: No, hits=-6.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 17 Mar 2011 13:04:12 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p2HD49Sh009227 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 17 Mar 2011 09:04:10 -0400 Received: from [10.3.113.47] (ovpn-113-47.phx2.redhat.com [10.3.113.47]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p2HD48c8001756; Thu, 17 Mar 2011 09:04:09 -0400 Subject: Re: Failures with exelib.exp testcase (was Re: minutes 2010-08-19) From: Mark Wielaard To: prasad@linux.vnet.ibm.com Cc: systemtap@sourceware.org In-Reply-To: <20110317090541.GA2416@in.ibm.com> References: <20110203050358.GA2488@in.ibm.com> <1296728173.3341.7.camel@springer.wildebeest.org> <20110203123353.GC2488@in.ibm.com> <1296737077.3341.22.camel@springer.wildebeest.org> <20110203185926.19226180954@magilla.sf.frob.com> <1297170815.3956.34.camel@springer.wildebeest.org> <20110214164945.GA2159@in.ibm.com> <1297715606.3344.29.camel@springer.wildebeest.org> <20110304071306.GA2328@in.ibm.com> <1299251193.3343.80.camel@springer.wildebeest.org> <20110317090541.GA2416@in.ibm.com> Content-Type: text/plain; charset="UTF-8" Date: Thu, 17 Mar 2011 13:04:00 -0000 Message-ID: <1300367047.32409.31.camel@springer.wildebeest.org> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2011-q1/txt/msg00480.txt.bz2 On Thu, 2011-03-17 at 14:35 +0530, K.Prasad wrote: > On Fri, Mar 04, 2011 at 04:06:33PM +0100, Mark Wielaard wrote: > > On Fri, 2011-03-04 at 12:43 +0530, K.Prasad wrote: > > > Index: systemtap/translate.cxx > > > =================================================================== > > > --- systemtap.orig/translate.cxx > > > +++ systemtap/translate.cxx > > > @@ -5018,7 +5018,30 @@ > > > || sym.st_value >= end // beyond current module, > > > || sym.st_value < base)) // before first section. > > > { > > > - Dwarf_Addr sym_addr = sym.st_value; > > > + Dwarf_Addr sym_addr; > > > +#ifdef __powerpc__ > > > > You shouldn't depend on this, do a check against the e_machine type of > > the elf file header to check that is it a ppc64 one. > > > > Done. > > > > +// Elf64_Addr opd_addr; > > > + Dwarf_Addr bias; > > > + Elf_Data *data = NULL; > > > + Elf_Data in, out; > > > + Elf_Scn *opd = dwfl_module_address_section (m, &sym_addr, &bias); > > > + Elf* elf = (dwfl_module_getelf (m, &bias)); > > > + > > > + if (opd != NULL) > > > + { > > > + data = elf_rawdata (opd, NULL); > > > + if (data == NULL) > > > + return DWARF_CB_ABORT; > > > + } > > > + out.d_buf = *userdata; > > > > userdata is a pointer to the (unused) unwindsym_dump_context pointer > > provided to us as dwfl_getmodules callback, why are you using it here? > > Don't you want to store the result in the opd_addr? > > > > Ok. Making it "out.d_buf = (void *)&opd_addr". > > > > + in.d_buf = (char *) data->d_buf + sym_addr; > > > + out.d_size = in.d_size = sizeof (Elf64_Addr); > > > + out.d_type = in.d_type = ELF_T_ADDR; > > > + if (elf64_xlatetom (&out, &in, elf_getident (elf, NULL)[EI_DATA]) != NULL) > > > + sym_addr = sym.st_value + bias; > > > > You are ignoring the error case. You are reusing the original > > sym.st_value, but haven't stored the newly fetched address in it (might > > be better to do that in a fresh variable anyway). Note that you are > > reusing bias above for both the dwfl_module_address_section () and > > dwfl_module_getelf () calls, you are using the result of the second, not > > the first here. > > I have a few doubts here....basically the dwfl_module_address_section() > uses 'sym_addr' variable and does not store data into it but we are > using it afresh after initialisation. I'm thinking if the 'sym_addr' in > the pseudocode referred to some other value which I did not decipher > correctly. dwfl_module_address_section () will adjust the given sym_addr to be relative to the section (see the description in libdwfl.h). sym_addr was originally initialized to sym.st_value, but I now see you changed that in your patch. > Secondly, I see that 'bias' will result in the same value from both > function calls - dwfl_module_address_section and dwfl_module_getelf > given that the same module 'm' is passed to them, so guess we don't have > to be worried at that. The bias from dwfl_module_address_section () is set to "the difference between addresses used in the returned section's headers and run-time addresses". The bias from dwfl_module_getelf () is set to "the difference between addresses within the loaded module and those in symbol tables or Dwarf information referring to it". I always get confused by these biases, so I am normally conservative in making sure I don't mix them if I am not 100% convinced they will always be the same. I would have to look at the source to be truly sure here. > What isn't clear to me is the error case you've mentioned and the new > address you'd like to store in sym.st_value. You have to handle errors, either by dropping out and producing an error or ignoring them/warning about them. I am not suggesting you store anything new in sym.st_value. But in the end sym_addr needs to point to the actual function address, since that is what we will use to store the address of the function for the section in the address map (at the end of the for loop as (addrmap[secidx])[sym_addr] = name). This then gets put in the stap-symbols.h table. BTW. David Smith found another instance of what you are working on, see http://sourceware.org/bugzilla/show_bug.cgi?id=12566 "usymbols.exp 64-bit test failing on ppc64" You might want to post your current incarnation of your patch so you can coordinate with him to see if it helps his testcase. Cheers, Mark