RFR: 8320331: G1 Full GC Heap verification relies on metadata not reset before verification [v2]

[Albert Mingkun Yang]

On Wed, 22 Nov 2023 11:02:19 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   please review this fix to full gc to properly set `parsable_bottom` for the heap regions marked through so that "during" verification (`-XX:+VerifyDuringGC`) does not access metadata already unlinked but not yet completely purged.
>> 
>> The test case reproduces crashes fairly well without the patch; the new test case is run with debug VMs only purely to save testing time.
>> 
>> Testing: gha, test case
>> 
>> Thanks,
>>   Thomas
>
> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
> 
>   iwaluya review, comments

Marked as reviewed by ayang (Reviewer).

src/hotspot/share/gc/g1/heapRegion.inline.hpp line 170:

> 168: }
> 169: 
> 170: inline void HeapRegion::prepare_for_full_gc() {

I don't believe this API should be part of this class. It appears entirely caller-specific, and it's unclear why the implementation is just a field-update without knowledge of the caller context. While the comment helps mitigate the situation, making it acceptable for a bugfix, I think it would be more appropriate to relocate methods related to parsable-bottom outside of this class.

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

PR Review: https://git.openjdk.org/jdk/pull/16733#pullrequestreview-1744134179
PR Review Comment: https://git.openjdk.org/jdk/pull/16733#discussion_r1401886735