Skip to content

Commit 587f703

Browse files
authored
fixing a bug in card mark stealing (#117968)
this fixes a problem with card mark stealing where we missed clamping the card clearing by the card stealing unit in `card_transition`. for this bug to appear the following conditions need to be met - + an object A straddles the 2mb card stealing unit and originally for that object a card below the 2mb boundary and a card that corresponds to at least 256 bytes above the 2mb boundary are set. and there are no reference fields inbetween. + one thread T0 is working on the 1st 2mb and discovers A and the first set card bit. this card doesn't need to be set, so `poo` is set the address that's described by the 2nd card since there're no reference fields inbetween. so `card_transition` is called which will call `clear_cards` on [1st card, (2nd card. and it stops at this line - ` card_table [end_word] &= highbits (~0, bits);` where it sees `end_card` with the 2nd card still set, but before it writes it back to `card_table[end_word]` + meanwhile, another thread T1 needs to be working on the memory starting from this 2mb boundary. it discovers the 2nd card doesn't need to be set, and none of the cards that correspond to the card bundle bit needs to be set so it clears the cards and the card bundle bit. + now T0 writes back to `card_table[end_word]` with the 2nd card bit set. it's not a problem when a card that shouldn't be set is set, given that its corresponding card bundle bit is also set. but it's definitely a problem if a card is set but its card bundle bit isn't, because next time when we have a cross gen reference, what's supposed to happen in the write barrier is either the card isn't already set and the WB will set the card and its corresponding card bundle bit, or the card is set and the WB wouldn't do anything. but now we have a situation where the card is set but the card bundle bit isn't, it just means the next GC that should be looking at this card wouldn't, if there were no other cards covered by that card bundle bit got newly set by the WB. the cleanest fix is to make sure we don't step outside of the 2mb boundary when we call `clear_cards` in `card_transition`. this issue was very hard to observe and debug - full credit goes to @ChrisAhna who also verified the fix.
1 parent 4d631c9 commit 587f703

File tree

1 file changed

+7
-4
lines changed

1 file changed

+7
-4
lines changed

src/coreclr/gc/gc.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42140,10 +42140,13 @@ BOOL gc_heap::card_transition (uint8_t* po, uint8_t* end, size_t card_word_end,
4214042140
//dprintf(3,(" Clearing cards [%zx, %zx[ ",
4214142141
dprintf(3,(" CC [%zx, %zx[ ",
4214242142
(size_t)card_address(card), (size_t)po));
42143-
clear_cards (card, card_of(po));
42144-
n_card_set -= (card_of (po) - card);
42145-
n_cards_cleared += (card_of (po) - card);
42146-
42143+
uint8_t* card_clearing_limit = po;
42144+
#ifdef FEATURE_CARD_MARKING_STEALING
42145+
card_clearing_limit = min (limit, po);
42146+
#endif // FEATURE_CARD_MARKING_STEALING
42147+
clear_cards (card, card_of (card_clearing_limit));
42148+
n_card_set -= (card_of (card_clearing_limit) - card);
42149+
n_cards_cleared += (card_of (card_clearing_limit) - card);
4214742150
}
4214842151
n_eph +=cg_pointers_found;
4214942152
cg_pointers_found = 0;

0 commit comments

Comments
 (0)