[foreign] RFR 8217414: Remove bounds check in BoundedPointer constructor

[Maurizio Cimadamore]

On 21/01/2019 13:12, Jorn Vernee wrote:
> There is a `checkAlive()` that gets called when a pointer is unboxed 
> [1], I thought you were talking about that check. But, I don't think 
> we should do a bounds check before passing a pointer to native code. 
> The "one past the end of a container" pointer idiom is common and some 
> APIs might expect to be passed such a pointer. It seems too 
> restrictive to require that pointers passed to native code have to be 
> in bounds (tbh I have my doubts about the liveliness check as well).

The whole point of the Panama API is to provide enriched safety. Now, if 
users want to opt out of that, we might (in the future) provide knobs to 
do so, but I see no point for passing a pointer that is not alive around 
(given it is known to contain garbage). Similarly, I see very little 
point in passing a pointer that is out of bounds - granted there might 
be few use cases for that (e.g. pass a 'limit' pointer to a library), 
but I'd prefer to stick with stricter semantics and loosen that up on a 
use case by use case basis.

I think the main point of this patch should be to _delay_ the check that 
is currently done on the constructor, not to remove it completely. It is 
ok if some piece of code temporarily produces an out of bound pointers 
(e.g. when iterating), but I think we should limit the exposure of that 
weird pointer to the rest of the machinery.

Maurizio

>
> Jorn
>
> [1] : 
> http://hg.openjdk.java.net/panama/dev/file/tip/src/java.base/share/classes/jdk/internal/foreign/memory/BoundedPointer.java#l75
>
> Maurizio Cimadamore schreef op 2019-01-21 13:56:
>> I agree that the check should removed from the constructor - but we
>> should make sure that checks happen when the pointer is used.
>> Dereference is one situation, function call is another. I guess
>> dereference is already covered (after all, a memory read will fail if
>> outside the boundaries), but I don't think passing an out of bound
>> pointer to a function is currently being checked, and it should if we
>> remove the constructor eager check
>>
>> Maurizio
>>
>> On 21/01/2019 12:00, Jorn Vernee wrote:
>>> Hi,
>>>
>>> I have a patch addressing 2 vacuously passing tests (api/PointerTest 
>>> and api/ArrayTest). The patch fixes the tests, and to make that 
>>> happen removes the bounds check in BoundedPointer's constructor. As 
>>> discussed offline; this check is not needed since we already de a 
>>> check when dereferencing a pointer in BoundedMemoryRegion, and the 
>>> check in the constructor is overly restrictive since it should be 
>>> fine to create out-of-bounds pointers as long as it is not 
>>> dereferenced. This does mean the tests needed to be tweaked since 
>>> now an access exception is throw at a later point.
>>>
>>> Please review the following.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8217414
>>> Webrev: 
>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8217414/webrev.00
>>>
>>> Thanks,
>>> Jorn