fix(debuginfo): Detect more hidden symbols in swift sources #316
Conversation
symbolic-debuginfo/src/macho.rs
Outdated
| n.starts_with("__?hidden#") | ||
| || n.starts_with("__hidden#") | ||
| || n.starts_with("_hidden#") |
There was a problem hiding this comment.
wouldn't it be better to be future proof and regex it?
hidden# looks like the common pattern, or even taking into consideration 1 or N _ and having or not ?
There was a problem hiding this comment.
Regexes are a bit expensive at this part, but we could use aho corasick in place of this.
That said, MachO dictates that there can only be a single or two underscores, and I'm not even sure why the symbols I'm looking at don't have the question mark.
There was a problem hiding this comment.
lgtm, @marandaneto has a valid feedback though. Not sure how likely it is that things are changing again
|
Verified this on a new build. Turns out that this was a regression in #127, where I replaced the RegEx |
* 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)
* master: build: Update pdb to 0.7.0 (#334) release: 8.0.5 meta: Changelog fix(debuginfo): Detect more hidden symbols in swift sources (#316) 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)
A regression in #127 broke detection of BCSymbolMaps. The RegEx
__?hidden#\d+_was replaced with a literal__?hidden#prefix and missed that?is an operator rather than a literal question mark.This RegEx was required due to another issue in the MachO symbol iterator, which unconditionally strips the first leading underscore to match names from DWARF debugging information. For hidden symbols, this does not make sense, however, and instead it should be retained.