From 822266b9dc7d8dc9e084192ae0f4bc95af4e8cf8 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Thu, 29 May 2014 16:55:06 -0700 Subject: Ignore catch blocks whose type can't be resolved. Reverts change 72b3e430d880ef57eaa6a34a0822165994052202 but keeps unit test and missing delete that would fail assertions on long jump context recycling. Change-Id: I926755e8b831b208aa7e1ce46421bef3793a1441 --- runtime/catch_block_stack_visitor.cc | 10 +--------- runtime/interpreter/interpreter_common.h | 8 +------- runtime/mirror/art_method.cc | 19 +++++++++---------- runtime/mirror/art_method.h | 5 +---- test/111-unresolvable-exception/expected.txt | 2 +- test/111-unresolvable-exception/src/Main.java | 2 +- 6 files changed, 14 insertions(+), 32 deletions(-) diff --git a/runtime/catch_block_stack_visitor.cc b/runtime/catch_block_stack_visitor.cc index 55b330a..b820276 100644 --- a/runtime/catch_block_stack_visitor.cc +++ b/runtime/catch_block_stack_visitor.cc @@ -50,17 +50,9 @@ bool CatchBlockStackVisitor::HandleTryItems(mirror::ArtMethod* method) { } if (dex_pc != DexFile::kDexNoIndex) { bool clear_exception = false; - bool exc_changed = false; StackHandleScope<1> hs(Thread::Current()); Handle to_find(hs.NewHandle((*exception_)->GetClass())); - uint32_t found_dex_pc = method->FindCatchBlock(to_find, dex_pc, &clear_exception, - &exc_changed); - if (UNLIKELY(exc_changed)) { - DCHECK_EQ(DexFile::kDexNoIndex, found_dex_pc); - exception_->Assign(self_->GetException(nullptr)); // TODO: Throw location? - // There is a new context installed, delete it. - delete self_->GetLongJumpContext(); - } + uint32_t found_dex_pc = method->FindCatchBlock(to_find, dex_pc, &clear_exception); exception_handler_->SetClearException(clear_exception); if (found_dex_pc != DexFile::kDexNoIndex) { exception_handler_->SetHandlerDexPc(found_dex_pc); diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h index cfc90a6..f69fecc 100644 --- a/runtime/interpreter/interpreter_common.h +++ b/runtime/interpreter/interpreter_common.h @@ -587,16 +587,10 @@ static inline uint32_t FindNextInstructionFollowingException(Thread* self, ThrowLocation throw_location; mirror::Throwable* exception = self->GetException(&throw_location); bool clear_exception = false; - bool new_exception = false; StackHandleScope<3> hs(self); Handle exception_class(hs.NewHandle(exception->GetClass())); uint32_t found_dex_pc = shadow_frame.GetMethod()->FindCatchBlock(exception_class, dex_pc, - &clear_exception, - &new_exception); - if (UNLIKELY(new_exception)) { - // Update the exception. - exception = self->GetException(&throw_location); - } + &clear_exception); if (found_dex_pc == DexFile::kDexNoIndex) { instrumentation->MethodUnwindEvent(self, this_object, shadow_frame.GetMethod(), dex_pc); diff --git a/runtime/mirror/art_method.cc b/runtime/mirror/art_method.cc index e2d3f41..c01fc72 100644 --- a/runtime/mirror/art_method.cc +++ b/runtime/mirror/art_method.cc @@ -230,7 +230,7 @@ uintptr_t ArtMethod::ToNativePc(const uint32_t dex_pc) { } uint32_t ArtMethod::FindCatchBlock(Handle exception_type, uint32_t dex_pc, - bool* has_no_move_exception, bool* exc_changed) { + bool* has_no_move_exception) { MethodHelper mh(this); const DexFile::CodeItem* code_item = mh.GetCodeItem(); // Set aside the exception while we resolve its type. @@ -251,17 +251,16 @@ uint32_t ArtMethod::FindCatchBlock(Handle exception_type, uint32_t dex_pc } // Does this catch exception type apply? Class* iter_exception_type = mh.GetClassFromTypeIdx(iter_type_idx); - if (iter_exception_type == nullptr) { - // Now have a NoClassDefFoundError as exception. + if (UNLIKELY(iter_exception_type == nullptr)) { + // Now have a NoClassDefFoundError as exception. Ignore in case the exception class was + // removed by a pro-guard like tool. // Note: this is not RI behavior. RI would have failed when loading the class. - *exc_changed = true; - - // TODO: Add old exception as suppressed. + self->ClearException(); + // Delete any long jump context as this routine is called during a stack walk which will + // release its in use context at the end. + delete self->GetLongJumpContext(); LOG(WARNING) << "Unresolved exception class when finding catch block: " - << mh.GetTypeDescriptorFromTypeIdx(iter_type_idx); - - // Return immediately. - return DexFile::kDexNoIndex; + << DescriptorToDot(mh.GetTypeDescriptorFromTypeIdx(iter_type_idx)); } else if (iter_exception_type->IsAssignableFrom(exception_type.Get())) { found_dex_pc = it.GetHandlerAddress(); break; diff --git a/runtime/mirror/art_method.h b/runtime/mirror/art_method.h index 2e8253f..f901512 100644 --- a/runtime/mirror/art_method.h +++ b/runtime/mirror/art_method.h @@ -398,11 +398,8 @@ class MANAGED ArtMethod : public Object { // Find the catch block for the given exception type and dex_pc. When a catch block is found, // indicates whether the found catch block is responsible for clearing the exception or whether // a move-exception instruction is present. - // In the process of finding a catch block we might trigger resolution errors. This is flagged - // by exc_changed, which indicates that a different exception is now stored in the thread and - // should be reloaded. uint32_t FindCatchBlock(Handle exception_type, uint32_t dex_pc, - bool* has_no_move_exception, bool* exc_changed) + bool* has_no_move_exception) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); static void SetClass(Class* java_lang_reflect_ArtMethod); diff --git a/test/111-unresolvable-exception/expected.txt b/test/111-unresolvable-exception/expected.txt index 052dd74..f8a1e96 100644 --- a/test/111-unresolvable-exception/expected.txt +++ b/test/111-unresolvable-exception/expected.txt @@ -1 +1 @@ -Caught class java.lang.NoClassDefFoundError +Got expected exception. diff --git a/test/111-unresolvable-exception/src/Main.java b/test/111-unresolvable-exception/src/Main.java index ba07ee1..adeb0a2 100644 --- a/test/111-unresolvable-exception/src/Main.java +++ b/test/111-unresolvable-exception/src/Main.java @@ -32,7 +32,7 @@ public class Main { throw new RuntimeException(); // Trigger exception handling. } catch (TestException e) { // This handler will have an unresolvable class. } catch (Exception e) { // General-purpose handler - System.out.println("Should not get here!"); + System.out.println("Got expected exception."); } } -- cgit v1.1