8079784: Unexpected IllegalAccessError when trying access InnerClasses attribute

Prevent classes in the InnerClasses attribute from being loaded unless they are actually being accessed.

Reviewed-by: dholmes, lfoltan

src/hotspot/share/oops/instanceKlass.cpp

@@ -2738,48 +2738,6 @@ void InstanceKlass::check_prohibited_package(Symbol* class_name,
   return;
 }
 
-// tell if two classes have the same enclosing class (at package level)
-bool InstanceKlass::is_same_package_member(const Klass* class2, TRAPS) const {
-  if (class2 == this) return true;
-  if (!class2->is_instance_klass())  return false;
-
-  // must be in same package before we try anything else
-  if (!is_same_class_package(class2))
-    return false;
-
-  // As long as there is an outer_this.getEnclosingClass,
-  // shift the search outward.
-  const InstanceKlass* outer_this = this;
-  for (;;) {
-    // As we walk along, look for equalities between outer_this and class2.
-    // Eventually, the walks will terminate as outer_this stops
-    // at the top-level class around the original class.
-    bool ignore_inner_is_member;
-    const Klass* next = outer_this->compute_enclosing_class(&ignore_inner_is_member,
-                                                            CHECK_false);
-    if (next == NULL)  break;
-    if (next == class2)  return true;
-    outer_this = InstanceKlass::cast(next);
-  }
-
-  // Now do the same for class2.
-  const InstanceKlass* outer2 = InstanceKlass::cast(class2);
-  for (;;) {
-    bool ignore_inner_is_member;
-    Klass* next = outer2->compute_enclosing_class(&ignore_inner_is_member,
-                                                    CHECK_false);
-    if (next == NULL)  break;
-    // Might as well check the new outer against all available values.
-    if (next == this)  return true;
-    if (next == outer_this)  return true;
-    outer2 = InstanceKlass::cast(next);
-  }
-
-  // If by this point we have not found an equality between the
-  // two classes, we know they are in separate package members.
-  return false;
-}
-
 bool InstanceKlass::find_inner_classes_attr(int* ooff, int* noff, TRAPS) const {
   constantPoolHandle i_cp(THREAD, constants());
   for (InnerClassesIterator iter(this); !iter.done(); iter.next()) {

src/hotspot/share/oops/instanceKlass.hpp

@@ -507,9 +507,6 @@ public:
                                        ClassLoaderData* loader_data,
                                        TRAPS);
  public:
-  // tell if two classes have the same enclosing class (at package level)
-  bool is_same_package_member(const Klass* class2, TRAPS) const;
-
   // initialization state
   bool is_loaded() const                   { return _init_state >= loaded; }
   bool is_linked() const                   { return _init_state >= linked; }

src/hotspot/share/runtime/reflection.cpp

@@ -750,10 +750,12 @@ void Reflection::check_for_inner_class(const InstanceKlass* outer, const Instanc
   InnerClassesIterator iter(outer);
   constantPoolHandle cp   (THREAD, outer->constants());
   for (; !iter.done(); iter.next()) {
-     int ioff = iter.inner_class_info_index();
-     int ooff = iter.outer_class_info_index();
+    int ioff = iter.inner_class_info_index();
+    int ooff = iter.outer_class_info_index();
 
-     if (inner_is_member && ioff != 0 && ooff != 0) {
+    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);
@@ -761,14 +763,16 @@ void Reflection::check_for_inner_class(const InstanceKlass* outer, const Instanc
             return;
           }
         }
-     }
-     if (!inner_is_member && ioff != 0 && ooff == 0 &&
-         cp->klass_name_at_matches(inner, ioff)) {
-        Klass* i = cp->klass_at(ioff, CHECK);
-        if (i == inner) {
-          return;
-        }
-     }
+      }
+    }
+
+    if (!inner_is_member && ioff != 0 && ooff == 0 &&
+        cp->klass_name_at_matches(inner, ioff)) {
+      Klass* i = cp->klass_at(ioff, CHECK);
+      if (i == inner) {
+        return;
+      }
+    }
   }
 
   // 'inner' not declared as an inner klass in outer

test/hotspot/jtreg/runtime/InnerClassesAttr/Base.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+package com.g;
+class Base {
+    static class Builder { }
+}

test/hotspot/jtreg/runtime/InnerClassesAttr/Child.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+package com.g;
+public final class Child extends Base {
+    public Builder setJobName() {
+        return null;
+    }
+}

test/hotspot/jtreg/runtime/InnerClassesAttr/InnerClassesTest.jasm

@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/**
+ * @test
+ * @bug 8079784
+ * @compile InnerClassesTest.jasm Base.java Child.java
+ * @run main com.n.InnerClassesTest
+ */
+
+// Test that if a class's InnerClasses attribute contains a class that is not
+// accessible to the class with the attribute then an IllegalAccessError
+// exception should not get thrown as a result of the class accessing other
+// classes in the InnerClasses attribute.
+//
+// In this test, class InnerClassesTest has an InnerClasses attribute with two
+// entries.  One for inaccessable (non-public) class com/g/Base and class
+// con/g/Base$Builder.  And, one entry for classes com/n/InnerClassTest and
+// com/n/InnerClasses/Test$Foo.  The test accesses com/n/InnerClsses/Test$Foo
+// by calling getSimpleName().  This should not cause an IllegalAccessError
+// exception to get thrown.
+//
+//
+// This jasm code was generated from the following Java code.  The only
+// difference is that, in the jasm code, the order of the entries in the
+// InnerClasses attributes for class InnerClassesTest were switched.
+//
+// package com.n;
+// import com.g.Child;
+//
+// public final class InnerClassesTest {
+//
+//     public static final class Foo { }
+//     void unused() {
+//         new Child().setJobName();
+//     }
+//
+//     public static void main(String[] args) {
+//         Class<?> clazz = InnerClassesTest.Foo.class;
+//         clazz.getSimpleName();
+//     }
+// }
+
+package com/n;
+
+super public final class InnerClassesTest$Foo version 53:0 {
+
+    public Method "<init>":"()V" stack 1 locals 1 {
+        aload_0;
+        invokespecial Method java/lang/Object."<init>":"()V";
+        return;
+    }
+
+public static final InnerClass Foo=class InnerClassesTest$Foo of class InnerClassesTest;
+
+} // end Class InnerClassesTest$Foo
+
+
+super public final class InnerClassesTest version 53:0 {
+
+
+    public Method "<init>":"()V" stack 1 locals 1 {
+        aload_0;
+        invokespecial    Method java/lang/Object."<init>":"()V";
+        return;
+    }
+
+    Method unused:"()V" stack 2 locals 1 {
+        new    class com/g/Child;
+        dup;
+        invokespecial    Method com/g/Child."<init>":"()V";
+        invokevirtual    Method com/g/Child.setJobName:"()Lcom/g/Base$Builder;";
+        pop;
+        return;
+    }
+
+    public static Method main:"([Ljava/lang/String;)V" stack 1 locals 2 {
+        ldc    class InnerClassesTest$Foo;
+        astore_1;
+        aload_1;
+        invokevirtual    Method java/lang/Class.getSimpleName:"()Ljava/lang/String;";
+        pop;
+        return;
+    }
+
+static InnerClass Builder=class com/g/Base$Builder of class com/g/Base;
+public static final InnerClass Foo=class InnerClassesTest$Foo of class InnerClassesTest;
+
+} // end Class InnerClassesTest