On Wed, 8 Apr 2026 08:07:05 GMT, Paul Hübner <[email protected]> wrote:
>> Fix a bug in the computation of layouts of abstract value classes inheriting >> fields. There was a missing case in the computation of the starting point >> for the insertion of new fields. The FieldLayoutAnalyzer has been extended >> to always verify that the alignment of the first field is compatible with >> the alignment constraint of the payload. No new test was necessary, the new >> verification in the FieldLayoutAnalyzer is sufficient to detect the issue >> when running the ValueRandomLayoutTest test. >> >> Testing in progress (tier1-4). >> >> Thanks. >> >> Fred > > src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 1172: > >> 1170: // the first inherited field. >> 1171: _payload_alignment = type2aelembytes(BasicType::T_LONG); >> 1172: _layout->set_start(_layout->first_field_block()); > > Nit: duplicate of what the concrete value class does, could be factored out > into a common path. This part of the field layout building has to set two parameters: the alignment constraint (if any) and the starting block for the insertion of new fields. The logic is organized based on the situation: concrete class vs abstract class, inherited fields vs no inherited fields. This leads to some duplication because some cases have similar parameter settings, but re-organizing the logic to be parameter based rather than case based would make the logic harder to read because then the 4 cases would have to be applied to each parameter setting. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2301#discussion_r3051365199
