A customer reported issues with some of their binaries thinking they had chained PDATA records (very uncommon for x64, so SizeBench doesn't support that yet). As I investigated, it turned out that there were two problems:
Some UNWIND_INFO structures have a version 0 and flags 0, so they can be skipped.
The PDATA doesn't always live in its own section, nor is it required to take up a full section. SizeBench was assuming that and that's wrong - instead we should look at the "Exception" directory of the PE optional header which has the exact location and length of the PDATA array. Because SizeBench was reading this wrong in some edge cases, we'd get garbage/misaligned data in the PDATA and think we were trying to parse chained PDATA when in fact that wasn't the case.
Briefly summarize what changed
Fix PDATA RVA range to be correct, and correspondingly update the tests.
If we find an UNWIND_INFO with version == 0 and flags == 0, just skip it.
While I was doing this, I also did a little cleanup:
Fix the missing IBinaryLocator that used to be present, somehow this probably got lost in the merge to get this moved from an MS-internal repo to GitHub (Fixes #22).
Fix a couple warnings that were lingering since the .NET 8 SDK updated, just for hygiene
Fix an issue where the Test PEs were being instrumented by the code coverage instrumentation during code coverage runs, which isn't desirable as we want these PEs to be bit-for-bit exactly what was checked in, since we look at a lot of data structures and test specific values precisely. This makes the tests way more reliable in Visual Studio (CI runs seemed unaffected this whole time)
How was the change tested?
All existing tests pass, and the PGOTests are much more reliable due to the code coverage fix included here.
Tested the customer-provided binaries and both now parse correctly when they used to throw on load before.
PR Checklist
[x] Contributor License Agreement (CLA) has been signed. If not, go here and sign the CLA
[x] Changes have been validated
[ ] ~Documentation updated. Please add or update any docs in the repo as necessary.~
Why is this change being made?
A customer reported issues with some of their binaries thinking they had chained PDATA records (very uncommon for x64, so SizeBench doesn't support that yet). As I investigated, it turned out that there were two problems:
UNWIND_INFOstructures have a version 0 and flags 0, so they can be skipped.Briefly summarize what changed
UNWIND_INFOwith version == 0 and flags == 0, just skip it.While I was doing this, I also did a little cleanup:
IBinaryLocatorthat used to be present, somehow this probably got lost in the merge to get this moved from an MS-internal repo to GitHub (Fixes #22).How was the change tested?
PR Checklist