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

[David Holmes]

Hi Harold,

This looks okay to me. A few comments below but only one real query.

On 26/09/2017 1:21 AM, harold seigel 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.

Okay.

> Additionally, this change improves the LinkageError exception error text 
> when a loader constraint violation occurs during vtable and itable 
> creation.

Hmmm :) I think I put those in initially. Not sure I 100% agree with the 
changed terminology, but I'll defer to you as the current expert in this 
area. :)

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

*.jasm: can you add a comment indicating why these are jasm files as it 
is not obvious to me what is special about them.

*/Test.java:
  - You can place multiple files on one @compile tag (and still list one 
file per line).
- you don't need to specify java.lang in the name of the exception classes

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

The real query:

1201     if (target == NULL || !target->is_public() || 
target->is_abstract() || target->is_overpass()) {
1202       // Entry does not resolve. Leave it empty for 
AbstractMethodError.
1203       if (!(target == NULL) && !target->is_public()) {
1204         // Stuff an IllegalAccessError throwing method in there 
instead.
1205         itableOffsetEntry::method_entry(_klass, 
method_table_offset)[m->itable_index()].
1206             initialize(Universe::throw_illegal_access_error());
1207       }

Not clear why you added the overpass check here? If it is non-public 
then you're replacing it with an IllegalAccessError instead of whatever 
the Overpass was going to throw. ??

Thanks,
David
-----

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