Fix exception handling during deoptimization
When interpreting a deoptimized shadow frame, we may start with a
pending exception thrown by a previous deoptimized shadow frame (from
a previous invoke). Therefore, we need to handle it before executing
any instruction, otherwise we execute incorrect code.
Because we need the DEX pc of the throwing instruction to find a
matching catch handler, we initialize deoptimized shadow frames with
the current DEX pc at the time the stack is deoptimized.
When we are about to interpret a deoptimized shadow frame, we need to
update the shadow frame with the DEX pc of the next instruction to
interpret. There are three cases:
- if there is no pending exception, this is the instruction following
the current one.
- if there is a pending exception and we found a matching catch
handler, this is the first instruction of this handler.
- if there is a pending exception but there is no matching catch
handler, we do not execute the deoptimized shadow frame and continue
to its caller.
The verifier now fails when a method starts with a move-exception
instruction. Indeed we cannot start executing a method with a pending
exception.
Bug: 19057915
Bug: 19041195
Bug: 18607595
Change-Id: I355ac81e6ac098edc7e3cc8c13dbfa24a2969ab2
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 1548cfd..b8dcb42 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -999,15 +999,14 @@
// back to an upcall.
NthCallerVisitor visitor(self, 1, true);
visitor.WalkStack(true);
- bool deoptimize = (visitor.caller != NULL) &&
+ bool deoptimize = (visitor.caller != nullptr) &&
(interpreter_stubs_installed_ || IsDeoptimized(visitor.caller));
- if (deoptimize && kVerboseInstrumentation) {
- LOG(INFO) << "Deoptimizing into " << PrettyMethod(visitor.caller);
- }
if (deoptimize) {
if (kVerboseInstrumentation) {
- LOG(INFO) << "Deoptimizing from " << PrettyMethod(method)
- << " result is " << std::hex << return_value.GetJ();
+ LOG(INFO) << StringPrintf("Deoptimizing %s by returning from %s with result %#" PRIx64 " in ",
+ PrettyMethod(visitor.caller).c_str(),
+ PrettyMethod(method).c_str(),
+ return_value.GetJ()) << *self;
}
self->SetDeoptimizationReturnValue(return_value);
return GetTwoWordSuccessValue(*return_pc,
diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index b04a18b..9d988e9 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -493,7 +493,23 @@
while (shadow_frame != NULL) {
self->SetTopOfShadowStack(shadow_frame);
const DexFile::CodeItem* code_item = shadow_frame->GetMethod()->GetCodeItem();
- value = Execute(self, code_item, *shadow_frame, value);
+ const uint32_t dex_pc = shadow_frame->GetDexPC();
+ uint32_t new_dex_pc;
+ if (UNLIKELY(self->IsExceptionPending())) {
+ const instrumentation::Instrumentation* const instrumentation =
+ Runtime::Current()->GetInstrumentation();
+ uint32_t found_dex_pc = FindNextInstructionFollowingException(self, *shadow_frame, dex_pc,
+ instrumentation);
+ new_dex_pc = found_dex_pc; // the dex pc of a matching catch handler
+ // or DexFile::kDexNoIndex if there is none.
+ } else {
+ const Instruction* instr = Instruction::At(&code_item->insns_[dex_pc]);
+ new_dex_pc = dex_pc + instr->SizeInCodeUnits(); // the dex pc of the next instruction.
+ }
+ if (new_dex_pc != DexFile::kDexNoIndex) {
+ shadow_frame->SetDexPC(new_dex_pc);
+ value = Execute(self, code_item, *shadow_frame, value);
+ }
ShadowFrame* old_frame = shadow_frame;
shadow_frame = shadow_frame->GetLink();
delete old_frame;
diff --git a/runtime/interpreter/interpreter_goto_table_impl.cc b/runtime/interpreter/interpreter_goto_table_impl.cc
index 8fcbf90..e4b3247 100644
--- a/runtime/interpreter/interpreter_goto_table_impl.cc
+++ b/runtime/interpreter/interpreter_goto_table_impl.cc
@@ -148,7 +148,10 @@
const void* const* currentHandlersTable;
bool notified_method_entry_event = false;
UPDATE_HANDLER_TABLE();
- if (LIKELY(dex_pc == 0)) { // We are entering the method as opposed to deoptimizing..
+ if (LIKELY(dex_pc == 0)) { // We are entering the method as opposed to deoptimizing.
+ if (kIsDebugBuild) {
+ self->AssertNoPendingException();
+ }
instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
if (UNLIKELY(instrumentation->HasMethodEntryListeners())) {
instrumentation->MethodEnterEvent(self, shadow_frame.GetThisObject(code_item->ins_size_),
@@ -236,6 +239,7 @@
HANDLE_INSTRUCTION_START(MOVE_EXCEPTION) {
Throwable* exception = self->GetException(nullptr);
+ DCHECK(exception != nullptr) << "No pending exception on MOVE_EXCEPTION instruction";
shadow_frame.SetVRegReference(inst->VRegA_11x(inst_data), exception);
self->ClearException();
ADVANCE(1);
diff --git a/runtime/interpreter/interpreter_switch_impl.cc b/runtime/interpreter/interpreter_switch_impl.cc
index 38665c7..2f85587 100644
--- a/runtime/interpreter/interpreter_switch_impl.cc
+++ b/runtime/interpreter/interpreter_switch_impl.cc
@@ -69,7 +69,10 @@
uint32_t dex_pc = shadow_frame.GetDexPC();
bool notified_method_entry_event = false;
const instrumentation::Instrumentation* const instrumentation = Runtime::Current()->GetInstrumentation();
- if (LIKELY(dex_pc == 0)) { // We are entering the method as opposed to deoptimizing..
+ if (LIKELY(dex_pc == 0)) { // We are entering the method as opposed to deoptimizing.
+ if (kIsDebugBuild) {
+ self->AssertNoPendingException();
+ }
if (UNLIKELY(instrumentation->HasMethodEntryListeners())) {
instrumentation->MethodEnterEvent(self, shadow_frame.GetThisObject(code_item->ins_size_),
shadow_frame.GetMethod(), 0);
@@ -161,6 +164,7 @@
case Instruction::MOVE_EXCEPTION: {
PREAMBLE();
Throwable* exception = self->GetException(nullptr);
+ DCHECK(exception != nullptr) << "No pending exception on MOVE_EXCEPTION instruction";
shadow_frame.SetVRegReference(inst->VRegA_11x(inst_data), exception);
self->ClearException();
inst = inst->Next_1xx();
diff --git a/runtime/quick_exception_handler.cc b/runtime/quick_exception_handler.cc
index 3517848..34f6713 100644
--- a/runtime/quick_exception_handler.cc
+++ b/runtime/quick_exception_handler.cc
@@ -204,9 +204,7 @@
CHECK(code_item != nullptr);
uint16_t num_regs = code_item->registers_size_;
uint32_t dex_pc = GetDexPc();
- const Instruction* inst = Instruction::At(code_item->insns_ + dex_pc);
- uint32_t new_dex_pc = dex_pc + inst->SizeInCodeUnits();
- ShadowFrame* new_frame = ShadowFrame::Create(num_regs, nullptr, m, new_dex_pc);
+ ShadowFrame* new_frame = ShadowFrame::Create(num_regs, nullptr, m, dex_pc);
StackHandleScope<3> hs(self_);
mirror::Class* declaring_class = m->GetDeclaringClass();
Handle<mirror::DexCache> h_dex_cache(hs.NewHandle(declaring_class->GetDexCache()));
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 88944d7..474a066 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -1643,6 +1643,12 @@
break;
case Instruction::MOVE_EXCEPTION: {
+ // We do not allow MOVE_EXCEPTION as the first instruction in a method. This is a simple case
+ // where one entrypoint to the catch block is not actually an exception path.
+ if (work_insn_idx_ == 0) {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "move-exception at pc 0x0";
+ break;
+ }
/*
* This statement can only appear as the first instruction in an exception handler. We verify
* that as part of extracting the exception type from the catch block list.
diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt
index 6cb08f4..019dc14 100644
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -13,4 +13,5 @@
b/18800943 (1)
b/18800943 (2)
MoveExc
+MoveExceptionOnEntry
Done!
diff --git a/test/800-smali/smali/move_exception_on_entry.smali b/test/800-smali/smali/move_exception_on_entry.smali
new file mode 100644
index 0000000..e7da2e3
--- /dev/null
+++ b/test/800-smali/smali/move_exception_on_entry.smali
@@ -0,0 +1,30 @@
+.class public LMoveExceptionOnEntry;
+
+.super Ljava/lang/Object;
+
+# Test that we cannot have a catch-handler with move-exception at the beginning of a method.
+
+.method public static moveExceptionOnEntry(I)I
+.registers 4
+:Label1
+ move-exception v2
+ const v1, 100
+ move v0, p0
+ add-int/lit8 p0, p0, 1
+
+:Label2
+ invoke-static {v0}, LMoveExceptionOnEntry;->foo(I)V
+
+:Label3
+ return v1
+
+.catchall {:Label2 .. :Label3} :Label1
+.end method
+
+.method public static foo(I)I
+.registers 4
+:Label1
+ return-void
+
+.end method
+
diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java
index 2eda850..b23896d 100644
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -50,25 +50,33 @@
// Create the test cases.
testCases = new LinkedList<TestCase>();
testCases.add(new TestCase("PackedSwitch", "PackedSwitch", "packedSwitch",
- new Object[]{123}, null, 123));
+ new Object[]{123}, null, 123));
testCases.add(new TestCase("b/17790197", "B17790197", "getInt", null, null, 100));
- testCases.add(new TestCase("b/17978759", "B17978759", "test", null, new VerifyError(), null));
+ testCases.add(new TestCase("b/17978759", "B17978759", "test", null, new VerifyError(),
+ null));
testCases.add(new TestCase("FloatBadArgReg", "FloatBadArgReg", "getInt",
- new Object[]{100}, null, 100));
+ new Object[]{100}, null, 100));
testCases.add(new TestCase("negLong", "negLong", "negLong", null, null, 122142L));
testCases.add(new TestCase("sameFieldNames", "sameFieldNames", "getInt", null, null, 7));
testCases.add(new TestCase("b/18380491", "B18380491ConcreteClass", "foo",
- new Object[]{42}, null, 42));
+ new Object[]{42}, null, 42));
testCases.add(new TestCase("invoke-super abstract", "B18380491ConcreteClass", "foo",
- new Object[]{0}, new AbstractMethodError(), null));
- testCases.add(new TestCase("BadCaseInOpRegRegReg", "BadCaseInOpRegRegReg", "getInt", null, null, 2));
+ new Object[]{0}, new AbstractMethodError(), null));
+ testCases.add(new TestCase("BadCaseInOpRegRegReg", "BadCaseInOpRegRegReg", "getInt", null,
+ null, 2));
testCases.add(new TestCase("CmpLong", "CmpLong", "run", null, null, 0));
- testCases.add(new TestCase("FloatIntConstPassing", "FloatIntConstPassing", "run", null, null, 2));
+ testCases.add(new TestCase("FloatIntConstPassing", "FloatIntConstPassing", "run", null,
+ null, 2));
testCases.add(new TestCase("b/18718277", "B18718277", "getInt", null, null, 0));
- testCases.add(new TestCase("b/18800943 (1)", "B18800943_1", "n_a", null, new VerifyError(), 0));
- testCases.add(new TestCase("b/18800943 (2)", "B18800943_2", "n_a", null, new VerifyError(), 0));
- testCases.add(new TestCase("MoveExc", "MoveExc", "run", null, new ArithmeticException(), null));
+ testCases.add(new TestCase("b/18800943 (1)", "B18800943_1", "n_a", null, new VerifyError(),
+ 0));
+ testCases.add(new TestCase("b/18800943 (2)", "B18800943_2", "n_a", null, new VerifyError(),
+ 0));
+ testCases.add(new TestCase("MoveExc", "MoveExc", "run", null, new ArithmeticException(),
+ null));
+ testCases.add(new TestCase("MoveExceptionOnEntry", "MoveExceptionOnEntry",
+ "moveExceptionOnEntry", new Object[]{0}, new VerifyError(), null));
}
public void runTests() {
diff --git a/test/802-deoptimization/expected.txt b/test/802-deoptimization/expected.txt
new file mode 100644
index 0000000..d5f1f08
--- /dev/null
+++ b/test/802-deoptimization/expected.txt
@@ -0,0 +1 @@
+CatchHandlerOnEntryWithoutMoveException OK
diff --git a/test/802-deoptimization/info.txt b/test/802-deoptimization/info.txt
new file mode 100644
index 0000000..104d40f
--- /dev/null
+++ b/test/802-deoptimization/info.txt
@@ -0,0 +1 @@
+Tests related to deoptimization
diff --git a/test/802-deoptimization/smali/catch_handler_on_entry.smali b/test/802-deoptimization/smali/catch_handler_on_entry.smali
new file mode 100644
index 0000000..836101e
--- /dev/null
+++ b/test/802-deoptimization/smali/catch_handler_on_entry.smali
@@ -0,0 +1,29 @@
+.class public LCatchHandlerOnEntry;
+
+.super Ljava/lang/Object;
+
+# Test we can execute a method starting with a catch handler (without
+# move-exception instruction). This method must be called with parameter
+# initialized to 0.
+#
+# We execute the catch handler (Label1) for the first time with p0 == 0.
+# We save its value in v0, increment p0 to 1 and execute the div-int
+# instruction (Label2) which throws an ArithmeticException (division by zero).
+# That exception is caught by the catch handler so we execute it a second time.
+# Now p0 == 1. When we we execute the div-int instruction, it succeeds and we
+# return its result: this is the initial value of v1 because "v1 = v1 / 1".
+.method public static catchHandlerOnEntry(I)I
+.registers 4
+:Label1
+ const v1, 100
+ move v0, p0
+ add-int/lit8 p0, p0, 1
+
+:Label2
+ invoke-static {v0}, LCatchHandlerOnEntryHelper;->throwExceptionDuringDeopt(I)V
+
+:Label3
+ return v1
+
+.catchall {:Label2 .. :Label3} :Label1
+.end method
diff --git a/test/802-deoptimization/src/CatchHandlerOnEntryHelper.java b/test/802-deoptimization/src/CatchHandlerOnEntryHelper.java
new file mode 100644
index 0000000..a88d31b
--- /dev/null
+++ b/test/802-deoptimization/src/CatchHandlerOnEntryHelper.java
@@ -0,0 +1,30 @@
+/*
+ * 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.
+ */
+
+/**
+ * Helper class used by smali test classes.
+ */
+public class CatchHandlerOnEntryHelper {
+
+ public static void throwExceptionDuringDeopt(int i) {
+ if (i == 0) {
+ DeoptimizationController.startDeoptomization();
+ throw new RuntimeException("Test exception");
+ } else {
+ DeoptimizationController.stopDeoptomization();
+ }
+ }
+}
diff --git a/test/802-deoptimization/src/DeoptimizationController.java b/test/802-deoptimization/src/DeoptimizationController.java
new file mode 100644
index 0000000..c031c07
--- /dev/null
+++ b/test/802-deoptimization/src/DeoptimizationController.java
@@ -0,0 +1,86 @@
+/*
+ * 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.
+ */
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Method;
+
+/**
+ * Controls deoptimization using dalvik.system.VMDebug class.
+ */
+public class DeoptimizationController {
+ public static void startDeoptomization() {
+ try {
+ File tempFile;
+ try {
+ tempFile = File.createTempFile("test", ".trace");
+ } catch (IOException e) {
+ System.setProperty("java.io.tmpdir", "/sdcard");
+ tempFile = File.createTempFile("test", ".trace");
+ }
+ tempFile.deleteOnExit();
+ String tempFileName = tempFile.getPath();
+
+ VMDebug.startMethodTracing(tempFileName, 0, 0, false, 1000);
+ if (VMDebug.getMethodTracingMode() == 0) {
+ throw new IllegalStateException("Not tracing.");
+ }
+ } catch (Exception exc) {
+ exc.printStackTrace(System.err);
+ }
+ }
+
+ public static void stopDeoptomization() {
+ try {
+ VMDebug.stopMethodTracing();
+ if (VMDebug.getMethodTracingMode() != 0) {
+ throw new IllegalStateException("Still tracing.");
+ }
+ } catch (Exception exc) {
+ exc.printStackTrace(System.err);
+ }
+ }
+
+ private static class VMDebug {
+ private static final Method startMethodTracingMethod;
+ private static final Method stopMethodTracingMethod;
+ private static final Method getMethodTracingModeMethod;
+
+ static {
+ try {
+ Class<?> c = Class.forName("dalvik.system.VMDebug");
+ startMethodTracingMethod = c.getDeclaredMethod("startMethodTracing", String.class,
+ Integer.TYPE, Integer.TYPE, Boolean.TYPE, Integer.TYPE);
+ stopMethodTracingMethod = c.getDeclaredMethod("stopMethodTracing");
+ getMethodTracingModeMethod = c.getDeclaredMethod("getMethodTracingMode");
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ public static void startMethodTracing(String filename, int bufferSize, int flags,
+ boolean samplingEnabled, int intervalUs) throws Exception {
+ startMethodTracingMethod.invoke(null, filename, bufferSize, flags, samplingEnabled,
+ intervalUs);
+ }
+ public static void stopMethodTracing() throws Exception {
+ stopMethodTracingMethod.invoke(null);
+ }
+ public static int getMethodTracingMode() throws Exception {
+ return (int) getMethodTracingModeMethod.invoke(null);
+ }
+ }
+}
diff --git a/test/802-deoptimization/src/Main.java b/test/802-deoptimization/src/Main.java
new file mode 100644
index 0000000..c8780de
--- /dev/null
+++ b/test/802-deoptimization/src/Main.java
@@ -0,0 +1,43 @@
+/*
+ * 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.
+ */
+
+import java.lang.reflect.Method;
+
+public class Main {
+ private static final int EXPECTED_RESULT = 100;
+ private static final int PARAMETER_VALUE = 0;
+
+ public static void main(String[] args) throws Throwable {
+ testCatchHandlerOnEntryWithoutMoveException();
+ }
+
+ /**
+ * Tests we correctly execute a method starting with a catch handler without
+ * move-exception instruction when throwing an exception during deoptimization.
+ */
+ private static void testCatchHandlerOnEntryWithoutMoveException() throws Throwable {
+ Class<?> c = Class.forName("CatchHandlerOnEntry");
+ Method m = c.getMethod("catchHandlerOnEntry", int.class);
+ Object result = m.invoke(null, new Object[]{PARAMETER_VALUE});
+ int intResult = ((Integer) result).intValue();
+ if (intResult == EXPECTED_RESULT) {
+ System.out.println("CatchHandlerOnEntryWithoutMoveException OK");
+ } else {
+ System.out.println("CatchHandlerOnEntryWithoutMoveException KO: result==" + intResult);
+ }
+ }
+}
+