8154172: C1: NPE is thrown instead of linkage error when invoking nonexistent method
authorvlivanov
Mon, 25 Apr 2016 21:25:22 +0300
changeset 38132 ba888a4f352a
parent 38131 7bb27ec1a3d8
child 38133 78b95467b9f1
8154172: C1: NPE is thrown instead of linkage error when invoking nonexistent method Reviewed-by: kvn
hotspot/src/share/vm/c1/c1_GraphBuilder.cpp
hotspot/test/compiler/linkage/CallSites.jasm
hotspot/test/compiler/linkage/LinkageErrors.java
--- a/hotspot/src/share/vm/c1/c1_GraphBuilder.cpp	Mon Apr 25 18:36:27 2016 +0300
+++ b/hotspot/src/share/vm/c1/c1_GraphBuilder.cpp	Mon Apr 25 21:25:22 2016 +0300
@@ -1742,24 +1742,13 @@
   const Bytecodes::Code bc_raw = stream()->cur_bc_raw();
   assert(declared_signature != NULL, "cannot be null");
 
-  // we have to make sure the argument size (incl. the receiver)
-  // is correct for compilation (the call would fail later during
-  // linkage anyway) - was bug (gri 7/28/99)
-  {
-    // Use raw to get rewritten bytecode.
-    const bool is_invokestatic = bc_raw == Bytecodes::_invokestatic;
-    const bool allow_static =
-          is_invokestatic ||
-          bc_raw == Bytecodes::_invokehandle ||
-          bc_raw == Bytecodes::_invokedynamic;
-    if (target->is_loaded()) {
-      if (( target->is_static() && !allow_static) ||
-          (!target->is_static() &&  is_invokestatic)) {
-        BAILOUT("will cause link error");
-      }
-    }
+  ciInstanceKlass* klass = target->holder();
+
+  // Make sure there are no evident problems with linking the instruction.
+  bool is_resolved = true;
+  if (klass->is_loaded() && !target->is_loaded()) {
+    is_resolved = false; // method not found
   }
-  ciInstanceKlass* klass = target->holder();
 
   // check if CHA possible: if so, change the code to invoke_special
   ciInstanceKlass* calling_klass = method()->holder();
@@ -1804,10 +1793,6 @@
     apush(arg);
   }
 
-  // NEEDS_CLEANUP
-  // I've added the target->is_loaded() test below but I don't really understand
-  // how klass->is_loaded() can be true and yet target->is_loaded() is false.
-  // this happened while running the JCK invokevirtual tests under doit.  TKR
   ciMethod* cha_monomorphic_target = NULL;
   ciMethod* exact_target = NULL;
   Value better_receiver = NULL;
@@ -1931,12 +1916,11 @@
   }
 
   // check if we could do inlining
