RFR (S) 8181171: Deleting method for RedefineClasses breaks ResolvedMethodName

[Daniel D. Daugherty]

On 2/22/19 6:36 PM, coleen.phillimore at oracle.com wrote:
> 8210457: JVM crash in ResolvedMethodTable::add_method(Handle)
> Summary: Add a function to call NSME in ResolvedMethodTable to replace 
> deleted methods.
>
> This Unsafe.throwX trick is also used for vtable initialization for 
> throwing IllegalAccessError.  Tested with redefinition tests in the 
> repository and tier1-3, and added tests.
>
> open webrev at http://cr.openjdk.java.net/~coleenp/2019/8181171.01/webrev

src/hotspot/share/classfile/javaClasses.cpp
     No comments.

src/hotspot/share/memory/universe.hpp
     No comments.

src/hotspot/share/memory/universe.cpp
     No comments.

src/hotspot/share/oops/method.cpp
     No comments.

src/hotspot/share/prims/jvmtiRedefineClasses.cpp
     L3533:       // There is a jmethodID, change it to point to the NSME.
         Please consider:
                  // Change the jmethodID to point to NSME.

src/hotspot/share/prims/resolvedMethodTable.hpp
     No comments.

src/hotspot/share/prims/resolvedMethodTable.cpp
     L134:     // Method may have been deleted, replace with NSME
     L135:     if (method == NULL) {
     L136:       method = Universe::throw_no_such_method_error();
         Please consider:
               if (method == NULL) {
                 // Replace deleted method with NSME.
                 method = Universe::throw_no_such_method_error();

src/java.base/share/classes/jdk/internal/misc/Unsafe.java
     No comments beyond Serguei's.

test/jdk/java/lang/instrument/NamedBuffer.java
     L91:     static boolean checkMatch(byte[] buf, byte[] name, int 
begIdx) {
     L92:         for (int i = 0; i < name.length; i++) {
     L93:             if (buf[i + begIdx] != name[i]) {
         This code assumes the buf.length >= name.length + begIdx
         which could lead to array index problem. Perhaps add this
         before L92:

                  if (buf.length < name.length + begIdx) {
                      return false;
                  }

     L101:     bytesForHostClass(char replace, String className) throws 
Throwable {
         A short header comment about what this function is doing would
         be a good idea. Update: David asked for something similar.

     L121:             if(checkMatch(buf, ptrnBytes, i)) {
         nit - please add space after 'if' and before '('

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

test/hotspot/jtreg/runtime/RedefineTests/libRedefineDeleteJmethod.c
     L46:     // printf("visible 0x%x expected %d\n", visible, expected);
         This commented out line appears unrelated to this test.

test/jdk/java/lang/instrument/RedefineAddDeleteMethod/DeleteMethodHandle/MethodHandleDeletedMethod.java
     L34:  * @compile redef/Xost.java redef/Yost.java
         File 
test/jdk/java/lang/instrument/RedefineAddDeleteMethod/DeleteMethodHandle/redef/Yost.java
         appears to be missing from the webrev. However, there appears to be
         no reference to Yost other than this compile line.


     L87:             System.out.println("Passed, expected NSME");
     L93:             System.out.println("Passed, expected NSME");
         Perhaps both of these should be:
                      System.out.println("Received expected NSME");

         and the indication of "Passed" should be just before main()
         returns.

test/jdk/java/lang/instrument/RedefineAddDeleteMethod/DeleteMethodHandle/redef/Xost.java
     L28:     // try to redefine Xhost.
         Should 'Xhost' be 'Xost'? Or??? Update: Serguei also mentioned 
this one.

Dan


> bug link https://bugs.openjdk.java.net/browse/JDK-8181171
>
> Thanks,
> Coleen
>