Unpoison Reactor Call() arguments when MSan instrumentation is disabled
For MemorySanitizer builds which do not have Reactor routine
instrumentation enabled, mark all Call() arguments as initialized by
calling __msan_unpoison_param(). This prevents false positives.
Note that we also already unpoisoned all memory writes, when proper
instrumentation is not enabled. While it's preferable to set
REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION=1 to avoid the false
negatives caused by unpoisoning, it currently causes slowdowns which
result in timeouts.
Bug: b/187467599
Change-Id: I5fdcbceb5cd795f5e3684df0cdc1ba6c1bef06e9
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/54348
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/Reactor/LLVMJIT.cpp b/src/Reactor/LLVMJIT.cpp
index 5c0f15d..2d9a801 100644
--- a/src/Reactor/LLVMJIT.cpp
+++ b/src/Reactor/LLVMJIT.cpp
@@ -548,6 +548,7 @@
// 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));
diff --git a/src/Reactor/LLVMReactor.cpp b/src/Reactor/LLVMReactor.cpp
index 8564f8a..4d78d58 100644
--- a/src/Reactor/LLVMReactor.cpp
+++ b/src/Reactor/LLVMReactor.cpp
@@ -516,13 +516,13 @@
#if !__has_feature(memory_sanitizer)
// thread_local variables in shared libraries are initialized at load-time,
// but this is not observed by MemorySanitizer if the loader itself was not
- // instrumented, leading to false-positive unitialized variable errors.
+ // instrumented, leading to false-positive uninitialized variable errors.
ASSERT(jit == nullptr);
ASSERT(Variable::unmaterializedVariables == nullptr);
#endif
jit = new JITBuilder(Nucleus::getDefaultConfig());
- Variable::unmaterializedVariables = new Variable::UnmaterializedVariables{};
+ Variable::unmaterializedVariables = new Variable::UnmaterializedVariables();
}
Nucleus::~Nucleus()
@@ -3595,6 +3595,19 @@
Value *Call(RValue<Pointer<Byte>> fptr, Type *retTy, std::initializer_list<Value *> args, std::initializer_list<Type *> argTys)
{
+ // 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)
+ {
+ // void __msan_unpoison_param(size_t n)
+ auto voidTy = llvm::Type::getVoidTy(*jit->context);
+ auto sizetTy = llvm::IntegerType::get(*jit->context, sizeof(size_t) * 8);
+ auto funcTy = llvm::FunctionType::get(voidTy, { sizetTy }, false);
+ auto func = jit->module->getOrInsertFunction("__msan_unpoison_param", funcTy);
+
+ jit->builder->CreateCall(func, { llvm::ConstantInt::get(sizetTy, args.size()) });
+ }
+
RR_DEBUG_INFO_UPDATE_LOC();
llvm::SmallVector<llvm::Type *, 8> paramTys;
for(auto ty : argTys) { paramTys.push_back(T(ty)); }
diff --git a/src/Reactor/SubzeroReactor.cpp b/src/Reactor/SubzeroReactor.cpp
index 301c6bd..8799eb8 100644
--- a/src/Reactor/SubzeroReactor.cpp
+++ b/src/Reactor/SubzeroReactor.cpp
@@ -929,7 +929,7 @@
#if !__has_feature(memory_sanitizer)
// thread_local variables in shared libraries are initialized at load-time,
// but this is not observed by MemorySanitizer if the loader itself was not
- // instrumented, leading to false-positive unitialized variable errors.
+ // instrumented, leading to false-positive uninitialized variable errors.
ASSERT(Variable::unmaterializedVariables == nullptr);
#endif
Variable::unmaterializedVariables = new Variable::UnmaterializedVariables{};
