On Fri, 23 Oct 2020 21:38:16 GMT, Jorn Vernee <jver...@openjdk.org> wrote:

>> This approach does not work for reference types, since they are erased to 
>> `Object`, and then exact checking will be performed on the erased reference 
>> types.
>> 
>> For example try this:
>> 
>> import java.lang.invoke.MethodHandles;
>> import java.lang.invoke.VarHandle;
>> 
>> public class Test {
>>     int x;
>> 
>>     public static void main(String[] args) throws Throwable {
>>         VarHandle x = MethodHandles.lookup().findVarHandle(Test.class, "x", 
>> int.class);
>>         VarHandle ex = x.asExact();
>>         Test t = new Test();
>>         ex.set(t, 1);
>>     }
>> }
>> 
>> Which results in:
>> 
>> Exception in thread "main" java.lang.invoke.WrongMethodTypeException: 
>> expected (Object,int)void but found (Test,int)void
>>      at Test.main(Test.java:11)
>> 
>> Exact type checking requires that match be performed on the VH access mode 
>> method type and the exact symbolic method type, something like:
>> 
>>     final static Object guard_L_L(VarHandle handle, Object arg0, 
>> VarHandle.AccessDescriptor ad) throws Throwable {
>>         if (handle.vform.exact && handle.accessModeType(ad.type) != 
>> ad.symbolicMethodTypeExact) {
>>             throw new WrongMethodTypeException("expected " + 
>> handle.vform.methodType_table_exact[ad.type] + " but found "
>>                     + ad.symbolicMethodTypeExact);
>>         }
>> 
>> Then it's precisely the same as `MH.invokeExact`, rather than `MH.invoke`.
>> 
>> A `VarForm` is a resource shared across many instances of the same _kind_ of 
>> `VarHandle`, so cannot be used for exact matching, except in specific 
>> scenarios e.g. access on a primitive array.
>
> @PaulSandoz Thanks. I initially tested this with memory access VarHandles, 
> which don't erase the receiver type. e.g.
> 
> MemoryLayout layout = MemoryLayout.ofSequence(10, JAVA_INT);
> VarHandle vh = layout.varHandle(int.class, 
> MemoryLayout.PathElement.sequenceElement());
> vh = vh.asExact();
> try (MemorySegment segment = MemorySegment.allocateNative(layout)) {
>     for (int i = 0; i <10; i++) {
>         vh.set(segment.baseAddress(), i, i);
>     }
> }
> 
> Will result in:
> Exception in thread "main" java.lang.invoke.WrongMethodTypeException: 
> expected (MemoryAddress,long,int)void but found (MemoryAddress,int,int)void
>       at 
> java.base/java.lang.invoke.VarHandleGuards.guard_LII_V(VarHandleGuards.java:915)
>       at main.Main.main(Main.java:18)
> 
> Which led me to believe the approach would work for other reference types. 
> But, I suppose the MethodTypes fed to memaccess VarForms are non-erased as an 
> exception rather than a rule.
> 
> I'll update the patch and sharpen the tests to check that the actual expected 
> type is correct (per the exception message).

@PaulSandoz I've implemented your suggestion. FWIW, the VH::accessModeType 
method took and AccessMode value as an argument, and the AccessDescriptor only 
stored the ordinal, so I added an `@Stable` array of values to AccessMode to 
map from ordinal to enum value. But, maybe it makes more sense to just store 
the AccessMode in the AccessDescriptor directly? (instead of the ordinal). Not 
sure what the original motivation was for only storing the ordinal.

I've also sharpened the tests to check the exception message. Do you think the 
testing is sufficient? (Note that I did not add tests to the template files 
since only a select set of argument type conversions causes the WMTE we're 
looking for. So, that's why I created a new test file).

FWIW, there seems to have been a bug in the implementation of 
IndirectVarHandle::accessModeTypeUncached, where it was using the VarHandle's 
type as the receiver argument (unlike all the other impls). I've fixed this by 
passing `null` instead, and also removed a workaround for this problem in 
VarHandles::maybeAdapt.

-------------

PR: https://git.openjdk.java.net/jdk/pull/843

Reply via email to