From aea737150671630adf2e43770aee855b39b848b0 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 18 Sep 2023 14:36:36 +0200 Subject: [PATCH] deps: V8: backport de9a5de2274f MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: Ignore flags implied by --predictable during hash computation https://chromium-review.googlesource.com/c/v8/v8/+/4681766 added code to ignore --predictable during hash computation of flags in order to produce reproducible code cache. This turns out to be not enough since the flags implied by --predictable also need to be ignored for it to work. This patch makes sure that they are ignored. Change-Id: Ifa36641efe3ca105706fd293be46fc974055d2d4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4851287 Commit-Queue: Joyee Cheung Reviewed-by: Leszek Swirski Reviewed-by: Patrick Thier Cr-Commit-Position: refs/heads/main@{#90022} Refs: https://github.com/v8/v8/commit/de9a5de2274f483961a04a933d84abd81dee9c07 PR-URL: https://github.com/nodejs/node/pull/49703 Refs: https://github.com/v8/v8/commit/b33bf2dfd26131cd51640b618853f2dfcfe5f728 Reviewed-By: Jiawen Geng Reviewed-By: Michaël Zasso Reviewed-By: Richard Lau --- common.gypi | 2 +- deps/v8/src/flags/flags.cc | 79 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/common.gypi b/common.gypi index c6d968c5e7447d..d783c7f970237a 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.15', + 'v8_embedder_string': '-node.16', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/flags/flags.cc b/deps/v8/src/flags/flags.cc index f43715ac32eb0d..78d7b82354cb04 100644 --- a/deps/v8/src/flags/flags.cc +++ b/deps/v8/src/flags/flags.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include "src/base/functional.h" @@ -103,7 +104,12 @@ struct Flag { const char* cmt_; // A comment about the flags purpose. bool owns_ptr_; // Does the flag own its string value? SetBy set_by_ = SetBy::kDefault; + // Name of the flag implying this flag, if any. const char* implied_by_ = nullptr; +#ifdef DEBUG + // Pointer to the flag implying this flag, if any. + const Flag* implied_by_ptr_ = nullptr; +#endif FlagType type() const { return type_; } @@ -113,6 +119,17 @@ struct Flag { bool PointsTo(const void* ptr) const { return valptr_ == ptr; } +#ifdef DEBUG + bool ImpliedBy(const void* ptr) const { + const Flag* current = this->implied_by_ptr_; + while (current != nullptr) { + if (current->PointsTo(ptr)) return true; + current = current->implied_by_ptr_; + } + return false; + } +#endif + bool bool_variable() const { return GetValue(); } void set_bool_variable(bool value, SetBy set_by) { @@ -333,6 +350,15 @@ struct Flag { if (IsAnyImplication(new_set_by)) { DCHECK_NOT_NULL(implied_by); implied_by_ = implied_by; +#ifdef DEBUG + // This only works when implied_by is a flag_name or !flag_name, but it + // can also be a condition e.g. flag_name > 3. Since this is only used for + // checks in DEBUG mode, we will just ignore the more complex conditions + // for now - that will just lead to a nullptr which won't be followed. + implied_by_ptr_ = static_cast( + FindFlagByName(implied_by[0] == '!' ? implied_by + 1 : implied_by)); + DCHECK_NE(implied_by_ptr_, this); +#endif } return change_flag; } @@ -534,8 +560,21 @@ uint32_t ComputeFlagListHash() { std::ostringstream modified_args_as_string; if (COMPRESS_POINTERS_BOOL) modified_args_as_string << "ptr-compr"; if (DEBUG_BOOL) modified_args_as_string << "debug"; + +#ifdef DEBUG + // These two sets are used to check that we don't leave out any flags + // implied by --predictable in the list below. + std::set flags_implied_by_predictable; + std::set flags_ignored_because_of_predictable; +#endif + for (const Flag& flag : flags) { if (flag.IsDefault()) continue; +#ifdef DEBUG + if (flag.ImpliedBy(&v8_flags.predictable)) { + flags_implied_by_predictable.insert(flag.name()); + } +#endif // We want to be able to flip --profile-deserialization without // causing the code cache to get invalidated by this hash. if (flag.PointsTo(&v8_flags.profile_deserialization)) continue; @@ -543,8 +582,48 @@ uint32_t ComputeFlagListHash() { // code caching. if (flag.PointsTo(&v8_flags.random_seed)) continue; if (flag.PointsTo(&v8_flags.predictable)) continue; + + // The following flags are implied by --predictable (some negated). + if (flag.PointsTo(&v8_flags.concurrent_sparkplug) || + flag.PointsTo(&v8_flags.concurrent_recompilation) || +#ifdef V8_ENABLE_MAGLEV + flag.PointsTo(&v8_flags.maglev_deopt_data_on_background) || + flag.PointsTo(&v8_flags.maglev_build_code_on_background) || +#endif + flag.PointsTo(&v8_flags.parallel_scavenge) || + flag.PointsTo(&v8_flags.concurrent_marking) || + flag.PointsTo(&v8_flags.concurrent_array_buffer_sweeping) || + flag.PointsTo(&v8_flags.parallel_marking) || + flag.PointsTo(&v8_flags.concurrent_sweeping) || + flag.PointsTo(&v8_flags.parallel_compaction) || + flag.PointsTo(&v8_flags.parallel_pointer_update) || + flag.PointsTo(&v8_flags.memory_reducer) || + flag.PointsTo(&v8_flags.cppheap_concurrent_marking) || + flag.PointsTo(&v8_flags.cppheap_incremental_marking) || + flag.PointsTo(&v8_flags.single_threaded_gc)) { +#ifdef DEBUG + if (flag.ImpliedBy(&v8_flags.predictable)) { + flags_ignored_because_of_predictable.insert(flag.name()); + } +#endif + continue; + } modified_args_as_string << flag; } + +#ifdef DEBUG + for (const char* name : flags_implied_by_predictable) { + if (flags_ignored_because_of_predictable.find(name) == + flags_ignored_because_of_predictable.end()) { + PrintF( + "%s should be added to the list of " + "flags_ignored_because_of_predictable\n", + name); + UNREACHABLE(); + } + } +#endif + std::string args(modified_args_as_string.str()); // Generate a hash that is not 0. uint32_t hash = static_cast(base::hash_range(