diff options
| -rw-r--r-- | src/class_linker.cc | 34 | ||||
| -rw-r--r-- | src/dex_verifier.cc | 17 | ||||
| -rw-r--r-- | src/dex_verifier.h | 5 |
3 files changed, 37 insertions, 19 deletions
diff --git a/src/class_linker.cc b/src/class_linker.cc index 51e5f3c2ec..f343ed47a6 100644 --- a/src/class_linker.cc +++ b/src/class_linker.cc @@ -1924,6 +1924,7 @@ void ClassLinker::VerifyClass(Class* klass) { const DexFile& dex_file = FindDexFile(klass->GetDexCache()); if (VerifyClassUsingOatFile(dex_file, klass) || verifier::DexVerifier::VerifyClass(klass, error_msg)) { + DCHECK(!Thread::Current()->IsExceptionPending()); // Make sure all classes referenced by catch blocks are resolved ResolveClassExceptionHandlerTypes(dex_file, klass); klass->SetStatus(Class::kStatusVerified); @@ -1934,7 +1935,7 @@ void ClassLinker::VerifyClass(Class* klass) { << " in " << klass->GetDexCache()->GetLocation()->ToModifiedUtf8() << " because: " << error_msg; Thread* self = Thread::Current(); - CHECK(!self->IsExceptionPending()) << self->GetException()->Dump(); + CHECK(!self->IsExceptionPending()); self->ThrowNewException("Ljava/lang/VerifyError;", error_msg.c_str()); CHECK_EQ(klass->GetStatus(), Class::kStatusVerifying) << PrettyDescriptor(klass); klass->SetStatus(Class::kStatusError); @@ -1961,17 +1962,34 @@ bool ClassLinker::VerifyClassUsingOatFile(const DexFile& dex_file, Class* klass) UniquePtr<const OatFile::OatClass> oat_class(oat_dex_file->GetOatClass(class_def_index)); CHECK(oat_class.get() != NULL) << descriptor; Class::Status status = oat_class->GetStatus(); - if (status == Class::kStatusError) { - Thread* self = Thread::Current(); - self->ThrowNewExceptionF("Ljava/lang/VerifyError;", "Compile-time verification of %s failed", - PrettyDescriptor(klass).c_str()); - klass->SetStatus(Class::kStatusError); - return true; - } if (status == Class::kStatusVerified || status == Class::kStatusInitialized) { return true; } + if (status == Class::kStatusError) { + // Compile time verification failed. Compile time verification can fail because we have + // incomplete type information. Consider the following: + // class ... { + // Foo x; + // .... () { + // if (...) { + // v1 gets assigned a type of resolved class Foo + // } else { + // v1 gets assigned a type of unresolved class Bar + // } + // iput x = v1 + // } } + // when we merge v1 following the if-the-else it results in Conflict + // (see verifier::RegType::Merge) as we can't know the type of Bar and we could possibly be + // allowing an unsafe assignment to the field x in the iput (javac may have compiled this as + // it knew Bar was a sub-class of Foo, but for us this may have been moved into a separate apk + // at compile time). + return false; + } if (status == Class::kStatusNotReady) { + // Status is uninitialized if we couldn't determine the status at compile time, for example, + // not loading the class. + // TODO: when the verifier doesn't rely on Class-es failing to resolve/load the type hierarchy + // isn't a problem and this case shouldn't occur return false; } LOG(FATAL) << "Unexpected class status: " << status; diff --git a/src/dex_verifier.cc b/src/dex_verifier.cc index c422066d58..0fcaabd725 100644 --- a/src/dex_verifier.cc +++ b/src/dex_verifier.cc @@ -3038,17 +3038,17 @@ const RegType& DexVerifier::GetCaughtExceptionType() { common_super = ®_types_.JavaLangThrowable(); } else { const RegType& exception = ResolveClassAndCheckAccess(iterator.GetHandlerTypeIndex()); - /* TODO: on error do we want to keep going? If we don't fail this we run the risk of - * having a non-Throwable introduced at runtime. However, that won't pass an instanceof - * test, so is essentially harmless. - */ - if(!reg_types_.JavaLangThrowable().IsAssignableFrom(exception)) { + if (common_super == NULL) { + // Unconditionally assign for the first handler. We don't assert this is a Throwable + // as that is caught at runtime + common_super = &exception; + } else if(!reg_types_.JavaLangThrowable().IsAssignableFrom(exception)) { + // We don't know enough about the type and the common path merge will result in + // Conflict. Fail here knowing the correct thing can be done at runtime. Fail(VERIFY_ERROR_GENERIC) << "unexpected non-exception class " << exception; return reg_types_.Unknown(); - } else if (common_super == NULL) { - common_super = &exception; } else if (common_super->Equals(exception)) { - // nothing to do + // odd case, but nothing to do } else { common_super = &common_super->Merge(exception, ®_types_); CHECK(reg_types_.JavaLangThrowable().IsAssignableFrom(*common_super)); @@ -3062,6 +3062,7 @@ const RegType& DexVerifier::GetCaughtExceptionType() { if (common_super == NULL) { /* no catch blocks, or no catches with classes we can find */ Fail(VERIFY_ERROR_GENERIC) << "unable to find exception handler"; + return reg_types_.Unknown(); } return *common_super; } diff --git a/src/dex_verifier.h b/src/dex_verifier.h index 1537cbf6b8..b5325fff75 100644 --- a/src/dex_verifier.h +++ b/src/dex_verifier.h @@ -1161,9 +1161,8 @@ class DexVerifier { /* * For the "move-exception" instruction at "work_insn_idx_", which must be at an exception handler - * address, determine the first common superclass of all exceptions that can land here. - * Returns NULL if no matching exception handler can be found, or if the exception is not a - * subclass of Throwable. + * address, determine the Join of all exceptions that can land here. Fails if no matching + * exception handler can be found or if the Join of exception types fails. */ const RegType& GetCaughtExceptionType(); |