A tool for mapping runtime addresses to symbol names.

A tool for mapping runtime addresses to symbol names.


BUG=123763
TEST=compare with pprof --symbols.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=147784

[Dai Mikurube (NOT FULLTIME), 2012-07-19 09:26:57]

Hi Marc,

Could you please take a look at these scripts?  They are similar to 'pprof
--symbols'.  pprof is at third_party/tcmalloc/chromium/src.

Details are in README.

They are required mainly for tools/deep_memory_profiler to
1) run dmprof more quickly, and
2) work with binaries compiled by Clang.

Binaries generated by Clang look like having less or wrong debug information
than gcc.  It makes addr2line too much slow for Clang binaries.

pprof uses addr2line.  But for our use, only 'nm' is enough.  These proposed
scripts use only nm.


fyi,
I separated these scripts into two steps (see README).  The first step stores
required symbol files on disk.  In contrast, pprof does this in one process on
memory.

The stored symbol files can be ported to other machines, and we can analyze with
them without Chrome's binary.  It's a great benefit for us.


fyi 2,
These scripts are written to be used both as a command, and as a Python library.
 We sometimes use them as a command, and I'm planning to use them from
tools/deep_memory_profiler/dmprof.

[M-A Ruel, 2012-07-19 15:21:15]

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
File tools/find_runtime_symbols/find_runtime_symbols.py (right):

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:2: 
Copyright in all files

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:14: return (entry.executable
== 'x' and re.match(
You know about os.path.splitext(entry.name)[1] ?

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:58: log.setLevel(loglevel)
Usually we make it module wide, but I don't mind.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:72: with
open(os.path.join(prepared_data_dir, 'maps'), mode='r') as FILE:
ALL CAPS are for constants. In other files too.

Use "f"

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:92: if (entry.begin <=
address and address < entry.end):
if entry.begin <= address < entry.end:
is valid python.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:93: if
symbol_table.has_key(entry.name):
if entry.name in symbol_table:

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:100: is_found = True
move this assignment out of the condition.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:105: return 0
Why not return a list and have it output after? E.g. remove the parameter
outputter.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:126: # FIX: Accept only .pre
data
Use a main() in every file where the code is not trivial.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/par...
File tools/find_runtime_symbols/parse_proc_maps.py (right):

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/par...
tools/find_runtime_symbols/parse_proc_maps.py:6: class ProcMapsEntry(object):
A oneliner docstring would help here and in a few other places.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/par...
tools/find_runtime_symbols/parse_proc_maps.py:52: _MAPS_PATTERN = re.compile(
All constants in every files at top of the file

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
File tools/find_runtime_symbols/prepare_symbol_info.py (right):

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
tools/find_runtime_symbols/prepare_symbol_info.py:15: def
_executable_condition(entry):
Why duplicate this function?

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
tools/find_runtime_symbols/prepare_symbol_info.py:35: matched =
re.match('^/proc/(.*)/maps$', os.path.realpath(maps_path))
You should make this clear that these files are linux (?) specific, have the
main refuse to run on other OSes.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
tools/find_runtime_symbols/prepare_symbol_info.py:79: if p_nm.wait() == 0:
See http://docs.python.org/library/subprocess.html#subprocess.Popen.wait

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
tools/find_runtime_symbols/prepare_symbol_info.py:90: 'file':
os.path.basename(os.path.basename(nm_filename)),
Did you mean dirname?

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
tools/find_runtime_symbols/prepare_symbol_info.py:93: }
funny alignment

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
tools/find_runtime_symbols/prepare_symbol_info.py:98: json.dump(nm_files, FILE,
indent=2, sort_keys=True)
json.dump in a w+ opened file? It's going to be invalid.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pro...
File tools/find_runtime_symbols/procedure_boundaries.py (right):

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pro...
tools/find_runtime_symbols/procedure_boundaries.py:1: #!/usr/bin/env python
Remove the shebang on files not executable.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pro...
tools/find_runtime_symbols/procedure_boundaries.py:9: def __init__(self, value):
The __init__ is not strictly necessary. You can use self.args[0] in your __str__
function.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pro...
tools/find_runtime_symbols/procedure_boundaries.py:52: function, number =
_ARGUMENT_TYPE_PATTERN.subn('', function)
A constant is not strictly necessary when used only at one place. Remember that
all functions inside module re keep a cache of recently used regexp so when it
is used inside a loop, it's fast enough.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pro...
tools/find_runtime_symbols/procedure_boundaries.py:53: if number == 0:
if not number:
unless you want to differentiate with None.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pro...
tools/find_runtime_symbols/procedure_boundaries.py:76: routine = '';
; ?

[Dai Mikurube (NOT FULLTIME), 2012-07-20 04:32:39]

Thanks, Marc.  Updated the patch.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
File tools/find_runtime_symbols/find_runtime_symbols.py (right):

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:2: 
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> Copyright in all files

Done.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:14: return (entry.executable
== 'x' and re.match(
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> You know about os.path.splitext(entry.name)[1] ?

I didn't know.  But, it should match foo.dll, libfoo.so.3, libfoo.1.2 and
chrome.  I guess splitext doesn't work well for such cases.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:58: log.setLevel(loglevel)
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> Usually we make it module wide, but I don't mind.

I wanted to set loglevel in _find_runtime_symbols scope.  I keep it as is.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:72: with
open(os.path.join(prepared_data_dir, 'maps'), mode='r') as FILE:
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> ALL CAPS are for constants. In other files too.
> 
> Use "f"

Done.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:92: if (entry.begin <=
address and address < entry.end):
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> if entry.begin <= address < entry.end:
> is valid python.

Done.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:93: if
symbol_table.has_key(entry.name):
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> if entry.name in symbol_table:

Done.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:100: is_found = True
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> move this assignment out of the condition.

Done.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:105: return 0
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> Why not return a list and have it output after? E.g. remove the parameter
> outputter.

It's to allow a file object for outputter (_FileOutput).  I don't want keep all
results in memory for some cases.  The results can be very large.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/fin...
tools/find_runtime_symbols/find_runtime_symbols.py:126: # FIX: Accept only .pre
data
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> Use a main() in every file where the code is not trivial.

I was misunderstanding about Python modules.  I was considering name conflict
for imported cases.  But, it doesn't happen!

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/par...
File tools/find_runtime_symbols/parse_proc_maps.py (right):

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/par...
tools/find_runtime_symbols/parse_proc_maps.py:6: class ProcMapsEntry(object):
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> A oneliner docstring would help here and in a few other places.

Done.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/par...
tools/find_runtime_symbols/parse_proc_maps.py:52: _MAPS_PATTERN = re.compile(
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> All constants in every files at top of the file

Done.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
File tools/find_runtime_symbols/prepare_symbol_info.py (right):

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
tools/find_runtime_symbols/prepare_symbol_info.py:15: def
_executable_condition(entry):
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> Why duplicate this function?

Moved it to util.py, and shared it.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
tools/find_runtime_symbols/prepare_symbol_info.py:35: matched =
re.match('^/proc/(.*)/maps$', os.path.realpath(maps_path))
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> You should make this clear that these files are linux (?) specific, have the
> main refuse to run on other OSes.

Done.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
tools/find_runtime_symbols/prepare_symbol_info.py:79: if p_nm.wait() == 0:
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> See http://docs.python.org/library/subprocess.html#subprocess.Popen.wait

The problem is that it's giving p_nm.stdout to c++filt's stdin. 
Popen.communicate() intercepts a part of p_nm.stdout, and c++filt fails with
p_nm.communicate().

I'd like to avoid reading contents from p_nm.stdout.  p_nm.stdout will be very
big...  Do you have any ideas for such a case?
(fyi, I can drop stderr.  It's just providing error messages for users.)

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
tools/find_runtime_symbols/prepare_symbol_info.py:90: 'file':
os.path.basename(os.path.basename(nm_filename)),
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> Did you mean dirname?

Ah, one of basename is not required.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
tools/find_runtime_symbols/prepare_symbol_info.py:93: }
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> funny alignment

Done.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pre...
tools/find_runtime_symbols/prepare_symbol_info.py:98: json.dump(nm_files, FILE,
indent=2, sort_keys=True)
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> json.dump in a w+ opened file? It's going to be invalid.

Done.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pro...
File tools/find_runtime_symbols/procedure_boundaries.py (right):

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pro...
tools/find_runtime_symbols/procedure_boundaries.py:1: #!/usr/bin/env python
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> Remove the shebang on files not executable. 

Done.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pro...
tools/find_runtime_symbols/procedure_boundaries.py:9: def __init__(self, value):
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> The __init__ is not strictly necessary. You can use self.args[0] in your
__str__
> function.

Thanks.  Done it.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pro...
tools/find_runtime_symbols/procedure_boundaries.py:52: function, number =
_ARGUMENT_TYPE_PATTERN.subn('', function)
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> A constant is not strictly necessary when used only at one place. Remember
that
> all functions inside module re keep a cache of recently used regexp so when it
> is used inside a loop, it's fast enough. 

As far as I remember, only one regexp is kept in module re. 
_get_short_function_name() is called very many times, and it loops only once or
twice in most cases.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pro...
tools/find_runtime_symbols/procedure_boundaries.py:53: if number == 0:
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> if not number:
> unless you want to differentiate with None.

Done.

http://codereview.chromium.org/10795028/diff/1/tools/find_runtime_symbols/pro...
tools/find_runtime_symbols/procedure_boundaries.py:76: routine = '';
On 2012/07/19 15:21:15, Marc-Antoine Ruel wrote:
> ; ?

Wow, removed them.  (I was translating this code from Perl...)

[M-A Ruel, 2012-07-20 14:10:21]

lgtm

http://codereview.chromium.org/10795028/diff/5008/tools/find_runtime_symbols/...
File tools/find_runtime_symbols/prepare_symbol_info.py (right):

http://codereview.chromium.org/10795028/diff/5008/tools/find_runtime_symbols/...
tools/find_runtime_symbols/prepare_symbol_info.py:57: with open(maps_path,
mode='r') as f:
optional: mode='r' is the default so it is optional

http://codereview.chromium.org/10795028/diff/5008/tools/find_runtime_symbols/...
File tools/find_runtime_symbols/procedure_boundaries.py (right):

http://codereview.chromium.org/10795028/diff/5008/tools/find_runtime_symbols/...
tools/find_runtime_symbols/procedure_boundaries.py:65: function =
_LEADING_TYPE_PATTERN.sub('\g<1>', function)
optional: you could return right away

[Dai Mikurube (NOT FULLTIME), 2012-07-21 04:50:10]

Thanks.  I'll be checking the "Commit".

http://codereview.chromium.org/10795028/diff/5008/tools/find_runtime_symbols/...
File tools/find_runtime_symbols/prepare_symbol_info.py (right):

http://codereview.chromium.org/10795028/diff/5008/tools/find_runtime_symbols/...
tools/find_runtime_symbols/prepare_symbol_info.py:57: with open(maps_path,
mode='r') as f:
On 2012/07/20 14:10:21, Marc-Antoine Ruel wrote:
> optional: mode='r' is the default so it is optional

Thanks.  I keep it as is to make it clear that it's reading.

http://codereview.chromium.org/10795028/diff/5008/tools/find_runtime_symbols/...
File tools/find_runtime_symbols/procedure_boundaries.py (right):

http://codereview.chromium.org/10795028/diff/5008/tools/find_runtime_symbols/...
tools/find_runtime_symbols/procedure_boundaries.py:65: function =
_LEADING_TYPE_PATTERN.sub('\g<1>', function)
On 2012/07/20 14:10:21, Marc-Antoine Ruel wrote:
> optional: you could return right away

Thanks.  Done.

[commit-bot: I haz the power, 2012-07-21 04:50:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmikurube@chromium.org/10795028/9

[commit-bot: I haz the power, 2012-07-21 06:09:37]

Change committed as 147784