[flags] Store all flag values in a single struct

Instead of defining one global (FLAG_foo) per flag, define all flag
values as fields in a global {v8_flags} struct. This guarantees that the
memory is contiguous, and together with proper alignment allows us to
later memory-protect that memory space.

In order to avoid rewriting all existing code that uses the {FLAG_foo}
syntax, we define global aliases: {FLAG_foo} is a reference to
{v8_flags.foo}.

After the next branch cut (v10.6), follow-up CLs will rewrite all
existing code to use the {v8_flags.foo} syntax, and after another branch
cut (v10.7) the aliases will be removed.
This should allow us to merge back most fixes to the previous branch
(N-1). Merges to stable (N-2) might still require resolving merge
conflicts manually, if they modify code that reads flags.

R=cbruni@chromium.org
CC=​sroettger@chromium.org

Bug: v8:12887
Change-Id: I8bc44429767f611484fe345d7268af1d55c98124
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3810187
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82423}

src/base/build_config.h

@@ -52,19 +52,33 @@ constexpr int kReturnAddressStackSlotCount =
 #if (defined(V8_HOST_ARCH_PPC) || defined(V8_HOST_ARCH_PPC64)) && !defined(_AIX)
 // Native PPC linux has large (64KB) physical pages.
 // Simulator (and Aix) need to use the same value as x64.
-const int kPageSizeBits = 19;
+constexpr int kPageSizeBits = 19;
 #elif defined(ENABLE_HUGEPAGE)
 // When enabling huge pages, adjust V8 page size to take up exactly one huge
 // page. This avoids huge-page-internal fragmentation for unused address ranges.
-const int kHugePageBits = 21;
-const int kHugePageSize = (1U) << kHugePageBits;
-const int kPageSizeBits = kHugePageBits;
+constexpr int kHugePageBits = 21;
+constexpr int kHugePageSize = 1 << kHugePageBits;
+constexpr int kPageSizeBits = kHugePageBits;
 #else
 // Arm64 supports up to 64k OS pages on Linux, however 4k pages are more common
 // so we keep the V8 page size at 256k. Nonetheless, we need to make sure we
 // don't decrease it further in the future due to reserving 3 OS pages for every
 // executable V8 page.
-const int kPageSizeBits = 18;
+constexpr int kPageSizeBits = 18;
+#endif
+
+// The minimal supported page size by the operation system. Any region aligned
+// to that size needs to be individually protectable via
+// {base::OS::SetPermission} and friends.
+#if defined(V8_OS_MACOS) && defined(V8_HOST_ARCH_ARM64)
+// MacOS on arm64 uses 16kB pages.
+constexpr int kMinimumOSPageSize = 16 * 1024;
+#elif defined(V8_OS_LINUX) && defined(V8_HOST_ARCH_ARM64)
+// Linux on arm64 can be configured for up to 64kB pages.
+constexpr int kMinimumOSPageSize = 64 * 1024;
+#else
+// Everything else uses 4kB pages.
+constexpr int kMinimumOSPageSize = 4 * 1024;
 #endif
 
 #endif  // V8_BASE_BUILD_CONFIG_H_

src/flags/flag-definitions.h

@@ -29,25 +29,22 @@
 #define DEFINE_NEG_NEG_IMPLICATION(whenflag, thenflag) \
   DEFINE_NEG_VALUE_IMPLICATION(whenflag, thenflag, false)
 
-// We want to declare the names of the variables for the header file.  Normally
-// this will just be an extern declaration, but for a readonly flag we let the
-// compiler make better optimizations by giving it the value.
+// With FLAG_MODE_DECLARE we declare the fields in the {FlagValues} struct.
+// Read-only flags are static constants instead of fields.
 #if defined(FLAG_MODE_DECLARE)
