RFR: 8373266: Strengthen constant CardTable base accesses [v2]
Thomas Schatzl
tschatzl at openjdk.org
Wed Jan 21 13:33:13 UTC 2026
On Wed, 7 Jan 2026 19:06:02 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:
>> Shenandoah and G1 are using CardTable for most of its infrastructure, but flip the card tables as they go, and maintain the actual card table reference in TLS. As such, accessing card table base from assembler and compilers runs into risk of accidentally encoding the wrong card table base in generated code.
>>
>> Most of the current code avoids this trouble by carefully implementing their GC barriers to avoid touching shared parts where card table base constness is assumed. _Except_ for JVMCI, that reads the card table base for G1 barrier set, and that is wrong. The JVMCI users would need to rectify this downstream.
>>
>> Shenandoah added a few asserts to catch these errors:
>> SHENANDOAHGC_ONLY(assert(!UseShenandoahGC, "Shenandoah byte_map_base is not constant.");)
>>
>> ...but G1 would also benefit from the similar safety mechanism.
>>
>> This PR strengthens the code to prevent future accidents.
>>
>> Additional testing:
>> - [x] Linux x86_64 server fastdebug, `hotspot_gc`
>> - [x] Linux x86_64 server fastdebug, `all` with Serial, Parallel, G1, Shenandoah, Z
>> - [x] Linux AArch64 server fastdebug, `all` with Serial, Parallel, G1, Shenandoah, Z
>> - [x] GHA, cross-compilation only
>
> Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains nine additional commits since the last revision:
>
> - Merge branch 'master' into JDK-8373266-cardtable-asserts
> - Another build fix
> - Fix Minimal builds
> - Shenandoah non-generational can have nullptr card table
> - Also simplify CTBS builder
> - CI should also mention "const"
> - Fix JVMCI by answering proper things
> - Merge branch 'master' into JDK-8373266-cardtable-asserts
> - More fixes
Looks good apart from minor nits (extra spaces). Sorry for taking so long.
src/hotspot/cpu/x86/gc/shared/cardTableBarrierSetAssembler_x86.cpp line 114:
> 112: __ subptr(end, addr); // end --> cards count
> 113:
> 114: __ mov64(tmp, (intptr_t) ctbs->card_table_base_const());
Suggestion:
__ mov64(tmp, (intptr_t)ctbs->card_table_base_const());
src/hotspot/os_cpu/linux_arm/javaThread_linux_arm.cpp line 47:
> 45: if (bs->is_a(BarrierSet::CardTableBarrierSet)) {
> 46: CardTableBarrierSet* ctbs = CardTableBarrierSet::barrier_set();
> 47: _card_table_base = (address) ctbs->card_table_base_const();
Suggestion:
_card_table_base = (address)ctbs->card_table_base_const();
For consistency
-------------
Changes requested by tschatzl (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/28703#pullrequestreview-3687245667
PR Review Comment: https://git.openjdk.org/jdk/pull/28703#discussion_r2712571786
PR Review Comment: https://git.openjdk.org/jdk/pull/28703#discussion_r2712573615
More information about the hotspot-dev
mailing list