Re: [PATCH v4 1/3] systemtap/tapsets.cxx: Fix dwarfless probes on multiple static functions

[Mark Wielaard <mjw at redhat dot com>]

On Wed, 2015-04-22 at 20:05 +0530, Hemant Kumar wrote:
> On 04/22/2015 07:10 PM, Mark Wielaard wrote:
> > @@ -8242,6 +8242,8 @@ symbol_table::purge_syscall_stubs()
> >     if (!addrs || addrs->empty())
> >       return;
> >     /* Highly unlikely that multiple symbols named "sys_ni_syscall" may exist */
> > +  if (addrs->size() > 1)
> > +    cerr << _("Multiple 'sys_ni_syscall' symbols found.");
> >     Dwarf_Addr stub_addr = addrs->front();
> >
> > Just so that if this highly unlikely scenario does occur we get a
> > warning something is fishy.
> >
> Right! looks good.

And I am glad we did add that warning.
Martin found it triggered on ppc64be (ELFv1 ABI).

It was caused by ppc64be using function descriptors and stap using both
the actual function entry symbol .sys_ni_syscall and the function
descriptor symbol sys_ni_syscall. Both resolved to the same address. And
we mangle the name of the function entry symbol to remove the leading
dot. So they also have the same name.

This was mostly harmless. But it showed some inefficiencies. Frank
solved the immediate issue by using address sets instead of lists, so
duplicate addresses are just not returned:

commit fb5b48419b8d74e6cb82e90ba0aa9e188db07043
Author: Frank Ch. Eigler <fche@redhat.com>
Date:   Wed Apr 22 16:53:51 2015 -0400

  tapsets.cxx: fix symbol/address lookup returned-data to sets passed
  by value
    
  The symbol_table lookup_symbol[_address] functions are safer if they
  return their result-sets by value rather than by pointer.  The latter
  in specific should be a set rather than a list, to properly eliminate
  duplicates.

Then I removed the hardcoded #ifdef __powerpc__ constructs in
tapsets.cxx and replaced them with a check of whether the target is
ppc64 ELFv1 abi. That way cross-stapping (is that a word?) should work
across arches too (but I haven't tested that, just that ppc64be and
ppc64le both work correctly). This also removes the actual duplicates,
so the maps aren't filled with extra func_infos (there could be lots of
duplicates in the kernel when using function descriptors).

commit 064a90a93b8702a9f2649b5d46494e6218c8a145
Author: Mark Wielaard <mjw@redhat.com>
Date:   Thu Apr 23 15:59:49 2015 +0200

  ppc64le doesn't have function descriptors. Remove __powerpc__ in
  tapsets.cxx
    
  Only process the opd section and do function descriptor mangling when
  the target is ppc64 ELFv1 ABI. Also filter out any duplicate
  func_infos.
  When seeing a symbol with a name starting with '.' we assume it is a
  regular function pointer and not a pointer to a function descriptor
  and mangle its name. That might create duplicates if there is also a
  function descriptor with that name (the address will already have been
  resolved to the same address).

Cheers,

Mark