RFR: 8292912: Make guard card in CardTable inaccessible

Thomas Schatzl tschatzl at openjdk.org
Mon Sep 5 12:59:11 UTC 2022


On Thu, 25 Aug 2022 10:29:48 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

> Simple change of making the guard card inaccessible.
> 
> Test: tier1-6

Changes requested by tschatzl (Reviewer).

src/hotspot/share/gc/g1/g1CardTable.cpp line 56:

> 54: 
> 55:   size_t num_cards = cards_required(_whole_heap.word_size());
> 56:   _last_valid_index = num_cards - 1;

It looks like `_last_valid_index` can be removed: it's only use is in asserts or very infrequently used methods afaict which imho does not warrant an explicit member now.

src/hotspot/share/gc/shared/cardTable.cpp line 97:

> 95:   // each card takes 1 byte; + 1 for the guard card
> 96:   size_t num_bytes = num_cards + 1;
> 97:   _byte_map_size = compute_byte_map_size(num_bytes);

This seems a pre-existing bug/oddity, but `_byte_map_size` includes the guard card; there are several asserts (in `CardTable::byte_for` and `PSCardTable::is_valid_card_address`) that check:

assert(result >= _byte_map && result < _byte_map + _byte_map_size,

which seems weird to me as the card at the guard card is somehow assumed to be a valid part of the card table.
I believe the `_byte_map_size` should not include the guard card itself (which we replaced by a guard page where access to it will cause an immediate error anyway), but pass the `+1` to the `ReservedSpace` constructor directly (and maybe adjust other locations that use it to print stuff, i.e. introduce a helper method that adds that +1?)

What do you think?

src/hotspot/share/gc/shared/cardTable.cpp line 382:

> 380: }
> 381: 
> 382: void CardTable::verify_guard() {

This empty method can be removed and everything that uses it only (`CardTable::verify`).

-------------

PR: https://git.openjdk.org/jdk/pull/10020



More information about the hotspot-gc-dev mailing list