Re: Failures with exelib.exp testcase (was Re: minutes 2010-08-19)

["K.Prasad" <prasad@linux.vnet.ibm.com>]

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:
> > On Mon, Feb 14, 2011 at 09:33:26PM +0100, Mark Wielaard wrote:
> > > On Mon, 2011-02-14 at 22:19 +0530, K.Prasad wrote:
> > > > On Tue, Feb 08, 2011 at 02:13:35PM +0100, Mark Wielaard wrote:
> > > > > On Thu, 2011-02-03 at 10:59 -0800, Roland McGrath wrote:
> > > > > > What you need to do is:
> > > > > > 
> > > > > > 	Elf64_Addr opd_addr;
> > > > > > 	Dwarf_Addr bias;
> > > > > > 	Elf_Scn *opd = dwfl_module_address_section (mod, &sym_addr, &bias);
> > > > > > 	if (opd == NULL) ...;
> > > > > > 	Elf_Data *data = elf_rawdata (opd, NULL);
> > > > > > 	if (data == NULL) ...;
> > > > > > 	Elf_Data in, out;
> > > > > > 	out.d_buf = &final_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 (elf, &out, &in, e_ident[EI_DATA]) == NULL) ...;
> > > > > > 	sym_addr = opd_addr + bias;
> > > > > 
> > 
> > So I made an attempt at turning the above piece of pseudocode into a patch
> > for systemtap/translate.cxx and here's what it turned out to be.
> > 
> > Upon execution the runtime experiences "Segmentation fault" at the line
> > if (elf64_xlatetom (&out, &in, elf_getident (elf, NULL)[EI_DATA]) != NULL)
> > and I'm trying to understand why.
> > 
> > Any suggestions or pointers to mistakes in creation of the patch are
> > welcome.
> > 
> > ---
> >  translate.cxx |   25 ++++++++++++++++++++++++-
> >  1 file changed, 24 insertions(+), 1 deletion(-)
> > 
> > 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.

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.

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.

Thanks,
K.Prasad