RFR: 8331865: Consolidate size and alignment checks in LayoutPath

[Per Minborg]

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)...

Do we have any performance tests available to see if there are any potential impacts?

src/java.base/share/classes/java/lang/invoke/VarHandleSegmentViewBase.java line 48:

> 46:     final long alignmentMask;
> 47: 
> 48:     VarHandleSegmentViewBase(VarForm form, boolean be, long alignmentMask, boolean exact) {

Nit: Copyright year. This also applies to some other files in this PR.

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

PR Comment: https://git.openjdk.org/jdk/pull/19251#issuecomment-2114955648
PR Review Comment: https://git.openjdk.org/jdk/pull/19251#discussion_r1603156019