On 2/22/19 6:36 PM, coleen.phillim...@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
