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/debugger.cc b/runtime/debugger.cc
index fe0bad2..af56810 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -262,6 +262,13 @@
<< " " << dex_pc;
}
+ // TODO Might be worth it to post ExceptionCatch event.
+ void ExceptionHandled(Thread* thread ATTRIBUTE_UNUSED,
+ Handle<mirror::Throwable> throwable ATTRIBUTE_UNUSED) OVERRIDE {
+ LOG(ERROR) << "Unexpected exception handled event in debugger";
+ }
+
+
private:
static bool IsReturn(ArtMethod* method, uint32_t dex_pc)
REQUIRES_SHARED(Locks::mutator_lock_) {
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 05384b4..6e457a4 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -108,6 +108,7 @@
have_watched_frame_pop_listeners_(false),
have_branch_listeners_(false),
have_invoke_virtual_or_interface_listeners_(false),
+ have_exception_handled_listeners_(false),
deoptimized_methods_lock_("deoptimized methods lock", kDeoptimizedMethodsLock),
deoptimization_enabled_(false),
interpreter_handler_table_(kMainHandlerTable),
@@ -510,6 +511,11 @@
watched_frame_pop_listeners_,
listener,
&have_watched_frame_pop_listeners_);
+ PotentiallyAddListenerTo(kExceptionHandled,
+ events,
+ exception_handled_listeners_,
+ listener,
+ &have_exception_handled_listeners_);
UpdateInterpreterHandlerTable();
}
@@ -592,6 +598,11 @@
watched_frame_pop_listeners_,
listener,
&have_watched_frame_pop_listeners_);
+ PotentiallyRemoveListenerFrom(kExceptionHandled,
+ events,
+ exception_handled_listeners_,
+ listener,
+ &have_exception_handled_listeners_);
UpdateInterpreterHandlerTable();
}
@@ -1114,10 +1125,28 @@
listener->ExceptionThrown(thread, h_exception);
}
}
+ // See b/65049545 for discussion about this behavior.
+ thread->AssertNoPendingException();
thread->SetException(h_exception.Get());
}
}
+void Instrumentation::ExceptionHandledEvent(Thread* thread,
+ mirror::Throwable* exception_object) const {
+ Thread* self = Thread::Current();
+ StackHandleScope<1> hs(self);
+ Handle<mirror::Throwable> h_exception(hs.NewHandle(exception_object));
+ if (HasExceptionHandledListeners()) {
+ // We should have cleared the exception so that callers can detect a new one.
+ DCHECK(thread->GetException() == nullptr);
+ for (InstrumentationListener* listener : exception_handled_listeners_) {
+ if (listener != nullptr) {
+ listener->ExceptionHandled(thread, h_exception);
+ }
+ }
+ }
+}
+
// Computes a frame ID by ignoring inlined frames.
size_t Instrumentation::ComputeFrameId(Thread* self,
size_t frame_depth,
diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
index 114db76..fec027e 100644
--- a/runtime/instrumentation.h
+++ b/runtime/instrumentation.h
@@ -130,6 +130,10 @@
Handle<mirror::Throwable> exception_object)
REQUIRES_SHARED(Locks::mutator_lock_) = 0;
+ // Call-back when an exception is caught/handled by java code.
+ virtual void ExceptionHandled(Thread* thread, Handle<mirror::Throwable> exception_object)
+ REQUIRES_SHARED(Locks::mutator_lock_) = 0;
+
// Call-back for when we execute a branch.
virtual void Branch(Thread* thread,
ArtMethod* method,
@@ -172,6 +176,7 @@
kBranch = 0x80,
kInvokeVirtualOrInterface = 0x100,
kWatchedFramePop = 0x200,
+ kExceptionHandled = 0x400,
};
enum class InstrumentationLevel {
@@ -349,12 +354,16 @@
return have_watched_frame_pop_listeners_;
}
+ bool HasExceptionHandledListeners() const REQUIRES_SHARED(Locks::mutator_lock_) {
+ return have_exception_handled_listeners_;
+ }
+
bool IsActive() const REQUIRES_SHARED(Locks::mutator_lock_) {
return have_dex_pc_listeners_ || have_method_entry_listeners_ || have_method_exit_listeners_ ||
have_field_read_listeners_ || have_field_write_listeners_ ||
have_exception_thrown_listeners_ || have_method_unwind_listeners_ ||
have_branch_listeners_ || have_invoke_virtual_or_interface_listeners_ ||
- have_watched_frame_pop_listeners_;
+ have_watched_frame_pop_listeners_ || have_exception_handled_listeners_;
}
// Any instrumentation *other* than what is needed for Jit profiling active?
@@ -362,7 +371,8 @@
return have_dex_pc_listeners_ || have_method_exit_listeners_ ||
have_field_read_listeners_ || have_field_write_listeners_ ||
have_exception_thrown_listeners_ || have_method_unwind_listeners_ ||
- have_branch_listeners_ || have_watched_frame_pop_listeners_;
+ have_branch_listeners_ || have_watched_frame_pop_listeners_ ||
+ have_exception_handled_listeners_;
}
// Inform listeners that a method has been entered. A dex PC is provided as we may install
@@ -452,6 +462,11 @@
void ExceptionThrownEvent(Thread* thread, mirror::Throwable* exception_object) const
REQUIRES_SHARED(Locks::mutator_lock_);
+ // Inform listeners that an exception has been handled. This is not sent for native code or for
+ // exceptions which reach the end of the thread's stack.
+ void ExceptionHandledEvent(Thread* thread, mirror::Throwable* exception_object) const
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
// Called when an instrumented method is entered. The intended link register (lr) is saved so
// that returning causes a branch to the method exit stub. Generates method enter events.
void PushInstrumentationStackFrame(Thread* self, mirror::Object* this_object,
@@ -637,6 +652,10 @@
// Do we have any invoke listeners? Short-cut to avoid taking the instrumentation_lock_.
bool have_invoke_virtual_or_interface_listeners_ GUARDED_BY(Locks::mutator_lock_);
+ // Do we have any exception handled listeners? Short-cut to avoid taking the
+ // instrumentation_lock_.
+ bool have_exception_handled_listeners_ GUARDED_BY(Locks::mutator_lock_);
+
// Contains the instrumentation level required by each client of the instrumentation identified
// by a string key.
typedef SafeMap<const char*, InstrumentationLevel> InstrumentationLevelTable;
@@ -663,6 +682,7 @@
std::list<InstrumentationListener*> field_write_listeners_ GUARDED_BY(Locks::mutator_lock_);
std::list<InstrumentationListener*> exception_thrown_listeners_ GUARDED_BY(Locks::mutator_lock_);
std::list<InstrumentationListener*> watched_frame_pop_listeners_ GUARDED_BY(Locks::mutator_lock_);
+ std::list<InstrumentationListener*> exception_handled_listeners_ GUARDED_BY(Locks::mutator_lock_);
// The set of methods being deoptimized (by the debugger) which must be executed with interpreter
// only.
diff --git a/runtime/instrumentation_test.cc b/runtime/instrumentation_test.cc
index 7390f4f..9b77d12 100644
--- a/runtime/instrumentation_test.cc
+++ b/runtime/instrumentation_test.cc
@@ -48,6 +48,7 @@
received_field_written_event(false),
received_field_written_object_event(false),
received_exception_thrown_event(false),
+ received_exception_handled_event(false),
received_branch_event(false),
received_invoke_virtual_or_interface_event(false),
received_watched_frame_pop(false) {}
@@ -131,6 +132,12 @@
received_exception_thrown_event = true;
}
+ void ExceptionHandled(Thread* self ATTRIBUTE_UNUSED,
+ Handle<mirror::Throwable> throwable ATTRIBUTE_UNUSED)
+ OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_) {
+ received_exception_handled_event = true;
+ }
+
void Branch(Thread* thread ATTRIBUTE_UNUSED,
ArtMethod* method ATTRIBUTE_UNUSED,
uint32_t dex_pc ATTRIBUTE_UNUSED,
@@ -163,6 +170,7 @@
received_field_written_event = false;
received_field_written_object_event = false;
received_exception_thrown_event = false;
+ received_exception_handled_event = false;
received_branch_event = false;
received_invoke_virtual_or_interface_event = false;
received_watched_frame_pop = false;
@@ -177,6 +185,7 @@
bool received_field_written_event;
bool received_field_written_object_event;
bool received_exception_thrown_event;
+ bool received_exception_handled_event;
bool received_branch_event;
bool received_invoke_virtual_or_interface_event;
bool received_watched_frame_pop;
@@ -369,6 +378,8 @@
return instr->HasFieldWriteListeners();
case instrumentation::Instrumentation::kExceptionThrown:
return instr->HasExceptionThrownListeners();
+ case instrumentation::Instrumentation::kExceptionHandled:
+ return instr->HasExceptionHandledListeners();
case instrumentation::Instrumentation::kBranch:
return instr->HasBranchListeners();
case instrumentation::Instrumentation::kInvokeVirtualOrInterface:
@@ -429,6 +440,13 @@
case instrumentation::Instrumentation::kWatchedFramePop:
instr->WatchedFramePopped(self, frame);
break;
+ case instrumentation::Instrumentation::kExceptionHandled: {
+ ThrowArithmeticExceptionDivideByZero();
+ mirror::Throwable* event_exception = self->GetException();
+ self->ClearException();
+ instr->ExceptionHandledEvent(self, event_exception);
+ break;
+ }
default:
LOG(FATAL) << "Unknown instrumentation event " << event_type;
UNREACHABLE();
@@ -455,6 +473,8 @@
(with_object && listener.received_field_written_object_event);
case instrumentation::Instrumentation::kExceptionThrown:
return listener.received_exception_thrown_event;
+ case instrumentation::Instrumentation::kExceptionHandled:
+ return listener.received_exception_handled_event;
case instrumentation::Instrumentation::kBranch:
return listener.received_branch_event;
case instrumentation::Instrumentation::kInvokeVirtualOrInterface:
@@ -484,6 +504,7 @@
// Check there is no registered listener.
EXPECT_FALSE(instr->HasDexPcListeners());
EXPECT_FALSE(instr->HasExceptionThrownListeners());
+ EXPECT_FALSE(instr->HasExceptionHandledListeners());
EXPECT_FALSE(instr->HasFieldReadListeners());
EXPECT_FALSE(instr->HasFieldWriteListeners());
EXPECT_FALSE(instr->HasMethodEntryListeners());
@@ -587,6 +608,10 @@
/*with_object*/ false);
}
+TEST_F(InstrumentationTest, ExceptionHandledEvent) {
+ TestEvent(instrumentation::Instrumentation::kExceptionHandled);
+}
+
TEST_F(InstrumentationTest, ExceptionThrownEvent) {
TestEvent(instrumentation::Instrumentation::kExceptionThrown);
}
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)
diff --git a/runtime/trace.cc b/runtime/trace.cc
index d7673f3..b30de79 100644
--- a/runtime/trace.cc
+++ b/runtime/trace.cc
@@ -811,6 +811,12 @@
LOG(ERROR) << "Unexpected exception thrown event in tracing";
}
+void Trace::ExceptionHandled(Thread* thread ATTRIBUTE_UNUSED,
+ Handle<mirror::Throwable> exception_object ATTRIBUTE_UNUSED)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ LOG(ERROR) << "Unexpected exception thrown event in tracing";
+}
+
void Trace::Branch(Thread* /*thread*/, ArtMethod* method,
uint32_t /*dex_pc*/, int32_t /*dex_pc_offset*/)
REQUIRES_SHARED(Locks::mutator_lock_) {
diff --git a/runtime/trace.h b/runtime/trace.h
index 8b0931d..49d5b22 100644
--- a/runtime/trace.h
+++ b/runtime/trace.h
@@ -181,6 +181,8 @@
void ExceptionThrown(Thread* thread,
Handle<mirror::Throwable> exception_object)
REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!*unique_methods_lock_) OVERRIDE;
+ void ExceptionHandled(Thread* thread, Handle<mirror::Throwable> exception_object)
+ REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!*unique_methods_lock_) OVERRIDE;
void Branch(Thread* thread,
ArtMethod* method,
uint32_t dex_pc,