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

[Henry Jen]

On Mon, 17 Feb 2020 13:45:55 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> The fundamental issue of the crash is that clang cursor/type may no longer valid after reparse another macro, and with pointer type lazily resolve the pointee type, if the clang cursor/type no longer valid, access invalid memory cause the crash.
>> 
>> The other issue is the a macro pointer to a record type like struct/union can cause NPE, and that cause the macro to be ignored on generation without a warning.
>> 
>> The fix remove the laziness from Type API perspective, but leave that as an implementation detail, so implementation of Pointer type need to figure out how to do that safely internally.
>> 
>> Record type in macro are reduces to void, as currently the Declaration implementation is depending on clang as well, we cannot guarantee that works after reparse.
> 
> 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.

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

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