1470223 - fix_stack_using_bpsyms.py should prefer FUNC records over PUBLIC records, currently does the opposite

Status: RESOLVED FIXED

(Testing :: General, enhancement, P3)

Description

[(not currently active) Ted Mielczarek]

While looking at the code for the symbolication API on symbols.mo I noticed that it had a bug where it would prefer PUBLIC records over FUNC records:
https://github.com/mozilla-services/tecken/issues/926#issuecomment-399162970

I went to look at fix_stack_using_bpsyms.py for comparison and it has the same bug:
https://dxr.mozilla.org/mozilla-central/rev/681eb7dfa324dd50403c382888929ea8b8b11b00/tools/rb/fix_stack_using_bpsyms.py#61

That code just sticks all FUNC and PUBLIC addresses in the same list and then binary searches it by address, and has a single dict of address => function name. PUBLIC records usually come after FUNC records in a .sym file, so we'll always overwrite FUNC info with PUBLIC info. Additionally, the code should use the size field in FUNC records to check whether an address actually falls within a function.

I think the code should be changed to build two lists of addresses: one for FUNCs and one for PUBLICs. Additionally FUNC sizes should be stored and we should check that the address falls within [func addr, func addr + size). If no FUNC is found for an address, then the closest PUBLIC record with an address less than the address in question can be returned.

See the Breakpad code that does these lookups for reference:
https://chromium.googlesource.com/breakpad/breakpad/+/b1226959a25b6a5311801d6f204b088c706e7c25/src/processor/basic_source_line_resolver.cc#228

Comment 1

[Gabriele Svelto [:gsvelto]]

Attachment: Bug 1470223 - Prefer FUNC entries to PUBLIC entries when fixing symbols

This patch changes the way we search symbols when fixing up a stack.
Previously we would find the closest PUBLIC or FUNC entry lower than a given
address. Because of how symbol files were processed we preferred PUBLIC
entries to FUNC ones. Now we look first for the function that contains the
address (obtained from the FUNC entries) then if none is available we look for
the closest, lower PUBLIC entry.

Comment 2

[Gabriele Svelto [:gsvelto]]

This is a tentative patch as I haven't tested it yet but I wanted some feedback on the overall approach.

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 9006571 [details]
Bug 1470223 - Prefer FUNC entries to PUBLIC entries when fixing symbols

Ted Mielczarek [:ted] [:ted.mielczarek] has approved the revision.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/552cf01fb131
Prefer FUNC entries to PUBLIC entries when fixing symbols r=ted

Comment 5

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/552cf01fb131