Skip to content

S
swiftshader
Commit dc8cbfa4 by Nicolas Capens Committed by swiftshader-scoped@luci-project-accounts.iam.gserviceaccount.com
Options

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: 's avatarNicolas Capens <nicolascapens@google.com> Presubmit-Ready: Nicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarAlexis Hétu <sugoi@google.com> Commit-Queue: Nicolas Capens <nicolascapens@google.com>
parent 951192f3
Showing with 20 additions and 18 deletions
src/Reactor/LLVMJIT.cpp
...@@ -55,10 +55,8 @@ extern "C" signed __aeabi_idivmod(); ...@@ -55,10 +55,8 @@ extern "C" signed __aeabi_idivmod();
#if __has_feature(memory_sanitizer) #if __has_feature(memory_sanitizer)
// TODO(b/155148722): Remove when we no longer unpoison all writes. // TODO(b/155148722): Remove when we no longer unpoison any writes.
# if !REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION # include "sanitizer/msan_interface.h"
# include "sanitizer/msan_interface.h"
# endif
# include <dlfcn.h> // dlsym() # include <dlfcn.h> // dlsym()
...@@ -552,18 +550,15 @@ class ExternalSymbolGenerator : public llvm::orc::JITDylib::DefinitionGenerator ...@@ -552,18 +550,15 @@ class ExternalSymbolGenerator : public llvm::orc::JITDylib::DefinitionGenerator
# endif # endif
#endif #endif
#if __has_feature(memory_sanitizer) #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_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_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_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_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))); 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 #endif
} }
}; };
...@@ -827,6 +822,11 @@ JITBuilder::JITBuilder(const rr::Config &config) ...@@ -827,6 +822,11 @@ JITBuilder::JITBuilder(const rr::Config &config)
{ {
module->setTargetTriple(LLVM_DEFAULT_TARGET_TRIPLE); module->setTargetTriple(LLVM_DEFAULT_TARGET_TRIPLE);
module->setDataLayout(JITGlobals::get()->getDataLayout()); module->setDataLayout(JITGlobals::get()->getDataLayout());
if(REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION)
{
msanInstrumentation = true;
}
} }
void JITBuilder::optimize(const rr::Config &cfg) void JITBuilder::optimize(const rr::Config &cfg)
...@@ -840,12 +840,10 @@ void JITBuilder::optimize(const rr::Config &cfg) ...@@ -840,12 +840,10 @@ void JITBuilder::optimize(const rr::Config &cfg)
llvm::legacy::PassManager passManager; llvm::legacy::PassManager passManager;
#if REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION if(__has_feature(memory_sanitizer) && msanInstrumentation)
if(__has_feature(memory_sanitizer))
{ {
passManager.add(llvm::createMemorySanitizerLegacyPassPass()); passManager.add(llvm::createMemorySanitizerLegacyPassPass());
} }
#endif
for(auto pass : cfg.getOptimization().getPasses()) for(auto pass : cfg.getOptimization().getPasses())
{ {
......
src/Reactor/LLVMReactor.cpp
...@@ -999,7 +999,7 @@ Value *Nucleus::createStore(Value *value, Value *ptr, Type *type, bool isVolatil ...@@ -999,7 +999,7 @@ Value *Nucleus::createStore(Value *value, Value *ptr, Type *type, bool isVolatil
auto elTy = T(type); auto elTy = T(type);
ASSERT(V(ptr)->getType()->getContainedType(0) == elTy); 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 // Mark all memory writes as initialized by calling __msan_unpoison
// void __msan_unpoison(const volatile void *a, size_t size) // void __msan_unpoison(const volatile void *a, size_t size)
...@@ -1105,7 +1105,7 @@ void Nucleus::createMaskedStore(Value *ptr, Value *val, Value *mask, unsigned in ...@@ -1105,7 +1105,7 @@ void Nucleus::createMaskedStore(Value *ptr, Value *val, Value *mask, unsigned in
auto func = llvm::Intrinsic::getDeclaration(jit->module.get(), llvm::Intrinsic::masked_store, { elVecTy, elVecPtrTy }); auto func = llvm::Intrinsic::getDeclaration(jit->module.get(), llvm::Intrinsic::masked_store, { elVecTy, elVecPtrTy });
jit->builder->CreateCall(func, { V(val), V(ptr), align, i1Mask }); 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 // Mark memory writes as initialized by calling __msan_unpoison
// void __msan_unpoison(const volatile void *a, size_t size) // void __msan_unpoison(const volatile void *a, size_t size)
...@@ -3613,7 +3613,7 @@ Value *Call(RValue<Pointer<Byte>> fptr, Type *retTy, std::initializer_list<Value ...@@ -3613,7 +3613,7 @@ Value *Call(RValue<Pointer<Byte>> fptr, Type *retTy, std::initializer_list<Value
{ {
// If this is a MemorySanitizer build, but Reactor routine instrumentation is not enabled, // If this is a MemorySanitizer build, but Reactor routine instrumentation is not enabled,
// mark all call arguments as initialized by calling __msan_unpoison_param(). // 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) // void __msan_unpoison_param(size_t n)
auto voidTy = llvm::Type::getVoidTy(*jit->context); auto voidTy = llvm::Type::getVoidTy(*jit->context);
......
src/Reactor/LLVMReactor.hpp
...@@ -118,6 +118,8 @@ public: ...@@ -118,6 +118,8 @@ public:
#ifdef ENABLE_RR_DEBUG_INFO #ifdef ENABLE_RR_DEBUG_INFO
std::unique_ptr<rr::DebugInfo> debugInfo; std::unique_ptr<rr::DebugInfo> debugInfo;
#endif #endif
bool msanInstrumentation = false;
}; };
inline std::memory_order atomicOrdering(llvm::AtomicOrdering memoryOrder) inline std::memory_order atomicOrdering(llvm::AtomicOrdering memoryOrder)
......
tests/ReactorUnitTests/ReactorUnitTests.cpp
...@@ -136,6 +136,7 @@ TEST(ReactorUnitTests, Trampoline) ...@@ -136,6 +136,7 @@ TEST(ReactorUnitTests, Trampoline)
EXPECT_EQ(result, 80); EXPECT_EQ(result, 80);
} }
#if REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION
TEST(ReactorUnitTests, Uninitialized) TEST(ReactorUnitTests, Uninitialized)
{ {
FunctionT<int()> function; FunctionT<int()> function;
...@@ -159,7 +160,7 @@ TEST(ReactorUnitTests, Uninitialized) ...@@ -159,7 +160,7 @@ TEST(ReactorUnitTests, Uninitialized)
auto routine = function(testName().c_str()); auto routine = function(testName().c_str());
if(!__has_feature(memory_sanitizer) || !REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION) if(!__has_feature(memory_sanitizer))
{ {
int result = routine(); int result = routine();
EXPECT_EQ(result, result); // Anything is fine, just don't crash EXPECT_EQ(result, result); // Anything is fine, just don't crash
...@@ -178,6 +179,7 @@ TEST(ReactorUnitTests, Uninitialized) ...@@ -178,6 +179,7 @@ TEST(ReactorUnitTests, Uninitialized)
"MemorySanitizer: use-of-uninitialized-value"); "MemorySanitizer: use-of-uninitialized-value");
} }
} }
#endif
TEST(ReactorUnitTests, Unreachable) TEST(ReactorUnitTests, Unreachable)
{ {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment