[explicit isolates] Make read-only root Heap accessors private

Now that ReadOnlyRoots is used everywhere to access the read-only roots,
this makes the Heap accessors for such roots private.

It also adds tests that the roots reachable from ReadOnlyRoots are all
in RO_SPACE as well as tests that the roots still publicly accessible
from Heap are not in RO_SPACE. There's a white list in the file for
the few roots where the root pointer itself can change. (For instance
materialized_objects points to empty_fixed_array to start with before
before later pointing to a mutable array).

Also fixes up new use of heap->empty_fixed_array() in elements.cc added
since I cleaned it up.

Bug: v8:7786
Change-Id: I9ac7985c9f85910b5b22d2f9f559dfd04d43ed44
Reviewed-on: https://chromium-review.googlesource.com/1126252Reviewed-by: Hannes Payer <hpayer@chromium.org>
Commit-Queue: Dan Elphick <delphick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54240}

src/elements.cc

@@ -2280,7 +2280,7 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
         heap->CanMoveObjectStart(*dst_elms)) {
       // Remove all the pointers to the FixedArrayBase we're going to left trim
       // from the heap.
-      receiver->set_elements(heap->empty_fixed_array());
+      receiver->set_elements(ReadOnlyRoots(heap).empty_fixed_array());
       // Update all the copies of this backing_store handle.
       *dst_elms.location() =
           BackingStore::cast(heap->LeftTrimFixedArray(*dst_elms, src_index));

src/heap/heap.h

@@ -793,11 +793,27 @@ class Heap {
   // ===========================================================================
 
   // Heap root getters.
+ private:
+  friend class ReadOnlyRoots;
+// RO_SPACE objects should be accessed via ReadOnlyRoots.
 #define ROOT_ACCESSOR(type, name, camel_name) inline type* name();
-  ROOT_LIST(ROOT_ACCESSOR)
+  STRONG_READ_ONLY_ROOT_LIST(ROOT_ACCESSOR)
 #undef ROOT_ACCESSOR
 
-  // Utility type maps.
+#define STRING_ACCESSOR(name, str) inline String* name();
+  INTERNALIZED_STRING_LIST(STRING_ACCESSOR)
+#undef STRING_ACCESSOR
+
+#define SYMBOL_ACCESSOR(name) inline Symbol* name();
+  PRIVATE_SYMBOL_LIST(SYMBOL_ACCESSOR)
+#undef SYMBOL_ACCESSOR
+
+#define SYMBOL_ACCESSOR(name, description) inline Symbol* name();
+  PUBLIC_SYMBOL_LIST(SYMBOL_ACCESSOR)
+  WELL_KNOWN_SYMBOL_LIST(SYMBOL_ACCESSOR)
+#undef SYMBOL_ACCESSOR
+
+// Utility type maps.
 #define STRUCT_MAP_ACCESSOR(NAME, Name, name) inline Map* name##_map();
   STRUCT_LIST(STRUCT_MAP_ACCESSOR)
 #undef STRUCT_MAP_ACCESSOR
@@ -807,24 +823,16 @@ class Heap {
   ALLOCATION_SITE_LIST(ALLOCATION_SITE_MAP_ACCESSOR)
 #undef ALLOCATION_SITE_MAP_ACCESSOR
 
+ public:
+#define ROOT_ACCESSOR(type, name, camel_name) inline type* name();
+  MUTABLE_ROOT_LIST(ROOT_ACCESSOR)
+#undef ROOT_ACCESSOR
+
 #define DATA_HANDLER_MAP_ACCESSOR(NAME, Name, Size, name) \
   inline Map* name##_map();
   DATA_HANDLER_LIST(DATA_HANDLER_MAP_ACCESSOR)
 #undef DATA_HANDLER_MAP_ACCESSOR
 
-#define STRING_ACCESSOR(name, str) inline String* name();
-  INTERNALIZED_STRING_LIST(STRING_ACCESSOR)
-#undef STRING_ACCESSOR
-
-#define SYMBOL_ACCESSOR(name) inline Symbol* name();
-  PRIVATE_SYMBOL_LIST(SYMBOL_ACCESSOR)
-#undef SYMBOL_ACCESSOR
-
-#define SYMBOL_ACCESSOR(name, description) inline Symbol* name();
-  PUBLIC_SYMBOL_LIST(SYMBOL_ACCESSOR)
-  WELL_KNOWN_SYMBOL_LIST(SYMBOL_ACCESSOR)
-#undef SYMBOL_ACCESSOR
-
 #define ACCESSOR_INFO_ACCESSOR(accessor_name, AccessorName) \
   inline AccessorInfo* accessor_name##_accessor();
   ACCESSOR_INFO_LIST(ACCESSOR_INFO_ACCESSOR)

test/cctest/BUILD.gn

@@ -209,6 +209,7 @@ v8_source_set("cctest_sources") {
     "test-random-number-generator.cc",
     "test-regexp.cc",
     "test-representation.cc",
+    "test-roots.cc",
     "test-sampler-api.cc",
     "test-serialize.cc",
     "test-strings.cc",

test/cctest/test-roots.cc

+// 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.
+
+#include "src/heap/heap.h"
+#include "src/roots-inl.h"
+#include "test/cctest/cctest.h"
+
+namespace v8 {
+namespace internal {
+
+namespace {
+AllocationSpace GetSpaceFromObject(Object* object) {
+  DCHECK(object->IsHeapObject());
+  return MemoryChunk::FromHeapObject(HeapObject::cast(object))
+      ->owner()
+      ->identity();
+}
+}  // namespace
+
+#define CHECK_IN_RO_SPACE(name)    \
+  HeapObject* name = roots.name(); \
+  CHECK_EQ(RO_SPACE, GetSpaceFromObject(name));
+
+// The following tests check that all the roots accessible via ReadOnlyRoots are
+// in RO_SPACE.
+TEST(TestStrongReadOnlyRoots) {
+  ReadOnlyRoots roots(CcTest::i_isolate());
+
+#define TEST_ROOT(type, name, camel_name) CHECK_IN_RO_SPACE(name)
+  STRONG_READ_ONLY_ROOT_LIST(TEST_ROOT)
+#undef TEST_ROOT
+}
+
+TEST(TestInternalizedStrings) {
+  ReadOnlyRoots roots(CcTest::i_isolate());
+
+#define TEST_ROOT(name, str) CHECK_IN_RO_SPACE(name)
+  INTERNALIZED_STRING_LIST(TEST_ROOT)
+#undef TEST_ROOT
+}
+
+TEST(TestPrivateSymbols) {
+  ReadOnlyRoots roots(CcTest::i_isolate());
+
+  PRIVATE_SYMBOL_LIST(CHECK_IN_RO_SPACE)
+}
+
+TEST(TestPublicSymbols) {
+  ReadOnlyRoots roots(CcTest::i_isolate());
+
+#define TEST_ROOT(name, description) CHECK_IN_RO_SPACE(name)
+  PUBLIC_SYMBOL_LIST(TEST_ROOT)
+  WELL_KNOWN_SYMBOL_LIST(TEST_ROOT)
+#undef TEST_ROOT
+}
+
+TEST(TestStructMaps) {
+  ReadOnlyRoots roots(CcTest::i_isolate());
+
+#define TEST_ROOT(NAME, Name, name) CHECK_IN_RO_SPACE(name##_map)
+  STRUCT_LIST(TEST_ROOT)
+#undef TEST_ROOT
+}
+
+TEST(TestAllocationSiteMaps) {
+  ReadOnlyRoots roots(CcTest::i_isolate());
+
+#define TEST_ROOT(NAME, Name, Size, name) CHECK_IN_RO_SPACE(name##_map)
+  ALLOCATION_SITE_LIST(TEST_ROOT)
+#undef TEST_ROOT
+}
+
+#undef CHECK_IN_RO_SPACE
+
+namespace {
+bool IsInitiallyMutable(Heap* heap, Object* object) {
+// Entries in this list are in STRONG_MUTABLE_ROOT_LIST, but may initially point
+// to objects that in RO_SPACE.
+#define INITIALLY_READ_ONLY_ROOT_LIST(V) \
+  V(materialized_objects)                \
+  V(retaining_path_targets)              \
+  V(retained_maps)
+
+#define TEST_CAN_BE_READ_ONLY(name) \
+  if (heap->name() == object) return false;
+  INITIALLY_READ_ONLY_ROOT_LIST(TEST_CAN_BE_READ_ONLY)
+#undef TEST_CAN_BE_READ_ONLY
+#undef INITIALLY_READ_ONLY_ROOT_LIST
+  return true;
+}
+}  // namespace
+
+#define CHECK_NOT_IN_RO_SPACE(name)                           \
+  Object* name = heap->name();                                \
+  if (name->IsHeapObject() && IsInitiallyMutable(heap, name)) \
+    CHECK_NE(RO_SPACE, GetSpaceFromObject(name));
+
+// The following tests check that all the roots accessible via public Heap
+// accessors are not in RO_SPACE with the exception of the objects listed in
+// INITIALLY_READ_ONLY_ROOT_LIST.
+TEST(TestHeapRootsNotReadOnly) {
+  Heap* heap = CcTest::i_isolate()->heap();
+
+#define TEST_ROOT(type, name, camel_name) CHECK_NOT_IN_RO_SPACE(name)
+  MUTABLE_ROOT_LIST(TEST_ROOT)
+#undef TEST_ROOT
+}
+
+TEST(TestAccessorInfosNotReadOnly) {
+  Heap* heap = CcTest::i_isolate()->heap();
+
+#define TEST_ROOT(name, AccessorName) CHECK_NOT_IN_RO_SPACE(name##_accessor)
+  ACCESSOR_INFO_LIST(TEST_ROOT)
+#undef TEST_ROOT
+}
+
+#undef CHECK_NOT_IN_RO_SPACE
+
+}  // namespace internal
+}  // namespace v8