[foreign] 8220296: Memory access should have layout with explicit endianness

[Henry Jen]

> On Mar 7, 2019, at 4:14 PM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
> 
> Ok, I re-read your changeset, and I might have an idea of what happened:
> 
> You started off with adding the exception in BoundedPointer (yes!).
> 
> Some tests (a lot?) started to fail.
> 

You can give it a try, I might be biased, but I believe that when a type realized on the system, the endianness is determined, thus I have implemented that since beginning.

As I was starting, I tried allocate, cast and places to add endianness, but there are just too many scenarios where user need to give a LayoutType, and very likely that would be without an endianness, especially when often used NativeTypes without endianness.

> You retracted to a position where creating a LayoutType will inject some endianness into the layout.
> 
> I can see that this solves the test problem - but this doesn't help with the scenario I mentioned in the previous email (same code behaving differently in different archs) and I believe that renders the BoundedPointer check useless (since now all LayoutType acquire _some_ endianness, by construction).
> 

Like you said, any memory access should have endianness. I don’t know how to explain that when I say an int32 without endianness, it should be default to the host endianness. If I do know I want a specific endianness, then I will specify that.

I can certainly try to add some more constants like NativeTypes.HE_UNT32, but I thought we don’t want to go there based on earlier iteration.

> The checks start like this:
> 
>  if (type.layout() instanceof Value) {
> 
> Now, 'type' is a LayoutType - if a LayoutType's layout always gets some endianness injected (as per your patch), when would the check in BoundedPointer fail?
> 

Right, this is more serve as an assert to make sure we have explicit endianness.

Cheers,
Henry


> Maurizio
> 
> On 07/03/2019 23:58, Maurizio Cimadamore wrote:
>> I don't get what problem you are trying to address - can you please expand further, and maybe provide some examples? 
>> 
>> To me, it looks like this is moving us away from the target we have set for ourselves. 
>> 
>> 1) NE = not endianness (that is, endianness is an optional property of layouts) 
>> 
>> 2) accessing memory w/o explicit endiannes is an error (note this is not even possible with byte buffer var handle, so there's a clear precedent) 
>> 
>> 3) only place where NE is tolerated is functions, where in fact endianness is redundant 
>> 
>> What you do on this patch is bad - because you can write the same code (e.g. read memory from NE layout and copy to a LE layout) and it will behave differently on different platforms, which is the very thing we want to avoid. 
>> 
>> Maurizio 
>> 
>> On 07/03/2019 19:24, Henry Jen wrote: 
>>> Hi, 
>>> 
>>> After spending quite some time to plug holes everywhere endianness need to be localized(allocation, cast, etc…), I think it’s best to be simple and consistent, and simply enforce LayoutType to have explicit endianness. 
>>> 
>>> This is pretty close to what I came out at very beginning, any NE is really just the host endianness. However, in this edition, the “localize” of endianness happens in LayoutType, not Layout. 
>>> 
>>> With that change, we enforce that function should not have byte-swapped layout. 
>>> 
>>> The webrev[1] is up for review, let me know what you think. 
>>> 
>>> Cheers, 
>>> Henry 
>>> 
>>> [1] http://cr.openjdk.java.net/~henryjen/panama/8220296/0/webrev/