Ensure non-standard returns release monitors
We were incorrectly failing to unlock monitors that were acquired by a
frame when the frame is popped off the stack in a non-standard manner.
This can lead to monitors having an incorrect count and being
impossible for any other thread to lock.
Test: ./test.py --host
Bug: 133901254
Change-Id: I2efbb8854dd5530c66d780f6853ec6e05a69c83b
diff --git a/runtime/interpreter/interpreter_switch_impl-inl.h b/runtime/interpreter/interpreter_switch_impl-inl.h
index 4525f7c..90acc7e 100644
--- a/runtime/interpreter/interpreter_switch_impl-inl.h
+++ b/runtime/interpreter/interpreter_switch_impl-inl.h
@@ -20,19 +20,24 @@
#include "interpreter_switch_impl.h"
#include "base/enums.h"
+#include "base/globals.h"
#include "base/memory_tool.h"
#include "base/quasi_atomic.h"
#include "dex/dex_file_types.h"
#include "dex/dex_instruction_list.h"
#include "experimental_flags.h"
+#include "handle_scope.h"
#include "interpreter_common.h"
+#include "interpreter/shadow_frame.h"
#include "jit/jit-inl.h"
#include "jvalue-inl.h"
#include "mirror/string-alloc-inl.h"
+#include "mirror/throwable.h"
#include "nth_caller_visitor.h"
#include "safe_math.h"
#include "shadow_frame-inl.h"
#include "thread.h"
+#include "verifier/method_verifier.h"
namespace art {
namespace interpreter {
@@ -49,12 +54,46 @@
template<bool do_access_check, bool transaction_active, Instruction::Format kFormat>
class InstructionHandler {
public:
+ template <bool kMonitorCounting>
+ static NO_INLINE void UnlockHeldMonitors(Thread* self, ShadowFrame* shadow_frame)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ DCHECK(shadow_frame->GetForcePopFrame());
+ // Unlock all monitors.
+ if (kMonitorCounting && shadow_frame->GetMethod()->MustCountLocks()) {
+ // Get the monitors from the shadow-frame monitor-count data.
+ shadow_frame->GetLockCountData().VisitMonitors(
+ [&](mirror::Object** obj) REQUIRES_SHARED(Locks::mutator_lock_) {
+ // Since we don't use the 'obj' pointer after the DoMonitorExit everything should be fine
+ // WRT suspension.
+ DoMonitorExit<do_assignability_check>(self, shadow_frame, *obj);
+ });
+ } else {
+ std::vector<verifier::MethodVerifier::DexLockInfo> locks;
+ verifier::MethodVerifier::FindLocksAtDexPc(shadow_frame->GetMethod(),
+ shadow_frame->GetDexPC(),
+ &locks,
+ Runtime::Current()->GetTargetSdkVersion());
+ for (const auto& reg : locks) {
+ if (UNLIKELY(reg.dex_registers.empty())) {
+ LOG(ERROR) << "Unable to determine reference locked by "
+ << shadow_frame->GetMethod()->PrettyMethod() << " at pc "
+ << shadow_frame->GetDexPC();
+ } else {
+ DoMonitorExit<do_assignability_check>(
+ self, shadow_frame, shadow_frame->GetVRegReference(*reg.dex_registers.begin()));
+ }
+ }
+ }
+ }
+
ALWAYS_INLINE WARN_UNUSED bool CheckForceReturn()
REQUIRES_SHARED(Locks::mutator_lock_) {
if (UNLIKELY(shadow_frame.GetForcePopFrame())) {
DCHECK(PrevFrameWillRetry(self, shadow_frame))
<< "Pop frame forced without previous frame ready to retry instruction!";
DCHECK(Runtime::Current()->AreNonStandardExitsEnabled());
+ UnlockHeldMonitors<do_assignability_check>(self, &shadow_frame);
+ DoMonitorCheckOnExit<do_assignability_check>(self, &shadow_frame);
if (UNLIKELY(NeedsMethodExitEvent(instrumentation))) {
SendMethodExitEvents(self,
instrumentation,
diff --git a/test/1953-pop-frame/expected.txt b/test/1953-pop-frame/expected.txt
index 906703d..079768d 100644
--- a/test/1953-pop-frame/expected.txt
+++ b/test/1953-pop-frame/expected.txt
@@ -96,3 +96,6 @@
art.Test1953.runTests(Test1953.java)
<Additional frames hidden>
result is NativeCallerObject { cnt: 1 } base-call count: 1
+Test stopped with monitor in enclosing frame.
+Single call with PopFrame on StandardTestObject { cnt: 0 } base-call-count: 0
+result is StandardTestObject { cnt: 2 } base-call count: 1
diff --git a/test/1953-pop-frame/src/art/Test1953.java b/test/1953-pop-frame/src/art/Test1953.java
index fa49134..ff41d24 100644
--- a/test/1953-pop-frame/src/art/Test1953.java
+++ b/test/1953-pop-frame/src/art/Test1953.java
@@ -461,9 +461,13 @@
public final Object lock;
public SynchronizedTestObject() {
+ this(new Object());
+ }
+
+ public SynchronizedTestObject(Object l) {
super();
cnt = 0;
- lock = new Object();
+ lock = l;
}
public void calledFunction() {
@@ -741,9 +745,12 @@
final int syncLine = Breakpoint.locationToLine(syncCalledFunction, 0) + 3;
final long syncLoc = Breakpoint.lineToLocation(syncCalledFunction, syncLine);
System.out.println("Test stopped using breakpoint with synchronized block");
- runTestOn(new SynchronizedTestObject(),
+ Object lock = new Object();
+ synchronized (lock) {}
+ runTestOn(new SynchronizedTestObject(lock),
(thr) -> setupSuspendBreakpointFor(syncCalledFunction, syncLoc, thr),
SuspendEvents::clearSuspendBreakpointFor);
+ synchronized (lock) {}
System.out.println("Test stopped on single step");
runTestOn(new StandardTestObject(),
@@ -919,6 +926,22 @@
runTestOn(new NativeCallerObject(),
(thr) -> setupSuspendMethodEvent(nativeCallerMethod, /*enter*/ false, thr),
SuspendEvents::clearSuspendMethodEvent);
+
+
+ final Object lock2 = new Object();
+ synchronized (lock2) {}
+ System.out.println("Test stopped with monitor in enclosing frame.");
+ runTestOn(new StandardTestObject() {
+ @Override
+ public void run() {
+ synchronized (lock2) {
+ super.run();
+ }
+ }
+ },
+ (thr) -> setupSuspendBreakpointFor(calledFunction, loc, thr),
+ SuspendEvents::clearSuspendBreakpointFor);
+ synchronized (lock2) {}
}
// Volatile is to prevent any future optimizations that could invalidate this test by doing
diff --git a/test/1954-pop-frame-jit/expected.txt b/test/1954-pop-frame-jit/expected.txt
index a20a045..e75ea64 100644
--- a/test/1954-pop-frame-jit/expected.txt
+++ b/test/1954-pop-frame-jit/expected.txt
@@ -116,3 +116,6 @@
art.Test1953.runTests(Test1953.java)
<Additional frames hidden>
result is NativeCallerObject { cnt: 1 } base-call count: 1
+Test stopped with monitor in enclosing frame.
+Single call with PopFrame on StandardTestObject { cnt: 0 } base-call-count: 0
+result is StandardTestObject { cnt: 2 } base-call count: 1
diff --git a/test/1955-pop-frame-jit-called/expected.txt b/test/1955-pop-frame-jit-called/expected.txt
index a20a045..e75ea64 100644
--- a/test/1955-pop-frame-jit-called/expected.txt
+++ b/test/1955-pop-frame-jit-called/expected.txt
@@ -116,3 +116,6 @@
art.Test1953.runTests(Test1953.java)
<Additional frames hidden>
result is NativeCallerObject { cnt: 1 } base-call count: 1
+Test stopped with monitor in enclosing frame.
+Single call with PopFrame on StandardTestObject { cnt: 0 } base-call-count: 0
+result is StandardTestObject { cnt: 2 } base-call count: 1
diff --git a/test/1956-pop-frame-jit-calling/expected.txt b/test/1956-pop-frame-jit-calling/expected.txt
index a20a045..e75ea64 100644
--- a/test/1956-pop-frame-jit-calling/expected.txt
+++ b/test/1956-pop-frame-jit-calling/expected.txt
@@ -116,3 +116,6 @@
art.Test1953.runTests(Test1953.java)
<Additional frames hidden>
result is NativeCallerObject { cnt: 1 } base-call count: 1
+Test stopped with monitor in enclosing frame.
+Single call with PopFrame on StandardTestObject { cnt: 0 } base-call-count: 0
+result is StandardTestObject { cnt: 2 } base-call count: 1