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

RESOLVED FIXED in Firefox 64

Status

enhancement
P3
normal
RESOLVED FIXED
Last year
10 months ago

People

(Reporter: ted, Assigned: gsvelto)

Tracking

Version 3
mozilla64
Points:
---

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

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
Priority: -- → P3
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.
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
This is a tentative patch as I haven't tested it yet but I wanted some feedback on the overall approach.
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.
Attachment #9006571 - Flags: review+
Pushed by gsvelto@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/552cf01fb131
Prefer FUNC entries to PUBLIC entries when fixing symbols r=ted
https://hg.mozilla.org/mozilla-central/rev/552cf01fb131
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.