8313816: Accessing jmethodID might lead to spurious crashes

Reviewed-by: coleenp
This commit is contained in:
Jaroslav Bachorik 2023-11-29 17:29:52 +00:00
parent b65ccff357
commit cdd1a6e851
8 changed files with 206 additions and 0 deletions
src/hotspot/share
test
hotspot/jtreg/serviceability/jvmti/thread/GetStackTrace/GetStackTraceAndRetransformTest
lib/jdk/test/whitebox

@ -4225,6 +4225,23 @@ bool InstanceKlass::should_clean_previous_versions_and_reset() {
return ret;
}
// This nulls out jmethodIDs for all methods in 'klass'
// It needs to be called explicitly for all previous versions of a class because these may not be cleaned up
// during class unloading.
// We can not use the jmethodID cache associated with klass directly because the 'previous' versions
// do not have the jmethodID cache filled in. Instead, we need to lookup jmethodID for each method and this
// is expensive - O(n) for one jmethodID lookup. For all contained methods it is O(n^2).
// The reason for expensive jmethodID lookup for each method is that there is no direct link between method and jmethodID.
void InstanceKlass::clear_jmethod_ids(InstanceKlass* klass) {
Array<Method*>* method_refs = klass->methods();
for (int k = 0; k < method_refs->length(); k++) {
Method* method = method_refs->at(k);
if (method != nullptr && method->is_obsolete()) {
method->clear_jmethod_id();
}
}
}
// Purge previous versions before adding new previous versions of the class and
// during class unloading.
void InstanceKlass::purge_previous_version_list() {
@ -4268,6 +4285,7 @@ void InstanceKlass::purge_previous_version_list() {
// Unlink from previous version list.
assert(pv_node->class_loader_data() == loader_data, "wrong loader_data");
InstanceKlass* next = pv_node->previous_versions();
clear_jmethod_ids(pv_node); // jmethodID maintenance for the unloaded class
pv_node->link_previous_versions(nullptr); // point next to null
last->link_previous_versions(next);
// Delete this node directly. Nothing is referring to it and we don't

@ -1080,6 +1080,8 @@ public:
bool idnum_can_increment() const { return has_been_redefined(); }
inline jmethodID* methods_jmethod_ids_acquire() const;
inline void release_set_methods_jmethod_ids(jmethodID* jmeths);
// This nulls out jmethodIDs for all methods in 'klass'
static void clear_jmethod_ids(InstanceKlass* klass);
// Lock during initialization
public:

@ -2273,6 +2273,20 @@ void Method::clear_jmethod_ids(ClassLoaderData* loader_data) {
loader_data->jmethod_ids()->clear_all_methods();
}
void Method::clear_jmethod_id() {
// Being at a safepoint prevents racing against other class redefinitions
assert(SafepointSynchronize::is_at_safepoint(), "should be at safepoint");
// The jmethodID is not stored in the Method instance, we need to look it up first
jmethodID methodid = find_jmethod_id_or_null();
// We need to make sure that jmethodID actually resolves to this method
// - multiple redefined versions may share jmethodID slots and if a method
// has already been rewired to a newer version we could be removing reference
// to a still existing method instance
if (methodid != nullptr && *((Method**)methodid) == this) {
*((Method**)methodid) = nullptr;
}
}
bool Method::has_method_vptr(const void* ptr) {
Method m;
// This assumes that the vtbl pointer is the first word of a C++ object.

@ -718,6 +718,7 @@ public:
// Clear methods
static void clear_jmethod_ids(ClassLoaderData* loader_data);
void clear_jmethod_id();
static void print_jmethod_ids_count(const ClassLoaderData* loader_data, outputStream* out) PRODUCT_RETURN;
// Get this method's jmethodID -- allocate if it doesn't exist

@ -2643,6 +2643,10 @@ WB_ENTRY(void, WB_PreTouchMemory(JNIEnv* env, jobject wb, jlong addr, jlong size
}
WB_END
WB_ENTRY(void, WB_CleanMetaspaces(JNIEnv* env, jobject target))
ClassLoaderDataGraph::safepoint_and_clean_metaspaces();
WB_END
#define CC (char*)
static JNINativeMethod methods[] = {
@ -2929,6 +2933,7 @@ static JNINativeMethod methods[] = {
{CC"unpinObject", CC"(Ljava/lang/Object;)V", (void*)&WB_UnpinObject},
{CC"setVirtualThreadsNotifyJvmtiMode", CC"(Z)Z", (void*)&WB_SetVirtualThreadsNotifyJvmtiMode},
{CC"preTouchMemory", CC"(JJ)V", (void*)&WB_PreTouchMemory},
{CC"cleanMetaspaces", CC"()V", (void*)&WB_CleanMetaspaces},
};

@ -0,0 +1,82 @@
/*
* Copyright (c) 2023, Datadog, Inc. 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 8313816
* @summary Test that a sequence of method retransformation and stacktrace capture while the old method
* version is still on stack does not lead to a crash when that method's jmethodID is used as
* an argument for JVMTI functions.
* @requires vm.jvmti
* @requires vm.flagless
* @library /test/lib
* @build jdk.test.whitebox.WhiteBox
* @modules java.instrument
* java.compiler
* @compile GetStackTraceAndRetransformTest.java
* @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
* @run main RedefineClassHelper
* @run main/othervm/native -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -javaagent:redefineagent.jar -agentlib:GetStackTraceAndRetransformTest GetStackTraceAndRetransformTest
*/
import jdk.test.whitebox.WhiteBox;
class Transformable {
static final String newClass = """
class Transformable {
static final String newClass = "";
static void redefineAndStacktrace() throws Exception {}
static void stacktrace() throws Exception {
capture(Thread.currentThread());
}
public static native void capture(Thread thread);
}
""";
static void redefineAndStacktrace() throws Exception {
// This call will cause the class to be retransformed.
// However, this method is still on stack so the subsequent attempt to capture the stacktrace
// will result into this frame being identified by the jmethodID of the previous method version.
RedefineClassHelper.redefineClass(Transformable.class, newClass);
capture(Thread.currentThread());
}
static void stacktrace() throws Exception {
}
public static native void capture(Thread thread);
}
public class GetStackTraceAndRetransformTest {
public static void main(String args[]) throws Throwable {
initialize(Transformable.class);
Transformable.redefineAndStacktrace();
Transformable.stacktrace();
WhiteBox.getWhiteBox().cleanMetaspaces();
check(2);
}
public static native void initialize(Class<?> target);
public static native void check(int expected);
}

@ -0,0 +1,82 @@
/*
* Copyright (c) 2023, Datadog, Inc. 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 <stdio.h>
#include <string.h>
#include "jvmti.h"
#include "jvmti_common.h"
#include "../get_stack_trace.h"
extern "C" {
static jvmtiEnv *jvmti = NULL;
static jmethodID* ids = NULL;
static int ids_size = 0;
JNIEXPORT jint JNICALL
Agent_OnLoad(JavaVM *jvm, char *options, void *reserved) {
jint res = jvm->GetEnv((void **) &jvmti, JVMTI_VERSION_1_1);
if (res != JNI_OK || jvmti == NULL) {
printf("Wrong result of a valid call to GetEnv!\n");
return JNI_ERR;
}
ids = (jmethodID*)malloc(sizeof(jmethodID) * 10);
return JNI_OK;
}
JNIEXPORT void JNICALL
Java_GetStackTraceAndRetransformTest_initialize(JNIEnv *env, jclass cls, jclass tgt) {
// we need to force jmethodids to be created for the methods we are going to retransform
env->GetStaticMethodID(tgt, "redefineAndStacktrace", "()V");
env->GetStaticMethodID(tgt, "stacktrace", "()V");
}
JNIEXPORT void JNICALL
Java_Transformable_capture(JNIEnv *env, jclass cls, jthread thread) {
jint count;
const int MAX_NUMBER_OF_FRAMES = 32;
jvmtiFrameInfo frames[MAX_NUMBER_OF_FRAMES];
jvmtiError err = jvmti->GetStackTrace(thread, 0, MAX_NUMBER_OF_FRAMES, frames, &count);
check_jvmti_status(env, err, "GetStackTrace failed.");
ids[ids_size++] = frames[1].method;
}
JNIEXPORT void JNICALL
Java_GetStackTraceAndRetransformTest_check(JNIEnv *jni, jclass cls, jint expected) {
if (ids_size != expected) {
fprintf(stderr, "Unexpected number methods captured: %d (expected %d)\n", ids_size, expected);
exit(2);
}
for (int i = 0; i < ids_size; i++) {
jclass rslt = NULL;
char* class_name = NULL;
jvmti->GetMethodDeclaringClass(ids[i], &rslt);
if (rslt != NULL) {
jvmti->GetClassSignature(rslt, &class_name, NULL);
}
}
}
}

@ -522,6 +522,8 @@ public class WhiteBox {
public native long metaspaceCapacityUntilGC();
public native long metaspaceSharedRegionAlignment();
public native void cleanMetaspaces();
// Metaspace Arena Tests
public native long createMetaspaceTestContext(long commit_limit, long reserve_limit);
public native void destroyMetaspaceTestContext(long context);