-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
[NFC] [HWASan] make getAndroidSlotPtr function generic #86200
[NFC] [HWASan] make getAndroidSlotPtr function generic #86200
Conversation
Created using spr 1.3.4
@llvm/pr-subscribers-compiler-rt-sanitizer @llvm/pr-subscribers-llvm-transforms Author: Florian Mayer (fmayer) ChangesThis is so we can use a different slot for stack MTE. Full diff: https://github.com/llvm/llvm-project/pull/86200.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h b/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h
index 158c358a9e4618..fb3ab33a0629da 100644
--- a/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h
+++ b/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h
@@ -84,7 +84,7 @@ bool isLifetimeIntrinsic(Value *V);
Value *readRegister(IRBuilder<> &IRB, StringRef Name);
Value *getFP(IRBuilder<> &IRB);
Value *getPC(const Triple &TargetTriple, IRBuilder<> &IRB);
-Value *getAndroidSanitizerSlotPtr(IRBuilder<> &IRB);
+Value *getAndroidSlotPtr(IRBuilder<> &IRB, int Slot);
} // namespace memtag
} // namespace llvm
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 3c95610fa3e80b..3b0d99abd0cd0c 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1220,8 +1220,11 @@ Value *HWAddressSanitizer::untagPointer(IRBuilder<> &IRB, Value *PtrLong) {
}
Value *HWAddressSanitizer::getHwasanThreadSlotPtr(IRBuilder<> &IRB) {
+ // Android provides a fixed TLS slot for sanitizers. See TLS_SLOT_SANITIZER
+ // in Bionic's libc/platform/bionic/tls_defines.h.
+ constexpr int SanitizerSlot = 6;
if (TargetTriple.isAArch64() && TargetTriple.isAndroid())
- return memtag::getAndroidSanitizerSlotPtr(IRB);
+ return memtag::getAndroidSlotPtr(IRB, SanitizerSlot);
return ThreadPtrGlobal;
}
diff --git a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
index fd94a120bc6655..7b1eb70168d894 100644
--- a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
+++ b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
@@ -273,14 +273,14 @@ Value *getFP(IRBuilder<> &IRB) {
IRB.getIntPtrTy(M->getDataLayout()));
}
-Value *getAndroidSanitizerSlotPtr(IRBuilder<> &IRB) {
+Value *getAndroidSlotPtr(IRBuilder<> &IRB, int Slot) {
Module *M = IRB.GetInsertBlock()->getParent()->getParent();
// Android provides a fixed TLS slot for sanitizers. See TLS_SLOT_SANITIZER
// in Bionic's libc/private/bionic_tls.h.
Function *ThreadPointerFunc =
Intrinsic::getDeclaration(M, Intrinsic::thread_pointer);
return IRB.CreateConstGEP1_32(IRB.getInt8Ty(),
- IRB.CreateCall(ThreadPointerFunc), 0x30);
+ IRB.CreateCall(ThreadPointerFunc), 8 * Slot);
}
} // namespace memtag
|
@@ -273,14 +273,14 @@ Value *getFP(IRBuilder<> &IRB) { | |||
IRB.getIntPtrTy(M->getDataLayout())); | |||
} | |||
|
|||
Value *getAndroidSanitizerSlotPtr(IRBuilder<> &IRB) { | |||
Value *getAndroidSlotPtr(IRBuilder<> &IRB, int Slot) { | |||
Module *M = IRB.GetInsertBlock()->getParent()->getParent(); | |||
// Android provides a fixed TLS slot for sanitizers. See TLS_SLOT_SANITIZER | |||
// in Bionic's libc/private/bionic_tls.h. | |||
Function *ThreadPointerFunc = | |||
Intrinsic::getDeclaration(M, Intrinsic::thread_pointer); | |||
return IRB.CreateConstGEP1_32(IRB.getInt8Ty(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be better to use IRB.getIntPtrTy()
and avoid 8*
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, in which case getPtrTy
is probably more correct, because that's what it is in AOSP https://cs.android.com/android/platform/superproject/main/+/main:bionic/libc/private/bionic_tls.h;l=60;drc=f5f04b19fe59f6ac3f0b94fc890d0ccff6821fa8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That changes tests, which makes this non-NFC. Let's not do in this patch.
This is so we can use a different slot for stack MTE.