Commit 2ca2f370 authored by Andreas Haas's avatar Andreas Haas Committed by Commit Bot

[wasm][asan][windows] Reset thread-in-wasm flag in memory_fill_wrapper

ASAN on Windows uses exceptions to manage its shadow memory. However,
this behavior can conflict with WebAssembly trap handler, because
WebAssembly trap handler are executed before the ASAN exception handler.

For some WebAssembly instructions we do not generate assembly code but
call to C functions instead. Since these functions are very simple, we
do not want to reset the thread-in-wasm flag before calling them.
However, when these functions trigger the ASAN exceptions, the
thread-in-wasm flag gets out-of-sync. This happened for the
memory_fill_wrapper function. Originally we thought that it's sufficient
to just mark the function with DISABLE_ASAN. However, this is not enough
because clang compiles the function to use memset, and memset gets
replaced by ASAN with asan_memset.

Therefore I decided now that just for sanitizer builds on Windows, we
reset the thread-in-wasm flag in memory_fill_wrapper. This is not ideal
because it's test-specific code within production code. However, the
alternatives also don't sound convincing.

Alternatives would be:
* Resetting the thread-in-wasm flag whenever we call a c-function
  - This would be unnecessary performance overhead for production code
    just to make a test work.
* Configure ASAN to not change memset.
  - This would weaken ASAN also for other cases.
* Disable ASAN for trap handlers, or trap handlers in ASAN builds.
  - This would reduce test coverage.

R=binji@chromium.org

Bug: chromium:957405
Change-Id: Ibd13c6fe7b898238f636db576552e3e4b278c04a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1617671
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: 's avatarBen Smith <binji@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61658}
parent 437d710f
...@@ -12,6 +12,23 @@ ...@@ -12,6 +12,23 @@
#include "src/base/bits.h" #include "src/base/bits.h"
#include "src/base/ieee754.h" #include "src/base/ieee754.h"
#include "src/memcopy.h" #include "src/memcopy.h"
#if defined(ADDRESS_SANITIZER) || defined(MEMORY_SANITIZER) || \
defined(THREAD_SANITIZER) || defined(LEAK_SANITIZER) || \
defined(UNDEFINED_SANITIZER)
#define V8_WITH_SANITIZER
#endif
#if defined(V8_OS_WIN) && defined(V8_WITH_SANITIZER)
// With ASAN on Windows we have to reset the thread-in-wasm flag. Exceptions
// caused by ASAN let the thread-in-wasm flag get out of sync. Even marking
// functions with DISABLE_ASAN is not sufficient when the compiler produces
// calls to memset. Therefore we add test-specific code for ASAN on
// Windows.
#define RESET_THREAD_IN_WASM_FLAG_FOR_ASAN_ON_WINDOWS
#include "src/trap-handler/trap-handler.h"
#endif
#include "src/utils.h" #include "src/utils.h"
#include "src/v8memory.h" #include "src/v8memory.h"
#include "src/wasm/wasm-external-refs.h" #include "src/wasm/wasm-external-refs.h"
...@@ -281,8 +298,14 @@ DISABLE_ASAN void memory_copy_wrapper(Address dst, Address src, uint32_t size) { ...@@ -281,8 +298,14 @@ DISABLE_ASAN void memory_copy_wrapper(Address dst, Address src, uint32_t size) {
// Asan on Windows triggers exceptions in this function that confuse the // Asan on Windows triggers exceptions in this function that confuse the
// WebAssembly trap handler, so Asan is disabled. See the comment on // WebAssembly trap handler, so Asan is disabled. See the comment on
// memory_copy_wrapper above for more info. // memory_copy_wrapper above for more info.
DISABLE_ASAN void memory_fill_wrapper(Address dst, uint32_t value, void memory_fill_wrapper(Address dst, uint32_t value, uint32_t size) {
uint32_t size) { #if defined(RESET_THREAD_IN_WASM_FLAG_FOR_ASAN_ON_WINDOWS)
bool thread_was_in_wasm = trap_handler::IsThreadInWasm();
if (thread_was_in_wasm) {
trap_handler::ClearThreadInWasm();
}
#endif
// Use an explicit forward copy to match the required semantics for the // Use an explicit forward copy to match the required semantics for the
// memory.fill instruction. It is assumed that the caller of this function // memory.fill instruction. It is assumed that the caller of this function
// has already performed bounds checks, so {dst + size} should not overflow. // has already performed bounds checks, so {dst + size} should not overflow.
...@@ -292,6 +315,11 @@ DISABLE_ASAN void memory_fill_wrapper(Address dst, uint32_t value, ...@@ -292,6 +315,11 @@ DISABLE_ASAN void memory_fill_wrapper(Address dst, uint32_t value,
for (; size > 0; size--) { for (; size > 0; size--) {
*dst8++ = value8; *dst8++ = value8;
} }
#if defined(RESET_THREAD_IN_WASM_FLAG_FOR_ASAN_ON_WINDOWS)
if (thread_was_in_wasm) {
trap_handler::SetThreadInWasm();
}
#endif
} }
static WasmTrapCallbackForTesting wasm_trap_callback_for_testing = nullptr; static WasmTrapCallbackForTesting wasm_trap_callback_for_testing = nullptr;
...@@ -309,3 +337,6 @@ void call_trap_callback_for_testing() { ...@@ -309,3 +337,6 @@ void call_trap_callback_for_testing() {
} // namespace wasm } // namespace wasm
} // namespace internal } // namespace internal
} // namespace v8 } // namespace v8
#undef V8_WITH_SANITIZER
#undef RESET_THREAD_IN_WASM_FLAG_FOR_ASAN_ON_WINDOWS
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