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

[Maurizio Cimadamore]

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 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).

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?

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/