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

[Vladimir Kozlov]

Testing passed. I am pushing it.

Vladimir

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