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

[Jorn Vernee]

Hi Sundar,

I have removed the unused code. I've also added a test for the various 
flexible/incomplete array cases that should be caught 
(jextract/incompleteArrays/IncompleteArrayTest.java), can you take a 
look?

Updated webrev: 
http://cr.openjdk.java.net/~jvernee/panama/webrevs/8217664/webrev.04/

Thanks,
Jorn

Sundararajan Athijegannathan schreef op 2019-01-25 09:57:
> jextract/clang changes fine. Tests pass on Mac. But as I mentioned,
> please remove unused code in tests rather than commenting it.
> 
> -Sundar
> 
> On 25/01/19, 7:51 AM, Sundararajan Athijegannathan wrote:
>> Unused code should be removed and not just commented (struct.h for 
>> eg). We've history in the repo and we can always go back.
>> 
>> PS. I'll build/test on Mac and respond.
>> 
>> -Sundar
>> 
>> On 25/01/19, 1:10 AM, Jorn Vernee wrote:
>>> Updated webrev for recent push: 
>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8217664/webrev.03/
>>> 
>>> Jorn
>>> 
>>> Jorn Vernee schreef op 2019-01-24 14:04:
>>>> 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