search

blueszhangsh / gperftools

Automatically exported from code.google.com/p/gperftools
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Shared library names not resolved under Solaris for symbol lookups #171

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Steps
1. Profile on Solaris with runtime dependencies on shared libraries.

Any pc offsets in process space that represent loaded shared libs do not
resolve to symbol names; instead pprof uses the nebulous "??".

The "TODO" at src/base/sysinfo.cc:708 acknowledges that object names from
procfs are not being resolved to filenames when the profiler dumps the
process memory map.

In Solaris there are symlinks in /proc/pid/path/obj for each object "obj".
 Following that symlink via readlink(...) furnishes a valid filename for
loaded libraries.

I'm happy to attach my small addition to the code if someone can help me
with 2 style points (I haven't reviewed much of the code here)...
1.) What is an appropriate way to preserve the pid?  I added it as a
private member of ProcMapsIterator but I fear some would consider that bad
form.

2.) Also since the call provides a char** for the return of filename I used
a static local variable to house my filename and passed back a pointer to
that... which also seems like it might violate some paradigms in use here.

I've recently returned to C++ from java land and this is the first tool
I've found that comes anywhere close to the types of profiling I can do in
the java universe.  I appreciate the effort and want to help out.

Original issue reported on code.google.com by jeffreyp...@gmail.com on 21 Sep 2009 at 2:16

GoogleCodeExporter commented 9 years ago 
Thanks for your offer to help!  We'll be glad to take patches.  Unfortunately, 
I lost
access to a solaris box so I can't do good testing anymore, so outside patches 
are
particularly important for that platform.

Storing the pid in ProcMapsIterator seems fine to me, if I understand right what
you're suggesting (I may not be).  The static local variable to hold the 
filename
probably won't work, as you suspect; I don't think it's safe if this iterator 
is used
from multiple threads.  I think the right thing to do is to make char
filename_[PATH_MAX] part of the ProcMapsIterator class for solaris, and just 
write
the filename into that, and return a pointer.  You'll have to document 
somewhere that
each Next() invalidates the pointer in name, but I think that's true in any 
case. 
For instance, in windows, name points into module_, but module_ contents change 
in
every call to Next().

If you are going to submit a patch, please also fill out
   http://code.google.com/legal/individual-cla-v1.0.html

I'm glad you're finding this project useful for you!

Original comment by csilv...@gmail.com on 23 Sep 2009 at 12:54

GoogleCodeExporter commented 9 years ago 
Attached is a patch.  Thanks for the style tips/c++ remediation.  I'm open to
criticism as this is my first patch ... produced in src/ with svn -diff -x -w. 
Likewise I tested it on a fresh checkout in src/ with patch -p0.

Original comment by jeffreyp...@gmail.com on 23 Sep 2009 at 8:36

Attachments:

GoogleCodeExporter commented 9 years ago 
OK, I've applied the patch, with some minor changes (mostly formatting).  I've
attached my version below; do you mind giving it a try just to make sure I 
didn't
make any mistakes?  (The patch may not apply 100% cleanly, because of some other
pending changes, but should hopefully be easy to apply regardless.)

This will be in the next release.

Original comment by csilv...@gmail.com on 16 Oct 2009 at 8:01

Attachments:

GoogleCodeExporter commented 9 years ago 
This should be fixed in perftools 1.5, just released.

Original comment by csilv...@gmail.com on 20 Jan 2010 at 11:11