[constant-tracking] Fix (Set|Map).p.(add|set) tampering check.

This makes the prototype add function check compatible with
constant field tracking (which is still under a flag).

Change-Id: I768feb55e1568f3e2642f573c9a79755fe3e8d9c
Bug: v8:5495, v8:8361
Reviewed-on: https://chromium-review.googlesource.com/c/1296481Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56926}

src/bootstrapper.cc

@@ -3360,6 +3360,9 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
 
     Handle<JSFunction> map_set = SimpleInstallFunction(
         isolate_, prototype, "set", Builtins::kMapPrototypeSet, 2, true);
+    // Check that index of "set" function in JSCollection is correct.
+    DCHECK_EQ(JSCollection::kAddFunctionDescriptorIndex,
+              prototype->map()->LastAdded());
     native_context()->set_map_set(*map_set);
 
     Handle<JSFunction> map_has = SimpleInstallFunction(
@@ -3461,6 +3464,9 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
 
     Handle<JSFunction> set_add = SimpleInstallFunction(
         isolate_, prototype, "add", Builtins::kSetPrototypeAdd, 1, true);
+    // Check that index of "add" function in JSCollection is correct.
+    DCHECK_EQ(JSCollection::kAddFunctionDescriptorIndex,
+              prototype->map()->LastAdded());
     native_context()->set_set_add(*set_add);
 
     Handle<JSFunction> set_delete = SimpleInstallFunction(
@@ -3552,11 +3558,16 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
     Handle<JSFunction> weakmap_get = SimpleInstallFunction(
         isolate_, prototype, "get", Builtins::kWeakMapGet, 1, true);
     native_context()->set_weakmap_get(*weakmap_get);
-    SimpleInstallFunction(isolate_, prototype, "has", Builtins::kWeakMapHas, 1,
-                          true);
+
     Handle<JSFunction> weakmap_set = SimpleInstallFunction(
         isolate_, prototype, "set", Builtins::kWeakMapPrototypeSet, 2, true);
+    // Check that index of "set" function in JSWeakCollection is correct.
+    DCHECK_EQ(JSWeakCollection::kAddFunctionDescriptorIndex,
+              prototype->map()->LastAdded());
+
     native_context()->set_weakmap_set(*weakmap_set);
+    SimpleInstallFunction(isolate_, prototype, "has", Builtins::kWeakMapHas, 1,
+                          true);
 
     JSObject::AddProperty(
         isolate_, prototype, factory->to_string_tag_symbol(),
@@ -3585,8 +3596,13 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
                           Builtins::kWeakSetPrototypeDelete, 1, true);
     SimpleInstallFunction(isolate_, prototype, "has", Builtins::kWeakSetHas, 1,
                           true);
+
     Handle<JSFunction> weakset_add = SimpleInstallFunction(
         isolate_, prototype, "add", Builtins::kWeakSetPrototypeAdd, 1, true);
+    // Check that index of "add" function in JSWeakCollection is correct.
+    DCHECK_EQ(JSWeakCollection::kAddFunctionDescriptorIndex,
+              prototype->map()->LastAdded());
+
     native_context()->set_weakset_add(*weakset_add);
 
     JSObject::AddProperty(

src/builtins/builtins-collections-gen.cc

@@ -100,6 +100,16 @@ class BaseCollectionsAssembler : public CodeStubAssembler {
   TNode<JSFunction> GetInitialAddFunction(Variant variant,
                                           TNode<Context> native_context);
 
+  // Checks whether {collection}'s initial add/set function has been modified
+  // (depending on {variant}, loaded from {native_context}).
+  void CheckIfInitialAddFunctionModified(Variant variant,
+                                         TNode<Context> native_context,
+                                         TNode<Object> collection,
+                                         Label* if_modified);
+
+  // Gets root index for the name of the add/set function.
+  TNode<Object> GetAddFunctionName(Variant variant);
+
   // Retrieves the offset to access the backing table from the collection.
   int GetTableOffset(Variant variant);
 
@@ -116,6 +126,10 @@ class BaseCollectionsAssembler : public CodeStubAssembler {
                                              TNode<Context> native_context,
                                              TNode<Object> collection);
 
+  // Gets the initial prototype map for given collection {variant}.
+  TNode<Map> GetInitialCollectionPrototype(Variant variant,
+                                           TNode<Context> native_context);
+
   // Loads an element from a fixed array.  If the element is the hole, returns
   // `undefined`.
   TNode<Object> LoadAndNormalizeFixedArrayElement(TNode<FixedArray> elements,
@@ -174,9 +188,8 @@ void BaseCollectionsAssembler::AddConstructorEntries(
     TNode<Object> table = AllocateTable(variant, context, at_least_space_for);
     StoreObjectField(collection, GetTableOffset(variant), table);
     GotoIf(IsNullOrUndefined(initial_entries), &exit);
-    GotoIfNot(
-        HasInitialCollectionPrototype(variant, native_context, collection),
-        &slow_loop);
+    CheckIfInitialAddFunctionModified(variant, native_context, collection,
+                                      &slow_loop);
     Branch(use_fast_loop.value(), &fast_loop, &slow_loop);
   }
   BIND(&fast_loop);
@@ -198,8 +211,16 @@ void BaseCollectionsAssembler::AddConstructorEntries(
     if (variant == kMap || variant == kWeakMap) {
       BIND(&if_may_have_side_effects);
 #if DEBUG
-      CSA_ASSERT(this, HasInitialCollectionPrototype(variant, native_context,
-                                                     collection));
+      {
+        // Check that add/set function has not been modified.
+        Label if_not_modified(this), if_modified(this);
+        CheckIfInitialAddFunctionModified(variant, native_context, collection,
+                                          &if_modified);
+        Goto(&if_not_modified);
+        BIND(&if_modified);
+        Unreachable();
+        BIND(&if_not_modified);
+      }
       CSA_ASSERT(this, WordEqual(original_initial_entries_map,
                                  LoadMap(initial_entries_jsarray)));
 #endif
@@ -323,6 +344,65 @@ void BaseCollectionsAssembler::AddConstructorEntriesFromIterable(
   BIND(&exit);
 }
 
+TNode<Object> BaseCollectionsAssembler::GetAddFunctionName(Variant variant) {
+  switch (variant) {
+    case kMap:
+    case kWeakMap:
+      return LoadRoot(RootIndex::kset_string);
+    case kSet:
+    case kWeakSet:
+      return LoadRoot(RootIndex::kadd_string);
+  }
+  UNREACHABLE();
+}
+
+void BaseCollectionsAssembler::CheckIfInitialAddFunctionModified(
+    Variant variant, TNode<Context> native_context, TNode<Object> collection,
+    Label* if_modified) {
+  Label done(this);
+  TVARIABLE(BoolT, result, Int32FalseConstant());
+
+  TNode<Map> prototype_map =
+      LoadMap(LoadMapPrototype(LoadMap(CAST(collection))));
+  GotoIfNot(WordEqual(prototype_map,
+                      GetInitialCollectionPrototype(variant, native_context)),
+            if_modified);
+
+  if (FLAG_track_constant_fields) {
+    // With constant field tracking, we need to make sure that the add/set
+    // function in the prototype has not been tampered with. We do this by
+    // checking that the slot in the prototype's descriptor array is still
+    // marked as const.
+    TNode<DescriptorArray> descriptors = LoadMapDescriptors(prototype_map);
+
+    STATIC_ASSERT(JSCollection::kAddFunctionDescriptorIndex ==
+                  JSWeakCollection::kAddFunctionDescriptorIndex);
+    int index = JSCollection::kAddFunctionDescriptorIndex;
+
+    // Assert the index is in-bounds.
+    CSA_ASSERT(this, SmiLessThan(SmiConstant(index),
+                                 LoadWeakFixedArrayLength(descriptors)));
+    // Assert that the name is correct. This essentially checks that
+    // kAddFunctionDescriptorArrayIndex corresponds to the insertion order in
+    // the bootstrapper.
+    CSA_ASSERT(this,
+               WordEqual(LoadWeakFixedArrayElement(
+                             descriptors, DescriptorArray::ToKeyIndex(index)),
+                         GetAddFunctionName(variant)));
+
+    TNode<Uint32T> details =
+        DescriptorArrayGetDetails(descriptors, Uint32Constant(index));
+
+    TNode<Uint32T> constness =
+        DecodeWord32<PropertyDetails::ConstnessField>(details);
+
+    GotoIfNot(
+        Word32Equal(constness,
+                    Int32Constant(static_cast<int>(PropertyConstness::kConst))),
+        if_modified);
+  }
+}
+
 TNode<Object> BaseCollectionsAssembler::AllocateJSCollection(
     TNode<Context> context, TNode<JSFunction> constructor,
     TNode<Object> new_target) {
@@ -462,8 +542,8 @@ void BaseCollectionsAssembler::GotoIfNotJSReceiver(Node* const obj,
   GotoIfNot(IsJSReceiver(obj), if_not_receiver);
 }
 
-TNode<BoolT> BaseCollectionsAssembler::HasInitialCollectionPrototype(
-    Variant variant, TNode<Context> native_context, TNode<Object> collection) {
+TNode<Map> BaseCollectionsAssembler::GetInitialCollectionPrototype(
+    Variant variant, TNode<Context> native_context) {
   int initial_prototype_index;
   switch (variant) {
     case kMap:
@@ -479,12 +559,16 @@ TNode<BoolT> BaseCollectionsAssembler::HasInitialCollectionPrototype(
       initial_prototype_index = Context::INITIAL_WEAKSET_PROTOTYPE_MAP_INDEX;
       break;
   }
-  TNode<Map> initial_prototype_map =
-      CAST(LoadContextElement(native_context, initial_prototype_index));
+  return CAST(LoadContextElement(native_context, initial_prototype_index));
+}
+
+TNode<BoolT> BaseCollectionsAssembler::HasInitialCollectionPrototype(
+    Variant variant, TNode<Context> native_context, TNode<Object> collection) {
   TNode<Map> collection_proto_map =
       LoadMap(LoadMapPrototype(LoadMap(CAST(collection))));
 
-  return WordEqual(collection_proto_map, initial_prototype_map);
+  return WordEqual(collection_proto_map,
+                   GetInitialCollectionPrototype(variant, native_context));
 }
 
 TNode<Object> BaseCollectionsAssembler::LoadAndNormalizeFixedArrayElement(

src/objects/js-collection.h

@@ -21,6 +21,7 @@ class JSCollection : public JSObject {
 
   static const int kTableOffset = JSObject::kHeaderSize;
   static const int kSize = kTableOffset + kPointerSize;
+  static const int kAddFunctionDescriptorIndex = 3;
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(JSCollection);
@@ -107,6 +108,8 @@ class JSWeakCollection : public JSObject {
   static const int kTableOffset = JSObject::kHeaderSize;
   static const int kSize = kTableOffset + kPointerSize;
 
+  static const int kAddFunctionDescriptorIndex = 3;
+
   // Iterates the function object according to the visiting policy.
   class BodyDescriptorImpl;