From 7642cfc90fc9c3ebfd8e3b5041915705c93b5cf0 Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Thu, 26 Feb 2015 10:56:09 +0000 Subject: [PATCH] Change how we report exceptions to the debugger. This is only a refactoring/cleanup. Bug fixes with respect to catch location, and more cleanups will follow. Change-Id: I30d3c6260b0c8f8115a811621397225b88f2063a --- runtime/debugger.cc | 3 --- runtime/entrypoints/entrypoint_utils-inl.h | 2 -- runtime/entrypoints_order_test.cc | 5 +---- runtime/instrumentation.cc | 2 -- runtime/interpreter/interpreter_common.cc | 11 +++++------ runtime/jni_internal.cc | 3 --- runtime/mirror/art_method.cc | 2 -- runtime/mirror/class.cc | 2 -- runtime/mirror/throwable.cc | 7 +++++++ runtime/mirror/throwable.h | 2 ++ runtime/oat.h | 2 +- runtime/quick_exception_handler.cc | 18 ++++-------------- runtime/quick_exception_handler.h | 3 +-- runtime/thread.cc | 13 +++++++------ runtime/thread.h | 21 ++++++--------------- 15 files changed, 34 insertions(+), 62 deletions(-) diff --git a/runtime/debugger.cc b/runtime/debugger.cc index c0dd19727..246125bd5 100644 --- a/runtime/debugger.cc +++ b/runtime/debugger.cc @@ -3709,7 +3709,6 @@ void Dbg::ExecuteMethod(DebugInvokeReq* pReq) { auto old_throw_method = hs.NewHandle(nullptr); auto old_exception = hs.NewHandle(nullptr); uint32_t old_throw_dex_pc; - bool old_exception_report_flag; { ThrowLocation old_throw_location; mirror::Throwable* old_exception_obj = soa.Self()->GetException(&old_throw_location); @@ -3717,7 +3716,6 @@ void Dbg::ExecuteMethod(DebugInvokeReq* pReq) { old_throw_method.Assign(old_throw_location.GetMethod()); old_exception.Assign(old_exception_obj); old_throw_dex_pc = old_throw_location.GetDexPc(); - old_exception_report_flag = soa.Self()->IsExceptionReportedToInstrumentation(); soa.Self()->ClearException(); } @@ -3772,7 +3770,6 @@ void Dbg::ExecuteMethod(DebugInvokeReq* pReq) { ThrowLocation gc_safe_throw_location(old_throw_this_object.Get(), old_throw_method.Get(), old_throw_dex_pc); soa.Self()->SetException(gc_safe_throw_location, old_exception.Get()); - soa.Self()->SetExceptionReportedToInstrumentation(old_exception_report_flag); } } diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h index 35579d66d..9d84e4ac4 100644 --- a/runtime/entrypoints/entrypoint_utils-inl.h +++ b/runtime/entrypoints/entrypoint_utils-inl.h @@ -614,7 +614,6 @@ inline void UnlockJniSynchronizedMethod(jobject locked, Thread* self) { // Save any pending exception over monitor exit call. mirror::Throwable* saved_exception = NULL; ThrowLocation saved_throw_location; - bool is_exception_reported = self->IsExceptionReportedToInstrumentation(); if (UNLIKELY(self->IsExceptionPending())) { saved_exception = self->GetException(&saved_throw_location); self->ClearException(); @@ -630,7 +629,6 @@ inline void UnlockJniSynchronizedMethod(jobject locked, Thread* self) { // Restore pending exception. if (saved_exception != NULL) { self->SetException(saved_throw_location, saved_exception); - self->SetExceptionReportedToInstrumentation(is_exception_reported); } } diff --git a/runtime/entrypoints_order_test.cc b/runtime/entrypoints_order_test.cc index daa24c93f..917335713 100644 --- a/runtime/entrypoints_order_test.cc +++ b/runtime/entrypoints_order_test.cc @@ -71,10 +71,7 @@ class EntrypointsOrderTest : public CommonRuntimeTest { EXPECT_OFFSET_DIFFP(Thread, tls32_, daemon, throwing_OutOfMemoryError, 4); EXPECT_OFFSET_DIFFP(Thread, tls32_, throwing_OutOfMemoryError, no_thread_suspension, 4); EXPECT_OFFSET_DIFFP(Thread, tls32_, no_thread_suspension, thread_exit_check_count, 4); - EXPECT_OFFSET_DIFFP(Thread, tls32_, thread_exit_check_count, - is_exception_reported_to_instrumentation_, 4); - EXPECT_OFFSET_DIFFP(Thread, tls32_, is_exception_reported_to_instrumentation_, - handling_signal_, 4); + EXPECT_OFFSET_DIFFP(Thread, tls32_, thread_exit_check_count, handling_signal_, 4); // TODO: Better connection. Take alignment into account. EXPECT_OFFSET_DIFF_GT3(Thread, tls32_.thread_exit_check_count, tls64_.trace_clock_base, 4, diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc index a054462b2..c94dab988 100644 --- a/runtime/instrumentation.cc +++ b/runtime/instrumentation.cc @@ -955,7 +955,6 @@ void Instrumentation::ExceptionCaughtEvent(Thread* thread, const ThrowLocation& mirror::Throwable* exception_object) const { if (HasExceptionCaughtListeners()) { DCHECK_EQ(thread->GetException(nullptr), exception_object); - bool is_exception_reported = thread->IsExceptionReportedToInstrumentation(); thread->ClearException(); std::shared_ptr> original(exception_caught_listeners_); for (InstrumentationListener* listener : *original.get()) { @@ -963,7 +962,6 @@ void Instrumentation::ExceptionCaughtEvent(Thread* thread, const ThrowLocation& exception_object); } thread->SetException(throw_location, exception_object); - thread->SetExceptionReportedToInstrumentation(is_exception_reported); } } diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc index 3ab7f3036..754602f94 100644 --- a/runtime/interpreter/interpreter_common.cc +++ b/runtime/interpreter/interpreter_common.cc @@ -491,12 +491,12 @@ uint32_t FindNextInstructionFollowingException(Thread* self, ThrowLocation throw_location; StackHandleScope<3> hs(self); Handle exception(hs.NewHandle(self->GetException(&throw_location))); - if (!self->IsExceptionReportedToInstrumentation() && instrumentation->HasExceptionCaughtListeners()) { + if (instrumentation->HasExceptionCaughtListeners() + && self->IsExceptionThrownByCurrentMethod(exception.Get())) { CatchLocationFinder clf(self, &exception); clf.WalkStack(false); instrumentation->ExceptionCaughtEvent(self, throw_location, clf.GetCatchMethod(), clf.GetCatchDexPc(), exception.Get()); - self->SetExceptionReportedToInstrumentation(true); } bool clear_exception = false; uint32_t found_dex_pc; @@ -507,13 +507,12 @@ uint32_t FindNextInstructionFollowingException(Thread* self, &clear_exception); } if (found_dex_pc == DexFile::kDexNoIndex) { + // 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); } else { - if (self->IsExceptionReportedToInstrumentation()) { - instrumentation->MethodUnwindEvent(self, shadow_frame.GetThisObject(), - shadow_frame.GetMethod(), dex_pc); - } + // Exception is caught in the current method. We will jump to the found_dex_pc. if (clear_exception) { self->ClearException(); } diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc index 37ad46e20..561302e33 100644 --- a/runtime/jni_internal.cc +++ b/runtime/jni_internal.cc @@ -466,7 +466,6 @@ class JNI { auto old_throw_method(hs.NewHandle(nullptr)); auto old_exception(hs.NewHandle(nullptr)); uint32_t old_throw_dex_pc; - bool old_is_exception_reported; { ThrowLocation old_throw_location; mirror::Throwable* old_exception_obj = soa.Self()->GetException(&old_throw_location); @@ -474,7 +473,6 @@ class JNI { old_throw_method.Assign(old_throw_location.GetMethod()); old_exception.Assign(old_exception_obj); old_throw_dex_pc = old_throw_location.GetDexPc(); - old_is_exception_reported = soa.Self()->IsExceptionReportedToInstrumentation(); soa.Self()->ClearException(); } ScopedLocalRef exception(env, @@ -496,7 +494,6 @@ class JNI { old_throw_dex_pc); soa.Self()->SetException(gc_safe_throw_location, old_exception.Get()); - soa.Self()->SetExceptionReportedToInstrumentation(old_is_exception_reported); } static jthrowable ExceptionOccurred(JNIEnv* env) { diff --git a/runtime/mirror/art_method.cc b/runtime/mirror/art_method.cc index 26f6f345d..85fc5f3c9 100644 --- a/runtime/mirror/art_method.cc +++ b/runtime/mirror/art_method.cc @@ -274,7 +274,6 @@ uint32_t ArtMethod::FindCatchBlock(Handle h_this, Handle excep ThrowLocation throw_location; StackHandleScope<1> hs(self); Handle exception(hs.NewHandle(self->GetException(&throw_location))); - bool is_exception_reported = self->IsExceptionReportedToInstrumentation(); self->ClearException(); // Default to handler not found. uint32_t found_dex_pc = DexFile::kDexNoIndex; @@ -311,7 +310,6 @@ uint32_t ArtMethod::FindCatchBlock(Handle h_this, Handle excep // Put the exception back. if (exception.Get() != nullptr) { self->SetException(throw_location, exception.Get()); - self->SetExceptionReportedToInstrumentation(is_exception_reported); } return found_dex_pc; } diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc index ae684b162..96b15dd67 100644 --- a/runtime/mirror/class.cc +++ b/runtime/mirror/class.cc @@ -84,7 +84,6 @@ void Class::SetStatus(Status new_status, Thread* self) { Handle old_throw_this_object(hs.NewHandle(old_throw_location.GetThis())); Handle old_throw_method(hs.NewHandle(old_throw_location.GetMethod())); uint32_t old_throw_dex_pc = old_throw_location.GetDexPc(); - bool is_exception_reported = self->IsExceptionReportedToInstrumentation(); Class* eiie_class; // Do't attempt to use FindClass if we have an OOM error since this can try to do more // allocations and may cause infinite loops. @@ -113,7 +112,6 @@ void Class::SetStatus(Status new_status, Thread* self) { ThrowLocation gc_safe_throw_location(old_throw_this_object.Get(), old_throw_method.Get(), old_throw_dex_pc); self->SetException(gc_safe_throw_location, old_exception.Get()); - self->SetExceptionReportedToInstrumentation(is_exception_reported); } static_assert(sizeof(Status) == sizeof(uint32_t), "Size of status not equal to uint32"); if (Runtime::Current()->IsActiveTransaction()) { diff --git a/runtime/mirror/throwable.cc b/runtime/mirror/throwable.cc index 61d85e2fe..fdfeb47da 100644 --- a/runtime/mirror/throwable.cc +++ b/runtime/mirror/throwable.cc @@ -69,6 +69,13 @@ bool Throwable::IsCheckedException() { return !InstanceOf(WellKnownClasses::ToClass(WellKnownClasses::java_lang_RuntimeException)); } +int32_t Throwable::GetStackDepth() { + Object* stack_state = GetStackState(); + if (stack_state == nullptr || !stack_state->IsObjectArray()) return -1; + ObjectArray* method_trace = down_cast*>(stack_state); + return method_trace->GetLength() - 1; +} + std::string Throwable::Dump() { std::string result(PrettyTypeOf(this)); result += ": "; diff --git a/runtime/mirror/throwable.h b/runtime/mirror/throwable.h index f90812d2e..c22475b4c 100644 --- a/runtime/mirror/throwable.h +++ b/runtime/mirror/throwable.h @@ -51,6 +51,8 @@ class MANAGED Throwable : public Object { return java_lang_Throwable_.Read(); } + int32_t GetStackDepth() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + static void SetClass(Class* java_lang_Throwable); static void ResetClass(); static void VisitRoots(RootCallback* callback, void* arg) diff --git a/runtime/oat.h b/runtime/oat.h index 7faf33bc8..f973b2897 100644 --- a/runtime/oat.h +++ b/runtime/oat.h @@ -32,7 +32,7 @@ class InstructionSetFeatures; class PACKED(4) OatHeader { public: static constexpr uint8_t kOatMagic[] = { 'o', 'a', 't', '\n' }; - static constexpr uint8_t kOatVersion[] = { '0', '5', '5', '\0' }; + static constexpr uint8_t kOatVersion[] = { '0', '5', '6', '\0' }; static constexpr const char* kImageLocationKey = "image-location"; static constexpr const char* kDex2OatCmdLineKey = "dex2oat-cmdline"; diff --git a/runtime/quick_exception_handler.cc b/runtime/quick_exception_handler.cc index 7bdf65235..1ddb76114 100644 --- a/runtime/quick_exception_handler.cc +++ b/runtime/quick_exception_handler.cc @@ -115,8 +115,7 @@ class CatchBlockStackVisitor FINAL : public StackVisitor { }; void QuickExceptionHandler::FindCatch(const ThrowLocation& throw_location, - mirror::Throwable* exception, - bool is_exception_reported) { + mirror::Throwable* exception) { DCHECK(!is_deoptimization_); if (kDebugExceptionDelivery) { mirror::String* msg = exception->GetDetailMessage(); @@ -147,23 +146,14 @@ void QuickExceptionHandler::FindCatch(const ThrowLocation& throw_location, } else { // Put exception back in root set with clear throw location. self_->SetException(ThrowLocation(), exception_ref.Get()); - self_->SetExceptionReportedToInstrumentation(is_exception_reported); } // The debugger may suspend this thread and walk its stack. Let's do this before popping // instrumentation frames. - if (!is_exception_reported) { - instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation(); + instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation(); + if (instrumentation->HasExceptionCaughtListeners() + && self_->IsExceptionThrownByCurrentMethod(exception)) { instrumentation->ExceptionCaughtEvent(self_, throw_location, handler_method_, handler_dex_pc_, exception_ref.Get()); - // We're not catching this exception but let's remind we already reported the exception above - // to avoid reporting it twice. - self_->SetExceptionReportedToInstrumentation(true); - } - bool caught_exception = (handler_method_ != nullptr && handler_dex_pc_ != DexFile::kDexNoIndex); - if (caught_exception) { - // We're catching this exception so we finish reporting it. We do it here to avoid doing it - // in the compiled code. - self_->SetExceptionReportedToInstrumentation(false); } } diff --git a/runtime/quick_exception_handler.h b/runtime/quick_exception_handler.h index 162b1dc74..a0e6a7929 100644 --- a/runtime/quick_exception_handler.h +++ b/runtime/quick_exception_handler.h @@ -44,8 +44,7 @@ class QuickExceptionHandler { UNREACHABLE(); } - void FindCatch(const ThrowLocation& throw_location, mirror::Throwable* exception, - bool is_exception_reported) + void FindCatch(const ThrowLocation& throw_location, mirror::Throwable* exception) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); void DeoptimizeStack() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); void UpdateInstrumentationStack() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); diff --git a/runtime/thread.cc b/runtime/thread.cc index 79d0066ee..fdb1f9dd4 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -1626,6 +1626,12 @@ template jobject Thread::CreateInternalStackTrace( template jobject Thread::CreateInternalStackTrace( const ScopedObjectAccessAlreadyRunnable& soa) const; +bool Thread::IsExceptionThrownByCurrentMethod(mirror::Throwable* exception) const { + CountStackDepthVisitor count_visitor(const_cast(this)); + count_visitor.WalkStack(); + return count_visitor.GetDepth() == exception->GetStackDepth(); +} + jobjectArray Thread::InternalStackTraceToStackTraceElementArray( const ScopedObjectAccessAlreadyRunnable& soa, jobject internal, jobjectArray output_array, int* stack_depth) { @@ -1745,7 +1751,6 @@ void Thread::ThrowNewWrappedException(const ThrowLocation& throw_location, Handle saved_throw_method(hs.NewHandle(throw_location.GetMethod())); // Ignore the cause throw location. TODO: should we report this as a re-throw? ScopedLocalRef cause(GetJniEnv(), soa.AddLocalReference(GetException(nullptr))); - bool is_exception_reported = IsExceptionReportedToInstrumentation(); ClearException(); Runtime* runtime = Runtime::Current(); @@ -1777,7 +1782,6 @@ void Thread::ThrowNewWrappedException(const ThrowLocation& throw_location, ThrowLocation gc_safe_throw_location(saved_throw_this.Get(), saved_throw_method.Get(), throw_location.GetDexPc()); SetException(gc_safe_throw_location, Runtime::Current()->GetPreAllocatedOutOfMemoryError()); - SetExceptionReportedToInstrumentation(is_exception_reported); return; } @@ -1830,7 +1834,6 @@ void Thread::ThrowNewWrappedException(const ThrowLocation& throw_location, ThrowLocation gc_safe_throw_location(saved_throw_this.Get(), saved_throw_method.Get(), throw_location.GetDexPc()); SetException(gc_safe_throw_location, exception.Get()); - SetExceptionReportedToInstrumentation(is_exception_reported); } else { jvalue jv_args[2]; size_t i = 0; @@ -1848,7 +1851,6 @@ void Thread::ThrowNewWrappedException(const ThrowLocation& throw_location, ThrowLocation gc_safe_throw_location(saved_throw_this.Get(), saved_throw_method.Get(), throw_location.GetDexPc()); SetException(gc_safe_throw_location, exception.Get()); - SetExceptionReportedToInstrumentation(is_exception_reported); } } } @@ -2033,14 +2035,13 @@ void Thread::QuickDeliverException() { CHECK(exception != nullptr); // Don't leave exception visible while we try to find the handler, which may cause class // resolution. - bool is_exception_reported = IsExceptionReportedToInstrumentation(); ClearException(); bool is_deoptimization = (exception == GetDeoptimizationException()); QuickExceptionHandler exception_handler(this, is_deoptimization); if (is_deoptimization) { exception_handler.DeoptimizeStack(); } else { - exception_handler.FindCatch(throw_location, exception, is_exception_reported); + exception_handler.FindCatch(throw_location, exception); } exception_handler.UpdateInstrumentationStack(); exception_handler.DoLongJump(); diff --git a/runtime/thread.h b/runtime/thread.h index 83cedbb7f..e4c91b72b 100644 --- a/runtime/thread.h +++ b/runtime/thread.h @@ -349,7 +349,6 @@ class Thread { void ClearException() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { tlsPtr_.exception = nullptr; tlsPtr_.throw_location.Clear(); - SetExceptionReportedToInstrumentation(false); } // Find catch block and perform long jump to appropriate exception handle @@ -366,6 +365,11 @@ class Thread { mirror::ArtMethod* GetCurrentMethod(uint32_t* dex_pc, bool abort_on_error = true) const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + // Returns whether the given exception was thrown by the current Java method being executed + // (Note that this includes native Java methods). + bool IsExceptionThrownByCurrentMethod(mirror::Throwable* exception) const + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + ThrowLocation GetCurrentLocationForThrow() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); void SetTopOfStack(StackReference* top_method) { @@ -842,14 +846,6 @@ class Thread { tlsPtr_.rosalloc_runs[index] = run; } - bool IsExceptionReportedToInstrumentation() const { - return tls32_.is_exception_reported_to_instrumentation_; - } - - void SetExceptionReportedToInstrumentation(bool reported) { - tls32_.is_exception_reported_to_instrumentation_ = reported; - } - void ProtectStack(); bool UnprotectStack(); @@ -976,8 +972,7 @@ class Thread { explicit tls_32bit_sized_values(bool is_daemon) : suspend_count(0), debug_suspend_count(0), thin_lock_thread_id(0), tid(0), daemon(is_daemon), throwing_OutOfMemoryError(false), no_thread_suspension(0), - thread_exit_check_count(0), is_exception_reported_to_instrumentation_(false), - handling_signal_(false), suspended_at_suspend_check(false) { + thread_exit_check_count(0), handling_signal_(false), suspended_at_suspend_check(false) { } union StateAndFlags state_and_flags; @@ -1014,10 +1009,6 @@ class Thread { // How many times has our pthread key's destructor been called? uint32_t thread_exit_check_count; - // When true this field indicates that the exception associated with this thread has already - // been reported to instrumentation. - bool32_t is_exception_reported_to_instrumentation_; - // True if signal is being handled by this thread. bool32_t handling_signal_; -- 2.11.0