[d8] only add arguments object if arguments are provided

This is to fix test262 tests which expect that there is no arguments
object.

Bug: v8:7186
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: I56205c29609666dc832297e4d36a4d487eae36cc
Reviewed-on: https://chromium-review.googlesource.com/c/1291469Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Mathias Bynens <mathias@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56890}

src/d8.cc

@@ -1940,23 +1940,22 @@ Local<Context> Shell::CreateEvaluationContext(Isolate* isolate) {
   InitializeModuleEmbedderData(context);
   Context::Scope scope(context);
 
-  i::Factory* factory = reinterpret_cast<i::Isolate*>(isolate)->factory();
-  i::JSArguments js_args = i::FLAG_js_arguments;
-  i::Handle<i::FixedArray> arguments_array =
-      factory->NewFixedArray(js_args.argc);
-  for (int j = 0; j < js_args.argc; j++) {
-    i::Handle<i::String> arg =
-        factory->NewStringFromUtf8(i::CStrVector(js_args[j])).ToHandleChecked();
-    arguments_array->set(j, *arg);
-  }
-  i::Handle<i::JSArray> arguments_jsarray =
-      factory->NewJSArrayWithElements(arguments_array);
-  context->Global()
-      ->Set(context,
-            String::NewFromUtf8(isolate, "arguments", NewStringType::kNormal)
-                .ToLocalChecked(),
-            Utils::ToLocal(arguments_jsarray))
-      .FromJust();
+  const std::vector<const char*>& args = options.arguments;
+  int size = static_cast<int>(args.size());
+  if (size > 0) {
+    Local<Array> array = Array::New(isolate, size);
+    for (int i = 0; i < size; i++) {
+      Local<String> arg =
+          v8::String::NewFromUtf8(isolate, args[i], v8::NewStringType::kNormal)
+              .ToLocalChecked();
+      Local<Number> index = v8::Number::New(isolate, i);
+      array->Set(context, index, arg).FromJust();
+    }
+    Local<String> name =
+        String::NewFromUtf8(isolate, "arguments", NewStringType::kInternalized)
+            .ToLocalChecked();
+    context->Global()->Set(context, name, array).FromJust();
+  }
   return handle_scope.Escape(context);
 }
 
