RFR: 8259839 SystemDictionary exports too much implementation [v3]

[Coleen Phillimore]

On Tue, 2 Feb 2021 02:24:56 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> No, a klass can be NULL if load_instance_class returns NULL in a user-defined class loader.
>> ClassLoader.loadClass() can return NULL.
>>     // Class is not found or has the wrong name, return NULL
>>     return NULL;
>> There's also a NULL return from the bootclasspath.
>> For instance when loading sun/launcher/resources/spi/launcherProvider, this package isn't yet in the bootloader's list of packages, so there's a NULL return from load_instance_class, which is returned by resolve_instance_class_or_null.
>> 
>> It is sort of confusing that resolve_instance_class_or_null can return an exception OR null.  if it returns an exception, the exception and throw_error is true, the exception is wrapped in the NoClassDefFoundError.  It's in the JVMS 5.3:
>> 
>> If the Java Virtual Machine ever attempts to load a class C during verification (§5.4.1) or resolution (§5.4.3) (but not initialization (§5.5)), and the class loader that is used to initiate loading of C throws an instance of ClassNotFoundException, then the Java Virtual Machine must throw an instance of NoClassDefFoundError whose cause is the instance of ClassNotFoundException.
>> 
>> We don't ignore the exception if it's not ClassNotFoundException, we don't clear the exception so it's returned by handle_resolution_exception. Wait, you're right.  I should have a return NULL in the HAS_PENDING_EXCEPTION case to not ignore the existing exception.
>> 
>> I was trying to figure out why no tests failed with this bug, as it appears that it would override an incoming exception, for instance ClassCircularityException.  Turns out that THROW_MSG -> new_exception() which throws the original exception if one is pending.  I'll fix this code anyway, but I was surprised by this.
>
> `// Class is not found or has the wrong name, return NULL`
> 
> If Class is not found then there should be a ClassNotFoundException, or NoClassDefFoundError pending.
> If the representation has the wrong name then again a NoClassDefFoundError should be pending.
> 
> However, I see there are some paths in `load_instance_class` related to the module system that will return NULL without there being a pending exception.

Yes, it would be clearer if load_instance_class returned an exception and not NULL.  I'll note this as a further cleanup because it is inconsistent.  There are many calls to SystemDictionary::resolve_or_null that expect NULL and not a pending exception though so there are a lot of paths through this code that all want different answers. I don't know how to improve it but I'll look at it.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2247