-#define FLAG_FULL(ftype, ctype, nam, def, cmt) \
-  V8_EXPORT_PRIVATE extern FlagValue<ctype> FLAG_##nam;
+#define FLAG_FULL(ftype, ctype, nam, def, cmt) FlagValue<ctype> nam{def};
 #define FLAG_READONLY(ftype, ctype, nam, def, cmt) \
-  static constexpr FlagValue<ctype> FLAG_##nam{def};
+  static constexpr FlagValue<ctype> nam{def};
 
-// We want to supply the actual storage and value for the flag variable in the
-// .cc file.  We only do this for writable flags.
-#elif defined(FLAG_MODE_DEFINE)
-#ifdef USING_V8_SHARED
-#define FLAG_FULL(ftype, ctype, nam, def, cmt) \
-  V8_EXPORT_PRIVATE extern FlagValue<ctype> FLAG_##nam;
-#else
+// Define a {FLAG_foo} alias per flag, pointing to {v8_flags.foo}.
+// This allows to still use the old and deprecated syntax for accessing flag
+// values. This will be removed after v10.7.
+// TODO(clemensb): Remove this after v10.7.
+#elif defined(FLAG_MODE_DEFINE_GLOBAL_ALIASES)
 #define FLAG_FULL(ftype, ctype, nam, def, cmt) \
-  V8_EXPORT_PRIVATE FlagValue<ctype> FLAG_##nam{def};
-#endif
+  inline auto& FLAG_##nam = v8_flags.nam;
+#define FLAG_READONLY(ftype, ctype, nam, def, cmt) \
+  inline auto constexpr& FLAG_##nam = v8_flags.nam;
 
 // We need to define all of our default values so that the Flag structure can
 // access them by pointer.  These are just used internally inside of one .cc,
@@ -60,29 +57,29 @@
 // printing / etc in the flag parser code.  We only do this for writable flags.
 #elif defined(FLAG_MODE_META)
 #define FLAG_FULL(ftype, ctype, nam, def, cmt) \
-  {Flag::TYPE_##ftype, #nam, &FLAG_##nam, &FLAGDEFAULT_##nam, cmt, false},
+  {Flag::TYPE_##ftype, #nam, &v8_flags.nam, &FLAGDEFAULT_##nam, cmt, false},
 #define FLAG_ALIAS(ftype, ctype, alias, nam)                       \
-  {Flag::TYPE_##ftype,  #alias, &FLAG_##nam, &FLAGDEFAULT_##nam, \
-    "alias for --" #nam, false},
+  {Flag::TYPE_##ftype,  #alias, &v8_flags.nam, &FLAGDEFAULT_##nam, \
+   "alias for --" #nam, false},  // NOLINT(whitespace/indent)
 
 // We produce the code to set flags when it is implied by another flag.
 #elif defined(FLAG_MODE_DEFINE_IMPLICATIONS)
 #define DEFINE_VALUE_IMPLICATION(whenflag, thenflag, value)   \
-  changed |= TriggerImplication(FLAG_##whenflag, #whenflag, &FLAG_##thenflag, \
-                                value, false);
+  changed |= TriggerImplication(v8_flags.whenflag, #whenflag, \
+                                &v8_flags.thenflag, value, false);
 
 // A weak implication will be overwritten by a normal implication or by an
 // explicit flag.
 #define DEFINE_WEAK_VALUE_IMPLICATION(whenflag, thenflag, value) \
-  changed |= TriggerImplication(FLAG_##whenflag, #whenflag, &FLAG_##thenflag, \
-                                value, true);
+  changed |= TriggerImplication(v8_flags.whenflag, #whenflag,    \
+                                &v8_flags.thenflag, value, true);
 
 #define DEFINE_GENERIC_IMPLICATION(whenflag, statement) \
-  if (FLAG_##whenflag) statement;
+  if (v8_flags.whenflag) statement;
 
 #define DEFINE_NEG_VALUE_IMPLICATION(whenflag, thenflag, value)    \
-  changed |= TriggerImplication(!FLAG_##whenflag, "!" #whenflag, \
-                                &FLAG_##thenflag, value, false);
+  changed |= TriggerImplication(!v8_flags.whenflag, "!" #whenflag, \
+                                &v8_flags.thenflag, value, false);
 
 // We apply a generic macro to the flags.
 #elif defined(FLAG_MODE_APPLY)
@@ -2349,7 +2346,7 @@ DEFINE_BOOL(enable_embedded_constant_pool, V8_EMBEDDED_CONSTANT_POOL,
 #undef DEFINE_ALIAS_FLOAT
 
 #undef FLAG_MODE_DECLARE
-#undef FLAG_MODE_DEFINE
+#undef FLAG_MODE_DEFINE_GLOBAL_ALIASES
 #undef FLAG_MODE_DEFINE_DEFAULTS
 #undef FLAG_MODE_META
 #undef FLAG_MODE_DEFINE_IMPLICATIONS

src/flags/flags.cc

@@ -28,12 +28,18 @@
 #include "src/wasm/wasm-limits.h"
 #endif  // V8_ENABLE_WEBASSEMBLY
 
-namespace v8 {
-namespace internal {
+namespace v8::internal {
 
-// Define all of our flags.
-#define FLAG_MODE_DEFINE
-#include "src/flags/flag-definitions.h"  // NOLINT(build/include)
+// Define {v8_flags}, declared in flags.h.
+FlagValues v8_flags;
+
+// {v8_flags} needs to be aligned to a memory page, and the size needs to be a
+// multiple of a page size. This is required for memory-protection of the memory
+// holding the {v8_flags} struct.
+// Both is guaranteed by the {alignas(kMinimumOSPageSize)} annotation on
+// {FlagValues}.
+static_assert(alignof(FlagValues) == kMinimumOSPageSize);
+static_assert(sizeof(FlagValues) % kMinimumOSPageSize == 0);
 
 // Define all of our flags default values.
 #define FLAG_MODE_DEFINE_DEFAULTS
@@ -199,14 +205,14 @@ struct Flag {
   }
 
   static bool ShouldCheckFlagContradictions() {
-    if (FLAG_allow_overwriting_for_next_flag) {
+    if (v8_flags.allow_overwriting_for_next_flag) {
       // Setting the flag manually to false before calling Reset() avoids this
       // becoming re-entrant.
-      FLAG_allow_overwriting_for_next_flag = false;
-      FindFlagByPointer(&FLAG_allow_overwriting_for_next_flag)->Reset();
+      v8_flags.allow_overwriting_for_next_flag = false;
+      FindFlagByPointer(&v8_flags.allow_overwriting_for_next_flag)->Reset();
       return false;
     }
-    return FLAG_abort_on_contradictory_flags && !FLAG_fuzzing;
+    return v8_flags.abort_on_contradictory_flags && !v8_flags.fuzzing;
   }
 
   // {change_flag} indicates if we're going to change the flag value.
@@ -261,7 +267,7 @@ struct Flag {
         case SetBy::kCommandLine:
           if (new_set_by == SetBy::kImplication && check_command_line_flags) {
             // Exit instead of abort for certain testing situations.
-            if (FLAG_exit_on_contradictory_flags) base::OS::ExitProcess(0);
+            if (v8_flags.exit_on_contradictory_flags) base::OS::ExitProcess(0);
             if (is_bool_flag) {
               FatalError{} << "Flag " << FlagName{name()}
                            << ": value implied by " << FlagName{implied_by}
@@ -274,7 +280,7 @@ struct Flag {
           } else if (new_set_by == SetBy::kCommandLine &&
                      check_command_line_flags) {
             // Exit instead of abort for certain testing situations.
-            if (FLAG_exit_on_contradictory_flags) base::OS::ExitProcess(0);
+            if (v8_flags.exit_on_contradictory_flags) base::OS::ExitProcess(0);
             if (is_bool_flag) {
               FatalError{} << "Command-line provided flag " << FlagName{name()}
                            << " specified as both true and false";
@@ -487,9 +493,9 @@ uint32_t ComputeFlagListHash() {
     if (flag.IsDefault()) continue;
     // We want to be able to flip --profile-deserialization without
     // causing the code cache to get invalidated by this hash.
-    if (flag.PointsTo(&FLAG_profile_deserialization)) continue;
-    // Skip FLAG_random_seed to allow predictable code caching.
-    if (flag.PointsTo(&FLAG_random_seed)) continue;
+    if (flag.PointsTo(&v8_flags.profile_deserialization)) continue;
+    // Skip v8_flags.random_seed to allow predictable code caching.
+    if (flag.PointsTo(&v8_flags.random_seed)) continue;
     modified_args_as_string << flag;
   }
   std::string args(modified_args_as_string.str());
@@ -688,7 +694,7 @@ int FlagList::SetFlagsFromCommandLine(int* argc, char** argv, bool remove_flags,
     }
   }
 
-  if (FLAG_help) {
+  if (v8_flags.help) {
     if (help_options.HasUsage()) {
       PrintF(stdout, "%s", help_options.usage());
     }
@@ -911,9 +917,4 @@ void FlagList::ResetFlagHash() {
   flag_hash = 0;
 }
 
-#undef FLAG_MODE_DEFINE
-#undef FLAG_MODE_DEFINE_DEFAULTS
-#undef FLAG_MODE_META
-
-}  // namespace internal
-}  // namespace v8
+}  // namespace v8::internal

src/flags/flags.h

@@ -8,16 +8,24 @@
 #include "src/base/optional.h"
 #include "src/common/globals.h"
 
+#if V8_ENABLE_WEBASSEMBLY
+// Include the wasm-limits.h header for some default values of Wasm flags.
+// This can be reverted once we can use designated initializations (C++20) for
+// {v8_flags} (defined in flags.cc) instead of specifying the default values in
+// the header and using the default constructor.
+#include "src/wasm/wasm-limits.h"
+#endif
+
 namespace v8::internal {
 
-// The value of a single flag (this is the type of all FLAG_* globals).
+// The value of a single flag (this is the type of all v8_flags.* fields).
 template <typename T>
 class FlagValue {
  public:
-  constexpr FlagValue(T value) : value_(value) {}
+  explicit constexpr FlagValue(T value) : value_(value) {}
 
-  // Implicitly convert to a {T}. Not marked {constexpr} so we do not compiler
-  // warnings about dead code (when checking readonly flags).
+  // Implicitly convert to a {T}. Not marked {constexpr} so we do not get
+  // compiler warnings about dead code (when checking readonly flags).
   operator T() const { return value_; }
 
   // Explicitly convert to a {T} via {value()}. This is {constexpr} so we can
@@ -31,9 +39,24 @@ class FlagValue {
   T value_;
 };
 
-// Declare all of our flags.
+// Declare a struct to hold all of our flags.
+struct alignas(kMinimumOSPageSize) FlagValues {
+  FlagValues() = default;
+  // No copying, moving, or assigning. This is a singleton struct.
+  FlagValues(const FlagValues&) = delete;
+  FlagValues(FlagValues&&) = delete;
+  FlagValues& operator=(const FlagValues&) = delete;
+  FlagValues& operator=(FlagValues&&) = delete;
+
 #define FLAG_MODE_DECLARE
-#include "src/flags/flag-definitions.h"
+#include "src/flags/flag-definitions.h"  // NOLINT(build/include)
+};
+
+V8_EXPORT_PRIVATE extern FlagValues v8_flags;
+
+// TODO(clemensb): Remove this after v10.7.
+#define FLAG_MODE_DEFINE_GLOBAL_ALIASES
+#include "src/flags/flag-definitions.h"  // NOLINT(build/include)
 
 // The global list of all flags.
 class V8_EXPORT_PRIVATE FlagList {