RFR 8079784: Unexpected IllegalAccessError when trying access InnerClasses attribute

[David Holmes]

Hi Harold,

Looks fine as-is I have one query below ...

One additional nit in jasm file - this purported line of source code is 
not correct:

  60 //         Class<?> clazz = Buggered.Foo.class;

as you aren't using that class.

On 10/10/2018 12:12 AM, Harold David Seigel wrote:
> Hi,
> 
> Please review this fix, proposed by Doug Simon, for JDK-8079784. The fix 
> prevents classes in the InnerClasses attribute from being loaded unless 
> they are actually being accessed.

That in itself seems reasonable. I'm surprised we don't do more sanity 
checking on the classes listed in the inner classes attribute - I would 
have expected at least a "same package" check.

Looking at the code itself:

       if (inner_is_member && ioff != 0 && ooff != 0) {
+       if (cp->klass_name_at_matches(outer, ooff) &&
+           cp->klass_name_at_matches(inner, ioff)) {
           Klass* o = cp->klass_at(ooff, CHECK);
           if (o == outer) {
             Klass* i = cp->klass_at(ioff, CHECK);
             if (i == inner) {
               return;
             }
           }
         }
+     }

I'm wondering how it is possible to have the names match and yet 
potentially o!=outer and i!=inner ?

> Also, while looking into this issue, I noticed that method 
> is_same_package_member() is not used.  So, I removed it as part of this 
> webrev.

In 8u it's called by Reflection::is_same_package_member, which in turn 
is unused. That was removed in 9 by the cleanup done in JDK-8140485. :)

Thanks,
David

> 
> Open Webrev: http://cr.openjdk.java.net/~hseigel/bug_8079784/webrev/
> 
> JBS Bug:  https://bugs.openjdk.java.net/browse/JDK-8079784
> 
> The fix was tested with the test in the webrev and by running Mach5 
> tiers 1 and 2 tests and builds on Linux-x64, Windows, and Mac OS X, 
> running tiers 3-5 tests on Linux-x64, and by running JCK-12 Lang and VM 
> tests on Linux-x64.
> 
> Thanks, Harold
>