RFR: 8286277: CDS VerifyError when calling clone() on object array [v2]

[Calvin Cheung]

On Tue, 17 May 2022 16:13:37 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> **Problem:**
>> 
>> When verifying an invokevirtual bytecode that calls a protected method in a superclass of the current class, the verifier requires that the object being invoked must be a subtype of the current class. This prevents invocation of protected methods in an unrelated class. For example, the following is disallowed -- the type `InvokeCloneInvalid` cannot invoke `clone()` on a `String`:
>> 
>> 
>> super public class InvokeCloneInvalid
>>     version 52:0
>> {
>>   public static Method test:"(Ljava/lang/String;)V"
>>     stack 1 locals 1
>>   {
>>     aload_0;
>>     invokevirtual Method "java/lang/Object".clone:"()Ljava/lang/Object;";
>>     return;
>>   }
>> }
>> 
>> 
>> However, there's a special case where invoking `Object.clone()` is allowed on all object arrays. Here's an example:
>> 
>> 
>> super public class InvokeCloneValid
>>     version 52:0
>> {
>>   public static Method test:"([Ljava/lang/Object;)V"
>>     stack 1 locals 1
>>   {
>>     aload_0;
>>     invokevirtual Method "java/lang/Object".clone:"()Ljava/lang/Object;";
>>     return;
>>   }
>> 
>> 
>> Before this PR. the Verifier would first do the assignability check. When a failure is encountered, the Verifier then checks for the special case and ignores the failure accordingly.
>> 
>> In the above case, the assignability check will fail because the object being invoked (an Object array) is not assignable to the current class (InvokeCloneValid)
>> 
>> The problem is that CDS remembers all the assignability checks and tries to replay them at runtime. Therefore, it will re-check the assignability of Object Array to InvokeCloneValid, and throw a VerifyError as a result.
>> 
>> **Proposed Fix**
>> 
>> In `ClassVerifier::verify_invoke_instructions()`, check the special case first, before doing the assignability check. This way, such invalid-but-ignored assignability checks are no longer performed and thus won't be remembered by CDS.
>> 
>> **Testing**
>> 
>> Tiers 1 - 4, plus 3 new test cases.
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Adding newline at end of file

Looks good. Just one nit.

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/VerifyObjArrayCloneTest.java line 32:

> 30:  *          /test/hotspot/jtreg/runtime/cds/appcds/test-classes
> 31:  * @build sun.hotspot.WhiteBox
> 32:   * @build InvokeCloneValid InvokeCloneInvalid VerifyObjArrayCloneTestApp

Please align line 32 with line 31.

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

Marked as reviewed by ccheung (Reviewer).

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