This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PR10000: emit _stp_relocate* calculations correctly for kernel/module global $data (Was: [SCM] systemtap: system-wide probe/trace tool branch, master, updated. release-0.9-238-g432f054)


Hi Roland,

On Wed, 2009-04-08 at 17:56 -0700, Roland McGrath wrote:
> > The crazy values come from then transforming the value with
> > dwfl_module_relocate_address().
> 
> I don't follow.  You mean you got some value from dwfl_module_getsym, right?

Yes, and the results of that (sym.st_value) put into
dwfl_module_relocate_address. Which is bogus in this case.

> I think it's giving you st_value + main.bias, basically.  (That's a bug.)
> Then this bogus address happens to lie somewhere very early in the text, so
> that it's actually an address in the ELF header or something.  Then
> dwfl_module_relocate_address tells you what that is relative to the
> earliest proper text section (so it can be negative), or something like that.

Yes, that seems precisely what is happening (except that negative
addresses don't exist, so we get something hugely large).

So what we could do at this time is make sure sym.value is between the
reported base of the module and the end of the module as returned by
dwfl_module_info() Then at least we only process "sane values" (even
though they might not be real function or data symbols).

> > Basically we are just doing the following from a dwfl_getmodules()
> > callback:
> [...]
> >       const char *name = dwfl_module_getsym(m, i, &sym, NULL);
> s/NULL/&shndx/
> >           && !(sym.st_shndx == SHN_UNDEF
> >                || sym.st_shndx == SHN_ABS))
> 
> You might not want to exclude SHN_ABS, really.  
> That is things like _edata and _end.

We exclude those since in some kernels on some architectures have vDSO
names marked as SHN_ABS && STT_FUNC (instead of STT_NOTYPE). Maybe we
should move the SHN_ABS check to only the kernel module.

> > So, we need to extend the check to include the section flags of the
> > given sym.st_shndx. 
> 
> For anal reasons, the &shndx filled by dwfl_module_getsym is what you use
> for the actual symbol, not st_shndx--but st_shndx is what you test for the
> SHN_* constants.

ehm, ok, odd.

> Another thought is to say that dwfl_module_getsym is intended mainly for
> symbols that translate to an actual address (i.e. in SHF_ALLOC sections or
> SHN_ABS).  Then all one is really looking for is an indicator whether the
> symndx you asked about has an st_value meaningful as an address or not.
> I've done that on the roland/getsym branch in elfutils git, if you want to
> take a look.

 This works like gelf_getsym except that st_value is always adjusted to
 an absolute value based on the module's location, when the symbol is in
 an SHF_ALLOC section.  If SHNDXP is non-null, it's set with the section
 index (whether from st_shndx or extended index table); in case of a
 symbol in a non-allocated section, *SHNDXP is instead set to -1.  */

Nice, that is the interface I want!
Unfortunately we also have to work with older versions.
But I guess I can already add the check for shndxp != -1 and it will
start working properly with newer versions.

Thanks,

Mark


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]