Don't truncate ranges of units #163
Conversation
This commit is an attempt to fix rust-lang/backtrace-rs#327. The behavior seen there was that CGUs would often overlap each other which would cause a previous loop which tried to ensure CGUs didn't overlap to avoid some CGUs being candidates for symbolizing. To fix the issue this commit removes the loop which alters the range of each CGU. Instead CGUs are only sorted by their starting function address. When looking up a CGU for a function the CGU's address range is used to initially filter the sort but afterwards the units function address table is also consulted. The first CGU which contains the function address is what ends up getting returned.
alexcrichton
force-pushed
the
fix-cgu-truncate
branch
from
f62ed5e
to
5575415
Compare
|
Ok and now with tests for macos! |
src/lib.rs
Outdated
| // No exact match was found, but this probe would fit at slot `i`. | ||
| // This means that slot `i-1` is the highest start address which is | ||
| // underneath `probe`, which is the CGU we'd like to start | ||
| // searching. |
There was a problem hiding this comment.
Nit: DWARF calls them compilation units (CU).
Given that CUs can be much larger than needed, it's not clear to me that this is the CU we need to start at. Why can't there be a CU with a lower start address that contains the probe address?
There was a problem hiding this comment.
Hm yes I'm entirely mixed up in how sorting works, let me see if I can find a solution which actually works.
There was a problem hiding this comment.
Ok I think the new logic should work out!
| small::test_function as u64, | ||
| "correctness::small::test_function", | ||
| ); | ||
| test(aux::foo as u64, "aux::foo"); |
There was a problem hiding this comment.
What are these two tests checking that wasn't covered before?
There was a problem hiding this comment.
The small module was me attempting to reproduce the original issue and the aux::foo test will fail on macOS before this PR but succeeds after this PR. I'm not entirely sure what makes rustc generate the object that causes this but w/e it is it seems to work.
|
Nah this isn't urgent, just something I noticed. Should be fine to defer a release! |
This commit is an attempt to fix rust-lang/backtrace-rs#327. The
behavior seen there was that CGUs would often overlap each other which
would cause a previous loop which tried to ensure CGUs didn't overlap to
avoid some CGUs being candidates for symbolizing.
To fix the issue this commit removes the loop which alters the range of
each CGU. Instead CGUs are only sorted by their starting function
address. When looking up a CGU for a function the CGU's address range is
used to initially filter the sort but afterwards the units function
address table is also consulted. The first CGU which contains the
function address is what ends up getting returned.