[runtime] Add arguments type: kRuntime and kJS.

This adds static types to the argument class that accesss the arguments in the
stack. kRuntime arguments are used by runtime functions and kJS arguments
are used to access the JS stack (eg. builtins).

The distinction allows the reversal of arguments in the JS stack without
changing the runtime arguments order.

Bug: v8:10201
Change-Id: I7c08164d53c4071c7910836fa733dee8ff7fa680
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2066985
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Auto-Submit: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66470}

include/v8.h

@@ -120,6 +120,8 @@ class EscapableHandleScope;
 template<typename T> class ReturnValue;
 
 namespace internal {
+enum class ArgumentsType;
+template <ArgumentsType>
 class Arguments;
 class DeferredHandles;
 class Heap;

src/api/api.cc

@@ -9950,7 +9950,7 @@ debug::ConsoleCallArguments::ConsoleCallArguments(
     : v8::FunctionCallbackInfo<v8::Value>(
           nullptr,
           // Drop the first argument (receiver, i.e. the "console" object).
-          args.address_of_arg_at(args.length() > 1 ? 1 : 0),
+          args.length() > 1 ? args.address_of_first_argument() : nullptr,
           args.length() - 1) {}
 
 int debug::GetStackFrameId(v8::Local<v8::StackFrame> frame) {

src/builtins/builtins-api.cc

@@ -105,9 +105,9 @@ V8_WARN_UNUSED_RESULT MaybeHandle<Object> HandleApiCallHelper(
     CallHandlerInfo call_data = CallHandlerInfo::cast(raw_call_data);
     Object data_obj = call_data.data();
 
-    FunctionCallbackArguments custom(isolate, data_obj, *function, raw_holder,
-                                     *new_target, args.address_of_arg_at(1),
-                                     args.length() - 1);
+    FunctionCallbackArguments custom(
+        isolate, data_obj, *function, raw_holder, *new_target,
+        args.address_of_first_argument(), args.length() - 1);
     Handle<Object> result = custom.Call(call_data);
 
     RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
@@ -269,9 +269,9 @@ V8_WARN_UNUSED_RESULT static Object HandleApiCallAsFunctionOrConstructor(
   {
     HandleScope scope(isolate);
     LOG(isolate, ApiObjectAccess("call non-function", obj));
-    FunctionCallbackArguments custom(isolate, call_data.data(), constructor,
-                                     obj, new_target, args.address_of_arg_at(1),
-                                     args.length() - 1);
+    FunctionCallbackArguments custom(
+        isolate, call_data.data(), constructor, obj, new_target,
+        args.address_of_first_argument(), args.length() - 1);
     Handle<Object> result_handle = custom.Call(call_data);
     if (result_handle.is_null()) {
       result = ReadOnlyRoots(isolate).undefined_value();

src/builtins/builtins-utils-inl.h

@@ -23,11 +23,21 @@ Handle<Object> BuiltinArguments::atOrUndefined(Isolate* isolate,
 Handle<Object> BuiltinArguments::receiver() const { return at<Object>(0); }
 
 Handle<JSFunction> BuiltinArguments::target() const {
-  return Arguments::at<JSFunction>(Arguments::length() - 1 - kTargetOffset);
+#ifdef V8_REVERSE_JSARGS
+  int index = kTargetOffset;
+#else
+  int index = Arguments::length() - 1 - kTargetOffset;
+#endif
+  return Handle<JSFunction>(address_of_arg_at(index));
 }
 
 Handle<HeapObject> BuiltinArguments::new_target() const {
-  return Arguments::at<HeapObject>(Arguments::length() - 1 - kNewTargetOffset);
+#ifdef V8_REVERSE_JSARGS
+  int index = kNewTargetOffset;
+#else
+  int index = Arguments::length() - 1 - kNewTargetOffset;
+#endif
+  return Handle<JSFunction>(address_of_arg_at(index));
 }
 
 }  // namespace internal

src/builtins/builtins-utils.h

@@ -15,7 +15,7 @@ namespace v8 {
 namespace internal {
 
 // Arguments object passed to C++ builtins.
-class BuiltinArguments : public Arguments {
+class BuiltinArguments : public JavaScriptArguments {
  public:
   BuiltinArguments(int length, Address* arguments)
       : Arguments(length, arguments) {
@@ -25,13 +25,22 @@ class BuiltinArguments : public Arguments {
 
   Object operator[](int index) const {
     DCHECK_LT(index, length());
-    return Arguments::operator[](index);
+    return Object(*address_of_arg_at(index + kArgsOffset));
   }
 
   template <class S = Object>
   Handle<S> at(int index) const {
     DCHECK_LT(index, length());
-    return Arguments::at<S>(index);
+    return Handle<S>(address_of_arg_at(index + kArgsOffset));
+  }
+
+  inline void set_at(int index, Object value) {
+    DCHECK_LT(index, length());
+    *address_of_arg_at(index + kArgsOffset) = value.ptr();
+  }
+
+  inline Address* address_of_first_argument() const {
+    return address_of_arg_at(kArgsOffset + 1);  // Skips receiver.
   }
 
   static constexpr int kNewTargetOffset = 0;
@@ -42,6 +51,12 @@ class BuiltinArguments : public Arguments {
   static constexpr int kNumExtraArgs = 4;
   static constexpr int kNumExtraArgsWithReceiver = 5;
 
+#ifdef V8_REVERSE_JSARGS
+  static constexpr int kArgsOffset = 4;
+#else
+  static constexpr int kArgsOffset = 0;
+#endif
+
   inline Handle<Object> atOrUndefined(Isolate* isolate, int index) const;
   inline Handle<Object> receiver() const;
   inline Handle<JSFunction> target() const;

src/common/globals.h

@@ -586,11 +586,19 @@ enum class HeapObjectReferenceType {
   STRONG,
 };
 
+enum class ArgumentsType {
+  kRuntime,
+  kJS,
+};
+
 // -----------------------------------------------------------------------------
 // Forward declarations for frequently used classes
 
 class AccessorInfo;
+template <ArgumentsType>
 class Arguments;
+using RuntimeArguments = Arguments<ArgumentsType::kRuntime>;
+using JavaScriptArguments = Arguments<ArgumentsType::kJS>;
 class Assembler;
 class ClassScope;
 class Code;

src/execution/arguments-inl.h

@@ -13,16 +13,15 @@
 namespace v8 {
 namespace internal {
 
-template <class S>
-Handle<S> Arguments::at(int index) const {
-  return Handle<S>::cast(at<Object>(index));
-}
-
-int Arguments::smi_at(int index) const {
+template <ArgumentsType T>
+int Arguments<T>::smi_at(int index) const {
   return Smi::ToInt(Object(*address_of_arg_at(index)));
 }
 
-double Arguments::number_at(int index) const { return (*this)[index].Number(); }
+template <ArgumentsType T>
+double Arguments<T>::number_at(int index) const {
+  return (*this)[index].Number();
+}
 
 }  // namespace internal
 }  // namespace v8

src/execution/arguments.h

@@ -30,6 +30,7 @@ namespace internal {
 // Note that length_ (whose value is in the integer range) is defined
 // as intptr_t to provide endian-neutrality on 64-bit archs.
 
+template <ArgumentsType arguments_type>
 class Arguments {
  public:
   Arguments(int length, Address* arguments)
@@ -58,25 +59,46 @@ class Arguments {
 
   inline Address* address_of_arg_at(int index) const {
     DCHECK_LT(static_cast<uint32_t>(index), static_cast<uint32_t>(length_));
+    int offset = index * kSystemPointerSize;
+#ifdef V8_REVERSE_JSARGS
+    if (arguments_type == ArgumentsType::kJS) {
+      offset = (length_ - index - 1) * kSystemPointerSize;
+    }
+#endif
     return reinterpret_cast<Address*>(reinterpret_cast<Address>(arguments_) -
-                                      index * kSystemPointerSize);
+                                      offset);
   }
 
   // Get the total number of arguments including the receiver.
   int length() const { return static_cast<int>(length_); }
 
   // Arguments on the stack are in reverse order (compared to an array).
-  FullObjectSlot first_slot() const { return slot_at(length() - 1); }
-  FullObjectSlot last_slot() const { return slot_at(0); }
+  FullObjectSlot first_slot() const {
+    int index = length() - 1;
+#ifdef V8_REVERSE_JSARGS
+    if (arguments_type == ArgumentsType::kJS) index = 0;
+#endif
+    return slot_at(index);
+  }
+
+  FullObjectSlot last_slot() const {
+    int index = 0;
+#ifdef V8_REVERSE_JSARGS
+    if (arguments_type == ArgumentsType::kJS) index = length() - 1;
+#endif
+    return slot_at(index);
+  }
 
  private:
   intptr_t length_;
   Address* arguments_;
 };
 
-template <>
-inline Handle<Object> Arguments::at(int index) const {
-  return Handle<Object>(address_of_arg_at(index));
+template <ArgumentsType T>
+template <class S>
+Handle<S> Arguments<T>::at(int index) const {
+  Handle<Object> obj = Handle<Object>(address_of_arg_at(index));
+  return Handle<S>::cast(obj);
 }
 
 double ClobberDoubleRegisters(double x1, double x2, double x3, double x4);
@@ -90,7 +112,7 @@ double ClobberDoubleRegisters(double x1, double x2, double x3, double x4);
 // TODO(cbruni): add global flag to check whether any tracing events have been
 // enabled.
 #define RUNTIME_FUNCTION_RETURNS_TYPE(Type, InternalType, Convert, Name)      \
-  static V8_INLINE InternalType __RT_impl_##Name(Arguments args,              \
+  static V8_INLINE InternalType __RT_impl_##Name(RuntimeArguments args,       \
                                                  Isolate* isolate);           \
                                                                               \
   V8_NOINLINE static Type Stats_##Name(int args_length, Address* args_object, \
@@ -98,7 +120,7 @@ double ClobberDoubleRegisters(double x1, double x2, double x3, double x4);
     RuntimeCallTimerScope timer(isolate, RuntimeCallCounterId::k##Name);      \
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.runtime"),                     \
                  "V8.Runtime_" #Name);                                        \
-    Arguments args(args_length, args_object);                                 \
+    RuntimeArguments args(args_length, args_object);                          \
     return Convert(__RT_impl_##Name(args, isolate));                          \
   }                                                                           \
                                                                               \
@@ -108,11 +130,11 @@ double ClobberDoubleRegisters(double x1, double x2, double x3, double x4);
     if (V8_UNLIKELY(TracingFlags::is_runtime_stats_enabled())) {              \
       return Stats_##Name(args_length, args_object, isolate);                 \
     }                                                                         \
-    Arguments args(args_length, args_object);                                 \
+    RuntimeArguments args(args_length, args_object);                          \
     return Convert(__RT_impl_##Name(args, isolate));                          \
   }                                                                           \
                                                                               \
-  static InternalType __RT_impl_##Name(Arguments args, Isolate* isolate)
+  static InternalType __RT_impl_##Name(RuntimeArguments args, Isolate* isolate)
 
 #define CONVERT_OBJECT(x) (x).ptr()
 #define CONVERT_OBJECTPAIR(x) (x)

src/objects/elements.cc

@@ -645,22 +645,23 @@ class ElementsAccessorBase : public InternalElementsAccessor {
     UNREACHABLE();
   }
 
-  uint32_t Push(Handle<JSArray> receiver, Arguments* args,
+  uint32_t Push(Handle<JSArray> receiver, JavaScriptArguments* args,
                 uint32_t push_size) final {
     return Subclass::PushImpl(receiver, args, push_size);
   }
 
-  static uint32_t PushImpl(Handle<JSArray> receiver, Arguments* args,
+  static uint32_t PushImpl(Handle<JSArray> receiver, JavaScriptArguments* args,
                            uint32_t push_sized) {
     UNREACHABLE();
   }
 
-  uint32_t Unshift(Handle<JSArray> receiver, Arguments* args,
+  uint32_t Unshift(Handle<JSArray> receiver, JavaScriptArguments* args,
                    uint32_t unshift_size) final {
     return Subclass::UnshiftImpl(receiver, args, unshift_size);
   }
 
-  static uint32_t UnshiftImpl(Handle<JSArray> receiver, Arguments* args,
+  static uint32_t UnshiftImpl(Handle<JSArray> receiver,
+                              JavaScriptArguments* args,
                               uint32_t unshift_size) {
     UNREACHABLE();
   }
@@ -2091,7 +2092,7 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
     return Subclass::RemoveElement(receiver, AT_START);
   }
 
-  static uint32_t PushImpl(Handle<JSArray> receiver, Arguments* args,
+  static uint32_t PushImpl(Handle<JSArray> receiver, JavaScriptArguments* args,
                            uint32_t push_size) {
     Handle<FixedArrayBase> backing_store(receiver->elements(),
                                          receiver->GetIsolate());
@@ -2099,7 +2100,8 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
                                   AT_END);
   }
 
-  static uint32_t UnshiftImpl(Handle<JSArray> receiver, Arguments* args,
+  static uint32_t UnshiftImpl(Handle<JSArray> receiver,
+                              JavaScriptArguments* args,
                               uint32_t unshift_size) {
     Handle<FixedArrayBase> backing_store(receiver->elements(),
                                          receiver->GetIsolate());
@@ -2347,7 +2349,7 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
 
   static uint32_t AddArguments(Handle<JSArray> receiver,
                                Handle<FixedArrayBase> backing_store,
-                               Arguments* args, uint32_t add_size,
+                               JavaScriptArguments* args, uint32_t add_size,
                                Where add_position) {
     uint32_t length = Smi::ToInt(receiver->length());
     DCHECK_LT(0, add_size);
@@ -2382,7 +2384,8 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
     return new_length;
   }
 
-  static void CopyArguments(Arguments* args, Handle<FixedArrayBase> dst_store,
+  static void CopyArguments(JavaScriptArguments* args,
+                            Handle<FixedArrayBase> dst_store,
                             uint32_t copy_size, uint32_t src_index,
                             uint32_t dst_index) {
     // Add the provided values.
@@ -2564,7 +2567,7 @@ class FastNonextensibleObjectElementsAccessor
  public:
   using BackingStore = typename KindTraits::BackingStore;
 
-  static uint32_t PushImpl(Handle<JSArray> receiver, Arguments* args,
+  static uint32_t PushImpl(Handle<JSArray> receiver, JavaScriptArguments* args,
                            uint32_t push_size) {
     UNREACHABLE();
   }
@@ -2659,7 +2662,7 @@ class FastSealedObjectElementsAccessor
 
   static Handle<Object> PopImpl(Handle<JSArray> receiver) { UNREACHABLE(); }
 
-  static uint32_t PushImpl(Handle<JSArray> receiver, Arguments* args,
+  static uint32_t PushImpl(Handle<JSArray> receiver, JavaScriptArguments* args,
                            uint32_t push_size) {
     UNREACHABLE();
   }
@@ -2771,7 +2774,7 @@ class FastFrozenObjectElementsAccessor
 
   static Handle<Object> PopImpl(Handle<JSArray> receiver) { UNREACHABLE(); }
 
-  static uint32_t PushImpl(Handle<JSArray> receiver, Arguments* args,
+  static uint32_t PushImpl(Handle<JSArray> receiver, JavaScriptArguments* args,
                            uint32_t push_size) {
     UNREACHABLE();
   }
@@ -4632,8 +4635,8 @@ class SlowStringWrapperElementsAccessor
 
 }  // namespace
 
-MaybeHandle<Object> ArrayConstructInitializeElements(Handle<JSArray> array,
-                                                     Arguments* args) {
+MaybeHandle<Object> ArrayConstructInitializeElements(
+    Handle<JSArray> array, JavaScriptArguments* args) {
   if (args->length() == 0) {
     // Optimize the case where there are no parameters passed.
     JSArray::Initialize(array, JSArray::kPreallocatedArrayElements);
@@ -4792,7 +4795,8 @@ void ElementsAccessor::TearDown() {
   elements_accessors_ = nullptr;
 }
 
-Handle<JSArray> ElementsAccessor::Concat(Isolate* isolate, Arguments* args,
+Handle<JSArray> ElementsAccessor::Concat(Isolate* isolate,
+                                         JavaScriptArguments* args,
                                          uint32_t concat_size,
                                          uint32_t result_len) {
   ElementsKind result_elements_kind = GetInitialFastElementsKind();

src/objects/elements.h

@@ -111,13 +111,13 @@ class ElementsAccessor {
                    Handle<Object> value, PropertyAttributes attributes,
                    uint32_t new_capacity) = 0;
 
-  static Handle<JSArray> Concat(Isolate* isolate, Arguments* args,
+  static Handle<JSArray> Concat(Isolate* isolate, JavaScriptArguments* args,
                                 uint32_t concat_size, uint32_t result_length);
 
-  virtual uint32_t Push(Handle<JSArray> receiver, Arguments* args,
+  virtual uint32_t Push(Handle<JSArray> receiver, JavaScriptArguments* args,
                         uint32_t push_size) = 0;
 
-  virtual uint32_t Unshift(Handle<JSArray> receiver, Arguments* args,
+  virtual uint32_t Unshift(Handle<JSArray> receiver, JavaScriptArguments* args,
                            uint32_t unshift_size) = 0;
 
   virtual Handle<Object> Pop(Handle<JSArray> receiver) = 0;
@@ -206,7 +206,7 @@ class ElementsAccessor {
 };
 
 V8_WARN_UNUSED_RESULT MaybeHandle<Object> ArrayConstructInitializeElements(
-    Handle<JSArray> array, Arguments* args);
+    Handle<JSArray> array, JavaScriptArguments* args);
 
 // Called directly from CSA.
 // {raw_context}: Context pointer.

src/objects/js-objects.cc

@@ -4564,8 +4564,8 @@ void JSObject::SetImmutableProto(Handle<JSObject> object) {
 }
 
 void JSObject::EnsureCanContainElements(Handle<JSObject> object,
-                                        Arguments* args, uint32_t first_arg,
-                                        uint32_t arg_count,
+                                        JavaScriptArguments* args,
+                                        uint32_t first_arg, uint32_t arg_count,
                                         EnsureElementsMode mode) {
   // Elements in |Arguments| are ordered backwards (because they're on the
   // stack), but the method that's called here iterates over them in forward

src/objects/js-objects.h

@@ -515,8 +515,8 @@ class JSObject : public TorqueGeneratedJSObject<JSObject, JSReceiver> {
                                               uint32_t length,
                                               EnsureElementsMode mode);
   static void EnsureCanContainElements(Handle<JSObject> object,
-                                       Arguments* arguments, uint32_t first_arg,
-                                       uint32_t arg_count,
+                                       JavaScriptArguments* arguments,
+                                       uint32_t first_arg, uint32_t arg_count,
                                        EnsureElementsMode mode);
 
   // Would we convert a fast elements array to dictionary mode given

src/runtime/runtime-array.cc

@@ -46,8 +46,8 @@ RUNTIME_FUNCTION(Runtime_NewArray) {
   HandleScope scope(isolate);
   DCHECK_LE(3, args.length());
   int const argc = args.length() - 3;
-  // TODO(bmeurer): Remove this Arguments nonsense.
-  Arguments argv(argc, args.address_of_arg_at(1));
+  // argv points to the arguments constructed by the JavaScript call.
+  JavaScriptArguments argv(argc, args.address_of_arg_at(1));
   CONVERT_ARG_HANDLE_CHECKED(JSFunction, constructor, 0);
   CONVERT_ARG_HANDLE_CHECKED(JSReceiver, new_target, argc + 1);
   CONVERT_ARG_HANDLE_CHECKED(HeapObject, type_info, argc + 2);

src/runtime/runtime-classes.cc

@@ -157,7 +157,8 @@ inline void SetHomeObject(Isolate* isolate, JSFunction method,
 //    shared name.
 template <typename Dictionary>
 MaybeHandle<Object> GetMethodAndSetHomeObjectAndName(
-    Isolate* isolate, Arguments& args,  // NOLINT(runtime/references)
+    Isolate* isolate,
+    RuntimeArguments& args,  // NOLINT(runtime/references)
     Smi index, Handle<JSObject> home_object, Handle<String> name_prefix,
     Handle<Object> key) {
   int int_index = index.value();
@@ -195,7 +196,7 @@ MaybeHandle<Object> GetMethodAndSetHomeObjectAndName(
 // shared name.
 Object GetMethodWithSharedNameAndSetHomeObject(
     Isolate* isolate,
-    Arguments& args,  // NOLINT(runtime/references)
+    RuntimeArguments& args,  // NOLINT(runtime/references)
     Object index, JSObject home_object) {
   DisallowHeapAllocation no_gc;
   int int_index = Smi::ToInt(index);
@@ -235,7 +236,7 @@ Handle<Dictionary> ShallowCopyDictionaryTemplate(
 template <typename Dictionary>
 bool SubstituteValues(Isolate* isolate, Handle<Dictionary> dictionary,
                       Handle<JSObject> receiver,
-                      Arguments& args,  // NOLINT(runtime/references)
+                      RuntimeArguments& args,  // NOLINT(runtime/references)
                       bool* install_name_accessor = nullptr) {
   Handle<Name> name_string = isolate->factory()->name_string();
 
@@ -312,7 +313,8 @@ bool AddDescriptorsByTemplate(
     Isolate* isolate, Handle<Map> map,
     Handle<DescriptorArray> descriptors_template,
     Handle<NumberDictionary> elements_dictionary_template,
-    Handle<JSObject> receiver, Arguments& args) {  // NOLINT(runtime/references)
+    Handle<JSObject> receiver,
+    RuntimeArguments& args) {  // NOLINT(runtime/references)
   int nof_descriptors = descriptors_template->number_of_descriptors();
 
   Handle<DescriptorArray> descriptors =
@@ -423,7 +425,7 @@ bool AddDescriptorsByTemplate(
     Handle<NumberDictionary> elements_dictionary_template,
     Handle<FixedArray> computed_properties, Handle<JSObject> receiver,
     bool install_name_accessor,
-    Arguments& args) {  // NOLINT(runtime/references)
+    RuntimeArguments& args) {  // NOLINT(runtime/references)
   int computed_properties_length = computed_properties->length();
 
   // Shallow-copy properties template.
@@ -511,7 +513,7 @@ bool InitClassPrototype(Isolate* isolate,
                         Handle<JSObject> prototype,
                         Handle<HeapObject> prototype_parent,
                         Handle<JSFunction> constructor,
-                        Arguments& args) {  // NOLINT(runtime/references)
+                        RuntimeArguments& args) {  // NOLINT(runtime/references)
   Handle<Map> map(prototype->map(), isolate);
   map = Map::CopyDropDescriptors(isolate, map);
   map->set_is_prototype_map(true);
@@ -555,11 +557,10 @@ bool InitClassPrototype(Isolate* isolate,
   }
 }
 
-bool InitClassConstructor(Isolate* isolate,
-                          Handle<ClassBoilerplate> class_boilerplate,
-                          Handle<HeapObject> constructor_parent,
-                          Handle<JSFunction> constructor,
-                          Arguments& args) {  // NOLINT(runtime/references)
+bool InitClassConstructor(
+    Isolate* isolate, Handle<ClassBoilerplate> class_boilerplate,
+    Handle<HeapObject> constructor_parent, Handle<JSFunction> constructor,
+    RuntimeArguments& args) {  // NOLINT(runtime/references)
   Handle<Map> map(constructor->map(), isolate);
   map = Map::CopyDropDescriptors(isolate, map);
   DCHECK(map->is_prototype_map());
@@ -611,7 +612,7 @@ bool InitClassConstructor(Isolate* isolate,
 MaybeHandle<Object> DefineClass(
     Isolate* isolate, Handle<ClassBoilerplate> class_boilerplate,
     Handle<Object> super_class, Handle<JSFunction> constructor,
-    Arguments& args) {  // NOLINT(runtime/references)
+    RuntimeArguments& args) {  // NOLINT(runtime/references)
   Handle<Object> prototype_parent;
   Handle<HeapObject> constructor_parent;