[explicit isolates] Remove some GetHeaps from objects.cc

Remove GetHeaps from String::MakeExternal by first checking the String
is not in RO_SPACE (by checking the MemoryChunk's identity) and only
then getting the heap from the chunk. Also changes
String::CanMakeExternal in api.cc to not use GetHeap.

Also remove check GetHeap()->Contains(this/map) that attempts to print
INVALID POINTER/INVALID MAP as it's much more likely that the code will
crash in either GetHeap or Contains before printing anything.

Bug: v8:7786
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: Id27ba84eb2ba78c71158d6f2e94a3b4176c34609
Reviewed-on: https://chromium-review.googlesource.com/1131128
Commit-Queue: Dan Elphick <delphick@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54361}

src/api.cc

@@ -6827,10 +6827,10 @@ bool v8::String::CanMakeExternal() {
   i::Handle<i::String> obj = Utils::OpenHandle(this);
   if (obj->IsExternalString()) return false;
 
-  // Old space strings should be externalized.
-  i::Heap* heap = obj->GetIsolate()->heap();
-  return !heap->new_space()->Contains(*obj) &&
-         !heap->read_only_space()->Contains(*obj);
+  // Only old space strings should be externalized.
+  i::MemoryChunk* chunk = i::MemoryChunk::FromHeapObject(*obj);
+  i::AllocationSpace space = chunk->owner()->identity();
+  return space != i::NEW_SPACE && space != i::RO_SPACE;
 }
 
 

src/objects.cc

@@ -2584,8 +2584,11 @@ bool String::MakeExternal(v8::String::ExternalStringResource* resource) {
   int size = this->Size();  // Byte size of the original string.
   // Abort if size does not allow in-place conversion.
   if (size < ExternalString::kShortSize) return false;
-  Heap* heap = GetHeap();
-  if (heap->read_only_space()->Contains(this)) return false;
+  MemoryChunk* chunk = MemoryChunk::FromHeapObject(this);
+  // Read-only strings cannot be made external, since that would mutate the
+  // string.
+  if (chunk->owner()->identity() == RO_SPACE) return false;
+  Heap* heap = chunk->heap();
   bool is_one_byte = this->IsOneByteRepresentation();
   bool is_internalized = this->IsInternalizedString();
   bool has_pointers = StringShape(this).IsIndirect();
@@ -2666,8 +2669,11 @@ bool String::MakeExternal(v8::String::ExternalOneByteStringResource* resource) {
   int size = this->Size();  // Byte size of the original string.
   // Abort if size does not allow in-place conversion.
   if (size < ExternalString::kShortSize) return false;
-  Heap* heap = GetHeap();
-  if (heap->read_only_space()->Contains(this)) return false;
+  MemoryChunk* chunk = MemoryChunk::FromHeapObject(this);
+  // Read-only strings cannot be made external, since that would mutate the
+  // string.
+  if (chunk->owner()->identity() == RO_SPACE) return false;
+  Heap* heap = chunk->heap();
   bool is_internalized = this->IsInternalizedString();
   bool has_pointers = StringShape(this).IsIndirect();
 
@@ -3286,17 +3292,6 @@ bool JSObject::IsUnmodifiedApiObject(Object** o) {
 }
 
 void HeapObject::HeapObjectShortPrint(std::ostream& os) {  // NOLINT
-  Heap* heap = GetHeap();
-  ReadOnlyRoots roots(heap);
-  if (!heap->Contains(this)) {
-    os << "!!!INVALID POINTER!!!";
-    return;
-  }
-  if (!heap->Contains(map())) {
-    os << "!!!INVALID MAP!!!";
-    return;
-  }
-
   os << AsHex(reinterpret_cast<Address>(this), kPointerHexDigits, true) << " ";
 
   if (IsString()) {
@@ -3409,17 +3404,20 @@ void HeapObject::HeapObjectShortPrint(std::ostream& os) {  // NOLINT
       os << "<PropertyArray[" << PropertyArray::cast(this)->length() << "]>";
       break;
     case FEEDBACK_CELL_TYPE: {
-      os << "<FeedbackCell[";
-      if (map() == roots.no_closures_cell_map()) {
-        os << "no closures";
-      } else if (map() == roots.one_closure_cell_map()) {
-        os << "one closure";
-      } else if (map() == roots.many_closures_cell_map()) {
-        os << "many closures";
-      } else {
-        os << "!!!INVALID MAP!!!";
+      {
+        ReadOnlyRoots roots = GetReadOnlyRoots();
+        os << "<FeedbackCell[";
+        if (map() == roots.no_closures_cell_map()) {
+          os << "no closures";
+        } else if (map() == roots.one_closure_cell_map()) {
+          os << "one closure";
+        } else if (map() == roots.many_closures_cell_map()) {
+          os << "many closures";
+        } else {
+          os << "!!!INVALID MAP!!!";
+        }
+        os << "]>";
       }
-      os << "]>";
       break;
     }
     case FEEDBACK_VECTOR_TYPE: