8229900: RedefineDeleteJmethod.java fails with -Xcheck:jni
authorcoleenp
Mon, 07 Oct 2019 10:04:01 -0400
changeset 58479 35ce0ad5870a
parent 58477 d148a7e7160c
child 58480 8ca46e186a63
child 58494 54c1ba464b78
8229900: RedefineDeleteJmethod.java fails with -Xcheck:jni Summary: Remove RedefineClasses adjustment and test, but improve checking for method/class matching. Reviewed-by: dholmes, sspitsyn
src/hotspot/share/prims/jniCheck.cpp
src/hotspot/share/prims/jniCheck.hpp
src/hotspot/share/prims/jvmtiRedefineClasses.cpp
test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineDeleteJmethod.java
test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/libRedefineDeleteJmethod.c
--- a/src/hotspot/share/prims/jniCheck.cpp	Mon Oct 07 15:58:04 2019 +0200
+++ b/src/hotspot/share/prims/jniCheck.cpp	Mon Oct 07 10:04:01 2019 -0400
@@ -448,16 +448,16 @@
 Method* jniCheck::validate_jmethod_id(JavaThread* thr, jmethodID method_id) {
   ASSERT_OOPS_ALLOWED;
   // do the fast jmethodID check first
-  Method* moop = Method::checked_resolve_jmethod_id(method_id);
-  if (moop == NULL) {
+  Method* m = Method::checked_resolve_jmethod_id(method_id);
+  if (m == NULL) {
     ReportJNIFatalError(thr, fatal_wrong_class_or_method);
   }
-  // jmethodIDs are supposed to be weak handles in the class loader data,
+  // jmethodIDs are handles in the class loader data,
   // but that can be expensive so check it last
   else if (!Method::is_method_id(method_id)) {
     ReportJNIFatalError(thr, fatal_non_weak_method);
   }
-  return moop;
+  return m;
 }
 
 
@@ -518,18 +518,29 @@
   }
 }
 
-void jniCheck::validate_call_object(JavaThread* thr, jobject obj, jmethodID method_id) {
-  /* validate the object being passed */
+void jniCheck::validate_call(JavaThread* thr, jclass clazz, jmethodID method_id, jobject obj) {
   ASSERT_OOPS_ALLOWED;
-  jniCheck::validate_jmethod_id(thr, method_id);
-  jniCheck::validate_object(thr, obj);
-}
+  Method* m = jniCheck::validate_jmethod_id(thr, method_id);
+  InstanceKlass* holder = m->method_holder();
+
+  if (clazz != NULL) {
+    Klass* k = jniCheck::validate_class(thr, clazz, false);
+    // Check that method is in the class, must be InstanceKlass
+    if (!InstanceKlass::cast(k)->is_subtype_of(holder)) {
+      ReportJNIFatalError(thr, fatal_wrong_class_or_method);
+    }
+  }
 
-void jniCheck::validate_call_class(JavaThread* thr, jclass clazz, jmethodID method_id) {
-  /* validate the class being passed */
-  ASSERT_OOPS_ALLOWED;
-  jniCheck::validate_jmethod_id(thr, method_id);
-  jniCheck::validate_class(thr, clazz, false);
+  if (obj != NULL) {
+    oop recv = jniCheck::validate_object(thr, obj);
+    assert(recv != NULL, "validate_object checks that");
+    Klass* ik = recv->klass();
+
+    // Check that the object is a subtype of method holder too.
+    if (!InstanceKlass::cast(ik)->is_subtype_of(holder)) {
+      ReportJNIFatalError(thr, fatal_wrong_class_or_method);
+    }
+  }
 }
 
 
@@ -595,8 +606,7 @@
                                 jboolean isStatic))
     functionEnter(thr);
     IN_VM(
-      jniCheck::validate_class(thr, cls, false);
-      jniCheck::validate_jmethod_id(thr, methodID);
+      jniCheck::validate_call(thr, cls, methodID);
     )
     jobject result = UNCHECKED()->ToReflectedMethod(env, cls, methodID,
                                                     isStatic);
@@ -852,8 +862,7 @@
     functionEnter(thr);
     va_list args;
     IN_VM(
-      jniCheck::validate_class(thr, clazz, false);
-      jniCheck::validate_jmethod_id(thr, methodID);
+      jniCheck::validate_call(thr, clazz, methodID);
     )
     va_start(args, methodID);
     jobject result = UNCHECKED()->NewObjectV(env,clazz,methodID,args);
@@ -869,8 +878,7 @@
                          va_list args))
     functionEnter(thr);
     IN_VM(
-      jniCheck::validate_class(thr, clazz, false);
-      jniCheck::validate_jmethod_id(thr, methodID);
+      jniCheck::validate_call(thr, clazz, methodID);
     )
     jobject result = UNCHECKED()->NewObjectV(env,clazz,methodID,args);
     functionExit(thr);
@@ -884,8 +892,7 @@
                          const jvalue *args))
     functionEnter(thr);
     IN_VM(
-      jniCheck::validate_class(thr, clazz, false);
-      jniCheck::validate_jmethod_id(thr, methodID);
+      jniCheck::validate_call(thr, clazz, methodID);
     )
     jobject result = UNCHECKED()->NewObjectA(env,clazz,methodID,args);
     functionExit(thr);
@@ -941,7 +948,7 @@
     functionEnter(thr); \
     va_list args; \
     IN_VM( \
-      jniCheck::validate_call_object(thr, obj, methodID); \
+      jniCheck::validate_call(thr, NULL, methodID, obj); \
     ) \
     va_start(args,methodID); \
     ResultType result =UNCHECKED()->Call##Result##MethodV(env, obj, methodID, \
@@ -959,7 +966,7 @@
                                     va_list args)) \
     functionEnter(thr); \
     IN_VM(\
-      jniCheck::validate_call_object(thr, obj, methodID); \
+      jniCheck::validate_call(thr, NULL, methodID, obj); \
     ) \
     ResultType result = UNCHECKED()->Call##Result##MethodV(env, obj, methodID,\
                                                            args); \
@@ -975,7 +982,7 @@
                                     const jvalue * args)) \
     functionEnter(thr); \
     IN_VM( \
-      jniCheck::validate_call_object(thr, obj, methodID); \
+      jniCheck::validate_call(thr, NULL, methodID, obj); \
     ) \
     ResultType result = UNCHECKED()->Call##Result##MethodA(env, obj, methodID,\
                                                            args); \
@@ -1002,7 +1009,7 @@
     functionEnter(thr);
     va_list args;
     IN_VM(
-      jniCheck::validate_call_object(thr, obj, methodID);
+      jniCheck::validate_call(thr, NULL, methodID, obj);
     )
     va_start(args,methodID);
     UNCHECKED()->CallVoidMethodV(env,obj,methodID,args);
@@ -1018,7 +1025,7 @@
                               va_list args))
     functionEnter(thr);
     IN_VM(
-      jniCheck::validate_call_object(thr, obj, methodID);
+      jniCheck::validate_call(thr, NULL, methodID, obj);
     )
     UNCHECKED()->CallVoidMethodV(env,obj,methodID,args);
     thr->set_pending_jni_exception_check("CallVoidMethodV");
@@ -1032,7 +1039,7 @@
                               const jvalue * args))
     functionEnter(thr);
     IN_VM(
-      jniCheck::validate_call_object(thr, obj, methodID);
+      jniCheck::validate_call(thr, NULL, methodID, obj);
     )
     UNCHECKED()->CallVoidMethodA(env,obj,methodID,args);
     thr->set_pending_jni_exception_check("CallVoidMethodA");
@@ -1049,8 +1056,7 @@
     functionEnter(thr); \
     va_list args; \
     IN_VM( \
-      jniCheck::validate_call_object(thr, obj, methodID); \
-      jniCheck::validate_call_class(thr, clazz, methodID); \
+      jniCheck::validate_call(thr, clazz, methodID, obj); \
     ) \
     va_start(args,methodID); \
     ResultType result = UNCHECKED()->CallNonvirtual##Result##MethodV(env, \
@@ -1072,8 +1078,7 @@
                                               va_list args)) \
     functionEnter(thr); \
     IN_VM( \
-      jniCheck::validate_call_object(thr, obj, methodID); \
-      jniCheck::validate_call_class(thr, clazz, methodID); \
+      jniCheck::validate_call(thr, clazz, methodID, obj); \
     ) \
     ResultType result = UNCHECKED()->CallNonvirtual##Result##MethodV(env, \
                                                                      obj, \
@@ -1093,8 +1098,7 @@
                                               const jvalue * args)) \
     functionEnter(thr); \
     IN_VM( \
-      jniCheck::validate_call_object(thr, obj, methodID); \
-      jniCheck::validate_call_class(thr, clazz, methodID); \
+      jniCheck::validate_call(thr, clazz, methodID, obj); \
     ) \
     ResultType result = UNCHECKED()->CallNonvirtual##Result##MethodA(env, \
                                                                      obj, \
@@ -1125,8 +1129,7 @@
     functionEnter(thr);
     va_list args;
     IN_VM(
-      jniCheck::validate_call_object(thr, obj, methodID);
-      jniCheck::validate_call_class(thr, clazz, methodID);
+      jniCheck::validate_call(thr, clazz, methodID, obj);
     )
     va_start(args,methodID);
     UNCHECKED()->CallNonvirtualVoidMethodV(env,obj,clazz,methodID,args);
@@ -1143,8 +1146,7 @@
                                         va_list args))
     functionEnter(thr);
     IN_VM(
-      jniCheck::validate_call_object(thr, obj, methodID);
-      jniCheck::validate_call_class(thr, clazz, methodID);
+      jniCheck::validate_call(thr, clazz, methodID, obj);
     )
     UNCHECKED()->CallNonvirtualVoidMethodV(env,obj,clazz,methodID,args);
     thr->set_pending_jni_exception_check("CallNonvirtualVoidMethodV");
@@ -1159,8 +1161,7 @@
                                         const jvalue * args))
     functionEnter(thr);
     IN_VM(
-      jniCheck::validate_call_object(thr, obj, methodID);
-      jniCheck::validate_call_class(thr, clazz, methodID);
+      jniCheck::validate_call(thr, clazz, methodID, obj);
     )
     UNCHECKED()->CallNonvirtualVoidMethodA(env,obj,clazz,methodID,args);
     thr->set_pending_jni_exception_check("CallNonvirtualVoidMethodA");
@@ -1253,8 +1254,7 @@
     functionEnter(thr); \
     va_list args; \
     IN_VM( \
-      jniCheck::validate_jmethod_id(thr, methodID); \
-      jniCheck::validate_class(thr, clazz, false); \
+      jniCheck::validate_call(thr, clazz, methodID); \
     ) \
     va_start(args,methodID); \
     ReturnType result = UNCHECKED()->CallStatic##Result##MethodV(env, \
@@ -1274,8 +1274,7 @@
                                           va_list args)) \
     functionEnter(thr); \
     IN_VM( \
-      jniCheck::validate_jmethod_id(thr, methodID); \
-      jniCheck::validate_class(thr, clazz, false); \
+      jniCheck::validate_call(thr, clazz, methodID); \
     ) \
     ReturnType result = UNCHECKED()->CallStatic##Result##MethodV(env, \
                                                                  clazz, \
@@ -1293,8 +1292,7 @@
                                           const jvalue *args)) \
     functionEnter(thr); \
     IN_VM( \
-      jniCheck::validate_jmethod_id(thr, methodID); \
-      jniCheck::validate_class(thr, clazz, false); \
+      jniCheck::validate_call(thr, clazz, methodID); \
     ) \
     ReturnType result = UNCHECKED()->CallStatic##Result##MethodA(env, \
                                                                  clazz, \
@@ -1323,8 +1321,7 @@
     functionEnter(thr);
     va_list args;
     IN_VM(
-      jniCheck::validate_jmethod_id(thr, methodID);
-      jniCheck::validate_class(thr, cls, false);
+      jniCheck::validate_call(thr, cls, methodID);
     )
     va_start(args,methodID);
     UNCHECKED()->CallStaticVoidMethodV(env,cls,methodID,args);
@@ -1340,8 +1337,7 @@
                                     va_list args))
     functionEnter(thr);
     IN_VM(
-      jniCheck::validate_jmethod_id(thr, methodID);
-      jniCheck::validate_class(thr, cls, false);
+      jniCheck::validate_call(thr, cls, methodID);
     )
     UNCHECKED()->CallStaticVoidMethodV(env,cls,methodID,args);
     thr->set_pending_jni_exception_check("CallStaticVoidMethodV");
@@ -1355,8 +1351,7 @@
                                     const jvalue * args))
     functionEnter(thr);
     IN_VM(
-      jniCheck::validate_jmethod_id(thr, methodID);
-      jniCheck::validate_class(thr, cls, false);
+      jniCheck::validate_call(thr, cls, methodID);
     )
     UNCHECKED()->CallStaticVoidMethodA(env,cls,methodID,args);
     thr->set_pending_jni_exception_check("CallStaticVoidMethodA");
--- a/src/hotspot/share/prims/jniCheck.hpp	Mon Oct 07 15:58:04 2019 +0200
+++ b/src/hotspot/share/prims/jniCheck.hpp	Mon Oct 07 10:04:01 2019 -0400
@@ -51,8 +51,7 @@
   static Klass* validate_class(JavaThread* thr, jclass clazz, bool allow_primitive = false);
   static void validate_class_descriptor(JavaThread* thr, const char* name);
   static void validate_throwable_klass(JavaThread* thr, Klass* klass);
