ART: Balanced locking
Change the verifier to check for balanced locking. When balanced
locking can't be guaranteed, use a new failure kind to punt to
the interpreter.
Add smali tests, with JNI code to check the balanced-locking result.
Bug: 23502994
Change-Id: Icd7db0be20ef2f69f0ac784de43dcba990035cd8
diff --git a/test/088-monitor-verification/expected.txt b/test/088-monitor-verification/expected.txt
index 07f5b0b..13b8c73 100644
--- a/test/088-monitor-verification/expected.txt
+++ b/test/088-monitor-verification/expected.txt
@@ -1,7 +1,12 @@
recursiveSync ok
nestedMayThrow ok
constantLock ok
-excessiveNesting ok
notNested ok
twoPath ok
triplet ok
+OK
+TooDeep
+NotStructuredOverUnlock
+NotStructuredUnderUnlock
+UnbalancedJoin
+UnbalancedStraight
diff --git a/test/088-monitor-verification/smali/NotStructuredOverUnlock.smali b/test/088-monitor-verification/smali/NotStructuredOverUnlock.smali
new file mode 100644
index 0000000..aa0c2d5
--- /dev/null
+++ b/test/088-monitor-verification/smali/NotStructuredOverUnlock.smali
@@ -0,0 +1,21 @@
+.class public LNotStructuredOverUnlock;
+
+.super Ljava/lang/Object;
+
+.method public static run(Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsInterpreted()V
+
+ # Lock twice, but unlock thrice.
+
+ monitor-enter v2 # 1
+ monitor-enter v2 # 2
+
+ monitor-exit v2 # 1
+ monitor-exit v2 # 2
+ monitor-exit v2 # 3
+
+ return-void
+
+.end method
diff --git a/test/088-monitor-verification/smali/NotStructuredUnderUnlock.smali b/test/088-monitor-verification/smali/NotStructuredUnderUnlock.smali
new file mode 100644
index 0000000..2c31fda
--- /dev/null
+++ b/test/088-monitor-verification/smali/NotStructuredUnderUnlock.smali
@@ -0,0 +1,21 @@
+.class public LNotStructuredUnderUnlock;
+
+.super Ljava/lang/Object;
+
+.method public static run(Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsInterpreted()V
+
+ # Lock thrice, but only unlock twice.
+
+ monitor-enter v2 # 1
+ monitor-enter v2 # 2
+ monitor-enter v2 # 3
+
+ monitor-exit v2 # 1
+ monitor-exit v2 # 2
+
+ return-void
+
+.end method
diff --git a/test/088-monitor-verification/smali/OK.smali b/test/088-monitor-verification/smali/OK.smali
new file mode 100644
index 0000000..596798d
--- /dev/null
+++ b/test/088-monitor-verification/smali/OK.smali
@@ -0,0 +1,68 @@
+.class public LOK;
+
+.super Ljava/lang/Object;
+
+.method public static run(Ljava/lang/Object;Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {v1, v2}, LOK;->runNoMonitors(Ljava/lang/Object;Ljava/lang/Object;)V
+
+ invoke-static {v1, v2}, LOK;->runStraightLine(Ljava/lang/Object;Ljava/lang/Object;)V
+
+ invoke-static {v1, v2}, LOK;->runBalancedJoin(Ljava/lang/Object;Ljava/lang/Object;)V
+
+ return-void
+
+.end method
+
+
+
+.method public static runNoMonitors(Ljava/lang/Object;Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsManaged()V
+
+ return-void
+
+.end method
+
+.method public static runStraightLine(Ljava/lang/Object;Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsManaged()V
+
+ monitor-enter v1 # 1
+ monitor-enter v2 # 2
+
+ monitor-exit v2 # 2
+ monitor-exit v1 # 1
+
+ return-void
+
+.end method
+
+.method public static runBalancedJoin(Ljava/lang/Object;Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsManaged()V
+
+ monitor-enter v1 # 1
+
+ if-eqz v2, :Lnull
+
+:LnotNull
+
+ monitor-enter v2 # 2
+ goto :Lend
+
+:Lnull
+ monitor-enter v2 # 2
+
+:Lend
+
+ monitor-exit v2 # 2
+ monitor-exit v1 # 1
+
+ return-void
+
+.end method
diff --git a/test/088-monitor-verification/smali/TooDeep.smali b/test/088-monitor-verification/smali/TooDeep.smali
new file mode 100644
index 0000000..1a8f2f0
--- /dev/null
+++ b/test/088-monitor-verification/smali/TooDeep.smali
@@ -0,0 +1,82 @@
+.class public LTooDeep;
+
+.super Ljava/lang/Object;
+
+.method public static run(Ljava/lang/Object;)V
+ .registers 3
+
+ # Lock depth is 33, which is more than the verifier supports. This should have been punted to
+ # the interpreter.
+ invoke-static {}, LMain;->assertCallerIsInterpreted()V
+
+ monitor-enter v2 # 1
+ monitor-enter v2 # 2
+ monitor-enter v2 # 3
+ monitor-enter v2 # 4
+ monitor-enter v2 # 5
+ monitor-enter v2 # 6
+ monitor-enter v2 # 7
+ monitor-enter v2 # 8
+ monitor-enter v2 # 9
+ monitor-enter v2 # 10
+ monitor-enter v2 # 11
+ monitor-enter v2 # 12
+ monitor-enter v2 # 13
+ monitor-enter v2 # 14
+ monitor-enter v2 # 15
+ monitor-enter v2 # 16
+ monitor-enter v2 # 17
+ monitor-enter v2 # 18
+ monitor-enter v2 # 19
+ monitor-enter v2 # 20
+ monitor-enter v2 # 21
+ monitor-enter v2 # 22
+ monitor-enter v2 # 23
+ monitor-enter v2 # 24
+ monitor-enter v2 # 25
+ monitor-enter v2 # 26
+ monitor-enter v2 # 27
+ monitor-enter v2 # 28
+ monitor-enter v2 # 29
+ monitor-enter v2 # 30
+ monitor-enter v2 # 31
+ monitor-enter v2 # 32
+ monitor-enter v2 # 33
+
+ monitor-exit v2 # 1
+ monitor-exit v2 # 2
+ monitor-exit v2 # 3
+ monitor-exit v2 # 4
+ monitor-exit v2 # 5
+ monitor-exit v2 # 6
+ monitor-exit v2 # 7
+ monitor-exit v2 # 8
+ monitor-exit v2 # 9
+ monitor-exit v2 # 10
+ monitor-exit v2 # 11
+ monitor-exit v2 # 12
+ monitor-exit v2 # 13
+ monitor-exit v2 # 14
+ monitor-exit v2 # 15
+ monitor-exit v2 # 16
+ monitor-exit v2 # 17
+ monitor-exit v2 # 18
+ monitor-exit v2 # 19
+ monitor-exit v2 # 20
+ monitor-exit v2 # 21
+ monitor-exit v2 # 22
+ monitor-exit v2 # 23
+ monitor-exit v2 # 24
+ monitor-exit v2 # 25
+ monitor-exit v2 # 26
+ monitor-exit v2 # 27
+ monitor-exit v2 # 28
+ monitor-exit v2 # 29
+ monitor-exit v2 # 30
+ monitor-exit v2 # 31
+ monitor-exit v2 # 32
+ monitor-exit v2 # 33
+
+ return-void
+
+.end method
diff --git a/test/088-monitor-verification/smali/UnbalancedJoin.smali b/test/088-monitor-verification/smali/UnbalancedJoin.smali
new file mode 100644
index 0000000..da8f773
--- /dev/null
+++ b/test/088-monitor-verification/smali/UnbalancedJoin.smali
@@ -0,0 +1,31 @@
+.class public LUnbalancedJoin;
+
+.super Ljava/lang/Object;
+
+.method public static run(Ljava/lang/Object;Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsInterpreted()V
+
+ if-eqz v2, :Lnull
+
+:LnotNull
+
+ monitor-enter v1 # 1
+ monitor-enter v2 # 2
+ goto :Lend
+
+:Lnull
+ monitor-enter v2 # 1
+ monitor-enter v1 # 2
+
+:Lend
+
+ # Lock levels are "opposite" for the joined flows.
+
+ monitor-exit v2 # 2
+ monitor-exit v1 # 1
+
+ return-void
+
+.end method
diff --git a/test/088-monitor-verification/smali/UnbalancedStraight.smali b/test/088-monitor-verification/smali/UnbalancedStraight.smali
new file mode 100644
index 0000000..68edb6c
--- /dev/null
+++ b/test/088-monitor-verification/smali/UnbalancedStraight.smali
@@ -0,0 +1,18 @@
+.class public LUnbalancedStraight;
+
+.super Ljava/lang/Object;
+
+.method public static run(Ljava/lang/Object;Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsInterpreted()V
+
+ monitor-enter v1 # 1
+ monitor-enter v2 # 2
+
+ monitor-exit v1 # 1 Unbalanced unlock.
+ monitor-exit v2 # 2
+
+ return-void
+
+.end method
diff --git a/test/088-monitor-verification/src/Main.java b/test/088-monitor-verification/src/Main.java
index b60c71e..af1eaea 100644
--- a/test/088-monitor-verification/src/Main.java
+++ b/test/088-monitor-verification/src/Main.java
@@ -14,6 +14,9 @@
* limitations under the License.
*/
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
/*
* Entry point and tests that are expected to succeed.
@@ -38,11 +41,6 @@
System.out.println("constantLock ok");
m.notExcessiveNesting();
- try {
- TooDeep.excessiveNesting();
- System.err.println("excessiveNesting did not throw");
- } catch (VerifyError ve) {}
- System.out.println("excessiveNesting ok");
m.notNested();
System.out.println("notNested ok");
@@ -55,6 +53,9 @@
m.triplet(obj1, obj2, 0);
System.out.println("triplet ok");
+
+ System.loadLibrary("arttest");
+ runSmaliTests();
}
/**
@@ -216,4 +217,62 @@
doNothing(localObj);
}
+
+ // Smali testing code.
+ private static void runSmaliTests() {
+ runTest("OK", new Object[] { new Object(), new Object() }, null);
+ runTest("TooDeep", new Object[] { new Object() }, null);
+ runTest("NotStructuredOverUnlock", new Object[] { new Object() },
+ IllegalMonitorStateException.class);
+ runTest("NotStructuredUnderUnlock", new Object[] { new Object() }, null);
+ // TODO: new IllegalMonitorStateException());
+ runTest("UnbalancedJoin", new Object[] { new Object(), new Object() }, null);
+ runTest("UnbalancedStraight", new Object[] { new Object(), new Object() }, null);
+ }
+
+ private static void runTest(String className, Object[] parameters, Class<?> excType) {
+ System.out.println(className);
+ try {
+ Class<?> c = Class.forName(className);
+
+ Method[] methods = c.getDeclaredMethods();
+
+ // For simplicity we assume that test methods are not overloaded. So searching by name
+ // will give us the method we need to run.
+ Method method = null;
+ for (Method m : methods) {
+ if (m.getName().equals("run")) {
+ method = m;
+ break;
+ }
+ }
+
+ if (method == null) {
+ System.out.println("Could not find test method for " + className);
+ } else if (!Modifier.isStatic(method.getModifiers())) {
+ System.out.println("Test method for " + className + " is not static.");
+ } else {
+ method.invoke(null, parameters);
+ if (excType != null) {
+ System.out.println("Expected an exception in " + className);
+ }
+ }
+ } catch (Throwable exc) {
+ if (excType == null) {
+ System.out.println("Did not expect exception " + exc + " for " + className);
+ exc.printStackTrace(System.out);
+ } else if (exc instanceof InvocationTargetException && exc.getCause() != null &&
+ exc.getCause().getClass().equals(excType)) {
+ // Expected exception is wrapped in InvocationTargetException.
+ } else if (!excType.equals(exc.getClass())) {
+ System.out.println("Expected " + excType.getName() + ", but got " + exc.getClass());
+ } else {
+ // Expected exception, do nothing.
+ }
+ }
+ }
+
+ // Helpers for the smali code.
+ public static native void assertCallerIsInterpreted();
+ public static native void assertCallerIsManaged();
}
diff --git a/test/088-monitor-verification/src/TooDeep.java b/test/088-monitor-verification/src/TooDeep.java
deleted file mode 100644
index 76192e5..0000000
--- a/test/088-monitor-verification/src/TooDeep.java
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * Copyright (C) 2010 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-
-/**
- * The class has a method with too many levels of nested "synchronized"
- * blocks. The verifier will reject it.
- *
- * (It would be perfectly okay if the verifier *didn't* reject this.
- * The goal here is just to exercise the failure path. It also serves
- * as a check to see if the monitor checks are enabled.)
- */
-public class TooDeep {
-
- public static void excessiveNesting() {
- synchronized (TooDeep.class) { // 1
- synchronized (TooDeep.class) { // 2
- synchronized (TooDeep.class) { // 3
- synchronized (TooDeep.class) { // 4
- synchronized (TooDeep.class) { // 5
- synchronized (TooDeep.class) { // 6
- synchronized (TooDeep.class) { // 7
- synchronized (TooDeep.class) { // 8
- synchronized (TooDeep.class) { // 9
- synchronized (TooDeep.class) { // 10
- synchronized (TooDeep.class) { // 11
- synchronized (TooDeep.class) { // 12
- synchronized (TooDeep.class) { // 13
- synchronized (TooDeep.class) { // 14
- synchronized (TooDeep.class) { // 15
- synchronized (TooDeep.class) { // 16
- synchronized (TooDeep.class) { // 17
- synchronized (TooDeep.class) { // 18
- synchronized (TooDeep.class) { // 19
- synchronized (TooDeep.class) { // 20
- synchronized (TooDeep.class) { // 21
- synchronized (TooDeep.class) { // 22
- synchronized (TooDeep.class) { // 23
- synchronized (TooDeep.class) { // 24
- synchronized (TooDeep.class) { // 25
- synchronized (TooDeep.class) { // 26
- synchronized (TooDeep.class) { // 27
- synchronized (TooDeep.class) { // 28
- synchronized (TooDeep.class) { // 29
- synchronized (TooDeep.class) { // 30
- synchronized (TooDeep.class) { // 31
- synchronized (TooDeep.class) { // 32
- synchronized (TooDeep.class) { // 33
- }}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}
- }
-}
diff --git a/test/088-monitor-verification/stack_inspect.cc b/test/088-monitor-verification/stack_inspect.cc
new file mode 100644
index 0000000..e2899c3
--- /dev/null
+++ b/test/088-monitor-verification/stack_inspect.cc
@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "jni.h"
+
+#include "base/logging.h"
+#include "dex_file-inl.h"
+#include "mirror/class-inl.h"
+#include "nth_caller_visitor.h"
+#include "runtime.h"
+#include "scoped_thread_state_change.h"
+#include "stack.h"
+#include "thread-inl.h"
+
+namespace art {
+
+// public static native void assertCallerIsInterpreted();
+
+extern "C" JNIEXPORT void JNICALL Java_Main_assertCallerIsInterpreted(JNIEnv* env, jclass) {
+ LOG(INFO) << "assertCallerIsInterpreted";
+
+ ScopedObjectAccess soa(env);
+ NthCallerVisitor caller(soa.Self(), 1, false);
+ caller.WalkStack();
+ CHECK(caller.caller != nullptr);
+ LOG(INFO) << PrettyMethod(caller.caller);
+ CHECK(caller.GetCurrentShadowFrame() != nullptr);
+}
+
+// public static native void assertCallerIsManaged();
+
+extern "C" JNIEXPORT void JNICALL Java_Main_assertCallerIsManaged(JNIEnv* env, jclass cls) {
+ // Note: needs some smarts to not fail if there is no managed code, at all.
+ LOG(INFO) << "assertCallerIsManaged";
+
+ ScopedObjectAccess soa(env);
+
+ mirror::Class* klass = soa.Decode<mirror::Class*>(cls);
+ const DexFile& dex_file = klass->GetDexFile();
+ const OatFile::OatDexFile* oat_dex_file = dex_file.GetOatDexFile();
+ if (oat_dex_file == nullptr) {
+ // No oat file, this must be a test configuration that doesn't compile at all. Ignore that the
+ // result will be that we're running the interpreter.
+ return;
+ }
+
+ NthCallerVisitor caller(soa.Self(), 1, false);
+ caller.WalkStack();
+ CHECK(caller.caller != nullptr);
+ LOG(INFO) << PrettyMethod(caller.caller);
+
+ if (caller.GetCurrentShadowFrame() == nullptr) {
+ // Not a shadow frame, this looks good.
+ return;
+ }
+
+ // This could be an interpret-only or a verify-at-runtime compilation, or a read-barrier variant,
+ // or... It's not really safe to just reject now. Let's look at the access flags. If the method
+ // was successfully verified, its access flags should be set to mark it preverified, except when
+ // we're running soft-fail tests.
+ if (Runtime::Current()->IsVerificationSoftFail()) {
+ // Soft-fail config. Everything should be running with interpreter access checks, potentially.
+ return;
+ }
+ CHECK(caller.caller->IsPreverified());
+}
+
+} // namespace art
diff --git a/test/Android.libarttest.mk b/test/Android.libarttest.mk
index fcb9f8a..6150e87 100644
--- a/test/Android.libarttest.mk
+++ b/test/Android.libarttest.mk
@@ -25,6 +25,7 @@
004-StackWalk/stack_walk_jni.cc \
004-UnsafeTest/unsafe_test.cc \
051-thread/thread_test.cc \
+ 088-monitor-verification/stack_inspect.cc \
116-nodex2oat/nodex2oat.cc \
117-nopatchoat/nopatchoat.cc \
118-noimage-dex2oat/noimage-dex2oat.cc \