[lworld] RFR: 8375434: [lworld] Cleanup null-marker check for InlineKlass::read_payload_from_addr

[Joel Sikström]

On Thu, 15 Jan 2026 13:53:59 GMT, Frederic Parain <fparain at openjdk.org> wrote:

>> Hello,
>> 
>> Right now we check the null-marker for the NULLABLE_ATOMIC_FLAT LayoutKind twice, both before and after allocating a heap instance. The first check returns nullptr early if the null-marker is set, as we don't have to allocate an object on the heap if we are going to return nullptr anyways. The other check is redundant, since if the null-marker is not set, the source object is non-null, which means the destination (res) object is non-null as well after copying, so the code inside it is unreachable.
>> 
>> Testing:
>> * Oracle's tier1-4, hotspot_valhalla, jdk_valhalla
>> * I also added a `gurantee` for the removed code and ran through hotspot_valhalla and jdk_valhalla locally.
>
> The second check  is needed because the source value might have been mutated between the first null-marker check and the value copying, and we don't want to have heap allocated instances being equivalent to a 'null' value.
> Here's the scenario:
>   - thread 1 starts to read a nullable flat value, it checks that the value is not marked as null, then proceeds to copy the value but is preempted
>   - thread 2 updates the nullable flat value by writing 'null' to it, which means setting the null-marker to zero, but also resetting all the other fields
>   - thread 1 resumes its read, allocate the heap instance, and copy the payload
>  
> Now, the problem is that there's a heap allocated instance with a payload that has been reset, which means fields contents might not be valid for this class. We don't want to have to check the null-marker each time we access a heap buffered value, we want to preserve the invariant that all heap buffered values are valid values for the class. So, the null marker is checked a second time in the heap allocated instance, and if it is zeroed, meaning the payload written to the instance is equivalent to null, then the instance is discarded.

@fparain I see, thank you for the example and explanation! As this was not immediately clear to me (and others), maybe I can move the direction of this PR to add a brief comment explaining why the additional null-marker check in the end is needed here?

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

PR Comment: https://git.openjdk.org/valhalla/pull/1914#issuecomment-3755264785