-  static void validate_call_object(JavaThread* thr, jobject obj, jmethodID method_id);
-  static void validate_call_class(JavaThread* thr, jclass clazz, jmethodID method_id);
+  static void validate_call(JavaThread* thr, jclass clazz, jmethodID method_id, jobject obj = NULL);
   static Method* validate_jmethod_id(JavaThread* thr, jmethodID method_id);
 };
 
--- a/src/hotspot/share/prims/jvmtiRedefineClasses.cpp	Mon Oct 07 15:58:04 2019 +0200
+++ b/src/hotspot/share/prims/jvmtiRedefineClasses.cpp	Mon Oct 07 10:04:01 2019 -0400
@@ -3528,15 +3528,6 @@
              "should be replaced");
     }
   }
-  // Update deleted jmethodID
-  for (int j = 0; j < _deleted_methods_length; ++j) {
-    Method* old_method = _deleted_methods[j];
-    jmethodID jmid = old_method->find_jmethod_id_or_null();
-    if (jmid != NULL) {
-      // Change the jmethodID to point to NSME.
-      Method::change_method_associated_with_jmethod_id(jmid, Universe::throw_no_such_method_error());
-    }
-  }
 }
 
 int VM_RedefineClasses::check_methods_and_mark_as_obsolete() {
--- a/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineDeleteJmethod.java	Mon Oct 07 15:58:04 2019 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,94 +0,0 @@
-/*
- * 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 8181171
- * @summary Test deleting static method pointing to by a jmethod
- * @library /test/lib
- * @modules java.base/jdk.internal.misc
- * @modules java.compiler
- *          java.instrument
- *          jdk.jartool/sun.tools.jar
- * @run main RedefineClassHelper
- * @run main/native/othervm -javaagent:redefineagent.jar -XX:+AllowRedefinitionToAddDeleteMethods -Xlog:redefine+class*=trace RedefineDeleteJmethod
- */
-
-class B {
-    private static int deleteMe() { System.out.println("deleteMe called"); return 5; }
-    public static int callDeleteMe() { return deleteMe(); }
-}
-
-public class RedefineDeleteJmethod {
-
-    public static String newB =
-        "class B {" +
-            "public static int callDeleteMe() { return 6; }" +
-        "}";
-
-    public static String newerB =
-        "class B {" +
-            "private static int deleteMe() { System.out.println(\"deleteMe (2) called\"); return 7; }" +
-            "public static int callDeleteMe() { return deleteMe(); }" +
-        "}";
-
-
-    static {
-        System.loadLibrary("RedefineDeleteJmethod");
-    }
-
-    static native int jniCallDeleteMe();
-
-    static void test(int expected, boolean nsme_expected) throws Exception {
-        // Call through static method
-        int res = B.callDeleteMe();
-        System.out.println("Result = " + res);
-        if (res != expected) {
-            throw new Error("returned " + res + " expected " + expected);
-        }
-
-        // Call through jmethodID, saved from first call.
-        try {
-            res = jniCallDeleteMe();
-            if (nsme_expected) {
-                throw new RuntimeException("Failed, NoSuchMethodError expected");
-            }
-            if (res != expected) {
-                throw new Error("returned " + res + " expected " + expected);
-            }
-        } catch (NoSuchMethodError ex) {
-            if (!nsme_expected) {
-                throw new RuntimeException("Failed, NoSuchMethodError not expected");
-            }
-            System.out.println("Passed, NoSuchMethodError expected");
-        }
-    }
-
-    public static void main(String[] args) throws Exception {
-        test(5, false);
-        RedefineClassHelper.redefineClass(B.class, newB);
-        test(6, true);
-        RedefineClassHelper.redefineClass(B.class, newerB);
-        test(7, true);
-    }
-}
--- a/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/libRedefineDeleteJmethod.c	Mon Oct 07 15:58:04 2019 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,47 +0,0 @@
-/*
- * 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.
- */
-
-#include <jni.h>
-
-jmethodID mid;
-jclass cls;
-static int count = 0;
-
-JNIEXPORT jint JNICALL
-Java_RedefineDeleteJmethod_jniCallDeleteMe(JNIEnv* env, jobject obj) {
-
-    if (count == 0) {
-      count++;
-      cls = (*env)->FindClass(env, "B");
-      if (NULL == cls) {
-          (*env)->FatalError(env, "could not find class");
-      }
-
-      mid = (*env)->GetStaticMethodID(env, cls, "deleteMe", "()I");
-      if (NULL == mid) {
-          (*env)->FatalError(env, "could not find method");
-      }
-    }
-
-    return (*env)->CallStaticIntMethod(env, cls, mid);
-}