On Wed, 22 Apr 2026 13:42:49 GMT, Casper Norrbin <[email protected]> wrote:
> Hi everyone, > > One of the `FlatFieldPayload` constructors uses `container->klass()` when > calling `inline_layout_info_adr`, even though the layout should be resolved > from `field_holder()`. When JNI `GetObjectField` or `SetObjectField` accesses > an inherited flat field on a derived class it ends up looking up inline > layout info from the that class instead of the class that actually declares > the field. In debug builds this triggers an assert, and in general it is the > wrong class to use for the lookup. > > This path is only reached through JNI field access. The original fix is from > @stefank. I added a regression test that tests JNI reads and writes of > inherited flat fields and checks that the inherited base field is resolved > correctly. The new test hits the assert before the fix and passes with the > patch applied. > > Testing: > - Tier 1 > - New test testing modifying inherited flat fields through JNI > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). The changes make sense and the test seems reasonable. Some comments and questions attached. test/hotspot/jtreg/runtime/valhalla/inlinetypes/InheritedFlatFieldTest.java line 33: > 31: > 32: /* > 33: * @test Is there merit enough to keep this test around for non-debug builds? I'm asking because `assert_post_construction_invariants` is defined for debug VMs. I think it's probably fine to keep. test/hotspot/jtreg/runtime/valhalla/inlinetypes/InheritedFlatFieldTest.java line 41: > 39: * @run main/othervm/native --enable-native-access=ALL-UNNAMED > 40: * -XX:+UnlockDiagnosticVMOptions > 41: * -XX:+UseFieldFlattening If you're explicitly enabling `UseFieldFlattening`, you might want to enable `UseNullFreeXXValueFlattening` where `XX` is the appropriate atomicity (atomic here, I believe). test/hotspot/jtreg/runtime/valhalla/inlinetypes/libInheritedFlatFieldTest.c line 26: > 24: #include <jni.h> > 25: > 26: #define VALUE_SIGNATURE > "Lruntime/valhalla/inlinetypes/InheritedFlatFieldTest$Value;" Nit: undefine these after you're done. ------------- PR Review: https://git.openjdk.org/valhalla/pull/2355#pullrequestreview-4161467962 PR Review Comment: https://git.openjdk.org/valhalla/pull/2355#discussion_r3130411287 PR Review Comment: https://git.openjdk.org/valhalla/pull/2355#discussion_r3129945976 PR Review Comment: https://git.openjdk.org/valhalla/pull/2355#discussion_r3129959286
