8350029: Illegal invokespecial interface not caught by verification

Reviewed-by: coleenp, matsaave

src/hotspot/share/classfile/verificationType.cpp

@@ -48,41 +48,50 @@ VerificationType VerificationType::from_tag(u1 tag) {
   }
 }
 
-bool VerificationType::resolve_and_check_assignability(InstanceKlass* klass, Symbol* name,
+// Potentially resolve the target class and from class, and check whether the from class is assignable
-         Symbol* from_name, bool from_field_is_protected, bool from_is_array, bool from_is_object, TRAPS) {
+// to the target class. The current_klass is the class being verified - it could also be the target in some
+// cases, and otherwise is needed to obtain the correct classloader for resolving the other classes.
+bool VerificationType::resolve_and_check_assignability(InstanceKlass* current_klass, Symbol* target_name, Symbol* from_name,
+                                                       bool from_field_is_protected, bool from_is_array,
+                                                       bool from_is_object, bool* target_is_interface, TRAPS) {
   HandleMark hm(THREAD);
-  Klass* this_class;
+  Klass* target_klass;
-  if (klass->is_hidden() && klass->name() == name) {
+  if (current_klass->is_hidden() && current_klass->name() == target_name) {
-    this_class = klass;
+    target_klass = current_klass;
   } else {
-    this_class = SystemDictionary::resolve_or_fail(
+    target_klass = SystemDictionary::resolve_or_fail(
-      name, Handle(THREAD, klass->class_loader()), true, CHECK_false);
+      target_name, Handle(THREAD, current_klass->class_loader()), true, CHECK_false);
     if (log_is_enabled(Debug, class, resolve)) {
-      Verifier::trace_class_resolution(this_class, klass);
+      Verifier::trace_class_resolution(target_klass, current_klass);
     }
   }
 
-  if (this_class->is_interface() && (!from_field_is_protected ||
+  bool is_intf = target_klass->is_interface();
+  if (target_is_interface != nullptr) {
+    *target_is_interface = is_intf;
+  }
+
+  if (is_intf && (!from_field_is_protected ||
       from_name != vmSymbols::java_lang_Object())) {
     // If we are not trying to access a protected field or method in
     // java.lang.Object then, for arrays, we only allow assignability
     // to interfaces java.lang.Cloneable and java.io.Serializable.
     // Otherwise, we treat interfaces as java.lang.Object.
     return !from_is_array ||
-      this_class == vmClasses::Cloneable_klass() ||
+      target_klass == vmClasses::Cloneable_klass() ||
-      this_class == vmClasses::Serializable_klass();
+      target_klass == vmClasses::Serializable_klass();
   } else if (from_is_object) {
-    Klass* from_class;
+    Klass* from_klass;
-    if (klass->is_hidden() && klass->name() == from_name) {
+    if (current_klass->is_hidden() && current_klass->name() == from_name) {
-      from_class = klass;
+      from_klass = current_klass;
     } else {
-      from_class = SystemDictionary::resolve_or_fail(
+      from_klass = SystemDictionary::resolve_or_fail(
-        from_name, Handle(THREAD, klass->class_loader()), true, CHECK_false);
+        from_name, Handle(THREAD, current_klass->class_loader()), true, CHECK_false);
       if (log_is_enabled(Debug, class, resolve)) {
-        Verifier::trace_class_resolution(from_class, klass);
+        Verifier::trace_class_resolution(from_klass, current_klass);
       }
     }
-    return from_class->is_subclass_of(this_class);
+    return from_klass->is_subclass_of(target_klass);
   }
 
   return false;
@@ -90,8 +99,8 @@ bool VerificationType::resolve_and_check_assignability(InstanceKlass* klass, Sym
 
 bool VerificationType::is_reference_assignable_from(
     const VerificationType& from, ClassVerifier* context,
-    bool from_field_is_protected, TRAPS) const {
+    bool from_field_is_protected, bool* this_is_interface, TRAPS) const {
-  InstanceKlass* klass = context->current_class();
+
   if (from.is_null()) {
     // null is assignable to any reference
     return true;
@@ -109,7 +118,7 @@ bool VerificationType::is_reference_assignable_from(
 #if INCLUDE_CDS
     if (CDSConfig::is_dumping_archive()) {
       bool skip_assignability_check = false;
-      SystemDictionaryShared::add_verification_constraint(klass,
+      SystemDictionaryShared::add_verification_constraint(context->current_class(),
               name(), from.name(), from_field_is_protected, from.is_array(),
               from.is_object(), &skip_assignability_check);
       if (skip_assignability_check) {
@@ -119,8 +128,9 @@ bool VerificationType::is_reference_assignable_from(
       }
     }
 #endif
-    return resolve_and_check_assignability(klass, name(), from.name(),
+    return resolve_and_check_assignability(context->current_class(), name(), from.name(),
-          from_field_is_protected, from.is_array(), from.is_object(), THREAD);
+                                           from_field_is_protected, from.is_array(),
+                                           from.is_object(), this_is_interface, THREAD);
   } else if (is_array() && from.is_array()) {
     VerificationType comp_this = get_component(context);
     VerificationType comp_from = from.get_component(context);

src/hotspot/share/classfile/verificationType.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2025, 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
@@ -288,7 +288,7 @@ class VerificationType {
           if (is_reference() && from.is_reference()) {
             return is_reference_assignable_from(from, context,
                                                 from_field_is_protected,
-                                                THREAD);
+                                                nullptr, THREAD);
           } else {
             return false;
           }
@@ -327,17 +327,24 @@ class VerificationType {
 
   void print_on(outputStream* st) const;
 
- private:
+  bool is_reference_assignable_from(const VerificationType& from, ClassVerifier* context,
+                                    bool from_field_is_protected, bool* this_is_interface, TRAPS) const;
 
-  bool is_reference_assignable_from(
+  static bool resolve_and_check_assignability(InstanceKlass* current_klass, Symbol* target_name,
-    const VerificationType&, ClassVerifier*, bool from_field_is_protected,
-    TRAPS) const;
-
- public:
-  static bool resolve_and_check_assignability(InstanceKlass* klass, Symbol* name,
                                               Symbol* from_name, bool from_field_is_protected,
                                               bool from_is_array, bool from_is_object,
+                                              TRAPS) {
+    return resolve_and_check_assignability(current_klass, target_name, from_name, from_field_is_protected,
+                                           from_is_array, from_is_object, nullptr, THREAD);
+  }
+
+ private:
+  static bool resolve_and_check_assignability(InstanceKlass* current_klass, Symbol* target_name,
+                                              Symbol* from_name, bool from_field_is_protected,
+                                              bool from_is_array, bool from_is_object,
+                                              bool* target_is_interface,
                                               TRAPS);
+
 };
 
 #endif // SHARE_CLASSFILE_VERIFICATIONTYPE_HPP

src/hotspot/share/classfile/verifier.cpp

@@ -2891,26 +2891,43 @@ void ClassVerifier::verify_invoke_instructions(
           "Illegal call to internal method");
       return;
     }
-  } else if (opcode == Bytecodes::_invokespecial
+  }
-             && !is_same_or_direct_interface(current_class(), current_type(), ref_class_type)
+  // invokespecial, when not <init>, must be to a method in the current class, a direct superinterface,
-             && !ref_class_type.equals(VerificationType::reference_type(
+  // or any superclass (including Object).
-                  current_class()->super()->name()))) {
+  else if (opcode == Bytecodes::_invokespecial
-    bool subtype = false;
+           && !is_same_or_direct_interface(current_class(), current_type(), ref_class_type)
-    bool have_imr_indirect = cp->tag_at(index).value() == JVM_CONSTANT_InterfaceMethodref;
+           && !ref_class_type.equals(VerificationType::reference_type(current_class()->super()->name()))) {
-    subtype = ref_class_type.is_assignable_from(
-               current_type(), this, false, CHECK_VERIFY(this));
-    if (!subtype) {
-      verify_error(ErrorContext::bad_code(bci),
-          "Bad invokespecial instruction: "
-          "current class isn't assignable to reference class.");
-       return;
-    } else if (have_imr_indirect) {
-      verify_error(ErrorContext::bad_code(bci),
-          "Bad invokespecial instruction: "
-          "interface method reference is in an indirect superinterface.");
-      return;
-    }
 
+    // We know it is not current class, direct superinterface or immediate superclass. That means it
+    // could be:
+    // - a totally unrelated class or interface
+    // - an indirect superinterface
+    // - an indirect superclass (including Object)
+    // We use the assignability test to see if it is a superclass, or else an interface, and keep track
+    // of the latter. Note that subtype can be true if we are dealing with an interface that is not actually
+    // implemented as assignability treats all interfaces as Object.
+
+    bool is_interface = false; // This can only be set true if the assignability check will return true
+                               // and we loaded the class. For any other "true" returns (e.g. same class
+                               // or Object) we either can't get here (same class already excluded above)
+                               // or we know it is not an interface (i.e. Object).
+    bool subtype = ref_class_type.is_reference_assignable_from(current_type(), this, false,
+                                                               &is_interface, CHECK_VERIFY(this));
+    if (!subtype) {  // Totally unrelated class
+      verify_error(ErrorContext::bad_code(bci),
+                   "Bad invokespecial instruction: "
+                   "current class isn't assignable to reference class.");
+      return;
+    } else {
+      // Indirect superclass (including Object), indirect interface, or unrelated interface.
+      // Any interface use is an error.
+      if (is_interface) {
+        verify_error(ErrorContext::bad_code(bci),
+                     "Bad invokespecial instruction: "
+                     "interface method to invoke is not in a direct superinterface.");
+        return;
+      }
+    }
   }
 
   // Get the verification types for the method's arguments.

src/hotspot/share/interpreter/linkResolver.cpp

@@ -1200,7 +1200,7 @@ Method* LinkResolver::linktime_resolve_special_method(const LinkInfo& link_info,
   }
 
   // ensure that invokespecial's interface method reference is in
-  // a direct superinterface, not an indirect superinterface
+  // a direct superinterface, not an indirect superinterface or unrelated interface
   Klass* current_klass = link_info.current_klass();
   if (current_klass != nullptr && resolved_klass->is_interface()) {
     InstanceKlass* klass_to_check = InstanceKlass::cast(current_klass);
@@ -1209,7 +1209,7 @@ Method* LinkResolver::linktime_resolve_special_method(const LinkInfo& link_info,
       stringStream ss;
       ss.print("Interface method reference: '");
       resolved_method->print_external_name(&ss);
-      ss.print("', is in an indirect superinterface of %s",
+      ss.print("', is not in a direct superinterface of %s",
                current_klass->external_name());
       THROW_MSG_NULL(vmSymbols::java_lang_IncompatibleClassChangeError(), ss.as_string());
     }

test/hotspot/jtreg/runtime/verifier/invokespecial/Run.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2025, 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.
+ */
+
+/**
+ * A class with a run()V method that doesn't implement Runnable.
+ */
+public class Run {
+    public void run() { }
+}

test/hotspot/jtreg/runtime/verifier/invokespecial/TestInvokeSpecialInterface.java

@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2025, 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 id=verified
+ * @compile Run.java UseMethodRef.jasm UseInterfaceMethodRef.jasm TestInvokeSpecialInterface.java
+ * @run main/othervm TestInvokeSpecialInterface true
+ */
+
+/*
+ * @test id=unverified
+ * @compile Run.java UseMethodRef.jasm UseInterfaceMethodRef.jasm TestInvokeSpecialInterface.java
+ * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:-BytecodeVerificationRemote TestInvokeSpecialInterface false
+ */
+
+public class TestInvokeSpecialInterface {
+    public static void main(String[] args) throws Throwable {
+        if (args[0].equals("true")) {
+            check_verified();
+        } else {
+            check_unverified();
+        }
+    }
+
+    static void check_verified() {
+        String veMsg = "interface method to invoke is not in a direct superinterface";
+        try {
+            UseMethodRef t = new UseMethodRef();
+            UseMethodRef.test(t);
+        }
+        catch(VerifyError ve) {
+            if (ve.getMessage().contains(veMsg)) {
+                System.out.println("Got expected: " + ve);
+            } else {
+                throw new RuntimeException("Unexpected VerifyError thrown", ve);
+            }
+        }
+
+        try {
+            UseInterfaceMethodRef t = new UseInterfaceMethodRef();
+            UseInterfaceMethodRef.test(t);
+        }
+        catch(VerifyError ve) {
+            if (ve.getMessage().contains(veMsg)) {
+                System.out.println("Got expected: " + ve);
+            } else {
+                throw new RuntimeException("Unexpected VerifyError thrown", ve);
+            }
+        }
+    }
+
+    static void check_unverified() {
+        try {
+            UseMethodRef t = new UseMethodRef();
+            UseMethodRef.test(t);
+        }
+        catch(IncompatibleClassChangeError icce) {
+            String icceMsg = "Method 'void java.lang.Runnable.run()' must be InterfaceMethodref constant";
+            if (icce.getMessage().contains(icceMsg)) {
+                System.out.println("Got expected: " + icce);
+            } else {
+                throw new RuntimeException("Unexpected IncompatibleClassChangeError", icce);
+            }
+        }
+
+        try {
+            UseInterfaceMethodRef t = new UseInterfaceMethodRef();
+            UseInterfaceMethodRef.test(t);
+        }
+        catch(IncompatibleClassChangeError icce) {
+            String icceMsg = "Interface method reference: 'void java.lang.Runnable.run()', is not in a direct superinterface of UseInterfaceMethodRef";
+            if (icce.getMessage().contains(icceMsg)) {
+                System.out.println("Got expected: " + icce);
+            } else {
+                throw new RuntimeException("Unexpected IncompatibleClassChangeError", icce);
+            }
+        }
+    }
+}

test/hotspot/jtreg/runtime/verifier/invokespecial/UseInterfaceMethodRef.jasm

@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2025, 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.
+ */
+
+class UseInterfaceMethodRef extends Run version 52:0 {
+
+    public Method "<init>":"()V" stack 1 locals 1 {
+        aload_0;
+        invokespecial   Method Run."<init>":"()V";
+        return;
+    }
+
+    public static Method test:"(LUseInterfaceMethodRef;)V" stack 2 {
+        aload_0;
+        invokespecial InterfaceMethod java/lang/Runnable.run:()V; // VerifyError
+        return;
+    }
+}

test/hotspot/jtreg/runtime/verifier/invokespecial/UseMethodRef.jasm

@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2025, 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.
+ */
+
+class UseMethodRef extends Run version 52:0 {
+
+    public Method "<init>":"()V" stack 1 locals 1 {
+        aload_0;
+        invokespecial   Method Run."<init>":"()V";
+        return;
+    }
+
+    public static Method test:"(LUseMethodRef;)V" stack 2 {
+        aload_0;
+        invokespecial Method java/lang/Runnable.run:()V; // VerifyError
+        return;
+    }
+}