Determine MSan instrumentation for Reactor at run-time
Previously the REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION macro
definition controlled whether we compiled the code for properly
instrumenting Reactor routines with the MemorySanitizer LLVM pass, or
compile the code to unpoison all memory writes.
This change makes us compile the code for both options, and select
between the two at run-time. Currently this has no net effect, but it
will allow selectively enabling MSan instrumentation to be done even
when REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION is not defined (or
false). Thus the define now means enabling instrumentation to always be
performed. This way we'll be able to gradually enable more use of
MSan instrumentation, where not already enforced by the build define.
The ReactorUnitTests.Uninitialized test was changed to only run on build
that have REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION set, since it
already assumed MSan builds to always have instrumentation enabled.
Bug: b/188205704
Change-Id: I54056ddc9b1d55fabbde6ae0f02d6c8cea3afad6
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/54888
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
Commit-Queue: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/Reactor/LLVMJIT.cpp b/src/Reactor/LLVMJIT.cpp
index 3f25295..64dd21e 100644
--- a/src/Reactor/LLVMJIT.cpp
+++ b/src/Reactor/LLVMJIT.cpp
@@ -55,10 +55,8 @@
#if __has_feature(memory_sanitizer)
-// TODO(b/155148722): Remove when we no longer unpoison all writes.
-# if !REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION
-# include "sanitizer/msan_interface.h"
-# endif
+// TODO(b/155148722): Remove when we no longer unpoison any writes.
+# include "sanitizer/msan_interface.h"
# include <dlfcn.h> // dlsym()
@@ -552,18 +550,15 @@
# endif
#endif
#if __has_feature(memory_sanitizer)
-
-// TODO(b/155148722): Remove when we no longer unpoison all writes.
-# if !REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION
- functions.try_emplace("msan_unpoison", reinterpret_cast<void *>(__msan_unpoison));
- functions.try_emplace("msan_unpoison_param", reinterpret_cast<void *>(__msan_unpoison_param));
-# endif
-
functions.try_emplace("emutls_get_address", reinterpret_cast<void *>(rr::getTLSAddress));
functions.try_emplace("emutls_v.__msan_retval_tls", reinterpret_cast<void *>(static_cast<uintptr_t>(rr::MSanTLS::retval)));
functions.try_emplace("emutls_v.__msan_param_tls", reinterpret_cast<void *>(static_cast<uintptr_t>(rr::MSanTLS::param)));
functions.try_emplace("emutls_v.__msan_va_arg_tls", reinterpret_cast<void *>(static_cast<uintptr_t>(rr::MSanTLS::va_arg)));
functions.try_emplace("emutls_v.__msan_va_arg_overflow_size_tls", reinterpret_cast<void *>(static_cast<uintptr_t>(rr::MSanTLS::va_arg_overflow_size)));
+
+ // TODO(b/155148722): Remove when we no longer unpoison any writes.
+ functions.try_emplace("msan_unpoison", reinterpret_cast<void *>(__msan_unpoison));
+ functions.try_emplace("msan_unpoison_param", reinterpret_cast<void *>(__msan_unpoison_param));
#endif
}
};
@@ -827,6 +822,11 @@
{
module->setTargetTriple(LLVM_DEFAULT_TARGET_TRIPLE);
module->setDataLayout(JITGlobals::get()->getDataLayout());
+
+ if(REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION)
+ {
+ msanInstrumentation = true;
+ }
}
void JITBuilder::optimize(const rr::Config &cfg)
@@ -840,12 +840,10 @@
llvm::legacy::PassManager passManager;
-#if REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION
- if(__has_feature(memory_sanitizer))
+ if(__has_feature(memory_sanitizer) && msanInstrumentation)
{
passManager.add(llvm::createMemorySanitizerLegacyPassPass());
}
-#endif
for(auto pass : cfg.getOptimization().getPasses())
{
diff --git a/src/Reactor/LLVMReactor.cpp b/src/Reactor/LLVMReactor.cpp
index c9569b9..72b45dd 100644
--- a/src/Reactor/LLVMReactor.cpp
+++ b/src/Reactor/LLVMReactor.cpp
@@ -999,7 +999,7 @@
auto elTy = T(type);
ASSERT(V(ptr)->getType()->getContainedType(0) == elTy);
- if(__has_feature(memory_sanitizer) && !REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION)
+ if(__has_feature(memory_sanitizer) && !jit->msanInstrumentation)
{
// Mark all memory writes as initialized by calling __msan_unpoison
// void __msan_unpoison(const volatile void *a, size_t size)
@@ -1105,7 +1105,7 @@
auto func = llvm::Intrinsic::getDeclaration(jit->module.get(), llvm::Intrinsic::masked_store, { elVecTy, elVecPtrTy });
jit->builder->CreateCall(func, { V(val), V(ptr), align, i1Mask });
- if(__has_feature(memory_sanitizer) && !REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION)
+ if(__has_feature(memory_sanitizer) && !jit->msanInstrumentation)
{
// Mark memory writes as initialized by calling __msan_unpoison
// void __msan_unpoison(const volatile void *a, size_t size)
@@ -3613,7 +3613,7 @@
{
// If this is a MemorySanitizer build, but Reactor routine instrumentation is not enabled,
// mark all call arguments as initialized by calling __msan_unpoison_param().
- if(__has_feature(memory_sanitizer) && !REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION)
+ if(__has_feature(memory_sanitizer) && !jit->msanInstrumentation)
{
// void __msan_unpoison_param(size_t n)
auto voidTy = llvm::Type::getVoidTy(*jit->context);
diff --git a/src/Reactor/LLVMReactor.hpp b/src/Reactor/LLVMReactor.hpp
index 181fbe7..54a44c3 100644
--- a/src/Reactor/LLVMReactor.hpp
+++ b/src/Reactor/LLVMReactor.hpp
@@ -118,6 +118,8 @@
#ifdef ENABLE_RR_DEBUG_INFO
std::unique_ptr<rr::DebugInfo> debugInfo;
#endif
+
+ bool msanInstrumentation = false;
};
inline std::memory_order atomicOrdering(llvm::AtomicOrdering memoryOrder)
diff --git a/tests/ReactorUnitTests/ReactorUnitTests.cpp b/tests/ReactorUnitTests/ReactorUnitTests.cpp
index 79fd5d4..499608e 100644
--- a/tests/ReactorUnitTests/ReactorUnitTests.cpp
+++ b/tests/ReactorUnitTests/ReactorUnitTests.cpp
@@ -136,6 +136,7 @@
EXPECT_EQ(result, 80);
}
+#if REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION
TEST(ReactorUnitTests, Uninitialized)
{
FunctionT<int()> function;
@@ -159,7 +160,7 @@
auto routine = function(testName().c_str());
- if(!__has_feature(memory_sanitizer) || !REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION)
+ if(!__has_feature(memory_sanitizer))
{
int result = routine();
EXPECT_EQ(result, result); // Anything is fine, just don't crash
@@ -178,6 +179,7 @@
"MemorySanitizer: use-of-uninitialized-value");
}
}
+#endif
TEST(ReactorUnitTests, Unreachable)
{
