[foreign-jextract] [Rev 01] RFR: 8239128: Macro defines constant pointers can crash VM via jextract API

[Henry Jen]

On Tue, 18 Feb 2020 01:34:08 GMT, Henry Jen <henryjen at openjdk.org> wrote:

>> src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/TypeMaker.java line 53:
>> 
>>> 52:      */
>>> 53:     void resolveTypeReferences() {
>>> 54:         pointers.forEach(TypeImpl.PointerImpl::type);
>> 
>> This approach seems flexible. It relies on keeping track of which pointers are created and then have the client call, at a later point 'resolveTypeReferences' to make sure that we fully convert all pointee clang types into clang-agnostic types.
>> 
>> I have few observations here:
>> * this mechanism is, general and should allow for all pointers to remove dependencies from clang-originated types, which should allow us to be able to close the clang index after we parse
>> * I'm a bit worried that the resolve logic is a bit too simplistic. In principle, it could be possible for resolve() to add new pointer types into `pointers`, so, resolve should use some kind of fixed point logic - e.g. keep going until we're sure _all_ types have been resolved, no matter how deep.
> 
> Good point, in case of pointer to pointer, current way of create pointer type will add another pointer and that could lead to issues. I'll add a test case to that.
> 
> However, we are setting for the resolveTypeReferences after we finished parsing a header, thus all types suppose to be known, and there is no more reason not to create pointer with pointee type directly.

I had something like this before, but as you can see, it's only used by Pointer, thus I simply consolidated them.

We cannot use j.i.clang.Type for identity, as the equal method could actually call into native code. The fact that same type not necessary by same instance of CXType struct actually bit me when I use clang.Type as key for unresolved map instead of plain list.

-------------

PR: https://git.openjdk.java.net/panama-foreign/pull/21