@@ -2760,6 +2759,14 @@ void SetFlagsFromString(const char* flags) {
 bool Shell::SetOptions(int argc, char* argv[]) {
   bool logfile_per_isolate = false;
   for (int i = 0; i < argc; i++) {
+    if (strcmp(argv[i], "--") == 0) {
+      argv[i] = nullptr;
+      for (int j = i + 1; j < argc; j++) {
+        options.arguments.push_back(argv[j]);
+        argv[j] = nullptr;
+      }
+      break;
+    }
     if (strcmp(argv[i], "--stress-opt") == 0) {
       options.stress_opt = true;
       argv[i] = nullptr;

src/d8.h

@@ -386,6 +386,7 @@ class ShellOptions {
   bool enable_os_system = false;
   bool quiet_load = false;
   int thread_pool_size = 0;
+  std::vector<const char*> arguments;
 };
 
 class Shell : public i::AllStatic {

src/flag-definitions.h

@@ -97,19 +97,6 @@
 #define COMMA ,
 
 #ifdef FLAG_MODE_DECLARE
-// Structure used to hold a collection of arguments to the JavaScript code.
-struct JSArguments {
- public:
-  inline const char*& operator[](int idx) const { return argv[idx]; }
-  static JSArguments Create(int argc, const char** argv) {
-    JSArguments args;
-    args.argc = argc;
-    args.argv = argv;
-    return args;
-  }
-  int argc;
-  const char** argv;
-};
 
 struct MaybeBoolFlag {
   static MaybeBoolFlag Create(bool has_value, bool value) {
@@ -166,16 +153,12 @@ struct MaybeBoolFlag {
 #define DEFINE_FLOAT(nam, def, cmt) FLAG(FLOAT, double, nam, def, cmt)
 #define DEFINE_SIZE_T(nam, def, cmt) FLAG(SIZE_T, size_t, nam, def, cmt)
 #define DEFINE_STRING(nam, def, cmt) FLAG(STRING, const char*, nam, def, cmt)
-#define DEFINE_ARGS(nam, cmt) \
-  FLAG(ARGS, JSArguments, nam, {0 COMMA nullptr}, cmt)
-
 #define DEFINE_ALIAS_BOOL(alias, nam) FLAG_ALIAS(BOOL, bool, alias, nam)
 #define DEFINE_ALIAS_INT(alias, nam) FLAG_ALIAS(INT, int, alias, nam)
 #define DEFINE_ALIAS_FLOAT(alias, nam) FLAG_ALIAS(FLOAT, double, alias, nam)
 #define DEFINE_ALIAS_SIZE_T(alias, nam) FLAG_ALIAS(SIZE_T, size_t, alias, nam)
 #define DEFINE_ALIAS_STRING(alias, nam) \
   FLAG_ALIAS(STRING, const char*, alias, nam)
-#define DEFINE_ALIAS_ARGS(alias, nam) FLAG_ALIAS(ARGS, JSArguments, alias, nam)
 
 #ifdef DEBUG
 #define DEFINE_DEBUG_BOOL DEFINE_BOOL
@@ -1137,8 +1120,6 @@ DEFINE_BOOL(dump_counters_nvp, false,
 DEFINE_BOOL(use_external_strings, false, "Use external strings for source code")
 
 DEFINE_STRING(map_counters, "", "Map counters to a file")
-DEFINE_ARGS(js_arguments,
-            "Pass all remaining arguments to the script. Alias for \"--\".")
 DEFINE_BOOL(mock_arraybuffer_allocator, false,
             "Use a mock ArrayBuffer allocator for testing.")
 DEFINE_SIZE_T(mock_arraybuffer_allocator_limit, 0,
@@ -1483,7 +1464,6 @@ DEFINE_BOOL(lite_mode, V8_LITE_BOOL,
 #undef DEFINE_INT
 #undef DEFINE_STRING
 #undef DEFINE_FLOAT
-#undef DEFINE_ARGS
 #undef DEFINE_IMPLICATION
 #undef DEFINE_NEG_IMPLICATION
 #undef DEFINE_NEG_VALUE_IMPLICATION
@@ -1492,7 +1472,6 @@ DEFINE_BOOL(lite_mode, V8_LITE_BOOL,
 #undef DEFINE_ALIAS_INT
 #undef DEFINE_ALIAS_STRING
 #undef DEFINE_ALIAS_FLOAT
-#undef DEFINE_ALIAS_ARGS
 
 #undef FLAG_MODE_DECLARE
 #undef FLAG_MODE_DEFINE

src/flags.cc

@@ -43,7 +43,6 @@ struct Flag {
     TYPE_FLOAT,
     TYPE_SIZE_T,
     TYPE_STRING,
-    TYPE_ARGS
   };
 
   FlagType type_;           // What type of flag, bool, int, or string.
@@ -107,11 +106,6 @@ struct Flag {
     owns_ptr_ = owns_ptr;
   }
 
-  JSArguments* args_variable() const {
-    DCHECK(type_ == TYPE_ARGS);
-    return reinterpret_cast<JSArguments*>(valptr_);
-  }
-
   bool bool_default() const {
     DCHECK(type_ == TYPE_BOOL);
     return *reinterpret_cast<const bool*>(defptr_);
@@ -147,11 +141,6 @@ struct Flag {
     return *reinterpret_cast<const char* const *>(defptr_);
   }
 
-  JSArguments args_default() const {
-    DCHECK(type_ == TYPE_ARGS);
-    return *reinterpret_cast<const JSArguments*>(defptr_);
-  }
-
   // Compare this flag's current value against the default.
   bool IsDefault() const {
     switch (type_) {
@@ -176,8 +165,6 @@ struct Flag {
         if (str1 == nullptr) return str2 == nullptr;
         return strcmp(str1, str2) == 0;
       }
-      case TYPE_ARGS:
-        return args_variable()->argc == 0;
     }
     UNREACHABLE();
   }
@@ -209,9 +196,6 @@ struct Flag {
       case TYPE_STRING:
         set_string_value(string_default(), false);
         break;
-      case TYPE_ARGS:
-        *args_variable() = args_default();
-        break;
     }
   }
 };
@@ -239,7 +223,6 @@ static const char* Type2String(Flag::FlagType type) {
     case Flag::TYPE_SIZE_T:
       return "size_t";
     case Flag::TYPE_STRING: return "string";
-    case Flag::TYPE_ARGS: return "arguments";
   }
   UNREACHABLE();
 }
@@ -275,16 +258,6 @@ std::ostream& operator<<(std::ostream& os, const Flag& flag) {  // NOLINT
       os << (str ? str : "nullptr");
       break;
     }
-    case Flag::TYPE_ARGS: {
-      JSArguments args = *flag.args_variable();
-      if (args.argc > 0) {
-        os << args[0];
-        for (int i = 1; i < args.argc; i++) {
-          os << args[i];
-        }
-      }
-      break;
-    }
   }
   return os;
 }
@@ -293,15 +266,9 @@ std::ostream& operator<<(std::ostream& os, const Flag& flag) {  // NOLINT
 // static
 std::vector<const char*>* FlagList::argv() {
   std::vector<const char*>* args = new std::vector<const char*>(8);
-  Flag* args_flag = nullptr;
   for (size_t i = 0; i < num_flags; ++i) {
     Flag* f = &flags[i];
     if (!f->IsDefault()) {
-      if (f->type() == Flag::TYPE_ARGS) {
-        DCHECK_NULL(args_flag);
-        args_flag = f;  // Must be last in arguments.
-        continue;
-      }
       {
         bool disabled = f->type() == Flag::TYPE_BOOL && !*f->bool_variable();
         std::ostringstream os;
@@ -315,15 +282,6 @@ std::vector<const char*>* FlagList::argv() {
       }
     }
   }
-  if (args_flag != nullptr) {
-    std::ostringstream os;
-    os << "--" << args_flag->name();
-    args->push_back(StrDup(os.str().c_str()));
-    JSArguments jsargs = *args_flag->args_variable();
-    for (int j = 0; j < jsargs.argc; j++) {
-      args->push_back(StrDup(jsargs[j]));
-    }
-  }
   return args;
 }
 
@@ -454,8 +412,7 @@ int FlagList::SetFlagsFromCommandLine(int* argc,
 
       // if we still need a flag value, use the next argument if available
       if (flag->type() != Flag::TYPE_BOOL &&
-          flag->type() != Flag::TYPE_MAYBE_BOOL &&
-          flag->type() != Flag::TYPE_ARGS && value == nullptr) {
+          flag->type() != Flag::TYPE_MAYBE_BOOL && value == nullptr) {
         if (i < *argc) {
           value = argv[i++];
         }
@@ -503,20 +460,6 @@ int FlagList::SetFlagsFromCommandLine(int* argc,
         case Flag::TYPE_STRING:
           flag->set_string_value(value ? StrDup(value) : nullptr, true);
           break;
-        case Flag::TYPE_ARGS: {
-          int start_pos = (value == nullptr) ? i : i - 1;
-          int js_argc = *argc - start_pos;
-          const char** js_argv = NewArray<const char*>(js_argc);
-          if (value != nullptr) {
-            js_argv[0] = StrDup(value);
-          }
-          for (int k = i; k < *argc; k++) {
-            js_argv[k - start_pos] = StrDup(argv[k]);
-          }
-          *flag->args_variable() = JSArguments::Create(js_argc, js_argv);
-          i = *argc;  // Consume all arguments
-          break;
-        }
       }
 
       // handle errors

test/cctest/test-flags.cc

@@ -183,70 +183,6 @@ TEST(Flags6b) {
   CHECK_EQ(3, FlagList::SetFlagsFromString(str, StrLength(str)));
 }
 
-
-TEST(FlagsJSArguments1) {
-  SetFlagsToDefault();
-  int argc = 6;
-  const char* argv[] = {"TestJSArgs1",
-                        "--testing-int-flag", "42",
-                        "--", "testing-float-flag", "7"};
-  CHECK_EQ(0, FlagList::SetFlagsFromCommandLine(&argc,
-                                                const_cast<char **>(argv),
-                                                true));
-  CHECK_EQ(42, FLAG_testing_int_flag);
-  CHECK_EQ(2.5, FLAG_testing_float_flag);
-  CHECK_EQ(2, FLAG_js_arguments.argc);
-  CHECK_EQ(0, strcmp(FLAG_js_arguments[0], "testing-float-flag"));
-  CHECK_EQ(0, strcmp(FLAG_js_arguments[1], "7"));
-  CHECK_EQ(1, argc);
-}
-
-
-TEST(FlagsJSArguments1b) {
-  SetFlagsToDefault();
-  const char* str = "--testing-int-flag 42 -- testing-float-flag 7";
-  CHECK_EQ(0, FlagList::SetFlagsFromString(str, StrLength(str)));
-  CHECK_EQ(42, FLAG_testing_int_flag);
-  CHECK_EQ(2.5, FLAG_testing_float_flag);
-  CHECK_EQ(2, FLAG_js_arguments.argc);
-  CHECK_EQ(0, strcmp(FLAG_js_arguments[0], "testing-float-flag"));
-  CHECK_EQ(0, strcmp(FLAG_js_arguments[1], "7"));
-}
-
-
-TEST(FlagsJSArguments2) {
-  SetFlagsToDefault();
-  const char* str = "--testing-int-flag 42 --js-arguments testing-float-flag 7";
-  CHECK_EQ(0, FlagList::SetFlagsFromString(str, StrLength(str)));
-  CHECK_EQ(42, FLAG_testing_int_flag);
-  CHECK_EQ(2.5, FLAG_testing_float_flag);
-  CHECK_EQ(2, FLAG_js_arguments.argc);
-  CHECK_EQ(0, strcmp(FLAG_js_arguments[0], "testing-float-flag"));
-  CHECK_EQ(0, strcmp(FLAG_js_arguments[1], "7"));
-}
-
-
-TEST(FlagsJSArguments3) {
-  SetFlagsToDefault();
-  const char* str = "--testing-int-flag 42 --js-arguments=testing-float-flag 7";
-  CHECK_EQ(0, FlagList::SetFlagsFromString(str, StrLength(str)));
-  CHECK_EQ(42, FLAG_testing_int_flag);
-  CHECK_EQ(2.5, FLAG_testing_float_flag);
-  CHECK_EQ(2, FLAG_js_arguments.argc);
-  CHECK_EQ(0, strcmp(FLAG_js_arguments[0], "testing-float-flag"));
-  CHECK_EQ(0, strcmp(FLAG_js_arguments[1], "7"));
-}
-
-
-TEST(FlagsJSArguments4) {
-  SetFlagsToDefault();
-  const char* str = "--testing-int-flag 42 --";
-  CHECK_EQ(0, FlagList::SetFlagsFromString(str, StrLength(str)));
-  CHECK_EQ(42, FLAG_testing_int_flag);
-  CHECK_EQ(0, FLAG_js_arguments.argc);
-}
-
-
 TEST(FlagsRemoveIncomplete) {
   // Test that processed command line arguments are removed, even
   // if the list of arguments ends unexpectedly.

test/mjsunit/d8/d8-arguments.js

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: -- 1 2 3
+
+assertEquals(["1", "2", "3"], arguments);

test/mjsunit/d8/d8-no-arguments.js

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+assertEquals(undefined, Object.getOwnPropertyDescriptor(this, "arguments"));

test/mjsunit/regress/regress-crbug-781116-1.js

@@ -9,7 +9,6 @@ function baz(obj, store) {
 }
 function bar(store) {
   baz(Array.prototype, store);
-  baz(this.arguments, true);
 }
 bar(false);
 bar(false);

test/mjsunit/regress/regress-crbug-781116-2.js

@@ -9,7 +9,6 @@ function baz(obj, store) {
 }
 function bar(store) {
   baz(Object.prototype, store);
-  baz(this.arguments, true);
 }
 bar(false);
 bar(false);

test/test262/test262.status

@@ -518,10 +518,6 @@
   'annexB/language/statements/for-await-of/iterator-close-return-emulates-undefined-throws-when-called': [FAIL],
   'annexB/language/statements/for-of/iterator-close-return-emulates-undefined-throws-when-called': [FAIL],
 
-  # https://bugs.chromium.org/p/v8/issues/detail?id=7186
-  'language/statements/class/fields-indirect-eval-err-contains-arguments': [FAIL],
-  'language/expressions/class/fields-indirect-eval-err-contains-arguments': [FAIL],
-
   # https://bugs.chromium.org/p/v8/issues/detail?id=7468
   'language/statements/class/privatename-not-valid-earlyerr-script-8': [FAIL],