-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[android] Move the string and other tags in pointers to the second byte because Android enabled memory tagging #40779
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2681,9 +2681,16 @@ emitMetadataAccessByMangledName(IRGenFunction &IGF, CanType type, | |
unsigned mangledStringSize; | ||
std::tie(mangledString, mangledStringSize) = | ||
IGM.getTypeRef(type, CanGenericSignature(), MangledTypeRefRole::Metadata); | ||
|
||
assert(mangledStringSize < 0x80000000u | ||
&& "2GB of mangled name ought to be enough for anyone"); | ||
|
||
// Android AArch64 reserves the top byte of the address for memory tagging | ||
// since Android 11, so only use the bottom 23 bits to store this size | ||
// and the 24th bit to signal that there is a size. | ||
if (IGM.Triple.isAndroid() && IGM.Triple.getArch() == llvm::Triple::aarch64) | ||
assert(mangledStringSize < 0x00800001u && | ||
"8MB of mangled name ought to be enough for Android AArch64"); | ||
else | ||
assert(mangledStringSize < 0x80000000u && | ||
"2GB of mangled name ought to be enough for anyone"); | ||
|
||
// Get or create the cache variable if necessary. | ||
auto cache = IGM.getAddrOfTypeMetadataDemanglingCacheVariable(type, | ||
|
@@ -2753,6 +2760,21 @@ emitMetadataAccessByMangledName(IRGenFunction &IGF, CanType type, | |
auto contBB = subIGF.createBasicBlock(""); | ||
llvm::Value *comparison = subIGF.Builder.CreateICmpSLT(load, | ||
llvm::ConstantInt::get(IGM.Int64Ty, 0)); | ||
|
||
// Check if the 24th bit is set on Android AArch64 and only instantiate the | ||
// type metadata if it is, as otherwise it might be negative only because | ||
// of the memory tag on Android. | ||
if (IGM.Triple.isAndroid() && | ||
IGM.Triple.getArch() == llvm::Triple::aarch64) { | ||
|
||
auto getBitAfterAndroidTag = subIGF.Builder.CreateAnd( | ||
load, llvm::ConstantInt::get(IGM.Int64Ty, 0x0080000000000000)); | ||
auto checkNotAndroidTag = subIGF.Builder.CreateICmpNE( | ||
getBitAfterAndroidTag, llvm::ConstantInt::get(IGM.Int64Ty, 0)); | ||
|
||
comparison = subIGF.Builder.CreateAnd(comparison, checkNotAndroidTag); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reworked the metadata patch I linked before and have it working for an arbitrary Android tag in the top byte, by using the scheme described in the comments. Strictly speaking, only the 24th bit matters on Android AArch64 with this new scheme, but I have it checking the 32nd "sign bit" too. This pull now gets trunk building and passing all but the one test I linked before, both on devices with and without pointer tagging. |
||
} | ||
|
||
comparison = subIGF.Builder.CreateExpect(comparison, | ||
llvm::ConstantInt::get(IGM.Int1Ty, 0)); | ||
subIGF.Builder.CreateCondBr(comparison, isUnfilledBB, contBB); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,10 +36,17 @@ static void setToMask(SpareBitVector &bits, unsigned size, uint64_t mask) { | |
/// Configures target-specific information for arm64 platforms. | ||
static void configureARM64(IRGenModule &IGM, const llvm::Triple &triple, | ||
SwiftTargetInfo &target) { | ||
setToMask(target.PointerSpareBits, 64, | ||
SWIFT_ABI_ARM64_SWIFT_SPARE_BITS_MASK); | ||
setToMask(target.ObjCPointerReservedBits, 64, | ||
SWIFT_ABI_ARM64_OBJC_RESERVED_BITS_MASK); | ||
if (triple.isAndroid()) { | ||
setToMask(target.PointerSpareBits, 64, | ||
SWIFT_ABI_ANDROID_ARM64_SWIFT_SPARE_BITS_MASK); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated to use the right tag mask at runtime, as selecting it at compile-time as I had it before wouldn't apply the right mask when cross-compiling, and rebased. |
||
setToMask(target.ObjCPointerReservedBits, 64, | ||
SWIFT_ABI_ANDROID_ARM64_OBJC_RESERVED_BITS_MASK); | ||
} else { | ||
setToMask(target.PointerSpareBits, 64, | ||
SWIFT_ABI_ARM64_SWIFT_SPARE_BITS_MASK); | ||
setToMask(target.ObjCPointerReservedBits, 64, | ||
SWIFT_ABI_ARM64_OBJC_RESERVED_BITS_MASK); | ||
} | ||
setToMask(target.IsObjCPointerBit, 64, SWIFT_ABI_ARM64_IS_OBJC_BIT); | ||
|
||
if (triple.isOSDarwin()) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -157,12 +157,22 @@ static_assert(alignof(HeapObject) == alignof(void*), | |
#endif | ||
#define _swift_abi_SwiftSpareBitsMask \ | ||
(__swift_uintptr_t) SWIFT_ABI_ARM64_SWIFT_SPARE_BITS_MASK | ||
#if defined(__ANDROID__) | ||
#define _swift_abi_ObjCReservedBitsMask \ | ||
(__swift_uintptr_t) SWIFT_ABI_ANDROID_ARM64_OBJC_RESERVED_BITS_MASK | ||
#else | ||
#define _swift_abi_ObjCReservedBitsMask \ | ||
(__swift_uintptr_t) SWIFT_ABI_ARM64_OBJC_RESERVED_BITS_MASK | ||
#endif | ||
#define _swift_abi_ObjCReservedLowBits \ | ||
(unsigned) SWIFT_ABI_ARM64_OBJC_NUM_RESERVED_LOW_BITS | ||
#if defined(__ANDROID__) | ||
#define _swift_BridgeObject_TaggedPointerBits \ | ||
(__swift_uintptr_t) SWIFT_ABI_DEFAULT_BRIDGEOBJECT_TAG_64 >> 8 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we make a separate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could, but note that this is only shifted for AArch64, so I'd have to define it only for that. I figured shifting it here is the best way to set that up, but I could add another constant instead if you like. |
||
#else | ||
#define _swift_BridgeObject_TaggedPointerBits \ | ||
(__swift_uintptr_t) SWIFT_ABI_DEFAULT_BRIDGEOBJECT_TAG_64 | ||
#endif | ||
|
||
#elif defined(__powerpc64__) | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.