[heap] Fix PagedSpace::SizeOfObjects calling from within FreeList::Allocate

src/heap/spaces.cc

Index: src/heap/spaces.cc
diff --git a/src/heap/spaces.cc b/src/heap/spaces.cc
index 9bba6dd89b9cfe6e3e3ba0933d6d89b87dfc78ca..8511b7a24b87d2347043e7d04e0ad09cf8dd2023 100644
--- a/src/heap/spaces.cc
+++ b/src/heap/spaces.cc
@@ -2386,16 +2386,14 @@ HeapObject* FreeList::Allocate(int size_in_bytes) {
   // skipped when scanning the heap.  This also puts it back in the free list
   // if it is big enough.
   owner_->Free(owner_->top(), old_linear_size);
+  owner_->SetTopAndLimit(nullptr, nullptr);
 
   owner_->heap()->incremental_marking()->OldSpaceStep(size_in_bytes -

[Michael Lippautz, 2015/09/24 16:39:53]

SizeOfObjects() > 0 is a condition to start incremental marking here, which
means that we sometimes didn't start it where we could have.

[Hannes Payer (out of office), 2015/09/25 08:33:09]

Good catch!

                                                       old_linear_size);
 
   int new_node_size = 0;
   FreeSpace* new_node = FindNodeFor(size_in_bytes, &new_node_size);
-  if (new_node == NULL) {
-    owner_->SetTopAndLimit(NULL, NULL);
-    return NULL;
-  }
+  if (new_node == nullptr) return nullptr;
 
   int bytes_left = new_node_size - size_in_bytes;
   DCHECK(bytes_left >= 0);
@@ -2439,10 +2437,6 @@ HeapObject* FreeList::Allocate(int size_in_bytes) {
     // linear allocation area.
     owner_->SetTopAndLimit(new_node->address() + size_in_bytes,
                            new_node->address() + new_node_size);
-  } else {
-    // TODO(gc) Try not freeing linear allocation region when bytes_left
-    // are zero.
-    owner_->SetTopAndLimit(NULL, NULL);
   }
 
   return new_node;
@@ -2553,7 +2547,9 @@ intptr_t PagedSpace::SizeOfObjects() {
   DCHECK(!FLAG_concurrent_sweeping ||
          heap()->mark_compact_collector()->sweeping_in_progress() ||
          (unswept_free_bytes_ == 0));
-  return Size() - unswept_free_bytes_ - (limit() - top());
+  intptr_t size = Size() - unswept_free_bytes_ - (limit() - top());
+  CHECK_GE(size, 0);

[Michael Lippautz, 2015/09/24 16:39:54]

If possible, I'd like to keep this here to catch potential bugs.

[Hannes Payer (out of office), 2015/09/25 08:33:08]

I guess a DCHECK would be sufficient. If you keep the CHECK, you should watch
the next canaries that ship with it. As you like.

[Michael Lippautz, 2015/09/25 09:05:17]

Let's make it a DCHECK for now as I am anyways testing a lot in this area.

+  return size;
 }