Integrated: 8331865: Consolidate size and alignment checks in LayoutPath

[Maurizio Cimadamore]

On Wed, 15 May 2024 15:43:45 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> When creating a nested memory access var handle, we ensure that the segment is accessed at the correct alignment for the root layout being accessed. But we do not ensure that the segment has at least the size of the accessed root layout. Example:
> 
> 
> MemoryLayout LAYOUT = sequenceLayout(2, structLayout(JAVA_INT.withName("x"), JAVA_INT.withName("y")));
> VarHandle X_HANDLE = LAYOUT.varHandle(PathElement.sequenceElement(), PathElement.groupElement("x"));
> 
> 
> If I have a memory segment whose size is 12, I can successfully call X_HANDLE on it with index 1, like so:
> 
> 
> MemorySegment segment = Arena.ofAuto().allocate(12);
> int x = (int)X_HANDLE.get(segment, 0, 1);
> 
> 
> This seems incorrect: after all, the provided segment doesn't have enough space to fit _two_ elements of the nested structs. 
> 
> This PR adds checks to detect this kind of scenario earlier, thus improving usability. To achieve this we could, in principle, just tweak `LayoutPath::checkEnclosingLayout` and add the required size check.
> 
> But doing so will add yet another redundant check on top of the other already added by [pull/19124](https://git.openjdk.org/jdk/pull/19124). Instead, this PR rethinks how memory segment var handles are created, in order to eliminate redundant checks.
> 
> The main observation is that size and alignment checks depend on an _enclosing_ layout. This layout *might* be the very same value layout being accessed (this is the case when e.g. using `JAVA_INT.varHandle()`). But, in the general case, the accessed value layout and the enclosing layout might differ (e.g. think about accessing a struct field).
> 
> Furthermore, the enclosing layout check depends on the _base_ offset at which the segment is accessed, _prior_ to any index computation that occurs if the accessed layout path has any open elements. It is important to notice that this base offset is only available when looking at the var handle that is returned to the user. For instance, an indexed var handle with coordinates:
> 
> 
> (MemorySegment, long /* base */, long /* index 1 */, long /* index 2 */, long /* index 3 */)
> 
> 
> Is obtained through adaptation, by taking a more basic var handle of the form:
> 
> 
> (MemorySegment, long /* offset */)
> 
> 
> And then injecting the result of the index multiplication into `offset`. As such, we can't add an enclosing layout check inside the var handle returned by `VarHandles::memorySegmentViewHandle`, as doing so will end up seeing the *wrong* offset (e.g. an offset in which the index part has already been mixed in)...

This pull request has now been integrated.

Changeset: c003c120
Author:    Maurizio Cimadamore <mcimadamore at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/c003c1207fae07bcfe5a6f642a9c05e6c591e7a6
Stats:     205 lines in 12 files changed: 72 ins; 39 del; 94 mod

8331865: Consolidate size and alignment checks in LayoutPath

Reviewed-by: psandoz, jvernee

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

PR: https://git.openjdk.org/jdk/pull/19251