8181171: Deleting method for RedefineClasses breaks ResolvedMethodName

8210457: JVM crash in ResolvedMethodTable::add_method(Handle)

Add a function to call NSME in ResolvedMethodTable to replace deleted methods.

Reviewed-by: sspitsyn, dholmes, dcubed

src/hotspot/share/classfile/javaClasses.cpp

@@ -3635,7 +3635,7 @@ oop java_lang_invoke_ResolvedMethodName::find_resolved_method(const methodHandle
     // distinct loaders) to ensure the metadata is kept alive.
     // This mirror may be different than the one in clazz field.
     new_resolved_method->obj_field_put(_vmholder_offset, m->method_holder()->java_mirror());
-    resolved_method = ResolvedMethodTable::add_method(Handle(THREAD, new_resolved_method));
+    resolved_method = ResolvedMethodTable::add_method(m, Handle(THREAD, new_resolved_method));
   }
   return resolved_method;
 }

src/hotspot/share/memory/universe.cpp

@@ -107,6 +107,7 @@ oop Universe::_the_min_jint_string                   = NULL;
 LatestMethodCache* Universe::_finalizer_register_cache = NULL;
 LatestMethodCache* Universe::_loader_addClass_cache    = NULL;
 LatestMethodCache* Universe::_throw_illegal_access_error_cache = NULL;
+LatestMethodCache* Universe::_throw_no_such_method_error_cache = NULL;
 LatestMethodCache* Universe::_do_stack_walk_cache     = NULL;
 oop Universe::_out_of_memory_error_java_heap          = NULL;
 oop Universe::_out_of_memory_error_metaspace          = NULL;
@@ -230,6 +231,7 @@ void Universe::metaspace_pointers_do(MetaspaceClosure* it) {
   _finalizer_register_cache->metaspace_pointers_do(it);
   _loader_addClass_cache->metaspace_pointers_do(it);
   _throw_illegal_access_error_cache->metaspace_pointers_do(it);
+  _throw_no_such_method_error_cache->metaspace_pointers_do(it);
   _do_stack_walk_cache->metaspace_pointers_do(it);
 }
 
@@ -271,6 +273,7 @@ void Universe::serialize(SerializeClosure* f) {
   _finalizer_register_cache->serialize(f);
   _loader_addClass_cache->serialize(f);
   _throw_illegal_access_error_cache->serialize(f);
+  _throw_no_such_method_error_cache->serialize(f);
   _do_stack_walk_cache->serialize(f);
 }
 
