[ReadOnlyRoots] Make empty_property_dictionary a RO root

Bug: v8:8238
Change-Id: I112b6ec9c32afe03f43670aa3ed572af7525b26b
Reviewed-on: https://chromium-review.googlesource.com/c/1270586
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Dan Elphick <delphick@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56491}

src/code-stub-assembler.h

@@ -28,8 +28,6 @@ enum class PrimitiveType { kBoolean, kNumber, kString, kSymbol };
 
 #define HEAP_MUTABLE_IMMOVABLE_OBJECT_LIST(V)                              \
   V(ArraySpeciesProtector, array_species_protector, ArraySpeciesProtector) \
-  V(EmptyPropertyDictionary, empty_property_dictionary,                    \
-    EmptyPropertyDictionary)                                               \
   V(PromiseSpeciesProtector, promise_species_protector,                    \
     PromiseSpeciesProtector)                                               \
   V(TypedArraySpeciesProtector, typed_array_species_protector,             \
@@ -44,6 +42,8 @@ enum class PrimitiveType { kBoolean, kNumber, kString, kSymbol };
   V(BooleanMap, boolean_map, BooleanMap)                                     \
   V(CodeMap, code_map, CodeMap)                                              \
   V(EmptyFixedArray, empty_fixed_array, EmptyFixedArray)                     \
+  V(EmptyPropertyDictionary, empty_property_dictionary,                      \
+    EmptyPropertyDictionary)                                                 \
   V(EmptySlowElementDictionary, empty_slow_element_dictionary,               \
     EmptySlowElementDictionary)                                              \
   V(empty_string, empty_string, EmptyString)                                 \

src/heap/object-stats.cc

@@ -750,7 +750,7 @@ bool ObjectStatsCollectorImpl::CanRecordFixedArray(FixedArrayBase* array) {
   return array != roots.empty_fixed_array() &&
          array != roots.empty_sloppy_arguments_elements() &&
          array != roots.empty_slow_element_dictionary() &&
-         array != heap_->empty_property_dictionary();
+         array != roots.empty_property_dictionary();
 }
 
 bool ObjectStatsCollectorImpl::IsCowArray(FixedArrayBase* array) {

src/heap/setup-heap-internal.cc

@@ -736,8 +736,8 @@ void Heap::CreateInitialObjects() {
     to_string_tag_symbol->set_is_interesting_symbol(true);
   }
 
-  Handle<NameDictionary> empty_property_dictionary =
-      NameDictionary::New(isolate(), 1, TENURED, USE_CUSTOM_MINIMUM_CAPACITY);
+  Handle<NameDictionary> empty_property_dictionary = NameDictionary::New(
+      isolate(), 1, TENURED_READ_ONLY, USE_CUSTOM_MINIMUM_CAPACITY);
   DCHECK(!empty_property_dictionary->HasSufficientCapacityToAdd(1));
   set_empty_property_dictionary(*empty_property_dictionary);
 

src/objects-inl.h

@@ -1943,8 +1943,7 @@ Object* Object::GetHash() {
 
   DCHECK(IsJSReceiver());
   JSReceiver* receiver = JSReceiver::cast(this);
-  Isolate* isolate = receiver->GetIsolate();
-  return receiver->GetIdentityHash(isolate);
+  return receiver->GetIdentityHash();
 }
 
 Handle<Object> ObjectHashTableShape::AsHandle(Handle<Object> key) {

src/objects.cc

@@ -6711,16 +6711,14 @@ Handle<NumberDictionary> JSObject::NormalizeElements(Handle<JSObject> object) {
 
 namespace {
 
-Object* SetHashAndUpdateProperties(Isolate* isolate, HeapObject* properties,
-                                   int hash) {
+Object* SetHashAndUpdateProperties(HeapObject* properties, int hash) {
   DCHECK_NE(PropertyArray::kNoHashSentinel, hash);
   DCHECK(PropertyArray::HashField::is_valid(hash));
 
-  Heap* heap = isolate->heap();
-  ReadOnlyRoots roots(heap);
+  ReadOnlyRoots roots = properties->GetReadOnlyRoots();
   if (properties == roots.empty_fixed_array() ||
       properties == roots.empty_property_array() ||
-      properties == heap->empty_property_dictionary()) {
+      properties == roots.empty_property_dictionary()) {
     return Smi::FromInt(hash);
   }
 
@@ -6740,7 +6738,7 @@ Object* SetHashAndUpdateProperties(Isolate* isolate, HeapObject* properties,
   return properties;
 }
 
-int GetIdentityHashHelper(Isolate* isolate, JSReceiver* object) {
+int GetIdentityHashHelper(JSReceiver* object) {
   DisallowHeapAllocation no_gc;
   Object* properties = object->raw_properties_or_hash();
   if (properties->IsSmi()) {
@@ -6760,11 +6758,9 @@ int GetIdentityHashHelper(Isolate* isolate, JSReceiver* object) {
   }
 
 #ifdef DEBUG
-  FixedArray* empty_fixed_array = ReadOnlyRoots(isolate).empty_fixed_array();
-  FixedArray* empty_property_dictionary =
-      isolate->heap()->empty_property_dictionary();
-  DCHECK(properties == empty_fixed_array ||
-         properties == empty_property_dictionary);
+  ReadOnlyRoots roots = object->GetReadOnlyRoots();
+  DCHECK(properties == roots.empty_fixed_array() ||
+         properties == roots.empty_property_dictionary());
 #endif
 
   return PropertyArray::kNoHashSentinel;
@@ -6778,7 +6774,7 @@ void JSReceiver::SetIdentityHash(int hash) {
 
   HeapObject* existing_properties = HeapObject::cast(raw_properties_or_hash());
   Object* new_properties =
-      SetHashAndUpdateProperties(GetIsolate(), existing_properties, hash);
+      SetHashAndUpdateProperties(existing_properties, hash);
   set_raw_properties_or_hash(new_properties);
 }
 
@@ -6787,25 +6783,24 @@ void JSReceiver::SetProperties(HeapObject* properties) {
                      PropertyArray::cast(properties)->length() == 0,
                  properties == GetReadOnlyRoots().empty_property_array());
   DisallowHeapAllocation no_gc;
-  Isolate* isolate = GetIsolate();
-  int hash = GetIdentityHashHelper(isolate, this);
+  int hash = GetIdentityHashHelper(this);
   Object* new_properties = properties;
 
   // TODO(cbruni): Make GetIdentityHashHelper return a bool so that we
   // don't have to manually compare against kNoHashSentinel.
   if (hash != PropertyArray::kNoHashSentinel) {
-    new_properties = SetHashAndUpdateProperties(isolate, properties, hash);
+    new_properties = SetHashAndUpdateProperties(properties, hash);
   }
 
   set_raw_properties_or_hash(new_properties);
 }
 
-Object* JSReceiver::GetIdentityHash(Isolate* isolate) {
+Object* JSReceiver::GetIdentityHash() {
   DisallowHeapAllocation no_gc;
 
-  int hash = GetIdentityHashHelper(isolate, this);
+  int hash = GetIdentityHashHelper(this);
   if (hash == PropertyArray::kNoHashSentinel) {
-    return ReadOnlyRoots(isolate).undefined_value();
+    return GetReadOnlyRoots().undefined_value();
   }
 
   return Smi::FromInt(hash);
@@ -6824,9 +6819,9 @@ Smi* JSReceiver::CreateIdentityHash(Isolate* isolate, JSReceiver* key) {
 Smi* JSReceiver::GetOrCreateIdentityHash(Isolate* isolate) {
   DisallowHeapAllocation no_gc;
 
-  Object* hash_obj = GetIdentityHash(isolate);
-  if (!hash_obj->IsUndefined(isolate)) {
-    return Smi::cast(hash_obj);
+  int hash = GetIdentityHashHelper(this);
+  if (hash != PropertyArray::kNoHashSentinel) {
+    return Smi::FromInt(hash);
   }
 
   return JSReceiver::CreateIdentityHash(isolate, this);
@@ -17680,9 +17675,12 @@ Handle<Derived> BaseNameDictionary<Derived, Shape>::Add(
   // SetNextEnumerationIndex.
   int index = dictionary->NextEnumerationIndex();
   details = details.set_index(index);
+  dictionary = AddNoUpdateNextEnumerationIndex(isolate, dictionary, key, value,
+                                               details, entry_out);
+  // Update enumeration index here in order to avoid potential modification of
+  // the canonical empty dictionary which lives in read only space.
   dictionary->SetNextEnumerationIndex(index + 1);
-  return AddNoUpdateNextEnumerationIndex(isolate, dictionary, key, value,
-                                         details, entry_out);
+  return dictionary;
 }
 
 template <typename Derived, typename Shape>

src/objects/hash-table-inl.h

@@ -8,7 +8,6 @@
 #include "src/objects/hash-table.h"
 
 #include "src/heap/heap.h"
-#include "src/objects-inl.h"
 #include "src/objects/fixed-array-inl.h"
 #include "src/roots-inl.h"
 

src/objects/js-objects-inl.h

@@ -764,13 +764,12 @@ NumberDictionary* JSObject::element_dictionary() {
 }
 
 void JSReceiver::initialize_properties() {
-  Heap* heap = GetHeap();
-  ReadOnlyRoots roots(heap);
+  ReadOnlyRoots roots = GetReadOnlyRoots();
   DCHECK(!Heap::InNewSpace(roots.empty_fixed_array()));
-  DCHECK(!Heap::InNewSpace(heap->empty_property_dictionary()));
+  DCHECK(!Heap::InNewSpace(roots.empty_property_dictionary()));
   if (map()->is_dictionary_map()) {
     WRITE_FIELD(this, kPropertiesOrHashOffset,
-                heap->empty_property_dictionary());
+                roots.empty_property_dictionary());
   } else {
     WRITE_FIELD(this, kPropertiesOrHashOffset, roots.empty_fixed_array());
   }
@@ -789,7 +788,7 @@ NameDictionary* JSReceiver::property_dictionary() const {
 
   Object* prop = raw_properties_or_hash();
   if (prop->IsSmi()) {
-    return GetHeap()->empty_property_dictionary();
+    return GetReadOnlyRoots().empty_property_dictionary();
   }
 
   return NameDictionary::cast(prop);

src/objects/js-objects.h

@@ -224,7 +224,7 @@ class JSReceiver : public HeapObject, public NeverReadOnlySpaceObject {
 
   // Retrieves a permanent object identity hash code. The undefined value might
   // be returned in case no hash was created yet.
-  Object* GetIdentityHash(Isolate* isolate);
+  Object* GetIdentityHash();
 
   // Retrieves a permanent object identity hash code. May create and store a
   // hash code if needed and none exists.

src/roots.h

@@ -197,6 +197,7 @@ class Symbol;
   V(FixedArray, empty_ordered_hash_set, EmptyOrderedHashSet)                   \
   V(FeedbackMetadata, empty_feedback_metadata, EmptyFeedbackMetadata)          \
   V(PropertyCell, empty_property_cell, EmptyPropertyCell)                      \
+  V(NameDictionary, empty_property_dictionary, EmptyPropertyDictionary)        \
   V(InterceptorInfo, noop_interceptor_info, NoOpInterceptorInfo)               \
   V(WeakFixedArray, empty_weak_fixed_array, EmptyWeakFixedArray)               \
   V(WeakArrayList, empty_weak_array_list, EmptyWeakArrayList)                  \
@@ -256,7 +257,6 @@ class Symbol;
   V(FixedArray, string_split_cache, StringSplitCache)                        \
   V(FixedArray, regexp_multiple_cache, RegExpMultipleCache)                  \
   /* Lists and dictionaries */                                               \
-  V(NameDictionary, empty_property_dictionary, EmptyPropertyDictionary)      \
   V(NameDictionary, public_symbol_table, PublicSymbolTable)                  \
   V(NameDictionary, api_symbol_table, ApiSymbolTable)                        \
   V(NameDictionary, api_private_symbol_table, ApiPrivateSymbolTable)         \

test/cctest/test-dictionary.cc

@@ -86,7 +86,7 @@ static void TestHashMap(Handle<HashMap> table) {
     CHECK_EQ(table->NumberOfElements(), i + 1);
     CHECK_NE(table->FindEntry(isolate, key), HashMap::kNotFound);
     CHECK_EQ(table->Lookup(key), *value);
-    CHECK(key->GetIdentityHash(isolate)->IsSmi());
+    CHECK(key->GetIdentityHash()->IsSmi());
   }
 
   // Keys never added to the map which already have an identity hash
@@ -96,7 +96,7 @@ static void TestHashMap(Handle<HashMap> table) {
     CHECK(key->GetOrCreateIdentityHash(isolate)->IsSmi());
     CHECK_EQ(table->FindEntry(isolate, key), HashMap::kNotFound);
     CHECK_EQ(table->Lookup(key), roots.the_hole_value());
-    CHECK(key->GetIdentityHash(isolate)->IsSmi());
+    CHECK(key->GetIdentityHash()->IsSmi());
   }
 
   // Keys that don't have an identity hash should not be found and also
@@ -104,7 +104,7 @@ static void TestHashMap(Handle<HashMap> table) {
   for (int i = 0; i < 100; i++) {
     Handle<JSReceiver> key = factory->NewJSArray(7);
     CHECK_EQ(table->Lookup(key), roots.the_hole_value());
-    Object* identity_hash = key->GetIdentityHash(isolate);
+    Object* identity_hash = key->GetIdentityHash();
     CHECK_EQ(roots.undefined_value(), identity_hash);
   }
 }
@@ -157,7 +157,7 @@ static void TestHashSet(Handle<HashSet> table) {
     table = HashSet::Add(isolate, table, key);
     CHECK_EQ(table->NumberOfElements(), i + 2);
     CHECK(table->Has(isolate, key));
-    CHECK(key->GetIdentityHash(isolate)->IsSmi());
+    CHECK(key->GetIdentityHash()->IsSmi());
   }
 
   // Keys never added to the map which already have an identity hash
@@ -166,7 +166,7 @@ static void TestHashSet(Handle<HashSet> table) {
     Handle<JSReceiver> key = factory->NewJSArray(7);
     CHECK(key->GetOrCreateIdentityHash(isolate)->IsSmi());
     CHECK(!table->Has(isolate, key));
-    CHECK(key->GetIdentityHash(isolate)->IsSmi());
+    CHECK(key->GetIdentityHash()->IsSmi());
   }
 
   // Keys that don't have an identity hash should not be found and also
@@ -174,7 +174,7 @@ static void TestHashSet(Handle<HashSet> table) {
   for (int i = 0; i < 100; i++) {
     Handle<JSReceiver> key = factory->NewJSArray(7);
     CHECK(!table->Has(isolate, key));
-    Object* identity_hash = key->GetIdentityHash(isolate);
+    Object* identity_hash = key->GetIdentityHash();
     CHECK_EQ(ReadOnlyRoots(CcTest::heap()).undefined_value(), identity_hash);
   }
 }

test/cctest/test-roots.cc

@@ -35,13 +35,16 @@ TEST(TestReadOnlyRoots) {
 namespace {
 bool IsInitiallyMutable(Factory* factory, Address object_address) {
 // Entries in this list are in STRONG_MUTABLE_ROOT_LIST, but may initially point
-// to objects that in RO_SPACE.
+// to objects that are in RO_SPACE.
 #define INITIALLY_READ_ONLY_ROOT_LIST(V)  \
+  V(api_private_symbol_table)             \
+  V(api_symbol_table)                     \
   V(builtins_constants_table)             \
   V(detached_contexts)                    \
   V(feedback_vectors_for_profiling_tools) \
   V(materialized_objects)                 \
   V(noscript_shared_function_infos)       \
+  V(public_symbol_table)                  \
   V(retained_maps)                        \
   V(retaining_path_targets)               \
   V(serialized_global_proxy_sizes)        \

tools/v8heapconst.py

@@ -293,42 +293,42 @@ KNOWN_MAPS = {
   ("RO_SPACE", 0x046f9): (131, "SelfReferenceMarkerMap"),
   ("RO_SPACE", 0x04761): (171, "Tuple2Map"),
   ("RO_SPACE", 0x04801): (173, "ArrayBoilerplateDescriptionMap"),
-  ("RO_SPACE", 0x04af1): (161, "InterceptorInfoMap"),
-  ("RO_SPACE", 0x06ef9): (153, "AccessCheckInfoMap"),
-  ("RO_SPACE", 0x06f49): (154, "AccessorInfoMap"),
-  ("RO_SPACE", 0x06f99): (155, "AccessorPairMap"),
-  ("RO_SPACE", 0x06fe9): (156, "AliasedArgumentsEntryMap"),
-  ("RO_SPACE", 0x07039): (157, "AllocationMementoMap"),
-  ("RO_SPACE", 0x07089): (158, "AsyncGeneratorRequestMap"),
-  ("RO_SPACE", 0x070d9): (159, "DebugInfoMap"),
-  ("RO_SPACE", 0x07129): (160, "FunctionTemplateInfoMap"),
-  ("RO_SPACE", 0x07179): (162, "InterpreterDataMap"),
-  ("RO_SPACE", 0x071c9): (163, "ModuleInfoEntryMap"),
-  ("RO_SPACE", 0x07219): (164, "ModuleMap"),
-  ("RO_SPACE", 0x07269): (165, "ObjectTemplateInfoMap"),
-  ("RO_SPACE", 0x072b9): (166, "PromiseCapabilityMap"),
-  ("RO_SPACE", 0x07309): (167, "PromiseReactionMap"),
-  ("RO_SPACE", 0x07359): (168, "PrototypeInfoMap"),
-  ("RO_SPACE", 0x073a9): (169, "ScriptMap"),
-  ("RO_SPACE", 0x073f9): (170, "StackFrameInfoMap"),
-  ("RO_SPACE", 0x07449): (172, "Tuple3Map"),
-  ("RO_SPACE", 0x07499): (174, "WasmDebugInfoMap"),
-  ("RO_SPACE", 0x074e9): (175, "WasmExportedFunctionDataMap"),
-  ("RO_SPACE", 0x07539): (176, "CallableTaskMap"),
-  ("RO_SPACE", 0x07589): (177, "CallbackTaskMap"),
-  ("RO_SPACE", 0x075d9): (178, "PromiseFulfillReactionJobTaskMap"),
-  ("RO_SPACE", 0x07629): (179, "PromiseRejectReactionJobTaskMap"),
-  ("RO_SPACE", 0x07679): (180, "PromiseResolveThenableJobTaskMap"),
-  ("RO_SPACE", 0x076c9): (181, "MicrotaskQueueMap"),
-  ("RO_SPACE", 0x07719): (182, "AllocationSiteWithWeakNextMap"),
-  ("RO_SPACE", 0x07769): (182, "AllocationSiteWithoutWeakNextMap"),
-  ("RO_SPACE", 0x077b9): (214, "LoadHandler1Map"),
-  ("RO_SPACE", 0x07809): (214, "LoadHandler2Map"),
-  ("RO_SPACE", 0x07859): (214, "LoadHandler3Map"),
-  ("RO_SPACE", 0x078a9): (221, "StoreHandler0Map"),
-  ("RO_SPACE", 0x078f9): (221, "StoreHandler1Map"),
-  ("RO_SPACE", 0x07949): (221, "StoreHandler2Map"),
-  ("RO_SPACE", 0x07999): (221, "StoreHandler3Map"),
+  ("RO_SPACE", 0x04b41): (161, "InterceptorInfoMap"),
+  ("RO_SPACE", 0x06f49): (153, "AccessCheckInfoMap"),
+  ("RO_SPACE", 0x06f99): (154, "AccessorInfoMap"),
+  ("RO_SPACE", 0x06fe9): (155, "AccessorPairMap"),
+  ("RO_SPACE", 0x07039): (156, "AliasedArgumentsEntryMap"),
+  ("RO_SPACE", 0x07089): (157, "AllocationMementoMap"),
+  ("RO_SPACE", 0x070d9): (158, "AsyncGeneratorRequestMap"),
+  ("RO_SPACE", 0x07129): (159, "DebugInfoMap"),
+  ("RO_SPACE", 0x07179): (160, "FunctionTemplateInfoMap"),
+  ("RO_SPACE", 0x071c9): (162, "InterpreterDataMap"),
+  ("RO_SPACE", 0x07219): (163, "ModuleInfoEntryMap"),
+  ("RO_SPACE", 0x07269): (164, "ModuleMap"),
+  ("RO_SPACE", 0x072b9): (165, "ObjectTemplateInfoMap"),
+  ("RO_SPACE", 0x07309): (166, "PromiseCapabilityMap"),
+  ("RO_SPACE", 0x07359): (167, "PromiseReactionMap"),
+  ("RO_SPACE", 0x073a9): (168, "PrototypeInfoMap"),
+  ("RO_SPACE", 0x073f9): (169, "ScriptMap"),
+  ("RO_SPACE", 0x07449): (170, "StackFrameInfoMap"),
+  ("RO_SPACE", 0x07499): (172, "Tuple3Map"),
+  ("RO_SPACE", 0x074e9): (174, "WasmDebugInfoMap"),
+  ("RO_SPACE", 0x07539): (175, "WasmExportedFunctionDataMap"),
+  ("RO_SPACE", 0x07589): (176, "CallableTaskMap"),
+  ("RO_SPACE", 0x075d9): (177, "CallbackTaskMap"),
+  ("RO_SPACE", 0x07629): (178, "PromiseFulfillReactionJobTaskMap"),
+  ("RO_SPACE", 0x07679): (179, "PromiseRejectReactionJobTaskMap"),
+  ("RO_SPACE", 0x076c9): (180, "PromiseResolveThenableJobTaskMap"),
+  ("RO_SPACE", 0x07719): (181, "MicrotaskQueueMap"),
+  ("RO_SPACE", 0x07769): (182, "AllocationSiteWithWeakNextMap"),
+  ("RO_SPACE", 0x077b9): (182, "AllocationSiteWithoutWeakNextMap"),
+  ("RO_SPACE", 0x07809): (214, "LoadHandler1Map"),
+  ("RO_SPACE", 0x07859): (214, "LoadHandler2Map"),
+  ("RO_SPACE", 0x078a9): (214, "LoadHandler3Map"),
+  ("RO_SPACE", 0x078f9): (221, "StoreHandler0Map"),
+  ("RO_SPACE", 0x07949): (221, "StoreHandler1Map"),
+  ("RO_SPACE", 0x07999): (221, "StoreHandler2Map"),
+  ("RO_SPACE", 0x079e9): (221, "StoreHandler3Map"),
   ("MAP_SPACE", 0x02201): (1057, "ExternalMap"),
   ("MAP_SPACE", 0x02251): (1072, "JSMessageObjectMap"),
 }
@@ -375,12 +375,13 @@ KNOWN_OBJECTS = {
   ("RO_SPACE", 0x04a41): "EmptyOrderedHashSet",
   ("RO_SPACE", 0x04a69): "EmptyFeedbackMetadata",
   ("RO_SPACE", 0x04a79): "EmptyPropertyCell",
-  ("RO_SPACE", 0x04aa1): "NoOpInterceptorInfo",
-  ("RO_SPACE", 0x04b41): "EmptyWeakArrayList",
-  ("RO_SPACE", 0x04b59): "InfinityValue",
-  ("RO_SPACE", 0x04b69): "MinusZeroValue",
-  ("RO_SPACE", 0x04b79): "MinusInfinityValue",
-  ("RO_SPACE", 0x04b89): "SelfReferenceMarker",
+  ("RO_SPACE", 0x04aa1): "EmptyPropertyDictionary",
+  ("RO_SPACE", 0x04af1): "NoOpInterceptorInfo",
+  ("RO_SPACE", 0x04b91): "EmptyWeakArrayList",
+  ("RO_SPACE", 0x04ba9): "InfinityValue",
+  ("RO_SPACE", 0x04bb9): "MinusZeroValue",
+  ("RO_SPACE", 0x04bc9): "MinusInfinityValue",
+  ("RO_SPACE", 0x04bd9): "SelfReferenceMarker",
   ("OLD_SPACE", 0x02211): "EmptyScript",
   ("OLD_SPACE", 0x02291): "ManyClosuresCell",
   ("OLD_SPACE", 0x022b1): "NoElementsProtector",