[foreign-abi] RFR 8236004: Memory access var handles should support MemoryAddress carrier

[Maurizio Cimadamore]

New patch (rebased):

http://cr.openjdk.java.net/~mcimadamore/panama/8236004_v4

I've removed some of the smart logic for detecting pointer size in 
Utils::POINTER_SIZE. The reason being that, currently, the VarHandle 
assumes that addresses are fetches as longs (e.g. Unsafe::getLong) 
anyway, so there's no way the current generated var handle will work on, 
say, 32-bit pointers. If we want to add support for smaller addresses, 
we will need to make the code generation scheme more nuanced. I think 
what we have is good for now though (as all the ABIs we support ATM 
feature 64-bit pointers)

Maurizio



On 18/12/2019 13:42, Maurizio Cimadamore wrote:
>
> On 18/12/2019 12:42, Jorn Vernee wrote:
>> Hi,
>>
>> Some comments:
>>
>> AddressVarHandleGenerator.java:
>> - `erase` method seems to be indented too far.
> That was kind of deliberate, to suggest that it was only used in the 
> constructor (we do this in other parts of the JDK).
>>
>> X-VarHandleMemoryAddressView.java.template:
>> - In the long2addr method I'd prefer using invokeExact instead of 
>> invoke, to make sure the call always gets inlined.
>>
>> LayoutPath.java:
>> - In dereferenceHandle, can the carrier check here be made to use 
>> Utils as well?
> Will fix these
>>
>> Rest looks good. (Casing over the ABI impls in Utils to get the 
>> POINTER_SIZE seems pretty unfortunate, but that was already on the 
>> todo list I believe).
>>
>> Also, the patch does not apply cleanly (probably due to the manual 
>> merge with foreign-memaccess I just did, sorry), so you might want to 
>> rebase.
>
> Yeah - I'm doing a rebase; after that I'll submit another iteration
>
> Cheers
> Maurizio
>
>>
>> cheers,
>> Jorn
>>
>> On 18/12/2019 12:03, Maurizio Cimadamore wrote:
>>> Here's a new revision which restores erased carrier and access types:
>>>
>>> http://cr.openjdk.java.net/~mcimadamore/panama/8236004_v3/
>>>
>>> Thanks for taking the time of retracing the steps and explaining how 
>>> linking is supposed to work; I feel I now have a much better 
>>> understanding of how things are connected.
>>>
>>> Maurizio
>>>
>>> On 17/12/2019 19:01, Paul Sandoz wrote:
>>>>
>>>>
>>>>> On Dec 17, 2019, at 10:47 AM, Maurizio Cimadamore 
>>>>> <maurizio.cimadamore at oracle.com 
>>>>> <mailto:maurizio.cimadamore at oracle.com>> wrote:
>>>>>>
>>>>>> Yes, it works fine, until you pass an instance of something that 
>>>>>> is not of MemoryAddressProxy e.g. pass in an instance of String, 
>>>>>> which will get spoofed as an instance of MemoryAddressProxy.  The 
>>>>>> guards will only check if the object is a reference type.
>>>>>
>>>>> Ok, I see what you mean, the problem is not that linking occurs 
>>>>> abnormally, the problem is that the underlying linkage is not 
>>>>> sound (in the absence of Java casts)
>>>>>
>>>> Yes, the linkage + invocation runtime checks are not sound.
>>>>
>>>>> I guess this means that same treatment should also be applied to 
>>>>> the MemoryAddressProxy parameter (e.g. of a VarHandle::set 
>>>>> operation) right?
>>>>>
>>>>
>>>> Yes, for all access modes where the arg of $type$ is 
>>>> of MemoryAddressProxy a cast check is required (which should 
>>>> optimize away).
>>>>
>>>> I think that can be performed by the addr2long method?
>>>>>
>>>> Paul.