From 90a33595bc637f5768a7726a186bdfe25efcd0d6 Mon Sep 17 00:00:00 2001 From: Sebastien Hertz Date: Fri, 16 Jan 2015 19:49:09 +0100 Subject: [PATCH] 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 (cherry picked from commit 270a0e16c3b8e5b95cbfdbd8996ac137c7c6322b) Change-Id: Ib4fdd0ad704b4f2369d229737c9cc77f559cea55 --- runtime/instrumentation.cc | 11 +++++------ runtime/interpreter/interpreter.cc | 18 +++++++++++++++++- runtime/interpreter/interpreter_goto_table_impl.cc | 6 +++++- runtime/interpreter/interpreter_switch_impl.cc | 6 +++++- runtime/quick_exception_handler.cc | 4 +--- runtime/verifier/method_verifier.cc | 6 ++++++ 6 files changed, 39 insertions(+), 12 deletions(-) diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc index 8282fab99..d06a67f92 100644 --- a/runtime/instrumentation.cc +++ b/runtime/instrumentation.cc @@ -1088,15 +1088,14 @@ TwoWordReturn Instrumentation::PopInstrumentationStackFrame(Thread* self, uintpt // 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 26c70999b..d2a3d09e1 100644 --- a/runtime/interpreter/interpreter.cc +++ b/runtime/interpreter/interpreter.cc @@ -500,7 +500,23 @@ void EnterInterpreterFromDeoptimize(Thread* self, ShadowFrame* shadow_frame, JVa StackHandleScope<1> hs(self); MethodHelper mh(hs.NewHandle(shadow_frame->GetMethod())); const DexFile::CodeItem* code_item = mh.GetMethod()->GetCodeItem(); - value = Execute(self, mh, 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, mh, 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 e098ac86e..f9c497809 100644 --- a/runtime/interpreter/interpreter_goto_table_impl.cc +++ b/runtime/interpreter/interpreter_goto_table_impl.cc @@ -147,7 +147,10 @@ JValue ExecuteGotoImpl(Thread* self, MethodHelper& mh, const DexFile::CodeItem* 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_), @@ -235,6 +238,7 @@ JValue ExecuteGotoImpl(Thread* self, MethodHelper& mh, const DexFile::CodeItem* 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 540149515..588818083 100644 --- a/runtime/interpreter/interpreter_switch_impl.cc +++ b/runtime/interpreter/interpreter_switch_impl.cc @@ -70,7 +70,10 @@ JValue ExecuteSwitchImpl(Thread* self, MethodHelper& mh, const DexFile::CodeItem 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); @@ -162,6 +165,7 @@ JValue ExecuteSwitchImpl(Thread* self, MethodHelper& mh, const DexFile::CodeItem 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 d4713a052..9ad221c07 100644 --- a/runtime/quick_exception_handler.cc +++ b/runtime/quick_exception_handler.cc @@ -203,9 +203,7 @@ class DeoptimizeStackVisitor FINAL : public StackVisitor { 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<2> hs(self_); mirror::Class* declaring_class = m->GetDeclaringClass(); Handle h_dex_cache(hs.NewHandle(declaring_class->GetDexCache())); diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index f46cd1420..ee7bd144b 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -1555,6 +1555,12 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) { 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. -- 2.11.0