Fix multi-payload enums with Class Existential payloads on 32-bit targets #77027
Conversation
This file contains 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
…gets Class existentials expose spare bits from all of the pointers, not just the first one. Due to a bad bug here, we were properly exposing spare bits from the first pointer, but then claiming that all bits of subsequent pointers were spare. This accidentally resulted in the correct operation on 64-bit targets (it picked the highest-order spare bit, which happened to be spare in both the broken mask and the correct mask). But on 32-bit targets, this exposed the high-order bits of pointers, which is incorrect. Expand the test a bit while we're here as well. Resolves rdar://132715829
|
@swift-ci Please test |
tbkka
commented
Oct 15, 2024
| auto mpePointerSpareBits = TC.getBuilder().getMultiPayloadEnumPointerMask(); | ||
| mask.andMask(mpePointerSpareBits, 0); | ||
| mask.keepOnlyLeastSignificantBytes(TC.targetPointerSize()); |
There was a problem hiding this comment.
The truncation in the previous code here inadvertently marked everything beyond the first pointer as spare bits. This accidentally worked for 64-bit because MPEs pick the most-significant spare bit.
tbkka
commented
Oct 15, 2024
| mask.keepOnlyLeastSignificantBytes(TC.targetPointerSize()); | ||
| auto mpePointerSpareBitMask = BitMask(TC.targetPointerSize(), mpePointerSpareBits); | ||
| for (int offset = 0; offset < (int)getSize(); offset += TC.targetPointerSize()) { | ||
| mask.andMask(mpePointerSpareBitMask, offset); |
There was a problem hiding this comment.
In the new code, we loop to apply the standard pointer spare bit mask to every pointer-sized field.
tbkka
commented
Oct 15, 2024
| @@ -10,9 +10,6 @@ | |||
| // UNSUPPORTED: use_os_stdlib | |||
| // UNSUPPORTED: asan | |||
|
|
|||
| // This is broken on ARM64_32, disable it temporarily until we can fix it. rdar://137351613 | |||
| // UNSUPPORTED: CPU=arm64_32 | |||
There was a problem hiding this comment.
Yay! It works for 32-bit watchOS now.
mikeash
approved these changes
Oct 15, 2024
|
@swift-ci Please test macOS Platform |
1 similar comment
|
@swift-ci Please test macOS Platform |
The first word in a class existential is the class pointer itself. This pointer exposes spare bits differently depending on the platform, which becomes apparent when you try to reflect an Optional carrying such an MPE. Add new test cases and some logic to zero out the first word of spare bit information only on platforms with 8-byte pointers.
tbkka
commented
Oct 16, 2024
| // itself, which can be tagged or have other mysteries on 64-bit, so | ||
| // it exposes no spare bits from the first word there... | ||
| auto pointerBytes = TC.targetPointerSize(); | ||
| if (pointerBytes == 8) { |
There was a problem hiding this comment.
If anyone is trying to get RemoteMirror to work on non-Darwin, they may need to tweak this some more.
|
@swift-ci Please test |
|
@swift-ci Please test |
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.
Class existentials expose spare bits from all of the pointers, not just the first one. Due to a bad bug here, we were properly exposing spare bits from the first pointer, but then claiming that all bits of subsequent pointers were spare. This accidentally resulted in the correct operation on 64-bit targets (it picked the highest-order spare bit, which happened to be spare in both the broken mask and the correct mask). But on 32-bit targets, this exposed the high-order bits of pointers, which is incorrect.
Expand the test a bit while we're here as well.
Resolves rdar://132715829