8299505: findVirtual on array classes incorrectly restricts the receiver type

Reviewed-by: mchung

src/java.base/share/classes/java/lang/invoke/MethodHandles.java

@@ -3928,6 +3928,14 @@ return mh1;
             throw m.makeAccessException(message, this);
         }
 
+        private boolean isArrayClone(byte refKind, Class<?> refc, MemberName m) {
+            return Modifier.isProtected(m.getModifiers()) &&
+                    refKind == REF_invokeVirtual &&
+                    m.getDeclaringClass() == Object.class &&
+                    m.getName().equals("clone") &&
+                    refc.isArray();
+        }
+
         /** Check public/protected/private bits on the symbolic reference class and its member. */
         void checkAccess(byte refKind, Class<?> refc, MemberName m) throws IllegalAccessException {
             assert(m.referenceKindIsConsistentWith(refKind) &&
@@ -3936,11 +3944,7 @@ return mh1;
             int allowedModes = this.allowedModes;
             if (allowedModes == TRUSTED)  return;
             int mods = m.getModifiers();
-            if (Modifier.isProtected(mods) &&
+            if (isArrayClone(refKind, refc, m)) {
-                    refKind == REF_invokeVirtual &&
-                    m.getDeclaringClass() == Object.class &&
-                    m.getName().equals("clone") &&
-                    refc.isArray()) {
                 // The JVM does this hack also.
                 // (See ClassVerifier::verify_invoke_instructions
                 // and LinkResolver::check_method_accessability.)
@@ -4100,7 +4104,12 @@ return mh1;
             MethodHandle mh = dmh;
             // Optionally narrow the receiver argument to lookupClass using restrictReceiver.
             if ((doRestrict && refKind == REF_invokeSpecial) ||
-                    (MethodHandleNatives.refKindHasReceiver(refKind) && restrictProtectedReceiver(method))) {
+                    (MethodHandleNatives.refKindHasReceiver(refKind) &&
+                            restrictProtectedReceiver(method) &&
+                            // All arrays simply inherit the protected Object.clone method.
+                            // The leading argument is already restricted to the requested
+                            // array type (not the lookup class).
+                            !isArrayClone(refKind, refc, method))) {
                 mh = restrictReceiver(method, dmh, lookupClass());
             }
             mh = maybeBindCaller(method, mh, boundCaller);

test/jdk/java/lang/invoke/MethodHandlesGeneralTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -187,14 +187,9 @@ public class MethodHandlesGeneralTest extends MethodHandlesTest {
         // test some ad hoc system methods
         testFindVirtual(false, PUBLIC, Object.class, Object.class, "clone");
 
-        // ##### FIXME - disable tests for clone until we figure out how they should work with modules
+        for (Class<?> cls : new Class<?>[]{ Object[].class, int[].class, boolean[].class, long[].class, float[].class, char[].class }) {
-
-        /*
-        testFindVirtual(true, PUBLIC, Object[].class, Object.class, "clone");
-        testFindVirtual(true, PUBLIC, int[].class, Object.class, "clone");
-        for (Class<?> cls : new Class<?>[]{ boolean[].class, long[].class, float[].class, char[].class })
             testFindVirtual(true, PUBLIC, cls, Object.class, "clone");
-         */
+        }
     }
 
     void testFindVirtual(Class<?> defc, Class<?> ret, String name, Class<?>... params) throws Throwable {
@@ -264,8 +259,8 @@ public class MethodHandlesGeneralTest extends MethodHandlesTest {
             assertEquals(MethodType.methodType(Object.class, rcvc), target.type());
             Object orig = argsWithSelf[0];
             assertEquals(orig.getClass(), res.getClass());
-            if (res instanceof Object[])
+            if (res instanceof Object[] arr)
-                assertArrayEquals((Object[])res, (Object[])argsWithSelf[0]);
+                assertArrayEquals(arr, (Object[])argsWithSelf[0]);
             assert(Arrays.deepEquals(new Object[]{res}, new Object[]{argsWithSelf[0]}));
         } else {
             assert(false) : Arrays.asList(positive, lookup, rcvc, defc, ret, name, deepToString(params));

test/jdk/java/lang/invoke/findVirtual/FindVirtualArrayCloneTest.java

@@ -0,0 +1,75 @@
+/*
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8299505
+ * @run junit FindVirtualArrayCloneTest
+ * @summary Test invocation of Object.clone for arrays
+ */
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+
+public class FindVirtualArrayCloneTest {
+
+    /**
+     * array clone should be bound to the array class (public)
+     */
+    @ValueSource(classes = {Object[].class, int[].class, long[].class, byte[].class, double[].class})
+    @ParameterizedTest
+    public void checkArrayCloneReceiver(Class<?> arrayClass) throws Throwable {
+        var lookup = MethodHandles.lookup();
+        var cloneMh = lookup.findVirtual(arrayClass, "clone", MethodType.methodType(Object.class));
+        assertSame(arrayClass, cloneMh.type().parameterType(0));
+    }
+
+    /**
+     * Object.clone should be bound to the lookup class (protected)
+     */
+    @Test
+    public void checkProtectedCloneReceiver() throws Throwable {
+        var lookup = MethodHandles.lookup();
+        var cloneMh = lookup.findVirtual(Object.class, "clone", MethodType.methodType(Object.class));
+        assertSame(lookup.lookupClass(), cloneMh.type().parameterType(0));
+    }
+
+    @Test
+    public void checkArrayClone() throws Throwable {
+        var array = new String[] {"apple", "pear", "banana"};
+        var regularClone = array.clone();
+        var cloneMh = MethodHandles.lookup()
+                .findVirtual(String[].class, "clone", MethodType.methodType(Object.class))
+                .asType(MethodType.methodType(String[].class, String[].class));
+        var mhClone = (String[]) cloneMh.invokeExact((String[]) array);
+        assertArrayEquals(array, mhClone);
+        assertArrayEquals(regularClone, mhClone);
+    }
+}