8282218: C1: Missing side effects of dynamic class loading during constant linkage

Reviewed-by: thartmann, kvn

src/hotspot/share/c1/c1_GraphBuilder.cpp

@@ -952,7 +952,7 @@ void GraphBuilder::load_constant() {
       case T_ARRAY  : // fall-through
       case T_OBJECT : {
         ciObject* obj = con.as_object();
-        if (!obj->is_loaded() || (PatchALot && (obj->is_null_object() || obj->klass() != ciEnv::current()->String_klass()))) {
+        if (!obj->is_loaded() || (PatchALot && !stream()->is_string_constant())) {
           // A Class, MethodType, MethodHandle, Dynamic, or String.
           patch_state = copy_state_before();
           t = new ObjectConstant(obj);
@@ -973,7 +973,9 @@ void GraphBuilder::load_constant() {
     }
     Value x;
     if (patch_state != NULL) {
-      bool kills_memory = stream()->is_dynamic_constant(); // arbitrary memory effects from running BSM during linkage
+      // Arbitrary memory effects from running BSM or class loading (using custom loader) during linkage.
+      bool kills_memory = stream()->is_dynamic_constant() ||
+                          (!stream()->is_string_constant() && !method()->holder()->has_trusted_loader());
       x = new Constant(t, patch_state, kills_memory);
     } else {
       x = new Constant(t);

src/hotspot/share/ci/ciInstanceKlass.cpp

@@ -100,6 +100,8 @@ ciInstanceKlass::ciInstanceKlass(Klass* k) :
     _is_shared = true;
   }
 
+  _has_trusted_loader = compute_has_trusted_loader();
+
   // Lazy fields get filled in only upon request.
   _super  = NULL;
   _java_mirror = NULL;
@@ -132,6 +134,7 @@ ciInstanceKlass::ciInstanceKlass(ciSymbol* name,
   _super = NULL;
   _java_mirror = NULL;
   _field_cache = NULL;
+  _has_trusted_loader = compute_has_trusted_loader();
 }
 
 
@@ -568,6 +571,15 @@ bool ciInstanceKlass::has_object_fields() const {
     );
 }
 
+bool ciInstanceKlass::compute_has_trusted_loader() {
+  ASSERT_IN_VM;
+  oop loader_oop = loader();
+  if (loader_oop == NULL) {
+    return true; // bootstrap class loader
+  }
+  return java_lang_ClassLoader::is_trusted_loader(loader_oop);
+}
+
 // ------------------------------------------------------------------
 // ciInstanceKlass::find_method
 //

src/hotspot/share/ci/ciInstanceKlass.hpp

@@ -59,6 +59,7 @@ private:
   bool                   _has_nonstatic_concrete_methods;
   bool                   _is_hidden;
   bool                   _is_record;
+  bool                   _has_trusted_loader;
 
   ciFlags                _flags;
 
@@ -107,6 +108,7 @@ protected:
   bool compute_shared_has_subklass();
   int  compute_nonstatic_fields();
   GrowableArray<ciField*>* compute_nonstatic_fields_impl(GrowableArray<ciField*>* super_fields);
+  bool compute_has_trusted_loader();
 
   // Update the init_state for shared klasses
   void update_if_shared(InstanceKlass::ClassState expected) {
@@ -287,6 +289,10 @@ public:
     return !is_interface() && !is_abstract();
   }
 
+  bool has_trusted_loader() const {
+    return _has_trusted_loader;
+  }
+
   // Replay support
 
   // Dump the current state of this klass for compilation replay.

src/hotspot/share/ci/ciStreams.cpp

@@ -259,7 +259,7 @@ constantTag ciBytecodeStream::get_constant_pool_tag(int index) const {
 // ------------------------------------------------------------------
 // ciBytecodeStream::get_raw_pool_tag
 //
-constantTag ciBytecodeStream::get_raw_pool_tag(int index) const {
+constantTag ciBytecodeStream::get_raw_pool_tag_at(int index) const {
   VM_ENTRY_MARK;
   return _method->get_Method()->constants()->tag_at(index);
 }

src/hotspot/share/ci/ciStreams.hpp

@@ -230,9 +230,14 @@ public:
   constantTag get_constant_pool_tag(int index) const;
   BasicType   get_basic_type_for_constant_at(int index) const;
 
-  constantTag get_raw_pool_tag(int index) const;
+  constantTag get_raw_pool_tag_at(int index) const;
 
-  // True if the klass-using bytecode points to an unresolved klass
+  constantTag get_raw_pool_tag() const {
+    int index = get_constant_pool_index();
+    return get_raw_pool_tag_at(index);
+  }
+
+    // True if the klass-using bytecode points to an unresolved klass
   bool is_unresolved_klass() const {
     constantTag tag = get_constant_pool_tag(get_klass_index());
     return tag.is_unresolved_klass();
@@ -243,12 +248,20 @@ public:
            cur_bc() == Bytecodes::_ldc_w  ||
            cur_bc() == Bytecodes::_ldc2_w, "not supported: %s", Bytecodes::name(cur_bc()));
 
-    int index = get_constant_pool_index();
-    constantTag tag = get_raw_pool_tag(index);
+    constantTag tag = get_raw_pool_tag();
     return tag.is_dynamic_constant() ||
            tag.is_dynamic_constant_in_error();
   }
 
+  bool is_string_constant() const {
+    assert(cur_bc() == Bytecodes::_ldc    ||
+           cur_bc() == Bytecodes::_ldc_w  ||
+           cur_bc() == Bytecodes::_ldc2_w, "not supported: %s", Bytecodes::name(cur_bc()));
+
+    constantTag tag = get_raw_pool_tag();
+    return tag.is_string();
+  }
+
   bool is_in_error() const {
     assert(cur_bc() == Bytecodes::_ldc    ||
            cur_bc() == Bytecodes::_ldc_w  ||

test/hotspot/jtreg/compiler/c1/TestClassConstantPatching.java

@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) 2022, 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 8282218
+ *
+ * @run main/othervm -Xcomp -XX:TieredStopAtLevel=1
+ *                   -XX:CompileCommand=compileonly,compiler.c1.TestClassConstantPatching$Test::run
+ *                      compiler.c1.TestClassConstantPatching
+ */
+package compiler.c1;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.reflect.Method;
+
+public class TestClassConstantPatching {
+    public static int COUNTER = 0;
+
+    static class CustomLoader extends ClassLoader {
+        @Override
+        protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+            if (name.startsWith(Test.class.getName())) {
+                Class<?> c = findLoadedClass(name);
+                if (c != null) {
+                    return c;
+                }
+                try {
+                    COUNTER++; // update counter on loading
+
+                    InputStream in = getSystemResourceAsStream(name.replace('.', File.separatorChar) + ".class");
+                    byte[] buf = in.readAllBytes();
+                    return defineClass(name, buf, 0, buf.length);
+                } catch (IOException e) {
+                    throw new ClassNotFoundException(name);
+                }
+            }
+            return super.loadClass(name, resolve);
+        }
+    }
+
+    public static class Test {
+        static class T {}
+
+        public static Class<?> run(boolean cond) {
+            int before = COUNTER;
+            Class<?> c = null;
+            if (cond) {
+                c = T.class;
+            }
+            int after = COUNTER;
+            if (cond && before == after) {
+                throw new AssertionError("missed update");
+            }
+            return c;
+        }
+    }
+
+    public static void main(String[] args) throws ReflectiveOperationException {
+        ClassLoader cl = new CustomLoader();
+
+        Class.forName(TestClassConstantPatching.class.getName(), true, cl); // preload counter holder class
+
+        Class<?> test = Class.forName(Test.class.getName(), true, cl);
+
+        Method m = test.getDeclaredMethod("run", boolean.class);
+
+        m.invoke(null, false);
+        m.invoke(null, true);
+
+        System.out.println("TEST PASSED");
+    }
+}
+