-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[scudo] Small cleanup of memory tagging code. #166860
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
Conversation
Make the systemSupportsMemoryTagging() function return even on system that don't support memory tagging. This avoids the need to always check if memory tagging is supported before calling th function. Make systemSupportsMemoryTagging() cache the getauxval return value instead of calling the function every time. Updated the code that calls systemSupportsMemoryTagging().
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Christopher Ferris (cferris1000) ChangesMake the systemSupportsMemoryTagging() function return even on system that don't support memory tagging. This avoids the need to always check if memory tagging is supported before calling th function. Make systemSupportsMemoryTagging() cache the getauxval return value instead of calling the function every time. Updated the code that calls systemSupportsMemoryTagging(). Full diff: https://github.com/llvm/llvm-project/pull/166860.diff 5 Files Affected:
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index ffe9554203241..2d0f4f53d2ac5 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -171,8 +171,7 @@ class Allocator {
Primary.Options.set(OptionBit::DeallocTypeMismatch);
if (getFlags()->delete_size_mismatch)
Primary.Options.set(OptionBit::DeleteSizeMismatch);
- if (allocatorSupportsMemoryTagging<AllocatorConfig>() &&
- systemSupportsMemoryTagging())
+ if (systemSupportsMemoryTagging())
Primary.Options.set(OptionBit::UseMemoryTagging);
QuarantineMaxChunkSize =
diff --git a/compiler-rt/lib/scudo/standalone/memtag.h b/compiler-rt/lib/scudo/standalone/memtag.h
index 83ebe676433eb..52897b3dca6ae 100644
--- a/compiler-rt/lib/scudo/standalone/memtag.h
+++ b/compiler-rt/lib/scudo/standalone/memtag.h
@@ -66,7 +66,8 @@ inline bool systemSupportsMemoryTagging() {
#ifndef HWCAP2_MTE
#define HWCAP2_MTE (1 << 18)
#endif
- return getauxval(AT_HWCAP2) & HWCAP2_MTE;
+ static bool SupportsMemoryTagging = getauxval(AT_HWCAP2) & HWCAP2_MTE;
+ return SupportsMemoryTagging;
}
inline bool systemDetectsMemoryTagFaultsTestOnly() {
@@ -261,9 +262,7 @@ inline uptr loadTag(uptr Ptr) {
#else
-inline NORETURN bool systemSupportsMemoryTagging() {
- UNREACHABLE("memory tagging not supported");
-}
+inline bool systemSupportsMemoryTagging() { return false; }
inline NORETURN bool systemDetectsMemoryTagFaultsTestOnly() {
UNREACHABLE("memory tagging not supported");
diff --git a/compiler-rt/lib/scudo/standalone/tests/memtag_test.cpp b/compiler-rt/lib/scudo/standalone/tests/memtag_test.cpp
index 09093e11452dd..d0d93316f212e 100644
--- a/compiler-rt/lib/scudo/standalone/tests/memtag_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/memtag_test.cpp
@@ -28,7 +28,6 @@ TEST(MemtagBasicDeathTest, Unsupported) {
EXPECT_DEATH(untagPointer((uptr)0), "not supported");
EXPECT_DEATH(extractTag((uptr)0), "not supported");
- EXPECT_DEATH(systemSupportsMemoryTagging(), "not supported");
EXPECT_DEATH(systemDetectsMemoryTagFaultsTestOnly(), "not supported");
EXPECT_DEATH(enableSystemMemoryTaggingTestOnly(), "not supported");
diff --git a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
index 855a3e6e6109f..8741c8299b57c 100644
--- a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
@@ -27,9 +27,7 @@
const scudo::uptr PageSize = scudo::getPageSizeCached();
template <typename Config> static scudo::Options getOptionsForConfig() {
- if (!Config::getMaySupportMemoryTagging() ||
- !scudo::archSupportsMemoryTagging() ||
- !scudo::systemSupportsMemoryTagging())
+ if (!scudo::systemSupportsMemoryTagging())
return {};
scudo::AtomicOptions AO;
AO.set(scudo::OptionBit::UseMemoryTagging);
diff --git a/compiler-rt/lib/scudo/standalone/tests/wrappers_cpp_test.cpp b/compiler-rt/lib/scudo/standalone/tests/wrappers_cpp_test.cpp
index c802ed22fbad0..84359251a02aa 100644
--- a/compiler-rt/lib/scudo/standalone/tests/wrappers_cpp_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/wrappers_cpp_test.cpp
@@ -187,8 +187,7 @@ TEST_F(ScudoWrappersCppTest, ThreadedNew) {
// TODO: Investigate why libc sometimes crashes with tag missmatch in
// __pthread_clockjoin_ex.
std::unique_ptr<scudo::ScopedDisableMemoryTagChecks> NoTags;
- if (!SCUDO_ANDROID && scudo::archSupportsMemoryTagging() &&
- scudo::systemSupportsMemoryTagging())
+ if (!SCUDO_ANDROID && scudo::systemSupportsMemoryTagging())
NoTags = std::make_unique<scudo::ScopedDisableMemoryTagChecks>();
Ready = false;
|
|
Ping. |
| #define HWCAP2_MTE (1 << 18) | ||
| #endif | ||
| return getauxval(AT_HWCAP2) & HWCAP2_MTE; | ||
| static bool SupportsMemoryTagging = getauxval(AT_HWCAP2) & HWCAP2_MTE; |
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.
Oh,
my thought during review: "nice! static locals are supported now" :)
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.
CC @pcc
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.
Memory tagging support status is deliberately cached in the UseMemoryTagging bit instead of here to avoid issues with static locals such as those pointed out by the buildbot.
|
@cferris1000 -- We are starting to hit the same problems pointed out by @vitalybuka here https://lab.llvm.org/buildbot/#/builders/24/builds/14562 in our Google toolchain builders as well. |
|
Let me revert this and I'll fix this. |
This reverts commit 046ae85.
Reverts #166860 The local static variable causes build failures.
…67425) Reverts llvm/llvm-project#166860 The local static variable causes build failures.
Make the systemSupportsMemoryTagging() function return even on system that don't support memory tagging. This avoids the need to always check if memory tagging is supported before calling th function.
Make systemSupportsMemoryTagging() cache the getauxval return value instead of calling the function every time.
Updated the code that calls systemSupportsMemoryTagging().