[foreign-jextract] RFR: 8262198: Overhaul bitfield parsing logic

[Jorn Vernee]

On Tue, 23 Feb 2021 17:07:26 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/UnionLayoutComputer.java line 93:
>> 
>>> 91:         if (actualSize < expectedSize) {
>>> 92:             // emit an extra padding of expected size to make sure union layout size is computed correctly
>>> 93:             addFieldLayout(MemoryLayout.ofPaddingBits(expectedSize));
>> 
>> Maybe an assert could be added here to check that `actualSize` is exactly `expectedSize` at the end, to catch the case where `actualSize` is larger than `expectedSize`. I think we have a problem otherwise.
>
> actual size can be less if there are bitfields - in which case we need to emit a padding word to make sure the union layout has right size.

Yes, I'm wondering if there can be cases where the actualSize is larger than the expected size, which isn't checked here currently.

>> src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/RecordLayoutComputer.java line 139:
>> 
>>> 137: 
>>> 138:     MemoryLayout bitfield(List<MemoryLayout> sublayouts) {
>>> 139:         return LayoutUtils.setBitfields(MemoryLayout.ofStruct(sublayouts.toArray(new MemoryLayout[0])));
>> 
>> Does this work correctly for unions? What if we have many bitfields that together end up being large than the union itself?
>> 
>> union Foo {
>>     int x;
>>     int bitfield1: 24;
>>     int bitfield2: 24;
>> };
>> Doesn't this get laid out like:
>> union Foo {
>>     int x;
>>     struct {
>>         int bitfield1: 24;
>>         int bitfield2: 24;
>>     }
>> };
>> 
>> With this patch? The layouts seem to differ between those two.
>
> My understanding is that bitfields in union are just disjoint. If you want them to be adjacent, then you need to put them inside a struct.
> 
> https://godbolt.org/z/nGoe38

I guess that's what I'm saying. The patch seems to be collecting all the bitfields at once and then emitting a struct layout that holds them right? But, since in a union bitfields overlap, it seems that by putting them adjacent in a struct like this has the potential to create a layout that is larger then the actual expected size.

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

PR: https://git.openjdk.java.net/panama-foreign/pull/459