Commit 87370c6b authored by Leszek Swirski's avatar Leszek Swirski Committed by V8 LUCI CQ

Revert "[flags] Add a sanity check for unchanged jitless flags"

This reverts commit 3a46c81c.

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 <jgruber@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> 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 <leszeks@chromium.org>
Owners-Override: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#77785}
parent 7482128a
......@@ -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<CheckedFlag>;
DEFINE_OPERATORS_FOR_FLAGS(CheckedFlags)
// The first isolate sets these, all following isolates read and verify against
// initial values.
std::atomic<uint32_t> 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<uint32_t>(current),
std::memory_order_relaxed, std::memory_order_relaxed);
if (is_initial_isolate) return; // Nothing to check.
if (static_cast<uint32_t>(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<i::IsolateAllocator> isolate_allocator,
bool is_shared)
: isolate_data_(this, isolate_allocator->GetPtrComprCageBase()),
......@@ -3092,8 +3042,6 @@ Isolate::Isolate(std::unique_ptr<i::IsolateAllocator> 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);
......
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