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

[Maurizio Cimadamore]

On 17/12/2019 16:57, Paul Sandoz wrote:
>
>
>> On Dec 17, 2019, at 2:28 AM, Maurizio Cimadamore 
>> <maurizio.cimadamore at oracle.com 
>> <mailto:maurizio.cimadamore at oracle.com>> wrote:
>>
>>
>> On 17/12/2019 01:07, Paul Sandoz wrote:
>>> Looking good but sorry I read the code incorrectly and confused 
>>> myself and lead you down the wrong path as a result. Still swapping 
>>> back all this stuff :-)
>>>
>>> I originally thought the VarHandle impls were incorrect generating 
>>> the accessModeTypeUncached methods returning method types with 
>>> erased components.
>>>
>>> I think we need to revert back to Object in 
>>> X-VarHandleMemoryAddressView.java.template
>>>
>>>      @ForceInline
>>> -    static MemoryAddressProxy checkAddress(Object obb, long offset, 
>>> long length, boolean ro) {
>>> -        MemoryAddressProxy oo = 
>>> (MemoryAddressProxy)Objects.requireNonNull(obb);
>>> +    static MemoryAddressProxy checkAddress(MemoryAddressProxy obb, 
>>> long offset, long length, boolean ro) {
>>> +        MemoryAddressProxy oo = Objects.requireNonNull(obb);
>>>
>>>          oo.checkAccess(offset, length, ro);
>>>          return oo;
>>>      }
>>>
>>>
>>>      @ForceInline
>>> -    static $type$ getVolatile0(VarHandleMemoryAddressBase handle, 
>>> Object obb, long base) {
>>> +    static $type$ getVolatile0(VarHandleMemoryAddressBase handle, 
>>> MemoryAddressProxy obb, long base) {
>>>
>>>          MemoryAddressProxy bb = checkAddress(obb, base, 
>>> handle.length, true);
>>>          return convEndian(handle.be,
>>>                            UNSAFE.get$RawType$Volatile(
>>>                                    bb.unsafeGetBase(),
>>>                                    offset(bb, base, handle.alignment)));
>>>      }
>>>
>>> Although it's fixed in your case the VarHandles linking machinery 
>>> operates on erased signatures.  Now I recall one major reason why: 
>>> it's so that invocation behaves similar to MH.invoke rather than 
>>> MH.invokeExtact, thus making it easier to pass subtypes without 
>>> having to declare an explicit cast at the call site (which would 
>>> have made it far more painful for adoption).  As a consequence the 
>>> VarHandle implementations must perform checks on ref types.
>>
>> I did a benchmark and it seems like everything is linked correctly 
>> (through the guards) as before?
>>
>
> 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).

I guess this means that same treatment should also be applied to the 
MemoryAddressProxy parameter (e.g. of a VarHandle::set operation) right?

Maurizio

>
> For example:
>
> @ForceInline
> @LambdaForm.Compiled
> final static Object guard_L_L(VarHandle handle, Object arg0, 
> VarHandle.AccessDescriptor ad) throws Throwable {
>     if (handle.vform.methodType_table[ad.type] == 
> ad.symbolicMethodTypeErased) {
>         Object r = MethodHandle.linkToStatic(handle, arg0, 
> handle.vform.getMemberName(ad.mode));
>         return ad.returnType.cast(r);
>     }
>     else {
>         MethodHandle mh = handle.getMethodHandle(ad.mode);
>   return mh.asType(ad.symbolicMethodTypeInvoker).invokeBasic(handle, 
> arg0);
>     }
> }
>
> The fast path checks ref equality of the erased signatures (the 
> expected from the vform compared to that at the call site, then passes 
> on the arguments to the internal linkToStatic which performs no type 
> checking (see also documentation on MH.invokeBasic).
>
> This enables VH access usage in say ForkJoin code to pass instances 
> ForkJoinTask or null without explicit casts i.e. MH.invoke semantics 
> but without the cost of an asType transformation.  The guard falls 
> back to asType when the erased signatures differ e.g. primitive 
> conversion.
>
> I think that now properly answers your question about how the heck 
> does the MemoryAddressProxy trick work! The VHs consume instances of 
> MemoryAddressProxy, users pass in instances of MemoryAddress, and 
> MemoryAddressImpl is on the only implementation of both interfaces.
>
>
>> And, if we erase the receiver type to Object, then, shouldn't we do 
>> the same for the VarHandle carrier (e.g. Object, not MemoryAddress?)
>>
>
> The VarHandle instance (the receiver) gets passed as the first 
> argument to the method when being linked (in the up call to the 
> MethodHandleNatives method).  We know in that case it's always a 
> VarHandle instance.   Same for the appendix type passed as the last 
> argument.
>
> Here us the snippet of code in 
> MerthodHandleNatives.varHandleOperationLinkerMethod:
>
> // Get the guard method type for linking
> final Class<?>[] guardParams = new Class<?>[sigType.parameterCount() + 2];
> // VarHandle at start
> guardParams[0] = VarHandle.class;
> for (int i = 0; i < sigType.parameterCount(); i++) {
>     guardParams[i + 1] = sigType.parameterType(i);
> }
> // Access descriptor at end
> guardParams[guardParams.length - 1] = VarHandle.AccessDescriptor.class;
> MethodType guardType = MethodType.makeImpl(guardReturnType, 
> guardParams, true);
>
>
> Hth,
> Paul.