Fix off-by-one error in DWARF reg-reg location (#317)#321
Merged
agocke merged 1 commit intoobjwriter/release/7.0from Nov 22, 2022
Merged
Fix off-by-one error in DWARF reg-reg location (#317)#321agocke merged 1 commit intoobjwriter/release/7.0from
agocke merged 1 commit intoobjwriter/release/7.0from
Conversation
The DWARF specification states that the form of an exprloc consists of an unsigned LEB128 length value, followed by the encoded location bytes of the specified length. For some reason we were adding one to the length value being emitted. This looks incorrect to me. The above calculation for REG-REG (a variable stored in two registers) correctly calculates the length of each register type tag, plus the size of the interpolating PIECE tags, plus the size of notation for each register. The extra byte looks wrong. I've tested this locally and it appears to resolve dotnet/runtime#77407. Unfortunately, it also causes llvm-dwarfdump --verify to constantly complain about missing base addresses. I can't confirm at the moment, but my suspicion is that this is revealing an existing bug. Even if this is somehow causing a new bug, I think the resulting symbols with this change are better than the alternative (no working symbols at all). (cherry picked from commit b85b64b)
jeffschwMSFT
approved these changes
Nov 17, 2022
Member
jeffschwMSFT
left a comment
There was a problem hiding this comment.
approved. we should take for consideration in 7.0.x
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #317
Customer impact
Without this change, symbols are broken on Linux release builds such that no debugging is functional. Found in both manual testing and customer reported soon after shipping.
Testing
Manual testing. Verified using llvm-dwarfdump and gdb, and verified no Windows regression using Visual Studio.
Risk
Low. This is a focused change, and Windows has been scouted for regressions. It's probably not possible to make Linux worse than it already is.