Revert "Revert "JVMTI Exception and ExceptionCatch events""
Fixed error where we were incorrectly not updating a ShadowFrame
dex_pc causing deoptimization errors.
Bug: 62821960
Bug: 65049545
Test: ./test.py --host -j50
Test: ./art/tools/run-libcore-tests.sh \
--mode=host --variant-X32 --debug
This reverts commit 959742483885779f106e000df6dd422fc8657931.
Change-Id: I91ab2bc3e645ddf0359c189b19a59a3ecf0d8921
diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index 9cb74f7..3349833 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -522,10 +522,8 @@
// null Instrumentation*.
const instrumentation::Instrumentation* const instrumentation =
first ? nullptr : 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.
+ new_dex_pc = MoveToExceptionHandler(
+ self, *shadow_frame, instrumentation) ? shadow_frame->GetDexPC() : DexFile::kDexNoIndex;
} else if (!from_code) {
// For the debugger and full deoptimization stack, we must go past the invoke
// instruction, as it already executed.
diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index ae461fd..0028b21 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -416,35 +416,57 @@
#undef EXPLICIT_DO_IPUT_QUICK_ALL_TEMPLATE_DECL
#undef EXPLICIT_DO_IPUT_QUICK_TEMPLATE_DECL
+// We execute any instrumentation events that are triggered by this exception and change the
+// shadow_frame's dex_pc to that of the exception handler if there is one in the current method.
+// Return true if we should continue executing in the current method and false if we need to go up
+// the stack to find an exception handler.
// We accept a null Instrumentation* meaning we must not report anything to the instrumentation.
-uint32_t FindNextInstructionFollowingException(
- Thread* self, ShadowFrame& shadow_frame, uint32_t dex_pc,
- const instrumentation::Instrumentation* instrumentation) {
+// TODO We should have a better way to skip instrumentation reporting or possibly rethink that
+// behavior.
+bool MoveToExceptionHandler(Thread* self,
+ ShadowFrame& shadow_frame,
+ const instrumentation::Instrumentation* instrumentation) {
self->VerifyStack();
StackHandleScope<2> hs(self);
Handle<mirror::Throwable> exception(hs.NewHandle(self->GetException()));
- if (instrumentation != nullptr && instrumentation->HasExceptionThrownListeners()
- && self->IsExceptionThrownByCurrentMethod(exception.Get())) {
+ if (instrumentation != nullptr &&
+ instrumentation->HasExceptionThrownListeners() &&
+ self->IsExceptionThrownByCurrentMethod(exception.Get())) {
+ // See b/65049545 for why we don't need to check to see if the exception has changed.
instrumentation->ExceptionThrownEvent(self, exception.Get());
}
bool clear_exception = false;
uint32_t found_dex_pc = shadow_frame.GetMethod()->FindCatchBlock(
- hs.NewHandle(exception->GetClass()), dex_pc, &clear_exception);
- if (found_dex_pc == DexFile::kDexNoIndex && instrumentation != nullptr) {
- if (shadow_frame.NeedsNotifyPop()) {
- instrumentation->WatchedFramePopped(self, shadow_frame);
+ hs.NewHandle(exception->GetClass()), shadow_frame.GetDexPC(), &clear_exception);
+ if (found_dex_pc == DexFile::kDexNoIndex) {
+ if (instrumentation != nullptr) {
+ if (shadow_frame.NeedsNotifyPop()) {
+ instrumentation->WatchedFramePopped(self, shadow_frame);
+ }
+ // Exception is not caught by the current method. We will unwind to the
+ // caller. Notify any instrumentation listener.
+ instrumentation->MethodUnwindEvent(self,
+ shadow_frame.GetThisObject(),
+ shadow_frame.GetMethod(),
+ shadow_frame.GetDexPC());
}
- // Exception is not caught by the current method. We will unwind to the
- // caller. Notify any instrumentation listener.
- instrumentation->MethodUnwindEvent(self, shadow_frame.GetThisObject(),
- shadow_frame.GetMethod(), dex_pc);
+ return false;
} else {
- // Exception is caught in the current method. We will jump to the found_dex_pc.
- if (clear_exception) {
+ shadow_frame.SetDexPC(found_dex_pc);
+ if (instrumentation != nullptr && instrumentation->HasExceptionHandledListeners()) {
+ self->ClearException();
+ instrumentation->ExceptionHandledEvent(self, exception.Get());
+ if (UNLIKELY(self->IsExceptionPending())) {
+ // Exception handled event threw an exception. Try to find the handler for this one.
+ return MoveToExceptionHandler(self, shadow_frame, instrumentation);
+ } else if (!clear_exception) {
+ self->SetException(exception.Get());
+ }
+ } else if (clear_exception) {
self->ClearException();
}
+ return true;
}
- return found_dex_pc;
}
void UnexpectedOpcode(const Instruction* inst, const ShadowFrame& shadow_frame) {
diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index 5b942f2..82e12f5 100644
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -462,9 +462,17 @@
return 3;
}
-uint32_t FindNextInstructionFollowingException(Thread* self, ShadowFrame& shadow_frame,
- uint32_t dex_pc, const instrumentation::Instrumentation* instrumentation)
- REQUIRES_SHARED(Locks::mutator_lock_);
+// We execute any instrumentation events triggered by throwing and/or handing the pending exception
+// and change the shadow_frames dex_pc to the appropriate exception handler if the current method
+// has one. If the exception has been handled and the shadow_frame is now pointing to a catch clause
+// we return true. If the current method is unable to handle the exception we return false.
+// This function accepts a null Instrumentation* as a way to cause instrumentation events not to be
+// reported.
+// TODO We might wish to reconsider how we cause some events to be ignored.
+bool MoveToExceptionHandler(Thread* self,
+ ShadowFrame& shadow_frame,
+ const instrumentation::Instrumentation* instrumentation)
+ REQUIRES_SHARED(Locks::mutator_lock_);
NO_RETURN void UnexpectedOpcode(const Instruction* inst, const ShadowFrame& shadow_frame)
__attribute__((cold))
diff --git a/runtime/interpreter/interpreter_switch_impl.cc b/runtime/interpreter/interpreter_switch_impl.cc
index f352960..69e091b 100644
--- a/runtime/interpreter/interpreter_switch_impl.cc
+++ b/runtime/interpreter/interpreter_switch_impl.cc
@@ -30,10 +30,7 @@
do { \
DCHECK(self->IsExceptionPending()); \
self->AllowThreadSuspension(); \
- uint32_t found_dex_pc = FindNextInstructionFollowingException(self, shadow_frame, \
- inst->GetDexPc(insns), \
- instr); \
- if (found_dex_pc == DexFile::kDexNoIndex) { \
+ if (!MoveToExceptionHandler(self, shadow_frame, instr)) { \
/* Structured locking is to be enforced for abnormal termination, too. */ \
DoMonitorCheckOnExit<do_assignability_check>(self, &shadow_frame); \
if (interpret_one_instruction) { \
@@ -42,7 +39,8 @@
} \
return JValue(); /* Handled in caller. */ \
} else { \
- int32_t displacement = static_cast<int32_t>(found_dex_pc) - static_cast<int32_t>(dex_pc); \
+ int32_t displacement = \
+ static_cast<int32_t>(shadow_frame.GetDexPC()) - static_cast<int32_t>(dex_pc); \
inst = inst->RelativeAt(displacement); \
} \
} while (false)
diff --git a/runtime/interpreter/mterp/mterp.cc b/runtime/interpreter/mterp/mterp.cc
index 88254a8..b8a7a2a 100644
--- a/runtime/interpreter/mterp/mterp.cc
+++ b/runtime/interpreter/mterp/mterp.cc
@@ -490,15 +490,7 @@
DCHECK(self->IsExceptionPending());
const instrumentation::Instrumentation* const instrumentation =
Runtime::Current()->GetInstrumentation();
- uint32_t found_dex_pc = FindNextInstructionFollowingException(self, *shadow_frame,
- shadow_frame->GetDexPC(),
- instrumentation);
- if (found_dex_pc == DexFile::kDexNoIndex) {
- return false;
- }
- // OK - we can deal with it. Update and continue.
- shadow_frame->SetDexPC(found_dex_pc);
- return true;
+ return MoveToExceptionHandler(self, *shadow_frame, instrumentation);
}
extern "C" void MterpCheckBefore(Thread* self, ShadowFrame* shadow_frame, uint16_t* dex_pc_ptr)