Debug garbage collector does not correctly remove cross-gen garbage

runtime/vm/gc_marker.cc

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/gc_marker.h"
 
 #include "vm/allocation.h"
 #include "vm/dart_api_state.h"
 #include "vm/isolate.h"
 #include "vm/log.h"
@@ skipping 173 matching lines @@
       uword next_weak = cur_weak->ptr()->next_;
       RawObject* raw_key = cur_weak->ptr()->key_;
       // Reset the next pointer in the weak property.
       cur_weak->ptr()->next_ = 0;
       if (raw_key->IsMarked()) {
         RawObject* raw_val = cur_weak->ptr()->value_;
         marked = marked || (raw_val->IsHeapObject() && !raw_val->IsMarked());
 
         // The key is marked so we make sure to properly visit all pointers
         // originating from this weak property.
-        VisitingOldObject(cur_weak);
+        if (cur_weak->IsOldObject()) {
+          VisitingOldObject(cur_weak);
+        } else {
+          VisitingOldObject(NULL);
+        }
         cur_weak->VisitPointersNonvirtual(this);
       } else {
         // Requeue this weak property to be handled later.
         EnqueueWeakProperty(cur_weak);
       }
       // Advance to next weak property in the queue.
       cur_weak = reinterpret_cast<RawWeakProperty*>(next_weak);
     }
     VisitingOldObject(NULL);
     return marked;
   }
 
   void DrainMarkingStack() {
     RawObject* raw_obj = work_list_.Pop();
     if ((raw_obj == NULL) && ProcessPendingWeakProperties()) {
       raw_obj = work_list_.Pop();
     }
 
     if (raw_obj == NULL) {
       ASSERT(visiting_old_object_ == NULL);
       return;
     }
     do {
       do {
         // First drain the marking stacks.
-        VisitingOldObject(raw_obj);
+        if (raw_obj->IsOldObject()) {
+          VisitingOldObject(raw_obj);
+        } else {
+          VisitingOldObject(NULL);
+        }
+
         const intptr_t class_id = raw_obj->GetClassId();
         if (class_id != kWeakPropertyCid) {
           marked_bytes_ += raw_obj->VisitPointersNonvirtual(this);
         } else {
           RawWeakProperty* raw_weak =
               reinterpret_cast<RawWeakProperty*>(raw_obj);
           marked_bytes_ += ProcessWeakProperty(raw_weak);
         }
         raw_obj = work_list_.Pop();
       } while (raw_obj != NULL);
@@ skipping 16 matching lines @@
 
   bool visit_function_code() const { return skipped_code_functions_ == NULL; }
 
   virtual void add_skipped_code_function(RawFunction* func) {
     ASSERT(!visit_function_code());
     skipped_code_functions_->Add(func);
   }
 
   void EnqueueWeakProperty(RawWeakProperty* raw_weak) {
     ASSERT(raw_weak->IsHeapObject());
     ASSERT(raw_weak->IsOldObject());

[rmacnak, 2017/06/08 21:23:48]

Update (visiting new-space weak properties now).

     ASSERT(raw_weak->IsWeakProperty());
     ASSERT(raw_weak->IsMarked());
     ASSERT(raw_weak->ptr()->next_ == 0);
     raw_weak->ptr()->next_ = reinterpret_cast<uword>(delayed_weak_properties_);
     delayed_weak_properties_ = raw_weak;
   }
 
   intptr_t ProcessWeakProperty(RawWeakProperty* raw_weak) {
     // The fate of the weak property is determined by its key.
     RawObject* raw_key = raw_weak->ptr()->key_;
     if (raw_key->IsHeapObject() && raw_key->IsOldObject() &&

[rmacnak, 2017/06/08 21:23:48]

Update (not all new-space objects are survivors)

         !raw_key->IsMarked()) {
       // Key was white. Enqueue the weak property.
       EnqueueWeakProperty(raw_weak);
       return raw_weak->Size();
     }
     // Key is gray or black. Make the weak property black.
     return raw_weak->VisitPointersNonvirtual(this);
   }
 
   // Called when all marking is complete.
@@ skipping 20 matching lines @@
 
   void VisitingOldObject(RawObject* obj) {
     ASSERT((obj == NULL) || obj->IsOldObject());
     visiting_old_object_ = obj;
   }
 
  private:
   void PushMarked(RawObject* raw_obj) {
     ASSERT(raw_obj->IsHeapObject());
     ASSERT((FLAG_verify_gc_contains)
-               ? page_space_->Contains(RawObject::ToAddr(raw_obj))
+               ? ((page_space_->Contains(RawObject::ToAddr(raw_obj))) ||
+                  (heap_->new_space()->Contains(RawObject::ToAddr(raw_obj))))
                : true);
 
     // Push the marked object on the marking stack.
     ASSERT(raw_obj->IsMarked());
     // We acquired the mark bit => no other task is modifying the header.
     // TODO(koda): For concurrent mutator, this needs synchronization. Consider
     // clearing these bits already in the CAS for the mark bit.
     raw_obj->ClearRememberedBitUnsynchronized();
     work_list_.Push(raw_obj);
   }
 
   static bool TryAcquireMarkBit(RawObject* raw_obj) {
     if (!sync) {
       if (raw_obj->IsMarked()) return false;
       raw_obj->SetMarkBitUnsynchronized();
       return true;
     }
     return raw_obj->TryAcquireMarkBit();
   }
 
   void MarkObject(RawObject* raw_obj, RawObject** p) {
     // Fast exit if the raw object is a Smi.
     if (!raw_obj->IsHeapObject()) {
       return;
     }
 
     // Fast exit if the raw object is marked.
     if (raw_obj->IsMarked()) {
+      if (raw_obj->IsNewObject()) {
+        ProcessNewSpaceObject(raw_obj, p);
+      }
       return;
     }
 
     // TODO(koda): Investigate performance impact of alternative branching:
     // if (smi or new) <-- can be done as single compare + conditional jump
     //   if (smi) return;
     //   else ...
     // if (marked) return;
     // ...
     if (raw_obj->IsNewObject()) {
       ProcessNewSpaceObject(raw_obj, p);
-      return;
     }
 
     if (!TryAcquireMarkBit(raw_obj)) {
       // Already marked.
       return;
     }
     if (RawObject::IsVariableSizeClassId(raw_obj->GetClassId())) {
       UpdateLiveOld(raw_obj->GetClassId(), raw_obj->Size());
     } else {
       UpdateLiveOld(raw_obj->GetClassId(), 0);
@@ skipping 54 matching lines @@
 typedef MarkingVisitorBase<true> SyncMarkingVisitor;
 
 
 static bool IsUnreachable(const RawObject* raw_obj) {
   if (!raw_obj->IsHeapObject()) {
     return false;
   }
   if (raw_obj == Object::null()) {
     return true;
   }
   if (!raw_obj->IsOldObject()) {

[rmacnak, 2017/06/08 21:23:48]

Update (not all new-space objects are survivors)

     return false;
   }
   return !raw_obj->IsMarked();
 }
 
 
 class MarkingWeakVisitor : public HandleVisitor {
  public:
   explicit MarkingWeakVisitor(Thread* thread) : HandleVisitor(thread) {}
 
@@ skipping 30 matching lines @@
 
 void GCMarker::IterateRoots(Isolate* isolate,
                             ObjectPointerVisitor* visitor,
                             intptr_t slice_index,
                             intptr_t num_slices) {
   ASSERT(0 <= slice_index && slice_index < num_slices);
   if ((slice_index == 0) || (num_slices <= 1)) {
     isolate->VisitObjectPointers(visitor,
                                  StackFrameIterator::kDontValidateFrames);
   }
-  if ((slice_index == 1) || (num_slices <= 1)) {
-    heap_->new_space()->VisitObjectPointers(visitor);
-  }
 
   // For now, we just distinguish two parts of the root set, so any remaining
   // slices are empty.
 }
 
 
 void GCMarker::IterateWeakRoots(Isolate* isolate, HandleVisitor* visitor) {
   ApiState* state = isolate->api_state();
   ASSERT(state != NULL);
   isolate->VisitWeakPersistentHandles(visitor);
@@ skipping 21 matching lines @@
 class ObjectIdRingClearPointerVisitor : public ObjectPointerVisitor {
  public:
   explicit ObjectIdRingClearPointerVisitor(Isolate* isolate)
       : ObjectPointerVisitor(isolate) {}
 
 
   void VisitPointers(RawObject** first, RawObject** last) {
     for (RawObject** current = first; current <= last; current++) {
       RawObject* raw_obj = *current;
       ASSERT(raw_obj->IsHeapObject());
       if (raw_obj->IsOldObject() && !raw_obj->IsMarked()) {

[rmacnak, 2017/06/08 21:23:48]

Update (not all new-space objects are survivors)

         // Object has become garbage. Replace it will null.
         *current = Object::null();
       }
     }
   }
 };
 
 
 void GCMarker::ProcessObjectIdTable(Isolate* isolate) {
 #ifndef PRODUCT
@@ skipping 228 matching lines @@
       // Phase 3: Finalize results from all markers (detach code, etc.).
       barrier.Exit();
     }
     ProcessWeakTables(page_space);
     ProcessObjectIdTable(isolate);
   }
   Epilogue(isolate, invoke_api_callbacks);
 }
 
 }  // namespace dart