@@ -689,6 +692,7 @@ jint universe_init() {
   Universe::_finalizer_register_cache = new LatestMethodCache();
   Universe::_loader_addClass_cache    = new LatestMethodCache();
   Universe::_throw_illegal_access_error_cache = new LatestMethodCache();
+  Universe::_throw_no_such_method_error_cache = new LatestMethodCache();
   Universe::_do_stack_walk_cache = new LatestMethodCache();
 
 #if INCLUDE_CDS
@@ -935,6 +939,11 @@ void Universe::initialize_known_methods(TRAPS) {
                           "throwIllegalAccessError",
                           vmSymbols::void_method_signature(), true, CHECK);
 
+  initialize_known_method(_throw_no_such_method_error_cache,
+                          SystemDictionary::internal_Unsafe_klass(),
+                          "throwNoSuchMethodError",
+                          vmSymbols::void_method_signature(), true, CHECK);
+
   // Set up method for registering loaded classes in class loader vector
   initialize_known_method(_loader_addClass_cache,
                           SystemDictionary::ClassLoader_klass(),

src/hotspot/share/memory/universe.hpp

@@ -138,6 +138,7 @@ class Universe: AllStatic {
   static LatestMethodCache* _finalizer_register_cache; // static method for registering finalizable objects
   static LatestMethodCache* _loader_addClass_cache;    // method for registering loaded classes in class loader vector
   static LatestMethodCache* _throw_illegal_access_error_cache; // Unsafe.throwIllegalAccessError() method
+  static LatestMethodCache* _throw_no_such_method_error_cache; // Unsafe.throwNoSuchMethodError() method
   static LatestMethodCache* _do_stack_walk_cache;      // method for stack walker callback
 
   // preallocated error objects (no backtrace)
@@ -322,6 +323,7 @@ class Universe: AllStatic {
   static Method*      loader_addClass_method()        { return _loader_addClass_cache->get_method(); }
 
   static Method*      throw_illegal_access_error()    { return _throw_illegal_access_error_cache->get_method(); }
+  static Method*      throw_no_such_method_error()    { return _throw_no_such_method_error_cache->get_method(); }
 
   static Method*      do_stack_walk_method()          { return _do_stack_walk_cache->get_method(); }
 

src/hotspot/share/oops/method.cpp

@@ -2120,7 +2120,8 @@ void Method::change_method_associated_with_jmethod_id(jmethodID jmid, Method* ne
   // Can't assert the method_holder is the same because the new method has the
   // scratch method holder.
   assert(resolve_jmethod_id(jmid)->method_holder()->class_loader()
-           == new_method->method_holder()->class_loader(),
+           == new_method->method_holder()->class_loader() ||
+           new_method->method_holder()->class_loader() == NULL, // allow Unsafe substitution
          "changing to a different class loader");
   // Just change the method in place, jmethodID pointer doesn't change.
   *((Method**)jmid) = new_method;

src/hotspot/share/prims/jvmtiRedefineClasses.cpp

@@ -3525,6 +3525,15 @@ void VM_RedefineClasses::update_jmethod_ids() {
              "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() {

src/hotspot/share/prims/resolvedMethodTable.cpp

@@ -120,18 +120,21 @@ oop ResolvedMethodTable::find_method(Method* method) {
   return entry;
 }
 
-oop ResolvedMethodTable::add_method(Handle resolved_method_name) {
+oop ResolvedMethodTable::add_method(const methodHandle& m, Handle resolved_method_name) {
   MutexLocker ml(ResolvedMethodTable_lock);
   DEBUG_ONLY(NoSafepointVerifier nsv);
 
+  Method* method = m();
   // Check if method has been redefined while taking out ResolvedMethodTable_lock, if so
-  // use new method.
+  // use new method.  The old method won't be deallocated because it's passed in as a Handle.
-  Method* method = (Method*)java_lang_invoke_ResolvedMethodName::vmtarget(resolved_method_name());
-  assert(method->is_method(), "must be method");
   if (method->is_old()) {
     // Replace method with redefined version
     InstanceKlass* holder = method->method_holder();
     method = holder->method_with_idnum(method->method_idnum());
+    if (method == NULL) {
+      // Replace deleted method with NSME.
+      method = Universe::throw_no_such_method_error();
+    }
     java_lang_invoke_ResolvedMethodName::set_vmtarget(resolved_method_name(), method);
   }
   // Set flag in class to indicate this InstanceKlass has entries in the table
@@ -226,13 +229,9 @@ void ResolvedMethodTable::adjust_method_entries(bool * trace_name_printed) {
 
       if (old_method->is_old()) {
 
-        if (old_method->is_deleted()) {
+        Method* new_method = (old_method->is_deleted()) ?
-          // leave deleted method in ResolvedMethod for now (this is a bug that we don't mark
+                              Universe::throw_no_such_method_error() :
-          // these on_stack)
+                              old_method->get_new_method();
-          continue;
-        }
-
-        Method* new_method = old_method->get_new_method();
         java_lang_invoke_ResolvedMethodName::set_vmtarget(mem_name, new_method);
 
         ResourceMark rm;

src/hotspot/share/prims/resolvedMethodTable.hpp

@@ -89,7 +89,7 @@ public:
 
   // Called from java_lang_invoke_ResolvedMethodName
   static oop find_method(Method* method);
-  static oop add_method(Handle rmethod_name);
+  static oop add_method(const methodHandle& method, Handle rmethod_name);
 
   static bool has_work() { return _dead_entries; }
   static void trigger_cleanup();

src/java.base/share/classes/jdk/internal/misc/Unsafe.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 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
@@ -3114,7 +3114,7 @@ public final class Unsafe {
      * @param offset field/element offset
      * @param mask the mask value
      * @return the previous value
-     * @since 1.9
+     * @since 9
      */
     @ForceInline
     public final int getAndBitwiseAndInt(Object o, long offset, int mask) {
@@ -3342,6 +3342,14 @@ public final class Unsafe {
         throw new IllegalAccessError();
     }
 
+    /**
+     * Throws NoSuchMethodError; for use by the VM for redefinition support.
+     * @since 13
+     */
+    private static void throwNoSuchMethodError() {
+        throw new NoSuchMethodError();
+    }
+
     /**
      * @return Returns true if the native byte ordering of this
      * platform is big-endian, false if it is little-endian.

test/hotspot/jtreg/runtime/RedefineTests/RedefineDeleteJmethod.java

@@ -0,0 +1,94 @@
+/*
+ * 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 -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);
+    }
+}

test/hotspot/jtreg/runtime/RedefineTests/libRedefineDeleteJmethod.c

@@ -0,0 +1,47 @@
+/*
+ * 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);
+}

test/jdk/java/lang/instrument/NamedBuffer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 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
@@ -84,4 +84,59 @@ public class NamedBuffer
                             actualSize);
         return resultBuffer;
     }
+
+    static final String DEST = System.getProperty("test.classes");
+    static final boolean VERBOSE = false;
+
+    static boolean checkMatch(byte[] buf, byte[] name, int begIdx) {
+        if (buf.length < name.length + begIdx) {
+            return false;
+        }
+        for (int i = 0; i < name.length; i++) {
+            if (buf[i + begIdx] != name[i]) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    // This function reads a class file from disk and replaces the first character of
+    // the name with the one passed as "replace".  Then goes through the bytecodes to
+    // replace all instances of the name of the class with the new name.  The
+    // redefinition tests use this to redefine Host$ classes with precompiled class files
+    // Xost.java, Yost.java and Zost.java.
+    static byte[]
+    bytesForHostClass(char replace, String className) throws Throwable {
+        String tail = className.substring(1);
+        String origClassName = "" + replace + tail;
+        File clsfile = new File(DEST + "/" + origClassName + ".class");
+
+        if (VERBOSE) {
+            System.out.println("   Reading bytes from " + clsfile);
+        }
+        byte[] buf = null;
+        try (FileInputStream str = new FileInputStream(clsfile)) {
+            buf = loadBufferFromStream(str);
+        }
+
+        boolean found = false;
+        int dollarSignIdx = className.indexOf('$');
+        int ptrnLen = (dollarSignIdx == -1) ? className.length() : dollarSignIdx;
+        byte[] ptrnBytes = origClassName.substring(0, ptrnLen).getBytes();
+        byte firstByte = className.getBytes()[0];
+
+        for (int i = 0; i < buf.length - ptrnLen; i++) {
+            if (checkMatch(buf, ptrnBytes, i)) {
+                if (VERBOSE) {
+                    System.out.println("Appear to have found " + origClassName + " starting at " + i);
+                }
+                buf[i] = firstByte;
+                found = true;
+            }
+        }
+        if (!found) {
+            throw new Error("Could not locate '" + ptrnBytes + "' name in byte array");
+        }
+        return buf;
+    }
 }

test/jdk/java/lang/instrument/RedefineAddDeleteMethod/DeleteMethodHandle/MethodHandleDeletedMethod.java

@@ -0,0 +1,97 @@
+/*
+ * 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 Break ResolvedMethodTable with redefined nest class.
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ * @modules java.compiler
+ *          java.instrument
+ *          jdk.jartool/sun.tools.jar
+ * @compile ../../NamedBuffer.java
+ * @compile redef/Xost.java
+ * @run main RedefineClassHelper
+ * @run main/othervm -javaagent:redefineagent.jar -Xlog:redefine+class+update*=debug,membername+table=debug MethodHandleDeletedMethod
+ */
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.lang.invoke.*;
+
+class Host {
+    static MethodHandle fooMH;
+
+    static class A {
+        private static void foo() { System.out.println("OLD foo called"); }
+    }
+    static void bar() throws NoSuchMethodError {
+        A.foo();
+    }
+    static void barMH() throws Throwable {
+        fooMH.invokeExact();
+    }
+
+    public static void reresolve() throws Throwable {
+        fooMH = MethodHandles.lookup().findStatic(A.class, "foo", MethodType.methodType(void.class));
+    }
+
+    static {
+        try {
+          fooMH = MethodHandles.lookup().findStatic(A.class, "foo", MethodType.methodType(void.class));
+        } catch (ReflectiveOperationException ex) {
+        }
+    }
+}
+
+public class MethodHandleDeletedMethod {
+
+    static final String DEST = System.getProperty("test.classes");
+    static final boolean VERBOSE = false;
+
+    private static byte[] bytesForHostClass(char replace) throws Throwable {
+        return NamedBuffer.bytesForHostClass(replace, "Host$A");
+    }
+
+    public static void main(java.lang.String[] unused) throws Throwable {
+        Host h = new Host();
+        h.bar();
+        h.barMH();
+        byte[] buf = bytesForHostClass('X');
+        RedefineClassHelper.redefineClass(Host.A.class, buf);
+        try {
+            h.bar();    // call deleted Method directly
+            throw new RuntimeException("Failed, expected NSME");
+        } catch (NoSuchMethodError nsme) {
+            System.out.println("Received expected NSME");
+        }
+        try {
+            h.barMH();  // call through MethodHandle for deleted Method
+            throw new RuntimeException("Failed, expected NSME");
+        } catch (NoSuchMethodError nsme) {
+            System.out.println("Received expected NSME");
+        }
+        System.out.println("Passed.");
+    }
+}

test/jdk/java/lang/instrument/RedefineAddDeleteMethod/DeleteMethodHandle/redef/Xost.java

@@ -0,0 +1,29 @@
+/*
+ * 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.
+ */
+
+public class Xost {
+    // Remove static private methods, in A in redefinition.
+    static class A { }
+    // Removed public method to get this to compile, but we don't
+    // try to redefine Host.
+}