commit dc5bb2195a9ab47c0ffab51364c2e18cee90a18c
author Nicolas Capens <capn@google.com> Mon Nov 02 23:01:59 2020 -0500
committer Nicolas Capens <nicolascapens@google.com> Thu Feb 18 17:16:22 2021 +0000
tree cdee1e18bbe814241d2cc12e8ce610a90e07e521
parent f890890f3830edfd6072d3b37f44f3b6b841038f

Disable MSan unpoisoning of memory writes

Prior to supporting MemorySanitizer instrumentation of Reactor routines,
false positives in C++ code were avoided by marking all memory that was
written to by the Reactor routines as unpoisoned.

Now this can be disabled to avoid false negatives, i.e. uses of
uninitialized data either in the Reactor routine or originating from it.

REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION controls whether we get
the new behavior or the old unpoisoning.

Bug: b/155148722
Change-Id: Ia35e3cdc1a60ba44045869884c64e9875c030291
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/49889
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>

src/Reactor/LLVMJIT.cpp

diff --git a/src/Reactor/LLVMJIT.cpp b/src/Reactor/LLVMJIT.cpp
index eaee17b..910cea8 100644
--- a/src/Reactor/LLVMJIT.cpp
+++ b/src/Reactor/LLVMJIT.cpp
@@ -54,7 +54,11 @@
 #endif
 
 #if __has_feature(memory_sanitizer)
-#	include "sanitizer/msan_interface.h"  // 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
+#		include "sanitizer/msan_interface.h"
+#	endif
 
 #	include <dlfcn.h>  // dlsym()
 
@@ -540,7 +544,11 @@
 #	endif
 #endif
 #if __has_feature(memory_sanitizer)
-			functions.try_emplace("msan_unpoison", reinterpret_cast<void *>(__msan_unpoison));  // 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
+			functions.try_emplace("msan_unpoison", reinterpret_cast<void *>(__msan_unpoison));
+#	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)));

src/Reactor/LLVMReactor.cpp

diff --git a/src/Reactor/LLVMReactor.cpp b/src/Reactor/LLVMReactor.cpp
index b9bb3d7..506e5dc 100644
--- a/src/Reactor/LLVMReactor.cpp
+++ b/src/Reactor/LLVMReactor.cpp
@@ -985,7 +985,7 @@
 			auto elTy = T(type);
 			ASSERT(V(ptr)->getType()->getContainedType(0) == elTy);
 
-			if(__has_feature(memory_sanitizer))
+			if(__has_feature(memory_sanitizer) && !REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION)
 			{
 				// Mark all memory writes as initialized by calling __msan_unpoison
 				// void __msan_unpoison(const volatile void *a, size_t size)
@@ -1091,7 +1091,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))
+	if(__has_feature(memory_sanitizer) && !REACTOR_ENABLE_MEMORY_SANITIZER_INSTRUMENTATION)
 	{
 		// Mark memory writes as initialized by calling __msan_unpoison
 		// void __msan_unpoison(const volatile void *a, size_t size)