[foreign-abi] RFR: 8248560: Specify the behaviour of the ForeignLinker returned by CSupport::getSystemLinker [v4]

[Maurizio Cimadamore]

On Wed, 16 Sep 2020 11:27:05 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Hi,
>> 
>> This patch creates a new class called CLinker, which extends ForeignLinker. The new class documents how linking works
>> in the class's javadoc. This is done in a separate class since the specification is only relevant to linkers that
>> support the C ABI.  With the new class there was no more need to have CSupport, so the contents of CSupport has all
>> been move to CLinker.
>> After some offline discussion with Maurizio, we realized that it doesn't make much sense to expose platform specific
>> layout constants, since it is not possible to combine them with a platform specific linker any way, so for a particular
>> platform, there is only one set of layouts that can be used, namely the ones that are dynamically picked based on the
>> current platform. For that reason, all platform specific layouts have instead been moved to an internal PlatformLayouts
>> class. In order for downstream APIs (like jextract) to be able to filter out the ABI attributes from layouts, I've
>> added a String constant to CLinker that is used as a name prefix for all the ABI attribute names, so clients can for
>> instance filter attribute names using `.startsWith(ABI_ATTR_NAME)`.  The ForeignLinker::name method has been dropped in
>> favor of an internal enum.  The rest of the patch is just mechanical CSupport -> CLinker renames.  Thanks, Jorn
>
> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove public attribute name prefix

Some more comments, now mostly focused on downcallHandle/upcallStub

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 118:

> 116:
> 117:     /**
> 118:      * Obtain a method handle which can be used to call a given native function.

This feels a bit thin. I would replace with:
Obtain a foreign method handle, with given type, which can be used to call a target foreign function at a given address
and featuring a given function descriptor.

Let's also consider renaming `downcallHandle` to `foreignHandle`

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 136:

> 134:      * the corresponding native stub will be deallocated.</p>
> 135:      *
> 136:      * <p>The method type of the target method handle is used for linking</p>

I think we can leave this out

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 129:

> 127:
> 128:     /**
> 129:      * Allocates a native stub segment which contains executable code to upcall into a given method handle.

Suggestion:

Allocates a native segment whose base address (see {@link MemorySegment#address} can can be passed to other foreign
functions (as a function pointer); calling such a function pointer from native code will result in the execution of the
provided method handle. <p> The returned segment is <em>not</em> thread-confined, and it only features
the {@link MemorySegment#CLOSE} access mode. When the returned segment is closed, the corresponding native stub will be
deallocated.

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

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