From 87370c6b58f48366b81f50495f6b8597e8d3c471 Mon Sep 17 00:00:00 2001 From: Leszek Swirski Date: Tue, 9 Nov 2021 08:34:18 +0000 Subject: [PATCH] Revert "[flags] Add a sanity check for unchanged jitless flags" This reverts commit 3a46c81c26ea52fc10d9114b3ee090bbe354db25. Reason for revert: Breaking roll (or rather, oh no, cast_shell is broken, need to fix that before relanding): https://ci.chromium.org/ui/p/chromium/builders/try/cast_shell_linux/1053410/overview Original change's description: > [flags] Add a sanity check for unchanged jitless flags > > V8 flags in general should not change in a process after the > first Isolate has been initialized. --jitless and related flags > especially sensitive to this, so we introduce a dedicated check > just for them. > > Bug: chromium:1262676, v8:9019, v8:12366 > Change-Id: I239726889d236a3785c1fdc076fa21d1b8983c92 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3260508 > Commit-Queue: Jakob Gruber > Reviewed-by: Michael Lippautz > Cr-Commit-Position: refs/heads/main@{#77759} Bug: chromium:1262676, v8:9019, v8:12366 Change-Id: Ie47d183bfd68633c3d30a13a038219051c38eba0 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3268734 Auto-Submit: Leszek Swirski Owners-Override: Leszek Swirski Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/heads/main@{#77785} --- src/execution/isolate.cc | 52 ---------------------------------------- 1 file changed, 52 deletions(-) diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc index cfe7293921..e081a34328 100644 --- a/src/execution/isolate.cc +++ b/src/execution/isolate.cc @@ -3017,56 +3017,6 @@ v8::PageAllocator* Isolate::page_allocator() const { return isolate_allocator_->page_allocator(); } -namespace { - -// These flags are all related to --jitless and *must not change within the -// same process*. See also: crbug.com/v8/9019. -enum class CheckedFlag { - kInitialized = 1 << 0, // Whether initial_checked_flag_values_ has been set. - kJitless = 1 << 1, - kRegexpInterpretAll = 1 << 2, - kExposeWasm = 1 << 3, - kOpt = 1 << 4, -}; -using CheckedFlags = base::Flags; -DEFINE_OPERATORS_FOR_FLAGS(CheckedFlags) - -// The first isolate sets these, all following isolates read and verify against -// initial values. -std::atomic initial_checked_flag_values_; - -void VerifyCheckedFlags() { - // TODO(all): While jitless flags are especially sensitive and cause - // hard-to-debug failures if misused, it may be good to check all V8 flags in - // a similar fashion - or at least have a systematic way of defining flags - // that must not change. For now, this ad-hoc check for jitless-related flags - // is intended to avoid trivial and accidental misuses. - CheckedFlags current = CheckedFlag::kInitialized; - if (FLAG_jitless) current |= CheckedFlag::kJitless; - if (FLAG_regexp_interpret_all) current |= CheckedFlag::kRegexpInterpretAll; -#if V8_ENABLE_WEBASSEMBLY - if (FLAG_expose_wasm) current |= CheckedFlag::kExposeWasm; -#endif - if (FLAG_opt) current |= CheckedFlag::kOpt; - - uint32_t initial_flags = 0; - const bool is_initial_isolate = - initial_checked_flag_values_.compare_exchange_strong( - initial_flags, static_cast(current), - std::memory_order_relaxed, std::memory_order_relaxed); - - if (is_initial_isolate) return; // Nothing to check. - - if (static_cast(current) != initial_flags) { - FATAL( - "An Isolate was spawned with different flag values than another " - "Isolate in the same process. Flags must remain unchanged within one " - "process."); - } -} - -} // namespace - Isolate::Isolate(std::unique_ptr isolate_allocator, bool is_shared) : isolate_data_(this, isolate_allocator->GetPtrComprCageBase()), @@ -3092,8 +3042,6 @@ Isolate::Isolate(std::unique_ptr isolate_allocator, TRACE_ISOLATE(constructor); CheckIsolateLayout(); - VerifyCheckedFlags(); - // ThreadManager is initialized early to support locking an isolate // before it is entered. thread_manager_ = new ThreadManager(this);