RFR 8186092: Unnecessary loader constraints produced when there are multiple defaults

[harold seigel]

Hi Karen,

Thanks for looking at this!  Please see embedded comments and updated 
webrev at:

    http://cr.openjdk.java.net/~hseigel/bug_8186092.4/webrev/

Thanks! Harold

On 9/28/2017 10:59 AM, Karen Kinnear wrote:
> Many thanks Harold. Looks good!
>
> Couple of minor comments:
> 1. klassVtable.cpp
>
> line 484 - “They are not resolved methods”. Since the target_method is intended to be a “selected method” this comment is a bit
> confusing. Perhaps just take out that sentence.
I removed the sentence.
>
> To answer David - an overpass is always created public. I like the improved assertion.
>
> So I code reviewed the original exception messages, and missed the problem too. These look corrected.
> Perhaps you could send the error test results from running the tests in the email so it is easier to see that the error messages are correct.
This is the full message from test vtableLdrConstraint (without your 
suggestions):

      loader constraint violation for class Task: when resolving
    overriding method "Task.m()LFoo;" the class loader (instance of
    PreemptingClassLoader) of the selected method's type Task, and the
    class loader (instance of
    jdk/internal/loader/ClassLoaders$AppClassLoader) for its super type
    J have different Class objects for the type Foo used in the signature

This is the full message from test itableLdrConstraint (without your 
suggestions):

      loader constraint violation in interface itable initialization for
    class C: when resolving method "I.m()LFoo;" the class loader
    (instance of PreemptingClassLoader) for super interface I, and the
    class loader (instance of
    jdk/internal/loader/ClassLoaders$AppClassLoader) of the selected
    method's type, J have different Class objects for the type Foo used
    in the signature

>
>
> To make this a wee bit clearer:
>   “when resolving method” line 1224 - should be “when selecting method”
> and
> “line 501 - "when resolving overriding” should be “when selecting overriding”
Done.  The new messages look like this:

vtableLdrConstraint test:

    loader constraint violation for class Task: when selecting
    overriding method "Task.m()LFoo;" the class loader (instance of
    PreemptingClassLoader) of the selected method's type Task, and the
    class loader (instance of
    jdk/internal/loader/ClassLoaders$AppClassLoader) for its super type
    J have different Class objects for the type Foo used in the signature


itableLdrConstraint test:

    loader constraint violation in interface itable initialization for
    class C: when selecting method "I.m()LFoo;" the class loader
    (instance of PreemptingClassLoader) for super interface I, and the
    class loader (instance of
    jdk/internal/loader/ClassLoaders$AppClassLoader) of the selected
    method's type, J have different Class objects for the type Foo used
    in the signature

>
> That also changes the error message in the tests - at least in vtableLdrConstraint/Test.java
> vtableAME/Test.java line 46: inheriting - extra “t”
Fixed.
>
> Thank you so much for covering so many test cases.
>
> thanks,
> Karen
>
>
>> On Sep 25, 2017, at 11:21 AM, harold seigel <harold.seigel at oracle.com> wrote:
>>
>> Hi,
>>
>> Please review this JDK-10 change to fix JDK-8186092.  The change prevents the checking of loader constraints during vtable and itable creation if the selected method is an overpass method. Overpass methods are created by the JVM to throw exceptions and so should not be subjected to loader constraint checking.
>>
>> Additionally, this change improves the LinkageError exception error text when a loader constraint violation occurs during vtable and itable creation.
>>
>> The fix includes four new tests, one test each to check that loader constraint checking is not done for overpass methods during vtable and itable creation, and one test each to test the new vtable and itable loader constraint error messages.
>>
>> Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8186092/webrev/
>>
>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8186092
>>
>> The change was tested with the JCK Lang and VM tests, the JTreg hotspot, java/io, java/lang, java/util, and other tests, the co-located NSK tests, JPRT, and with RBT tier2 - tier5 tests.
>>
>> Thanks, Harold
>>