Skip to content

Commit

Permalink
Fix ro segments with regions (dotnet#58811)
Browse files Browse the repository at this point in the history
There were a couple places in the mark and relocation phases where the regions logic couldn't cope with objects outside of the memory range reserved for the GC heap. These objects may occur in read-only segments (but only if FEATURE_BASICFREEZE is enabled).

The check for objects within the address range reserved for the GC is centralized in helper is_in_heap_range. If FEATURE_BASICFREEZE is enabled, this helper checks that the object is actually in the address range for the heap, otherwise a simple check for non-null is sufficient. There are asserts to make sure that objects are either null, in range, or in a read-only segment if FEATURE_BASICFREEZE is enabled.
  • Loading branch information
PeterSolMS authored Sep 17, 2021
1 parent 7638249 commit ae46d07
Showing 1 changed file with 26 additions and 6 deletions.
32 changes: 26 additions & 6 deletions src/coreclr/gc/gc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22286,13 +22286,31 @@ BOOL gc_heap::gc_mark1 (uint8_t* o)
return marked;
}

#ifdef USE_REGIONS
inline bool is_in_heap_range (uint8_t* o)
{
#ifdef FEATURE_BASICFREEZE
// we may have frozen objects in read only segments
// outside of the reserved address range of the gc heap
assert (((g_gc_lowest_address <= o) && (o < g_gc_highest_address)) ||
(o == nullptr) || (ro_segment_lookup (o) != nullptr));
return ((g_gc_lowest_address <= o) && (o < g_gc_highest_address));
#else //FEATURE_BASICFREEZE
// without frozen objects, every non-null pointer must be
// within the heap
assert ((o == nullptr) || (g_gc_lowest_address <= o) && (o < g_gc_highest_address));
return (o != nullptr);
#endif //FEATURE_BASICFREEZE
}
#endif //USE_REGIONS

inline
BOOL gc_heap::gc_mark (uint8_t* o, uint8_t* low, uint8_t* high, int condemned_gen)
{
#ifdef USE_REGIONS
assert (low == 0);
assert (high == 0);
if (o)
if (is_in_heap_range (o))
{
BOOL already_marked = marked (o);
if (already_marked)
Expand Down Expand Up @@ -23318,7 +23336,7 @@ inline
void gc_heap::mark_object (uint8_t* o THREAD_NUMBER_DCL)
{
#ifdef USE_REGIONS
if ((o != nullptr) && is_in_condemned_gc (o))
if (is_in_heap_range (o) && is_in_condemned_gc (o))
{
mark_object_simple (&o THREAD_NUMBER_ARG);
}
Expand Down Expand Up @@ -30499,7 +30517,7 @@ void gc_heap::relocate_address (uint8_t** pold_address THREAD_NUMBER_DCL)
{
uint8_t* old_address = *pold_address;
#ifdef USE_REGIONS
if (!old_address || !should_check_brick_for_reloc (old_address))
if (!is_in_heap_range (old_address) || !should_check_brick_for_reloc (old_address))
{
return;
}
Expand Down Expand Up @@ -30639,7 +30657,8 @@ gc_heap::check_demotion_helper (uint8_t** pval, uint8_t* parent_obj)
{
#ifdef USE_REGIONS
uint8_t* child_object = *pval;
if (!child_object) return;
if (!is_in_heap_range (child_object))
return;
int child_object_plan_gen = get_region_plan_gen_num (child_object);
bool child_obj_demoted_p = is_region_demoted (child_object);

Expand Down Expand Up @@ -35875,7 +35894,8 @@ gc_heap::mark_through_cards_helper (uint8_t** poo, size_t& n_gen,
assert (nhigh == 0);
assert (next_boundary == 0);
uint8_t* child_object = *poo;
if (!child_object) return;
if (!is_in_heap_range (child_object))
return;
int child_object_gen = get_region_gen_num (child_object);
int saved_child_object_gen = child_object_gen;
uint8_t* saved_child_object = child_object;
Expand Down Expand Up @@ -43182,7 +43202,7 @@ bool GCHeap::IsPromoted(Object* object)
else
{
#ifdef USE_REGIONS
return (o ? (gc_heap::is_in_condemned_gc (o) ? gc_heap::is_mark_set (o) : true) : true);
return (is_in_heap_range (o) ? (gc_heap::is_in_condemned_gc (o) ? gc_heap::is_mark_set (o) : true) : true);
#else
gc_heap* hp = gc_heap::heap_of (o);
return (!((o < hp->gc_high) && (o >= hp->gc_low))
Expand Down

0 comments on commit ae46d07

Please sign in to comment.