# HG changeset patch # User coleenp # Date 1570457041 14400 # Node ID 35ce0ad5870ac374d4f4039316c1c5691aee241a # Parent d148a7e7160c07a550b610384ea396f4d3fa2029 8229900: RedefineDeleteJmethod.java fails with -Xcheck:jni Summary: Remove RedefineClasses adjustment and test, but improve checking for method/class matching. Reviewed-by: dholmes, sspitsyn diff -r d148a7e7160c -r 35ce0ad5870a src/hotspot/share/prims/jniCheck.cpp --- 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"); diff -r d148a7e7160c -r 35ce0ad5870a src/hotspot/share/prims/jniCheck.hpp --- 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); }; diff -r d148a7e7160c -r 35ce0ad5870a src/hotspot/share/prims/jvmtiRedefineClasses.cpp --- 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() { diff -r d148a7e7160c -r 35ce0ad5870a test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineDeleteJmethod.java --- 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); - } -} diff -r d148a7e7160c -r 35ce0ad5870a test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/libRedefineDeleteJmethod.c --- 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 - -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); -}