8256189: Exact VarHandle tests should test withInvokeBehavior() works as expected

Reviewed-by: mcimadamore, chegar

Author: JornVernee

src/java.base/share/classes/java/lang/invoke/X-VarHandleMemoryAccess.java.template

@@ -39,7 +39,7 @@ import static java.lang.invoke.MethodHandleStatics.UNSAFE;
 final class MemoryAccessVarHandle$Type$Helper extends MemoryAccessVarHandleBase {
 
     static final boolean BE = UNSAFE.isBigEndian();
-    
+
     static final ScopedMemoryAccess SCOPED_MEMORY_ACCESS = ScopedMemoryAccess.getScopedMemoryAccess();
 
     static final int VM_ALIGN = $BoxType$.BYTES - 1;
@@ -66,7 +66,7 @@ final class MemoryAccessVarHandle$Type$Helper extends MemoryAccessVarHandleBase
     public MemoryAccessVarHandle$Type$Helper withInvokeBehavior() {
         return !hasInvokeExactBehavior() ?
                 this :
-                new MemoryAccessVarHandle$Type$Helper(skipAlignmentMaskCheck, be, length, alignmentMask, true);
+                new MemoryAccessVarHandle$Type$Helper(skipAlignmentMaskCheck, be, length, alignmentMask, false);
     }
 
 #if[floatingPoint]

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/Utils.java

@@ -38,11 +38,17 @@
 import java.util.Optional;
 import java.util.function.Supplier;
 
+import static sun.security.action.GetPropertyAction.*;
+
 /**
  * This class contains misc helper functions to support creation of memory segments.
  */
 public final class Utils {
 
+    // used when testing invoke exact behavior of memory access handles
+    private static final boolean SHOULD_ADAPT_HANDLES
+        = Boolean.parseBoolean(privilegedGetProperty("jdk.internal.foreign.SHOULD_ADAPT_HANDLES", "true"));
+
     private static final String foreignRestrictedAccess = Optional.ofNullable(VM.getSavedProperty("foreign.restricted"))
             .orElse("deny");
 
@@ -72,7 +78,9 @@ public static long bitsToBytesOrThrow(long bits, Supplier<RuntimeException> exFa
     public static VarHandle fixUpVarHandle(VarHandle handle) {
         // This adaptation is required, otherwise the memory access var handle will have type MemorySegmentProxy,
         // and not MemorySegment (which the user expects), which causes performance issues with asType() adaptations.
-        return MemoryHandles.filterCoordinates(handle, 0, SEGMENT_FILTER);
+        return SHOULD_ADAPT_HANDLES
+            ? MemoryHandles.filterCoordinates(handle, 0, SEGMENT_FILTER)
+            : handle;
     }
 
     private static MemorySegmentProxy filterSegment(MemorySegment segment) {

test/jdk/java/lang/invoke/VarHandles/VarHandleTestExact.java

@@ -24,16 +24,31 @@
 /*
  * @test
  * @modules jdk.incubator.foreign
+ *          java.base/jdk.internal.access.foreign
  *
- * @run testng/othervm -Xverify:all VarHandleTestExact
- * @run testng/othervm -Xverify:all -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=true VarHandleTestExact
- * @run testng/othervm -Xverify:all -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=false VarHandleTestExact
- * @run testng/othervm -Xverify:all -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=true VarHandleTestExact
+ * @run testng/othervm -Xverify:all
+ *   -Djdk.internal.foreign.SHOULD_ADAPT_HANDLES=false
+ *   VarHandleTestExact
+ * @run testng/othervm -Xverify:all
+ *   -Djdk.internal.foreign.SHOULD_ADAPT_HANDLES=false
+ *   -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=true
+ *   -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=true
+ *   VarHandleTestExact
+ * @run testng/othervm -Xverify:all
+ *   -Djdk.internal.foreign.SHOULD_ADAPT_HANDLES=false
+ *   -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false
+ *   -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=false
+ *   VarHandleTestExact
+ * @run testng/othervm -Xverify:all
+ *   -Djdk.internal.foreign.SHOULD_ADAPT_HANDLES=false
+ *   -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false
+ *   -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=true
+ *   VarHandleTestExact
  */
 
-import jdk.incubator.foreign.MemoryAddress;
 import jdk.incubator.foreign.MemoryHandles;
 import jdk.incubator.foreign.MemorySegment;
+import jdk.internal.access.foreign.MemorySegmentProxy;
 import org.testng.SkipException;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
@@ -46,6 +61,7 @@
 import java.nio.ByteOrder;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.function.Consumer;
 
 import static org.testng.Assert.*;
 
@@ -80,24 +96,12 @@ public void testExactSet(String fieldBaseName, Class<?> fieldType, boolean ro, O
             throws NoSuchFieldException, IllegalAccessException {
         if (ro) throw new SkipException("Can not test setter with read only field");
         VarHandle vh = MethodHandles.lookup().findVarHandle(Widget.class, fieldBaseName + "_RW", fieldType);
-        assertFalse(vh.hasInvokeExactBehavior());
         Widget w = new Widget();
 
-        try {
-            vh.set(w, testValue);
-            vh.withInvokeBehavior().set(w, testValue);
-        } catch (WrongMethodTypeException wmte) {
-            fail("Unexpected exception", wmte);
-        }
-
-        vh = vh.withInvokeExactBehavior();
-        assertTrue(vh.hasInvokeExactBehavior());
-        try {
-            setter.set(vh, w, testValue); // should throw
-            fail("Exception expected");
-        } catch (WrongMethodTypeException wmte) {
-            assertMatches(wmte.getMessage(),".*\\Qexpected (Widget," + fieldType.getSimpleName() + ")void \\E.*");
-        }
+        doTest(vh,
+            tvh -> tvh.set(w, testValue),
+            tvh -> setter.set(tvh, w, testValue),
+            ".*\\Qexpected (Widget," + fieldType.getSimpleName() + ")void \\E.*");
     }
 
     @Test(dataProvider = "dataObjectAccess")
@@ -106,24 +110,12 @@ public void testExactGet(String fieldBaseName, Class<?> fieldType, boolean ro, O
                              SetStaticX staticSetter, GetStaticX staticGetter)
             throws NoSuchFieldException, IllegalAccessException {
         VarHandle vh = MethodHandles.lookup().findVarHandle(Widget.class, fieldBaseName + (ro ? "_RO" : "_RW"), fieldType);
-        assertFalse(vh.hasInvokeExactBehavior());
         Widget w = new Widget();
 
-        try {
-            Object o = vh.get(w);
-            Object o2 = vh.withInvokeBehavior().get(w);
-        } catch (WrongMethodTypeException wmte) {
-            fail("Unexpected exception", wmte);
-        }
-
-        vh = vh.withInvokeExactBehavior();
-        assertTrue(vh.hasInvokeExactBehavior());
-        try {
-            getter.get(vh, w); // should throw
-            fail("Exception expected");
-        } catch (WrongMethodTypeException wmte) {
-            assertMatches(wmte.getMessage(),".*\\Qexpected (Widget)" + fieldType.getSimpleName() + " \\E.*");
-        }
+        doTest(vh,
+            tvh -> tvh.get(w),
+            tvh -> getter.get(tvh, w),
+            ".*\\Qexpected (Widget)" + fieldType.getSimpleName() + " \\E.*");
     }
 
     @Test(dataProvider = "dataObjectAccess")
@@ -133,23 +125,11 @@ public void testExactSetStatic(String fieldBaseName, Class<?> fieldType, boolean
             throws NoSuchFieldException, IllegalAccessException {
         if (ro) throw new SkipException("Can not test setter with read only field");
         VarHandle vh = MethodHandles.lookup().findStaticVarHandle(Widget.class, fieldBaseName + "_SRW", fieldType);
-        assertFalse(vh.hasInvokeExactBehavior());
-
-        try {
-            vh.set(testValue);
-            vh.withInvokeBehavior().set(testValue);
-        } catch (WrongMethodTypeException wmte) {
-            fail("Unexpected exception", wmte);
-        }
 
-        vh = vh.withInvokeExactBehavior();
-        assertTrue(vh.hasInvokeExactBehavior());
-        try {
-            staticSetter.set(vh, testValue); // should throw
-            fail("Exception expected");
-        } catch (WrongMethodTypeException wmte) {
-            assertMatches(wmte.getMessage(),".*\\Qexpected (" + fieldType.getSimpleName() + ")void \\E.*");
-        }
+        doTest(vh,
+            tvh -> tvh.set(testValue),
+            tvh -> staticSetter.set(tvh, testValue),
+            ".*\\Qexpected (" + fieldType.getSimpleName() + ")void \\E.*");
     }
 
     @Test(dataProvider = "dataObjectAccess")
@@ -158,94 +138,74 @@ public void testExactGetStatic(String fieldBaseName, Class<?> fieldType, boolean
                                    SetStaticX staticSetter, GetStaticX staticGetter)
             throws NoSuchFieldException, IllegalAccessException {
         VarHandle vh = MethodHandles.lookup().findStaticVarHandle(Widget.class, fieldBaseName + (ro ? "_SRO" : "_SRW"), fieldType);
-        assertFalse(vh.hasInvokeExactBehavior());
-
-        try {
-            Object o = vh.get();
-            Object o2 = vh.withInvokeBehavior().get();
-        } catch (WrongMethodTypeException wmte) {
-            fail("Unexpected exception", wmte);
-        }
 
-        vh = vh.withInvokeExactBehavior();
-        assertTrue(vh.hasInvokeExactBehavior());
-        try {
-            staticGetter.get(vh); // should throw
-            fail("Exception expected");
-        } catch (WrongMethodTypeException wmte) {
-            assertMatches(wmte.getMessage(),".*\\Qexpected ()" + fieldType.getSimpleName() + " \\E.*");
-        }
+        doTest(vh,
+            tvh -> tvh.get(),
+            tvh -> staticGetter.get(tvh),
+            ".*\\Qexpected ()" + fieldType.getSimpleName() + " \\E.*");
     }
 
     @Test(dataProvider = "dataSetArray")
     public void testExactArraySet(Class<?> arrayClass, Object testValue, SetArrayX setter) {
         VarHandle vh = MethodHandles.arrayElementVarHandle(arrayClass);
         Object arr = Array.newInstance(arrayClass.componentType(), 1);
-        assertFalse(vh.hasInvokeExactBehavior());
-
-        try {
-            vh.set(arr, 0, testValue);
-            vh.withInvokeBehavior().set(arr, 0, testValue);
-        } catch (WrongMethodTypeException wmte) {
-            fail("Unexpected exception", wmte);
-        }
 
-        vh = vh.withInvokeExactBehavior();
-        assertTrue(vh.hasInvokeExactBehavior());
-        try {
-            setter.set(vh, arr, testValue); // should throw
-            fail("Exception expected");
-        } catch (WrongMethodTypeException wmte) {
-            assertMatches(wmte.getMessage(),
-                ".*\\Qexpected (" + arrayClass.getSimpleName() + ",int," + arrayClass.componentType().getSimpleName() + ")void \\E.*");
-        }
+        doTest(vh,
+            tvh -> tvh.set(arr, 0, testValue),
+            tvh -> setter.set(tvh, arr, testValue),
+            ".*\\Qexpected (" + arrayClass.getSimpleName() + ",int," + arrayClass.componentType().getSimpleName() + ")void \\E.*");
     }
 
     @Test(dataProvider = "dataSetBuffer")
     public void testExactBufferSet(Class<?> arrayClass, Object testValue, SetBufferX setter) {
         VarHandle vh = MethodHandles.byteBufferViewVarHandle(arrayClass, ByteOrder.nativeOrder());
-        assertFalse(vh.hasInvokeExactBehavior());
         ByteBuffer buff = ByteBuffer.allocateDirect(8);
 
+        doTest(vh,
+            tvh -> tvh.set(buff, 0, testValue),
+            tvh -> setter.set(tvh, buff, testValue),
+            ".*\\Qexpected (ByteBuffer,int," + arrayClass.componentType().getSimpleName() + ")void \\E.*");
+    }
+
+    @Test(dataProvider = "dataSetMemorySegment")
+    public void testExactSegmentSet(Class<?> carrier, Object testValue, SetSegmentX setter) {
+        VarHandle vh = MemoryHandles.varHandle(carrier, ByteOrder.nativeOrder());
+        try (MemorySegment seg = MemorySegment.allocateNative(8)) {
+
+            doTest(vh,
+                tvh -> tvh.set(seg, 0L, testValue),
+                tvh -> setter.set(tvh, seg, 0L, testValue),
+                ".*\\Qexpected (MemorySegmentProxy,long," + carrier.getSimpleName() + ")void \\E.*");
+        }
+    }
+
+    private static void doTest(VarHandle invokeHandle, Consumer<VarHandle> invokeTest,
+                               Consumer<VarHandle> invokeExactTest, String expectedMessage) {
+        assertFalse(invokeHandle.hasInvokeExactBehavior());
+        assertSame(invokeHandle, invokeHandle.withInvokeBehavior());
         try {
-            vh.set(buff, 0, testValue);
-            vh.withInvokeBehavior().set(buff, 0, testValue);
+            invokeTest.accept(invokeHandle);
         } catch (WrongMethodTypeException wmte) {
             fail("Unexpected exception", wmte);
         }
 
-        vh = vh.withInvokeExactBehavior();
-        assertTrue(vh.hasInvokeExactBehavior());
+        VarHandle invokeExactHandle = invokeHandle.withInvokeExactBehavior();
+        assertTrue(invokeExactHandle.hasInvokeExactBehavior());
+        assertSame(invokeExactHandle, invokeExactHandle.withInvokeExactBehavior());
         try {
-            setter.set(vh, buff, testValue); // should throw
+            invokeExactTest.accept(invokeExactHandle); // should throw
             fail("Exception expected");
         } catch (WrongMethodTypeException wmte) {
-            assertMatches(wmte.getMessage(),
-                ".*\\Qexpected (ByteBuffer,int," + arrayClass.componentType().getSimpleName() + ")void \\E.*");
+            assertMatches(wmte.getMessage(), expectedMessage);
         }
-    }
 
-    @Test(dataProvider = "dataSetMemorySegment")
-    public void testExactSegmentSet(Class<?> carrier, Object testValue, SetSegmentX setter) {
-        VarHandle vh = MemoryHandles.varHandle(carrier, ByteOrder.nativeOrder());
-        assertFalse(vh.hasInvokeExactBehavior());
-        try (MemorySegment seg = MemorySegment.allocateNative(8)) {
-            try {
-                vh.set(seg, 0L, testValue);
-                vh.withInvokeBehavior().set(seg, 0L, testValue);
-            } catch (WrongMethodTypeException wmte) {
-                fail("Unexpected exception", wmte);
-            }
-
-            vh = vh.withInvokeExactBehavior();
-            assertTrue(vh.hasInvokeExactBehavior());
-            try {
-                setter.set(vh, seg, 0L, testValue); // should throw
-                fail("Exception expected");
-            } catch (WrongMethodTypeException wmte) {
-                assertMatches(wmte.getMessage(),
-                    ".*\\Qexpected (MemorySegment,long," + carrier.getSimpleName() + ")void \\E.*");
-            }
+        // try going back
+        VarHandle invokeHandle2 = invokeExactHandle.withInvokeBehavior();
+        assertFalse(invokeHandle2.hasInvokeExactBehavior());
+        try {
+            invokeTest.accept(invokeHandle2);
+        } catch (WrongMethodTypeException wmte) {
+            fail("Unexpected exception", wmte);
         }
     }
 
@@ -280,7 +240,7 @@ private interface SetBufferX {
     }
 
     private interface SetSegmentX {
-        void set(VarHandle vh, MemorySegment segment, long offser, Object testValue);
+        void set(VarHandle vh, MemorySegment segment, long offset, Object testValue);
     }
 
     private static void consume(Object o) {}
@@ -417,11 +377,11 @@ public static Object[][] dataSetMemorySegment() {
         List<Object[]> cases = new ArrayList<>();
 
         // create a bunch of different sig-poly call sites
-        testCaseSegmentSet(cases, long.class, 1234,         (vh, seg, off, tv) -> vh.set(seg, off, (int) tv));
-        testCaseSegmentSet(cases, long.class, (char) 1234,  (vh, seg, off, tv) -> vh.set(seg, off, (char) tv));
-        testCaseSegmentSet(cases, long.class, (short) 1234, (vh, seg, off, tv) -> vh.set(seg, off, (short) tv));
-        testCaseSegmentSet(cases, long.class, (byte) 1234,  (vh, seg, off, tv) -> vh.set(seg, off, (byte) tv));
-        testCaseSegmentSet(cases, double.class, 1234F,      (vh, seg, off, tv) -> vh.set(seg, off, (float) tv));
+        testCaseSegmentSet(cases, long.class, 1234,         (vh, seg, off, tv) -> vh.set((MemorySegmentProxy) seg, off, (int) tv));
+        testCaseSegmentSet(cases, long.class, (char) 1234,  (vh, seg, off, tv) -> vh.set((MemorySegmentProxy) seg, off, (char) tv));
+        testCaseSegmentSet(cases, long.class, (short) 1234, (vh, seg, off, tv) -> vh.set((MemorySegmentProxy) seg, off, (short) tv));
+        testCaseSegmentSet(cases, long.class, (byte) 1234,  (vh, seg, off, tv) -> vh.set((MemorySegmentProxy) seg, off, (byte) tv));
+        testCaseSegmentSet(cases, double.class, 1234F,      (vh, seg, off, tv) -> vh.set((MemorySegmentProxy) seg, off, (float) tv));
 
         return cases.toArray(Object[][]::new);
     }