-
-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(symcache): Support lookup for public syms larger than 65k #320
Conversation
symbolic-symcache/src/format.rs
Outdated
@@ -209,11 +209,18 @@ impl FuncRecord { | |||
} | |||
|
|||
/// The instruction address _after_ the end of the function. | |||
/// | |||
/// TODO(ja): Describe special case. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The special case is: len of 0xffff means we don't know how big it is and therefore assume it goes all the way to the end.
symbolic-symcache/src/format.rs
Outdated
} | ||
|
||
/// Checks whether the given address is covered by the function. | ||
/// | ||
/// TODO(ja): Describe special case. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The special case here is that we use <=
to treat records with len == 0
?
* master: fix(symcache): Support lookup for public syms larger than 65k (#320)
With getsentry/symbolic#320, we changed how trailing PUBLIC records in incomplete Breakpad symbols are handled during native symbolication. The unreal tests contain such incomplete symbols as fixtures to reduce their overall size, which now creates changes in the snapshot. We are not really interested in those additonal frames. By adding a phantom function at the end, Symbolicator does not apply the trailing symbol to all intermediate frames, which keeps the snapshots stable.
* master: test: add similar-asserts' assert_eq (#333) release: 8.0.4 meta: Changelog for 8.0.4 build: Drop support for python 2.7 (#328) meta(vscode): Fix include paths for C++ (#331) doc(debuginfo): Add descriptions of records to breakpad.pest (#329) build: Replace virtualenv with venv doc(symcache): Symcache documentation fix(debuginfo): Support debug_addr indexes in DWARF functions (#326) fix(symcache): Fixed bug that caused functions to have len 0 (#324) ref(symcache): FuncRecord::len must be nonzero (#323) fix: Clippy 1.50 lints (#322) fix(symcache): Support lookup for public syms larger than 65k (#320) fix(symcache): Compute correct line offsets (#319) release: 8.0.3 meta: Changelog build: Update goblin to 0.3.1 (#318) fix(elf): Consider sections of type SHT_MIPS_DWARF (#317)
Fixes part of #284 (comment)
Public symbol records often have no size. If there's a gap larger than 65k, the symcache lookup would not resolve the last symbol. For example, with this Breakpad symbol a lookup for
0x180000
should resolvesymbol_a
:In practice, it is likely that there are symbols missing and the lookup would in fact be wrong. However, there could be the possibility of such large functions and with how symbol tables are structured, this would be the expected result. Note that this now also matches Breakpad's behavior.