[foreign-abi] minimizing the surface of restricted native access

[Jorn Vernee]

Hi Ty,

Response inline...

On 29/11/2019 12:21, Ty Young wrote:
>
>
> On 11/29/19 3:07 AM, Maurizio Cimadamore wrote:
>> Hi Ty,
>> without entering in the specifics of your example - let's say you 
>> know that getpid() is at address 42 (of course the address will be 
>> much bigger, and even be a 'negative' Java long in reality).
>>
>> With the proposed model, you would be able to do:
>>
>> MemoryAddress pid = MemoryAddress.ofLong(42);
>>
>> And then you would be able to obtain a MethodHandle out of that, 
>> using the SystemABI:
>>
>> MethodHandle handle = SystemABI.getInstance().downcallHandle(pid, 
>> MethodType.methodType(int.class), FunctionDescriptor.of(C_INT));
>>
>> Then you can call the handle, normally.
>>
>> The only "unsafe" operation there is here, is the call to SystemABI 
>> to obtain the MethodHandle. This is a place where the API has to 
>> trust the developer because:
>>
>> - the entry point address could be invalid
>> - the function descriptor could be invalid leading to wrong calling 
>> sequence
>>
>> In all these cases the results of calling an ill-formed handle would 
>> be unpredictable (and there's no way to make this API safe - C 
>> libraries w/o debugging info have zero info about function 
>> signatures, so there's no way to check things - which leaves us in 
>> the best effort-land **) - some of these situations can also arise 
>> with JNI if the signature of the native method and the underlying C 
>> header/impl are out of sync.
>>
>> But the memory access API (in its ABI-ready incarnation) should give 
>> you a way to at least construct a pointer to the desired function.
>
>
> Thanks for the clarification.
>
>
> Are there any plans to make jdk.internal.foreign a default export? 
> Currently it's one of the seemingly very few "internal" modules that 
> aren't exported and I'll have to recompile from source with it 
> exported in order to use it.

None of the packages with 'internal' in the name will be exported, since 
they are not part of the public API, but an implementation detail. This 
naming pattern is followed in many places in the JDK (e.g. java.base has 
a bunch of these packages [1], which are not exported by default [2]).

Just like reflectively changing 'private' fields of a class is 
ill-advised, because you can end up going around invariant 
checks/assumptions and break the functioning of the class, you can break 
invariants by directly accessing an implementation detail in an internal 
package. So, no, this package will not be exported by default (just like 
we don't make all our fields 'public'). This is also important for 
keeping the option open of changing the implementation, which is much 
harder if users outside of the JDK depend on that implementation.

Would you wish to access an internal package any ways, you would need to 
provide an additional command line flag (--add-exports ...) to break the 
encapsulation (similar to setAccessible(true) for hacking into a private 
field). By providing the command line flag you explicitly except the 
risk of breaking the VM, and the risk that the code you depend on can 
change it's behavior or shape at any time, or becomes inaccessible 
altogether. i.e. it's similar to removing a "warranty void if removed" 
sticker found on some consumer electronics.

Of course, if you're making a library that depends on internal APIs, the 
users of your library must also explicitly opt into using these 
implementation details (through your library), and thus also explicitly 
accept the risks tied to the use of internal code. While you may be 
comfortable with removing the 'sticker', your clients might not be, so 
they again have to opt in.

Jorn

[1] : 
https://github.com/openjdk/jdk/tree/master/src/java.base/share/classes/jdk/internal
[2] : 
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/module-info.java

>>
>> Maurizio
>>
>> (**) The VM already has a nice diagnostic option called "-Xcheck:jni" 
>> which performs a variety of checks; but, again, this is a best-effort 
>> attempt and cannot detect all possible things that can go wrong (such 
>> as mismatches between Java signatures and C signatures).