Reviewers: Hannes Payer,

Description:
Add filler at the new space top when forcing scavenge.

We only seem to force scavenge in our cctest test suite, so this is
expected to fix some flakiness in our tests, but it will not
improve stability of v8 itself.

[email protected]
BUG=

Please review this at https://codereview.chromium.org/167423004/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+46, -15 lines):
  M src/heap.h
  M src/heap.cc
  M test/cctest/test-mementos.cc


Index: src/heap.cc
diff --git a/src/heap.cc b/src/heap.cc
index c7f363821fcb25ea226bbaa8f6456078ed8551a5..5f122ab9e0b93b4ddf212eeb44cff624cdd2b774 100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -773,6 +773,21 @@ void Heap::CollectAllAvailableGarbage(const char* gc_reason) {
 }


+void Heap::EnsureFillerObjectAtTop() {
+  // There may be an allocation memento behind every object in new space.
+  // If we evacuate a not full new space or if we are on the last page of
+  // the new space, then there may be uninitialized memory behind the top
+  // pointer of the new space page. We store a filler object there to
+  // identify the unused space.
+  Address from_top = new_space_.top();
+  Address from_limit = new_space_.limit();
+  if (from_top < from_limit) {
+    int remaining_in_page = static_cast<int>(from_limit - from_top);
+    CreateFillerObjectAt(from_top, remaining_in_page);
+  }
+}
+
+
 bool Heap::CollectGarbage(GarbageCollector collector,
                           const char* gc_reason,
                           const char* collector_reason,
@@ -789,17 +804,7 @@ bool Heap::CollectGarbage(GarbageCollector collector,
   allocation_timeout_ = Max(6, FLAG_gc_interval);
 #endif

-  // There may be an allocation memento behind every object in new space.
-  // If we evacuate a not full new space or if we are on the last page of
-  // the new space, then there may be uninitialized memory behind the top
-  // pointer of the new space page. We store a filler object there to
-  // identify the unused space.
-  Address from_top = new_space_.top();
-  Address from_limit = new_space_.limit();
-  if (from_top < from_limit) {
-    int remaining_in_page = static_cast<int>(from_limit - from_top);
-    CreateFillerObjectAt(from_top, remaining_in_page);
-  }
+  EnsureFillerObjectAtTop();

   if (collector == SCAVENGER && !incremental_marking()->IsStopped()) {
     if (FLAG_trace_incremental_marking) {
@@ -875,6 +880,7 @@ int Heap::NotifyContextDisposed() {

 void Heap::PerformScavenge() {
   GCTracer tracer(this, NULL, NULL);
+  EnsureFillerObjectAtTop();
   if (incremental_marking()->IsStopped()) {
     PerformGarbageCollection(SCAVENGER, &tracer);
   } else {
Index: src/heap.h
diff --git a/src/heap.h b/src/heap.h
index 2645d27f45792888b21f90932c475a5d88ce8c1e..544fe5e2e04f13c34105234d9d4b9033f751f83a 100644
--- a/src/heap.h
+++ b/src/heap.h
@@ -2135,6 +2135,11 @@ class Heap {
   GarbageCollector SelectGarbageCollector(AllocationSpace space,
                                           const char** reason);

+  // Make sure there is a filler value behind the top of the new space
+  // so that the GC does not confuse some unintialized/stale memory
+  // with the allocation memento of the object at the top
+  void EnsureFillerObjectAtTop();
+
   // Performs garbage collection operation.
   // Returns whether there is a chance that another major GC could
   // collect more garbage.
Index: test/cctest/test-mementos.cc
diff --git a/test/cctest/test-mementos.cc b/test/cctest/test-mementos.cc
index f59eef9485f8257ef158117dc5b6e69a2f82da09..5cc116072116c16ca6463747d69e9ae4f7623bbd 100644
--- a/test/cctest/test-mementos.cc
+++ b/test/cctest/test-mementos.cc
@@ -29,11 +29,8 @@

 using namespace v8::internal;

-TEST(Regress340063) {
-  CcTest::InitializeVM();
-  if (!i::FLAG_allocation_site_pretenuring) return;
-  v8::HandleScope scope(CcTest::isolate());

+static void SetUpNewSpaceWithPoisonedMementoAtTop() {
   Isolate* isolate = CcTest::i_isolate();
   Heap* heap = isolate->heap();
   NewSpace* new_space = heap->new_space();
@@ -52,8 +49,31 @@ TEST(Regress340063) {
   memento->set_map_no_write_barrier(heap->allocation_memento_map());
   memento->set_allocation_site(
reinterpret_cast<AllocationSite*>(kHeapObjectTag), SKIP_WRITE_BARRIER);
+}
+
+
+TEST(Regress340063) {
+  CcTest::InitializeVM();
+  if (!i::FLAG_allocation_site_pretenuring) return;
+  v8::HandleScope scope(CcTest::isolate());
+
+
+  SetUpNewSpaceWithPoisonedMementoAtTop();

   // Call GC to see if we can handle a poisonous memento right after the
   // current new space top pointer.
+  Heap* heap = CcTest::i_isolate()->heap();
   heap->CollectAllGarbage(Heap::kAbortIncrementalMarkingMask);
 }
+
+
+TEST(BadMementoAfterTopForceScavenge) {
+  CcTest::InitializeVM();
+  if (!i::FLAG_allocation_site_pretenuring) return;
+  v8::HandleScope scope(CcTest::isolate());
+
+  SetUpNewSpaceWithPoisonedMementoAtTop();
+
+  // Force GC to test the poisoned memento handling
+  CcTest::i_isolate()->heap()->PerformScavenge();
+}


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to