GC: Fix CheckPromoted for frozen objects

src/coreclr/gc/gc.cpp

@@ -8460,19 +8460,13 @@ uint32_t* translate_mark_array (uint32_t* ma)
     return (uint32_t*)((uint8_t*)ma - size_mark_array_of (0, g_gc_lowest_address));
 }
 
-// from and end must be page aligned addresses.
-void gc_heap::clear_mark_array (uint8_t* from, uint8_t* end, BOOL check_only/*=TRUE*/
 #ifdef FEATURE_BASICFREEZE
-                                , BOOL read_only/*=FALSE*/
-#endif // FEATURE_BASICFREEZE
-                                )
+// end must be page aligned addresses.
+void gc_heap::clear_mark_array (uint8_t* from, uint8_t* end, BOOL read_only/*=FALSE*/)
 {
-    if(!gc_can_use_concurrent)
-        return;
+    assert (gc_can_use_concurrent);
 
-#ifdef FEATURE_BASICFREEZE
     if (!read_only)
-#endif // FEATURE_BASICFREEZE
     {
         assert (from == align_on_mark_word (from));
     }
@@ -8489,45 +8483,41 @@ void gc_heap::clear_mark_array (uint8_t* from, uint8_t* end, BOOL check_only/*=T
         size_t beg_word = mark_word_of (align_on_mark_word (from));
         //align end word to make sure to cover the address
         size_t end_word = mark_word_of (align_on_mark_word (end));
-        dprintf (3, ("Calling clearing mark array [%Ix, %Ix[ for addresses [%Ix, %Ix[(%s)",
+        dprintf (3, ("Calling clearing mark array [%Ix, %Ix[ for addresses [%Ix, %Ix[",
                      (size_t)mark_word_address (beg_word),
                      (size_t)mark_word_address (end_word),
-                     (size_t)from, (size_t)end,
-                     (check_only ? "check_only" : "clear")));
-        if (!check_only)
-        {
-            uint8_t* op = from;
-            while (op < mark_word_address (beg_word))
-            {
-                mark_array_clear_marked (op);
-                op += mark_bit_pitch;
-            }
+                     (size_t)from, (size_t)end));
 
-            memset (&mark_array[beg_word], 0, (end_word - beg_word)*sizeof (uint32_t));
+        uint8_t* op = from;
+        while (op < mark_word_address (beg_word))
+        {
+            mark_array_clear_marked (op);
+            op += mark_bit_pitch;
         }
+
+        memset (&mark_array[beg_word], 0, (end_word - beg_word)*sizeof (uint32_t));
+
 #ifdef _DEBUG
-        else
+        //Beware, it is assumed that the mark array word straddling
+        //start has been cleared before
+        //verify that the array is empty.
+        size_t  markw = mark_word_of (align_on_mark_word (from));
+        size_t  markw_end = mark_word_of (align_on_mark_word (end));
+        while (markw < markw_end)
         {
-            //Beware, it is assumed that the mark array word straddling
-            //start has been cleared before
-            //verify that the array is empty.
-            size_t  markw = mark_word_of (align_on_mark_word (from));
-            size_t  markw_end = mark_word_of (align_on_mark_word (end));
-            while (markw < markw_end)
-            {
-                assert (!(mark_array [markw]));
-                markw++;
-            }
-            uint8_t* p = mark_word_address (markw_end);
-            while (p < end)
-            {
-                assert (!(mark_array_marked (p)));
-                p++;
-            }
+            assert (!(mark_array [markw]));
+            markw++;
+        }
+        uint8_t* p = mark_word_address (markw_end);
+        while (p < end)
+        {
+            assert (!(mark_array_marked (p)));
+            p++;
         }
 #endif //_DEBUG
     }
 }
+#endif // FEATURE_BASICFREEZE
 #endif //BACKGROUND_GC
 
 //These work on untranslated card tables
@@ -9458,6 +9448,7 @@ void gc_heap::copy_brick_card_table()
 }
 
 #ifdef FEATURE_BASICFREEZE
+// Note that we always insert at the head of the max_generation segment list.
 BOOL gc_heap::insert_ro_segment (heap_segment* seg)
 {
 #ifdef FEATURE_EVENT_TRACE
@@ -9477,7 +9468,6 @@ BOOL gc_heap::insert_ro_segment (heap_segment* seg)
         return FALSE;
     }
 
-    //insert at the head of the segment list
     generation* gen2 = generation_of (max_generation);
     heap_segment* oldhead = generation_start_segment (gen2);
     heap_segment_next (seg) = oldhead;
@@ -9515,13 +9505,12 @@ BOOL gc_heap::insert_ro_segment (heap_segment* seg)
 // which portion of the mark array was committed and only decommit that.
 void gc_heap::remove_ro_segment (heap_segment* seg)
 {
-//clear the mark bits so a new segment allocated in its place will have a clear mark bits
+    //clear the mark bits so a new segment allocated in its place will have a clear mark bits
 #ifdef BACKGROUND_GC
     if (gc_can_use_concurrent)
     {
         clear_mark_array (align_lower_mark_word (max (heap_segment_mem (seg), lowest_address)),
-                      align_on_card_word (min (heap_segment_allocated (seg), highest_address)),
-                      false); // read_only segments need the mark clear
+                          align_on_card_word (min (heap_segment_allocated (seg), highest_address)));
     }
 #endif //BACKGROUND_GC
 
@@ -11000,20 +10989,39 @@ inline size_t my_get_size (Object* ob)
 #endif //COLLECTIBLE_CLASS
 
 #ifdef BACKGROUND_GC
+#ifdef FEATURE_BASICFREEZE
 inline
 void gc_heap::seg_clear_mark_array_bits_soh (heap_segment* seg)
 {
     uint8_t* range_beg = 0;
     uint8_t* range_end = 0;
     if (bgc_mark_array_range (seg, FALSE, &range_beg, &range_end))
     {
-        clear_mark_array (range_beg, align_on_mark_word (range_end), FALSE
-#ifdef FEATURE_BASICFREEZE
-            , TRUE
-#endif // FEATURE_BASICFREEZE
-            );
+        clear_mark_array (range_beg, align_on_mark_word (range_end), TRUE);
+    }
+}
+
+inline
+void gc_heap::seg_set_mark_array_bits_soh (heap_segment* seg)
+{
+    uint8_t* range_beg = 0;
+    uint8_t* range_end = 0;
+    if (bgc_mark_array_range (seg, FALSE, &range_beg, &range_end))
+    {
+        size_t beg_word = mark_word_of (align_on_mark_word (range_beg));
+        size_t end_word = mark_word_of (align_on_mark_word (range_end));
+
+        uint8_t* op = range_beg;
+        while (op < mark_word_address (beg_word))
+        {
+            mark_array_set_marked (op);
+            op += mark_bit_pitch;
+        }
+
+        memset (&mark_array[beg_word], 0xFF, (end_word - beg_word)*sizeof (uint32_t));
     }
 }
+#endif //FEATURE_BASICFREEZE
 
 void gc_heap::bgc_clear_batch_mark_array_bits (uint8_t* start, uint8_t* end)
 {
@@ -26493,6 +26501,19 @@ void gc_heap::mark_phase (int condemned_gen_number, BOOL mark_only_p)
             }
         }
 
+#ifdef FEATURE_BASICFREEZE
+#ifdef USE_REGIONS
+        assert (!ro_segments_in_range);
+#else //USE_REGIONS
+        if (ro_segments_in_range)
+        {
+            dprintf(3,("Marking in range ro segments"));
+            mark_ro_segments();
+            // Should fire an ETW event here.
+        }
+#endif //USE_REGIONS
+#endif //FEATURE_BASICFREEZE
+
         dprintf(3,("Marking Roots"));
 
         GCScan::GcScanRoots(GCHeap::Promote,
@@ -27538,6 +27559,18 @@ void gc_heap::process_ephemeral_boundaries (uint8_t* x,
 }
 #endif //!USE_REGIONS
 
+#ifdef FEATURE_BASICFREEZE
+inline
+void gc_heap::seg_set_mark_bits (heap_segment* seg)
+{
+    uint8_t* o = heap_segment_mem (seg);
+    while (o < heap_segment_allocated (seg))
+    {
+        set_marked (o);
+        o = o + Align (size(o));
+    }
+}
+
 inline
 void gc_heap::seg_clear_mark_bits (heap_segment* seg)
 {
@@ -27548,36 +27581,77 @@ void gc_heap::seg_clear_mark_bits (heap_segment* seg)
         {
             clear_marked (o);
         }
-        o = o  + Align (size (o));
+        o = o + Align (size (o));
     }
 }
 
-#ifdef FEATURE_BASICFREEZE
-void gc_heap::sweep_ro_segments (heap_segment* start_seg)
+// We have to do this for in range ro segments because these objects' life time isn't accurately
+// expressed. The expectation is all objects on ro segs are live. So we just artifically mark 
+// all of them on the in range ro segs.
+void gc_heap::mark_ro_segments()
 {
-    //go through all of the segment in range and reset the mark bit
-    heap_segment* seg = start_seg;
-
-    while (seg)
+#ifdef USE_REGIONS
+    assert (!ro_segments_in_range);
+#else //USE_REGIONS
+    if ((settings.condemned_generation == max_generation) && ro_segments_in_range)
     {
-        if (heap_segment_read_only_p (seg) &&
-            heap_segment_in_range_p (seg))
+        heap_segment* seg = generation_start_segment (generation_of (max_generation));
+
+        while (seg)
         {
-#ifdef BACKGROUND_GC
-            if (settings.concurrent)
+            if (!heap_segment_read_only_p (seg))
+                break;
+
+            if (heap_segment_in_range_p (seg))
             {
-                seg_clear_mark_array_bits_soh (seg);
+#ifdef BACKGROUND_GC
+                if (settings.concurrent)
+                {
+                    seg_set_mark_array_bits_soh (seg);
+                }
+                else
+#endif //BACKGROUND_GC
+                {
+                    seg_set_mark_bits (seg);
+                }
             }
-            else
+            seg = heap_segment_next (seg);
+        }
+    }
+#endif //USE_REGIONS
+}
+
+void gc_heap::sweep_ro_segments()
+{
+#ifdef USE_REGIONS
+    assert (!ro_segments_in_range);
+#else //USE_REGIONS
+    if ((settings.condemned_generation == max_generation) && ro_segments_in_range)
+    {
+        heap_segment* seg = generation_start_segment (generation_of (max_generation));;
+
+        while (seg)
+        {
+            if (!heap_segment_read_only_p (seg))
+                break;
+
+            if (heap_segment_in_range_p (seg))
             {
-                seg_clear_mark_bits (seg);
-            }
-#else //BACKGROUND_GC
-            seg_clear_mark_bits (seg);
+#ifdef BACKGROUND_GC
+                if (settings.concurrent)
+                {
+                    seg_clear_mark_array_bits_soh (seg);
+                }
+                else
 #endif //BACKGROUND_GC
+                {
+                    seg_clear_mark_bits (seg);
+                }
+            }
+            seg = heap_segment_next (seg);
         }
-        seg = heap_segment_next (seg);
     }
+#endif //USE_REGIONS
 }
 #endif // FEATURE_BASICFREEZE
 
@@ -28956,16 +29030,8 @@ void gc_heap::plan_phase (int condemned_gen_number)
     }
 
 #ifdef FEATURE_BASICFREEZE
-#ifdef USE_REGIONS
-    assert (!ro_segments_in_range);
-#else //USE_REGIONS
-    if ((generation_start_segment (condemned_gen1) != ephemeral_heap_segment) &&
-        ro_segments_in_range)
-    {
-        sweep_ro_segments (generation_start_segment (condemned_gen1));
-    }
-#endif //USE_REGIONS
-#endif // FEATURE_BASICFREEZE
+    sweep_ro_segments();
+#endif //FEATURE_BASICFREEZE
 
 #ifndef MULTIPLE_HEAPS
     int condemned_gen_index = get_stop_generation_index (condemned_gen_number);
@@ -35007,6 +35073,20 @@ void gc_heap::background_mark_phase ()
 
     dprintf (GTC_LOG, ("FM: h%d: loh: %Id, soh: %Id, poh: %Id", heap_number, total_loh_size, total_soh_size, total_poh_size));
 
+#ifdef FEATURE_BASICFREEZE
+#ifdef USE_REGIONS
+    assert (!ro_segments_in_range);
+#else //USE_REGIONS
+    if (ro_segments_in_range)
+    {
+        dprintf (2, ("nonconcurrent marking in range ro segments"));
+        mark_ro_segments();
+        //concurrent_print_time_delta ("nonconcurrent marking in range ro segments");
+        concurrent_print_time_delta ("NRRO");
+    }
+#endif //USE_REGIONS
+#endif //FEATURE_BASICFREEZE
+
     dprintf (2, ("nonconcurrent marking stack roots"));
     GCScan::GcScanRoots(background_promote,
                             max_generation, max_generation,
@@ -42364,13 +42444,8 @@ void gc_heap::background_sweep()
 #endif //DOUBLY_LINKED_FL
 
 #ifdef FEATURE_BASICFREEZE
-    generation* max_gen         = generation_of (max_generation);
-    if ((generation_start_segment (max_gen) != ephemeral_heap_segment) &&
-        ro_segments_in_range)
-    {
-        sweep_ro_segments (generation_start_segment (max_gen));
-    }
-#endif // FEATURE_BASICFREEZE
+    sweep_ro_segments();
+#endif //FEATURE_BASICFREEZE
 
     if (current_c_gc_state != c_gc_state_planning)
     {
@@ -45317,6 +45392,9 @@ bool GCHeap::IsPromoted(Object* object)
     if (o)
     {
         ((CObjectHeader*)o)->Validate(TRUE, TRUE, is_marked);
+
+        // Frozen objects aren't expected to be "not promoted" here
+        assert(is_marked || !IsInFrozenSegment(object));
     }
 #endif //_DEBUG