Commit fee168ce authored by Clemens Backes's avatar Clemens Backes Committed by V8 LUCI CQ

[wasm] Check correctness of thread-local write protection

We make an undocumented assumption in {CodeSpaceWriteScope} that a
single thread will only work on one module at a time. If this is
violated, the thread-local {code_space_write_nesting_level_} would
prevent the second module from being switched to writable.

This CL adds a second thread local (in debug only) to check that if
there is already a {CodeSpaceWriteScope} open that it contains the same
{NativeModule} as any nested scope.

R=jkummerow@chromium.org

Change-Id: I43fa886d9d0fdf0e1846137dc411745fcca471fa
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3074477
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76134}
parent 4a7abdc3
...@@ -12,16 +12,30 @@ namespace internal { ...@@ -12,16 +12,30 @@ namespace internal {
namespace wasm { namespace wasm {
thread_local int CodeSpaceWriteScope::code_space_write_nesting_level_ = 0; thread_local int CodeSpaceWriteScope::code_space_write_nesting_level_ = 0;
// The thread-local counter (above) is only valid if a single thread only works
// on one module at a time. This second thread-local checks that.
#if defined(DEBUG) && (!defined(V8_OS_MACOSX) || !defined(V8_HOST_ARCH_ARM64))
thread_local NativeModule* CodeSpaceWriteScope::current_native_module_ =
nullptr;
#endif
// TODO(jkummerow): Background threads could permanently stay in // TODO(jkummerow): Background threads could permanently stay in
// writable mode; only the main thread has to switch back and forth. // writable mode; only the main thread has to switch back and forth.
#if defined(V8_OS_MACOSX) && defined(V8_HOST_ARCH_ARM64) #if defined(V8_OS_MACOSX) && defined(V8_HOST_ARCH_ARM64)
CodeSpaceWriteScope::CodeSpaceWriteScope(NativeModule*) { CodeSpaceWriteScope::CodeSpaceWriteScope(NativeModule*) {
#else #else // !defined(V8_OS_MACOSX) || !defined(V8_HOST_ARCH_ARM64)
CodeSpaceWriteScope::CodeSpaceWriteScope(NativeModule* native_module) CodeSpaceWriteScope::CodeSpaceWriteScope(NativeModule* native_module)
: native_module_(native_module) { : native_module_(native_module) {
#endif #ifdef DEBUG
if (code_space_write_nesting_level_ == 0) SetWritable(); if (code_space_write_nesting_level_ == 0) {
current_native_module_ = native_module;
}
DCHECK_EQ(native_module, current_native_module_);
#endif // DEBUG
#endif // !defined(V8_OS_MACOSX) || !defined(V8_HOST_ARCH_ARM64)
if (code_space_write_nesting_level_ == 0) {
SetWritable();
}
code_space_write_nesting_level_++; code_space_write_nesting_level_++;
} }
...@@ -45,7 +59,7 @@ void CodeSpaceWriteScope::SetExecutable() const { ...@@ -45,7 +59,7 @@ void CodeSpaceWriteScope::SetExecutable() const {
} }
#pragma clang diagnostic pop #pragma clang diagnostic pop
#else // Not Mac-on-arm64. #else // !defined(V8_OS_MACOSX) || !defined(V8_HOST_ARCH_ARM64)
void CodeSpaceWriteScope::SetWritable() const { void CodeSpaceWriteScope::SetWritable() const {
DCHECK_NOT_NULL(native_module_); DCHECK_NOT_NULL(native_module_);
...@@ -68,7 +82,7 @@ void CodeSpaceWriteScope::SetExecutable() const { ...@@ -68,7 +82,7 @@ void CodeSpaceWriteScope::SetExecutable() const {
} }
} }
#endif // defined(V8_OS_MACOSX) && defined(V8_HOST_ARCH_ARM64) #endif // !defined(V8_OS_MACOSX) || !defined(V8_HOST_ARCH_ARM64)
} // namespace wasm } // namespace wasm
} // namespace internal } // namespace internal
......
...@@ -55,6 +55,9 @@ class V8_NODISCARD CodeSpaceWriteScope final { ...@@ -55,6 +55,9 @@ class V8_NODISCARD CodeSpaceWriteScope final {
private: private:
static thread_local int code_space_write_nesting_level_; static thread_local int code_space_write_nesting_level_;
#if defined(DEBUG) && (!defined(V8_OS_MACOSX) || !defined(V8_HOST_ARCH_ARM64))
static thread_local NativeModule* current_native_module_;
#endif
void SetWritable() const; void SetWritable() const;
void SetExecutable() const; void SetExecutable() const;
......
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