-  if (!PatchALot && Inline && klass->is_loaded() &&
+  if (!PatchALot && Inline && is_resolved &&
+      klass->is_loaded() && target->is_loaded() &&
       (klass->is_initialized() || klass->is_interface() && target->holder()->is_initialized())
-      && target->is_loaded()
       && !patch_for_appendix) {
     // callee is known => check if we have static binding
-    assert(target->is_loaded(), "callee must be known");
     if (code == Bytecodes::_invokestatic  ||
         code == Bytecodes::_invokespecial ||
         code == Bytecodes::_invokevirtual && target->is_final_method() ||
@@ -1993,7 +1977,7 @@
   // Currently only supported on Sparc.
   // The UseInlineCaches only controls dispatch to invokevirtuals for
   // loaded classes which we weren't able to statically bind.
-  if (!UseInlineCaches && is_loaded && code == Bytecodes::_invokevirtual
+  if (!UseInlineCaches && is_resolved && is_loaded && code == Bytecodes::_invokevirtual
       && !target->can_be_statically_bound()) {
     // Find a vtable index if one is available
     // For arrays, callee_holder is Object. Resolving the call with
@@ -2006,35 +1990,37 @@
   }
 #endif
 
-  if (recv != NULL &&
-      (code == Bytecodes::_invokespecial ||
-       !is_loaded || target->is_final())) {
-    // invokespecial always needs a NULL check.  invokevirtual where
-    // the target is final or where it's not known that whether the
-    // target is final requires a NULL check.  Otherwise normal
-    // invokevirtual will perform the null check during the lookup
-    // logic or the unverified entry point.  Profiling of calls
-    // requires that the null check is performed in all cases.
-    null_check(recv);
-  }
-
-  if (is_profiling()) {
-    if (recv != NULL && profile_calls()) {
+  if (is_resolved) {
+    // invokespecial always needs a NULL check. invokevirtual where the target is
+    // final or where it's not known whether the target is final requires a NULL check.
+    // Otherwise normal invokevirtual will perform the null check during the lookup
+    // logic or the unverified entry point.  Profiling of calls requires that
+    // the null check is performed in all cases.
+    bool do_null_check = (recv != NULL) &&
+        (code == Bytecodes::_invokespecial || !is_loaded || target->is_final() || (is_profiling() && profile_calls()));
+
+    if (do_null_check) {
       null_check(recv);
     }
-    // Note that we'd collect profile data in this method if we wanted it.
-    compilation()->set_would_profile(true);
-
-    if (profile_calls()) {
-      assert(cha_monomorphic_target == NULL || exact_target == NULL, "both can not be set");
-      ciKlass* target_klass = NULL;
-      if (cha_monomorphic_target != NULL) {
-        target_klass = cha_monomorphic_target->holder();
-      } else if (exact_target != NULL) {
-        target_klass = exact_target->holder();
+
+    if (is_profiling()) {
+      // Note that we'd collect profile data in this method if we wanted it.
+      compilation()->set_would_profile(true);
+
+      if (profile_calls()) {
+        assert(cha_monomorphic_target == NULL || exact_target == NULL, "both can not be set");
+        ciKlass* target_klass = NULL;
+        if (cha_monomorphic_target != NULL) {
+          target_klass = cha_monomorphic_target->holder();
+        } else if (exact_target != NULL) {
+          target_klass = exact_target->holder();
+        }
+        profile_call(target, recv, target_klass, collect_args_for_profiling(args, NULL, false), false);
       }
-      profile_call(target, recv, target_klass, collect_args_for_profiling(args, NULL, false), false);
     }
+  } else {
+    // No need in null check or profiling: linkage error will be thrown at runtime
+    // during resolution.
   }
 
   Invoke* result = new Invoke(code, result_type, recv, args, vtable_index, target, state_before);
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/linkage/CallSites.jasm	Mon Apr 25 21:25:22 2016 +0300
@@ -0,0 +1,91 @@
+/*
+ * Copyright (c) 2016, 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.
+ *
+ */
+super class I
+version 52:0
+{
+
+}
+
+
+super class CallSites
+version 52:0
+{
+
+    // Non-existent methods.
+
+    // I.m1()V vs I.m1(I)V
+    public static Method testI1:"(LI;)V"
+        stack 1 locals 1
+    {
+        aload_0;
+        invokeinterface InterfaceMethod I."m1":"()V", 1; // throws NSME
+        return;
+    }
+
+    // X.m1()V vs X.m1(I)V
+    public static Method testX1:"(LX;)V"
+        stack 1 locals 1
+    {
+        aload_0;
+        invokevirtual Method X."m1":"()V"; // throws NSME
+        return;
+    }
+
+    // invokestatic of instance methods.
+
+    public static Method testI2:"()V"
+        stack 1 locals 0
+    {
+        iconst_0;
+        invokestatic InterfaceMethod I."m1":"(I)V"; // throws ICCE
+        return;
+    }
+
+    public static Method testX2:"()V"
+        stack 1 locals 0
+    {
+        iconst_0;
+        invokestatic Method X."m1":"(I)V"; // throws ICCE
+        return;
+    }
+
+    // Virtual invocation of static methods.
+
+    public static Method testI3:"(LI;)V"
+        stack 1 locals 1
+    {
+        aload_0;
+        invokeinterface InterfaceMethod I."s1":"()V", 1; // throws ICCE
+        return;
+    }
+
+    public static Method testX3:"(LX;)V"
+        stack 1 locals 1
+    {
+        aload_0;
+        invokevirtual Method X."s1":"()V";  // throws ICCE
+        return;
+    }
+
+} // end Class CallSites
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/linkage/LinkageErrors.java	Mon Apr 25 21:25:22 2016 +0300
@@ -0,0 +1,106 @@
+/*
+ * Copyright (c) 2016, 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 8132879
+ * @compile CallSites.jasm
+ * @run main/othervm -Xverify:all -Xbatch -XX:CompileCommand=dontinline,Test::test* LinkageErrors
+ */
+
+import java.lang.invoke.*;
+
+interface I {
+    void m1(int i);
+    static void s1() {}
+}
+
+class A implements I {
+    public void m1(int i) {}
+}
+
+class X {
+    public        void m1(int i) {}
+    public final  void f1(int i) {}
+    public static void s1(int i) {}
+}
+
+public class LinkageErrors {
+    final static MethodHandles.Lookup L = MethodHandles.lookup();
+
+    static void test(MethodHandle mh) {
+        try {
+            mh.invokeExact();
+            throw new AssertionError("No exception thrown");
+        } catch (LinkageError e) {
+            return; // expected
+        } catch (AssertionError e) {
+            throw e; // rethrow
+        } catch (Throwable e) {
+            throw new AssertionError("Unexpected exception", e);
+        }
+    }
+
+    public static void main(String args[]) throws Throwable {
+        Class<?> test = Class.forName("CallSites");
+
+        // Non-existent method lookups.
+        MethodHandle testI1 = L.findStatic(test, "testI1", MethodType.methodType(void.class, I.class));
+        MethodHandle testX1 = L.findStatic(test, "testX1", MethodType.methodType(void.class, X.class));
+
+        MethodHandle testI1_A    = testI1.bindTo(new A());
+        MethodHandle testI1_null = testI1.bindTo(null);
+        MethodHandle testX1_X    = testX1.bindTo(new X());
+        MethodHandle testX1_null = testX1.bindTo(null);
+
+        // invokestatic of instance methods.
+        MethodHandle testI2 = L.findStatic(test, "testI2", MethodType.methodType(void.class));
+        MethodHandle testX2 = L.findStatic(test, "testX2", MethodType.methodType(void.class));
+
+        MethodHandle testI3 = L.findStatic(test, "testI3", MethodType.methodType(void.class, I.class));
+        MethodHandle testX3 = L.findStatic(test, "testX3", MethodType.methodType(void.class, X.class));
+
+        // Virtual invocation of static methods.
+        MethodHandle testI3_A    = testI3.bindTo(new A());
+        MethodHandle testI3_null = testI3.bindTo(null);
+        MethodHandle testX3_X    = testX3.bindTo(new X());
+        MethodHandle testX3_null = testX3.bindTo(null);
+
+        for (int i = 0; i < 20_000; i++) {
+            test(testI1_A);
+            test(testI1_null);
+            test(testX1_X);
+            test(testX1_null);
+
+            test(testI2);
+            test(testX2);
+
+            test(testI3_A);
+            test(testI3_null);
+            test(testX3_X);
+            test(testX3_null);
+        }
+
+        System.out.println("TEST PASSED");
+    }
+}