RFR 8079784: Unexpected IllegalAccessError when trying access InnerClasses attribute

[David Holmes]

Hi Lois,

On 11/10/2018 1:35 AM, Lois Foltan wrote:
> On 10/9/2018 9:35 PM, David Holmes wrote:
> 
>> 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 ?
> 
> David,
> 
> I think this might be the example you are looking for?
> 
> public class OuterClass {
> 
>      private class Inner {
>          String _field1;
>          private class C { int _c_field1; }
>          public void inner_method() {
>              _field1 = "Friend";
>              System.out.println("Hello " + _field1);
>          }
>      }
> 
>      private class C {
>          String _c_field;
>      }
> 
>      public void test_method() {
>          Inner i = new Inner();
>          i.inner_method();
>      }
> 
>      public static void main( String[] args ) {
>          OuterClass h = new OuterClass();
>          h.test_method();
>      }
> }
> 
> The InnerClasses attribute has two classes named "C" that contain 
> differing outer classes.

Simple names yes, but we should be dealing with full names here. If we 
only compare simple names then the bug would not be fixed.

Cheers,
David
-----

> SourceFile: "OuterClass.java"
> NestMembers:
>    OuterClass$C
>    OuterClass$Inner
>    OuterClass$Inner$C
> InnerClasses:
>    private #10= #9 of #5;                  // C=class OuterClass$C of 
> class OuterClass
>    private #12= #2 of #5;                  // Inner=class 
> OuterClass$Inner of class OuterClass
>    private #10= #23 of #2;                 // C=class OuterClass$Inner$C 
> of class OuterClass$Inner
> 
> Thanks,
> Lois
> 
>>
>>> 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
>>>
>