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

[Doug Simon]

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> 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 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> 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
>>>>> 
>>>> 
>>>