[13] RFR(S) 8226566: [JVMCI] java.* classes are no longer necessarily resolved by the boot class loader

[Vladimir Kozlov]

Thank you, Dean

Vladimir

On 7/3/19 12:21 PM, dean.long at oracle.com wrote:
> Yes, that looks good.
> 
> dl
> 
> On 7/2/19 3:36 PM, Vladimir Kozlov wrote:
>> Thank you, Doug
>>
>> https://cr.openjdk.java.net/~kvn/8226566/webrev.01/
>>
>> Dean, do you agree?
>>
>> I started new testing.
>>
>> Thanks,
>> Vladimir
>>
>> On 7/2/19 3:01 PM, Doug Simon wrote:
>>> Based on Dean’s input, this is the change we now want:
>>>
>>> *if* (elementType.getName().startsWith("Ljava/") && hasSameClassLoader(/runtime/().getJavaLangObject())) {
>>> // Classes in a java.* package defined by the boot class loader are always resolved.
>>> *return* *true*;
>>>          }
>>>
>>> I can confirm it also fixes the reported problem: https://github.com/oracle/graal/issues/1409#issuecomment-504254246
>>>
>>> -Doug
>>>
>>>> On 2 Jul 2019, at 20:10, Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>> wrote:
>>>>
>>>> Hi Doug,
>>>>
>>>> Do you have updated patch for it? Or we should just drop these changes?
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 6/24/19 12:12 PM, Doug Simon wrote:
>>>>>> On 24 Jun 2019, at 20:40, dean.long at oracle.com <mailto:dean.long at oracle.com> wrote:
>>>>>>
>>>>>> The code isn't restricted to just java.* anymore.  Now it's any type that the platform classloader defines.  Isn't 
>>>>>> it possible that the class was resolved by the platform classloader, but accessingClass has a custom classloader 
>>>>>> that throws ClassNotFound?
>>>>> I see your point - we should retain the check for java.*.
>>>>> -Doug
>>>>>> On 6/22/19 4:25 AM, Doug Simon wrote:
>>>>>>>
>>>>>>>> On 22 Jun 2019, at 01:14, dean.long at oracle.com <mailto:dean.long at oracle.com> <mailto:dean.long at oracle.com> wrote:
>>>>>>>>
>>>>>>>> Doesn't this assume that the classloader for accessingClass is well-behaved and delegates to the classloader for 
>>>>>>>> Object?  If the classloader is not well-behaved, is it safe to return true here?
>>>>>>>
>>>>>>> I don’t think it's possible for anything but the boot or platform class loader to load java.* classes:
>>>>>>>
>>>>>>> https://github.com/openjdk/jdk13/blob/master/src/java.base/share/classes/java/lang/ClassLoader.java#L891-L899 
>>>>>>> <https://github.com/openjdk/jdk13/blob/master/src/java.base/share/classes/java/lang/ClassLoader.java#L891-L899>
>>>>>>>
>>>>>>> I’ve tested this:
>>>>>>>
>>>>>>> public class Main extends ClassLoader {
>>>>>>>     @Override
>>>>>>>     protected Class<?> findClass(String name) throws ClassNotFoundException {
>>>>>>>         byte[] b = {};
>>>>>>>         return defineClass(name, b, 0, b.length);
>>>>>>>     }
>>>>>>>
>>>>>>>     @Override
>>>>>>>     protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
>>>>>>>         return findClass(name);
>>>>>>>     }
>>>>>>>
>>>>>>>     public static void main(String[] args) throws Throwable {
>>>>>>>         Main cl = new Main();
>>>>>>>         for (String name : args) {
>>>>>>>             try {
>>>>>>>                 System.out.printf("%s: loader = %s%n", name, Class.forName(name).getClassLoader());
>>>>>>>                 System.out.println(Class.forName(name, true, cl));
>>>>>>>             } catch (Exception e) {
>>>>>>>                 e.printStackTrace();
>>>>>>>             }
>>>>>>>         }
>>>>>>>     }
>>>>>>> }
>>>>>>>
>>>>>>>> java -version
>>>>>>> java version "11.0.3" 2019-04-16 LTS
>>>>>>> Java(TM) SE Runtime Environment 18.9 (build 11.0.3+12-LTS)
>>>>>>> Java HotSpot(TM) 64-Bit Server VM 18.9 (build 11.0.3+12-LTS, mixed mode)
>>>>>>>> java Main java.lang.Boolean java.sql.Array
>>>>>>> java.lang.Boolean: loader = null
>>>>>>> java.lang.SecurityException: Prohibited package name: java.lang
>>>>>>> at java.base/java.lang.ClassLoader.preDefineClass(ClassLoader.java:898)
>>>>>>> at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1014)
>>>>>>> at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:877)
>>>>>>> at Main.findClass(Main.java:5)
>>>>>>> at Main.loadClass(Main.java:10)
>>>>>>> at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
>>>>>>> at java.base/java.lang.Class.forName0(Native Method)
>>>>>>> at java.base/java.lang.Class.forName(Class.java:398)
>>>>>>> at Main.main(Main.java:18)
>>>>>>> java.sql.Array: loader = jdk.internal.loader.ClassLoaders$PlatformClassLoader at 47d384ee
>>>>>>> java.lang.SecurityException: Prohibited package name: java.sql
>>>>>>> at java.base/java.lang.ClassLoader.preDefineClass(ClassLoader.java:898)
>>>>>>> at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1014)
>>>>>>> at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:877)
>>>>>>> at Main.findClass(Main.java:5)
>>>>>>> at Main.loadClass(Main.java:10)
>>>>>>> at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
>>>>>>> at java.base/java.lang.Class.forName0(Native Method)
>>>>>>> at java.base/java.lang.Class.forName(Class.java:398)
>>>>>>> at Main.main(Main.java:18)
>>>>>>>
>>>>>>>> What happens if we just remove the special case for java.lang.Object's classloader?  Does it hurt performance?
>>>>>>>
>>>>>>> It avoids a VM call for resolving all boot classes during compilation. We could also avoid the VM call for when 
>>>>>>> resolving platform classes as well but I’m not sure how to get a ResolvedJavaType for a platform class.
>>>>>>>
>>>>>>> I don’t recall the measurements on the performance impact of this optimization. Maybe it doesn’t show up. 
>>>>>>> However, in libgraal VM calls are relatively more expensive so seems like its worth keeping this shortcut.
>>>>>>>
>>>>>>> -Doug
>>>>>>>
>>>>>>>>
>>>>>>>> On 6/21/19 3:47 PM, Vladimir Kozlov wrote:
>>>>>>>>> https://cr.openjdk.java.net/~kvn/8226566/webrev.00/ <https://cr.openjdk.java.net/~kvn/8226566/webrev.00/>
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8226566 <https://bugs.openjdk.java.net/browse/JDK-8226566>
>>>>>>>>>
>>>>>>>>> Doug proposed the fix. I tested it.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Vladimir
>>>>>>>>
>>>>>>>
>>>>>>
>>>
>