[heap, runtime] Fix conversion of large strings to thin strings.

This patch removes creation of fillers in the middle of a large page and
fixes assert in Heap::NotifyObjectLayoutChange.

The fillers in large pages are useless since we do not sweep large
object space.

Bug: chromium:752426
Change-Id: I01c230223f28d6d54b7362ee70e9d83de50678fd
Reviewed-on: https://chromium-review.googlesource.com/601994
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47221}

src/heap/heap.cc

@@ -3307,6 +3307,11 @@ AllocationResult Heap::AllocateBytecodeArray(int length,
 
 HeapObject* Heap::CreateFillerObjectAt(Address addr, int size,
                                        ClearRecordedSlots mode) {
+  // In large object space fillers can be created only at the beginning
+  // of the large page.
+  DCHECK_IMPLIES(
+      lo_space_->ContainsSlow(addr),
+      HeapObject::cast(lo_space_->FindObject(addr))->address() == addr);
   if (size == 0) return nullptr;
   HeapObject* filler = HeapObject::FromAddress(addr);
   if (size == kPointerSize) {
@@ -3513,6 +3518,15 @@ void Heap::RightTrimFixedArray(FixedArrayBase* object, int elements_to_trim) {
   }
 }
 
+void Heap::RightTrimString(String* string, int old_size, int new_size) {
+  if (old_size == new_size) return;
+  DCHECK_LE(new_size, old_size);
+  if (!lo_space_->Contains(string)) {
+    Address filler = string->address() + new_size;
+    CreateFillerObjectAt(filler, old_size - new_size, ClearRecordedSlots::kNo);
+  }
+  AdjustLiveBytes(string, -(old_size - new_size));
+}
 
 AllocationResult Heap::AllocateFixedTypedArrayWithExternalPointer(
     int length, ExternalArrayType array_type, void* external_pointer,
@@ -4611,7 +4625,8 @@ void Heap::RegisterDeserializedObjectsForBlackAllocation(
 
 void Heap::NotifyObjectLayoutChange(HeapObject* object, int size,
                                     const DisallowHeapAllocation&) {
-  DCHECK(InOldSpace(object) || InNewSpace(object));
+  DCHECK(InOldSpace(object) || InNewSpace(object) ||
+         (lo_space()->Contains(object) && object->IsString()));
   if (FLAG_incremental_marking && incremental_marking()->IsMarking()) {
     incremental_marking()->MarkBlackAndPush(object);
     if (InOldSpace(object) && incremental_marking()->IsCompacting()) {

src/heap/heap.h

@@ -721,6 +721,8 @@ class Heap {
   // Trim the given array from the right.
   void RightTrimFixedArray(FixedArrayBase* obj, int elements_to_trim);
 
+  void RightTrimString(String* string, int old_size, int new_size);
+
   // Converts the given boolean condition to JavaScript boolean value.
   inline Oddball* ToBoolean(bool condition);
 

src/objects.cc

@@ -2625,8 +2625,7 @@ bool String::MakeExternal(v8::String::ExternalStringResource* resource) {
 
   // Byte size of the external String object.
   int new_size = this->SizeFromMap(new_map);
-  heap->CreateFillerObjectAt(this->address() + new_size, size - new_size,
-                             ClearRecordedSlots::kNo);
+  heap->RightTrimString(this, size, new_size);
   if (has_pointers) {
     heap->ClearRecordedSlotRange(this->address(), this->address() + new_size);
   }
@@ -2638,8 +2637,6 @@ bool String::MakeExternal(v8::String::ExternalStringResource* resource) {
   ExternalTwoByteString* self = ExternalTwoByteString::cast(this);
   self->set_resource(resource);
   if (is_internalized) self->Hash();  // Force regeneration of the hash value.
-
-  heap->AdjustLiveBytes(this, new_size - size);
   return true;
 }
 
@@ -2696,8 +2693,7 @@ bool String::MakeExternal(v8::String::ExternalOneByteStringResource* resource) {
 
   // Byte size of the external String object.
   int new_size = this->SizeFromMap(new_map);
-  heap->CreateFillerObjectAt(this->address() + new_size, size - new_size,
-                             ClearRecordedSlots::kNo);
+  heap->RightTrimString(this, size, new_size);
   if (has_pointers) {
     heap->ClearRecordedSlotRange(this->address(), this->address() + new_size);
   }
@@ -2709,8 +2705,6 @@ bool String::MakeExternal(v8::String::ExternalOneByteStringResource* resource) {
   ExternalOneByteString* self = ExternalOneByteString::cast(this);
   self->set_resource(resource);
   if (is_internalized) self->Hash();  // Force regeneration of the hash value.
-
-  heap->AdjustLiveBytes(this, new_size - size);
   return true;
 }
 
@@ -17087,13 +17081,7 @@ void MakeStringThin(String* string, String* internalized, Isolate* isolate) {
     string->synchronized_set_map(*map);
     ThinString* thin = ThinString::cast(string);
     thin->set_actual(internalized);
-    Address thin_end = thin->address() + ThinString::kSize;
-    int size_delta = old_size - ThinString::kSize;
-    if (size_delta != 0) {
-      Heap* heap = isolate->heap();
-      heap->CreateFillerObjectAt(thin_end, size_delta, ClearRecordedSlots::kNo);
-      heap->AdjustLiveBytes(thin, -size_delta);
-    }
+    isolate->heap()->RightTrimString(thin, old_size, ThinString::kSize);
   }
 }
 

test/cctest/test-strings.cc

@@ -1220,6 +1220,33 @@ TEST(InternalizeExternal) {
   CcTest::CollectGarbage(i::OLD_SPACE);
 }
 
+TEST(LargeThinString) {
+  CcTest::InitializeVM();
+  i::Isolate* isolate = CcTest::i_isolate();
+  Heap* heap = isolate->heap();
+  v8::HandleScope handle_scope(CcTest::isolate());
+  const char* string_generator =
+      "var result = 'a';"
+      "for (var i = 0; i < 20; i++) { result += result;}"
+      "result;";
+  v8::Local<v8::String> v8_string1 =
+      CompileRun(string_generator)
+          ->ToString(CcTest::isolate()->GetCurrentContext())
+          .ToLocalChecked();
+  Handle<String> string1 = v8::Utils::OpenHandle(*v8_string1);
+  isolate->factory()->InternalizeName(string1);
+  v8::Local<v8::String> v8_string2 =
+      CompileRun(string_generator)
+          ->ToString(CcTest::isolate()->GetCurrentContext())
+          .ToLocalChecked();
+  Handle<String> string2 = v8::Utils::OpenHandle(*v8_string2);
+  string2 = String::Flatten(string2);
+  isolate->factory()->InternalizeName(string2);
+  CHECK(heap->lo_space()->Contains(*string2));
+  CHECK(string2->IsThinString());
+  CcTest::CollectGarbage(i::OLD_SPACE);
+}
+
 TEST(SliceFromExternal) {
   FLAG_string_slices = true;
   CcTest::InitializeVM();