[flags] Remove method to reset flags to default

After flags are frozen, this will not work any more. It's also not
required, as flags cannot be accessed after teardown anyway.
This CL changes that to only release the memory of dynamically allocated
string flags, which is something we still need to do after
write-protecting the flags anyway.

R=tebbi@chromium.org

Bug: v8:12887
Change-Id: Iff0e3845cbd91fb59878b2ed36a44d6df00572f4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3695379Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81030}

src/flags/flags.cc

@@ -327,6 +327,11 @@ struct Flag {
     UNREACHABLE();
   }
 
+  void ReleaseDynamicAllocations() {
+    if (type_ != TYPE_STRING) return;
+    if (owns_ptr_) DeleteArray(string_value());
+  }
+
   // Set a flag back to it's default value.
   void Reset() {
     switch (type_) {
@@ -775,12 +780,10 @@ bool FlagList::IsFrozen() {
 }
 
 // static
-void FlagList::ResetAllFlags() {
-  // Reset is allowed even if flags are frozen. They stay frozen though, because
-  // they are not expected to ever be used again.
+void FlagList::ReleaseDynamicAllocations() {
   flag_hash = 0;
   for (size_t i = 0; i < num_flags; ++i) {
-    flags[i].Reset();
+    flags[i].ReleaseDynamicAllocations();
   }
 }
 

src/flags/flags.h

@@ -89,8 +89,9 @@ class V8_EXPORT_PRIVATE FlagList {
   // Returns true if the flags are currently frozen.
   static bool IsFrozen();
 
-  // Reset all flags to their default value.
-  static void ResetAllFlags();
+  // Free dynamically allocated memory of strings. This is called during
+  // teardown; flag values cannot be used afterwards any more.
+  static void ReleaseDynamicAllocations();
 
   // Print help to stdout with flags, types, and default values.
   static void PrintHelp();

src/init/v8.cc

@@ -279,7 +279,7 @@ void V8::Dispose() {
   ElementsAccessor::TearDown();
   RegisteredExtension::UnregisterAll();
   Isolate::DisposeOncePerProcess();
-  FlagList::ResetAllFlags();  // Frees memory held by string arguments.
+  FlagList::ReleaseDynamicAllocations();
   AdvanceStartupState(V8StartupState::kV8Disposed);
 }
 

test/unittests/flags/flag-definitions-unittest.cc

@@ -47,15 +47,9 @@ void TestDefault() {
 // This test must be executed first!
 TEST_F(FlagsTest, Default) { TestDefault(); }
 
-static void SetFlagsToDefault() {
-  FlagList::ResetAllFlags();
-  TestDefault();
-}
-
 TEST_F(FlagsTest, Flags1) { FlagList::PrintHelp(); }
 
 TEST_F(FlagsTest, Flags2) {
-  SetFlagsToDefault();
   int argc = 8;
   const char* argv[] = {"Test2",
                         "-notesting-bool-flag",
@@ -77,7 +71,6 @@ TEST_F(FlagsTest, Flags2) {
 }
 
 TEST_F(FlagsTest, Flags2b) {
-  SetFlagsToDefault();
   const char* str =
       " -notesting-bool-flag notaflag   --testing_int_flag=77 "
       "-notesting-maybe-bool-flag   "
@@ -93,7 +86,6 @@ TEST_F(FlagsTest, Flags2b) {
 }
 
 TEST_F(FlagsTest, Flags3) {
-  SetFlagsToDefault();
   int argc = 9;
   const char* argv[] = {"Test3",
                         "--testing_bool_flag",
@@ -116,7 +108,6 @@ TEST_F(FlagsTest, Flags3) {
 }
 
 TEST_F(FlagsTest, Flags3b) {
-  SetFlagsToDefault();
   const char* str =
       "--testing_bool_flag --testing-maybe-bool-flag notaflag "
       "--testing_int_flag -666 "
@@ -132,7 +123,6 @@ TEST_F(FlagsTest, Flags3b) {
 }
 
 TEST_F(FlagsTest, Flags4) {
-  SetFlagsToDefault();
   int argc = 3;
   const char* argv[] = {"Test4", "--testing_bool_flag", "--foo"};
   CHECK_EQ(0, FlagList::SetFlagsFromCommandLine(&argc, const_cast<char**>(argv),
@@ -142,14 +132,12 @@ TEST_F(FlagsTest, Flags4) {
 }
 
 TEST_F(FlagsTest, Flags4b) {
-  SetFlagsToDefault();
   const char* str = "--testing_bool_flag --foo";
   CHECK_EQ(2, FlagList::SetFlagsFromString(str, strlen(str)));
   CHECK(!FLAG_testing_maybe_bool_flag.value().has_value());
 }
 
 TEST_F(FlagsTest, Flags5) {
-  SetFlagsToDefault();
   int argc = 2;
   const char* argv[] = {"Test5", "--testing_int_flag=\"foobar\""};
   CHECK_EQ(1, FlagList::SetFlagsFromCommandLine(&argc, const_cast<char**>(argv),
@@ -158,13 +146,11 @@ TEST_F(FlagsTest, Flags5) {
 }
 
 TEST_F(FlagsTest, Flags5b) {
-  SetFlagsToDefault();
   const char* str = "                     --testing_int_flag=\"foobar\"";
   CHECK_EQ(1, FlagList::SetFlagsFromString(str, strlen(str)));
 }
 
 TEST_F(FlagsTest, Flags6) {
-  SetFlagsToDefault();
   int argc = 4;
   const char* argv[] = {"Test5", "--testing-int-flag", "0",
                         "--testing_float_flag"};
@@ -174,7 +160,6 @@ TEST_F(FlagsTest, Flags6) {
 }
 
 TEST_F(FlagsTest, Flags6b) {
-  SetFlagsToDefault();
   const char* str = "       --testing-int-flag 0      --testing_float_flag    ";
   CHECK_EQ(3, FlagList::SetFlagsFromString(str, strlen(str)));
 }
@@ -182,7 +167,6 @@ TEST_F(FlagsTest, Flags6b) {
 TEST_F(FlagsTest, FlagsRemoveIncomplete) {
   // Test that processed command line arguments are removed, even
   // if the list of arguments ends unexpectedly.
-  SetFlagsToDefault();
   int argc = 3;
   const char* argv[] = {"", "--testing-bool-flag", "--expose-gc-as"};
   CHECK_EQ(2, FlagList::SetFlagsFromCommandLine(&argc, const_cast<char**>(argv),