Skip to content
This repository has been archived by the owner. It is now read-only.
Permalink
Browse files
8249293: Unsafe stackwalk in VM_GetOrSetLocal::doit_prologue()
Reviewed-by: sspitsyn, dholmes
  • Loading branch information
reinrich committed Jul 31, 2020
1 parent 19df711 commit cc53f8a628e69b61bd87b386a4d0c940bcc41e12
Show file tree
Hide file tree
Showing 4 changed files with 614 additions and 26 deletions.
@@ -579,16 +579,13 @@ bool VM_GetOrSetLocal::check_slot_type_lvt(javaVFrame* jvf) {
jobject jobj = _value.l;
if (_set && slot_type == T_OBJECT && jobj != NULL) { // NULL reference is allowed
// Check that the jobject class matches the return type signature.
JavaThread* cur_thread = JavaThread::current();
HandleMark hm(cur_thread);

Handle obj(cur_thread, JNIHandles::resolve_external_guard(jobj));
oop obj = JNIHandles::resolve_external_guard(jobj);
NULL_CHECK(obj, (_result = JVMTI_ERROR_INVALID_OBJECT, false));
Klass* ob_k = obj->klass();
NULL_CHECK(ob_k, (_result = JVMTI_ERROR_INVALID_OBJECT, false));

const char* signature = (const char *) sign_sym->as_utf8();
if (!is_assignable(signature, ob_k, cur_thread)) {
if (!is_assignable(signature, ob_k, VMThread::vm_thread())) {
_result = JVMTI_ERROR_TYPE_MISMATCH;
return false;
}
@@ -629,34 +626,33 @@ static bool can_be_deoptimized(vframe* vf) {
return (vf->is_compiled_frame() && vf->fr().can_be_deoptimized());
}

bool VM_GetOrSetLocal::doit_prologue() {
void VM_GetOrSetLocal::doit() {
_jvf = get_java_vframe();
NULL_CHECK(_jvf, false);
if (_jvf == NULL) {
return;
};

Method* method = _jvf->method();
if (getting_receiver()) {
if (method->is_static()) {
_result = JVMTI_ERROR_INVALID_SLOT;
return false;
return;
}
} else {
if (method->is_native()) {
_result = JVMTI_ERROR_OPAQUE_FRAME;
return;
}
return true;
}

if (method->is_native()) {
_result = JVMTI_ERROR_OPAQUE_FRAME;
return false;
}

if (!check_slot_type_no_lvt(_jvf)) {
return false;
}
if (method->has_localvariable_table()) {
return check_slot_type_lvt(_jvf);
if (!check_slot_type_no_lvt(_jvf)) {
return;
}
if (method->has_localvariable_table() &&
!check_slot_type_lvt(_jvf)) {
return;
}
}
return true;
}

void VM_GetOrSetLocal::doit() {
InterpreterOopMap oop_mask;
_jvf->method()->mask_for(_jvf->bci(), &oop_mask);
if (oop_mask.is_dead(_index)) {
@@ -695,7 +691,7 @@ void VM_GetOrSetLocal::doit() {
return;
}
StackValueCollection *locals = _jvf->locals();
Thread* current_thread = Thread::current();
Thread* current_thread = VMThread::vm_thread();
HandleMark hm(current_thread);

switch (_type) {
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 2020, 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
@@ -347,7 +347,6 @@ class VM_GetOrSetLocal : public VM_Operation {
jvalue value() { return _value; }
jvmtiError result() { return _result; }

bool doit_prologue();
void doit();
bool allow_nested_vm_operations() const;
const char* name() const { return "get/set locals"; }
@@ -0,0 +1,149 @@
/*
* Copyright (c) 2020 SAP SE. 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 8249293
*
* @summary Test if stack walk to get local variable in the JVMTI implementation is safe if the
* target thread is not suspended.
*
* @comment The main/target thread uses recursion to build a large stack, then
* calls a native method to notify the JVMTI agent thread to get a
* local variable deep in the stack. This prolongs the stack walk. The
* target thread's stack is walkable while in native. After sending the
* notification it waits a while to give the agent time to reach the
* stack walk, then it returns from native. This is when its stack
* becomes not walkable again.
*
* @library /test/lib
* @compile GetLocalWithoutSuspendTest.java
* @run main/othervm/native
* -agentlib:GetLocalWithoutSuspendTest
* -Xbatch
* GetLocalWithoutSuspendTest
*/

public class GetLocalWithoutSuspendTest {

public static final int M = 1 << 20;

public static final int TEST_ITERATIONS = 200;

/**
* Native method to notify the agent thread to call GetLocalObject() on this thread.
*
* @param depth Depth of target frame for GetLocalObject() call. Should be
* large value to prolong the unsafe stack walk.
* @param waitTime Time to wait after notify with
* walkable stack before returning an becoming unsafe again.
* @return Dummy value.
*/
public static native void notifyAgentToGetLocal(int depth, int waitTime);

/**
* Notify agent thread that we are shutting down and wait for it to terminate.
*/
public static native void shutDown();

/**
* Provide agent thread with reference to target thread.
* @param target The target thread
*/
public static native void setTargetThread(Thread target);

public static void main(String[] args) throws Exception {
new GetLocalWithoutSuspendTest().runTest();
}

/**
* Wait cycles in native, i.e. with walkable stack, after notifying agent
* thread to do GetLocalObject() call.
*/
public int waitCycles = 1;

public void runTest() throws Exception {
log("Set target thread for get local variable calls by agent.");
setTargetThread(Thread.currentThread());

log("Test how many frames fit on the stack by performing recursive calls until");
log("StackOverflowError is thrown");
int targetDepth = recursiveMethod(0, M);
log("Testing with target depth: " + targetDepth);

log("Begin Test.");
long start = System.currentTimeMillis();
for (int iterations = 0; iterations < TEST_ITERATIONS; iterations++) {
long now = System.currentTimeMillis();
log((now - start) + " ms Iteration : " + iterations +
" waitTime : " + waitCycles);
int newTargetDepth = recursiveMethod(0, targetDepth);
if (newTargetDepth < targetDepth) {
// A StackOverflowError can occur due to (re-)compilation. We
// don't reach the native method notifyAgentToGetLocal() then
// which is a prerequisite to trigger the problematic race
// condition. So we reduce the targetDepth to avoid stack
// overflow.
log("StackOverflowError during test.");
log("Old target depth: " + targetDepth);
log("Retry with new target depth: " + newTargetDepth);
targetDepth = newTargetDepth;
}
iterations++;
// Double wait time, but limit to roughly 10^6 cycles.
waitCycles = (waitCycles << 1) & (M - 1);
waitCycles = waitCycles == 0 ? 1 : waitCycles;
}

// Notify agent thread that we are shutting down and wait for it to terminate.
shutDown();

log("Successfully finished test");
}

/**
* Perform recursive calls until the target stack depth is reached or the stack overflows.
* Call {@link #notifyAgentToGetLocal(int, int)} if the target depth is reached.
*
* @param depth Current recursion depth
* @param targetStackDepth Target recursion depth
* @return Depth at which the recursion was ended
*/
public int recursiveMethod(int depth, int targetStackDepth) {
int maxDepth = depth;
try {
if (depth == targetStackDepth) {
notifyAgentToGetLocal(depth - 100, waitCycles);
} else {
maxDepth = recursiveMethod(depth + 1, targetStackDepth);
}
} catch (StackOverflowError e) {
// Don't print message here, because this would likely trigger a new StackOverflowError
}
return maxDepth;
}

public static void log(String m) {
System.out.println("### Java-Test: " + m);
}
}

0 comments on commit cc53f8a

Please sign in to comment.