fix(symcache): Explicitly map "holes" between functions #897
Conversation
Currently, if we look up an address that falls "between" two functions/symbols, we will always resolve it to the first of the two functions/symbols because each range implicitly extends to the start of the next one. This can lead to bad user experience when we map (invalid) addresses to entirely nonsensical functions. Therefore, we now explicitly insert a mapping at the end of the function that maps to "nowhere" to denote the function's end.
| if let btree_map::Entry::Vacant(vacant_entry) = self.ranges.entry(end_address) { | ||
| vacant_entry.insert(NO_SOURCE_LOCATION); | ||
| } | ||
| } |
There was a problem hiding this comment.
Out of curiosity, why do we need to add this logic in two different places?
There was a problem hiding this comment.
We insert both "symbols" and "functions" into the symcache. A symbol is extracted from the symbol table in the debug file and has no line records, just a name and an address (and maybe a size). So functions and symbols take two separate code paths.
| @@ -10,6 +10,7 @@ use symbolic_debuginfo::{DebugSession, FileFormat, Function, ObjectLike, Symbol} | |||
| use watto::{Pod, StringTable, Writer}; | |||
|
|
|||
| use super::{raw, transform}; | |||
| use crate::raw::NO_SOURCE_LOCATION; | |||
There was a problem hiding this comment.
What will the user see in Sentry with this approach?
There was a problem hiding this comment.
They would see an unsymbolicated frame with a "missing_symbol" status, like this (in JSON):
{
"status": "missing_symbol",
"original_index": 2,
"instruction_addr": "0x100821ae0",
"package": "…",
"function": "<redacted>",
"in_app": true
}There was a problem hiding this comment.
There was a problem hiding this comment.
I think that's sufficient for the use case that lead to this PR in Slack. Thanks again @loewenheim 💯
There was a problem hiding this comment.
the code and logic looks reasonable. you folks mentioned a slack conversation? can you distill that down to an explanation of what exactly is happening here?
why is a stack frame pointing at the end of a function?
maybe because its a diverging function call, and the "return address" being pushed to the stack is never expected to be actually returned to at runtime, so its pointing past the callers call instruction to some nop instructions?
| #[test] | ||
| fn test_lookup_between_functions() { | ||
| let buffer = ByteView::open(fixture( | ||
| "macos/3CD3E3CC-281E-3AF3-84EB-CDE6D56F0559.dSYM/Contents/Resources/DWARF/CrashLibiOS", |
There was a problem hiding this comment.
can you only commit the actual dwarf payload and remove all the other dSYM junk thats not useful?
also, that file is a ~1.5M. is it possible to slim that down somehow?
There was a problem hiding this comment.
I decided to just reuse the Python fixture instead of committing a copy. Didn't pay attention to the file size before.
The essence is that we have a user case where we are getting a frame with an address that falls "between" functions. Under the current logic, we map it to the last function before it and that creates an entirely nonsensical frame (the function we're finding has nothing to do with the error that happened). The user thinks that this is caused by stack corruption. |
|
Well yes, so much is obvious. I was rather wondering what legitimate reason there might be that a return addr on the stack is pointing past the end of the function? |
Currently, if we look up an address that falls "between" two functions/symbols, we will always resolve it to the first of the two functions/symbols because each range implicitly extends to the start of the next one. This can lead to bad user experience when we map (invalid) addresses to entirely nonsensical functions.
Therefore, we now explicitly insert a mapping at the end of the function that maps to "nowhere" to denote the function's end.