RFR: 8340205: Native linker allows MemoryLayout consisting of only PaddingLayout [v5]

[Maurizio Cimadamore]

On Mon, 30 Sep 2024 14:05:23 GMT, Vladimir Kozelkov <duke at openjdk.org> wrote:

> > abstract "reduction" rules
> 
> I already use a similar "reduction" in stripNames(), but for all layouts, not just PaddingLayout. For example, nested structures and unions are unpacked to be flatter and the MethodHandle cache is more efficient. I like this approach.

It seems something that makes sense for the implementation to do (e.g. to maximize number of cache hits). Not sure if e.g. `struct { x, y }` == `struct { struct { x } { struct y } }` in all ABIs, so probably something that the linker will want to do carefully.

It is also possible that maybe the javadoc could get away by just saying that there must be some padding, w/o really having to specify how such padding should be encoded (e.g. via a single padding layout, or multiple, or nested in a sequence). 

> 
> > current PR to reject that
> 
> In its current state it doesn't seem to do this. Will you be making further changes?

I believe we will need to make further changes based on your additional examples. E.g. we need to decide how liberal we want to be with respect to padding and then enforce that uniformly. I agree that the current patch doesn't enforce the invariants I had in mind reliably.

> 
> What about my examples 3 and 4 with overaligned unions? They look like a serious problem.

Alignment and padding is also a problem. In that you can't merge padding layouts if the alignment of the merged parts is different (which is why I was initially skeptical that we could capture all these rules in the javadoc). Realistically, the Linker should probably reject any padding layout that has alignment other than 1. As a stretch we could allow padding that is naturally aligned (e.g. padding of size 4, aligned to 4), but even that seems a bit overkill.

Also, in your union example there's another problem as well: the additional padding field is redundant, at least according to the rules in the javadoc: there's a single non-padding field with size 32 and alignment 8. I don't think anything else is needed here.

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

PR Comment: https://git.openjdk.org/jdk/pull/21041#issuecomment-2383353460