Clean up common argument objects that share length property

This CL adds a bit more rigor to the handling of length properties
in JSObject-derived classes that explicitly contain that property
inline.

This involves:
- Introducing a new superclass of JSArgumentsObject called
  JSArgumentsObjectWithLength that is shared with other object
  instances that also have a fixed length property.
- Adding JSArgumentsObjectWithLength to the type hierarchy in Torque,
  including adding fast-cases for leading the length property for all
  classes deriving from JSObjectWithLength.
- Adding more rigor to Context and NativeContext handling in base.tq.
  This is useful for the map checks required to verify objects are
  argument object types derived from JSArgumentsObjectWithLength.

Change-Id: I2f0a20601ffcb90b3767cbaeb766e9998d3462ec
Reviewed-on: https://chromium-review.googlesource.com/1248661
Commit-Queue: Daniel Clifford <danno@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56289}

src/builtins/array.tq

@@ -14,16 +14,6 @@ module array {
   type FastDoubleElements;
   type DictionaryElements;
 
-  macro GetLengthProperty(context: Context, o: Object): Number {
-    if (BranchIfFastJSArray(o, context)) {
-      const a: JSArray = UnsafeCast<JSArray>(o);
-      return a.length_fast;
-    } else
-      deferred {
-        return ToLength_Inline(context, GetProperty(context, o, kLengthString));
-      }
-  }
-
   macro EnsureWriteableFastElements(array: JSArray) {
     assert(IsFastElementsKind(array.map.elements_kind));
 

src/builtins/base.tq

@@ -26,6 +26,7 @@ type AbstractCode extends HeapObject generates 'TNode<AbstractCode>';
 type Code extends AbstractCode generates 'TNode<Code>';
 type JSReceiver extends HeapObject generates 'TNode<JSReceiver>';
 type Context extends HeapObject generates 'TNode<Context>';
+type NativeContext extends Context generates 'TNode<Context>';
 type String extends HeapObject generates 'TNode<String>';
 type Oddball extends HeapObject generates 'TNode<Oddball>';
 type HeapNumber extends HeapObject generates 'TNode<HeapNumber>';
@@ -35,7 +36,10 @@ type Numeric = Number|BigInt;
 type Boolean extends Oddball generates 'TNode<Oddball>';
 type JSProxy extends JSReceiver generates 'TNode<JSProxy>';
 type JSObject extends JSReceiver generates 'TNode<JSObject>';
-type JSArray extends JSObject generates 'TNode<JSArray>';
+type JSArgumentsObjectWithLength extends JSObject
+    generates 'TNode<JSArgumentsObjectWithLength>';
+type JSArray extends JSArgumentsObjectWithLength
+    generates 'TNode<JSArray>';
 type JSFunction extends JSObject generates 'TNode<JSFunction>';
 type JSBoundFunction extends JSObject generates 'TNode<JSBoundFunction>';
 type Callable = JSFunction|JSBoundFunction|JSProxy;
@@ -51,6 +55,21 @@ type FixedTypedArray extends FixedTypedArrayBase
 type NumberDictionary extends HeapObject
     generates 'TNode<NumberDictionary>';
 
+type NativeContextSlot generates 'TNode<IntPtrT>' constexpr 'int32_t';
+const FAST_ALIASED_ARGUMENTS_MAP_INDEX: constexpr NativeContextSlot
+    generates 'Context::FAST_ALIASED_ARGUMENTS_MAP_INDEX';
+const SLOW_ALIASED_ARGUMENTS_MAP_INDEX: constexpr NativeContextSlot
+    generates 'Context::SLOW_ALIASED_ARGUMENTS_MAP_INDEX';
+const STRICT_ARGUMENTS_MAP_INDEX: constexpr NativeContextSlot
+    generates 'Context::STRICT_ARGUMENTS_MAP_INDEX';
+const SLOPPY_ARGUMENTS_MAP_INDEX: constexpr NativeContextSlot
+    generates 'Context::SLOPPY_ARGUMENTS_MAP_INDEX';
+extern operator '[]' macro LoadContextElement(
+    NativeContext, NativeContextSlot): Object;
+
+extern operator '[]' macro LoadContextElement(Context, intptr): Object;
+extern operator '[]' macro LoadContextElement(Context, Smi): Object;
+
 type JSArrayBuffer extends JSObject generates 'TNode<JSArrayBuffer>';
 type JSArrayBufferView extends JSObject
     generates 'TNode<JSArrayBufferView>';
@@ -450,6 +469,8 @@ extern macro ChangeNumberToFloat64(Number): float64;
 extern macro ChangeFloat64ToUintPtr(float64): uintptr;
 extern macro ChangeInt32ToIntPtr(int32): intptr;   // Sign-extends.
 extern macro ChangeUint32ToWord(uint32): uintptr;  // Doesn't sign-extend.
+extern macro LoadNativeContext(Context): NativeContext;
+extern macro LoadJSArrayElementsMap(constexpr ElementsKind, Context): Map;
 
 extern macro NumberConstant(constexpr float64): Number;
 extern macro NumberConstant(constexpr int32): Number;
@@ -463,6 +484,7 @@ extern macro BoolConstant(constexpr bool): bool;
 extern macro StringConstant(constexpr string): String;
 extern macro LanguageModeConstant(constexpr LanguageMode): LanguageMode;
 extern macro Int32Constant(constexpr ElementsKind): ElementsKind;
+extern macro IntPtrConstant(constexpr NativeContextSlot): NativeContextSlot;
 
 macro FromConstexpr<A: type>(o: constexpr int31): A;
 FromConstexpr<intptr>(i: constexpr int31): intptr {
@@ -522,6 +544,11 @@ FromConstexpr<String>(s: constexpr string): String {
 FromConstexpr<Object>(s: constexpr string): Object {
   return StringConstant(s);
 }
+macro FromConstexpr<A: type>(e: constexpr NativeContextSlot): A;
+FromConstexpr<NativeContextSlot>(c: constexpr NativeContextSlot):
+    NativeContextSlot {
+  return IntPtrConstant(c);
+}
 
 macro Convert<A: type>(i: constexpr int31): A {
   return i;
@@ -607,6 +634,7 @@ Convert<intptr>(r: RawPtr): intptr {
 extern macro UnsafeCastNumberToHeapNumber(Number): HeapNumber;
 extern macro UnsafeCastObjectToFixedArrayBase(Object): FixedArrayBase;
 extern macro UnsafeCastObjectToFixedArray(Object): FixedArray;
+extern macro UnsafeCastObjectToContext(Object): Context;
 extern macro UnsafeCastObjectToFixedDoubleArray(Object): FixedDoubleArray;
 extern macro UnsafeCastObjectToHeapNumber(Object): HeapNumber;
 extern macro UnsafeCastObjectToCallable(Object): Callable;
@@ -670,6 +698,46 @@ UnsafeCast<Map>(o: Object): Map {
 UnsafeCast<FixedArrayBase>(o: Object): FixedArrayBase {
   return UnsafeCastObjectToFixedArrayBase(o);
 }
+UnsafeCast<Context>(o: Object): Context {
+  return UnsafeCastObjectToContext(o);
+}
+
+// RawCasts should *never* be used anywhere in Torque code except for
+// in Torque-based UnsafeCast operators preceeded by an appropriate
+// type check().
+extern macro RawCastObjectToJSArgumentsObjectWithLength(Object):
+    JSArgumentsObjectWithLength;
+
+macro BranchIfJSArgumentsObjectWithLength(context: Context, o: Object): never
+    labels True, False {
+  const heapObject: HeapObject = Cast<HeapObject>(o) otherwise False;
+  const map: Map = heapObject.map;
+  const nativeContext: NativeContext = LoadNativeContext(context);
+  if (map == nativeContext[FAST_ALIASED_ARGUMENTS_MAP_INDEX]) goto True;
+  if (map == nativeContext[SLOW_ALIASED_ARGUMENTS_MAP_INDEX]) goto True;
+  if (map == nativeContext[STRICT_ARGUMENTS_MAP_INDEX]) goto True;
+  if (map != nativeContext[SLOPPY_ARGUMENTS_MAP_INDEX]) goto False;
+  goto True;
+}
+
+macro UnsafeCast<A: type>(context: Context, o: Object): A;
+UnsafeCast<JSArgumentsObjectWithLength>(
+    context: Context, o: Object): JSArgumentsObjectWithLength {
+  assert(BranchIfJSArgumentsObjectWithLength(context, o));
+  return RawCastObjectToJSArgumentsObjectWithLength(o);
+}
+
+macro Cast<A: type>(context: Context, o: Object): A
+    labels CastError;
+Cast<JSArgumentsObjectWithLength>(context: Context, o: Object):
+    JSArgumentsObjectWithLength
+    labels CastError {
+  if (BranchIfJSArgumentsObjectWithLength(context, o)) {
+    return UnsafeCast<JSArgumentsObjectWithLength>(context, o);
+  } else {
+    goto CastError;
+  }
+}
 
 const kCOWMap: Map = UnsafeCast<Map>(LoadRoot(kFixedCOWArrayMapRootIndex));
 const kEmptyFixedArray: FixedArrayBase =
@@ -704,6 +772,8 @@ extern operator '.elements=' macro StoreElements(JSObject, FixedArrayBase);
 
 extern operator '.length' macro LoadJSTypedArrayLength(JSTypedArray): Smi;
 extern operator '.length' macro LoadJSArrayLength(JSArray): Number;
+extern operator '.length' macro LoadJSArgumentsObjectWithLength(
+    JSArgumentsObjectWithLength): Object;
 extern operator '.length_fast' macro LoadFastJSArrayLength(JSArray): Smi;
 extern operator '.length=' macro StoreJSArrayLength(JSArray, Smi);
 
@@ -896,3 +966,23 @@ macro ToIndex(input: Object, context: Context): Number
 
   return value;
 }
+
+macro GetLengthProperty(context: Context, o: Object): Number {
+  let length: Object;
+  try {
+    try {
+      return (Cast<JSArray>(o) otherwise CheckArgs).length;
+    }
+    label CheckArgs {
+      const a: JSArgumentsObjectWithLength =
+          Cast<JSArgumentsObjectWithLength>(context, o) otherwise Slow;
+      return Cast<Smi>(length = a.length) otherwise ToLength;
+    }
+  }
+  label Slow deferred {
+    return ToLength_Inline(context, GetProperty(context, o, kLengthString));
+  }
+  label ToLength deferred {
+    return ToLength_Inline(context, length);
+  }
+}

src/builtins/builtins-array-gen.cc

@@ -1378,8 +1378,8 @@ TF_BUILTIN(ArrayPrototypeSlice, ArrayPrototypeSliceCodeStubAssembler) {
   Goto(&generic_length);
 
   BIND(&load_arguments_length);
-  Node* arguments_length =
-      LoadObjectField(array_receiver, JSArgumentsObject::kLengthOffset);
+  Node* arguments_length = LoadObjectField(
+      array_receiver, JSArgumentsObjectWithLength::kLengthOffset);
   GotoIf(TaggedIsNotSmi(arguments_length), &generic_length);
   o = CAST(receiver);
   len.Bind(arguments_length);

src/builtins/builtins-call-gen.cc

@@ -193,9 +193,9 @@ void CallOrConstructBuiltinsAssembler::CallOrConstructWithArrayLike(
   BIND(&if_arguments);
   {
     TNode<JSArgumentsObject> js_arguments = CAST(arguments_list);
-    // Try to extract the elements from an JSArgumentsObject.
-    TNode<Object> length =
-        LoadObjectField(js_arguments, JSArgumentsObject::kLengthOffset);
+    // Try to extract the elements from an JSArgumentsObjectWithLength.
+    TNode<Object> length = LoadObjectField(
+        js_arguments, JSArgumentsObjectWithLength::kLengthOffset);
     TNode<FixedArrayBase> elements = LoadElements(js_arguments);
     TNode<Smi> elements_length = LoadFixedArrayBaseLength(elements);
     GotoIfNot(WordEqual(length, elements_length), &if_runtime);

src/code-stub-assembler.cc

@@ -1545,6 +1545,11 @@ TNode<Number> CodeStubAssembler::LoadJSArrayLength(SloppyTNode<JSArray> array) {
   return CAST(LoadObjectField(array, JSArray::kLengthOffset));
 }
 
+TNode<Object> CodeStubAssembler::LoadJSArgumentsObjectWithLength(
+    SloppyTNode<JSArgumentsObjectWithLength> array) {
+  return LoadObjectField(array, JSArgumentsObjectWithLength::kLengthOffset);
+}
+
 TNode<Smi> CodeStubAssembler::LoadFastJSArrayLength(
     SloppyTNode<JSArray> array) {
   TNode<Object> length = LoadJSArrayLength(array);
@@ -2525,8 +2530,16 @@ TNode<Object> CodeStubAssembler::LoadContextElement(
 TNode<Object> CodeStubAssembler::LoadContextElement(
     SloppyTNode<Context> context, SloppyTNode<IntPtrT> slot_index) {
   Node* offset =
-      IntPtrAdd(TimesPointerSize(slot_index),
-                IntPtrConstant(Context::kHeaderSize - kHeapObjectTag));
+      ElementOffsetFromIndex(slot_index, PACKED_ELEMENTS, INTPTR_PARAMETERS,
+                             Context::kHeaderSize - kHeapObjectTag);
+  return UncheckedCast<Object>(Load(MachineType::AnyTagged(), context, offset));
+}
+
+TNode<Object> CodeStubAssembler::LoadContextElement(TNode<Context> context,
+                                                    TNode<Smi> slot_index) {
+  Node* offset =
+      ElementOffsetFromIndex(slot_index, PACKED_ELEMENTS, SMI_PARAMETERS,
+                             Context::kHeaderSize - kHeapObjectTag);
   return UncheckedCast<Object>(Load(MachineType::AnyTagged(), context, offset));
 }
 

src/code-stub-assembler.h

@@ -11,6 +11,7 @@
 #include "src/compiler/code-assembler.h"
 #include "src/globals.h"
 #include "src/objects.h"
+#include "src/objects/arguments.h"
 #include "src/objects/bigint.h"
 #include "src/roots.h"
 
@@ -346,6 +347,10 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
     return CAST(p_o);
   }
 
+  TNode<Context> UnsafeCastObjectToContext(TNode<Object> p_o) {
+    return CAST(p_o);
+  }
+
   TNode<FixedDoubleArray> UnsafeCastObjectToFixedDoubleArray(
       TNode<Object> p_o) {
     return CAST(p_o);
@@ -403,6 +408,11 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
 
   TNode<Map> UnsafeCastObjectToMap(TNode<Object> p_o) { return CAST(p_o); }
 
+  TNode<JSArgumentsObjectWithLength> RawCastObjectToJSArgumentsObjectWithLength(
+      TNode<Object> p_o) {
+    return TNode<JSArgumentsObjectWithLength>::UncheckedCast(p_o);
+  }
+
   Node* MatchesParameterMode(Node* value, ParameterMode mode);
 
 #define PARAMETER_BINOP(OpName, IntPtrOpName, SmiOpName) \
@@ -857,6 +867,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   // Load the elements backing store of a JSObject.
   TNode<FixedArrayBase> LoadElements(SloppyTNode<JSObject> object);
   // Load the length of a JSArray instance.
+  TNode<Object> LoadJSArgumentsObjectWithLength(
+      SloppyTNode<JSArgumentsObjectWithLength> array);
+  // Load the length of a JSArray instance.
   TNode<Number> LoadJSArrayLength(SloppyTNode<JSArray> array);
   // Load the length of a fast JSArray instance. Returns a positive Smi.
   TNode<Smi> LoadFastJSArrayLength(SloppyTNode<JSArray> array);
@@ -1120,6 +1133,8 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
                                    int slot_index);
   TNode<Object> LoadContextElement(SloppyTNode<Context> context,
                                    SloppyTNode<IntPtrT> slot_index);
+  TNode<Object> LoadContextElement(TNode<Context> context,
+                                   TNode<Smi> slot_index);
   void StoreContextElement(SloppyTNode<Context> context, int slot_index,
                            SloppyTNode<Object> value);
   void StoreContextElement(SloppyTNode<Context> context,

src/compiler/access-builder.cc

@@ -801,10 +801,11 @@ FieldAccess AccessBuilder::ForValue() {
 
 // static
 FieldAccess AccessBuilder::ForArgumentsLength() {
-  FieldAccess access = {kTaggedBase,         JSArgumentsObject::kLengthOffset,
-                        Handle<Name>(),      MaybeHandle<Map>(),
-                        Type::NonInternal(), MachineType::AnyTagged(),
-                        kFullWriteBarrier};
+  FieldAccess access = {
+      kTaggedBase,         JSArgumentsObjectWithLength::kLengthOffset,
+      Handle<Name>(),      MaybeHandle<Map>(),
+      Type::NonInternal(), MachineType::AnyTagged(),
+      kFullWriteBarrier};
   return access;
 }
 

src/compiler/js-call-reducer.cc

@@ -3029,7 +3029,7 @@ Reduction JSCallReducer::ReduceCallOrConstructWithArrayLikeOrSpread(
         if (access.offset == JSArray::kLengthOffset) {
           // Ignore uses for arguments#length.
           STATIC_ASSERT(JSArray::kLengthOffset ==
-                        JSArgumentsObject::kLengthOffset);
+                        JSArgumentsObjectWithLength::kLengthOffset);
           continue;
         } else if (access.offset == JSObject::kElementsOffset) {
           // Ignore safe uses for arguments#elements.

src/objects/arguments-inl.h

@@ -63,7 +63,8 @@ bool JSSloppyArgumentsObject::GetSloppyArgumentsLength(Isolate* isolate,
     return false;
   }
   DCHECK(object->HasFastElements() || object->HasFastArgumentsElements());
-  Object* len_obj = object->InObjectPropertyAt(JSArgumentsObject::kLengthIndex);
+  Object* len_obj =
+      object->InObjectPropertyAt(JSArgumentsObjectWithLength::kLengthIndex);
   if (!len_obj->IsSmi()) return false;
   *out = Max(0, Smi::ToInt(len_obj));
 

src/objects/arguments.h

@@ -14,11 +14,21 @@
 namespace v8 {
 namespace internal {
 
+// Superclass for all objects with instance type {JS_ARGUMENTS_TYPE}
+class JSArgumentsObject : public JSObject {
+ public:
+  DECL_VERIFIER(JSArgumentsObject)
+  DECL_CAST(JSArgumentsObject)
+
+ private:
+  DISALLOW_IMPLICIT_CONSTRUCTORS(JSArgumentsObject);
+};
+
 // Common superclass for JSSloppyArgumentsObject and JSStrictArgumentsObject.
 // Note that the instance type {JS_ARGUMENTS_TYPE} does _not_ guarantee the
 // below layout, the in-object properties might have transitioned to dictionary
 // mode already. Only use the below layout with the specific initial maps.
-class JSArgumentsObject : public JSObject {
+class JSArgumentsObjectWithLength : public JSArgumentsObject {
  public:
   // Offsets of object fields.
   static const int kLengthOffset = JSObject::kHeaderSize;
@@ -26,19 +36,19 @@ class JSArgumentsObject : public JSObject {
   // Indices of in-object properties.
   static const int kLengthIndex = 0;
 
-  DECL_VERIFIER(JSArgumentsObject)
-  DECL_CAST(JSArgumentsObject)
+  DECL_VERIFIER(JSArgumentsObjectWithLength)
+  DECL_CAST(JSArgumentsObjectWithLength)
 
  private:
-  DISALLOW_IMPLICIT_CONSTRUCTORS(JSArgumentsObject);
+  DISALLOW_IMPLICIT_CONSTRUCTORS(JSArgumentsObjectWithLength);
 };
 
 // JSSloppyArgumentsObject is just a JSObject with specific initial map.
 // This initial map adds in-object properties for "length" and "callee".
-class JSSloppyArgumentsObject : public JSArgumentsObject {
+class JSSloppyArgumentsObject : public JSArgumentsObjectWithLength {
  public:
   // Offsets of object fields.
-  static const int kCalleeOffset = JSArgumentsObject::kSize;
+  static const int kCalleeOffset = JSArgumentsObjectWithLength::kSize;
   static const int kSize = kCalleeOffset + kPointerSize;
   // Indices of in-object properties.
   static const int kCalleeIndex = kLengthIndex + 1;
@@ -53,10 +63,10 @@ class JSSloppyArgumentsObject : public JSArgumentsObject {
 
 // JSStrictArgumentsObject is just a JSObject with specific initial map.
 // This initial map adds an in-object property for "length".
-class JSStrictArgumentsObject : public JSArgumentsObject {
+class JSStrictArgumentsObject : public JSArgumentsObjectWithLength {
  public:
   // Offsets of object fields.
-  static const int kSize = JSArgumentsObject::kSize;
+  static const int kSize = JSArgumentsObjectWithLength::kSize;
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(JSStrictArgumentsObject);