[foreign] RFR 8212987 : Binder should allows negative values for native pointers

[Jorn Vernee]

Ok, I have one more addition to this. I figured out the bug I was seeing 
before; it turns out I was passing a double offset.

I've removed the hackish anonymous class, and removed 
BoundedMemoryRegion.EVERYTHING for now since the length is just not big 
enough to cover all cases. Instead I'm now creating a new memory region 
at the requested offset for BoundedPointer. I looked into treating 
`length` as unsigned as well but this was pretty tricky, since to 
represent the length of EVERYTHING you need to have a 65-bit number. So 
I was working with lengthMinusOne everywhere but that was a lot more 
confusing. I think what I have right now is better than that.

Updated webrev: 
http://cr.openjdk.java.net/~jvernee/panama/webrevs/8212987/webrev.07/

Thanks for all the reviews and discussion so far.

Cheers,
Jorn

Jorn Vernee schreef op 2018-11-01 17:42:
> Henry Jen schreef op 2018-11-01 16:21:
>> I was hesitate to reply as the fix is kind of hackish, but not ready
>> to suggest we go all the way to enable full range of 64-bit, after
>> all, Long.MAX_VALUE is giving us 8EB and that’s seems way more than
>> enough. But to completely address rather than simply make it work for
>> special cases, we probably should just treat address as unsigned and
>> use Long.compareUnsigned to do comparison.
> 
> It feels kind of hackish to me too tbh, especially since I can only do
> partial testing. But if all the tests pass on your side as well then I
> see no problem for now. If this is not the case then I will leave it
> for now and focus on something else until I also have the full test
> suite running as well.
> 
> As an interim alternative for dealing with special values maybe we
> could add a 'dummy' pointer type that is used for negative values, but
> does not support getting/setting and is really only a wrapper around a
> `long`. The way to use it would be to pass it back to native which
> _can_ handle the negative value. But that is not a long term solution,
> since some systems use negative pointers as normal, as pointed out by
> Florian [1].
> 
>> Therefore, max length for a memory always 0 - min. The length should
>> be Long.compareUnsigned(length, maxLength) <= 0, with all other offset
>> operation compare against length established at construction time with
>> Long.compareUnsigned, that should be enough to deal with this.
> 
> OK, I will use Long.compareUnsigned to do the check.
> 
>> However, taking offset with large(negative) number seems really 
>> confusing.
> 
> Yes, so hopefully in almost all cases this will be abstracted away,
> and not really visible. `long` is kind of a sub-optimal carrier type
> for a pointer value for this reason I think. It's OK to store the
> pointer, but things get confusing when you try to look at it, since it
> will show up as a signed integer. But maybe once we have value types
> we could consider having an UnsignedLong type which has the right
> semantics and representation to it?
> 
> Here is a version that uses Long.compareUnsigned to do the overflow
> check:
> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8212987/webrev.06/
> 
> There are 2 lines of factory methods; the first which calculate the
> length and then pass args to a private internal factory method, the
> others that check the input length for overflow and then pass args to
> the internal factory. The internal factory does not do the overflow
> check.
> 
> Jorn
> 
>> What do you think?
>> 
>> Cheers,
>> Henry
> 
> [1] : 
> http://mail.openjdk.java.net/pipermail/panama-dev/2018-October/002995.html