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