Commit af907708 by Nicolas Capens Committed by Nicolas Capens

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: 's avatarNicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarAlexis Hétu <sugoi@google.com>
parent b461e20f
...@@ -548,6 +548,7 @@ class ExternalSymbolGenerator : public llvm::orc::JITDylib::DefinitionGenerator ...@@ -548,6 +548,7 @@ class ExternalSymbolGenerator : public llvm::orc::JITDylib::DefinitionGenerator
// TODO(b/155148722): Remove when we no longer unpoison all writes. // TODO(b/155148722): Remove when we no longer unpoison all writes.
# if !REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION # if !REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION
functions.try_emplace("msan_unpoison", reinterpret_cast<void *>(__msan_unpoison)); functions.try_emplace("msan_unpoison", reinterpret_cast<void *>(__msan_unpoison));
functions.try_emplace("msan_unpoison_param", reinterpret_cast<void *>(__msan_unpoison_param));
# endif # endif
functions.try_emplace("emutls_get_address", reinterpret_cast<void *>(rr::getTLSAddress)); functions.try_emplace("emutls_get_address", reinterpret_cast<void *>(rr::getTLSAddress));
......
...@@ -516,13 +516,13 @@ Nucleus::Nucleus() ...@@ -516,13 +516,13 @@ Nucleus::Nucleus()
#if !__has_feature(memory_sanitizer) #if !__has_feature(memory_sanitizer)
// thread_local variables in shared libraries are initialized at load-time, // thread_local variables in shared libraries are initialized at load-time,
// but this is not observed by MemorySanitizer if the loader itself was not // 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(jit == nullptr);
ASSERT(Variable::unmaterializedVariables == nullptr); ASSERT(Variable::unmaterializedVariables == nullptr);
#endif #endif
jit = new JITBuilder(Nucleus::getDefaultConfig()); jit = new JITBuilder(Nucleus::getDefaultConfig());
Variable::unmaterializedVariables = new Variable::UnmaterializedVariables{}; Variable::unmaterializedVariables = new Variable::UnmaterializedVariables();
} }
Nucleus::~Nucleus() Nucleus::~Nucleus()
...@@ -3595,6 +3595,19 @@ RValue<Pointer<Byte>> ConstantData(void const *data, size_t size) ...@@ -3595,6 +3595,19 @@ RValue<Pointer<Byte>> ConstantData(void const *data, size_t size)
Value *Call(RValue<Pointer<Byte>> fptr, Type *retTy, std::initializer_list<Value *> args, std::initializer_list<Type *> argTys) 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(); RR_DEBUG_INFO_UPDATE_LOC();
llvm::SmallVector<llvm::Type *, 8> paramTys; llvm::SmallVector<llvm::Type *, 8> paramTys;
for(auto ty : argTys) { paramTys.push_back(T(ty)); } for(auto ty : argTys) { paramTys.push_back(T(ty)); }
......
...@@ -929,7 +929,7 @@ Nucleus::Nucleus() ...@@ -929,7 +929,7 @@ Nucleus::Nucleus()
#if !__has_feature(memory_sanitizer) #if !__has_feature(memory_sanitizer)
// thread_local variables in shared libraries are initialized at load-time, // thread_local variables in shared libraries are initialized at load-time,
// but this is not observed by MemorySanitizer if the loader itself was not // 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); ASSERT(Variable::unmaterializedVariables == nullptr);
#endif #endif
Variable::unmaterializedVariables = new Variable::UnmaterializedVariables{}; Variable::unmaterializedVariables = new Variable::UnmaterializedVariables{};
......
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