[foreign] RFR: 8223489: _Atomic types can cause StackOverflowError

[Henry Jen]

Yes, I did some experiments and knew that works.

Make the client.c into client.h and remove main to be more close to our situation.

However, jextract still seems to have some issue with that,

jextract -C -include-pch -C test.h.gch client.h, I see the following message

WARNING: nothing to generate

Cheers,
Henry

> On May 15, 2019, at 3:11 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
> 
> The error you are getting is odd.
> 
> I'd like to look at the patch you are trying, if possible.
> 
> I tried something like this:
> 
> $ cat test.h
> 
> typedef unsigned char uchar_t;
> typedef _Atomic(int) atomic_int_t;
> 
> struct SomeTypes {
>    _Atomic uchar_t auc;
>    volatile unsigned int vui;
>    _Atomic atomic_int_t aai;
> };
> 
> typedef _Atomic struct SomeTypes atomic_some_types_t;
> 
> 
> $ cat client.c
> 
> struct SomeTypes foo;
> 
> 
> int main(void) {
> int x = foo.vui;
> return 0;
> }
> 
> $ clang test.h
> 
> $ clang -include-pch test.h.gch client.c
> 
> This works ok, and 'SomeTypes' is resolved to the right type name...
> 
> Maurizio
> 
> On 15/05/2019 05:36, Henry Jen wrote:
>> BTW, I can get away with incomplete type by adding CXTranslationUnit_Incomplete option, but not sure about typedef,
>> 
>> java.lang.IllegalArgumentException: Error with snippet: uchar_t var;
>> /var/folders/nk/wl6yv8l565g17ykv8658hcq80000gn/T/jextract$4453798577315022046.h:1:1: error: unknown type name 'uchar_t'
>> 
>> Cheers,
>> Henry
>> 
>> 
>>> On May 14, 2019, at 9:10 PM, Henry Jen <henry.jen at oracle.com> wrote:
>>> 
>>> Use the reparse mechanism as ClangParser seems a good idea, but there are complications using precompile header.
>>> 
>>> In the test case I have, the struct SomeTypes is declared in the header file, but the way we creating new TU use the pch doesn’t work:
>>> 
>>> java.lang.IllegalArgumentException: Error with snippet: struct SomeTypes var;
>>> /var/folders/nk/wl6yv8l565g17ykv8658hcq80000gn/T/jextract$17213029476260309473.h:1:18: error: tentative definition has type 'struct SomeTypes' that is never completed
>>> 
>>> I like the idea of reparse, but current way is not working(or I haven’t figure it out), and I am not sure if use or clone(save then create) the original TU would have any side effect. At this point, I think my current implementation is more certain with the downside that we don’t have all builtin types, but at this point, that’s all we supported anyway.
>>> 
>>> Since this is to work-around current libclang, I expect the better solution is to bundle a patched libclang. But until then, this temporary fix should be good enough.
>>> 
>>> Cheers,
>>> Henry
>>> 
>>>> On May 14, 2019, at 4:03 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>>>> 
>>>> I see what you are doing here. I believe there's a cleaner way to get there, which is what we've done to process macros.
>>>> 
>>>> That is, when you see a cursor with:
>>>> 
>>>> _Atomic("....")
>>>> 
>>>> extract the innards of that cursor, and create a new temporary compilation unit for that one, as we do for macros. I think most of the logic we use there should be applicable in this case (only the snippet to be generated for the speculative compilation would be different). And, probably, if we go down that path we could refactor the code a bit so that the functionalities used for the 'speculative' clang evaluation can be shared across macro/atomic support.
>>>> 
>>>> Maurizio
>>>> 
>>>> On 14/05/2019 02:58, Henry Jen wrote:
>>>>> Hi,
>>>>> 
>>>>> Please review a webrev[1] that detects an atomic type to get the correct layout for the type.
>>>>> 
>>>>> A proper solution would be have libclang expose the atomic types, so we don’t need to add the ugly hacks in Java to find the type. A patch is submitted[2] to clang project and hopefully this can be fixed in future release.
>>>>> 
>>>>> Before that happens, we have our java clang binding trying do that work by:
>>>>> - Put together a temporary header file with C11 types to parse, so we can get this builtin-types.
>>>>> - During the cursor-traversing, we will add in extra types declared in the header files
>>>>> - For an atomic type, we use the type string to get the underlying type.
>>>>> 
>>>>> Note that an atomic type is always a builtin type without valid declaration cursor, so we cannot get the translation unit the type actually defined in, so we are doing a global search. Since jextract will only created one translation unit, the hack should work just fine without much pollution.
>>>>> 
>>>>> Cheers,
>>>>> Henry
>>>>> 
>>>>> [1] http://cr.openjdk.java.net/~henryjen/panama/8223489/0/webrev/
>>>>> [2] https://reviews.llvm.org/D61716