[10] RFR (S): 8196022: java.lang.VerifyError is thrown instead of java.lang.IllegalAccessError

[Vladimir Ivanov]

Thanks for review, Paul.

> This is quite a subtle and confusing area. I believe you have the mapping correct with a cleaner correspondence between reflective exceptions and errors. +1

FTR I don't insist on pushing it into 10. The missing checks for corner 
cases in SystemDictionary::link_method_handle_constant() is covered 
later during down call, so the only risk is that JDK-8188145 [1] fix 
won't work for those corner cases, which is very low.

So, if anybody doesn't feel comfortable pushing it into 10, I'm fine 
with delaying the fix till 11.

> I am having a hard time understanding how a ClassNotFoundException can originate when resolving a member name from symbolic information (and when resolving in Java via a lookup Class objects are required).
> 
> Perhaps it’s due to logic in SystemDictionary::handle_resolution_exception:

Yes, exactly. I had hard time proving 
SystemDictionary::handle_resolution_exception isn't called with 
throw_error = false during method/field resolution, so decided to play 
it safe and cover it on JVM side to avoid dealing with that on JDK side.

Best regards,
Vladimir Ivanov

[1] https://bugs.openjdk.java.net/browse/JDK-8188145

> Klass* SystemDictionary::handle_resolution_exception(Symbol* class_name,
>                                                       bool throw_error,
>                                                       Klass* klass, TRAPS) {
>    if (HAS_PENDING_EXCEPTION) {
>      // If we have a pending exception we forward it to the caller, unless throw_error is true,
>      // in which case we have to check whether the pending exception is a ClassNotFoundException,
>      // and if so convert it to a NoClassDefFoundError
>      // And chain the original ClassNotFoundException
>      if (throw_error && PENDING_EXCEPTION->is_a(SystemDictionary::ClassNotFoundException_klass())) {
>        ResourceMark rm(THREAD);
>        assert(klass == NULL, "Should not have result with exception pending");
>        Handle e(THREAD, PENDING_EXCEPTION);
>        CLEAR_PENDING_EXCEPTION;
>        THROW_MSG_CAUSE_NULL(vmSymbols::java_lang_NoClassDefFoundError(), class_name->as_C_string(), e);
>      } else {
>        return NULL;
>      }
>    }
>    // Class not found, throw appropriate error or exception depending on value of throw_error
>    if (klass == NULL) {
>      ResourceMark rm(THREAD);
>      if (throw_error) {
>        THROW_MSG_NULL(vmSymbols::java_lang_NoClassDefFoundError(), class_name->as_C_string());
>      } else {
>        THROW_MSG_NULL(vmSymbols::java_lang_ClassNotFoundException(), class_name->as_C_string());
>      }
>    }
>    return klass;
> }
> 
> Paul.
> 
>> On Jan 25, 2018, at 4:49 PM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
>>
>> http://cr.openjdk.java.net/~vlivanov/8196022/webrev.00/index.html
>> https://bugs.openjdk.java.net/browse/JDK-8196022
>>
>> It's a followup on JDK-8188145 [1] which fixed what exceptions are thrown. It (unexpectedly :-)) fixed inaccuracy there: MemberName resolution code on JDK side converted most of LinkageErrors into IllegalAccessErrors, but after JDK-8188145 [1] there's no upcall anymore when resolution fails, so exceptions are reported as is which is the right thing.
>>
>> The fix cleans JDK part to behave the same way.
>>
>> Also, fixed MethodHandles::resolve_MemberName() to handle corner cases when empty handle is returned, but no exception is thrown. An exception should be thrown in that case as it is in MHN_resolve_Mem.
>>
>> As a simplifying change, converting ClassNotFoundException to NoClassDefFoundError was added to avoid dealing with it on JDK side.
>>
>> Testing: hs-tier1, hs-tier2, vm.defmeth, jck-runtime-10, jdk/java/lang/invoke/, hotspot/runtime/invokedynamic/
>>
>> Thanks!
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8188145
>