[MC] [Win64EH] Clarify the comment about a skipped case of packed unwind info #169784
Conversation
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
Member
|
@llvm/pr-subscribers-llvm-mc Author: Martin Storsjö (mstorsjo) ChangesClarify that the issue that is being worked around by 924defa (discussed in #54879) is fixed in the latest versions, and reference versions that still have the bug present. As long as we can't be sure the generated code won't run on older versions, the workaround still stands though (and it is unlikely that we'd ever really hit this case in practice - so there's not much gain to be had by removing the workaround). Full diff: https://github.com/llvm/llvm-project/pull/169784.diff 1 Files Affected:
diff --git a/llvm/lib/MC/MCWin64EH.cpp b/llvm/lib/MC/MCWin64EH.cpp
index 6d146f6cedd6e..ca457c0db8b8e 100644
--- a/llvm/lib/MC/MCWin64EH.cpp
+++ b/llvm/lib/MC/MCWin64EH.cpp
@@ -1260,6 +1260,8 @@ static bool tryARM64PackedUnwind(WinEH::FrameInfo *info, uint32_t FuncLength,
// RtlVirtualUnwind behaves as if it does expect the epilogue to contain
// the same nops. See https://github.com/llvm/llvm-project/issues/54879.
// To play it safe, don't produce packed unwind info with homed parameters.
+ // This seems to be fixed in 10.0.26100.6899, but was still broken at least
+ // in 10.0.22000.2176.
if (H)
return false;
int IntSZ = 8 * RegI;
|
mstorsjo
force-pushed
the
arm64-unwind-workaround-version
branch
from
50aa5ae to
725194a
Compare
mstorsjo
changed the title
…ind info Clarify the comment from 924defa. There is no longer any ambiguity about this case; newer versions of Windows correctly match the documentation, making it clear that the older versions were incorrect. Mention specific versions that have and don't have the inconsistency. Even if we wouldn't care about the older versions of Windows, we can't enable this case of unwind info packing, unless the implementation also is changed to match for asymmetrical prologs/epilogs.
mstorsjo
force-pushed
the
arm64-unwind-workaround-version
branch
from
725194a to
f0f1b5b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Clarify the comment from 924defa. There is no longer any ambiguity about this case; newer versions of Windows correctly match the documentation, making it clear that the older versions were incorrect. Mention specific versions that have and don't have the inconsistency.
Even if we wouldn't care about the older versions of Windows, we can't enable this case of unwind info packing, unless the implementation also is changed to match for asymmetrical prologs/epilogs.