On Sat, 6 May 2023 18:15:56 GMT, Chen Liang <li...@openjdk.org> wrote:

> The access hack for array class clone is only applied to `checkAccess` but 
> missing before call to `restrictProtectedReceiver`, causing the array 
> receiver type to be incorrectly replaced by the lookupClass type. This patch 
> fixes that and adds a test to ensure an original lookup resolves `clone` for 
> both array classes (public) and Object (inherited protected) correctly, and 
> restores the old MethodHandlesGeneralTest from 
> [JDK-8001105](https://bugs.openjdk.org/browse/JDK-8001105) which ensures 
> correctness for publicLookup (which is already correct).

Thanks for taking this.   The patch looks right.    It should special case 
`Object.clone` for array types not to restrict the receiver to the lookup class.

src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 4023:

> 4021:             // The accessing class only has the right to use a 
> protected member
> 4022:             // on itself or a subclass.  Enforce that restriction, from 
> JVMS 5.4.4, etc.
> 4023:             // Since array have access hacks, need to guard against 
> array clone() before

I suggest to move this new comment to `getDirectMethodCommon`, the relevant 
caller of this method.  See suggested wordings below.

src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 4112:

> 4110:                     (MethodHandleNatives.refKindHasReceiver(refKind) &&
> 4111:                             !isArrayClone(refKind, refc, method) &&
> 4112:                             restrictProtectedReceiver(method))) {

Suggestion:

                    (MethodHandleNatives.refKindHasReceiver(refKind) &&
                            restrictProtectedReceiver(method) &&
                            // All arrays simply inherit the protected 
Object.clone method.
                            // The leading argument is restricted to the 
requested array type
                            // (not the lookup class).
                            !isArrayClone(refKind, refc, method))) {

test/jdk/java/lang/invoke/findVirtual/FindVirtualArrayCloneTest.java line 28:

> 26:  * @bug 8299505
> 27:  * @run junit FindVirtualArrayCloneTest
> 28:  * @summary Ensures Arrays' clone doesn't have incorrect receiver type 
> bound

Suggestion:

 * @summary Test invocation of Object.clone for arrays

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

PR Review: https://git.openjdk.org/jdk/pull/13855#pullrequestreview-1451988301
PR Review Comment: https://git.openjdk.org/jdk/pull/13855#discussion_r1210934822
PR Review Comment: https://git.openjdk.org/jdk/pull/13855#discussion_r1210935373
PR Review Comment: https://git.openjdk.org/jdk/pull/13855#discussion_r1210936278

Reply via email to