8212117: Class.forName may return a reference to a loaded but not linked Class

Brent Christian · Brent Christian · commit a4613d4a8f3f · 2019-09-09T11:04:04.000-07:00
Reviewed-by: dholmes, mchung
diff --git a/make/hotspot/symbols/symbols-unix b/make/hotspot/symbols/symbols-unix
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2016, 2019, 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
@@ -149,6 +149,7 @@ JVM_IsSupportedJNIVersion
 JVM_IsThreadAlive
 JVM_IsVMGeneratedMethodIx
 JVM_LatestUserDefinedLoader
+JVM_LinkClass
 JVM_LoadLibrary
 JVM_MaxMemory
 JVM_MaxObjectInspectionAge
diff --git a/src/hotspot/share/include/jvm.h b/src/hotspot/share/include/jvm.h
@@ -345,6 +345,11 @@ JVM_GetCallerClass(JNIEnv *env);
 JNIEXPORT jclass JNICALL
 JVM_FindPrimitiveClass(JNIEnv *env, const char *utf);
 
+/*
+ * Link the 'arg' class
+ */
+JNIEXPORT void JNICALL
+JVM_LinkClass(JNIEnv *env, jclass classClass, jclass arg);
 
 /*
  * Find a class from a boot class loader. Returns NULL if class not found.
diff --git a/src/hotspot/share/prims/jni.cpp b/src/hotspot/share/prims/jni.cpp
@@ -417,7 +417,7 @@ JNI_ENTRY(jclass, jni_FindClass(JNIEnv *env, const char *name))
   }
 
   TempNewSymbol sym = SymbolTable::new_symbol(name);
-  result = find_class_from_class_loader(env, sym, true, loader,
+  result = find_class_from_class_loader(env, sym, true, true, loader,
                                         protection_domain, true, thread);
 
   if (log_is_enabled(Debug, class, resolve) && result != NULL) {
@@ -3289,7 +3289,7 @@ static jclass lookupOne(JNIEnv* env, const char* name, TRAPS) {
   Handle protection_domain; // null protection domain
 
   TempNewSymbol sym = SymbolTable::new_symbol(name);
-  jclass result =  find_class_from_class_loader(env, sym, true, loader, protection_domain, true, CHECK_NULL);
+  jclass result =  find_class_from_class_loader(env, sym, true, true, loader, protection_domain, true, CHECK_NULL);
 
   if (log_is_enabled(Debug, class, resolve) && result != NULL) {
     trace_class_resolution(java_lang_Class::as_Klass(JNIHandles::resolve_non_null(result)));
diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp
@@ -717,6 +717,17 @@ JVM_END
 // Misc. class handling ///////////////////////////////////////////////////////////
 
 
+JVM_ENTRY(void, JVM_LinkClass(JNIEnv* env, jclass classClass, jclass arg))
+  JVMWrapper("JVM_LinkClass");
+
+  oop r = JNIHandles::resolve(arg);
+  Klass* klass = java_lang_Class::as_Klass(r);
+
+  if (!ClassForNameDeferLinking && klass->is_instance_klass()) {
+    InstanceKlass::cast(klass)->link_class(CHECK);
+  }
+JVM_END
+
 JVM_ENTRY(jclass, JVM_GetCallerClass(JNIEnv* env))
   JVMWrapper("JVM_GetCallerClass");
 
@@ -827,9 +838,10 @@ JVM_ENTRY(jclass, JVM_FindClassFromCaller(JNIEnv* env, const char* name,
 
   Handle h_loader(THREAD, loader_oop);
   Handle h_prot(THREAD, protection_domain);
-  jclass result = find_class_from_class_loader(env, h_name, init, h_loader,
-                                               h_prot, false, THREAD);
 
+  jboolean link = !ClassForNameDeferLinking;
+  jclass result = find_class_from_class_loader(env, h_name, init, link, h_loader,
+                                               h_prot, false, THREAD);
   if (log_is_enabled(Debug, class, resolve) && result != NULL) {
     trace_class_resolution(java_lang_Class::as_Klass(JNIHandles::resolve_non_null(result)));
   }
@@ -866,7 +878,7 @@ JVM_ENTRY(jclass, JVM_FindClassFromClass(JNIEnv *env, const char *name,
   }
   Handle h_loader(THREAD, class_loader);
   Handle h_prot  (THREAD, protection_domain);
-  jclass result = find_class_from_class_loader(env, h_name, init, h_loader,
+  jclass result = find_class_from_class_loader(env, h_name, init, false, h_loader,
                                                h_prot, true, thread);
 
   if (log_is_enabled(Debug, class, resolve) && result != NULL) {
@@ -3424,9 +3436,12 @@ JNIEXPORT void JNICALL JVM_RawMonitorExit(void *mon) {
 
 // Shared JNI/JVM entry points //////////////////////////////////////////////////////////////
 
-jclass find_class_from_class_loader(JNIEnv* env, Symbol* name, jboolean init,
+jclass find_class_from_class_loader(JNIEnv* env, Symbol* name, jboolean init, jboolean link,
                                     Handle loader, Handle protection_domain,
                                     jboolean throwError, TRAPS) {
+  // Initialization also implies linking - check for coherent args
+  assert((init && link) || !init, "incorrect use of init/link arguments");
+
   // Security Note:
   //   The Java level wrapper will perform the necessary security check allowing
   //   us to pass the NULL as the initiating class loader.  The VM is responsible for
@@ -3435,9 +3450,11 @@ jclass find_class_from_class_loader(JNIEnv* env, Symbol* name, jboolean init,
   //   if there is no security manager in 3-arg Class.forName().
   Klass* klass = SystemDictionary::resolve_or_fail(name, loader, protection_domain, throwError != 0, CHECK_NULL);
 
-  // Check if we should initialize the class
+  // Check if we should initialize the class (which implies linking), or just link it
   if (init && klass->is_instance_klass()) {
     klass->initialize(CHECK_NULL);
+  } else if (link && klass->is_instance_klass()) {
+    InstanceKlass::cast(klass)->link_class(CHECK_NULL);
   }
   return (jclass) JNIHandles::make_local(env, klass->java_mirror());
 }
diff --git a/src/hotspot/share/prims/jvm_misc.hpp b/src/hotspot/share/prims/jvm_misc.hpp
@@ -31,7 +31,8 @@
 // Useful entry points shared by JNI and JVM interface.
 // We do not allow real JNI or JVM entry point to call each other.
 
-jclass find_class_from_class_loader(JNIEnv* env, Symbol* name, jboolean init, Handle loader, Handle protection_domain, jboolean throwError, TRAPS);
+jclass find_class_from_class_loader(JNIEnv* env, Symbol* name, jboolean init, jboolean link,
+                                    Handle loader, Handle protection_domain, jboolean throwError, TRAPS);
 
 void trace_class_resolution(Klass* to_class);
 
diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp
@@ -2164,6 +2164,9 @@ const size_t minimumSymbolTableSize = 1024;
           "Maximum total size of NIO direct-buffer allocations")            \
           range(0, max_jlong)                                               \
                                                                             \
+  product(bool, ClassForNameDeferLinking, false,                            \
+          "Revert to not linking in Class.forName()")                       \
+                                                                            \
   /* Flags used for temporary code during development  */                   \
                                                                             \
   diagnostic(bool, UseNewCode, false,                                       \
diff --git a/src/java.base/share/classes/java/lang/Class.java b/src/java.base/share/classes/java/lang/Class.java
@@ -392,6 +392,10 @@ public static Class<?> forName(String className)
      *
      * @see       java.lang.Class#forName(String)
      * @see       java.lang.ClassLoader
+     *
+     * @jls 12.2 Loading of Classes and Interfaces
+     * @jls 12.3 Linking of Classes and Interfaces
+     * @jls 12.4 Initialization of Classes and Interfaces
      * @since     1.2
      */
     @CallerSensitive
@@ -438,6 +442,10 @@ private static native Class<?> forName0(String name, boolean initialize,
      * <p> This method does not check whether the requested class is
      * accessible to its caller. </p>
      *
+     * <p> Note that this method throws errors related to loading and linking as
+     * specified in Sections 12.2 and 12.3 of <em>The Java Language
+     * Specification</em>.
+     *
      * @apiNote
      * This method returns {@code null} on failure rather than
      * throwing a {@link ClassNotFoundException}, as is done by
@@ -465,6 +473,8 @@ private static native Class<?> forName0(String name, boolean initialize,
      *         in a module.</li>
      *         </ul>
      *
+     * @jls 12.2 Loading of Classes and Interfaces
+     * @jls 12.3 Linking of Classes and Interfaces
      * @since 9
      * @spec JPMS
      */
@@ -488,13 +498,21 @@ public static Class<?> forName(Module module, String name) {
             cl = module.getClassLoader();
         }
 
+        Class<?> ret;
         if (cl != null) {
-            return cl.loadClass(module, name);
+            ret = cl.loadClass(module, name);
         } else {
-            return BootLoader.loadClass(module, name);
+            ret = BootLoader.loadClass(module, name);
         }
+        if (ret != null) {
+            // The loaded class should also be linked
+            linkClass(ret);
+        }
+        return ret;
     }
 
+    private static native void linkClass(Class<?> c);
+
     /**
      * Creates a new instance of the class represented by this {@code Class}
      * object.  The class is instantiated as if by a {@code new}
diff --git a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
@@ -1927,12 +1927,17 @@ public MethodHandle findConstructor(Class<?> refc, MethodType type) throws NoSuc
         }
 
         /**
-         * Looks up a class by name from the lookup context defined by this {@code Lookup} object. The static
+         * Looks up a class by name from the lookup context defined by this {@code Lookup} object.
+         * This method attempts to locate, load, and link the class, and then determines whether
+         * the class is accessible to this {@code Lookup} object.  The static
          * initializer of the class is not run.
          * <p>
          * The lookup context here is determined by the {@linkplain #lookupClass() lookup class}, its class
-         * loader, and the {@linkplain #lookupModes() lookup modes}. In particular, the method first attempts to
-         * load the requested class, and then determines whether the class is accessible to this lookup object.
+         * loader, and the {@linkplain #lookupModes() lookup modes}.
+         * <p>
+         * Note that this method throws errors related to loading and linking as
+         * specified in Sections 12.2 and 12.3 of <em>The Java Language
+         * Specification</em>.
          *
          * @param targetName the fully qualified name of the class to be looked up.
          * @return the requested class.
@@ -1944,6 +1949,9 @@ public MethodHandle findConstructor(Class<?> refc, MethodType type) throws NoSuc
          * modes.
          * @exception SecurityException if a security manager is present and it
          *                              <a href="MethodHandles.Lookup.html#secmgr">refuses access</a>
+         *
+         * @jls 12.2 Loading of Classes and Interfaces
+         * @jls 12.3 Linking of Classes and Interfaces
          * @since 9
          */
         public Class<?> findClass(String targetName) throws ClassNotFoundException, IllegalAccessException {
diff --git a/src/java.base/share/classes/sun/launcher/LauncherHelper.java b/src/java.base/share/classes/sun/launcher/LauncherHelper.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2007, 2019, 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
@@ -62,6 +62,7 @@
 import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.security.AccessControlException;
 import java.text.Normalizer;
 import java.text.MessageFormat;
 import java.util.ArrayList;
@@ -724,6 +725,9 @@ private static Class<?> loadModuleMainClass(String what) {
         } catch (LinkageError le) {
             abort(null, "java.launcher.module.error3", mainClass, m.getName(),
                     le.getClass().getName() + ": " + le.getLocalizedMessage());
+        } catch (AccessControlException ace) {
+            abort(ace, "java.launcher.module.error5", mainClass, m.getName(),
+                    ace.getClass().getName(), ace.getLocalizedMessage());
         }
         if (c == null) {
             abort(null, "java.launcher.module.error2", mainClass, mainModule);
@@ -780,6 +784,9 @@ private static Class<?> loadMainClass(int mode, String what) {
         } catch (LinkageError le) {
             abort(le, "java.launcher.cls.error6", cn,
                     le.getClass().getName() + ": " + le.getLocalizedMessage());
+        } catch (AccessControlException ace) {
+            abort(ace, "java.launcher.cls.error7", cn,
+                    ace.getClass().getName(), ace.getLocalizedMessage());
         }
         return mainClass;
     }
diff --git a/src/java.base/share/native/libjava/Class.c b/src/java.base/share/native/libjava/Class.c
@@ -76,6 +76,7 @@ static JNINativeMethod methods[] = {
     {"getRawTypeAnnotations", "()" BA,      (void *)&JVM_GetClassTypeAnnotations},
     {"getNestHost0",         "()" CLS,      (void *)&JVM_GetNestHost},
     {"getNestMembers0",      "()[" CLS,     (void *)&JVM_GetNestMembers},
+    {"linkClass",            "(" CLS ")V",  (void *)&JVM_LinkClass},
 };
 
 #undef OBJ
diff --git a/test/hotspot/jtreg/gc/logging/TestMetaSpaceLog.java b/test/hotspot/jtreg/gc/logging/TestMetaSpaceLog.java
@@ -130,7 +130,7 @@ public static void main(String args[]) {
     public static void loadClass(WhiteBox wb) {
       try {
         URLClassLoader ucl = new URLClassLoader(urls);
-        Class.forName("case00", false, ucl);
+        ucl.loadClass("case00");
       } catch (Exception e) {
         e.printStackTrace();
       }
diff --git a/test/hotspot/jtreg/serviceability/jvmti/ClassStatus/ClassStatus.java b/test/hotspot/jtreg/serviceability/jvmti/ClassStatus/ClassStatus.java
@@ -0,0 +1,92 @@
+/*
+ * Copyright (c) 2019, 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 8212117
+ * @summary Verify JVMTI GetClassStatus returns CLASS_PREPARE after call to Class.forName()
+ * @run main/othervm/native -agentlib:ClassStatus ClassStatus
+ */
+
+
+public class ClassStatus {
+    static {
+        try {
+            System.out.println("ClassStatus static block");
+            System.loadLibrary("ClassStatus");
+        } catch (UnsatisfiedLinkError ule) {
+            System.err.println("Could not load ClassStatus library");
+            System.err.println("java.library.path: "
+                + System.getProperty("java.library.path"));
+            throw ule;
+        }
+    }
+
+    static native int check(Class klass);
+
+    public static void main(String[] args) throws ClassNotFoundException {
+        ClassLoader loader = ClassStatus.class.getClassLoader();
+        Module module = loader.getUnnamedModule();
+
+        // Load class, but don't initialize it
+        Class foo2 = Class.forName(module, "Foo2");
+        Class foo3 = Class.forName("Foo3", false, loader);
+
+        System.out.println("Loaded: " + foo2);
+        System.out.println("Loaded: " + foo3);
+
+        int status2 = check(foo2);
+        int status3 = check(foo3);
+
+        new Foo2().bar();
+        new Foo3().bar();
+
+        if (status2 != 0) {
+            System.out.println("The agent returned non-zero exit status for Foo2: " + status2);
+        }
+        if (status3 != 0) {
+            System.out.println("The agent returned non-zero exit status for Foo3: " + status3);
+        }
+        if (status2 != 0 || status3 != 0) {
+            throw new RuntimeException("Non-zero status returned from the agent");
+        }
+    }
+}
+
+class Foo2 {
+    static {
+        System.out.println("Foo2 is initialized");
+    }
+    void bar() {
+        System.out.println("Foo2.bar() is called");
+    }
+}
+
+class Foo3 {
+    static {
+        System.out.println("Foo3 is initialized");
+    }
+    void bar() {
+        System.out.println("Foo3.bar() is called");
+    }
+}
diff --git a/test/hotspot/jtreg/serviceability/jvmti/ClassStatus/libClassStatus.c b/test/hotspot/jtreg/serviceability/jvmti/ClassStatus/libClassStatus.c

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`#`
`2`		`-# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.`
	`2`	`+# Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.`
`3`	`3`	`# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.`
`4`	`4`	`#`
`5`	`5`	`# This code is free software; you can redistribute it and/or modify it`
`@@ -149,6 +149,7 @@ JVM_IsSupportedJNIVersion`
`149`	`149`	`JVM_IsThreadAlive`
`150`	`150`	`JVM_IsVMGeneratedMethodIx`
`151`	`151`	`JVM_LatestUserDefinedLoader`
	`152`	`+JVM_LinkClass`
`152`	`153`	`JVM_LoadLibrary`
`153`	`154`	`JVM_MaxMemory`
`154`	`155`	`JVM_MaxObjectInspectionAge`
Original file line number	Diff line number	Diff line change
`@@ -2164,6 +2164,9 @@ const size_t minimumSymbolTableSize = 1024;`
`2164`	`2164`	`"Maximum total size of NIO direct-buffer allocations") \`
`2165`	`2165`	`range(0, max_jlong) \`
`2166`	`2166`	`\`
	`2167`	`+ product(bool, ClassForNameDeferLinking, false, \`
	`2168`	`+ "Revert to not linking in Class.forName()") \`
	`2169`	`+ \`
`2167`	`2170`	`/* Flags used for temporary code during development */ \`
`2168`	`2171`	`\`
`2169`	`2172`	`diagnostic(bool, UseNewCode, false, \`