RFR(S) 8226645: [TESTBUG] some AppCDS tests relies on illegal reflective access

[Ioi Lam]

Hi Calvin,

Looks good.

Thanks
- Ioi

On 8/9/19 8:50 PM, Calvin Cheung wrote:
> Hi Ioi,
>
> Thanks for your review!
>
> On 8/9/19 1:07 PM, Ioi Lam wrote:
>> Hi Calvin,
>>
>> This makes it hard to tell where the privateLookupIn is from.
>>
>>     import static java.lang.invoke.MethodHandles.*;
>>     ....
>>     Lookup lookup = privateLookupIn(target, lookup());
>>
>> I think it's better to do this:
>>
>>     import static java.lang.invoke.MethodHandles;
>>     ....
>>     Lookup lookup = MethodHandles.privateLookupIn(target, 
>> MethodHandles.lookup());
> Fixed.
>>
>>
>> Also, instead of forcing every test case to carry a class called 
>> "Hello", I think
>> it's better to do this:
>>
>>
>>     /**
>>      * Define the class as stored in clsFile, in the same loader as 
>> peerClass,
>>      * with the following modification:
>>      * <ul>....</ul>
>>      */
>>     public static Class<?> defineModifiedClass(Class<?> peerClass, 
>> File clsFile,
>>                            String fromString, String toString)
>>         throws .....
>>     {
>>         ....
>>         Lookup lookup = MethodHandles.privateLookupIn(peerClass, 
>> MethodHandles.lookup());
>>         Class<?> cls = lookup.defineClass(buff);
>>         ....
> This works with defineModifiedClass but not with the 
> defineClassFromJAR. It is because the current usage of 
> defineClassFromJAR is that the class is supposed to be loaded by a 
> URLClassLoader. If we just pass the test class as the peerClass, the 
> loader will be AppClassLoader. Therefore, I need to keep the Hello 
> class for the LoaderSegregationTest but the loading of the Hello class 
> will be in the LoaderSegregation app class instead of in Util.java. I 
> also modified defineClassFromJAR so that its signature is the same as 
> defineModifiedClass.
>>
>> Then the caller of defineModifiedClass can be changed from:
>>
>> Class superClass = 
>> Util.defineModifiedClass(RewriteBytecodes.class.getClassLoader(), 
>> clsFile, from, to);
>>
>> to
>>
>> Class superClass = Util.defineModifiedClass(RewriteBytecodes.class, 
>> clsFile, from, to);
>
> Fixed.
>
> updated webrev:
>
>     http://cr.openjdk.java.net/~ccheung/8226645/webrev.01/
>
> thanks,
>
> Calvin
>
>>
>> Thanks
>> - Ioi
>>
>>
>>
>> On 8/9/19 11:19 AM, Calvin Cheung wrote:
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8226645
>>>
>>> webrev: http://cr.openjdk.java.net/~ccheung/8226645/webrev.00/
>>>
>>> Updating couple of AppCDS tests to use Lookup.defineClass instead of 
>>> ClassLoader.defineClass. A dummy class will be loaded by the 
>>> requested class loader and then a lookup instance is obtained using 
>>> the privateLookupIn API.
>>>
>>> Testing: mach5 tiers1 - 3.
>>>
>>>              locally on linux-x64 with -vmoption:--illegal-access=deny
>>>
>>> thanks,
>>>
>>> Calvin
>>>
>>