[foreign] RFR 8218153: Read/Write of Value layout should honor declared endianness

[Maurizio Cimadamore]

On 02/02/2019 06:32, Henry Jen wrote:
> On Feb 1, 2019, at 9:49 PM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>> Looks good - I have some (minor) points/questions:
>>
>> * How comfortable are we to always infer platform endianness for Address layout? Should that be explicit too? (while for real pointers that probably doesn't matter, for protocol schema addresses, an address could be just some index, hence stored in LE/BE format?)
>>
> I won’t mind to remove the default endianness constructs like we do for Value, after all, it’s a similar argument.
>
>> * We could have utility helper methods for extracting signedness and byte-reversal booleans out of a Layout - the same code is replicated in a number of places
> Do you mean Util.isSigned(Layout) and Util.isPlatformEndian(Layout)? or Value.isSigned() and Value.isNativeByteOrder()?
>
>> * I'd like to understand more about DirectNativeInvoker limitations (mostly because this will carry over to linkToNative too) - what exactly goes wrong if we allow mismatching endianness there? Let me unpack this a bit: the direct invoker is only concerned in those calls that occur via registers only - why is endianness a factor here - since we are never storing into memory?
>>
> DNI will call the JNI methods with the Java argument value. But when we specify a function signature with a different endian from platform, because the value is not going through Reference we won’t get the proper value. The EndianTest.java:322 illustrated such use case.
>
>> * Why is jextract also inferring platform endianness? Isn't the clang API powerful enough for that? I wonder what happens if an header is extracted in a VM that is running in a different environment.
>>
> jextract runs on a platform should spit out that platform endianness, just as the native type, of course unless we are considering cross-compiling cases. libclang deal with type, I don’t think it worries about endianness.
>
> Cheers,
> Henry
>
>
>> Maurizio
>>
>> On 02/02/2019 01:36, Henry Jen wrote:
>>> Updated webrev[1], the Value.Endianness class is basically same as java.nio.ByteOrder, they should probably be consolidated.
>>>
>>> The change is mainly to get rid of implicit endianness construction of Value instances, which is good to force considering of the correct endianness of the type. Removed INT16/32/64 from NativeTypes.
>>>
>>> Existing usage of INT16/32/64 are reviewed and
>>> - changed for platform endianness when the type is expected for native function, where are expected to be platform endian.
>>> - changed to LE version if it’s pure Java(where as long as same endianness is used, that should just work) or not relevant.
>>> - In some cases, changed to INT if native C type int is expected.
>>>
>>> [1] https://cr.openjdk.java.net/~henryjen/panama/endianness/05/webrev/
>>>
>>> Cheers,
>>> Henry
>>>
>>>
>>>> On Feb 1, 2019, at 2:12 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>>>>
>>>> Yes, or even use LE types everywhere, since that's what we support right now anyway, and when we have better higher-level types we'll do the switch.
>>>>
>>>> Maurizio
>>>>
>>>> On 01/02/2019 02:24, Henry Jen wrote:
>>>>> So we are saying I revert back to have the
>>>>>
>>>>> final static INT16 = ByteOrder.naturalOrder() == ByteOrder.BIG_ENDIAN ? BE_INT16 : LE_INT16 in user’s code, that’s in EndiannessTest.java in this patch, and eventually all code where we are writing now to get JVM compatible INT16, INT32, INT64. Luckily we probably only need C types available now via SystemtemABI in most places.
>>>>>
>>>>> Cheers,
>>>>> Henry
>>>>>
>>>>>
>>>>>> On Jan 31, 2019, at 5:51 PM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>>>>>>
>>>>>>
>>>>>> On 01/02/2019 01:45, Henry Jen wrote:
>>>>>>> We cannot just drop INT16, name it as PE_INT16 is fine. That’s the them of the discussion, we need something programmatically portable on JVM.
>>>>>> I would question the use of the term "need" here - longer term? Sure - but I can defo see the API not having platform-dependent layout types in the short term.
>>>>>>
>>>>>> In  other words, there are two parts to this problem; one is to take endianness into account when accessing memory, which you have done. The second is to provide a reasonable kit of starter layout types for people to use. These two problems don't have (and shouldn't) to be tackled at the same time.
>>>>>>
>>>>>> John suggested to go for all explicit mode - I'd say let's stick with that plan, and see what use cases will crop up in common code, and find ways (John's static import idea is a fine one - there could be others) to address them accordingly.
>>>>>>
>>>>>> Maurizio
>>>>>>
>>>>>>> Cheers,
>>>>>>> Henry
>>>>>>>
>>>>>>>
>>>>>>>> On Jan 31, 2019, at 5:13 PM, John Rose <john.r.rose at oracle.com> wrote:
>>>>>>>>
>>>>>>>> On Jan 31, 2019, at 5:00 PM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>>>>>>>>> I'm saying make everything explicit - and drop INT16 at least for now, for either LE_INT16, or some other scheme.
>>>>>>>>>
>>>>>>>> That would be fine with me also, of course.