[foreign] RFR 8217664: jextract should check for error code returns from Type::getOffsetOf and Type::size

[Maurizio Cimadamore]

I'll leave the detailed review to Sundar - I just wanted to say that I 
noticed this issues as well in the past (e.g. Clang API returning weird 
things for incomplete arrays).

On the binder test - maybe it shouldn't be in the jextract folder? Also, 
while you test that allocation works, the test is basically checking if 
two method calls return null as expected - which is a tad weird? 
Shouldn't we also check that we can actually get an instance of that 
array and that the size should be set to zero? (e.g. binder should not 
crash when dereferencing one of these odd zero-length arrays).

Maurizio

On 24/01/2019 13:04, Jorn Vernee wrote:
> Hi,
>
> jextract uses libclang functions to query the size of a type and 
> offset of a field. The returned values are used for instance in layout 
> calculations. But, these functions can also return error codes and 
> there is currently no check for them. They are happily being used in 
> the layout calculations, and this can cause some strange errors, or in 
> some cases incorrect layouts are generated. To give an example:
>
>     struct Foo {
>         int x;
>         void *list_of_data[];
>     };
>
> The expected layout of this struct (which uses an incomplete/flexible 
> array field) should be `[i32x32[0u64]]` but jextract instead generates 
> `[i32[0u64]x32]`. This particular case is due to a bug in libclang 
> where requesting the offset of a field in a type with a flexible array 
> member always returns an incomplete type error, even though getting 
> the offset of each fields should be possible. This error code (-2) is 
> then used in the layout calculations and yields the incorrect result.
>
> So I've added checking to the returned value of Type::size and 
> Type::getOffsetOf and throwing an exception in case an error code is 
> returned. I'm also explicitly checking for the flexible array case and 
> throwing a UOE in that case. This also means I had to disable the 
> flexible array jextract test [1]. The test was passing, but at least 
> on Windows the wrong layout was being generated. Since we can not 
> reliably compute padding for types with flexible arrays using the 
> current method, I think it's better to throw an exception for now 
> until we can find a workaround for this bug in libclang, or the bug 
> gets fixed.
>
> Using a type with a flexible/incomplete array member in the binder 
> should still be fine of course, so I've added a separate test for that.
>
> Please review the following.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8217664
> Webrev: 
> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8217664/webrev.00/
>
> Thanks,
> Jorn
>
> [1] : 
> http://hg.openjdk.java.net/panama/dev/file/c40a5a6dc24d/test/jdk/com/sun/tools/jextract/testStruct/struct.h#l85