RFR (S) JDK-8202758: SIGSEGV calling Class.forName(String,Boolean,ClassLoader) with mocked loader

[David Holmes]

Hi Coleen,

My 2c

On 25/05/2018 6:10 AM, coleen.phillimore at oracle.com wrote:
> 
> http://cr.openjdk.java.net/~lfoltan/bug_jdk8202758/webrev/test/hotspot/jtreg/runtime/modules/ClassLoaderNoUnnamedModuleTest.java.html 
> 
> 
> This doesn't seem to use InMemoryJavaCompiler but imports it.
> 
> http://cr.openjdk.java.net/~lfoltan/bug_jdk8202758/webrev/src/hotspot/share/classfile/moduleEntry.cpp.udiff.html 
> 
> 
> Are you sure we want to do a guarantee()?  That will look like a VM 
> crash to the user who will report it as a bug, so they and others will 
> still think it's our bug.  Generally when we produce an hs_err_pid.log 
> file, people think it's a bug in the vm (except the OOM variants but 
> sometimes that's debatable too).
> 
> I took out all the TRAPS to the call stack for where this is called, but 
> you could put them back in and throw an exception instead (NPE) which 
> would give the user his file and line where the error occurred.   Or 
> else create a variant of vm_exit_during_initialization that doesn't say 
> it's during initialization in order to exit the vm quickly with a message.

An exception would legitimise this too much. This isn't a simple 
programming error, but a subversion of the language semantics by 
skipping the superclass constructor.

A vm_exit other than at initialization is not something I'd want to 
encourage use of. We only have the at-initialization variant to handle 
the inability to throw exceptions at that time and to avoid ugly crashes.

In this case, I think a guarantee failure is warranted - though I wonder 
whether we can be more specific about the potential cause? I agree this 
will come back as a bug report against the JVM rather than against 
Mockito/Objgenesis/whomever. Not sure what we can do there.

Cheers,
David

> Thanks,
> Coleen
> 
> 
> On 5/24/18 1:35 PM, Lois Foltan wrote:
>> Please review this change to ensure that a given ClassLoader's unnamed 
>> Module is a valid instance of java.lang.Module with the JVM.
>>
>> open webrev at http://cr.openjdk.java.net/~lfoltan/bug_jdk8202758/webrev/
>> bug link at https://bugs.openjdk.java.net/browse/JDK-8202758
>>
>> Testing: hs-tier1-3, jdk-tier1-3 in progress
>>
>> Thanks,
>> Lois
>