On Tue, 4 Nov 2025 13:06:05 GMT, Coleen Phillimore <[email protected]> wrote:
>> Hi all, >> >> This PR fixes [JDK-8370479](https://bugs.openjdk.org/browse/JDK-8370479), as >> we currently don't emit barriers correctly. Tested tiers 1-3. >> >> ## Context >> >> If we consider a value class/record that contains a single field, which is a >> reference to an identity object: >> >> public static value record Element(Identity underlying) {} >> public static class Identity {} >> >> We can create a flattened array via the JDK-internal `ValueClass` API: >> >> Object[] array = ValueClass.newNullableAtomicArray(Element.class, 16); >> >> This will indeed be flattened when running with compressed oops. T the >> reference to `underlying` will be four bytes, and the null-marker an >> additional byte. Hence, we are below the 64-bit limit. Copying this array >> via `Arrays.copyOf` will trigger Valhalla-specific copying. >> >> When running with G1, there are various crashes and verification errors. >> This should not impact ZGC, as the pointers are too large to be flattened in >> a nullable array. >> >> ## New Barrier Emission >> >> We do not emit a post-write barrier when copying to an uninitialized memory >> destination. The tables below summarize what barriers, if any, are emitted >> both in the old and new versions of the copy implementation. Note that >> Serial, Parallel and G1 have the notion of post-write barriers to track >> intergenerational references. G1 is the only GC requiring pre-write >> barriers. >> >> **Old G1 barrier emission during flat array copying:** >> | | oopless | contains oops | >> | --- | --- | --- | >> | uninitialized | | | >> | initialized | | pre, post | >> >> **New G1 barrier emission during flat array copying:** >> | | oopless | contains oops | >> | --- | --- | --- | >> | uninitialized | | post | >> | initialized | | pre, post | >> >> As mentioned, when copying to uninitialized memory, a cross-generational >> could be "lost" due to the lack of a post-write barrier. We should *not* use >> a pre-write barrier when copying to uninitialized memory when running with >> G1. Doing so means we may get garbage in our SATB buffers. >> >> ## New Test Cases >> >> I introduce a test scenario where we grow a flat array similar to how one >> would grow an `ArrayList`. This should generate plenty of garbage, and >> triggers this crash even without the whitebox GC. I test the three GCs that >> use `ModRefBarrierSet`: Serial, Parallel and G1. These are tweaked to be >> less concurrent/parallel to aid with reproducability in case of crashes. >> >> ## Fixed Oop Printing >> >> When G1 verification fails, it tries to p... > > src/hotspot/share/classfile/javaClasses.inline.hpp line 135: > >> 133: return obj != nullptr && obj->klass_without_asserts() == >> vmClasses::String_klass(); >> 134: } >> 135: > > Can you make a small upstream patch for this in mainline (either in addition > or instead of this part of the change?) Sure, I'll do it separately, I want to get this fixed in Valhalla ASAP. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1713#discussion_r2490465812
