Re-run verification when it fails at compile time
Verification may fail at compile time as we have incomplete information.
Always re-run at runtime in this case and don't report a VerifyError
yet.
Clean up GetCaughtExceptionType to better handle unresolved exceptions.
Change-Id: Ic526d0a1b43eea9783d540b7432cf22bf244bade
diff --git a/src/class_linker.cc b/src/class_linker.cc
index 51e5f3c..f343ed4 100644
--- a/src/class_linker.cc
+++ b/src/class_linker.cc
@@ -1924,6 +1924,7 @@
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 @@
<< " 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 @@
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 c422066..0fcaabd 100644
--- a/src/dex_verifier.cc
+++ b/src/dex_verifier.cc
@@ -3038,17 +3038,17 @@
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 @@
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 1537cbf..b5325ff 100644
--- a/src/dex_verifier.h
+++ b/src/dex_verifier.h
@@ -1161,9 +1161,8 @@
/*
* 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();
