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

Revert "[wasm] Check correctness of thread-local write protection"

This reverts commit fee168ce.

Reason for revert: The DCHECK fails when freeing code
(https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/38292/overview),
which means that the current code is not correct. The added DCHECK
makes the bots red though, so the state before this CL was still
better.

Original change's description:
> [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: Jakob Kummerow <jkummerow@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#76134}

TBR=jkummerow@chromium.org

Change-Id: I5262b0e886f99a64452966345fc084a1ab750459
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3078360Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76146}
parent fe492561
......@@ -12,12 +12,6 @@ namespace internal {
namespace wasm {
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) && !V8_HAS_PTHREAD_JIT_WRITE_PROTECT
thread_local NativeModule* CodeSpaceWriteScope::current_native_module_ =
nullptr;
#endif
// TODO(jkummerow): Background threads could permanently stay in
// writable mode; only the main thread has to switch back and forth.
......@@ -26,16 +20,8 @@ CodeSpaceWriteScope::CodeSpaceWriteScope(NativeModule*) {
#else // !V8_HAS_PTHREAD_JIT_WRITE_PROTECT
CodeSpaceWriteScope::CodeSpaceWriteScope(NativeModule* native_module)
: native_module_(native_module) {
#ifdef DEBUG
if (code_space_write_nesting_level_ == 0) {
current_native_module_ = native_module;
}
DCHECK_EQ(native_module, current_native_module_);
#endif // DEBUG
#endif // !V8_HAS_PTHREAD_JIT_WRITE_PROTECT
if (code_space_write_nesting_level_ == 0) {
SetWritable();
}
if (code_space_write_nesting_level_ == 0) SetWritable();
code_space_write_nesting_level_++;
}
......
......@@ -55,9 +55,6 @@ class V8_NODISCARD CodeSpaceWriteScope final {
private:
static thread_local int code_space_write_nesting_level_;
#if defined(DEBUG) && !V8_HAS_PTHREAD_JIT_WRITE_PROTECT
static thread_local NativeModule* current_native_module_;
#endif
void SetWritable() 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