Skip to content

Commit

Permalink
8318895: Deoptimization results in incorrect lightweight locking stack
Browse files Browse the repository at this point in the history
Reviewed-by: shade
Backport-of: ea1ffa34192448317ce9a61a3588b0dee3a2ef44
  • Loading branch information
rkennke committed Apr 9, 2024
1 parent 71b6e9d commit 17daee8
Show file tree
Hide file tree
Showing 2 changed files with 163 additions and 3 deletions.
17 changes: 14 additions & 3 deletions src/hotspot/share/runtime/deoptimization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
#include "runtime/stackFrameStream.inline.hpp"
#include "runtime/stackWatermarkSet.hpp"
#include "runtime/stubRoutines.hpp"
#include "runtime/synchronizer.hpp"
#include "runtime/thread.hpp"
#include "runtime/threadSMR.hpp"
#include "runtime/threadWXSetters.inline.hpp"
Expand Down Expand Up @@ -1520,9 +1521,19 @@ bool Deoptimization::relock_objects(JavaThread* thread, GrowableArray<MonitorInf
}
}
}
BasicLock* lock = mon_info->lock();
ObjectSynchronizer::enter(obj, lock, deoptee_thread);
assert(mon_info->owner()->is_locked(), "object must be locked now");
if (LockingMode == LM_LIGHTWEIGHT && exec_mode == Unpack_none) {
// We have lost information about the correct state of the lock stack.
// Inflate the locks instead. Enter then inflate to avoid races with
// deflation.
ObjectSynchronizer::enter(obj, nullptr, deoptee_thread);
assert(mon_info->owner()->is_locked(), "object must be locked now");
ObjectMonitor* mon = ObjectSynchronizer::inflate(deoptee_thread, obj(), ObjectSynchronizer::inflate_cause_vm_internal);
assert(mon->owner() == deoptee_thread, "must be");
} else {
BasicLock* lock = mon_info->lock();
ObjectSynchronizer::enter(obj, lock, deoptee_thread);
assert(mon_info->owner()->is_locked(), "object must be locked now");
}
}
}
}
Expand Down
149 changes: 149 additions & 0 deletions test/jdk/com/sun/jdi/EATests.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
* -XX:+WhiteBoxAPI
* -Xbatch
* -XX:+DoEscapeAnalysis -XX:+EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks -XX:+UseBiasedLocking
* -XX:+UnlockExperimentalVMOptions -XX:LockingMode=1
* @run driver EATests
* -XX:+UnlockDiagnosticVMOptions
* -Xms256m -Xmx256m
Expand All @@ -50,6 +51,7 @@
* -XX:+WhiteBoxAPI
* -Xbatch
* -XX:+DoEscapeAnalysis -XX:+EliminateAllocations -XX:-EliminateLocks -XX:+EliminateNestedLocks -XX:+UseBiasedLocking -XX:-UseOptoBiasInlining
* -XX:+UnlockExperimentalVMOptions -XX:LockingMode=1
* @run driver EATests
* -XX:+UnlockDiagnosticVMOptions
* -Xms256m -Xmx256m
Expand All @@ -58,6 +60,7 @@
* -XX:+WhiteBoxAPI
* -Xbatch
* -XX:+DoEscapeAnalysis -XX:-EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks -XX:+UseBiasedLocking
* -XX:+UnlockExperimentalVMOptions -XX:LockingMode=1
* @run driver EATests
* -XX:+UnlockDiagnosticVMOptions
* -Xms256m -Xmx256m
Expand All @@ -66,6 +69,7 @@
* -XX:+WhiteBoxAPI
* -Xbatch
* -XX:-DoEscapeAnalysis -XX:-EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks -XX:+UseBiasedLocking
* -XX:+UnlockExperimentalVMOptions -XX:LockingMode=1
* @run driver EATests
* -XX:+UnlockDiagnosticVMOptions
* -Xms256m -Xmx256m
Expand All @@ -74,6 +78,7 @@
* -XX:+WhiteBoxAPI
* -Xbatch
* -XX:+DoEscapeAnalysis -XX:+EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks -XX:-UseBiasedLocking
* -XX:+UnlockExperimentalVMOptions -XX:LockingMode=1
* @run driver EATests
* -XX:+UnlockDiagnosticVMOptions
* -Xms256m -Xmx256m
Expand All @@ -82,6 +87,7 @@
* -XX:+WhiteBoxAPI
* -Xbatch
* -XX:+DoEscapeAnalysis -XX:-EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks -XX:-UseBiasedLocking
* -XX:+UnlockExperimentalVMOptions -XX:LockingMode=1
* @run driver EATests
* -XX:+UnlockDiagnosticVMOptions
* -Xms256m -Xmx256m
Expand All @@ -90,6 +96,44 @@
* -XX:+WhiteBoxAPI
* -Xbatch
* -XX:-DoEscapeAnalysis -XX:-EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks -XX:-UseBiasedLocking
* -XX:+UnlockExperimentalVMOptions -XX:LockingMode=1
*
* @run driver EATests
* -XX:+UnlockDiagnosticVMOptions
* -Xms256m -Xmx256m
* -Xbootclasspath/a:.
* -XX:CompileCommand=dontinline,*::dontinline_*
* -XX:+WhiteBoxAPI
* -Xbatch
* -XX:+DoEscapeAnalysis -XX:+EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks
* -XX:+UnlockExperimentalVMOptions -XX:LockingMode=2
* @run driver EATests
* -XX:+UnlockDiagnosticVMOptions
* -Xms256m -Xmx256m
* -Xbootclasspath/a:.
* -XX:CompileCommand=dontinline,*::dontinline_*
* -XX:+WhiteBoxAPI
* -Xbatch
* -XX:+DoEscapeAnalysis -XX:+EliminateAllocations -XX:-EliminateLocks -XX:+EliminateNestedLocks
* -XX:+UnlockExperimentalVMOptions -XX:LockingMode=2
* @run driver EATests
* -XX:+UnlockDiagnosticVMOptions
* -Xms256m -Xmx256m
* -Xbootclasspath/a:.
* -XX:CompileCommand=dontinline,*::dontinline_*
* -XX:+WhiteBoxAPI
* -Xbatch
* -XX:+DoEscapeAnalysis -XX:-EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks
* -XX:+UnlockExperimentalVMOptions -XX:LockingMode=2
* @run driver EATests
* -XX:+UnlockDiagnosticVMOptions
* -Xms256m -Xmx256m
* -Xbootclasspath/a:.
* -XX:CompileCommand=dontinline,*::dontinline_*
* -XX:+WhiteBoxAPI
* -Xbatch
* -XX:-DoEscapeAnalysis -XX:-EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks
* -XX:+UnlockExperimentalVMOptions -XX:LockingMode=2
*
* @comment Excercise -XX:+DeoptimizeObjectsALot. Mostly to prevent bit-rot because the option is meant to stress object deoptimization
* with non-synthetic workloads.
Expand Down Expand Up @@ -233,6 +277,7 @@ public static void main(String[] args) {
// Relocking test cases
new EARelockingSimpleTarget() .run();
new EARelockingSimple_2Target() .run();
new EARelockingSimpleWithAccessInOtherThreadTarget() .run();
new EARelockingRecursiveTarget() .run();
new EARelockingNestedInflatedTarget() .run();
new EARelockingNestedInflated_02Target() .run();
Expand Down Expand Up @@ -353,6 +398,7 @@ protected void runTests() throws Exception {
// Relocking test cases
new EARelockingSimple() .run(this);
new EARelockingSimple_2() .run(this);
new EARelockingSimpleWithAccessInOtherThread() .run(this);
new EARelockingRecursive() .run(this);
new EARelockingNestedInflated() .run(this);
new EARelockingNestedInflated_02() .run(this);
Expand Down Expand Up @@ -1787,6 +1833,7 @@ public void dontinline_testMethod() {
*/
class EARelockingSimple_2 extends EATestCaseBaseDebugger {


public void runTestCase() throws Exception {
BreakpointEvent bpe = resumeTo(TARGET_TESTCASE_BASE_NAME, "dontinline_brkpt", "()V");
printStack(bpe.thread());
Expand All @@ -1812,6 +1859,66 @@ public void dontinline_testMethod() {

/////////////////////////////////////////////////////////////////////////////

// The debugger reads and publishes an object with eliminated locking to an instance field.
// A 2nd thread in the debuggee finds it there and changes its state using a synchronized method.
// Without eager relocking the accesses are unsynchronized which can be observed.
class EARelockingSimpleWithAccessInOtherThread extends EATestCaseBaseDebugger {

public void runTestCase() throws Exception {
BreakpointEvent bpe = resumeTo(TARGET_TESTCASE_BASE_NAME, "dontinline_brkpt", "()V");
printStack(bpe.thread());
String l1ClassName = EARelockingSimpleWithAccessInOtherThreadTarget.SyncCounter.class.getName();
ObjectReference ctr = getLocalRef(bpe.thread().frame(1), l1ClassName, "l1");
setField(testCase, "sharedCounter", ctr);
terminateEndlessLoop();
}
}

class EARelockingSimpleWithAccessInOtherThreadTarget extends EATestCaseBaseTarget {

public static class SyncCounter {
private int val;
public synchronized int inc() { return val++; }
}

public volatile SyncCounter sharedCounter;

@Override
public void setUp() {
super.setUp();
doLoop = true;
Thread t = new Thread() {
public void run() {
while (doLoop) {
SyncCounter ctr = sharedCounter;
if (ctr != null) {
ctr.inc();
}
}
}
};
t.setDaemon(true);
t.start();
}

public void dontinline_testMethod() {
SyncCounter l1 = new SyncCounter();
synchronized (l1) { // Eliminated locking
l1.inc();
dontinline_brkpt(); // Debugger publishes l1 to sharedCounter.
iResult = l1.inc(); // Changes by the 2nd thread will be observed if l1
// was not relocked before passing it to the debugger.
}
}

@Override
public int getExpectedIResult() {
return 1;
}
}

/////////////////////////////////////////////////////////////////////////////

// Test recursive locking
class EARelockingRecursiveTarget extends EATestCaseBaseTarget {

Expand Down Expand Up @@ -2109,6 +2216,48 @@ public boolean testFrameShouldBeDeoptimized() {

/////////////////////////////////////////////////////////////////////////////

/**
* Similar to {@link EARelockingArgEscapeLWLockedInCalleeFrame_2Target}. It does
* not use recursive locking and exposed a bug in the lightweight-locking implementation.
*/
class EARelockingArgEscapeLWLockedInCalleeFrameNoRecursive extends EATestCaseBaseDebugger {

public void runTestCase() throws Exception {
BreakpointEvent bpe = resumeTo(TARGET_TESTCASE_BASE_NAME, "dontinline_brkpt", "()V");
printStack(bpe.thread());
@SuppressWarnings("unused")
ObjectReference o = getLocalRef(bpe.thread().frame(2), XYVAL_NAME, "l1");
}
}

class EARelockingArgEscapeLWLockedInCalleeFrameNoRecursiveTarget extends EATestCaseBaseTarget {

@Override
public void setUp() {
super.setUp();
testMethodDepth = 2;
}

public void dontinline_testMethod() {
XYVal l1 = new XYVal(1, 1); // NoEscape, scalar replaced
XYVal l2 = new XYVal(4, 2); // NoEscape, scalar replaced
XYVal l3 = new XYVal(5, 3); // ArgEscape
synchronized (l1) { // eliminated
synchronized (l2) { // eliminated
l3.dontinline_sync_method(this); // l3 escapes
}
}
iResult = l2.x + l2.y;
}

@Override
public int getExpectedIResult() {
return 6;
}
}

/////////////////////////////////////////////////////////////////////////////

/**
* Test relocking eliminated (nested) locks of an object on which the
* target thread currently waits.
Expand Down

0 comments on commit 17daee8

Please sign in to comment.