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

[Sundararajan Athijegannathan]

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