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

[Henry Jen]

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
>