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

[Vladimir Kozlov]

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