[lworld] RFR: 8373779: [lworld] Type confusion with InlineKlassFixedBlock fields

[Tobias Hartmann]

On Tue, 16 Dec 2025 10:35:31 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

> The InlineKlassFixedBlock class has the following fields:
> 
> class InlineKlassFixedBlock {
>   Array<SigEntry>** _extended_sig;
>   Array<VMRegPair>** _return_regs;
>   address* _pack_handler;
>   address* _pack_handler_jobject;
>   address* _unpack_handler;
>   int* _null_reset_value_offset;
> 
> 
> with associated functions that all look something like this:
> 
>   address adr_extended_sig() const {
>     assert(_adr_inlineklass_fixed_block != nullptr, "Should have been initialized");
>     return ((address)_adr_inlineklass_fixed_block) + in_bytes(byte_offset_of(InlineKlassFixedBlock, _extended_sig));
>   }
> 
> 
> Given the name of the function you would expect that it returned the address of the `_extended_sig` field, and the implementation seems to support this observation. So, the type above should be `Array<SigEntry>***`. However, callers of this function expects the type to be `Array<SigEntry>**` and casts it as such.
> 
> This all seems to work because we erase the type by casting to `address` and then it doesn't matter what types the `InlineKlassFixedBlock` fields have, as long as they are pointer fields.
> 
> I propose that we restructure the code to reduce the casting and fix the type confusion.
> 
> As part of this cleanup I've also:
> 
> 1) Moved the initialization of the InlineKlassFixedBlock to the InlineKlass constructor.
> 
> I think we can pull this even further and move the InlineKlassFixedBlock over to inlineKlass.hpp instead of having it in instanceKlass.hpp. And adding a proper constructor for this class instead of the explicit set of the fields. I've refrained from doing that change in this PR.
> 
> 2) Simplified (IMHO) the address calculation of the location of the fixed block.
> 
> 3) Moved the fixed block access functions together
> 
> Alternative:
> 
> One alternatives that we could consider is to skip adding the setters because they are all used from InlineKlass, which has direct access to the fields of the fixed block. Then the code would simply look like this:
> 
> fixed_block()._field = value;
> 
> 
> I'm leaving this open for reviewers nudge what they think is cleanest. 
> 
> Question:
> 
> Has the name "fixed block" some meaning that conveys a concept of value classes?
> 
> Testing:
> 
> tier1-3 on an earlier version, but I've made some tweaks after that and will rerun testing.

This is a nice cleanup, thanks Stefan!

> Has the name "fixed block" some meaning that conveys a concept of value classes?

No, I don't think so.

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

Marked as reviewed by thartmann (Committer).

PR Review: https://git.openjdk.org/valhalla/pull/1803#pullrequestreview-3582694272