[foreign-abi] RFR: 8237585: Dismantle ForeignUnsafe (foreign-abi part)

[Maurizio Cimadamore]

On Tue, 25 Feb 2020 15:03:35 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> Removes ForeignUnsafe, moves `getUnsafeBase` and `getUnsafeAccess` to `MemoryAddress`, and moves `ofNativeUnchecked` to `MemorySegment`.
> 
> I've added a warning about the safety of these methods to the javadoc.
> 
> Use of the methods is guarded by a check against a system property, `jdk.incubator.foreign.permitUnsafeInterop` for the first 2, and `jdk.incubator.foreign.permitUncheckedSegments` for the latter. This needs to be set to `true` when running an application, or otherwise the use of these methods results in an `IllegalAccessError` being thrown.
> 
> I've updated the test accordingly, and also remove the FASTPATH option from the SysV ABI implementation, since it was not actually being used, but still causing the tests to be run multiple times.

The code moves look fine, but I'd like to consolidate everything under a single flag instead of using two. 

I'd also like to see something similar to what was done for illegal access in modules, where we can specify different behavior - e.g. permit, warn, error.

Let's just use a single flag `jdk.incubator.foreign.restrictedMethods=permit/deny/warn/debug`

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java line 154:

> 153:      * <p>
> 154:      * This method is <em>unsafe</em>. It's use can result in putting the VM in a corrupt state when used incorrectly,
> 155:      * and is provided solely to cover use-cases that can not otherwise be addressed safely. When used incorrectly, there

Typo: `It's` -> `Its` (same in others)

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java line 156:

> 155:      * and is provided solely to cover use-cases that can not otherwise be addressed safely. When used incorrectly, there
> 156:      * are no guarantees made about the behaviour of the program. Particularly, incorrect use is not guaranteed to
> 157:      * result in a VM crash, but might instead silently cause memory to be corrupted.

I'd replace `Particularly, incorrect use is not guaranteed to result in a VM crash, but might instead silently cause memory to be corrupted.` with `More specifically, incorrect uses of this method might result in a JVM crash or, worse, might silently result in memory corruption.`

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



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