Generalize vdex class redefinition check
The check introduced in CL If0c56b1970d8ebe701d198ffccec52f586aea9e6
skips fast verification if an apk's class is overshadowed by a class in
boot classpath because the vdex dependencies do not contain intra-apk
dependencies.
However, the change only checks for presence of a duplicate class in the
boot classloader, while a duplicate class could be in any of the parent
classloaders. Fix this and move the check into VerifierDeps to make it
a proper part of the verification process.
The CL also refactors VerifierDeps::ValidateDependencies to output
an error string for better logging.
Bug: 122968669
Test: test/testrunner/testrunner.py -t 719
Test: m test-art-gtest-verifier_deps_test
Change-Id: I0d06b82e31088c58d4493723a5435309740f1d0c
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index d46cffb..bfa039e 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -1765,42 +1765,6 @@
}
}
-// Returns true if any of the given dex files define a class from the boot classpath.
-static bool DexFilesRedefineBootClasses(
- const std::vector<const DexFile*>& dex_files,
- TimingLogger* timings) {
- TimingLogger::ScopedTiming t("Fast Verify: Boot Class Redefinition Check", timings);
-
- ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
- Thread* self = Thread::Current();
- ScopedObjectAccess soa(self);
-
- bool foundRedefinition = false;
- for (const DexFile* dex_file : dex_files) {
- for (ClassAccessor accessor : dex_file->GetClasses()) {
- const char* descriptor = accessor.GetDescriptor();
- StackHandleScope<1> hs_class(self);
- Handle<mirror::Class> klass =
- hs_class.NewHandle(class_linker->FindSystemClass(self, descriptor));
- if (klass == nullptr) {
- self->ClearException();
- } else {
- LOG(WARNING) << "Redefinition of boot class " << descriptor
- << " App dex file: " << accessor.GetDexFile().GetLocation()
- << " Boot dex file: " << klass->GetDexFile().GetLocation();
- foundRedefinition = true;
- if (!VLOG_IS_ON(verifier)) {
- // If we are not in verbose mode, return early.
- // Otherwise continue and log all the collisions for easier debugging.
- return true;
- }
- }
- }
- }
-
- return foundRedefinition;
-}
-
bool CompilerDriver::FastVerify(jobject jclass_loader,
const std::vector<const DexFile*>& dex_files,
TimingLogger* timings,
@@ -1813,21 +1777,14 @@
}
TimingLogger::ScopedTiming t("Fast Verify", timings);
- // We cannot do fast verification if the app redefines classes from the boot classpath.
- // Vdex does not record resolution chains for boot classes and we might wrongfully
- // resolve a class to the app when it should have been resolved to the boot classpath
- // (e.g. if we verified against the SDK and the app redefines a boot class which is not
- // in the SDK.)
- if (DexFilesRedefineBootClasses(dex_files, timings)) {
- LOG(WARNING) << "Found redefinition of boot classes. Not doing fast verification.";
- return false;
- }
-
ScopedObjectAccess soa(Thread::Current());
StackHandleScope<2> hs(soa.Self());
Handle<mirror::ClassLoader> class_loader(
hs.NewHandle(soa.Decode<mirror::ClassLoader>(jclass_loader)));
- if (!verifier_deps->ValidateDependencies(class_loader, soa.Self())) {
+ std::string error_msg;
+
+ if (!verifier_deps->ValidateDependencies(class_loader, soa.Self(), &error_msg)) {
+ LOG(WARNING) << "Fast verification failed: " << error_msg;
return false;
}
diff --git a/compiler/verifier_deps_test.cc b/compiler/verifier_deps_test.cc
index 092e931..5c6b815 100644
--- a/compiler/verifier_deps_test.cc
+++ b/compiler/verifier_deps_test.cc
@@ -449,6 +449,28 @@
has_unverified_classes;
}
+ // Load the dex file again with a new class loader, decode the VerifierDeps
+ // in `buffer`, allow the caller to modify the deps and then run validation.
+ template<typename Fn>
+ bool RunValidation(Fn fn, const std::vector<uint8_t>& buffer, std::string* error_msg) {
+ ScopedObjectAccess soa(Thread::Current());
+
+ jobject second_loader = LoadDex("VerifierDeps");
+ const auto& second_dex_files = GetDexFiles(second_loader);
+
+ VerifierDeps decoded_deps(second_dex_files, ArrayRef<const uint8_t>(buffer));
+ VerifierDeps::DexFileDeps* decoded_dex_deps =
+ decoded_deps.GetDexFileDeps(*second_dex_files.front());
+
+ // Let the test modify the dependencies.
+ fn(*decoded_dex_deps);
+
+ StackHandleScope<1> hs(soa.Self());
+ Handle<mirror::ClassLoader> new_class_loader =
+ hs.NewHandle<mirror::ClassLoader>(soa.Decode<mirror::ClassLoader>(second_loader));
+ return decoded_deps.ValidateDependencies(new_class_loader, soa.Self(), error_msg);
+ }
+
std::unique_ptr<verifier::VerifierDeps> verifier_deps_;
std::vector<const DexFile*> dex_files_;
const DexFile* primary_dex_file_;
@@ -1177,8 +1199,9 @@
}
TEST_F(VerifierDepsTest, VerifyDeps) {
- VerifyDexFile();
+ std::string error_msg;
+ VerifyDexFile();
ASSERT_EQ(1u, NumberOfCompiledDexFiles());
ASSERT_TRUE(HasEachKindOfRecord());
@@ -1186,249 +1209,166 @@
// the existing `class_loader_` may contain erroneous classes,
// that ClassLinker::FindClass won't return.
- ScopedObjectAccess soa(Thread::Current());
- StackHandleScope<1> hs(soa.Self());
- MutableHandle<mirror::ClassLoader> new_class_loader(hs.NewHandle<mirror::ClassLoader>(nullptr));
- {
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_TRUE(verifier_deps_->ValidateDependencies(new_class_loader, soa.Self()));
- }
-
std::vector<uint8_t> buffer;
verifier_deps_->Encode(dex_files_, &buffer);
ASSERT_FALSE(buffer.empty());
- {
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_TRUE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
+ // Check that dependencies are satisfied after decoding `buffer`.
+ ASSERT_TRUE(RunValidation([](VerifierDeps::DexFileDeps&) {}, buffer, &error_msg))
+ << error_msg;
- // Fiddle with the dependencies to make sure we catch any change and fail to verify.
+ // Mess with the dependencies to make sure we catch any change and fail to verify.
+ ASSERT_FALSE(RunValidation([](VerifierDeps::DexFileDeps& deps) {
+ deps.assignable_types_.insert(*deps.unassignable_types_.begin());
+ }, buffer, &error_msg));
- {
- // Mess up with the assignable_types.
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
- deps->assignable_types_.insert(*deps->unassignable_types_.begin());
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
+ // Mess with the unassignable_types.
+ ASSERT_FALSE(RunValidation([](VerifierDeps::DexFileDeps& deps) {
+ deps.unassignable_types_.insert(*deps.assignable_types_.begin());
+ }, buffer, &error_msg));
- {
- // Mess up with the unassignable_types.
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
- deps->unassignable_types_.insert(*deps->assignable_types_.begin());
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
+ // Mess with classes.
+ ASSERT_FALSE(RunValidation([](VerifierDeps::DexFileDeps& deps) {
+ for (const auto& entry : deps.classes_) {
+ if (entry.IsResolved()) {
+ deps.classes_.insert(VerifierDeps::ClassResolution(
+ entry.GetDexTypeIndex(), VerifierDeps::kUnresolvedMarker));
+ return;
+ }
+ }
+ LOG(FATAL) << "Could not find any resolved classes";
+ UNREACHABLE();
+ }, buffer, &error_msg));
+ ASSERT_FALSE(RunValidation([](VerifierDeps::DexFileDeps& deps) {
+ for (const auto& entry : deps.classes_) {
+ if (!entry.IsResolved()) {
+ deps.classes_.insert(VerifierDeps::ClassResolution(
+ entry.GetDexTypeIndex(), VerifierDeps::kUnresolvedMarker - 1));
+ return;
+ }
+ }
+ LOG(FATAL) << "Could not find any unresolved classes";
+ UNREACHABLE();
+ }, buffer, &error_msg));
+ ASSERT_FALSE(RunValidation([](VerifierDeps::DexFileDeps& deps) {
+ for (const auto& entry : deps.classes_) {
+ if (entry.IsResolved()) {
+ deps.classes_.insert(VerifierDeps::ClassResolution(
+ entry.GetDexTypeIndex(), entry.GetAccessFlags() - 1));
+ return;
+ }
+ }
+ LOG(FATAL) << "Could not find any resolved classes";
+ UNREACHABLE();
+ }, buffer, &error_msg));
- // Mess up with classes.
- {
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
- bool found = false;
- for (const auto& entry : deps->classes_) {
- if (entry.IsResolved()) {
- deps->classes_.insert(VerifierDeps::ClassResolution(
- entry.GetDexTypeIndex(), VerifierDeps::kUnresolvedMarker));
- found = true;
- break;
- }
- }
- ASSERT_TRUE(found);
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
+ // Mess with fields.
+ ASSERT_FALSE(RunValidation([](VerifierDeps::DexFileDeps& deps) {
+ for (const auto& entry : deps.fields_) {
+ if (entry.IsResolved()) {
+ deps.fields_.insert(VerifierDeps::FieldResolution(entry.GetDexFieldIndex(),
+ VerifierDeps::kUnresolvedMarker,
+ entry.GetDeclaringClassIndex()));
+ return;
+ }
+ }
+ LOG(FATAL) << "Could not find any resolved fields";
+ UNREACHABLE();
+ }, buffer, &error_msg));
+ ASSERT_FALSE(RunValidation([](VerifierDeps::DexFileDeps& deps) {
+ for (const auto& entry : deps.fields_) {
+ if (!entry.IsResolved()) {
+ constexpr dex::StringIndex kStringIndexZero(0); // We know there is a class there.
+ deps.fields_.insert(VerifierDeps::FieldResolution(0 /* we know there is a field there */,
+ VerifierDeps::kUnresolvedMarker - 1,
+ kStringIndexZero));
+ return;
+ }
+ }
+ LOG(FATAL) << "Could not find any unresolved fields";
+ UNREACHABLE();
+ }, buffer, &error_msg));
+ ASSERT_FALSE(RunValidation([](VerifierDeps::DexFileDeps& deps) {
+ for (const auto& entry : deps.fields_) {
+ if (entry.IsResolved()) {
+ deps.fields_.insert(VerifierDeps::FieldResolution(entry.GetDexFieldIndex(),
+ entry.GetAccessFlags() - 1,
+ entry.GetDeclaringClassIndex()));
+ return;
+ }
+ }
+ LOG(FATAL) << "Could not find any resolved fields";
+ UNREACHABLE();
+ }, buffer, &error_msg));
+ ASSERT_FALSE(RunValidation([](VerifierDeps::DexFileDeps& deps) {
+ for (const auto& entry : deps.fields_) {
+ constexpr dex::StringIndex kNewTypeIndex(0);
+ if (entry.GetDeclaringClassIndex() != kNewTypeIndex) {
+ deps.fields_.insert(VerifierDeps::FieldResolution(entry.GetDexFieldIndex(),
+ entry.GetAccessFlags(),
+ kNewTypeIndex));
+ return;
+ }
+ }
+ LOG(FATAL) << "Could not find any suitable fields";
+ UNREACHABLE();
+ }, buffer, &error_msg));
- {
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
- bool found = false;
- for (const auto& entry : deps->classes_) {
- if (!entry.IsResolved()) {
- deps->classes_.insert(VerifierDeps::ClassResolution(
- entry.GetDexTypeIndex(), VerifierDeps::kUnresolvedMarker - 1));
- found = true;
- break;
- }
- }
- ASSERT_TRUE(found);
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
-
- {
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
- bool found = false;
- for (const auto& entry : deps->classes_) {
- if (entry.IsResolved()) {
- deps->classes_.insert(VerifierDeps::ClassResolution(
- entry.GetDexTypeIndex(), entry.GetAccessFlags() - 1));
- found = true;
- break;
- }
- }
- ASSERT_TRUE(found);
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
-
- // Mess up with fields.
- {
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
- bool found = false;
- for (const auto& entry : deps->fields_) {
- if (entry.IsResolved()) {
- deps->fields_.insert(VerifierDeps::FieldResolution(entry.GetDexFieldIndex(),
- VerifierDeps::kUnresolvedMarker,
- entry.GetDeclaringClassIndex()));
- found = true;
- break;
- }
- }
- ASSERT_TRUE(found);
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
-
- {
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
- bool found = false;
- for (const auto& entry : deps->fields_) {
- if (!entry.IsResolved()) {
- constexpr dex::StringIndex kStringIndexZero(0); // We know there is a class there.
- deps->fields_.insert(VerifierDeps::FieldResolution(0 /* we know there is a field there */,
- VerifierDeps::kUnresolvedMarker - 1,
- kStringIndexZero));
- found = true;
- break;
- }
- }
- ASSERT_TRUE(found);
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
-
- {
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
- bool found = false;
- for (const auto& entry : deps->fields_) {
- if (entry.IsResolved()) {
- deps->fields_.insert(VerifierDeps::FieldResolution(entry.GetDexFieldIndex(),
- entry.GetAccessFlags() - 1,
- entry.GetDeclaringClassIndex()));
- found = true;
- break;
- }
- }
- ASSERT_TRUE(found);
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
-
- {
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
- bool found = false;
- for (const auto& entry : deps->fields_) {
- constexpr dex::StringIndex kNewTypeIndex(0);
- if (entry.GetDeclaringClassIndex() != kNewTypeIndex) {
- deps->fields_.insert(VerifierDeps::FieldResolution(entry.GetDexFieldIndex(),
- entry.GetAccessFlags(),
- kNewTypeIndex));
- found = true;
- break;
- }
- }
- ASSERT_TRUE(found);
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
-
- // Mess up with methods.
- {
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
- bool found = false;
- std::set<VerifierDeps::MethodResolution>* methods = &deps->methods_;
- for (const auto& entry : *methods) {
- if (entry.IsResolved()) {
- methods->insert(VerifierDeps::MethodResolution(entry.GetDexMethodIndex(),
- VerifierDeps::kUnresolvedMarker,
- entry.GetDeclaringClassIndex()));
- found = true;
- break;
- }
- }
- ASSERT_TRUE(found);
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
-
- {
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
- bool found = false;
- std::set<VerifierDeps::MethodResolution>* methods = &deps->methods_;
- for (const auto& entry : *methods) {
- if (!entry.IsResolved()) {
- constexpr dex::StringIndex kStringIndexZero(0); // We know there is a class there.
- methods->insert(VerifierDeps::MethodResolution(0 /* we know there is a method there */,
- VerifierDeps::kUnresolvedMarker - 1,
- kStringIndexZero));
- found = true;
- break;
- }
- }
- ASSERT_TRUE(found);
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
-
- {
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
- bool found = false;
- std::set<VerifierDeps::MethodResolution>* methods = &deps->methods_;
- for (const auto& entry : *methods) {
- if (entry.IsResolved()) {
- methods->insert(VerifierDeps::MethodResolution(entry.GetDexMethodIndex(),
- entry.GetAccessFlags() - 1,
- entry.GetDeclaringClassIndex()));
- found = true;
- break;
- }
- }
- ASSERT_TRUE(found);
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
-
- {
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
- VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
- bool found = false;
- std::set<VerifierDeps::MethodResolution>* methods = &deps->methods_;
- for (const auto& entry : *methods) {
- constexpr dex::StringIndex kNewTypeIndex(0);
- if (entry.IsResolved() && entry.GetDeclaringClassIndex() != kNewTypeIndex) {
- methods->insert(VerifierDeps::MethodResolution(entry.GetDexMethodIndex(),
- entry.GetAccessFlags(),
- kNewTypeIndex));
- found = true;
- break;
- }
- }
- ASSERT_TRUE(found);
- new_class_loader.Assign(soa.Decode<mirror::ClassLoader>(LoadDex("VerifierDeps")));
- ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self()));
- }
+ // Mess with methods.
+ ASSERT_FALSE(RunValidation([](VerifierDeps::DexFileDeps& deps) {
+ std::set<VerifierDeps::MethodResolution>* methods = &deps.methods_;
+ for (const auto& entry : *methods) {
+ if (entry.IsResolved()) {
+ methods->insert(VerifierDeps::MethodResolution(entry.GetDexMethodIndex(),
+ VerifierDeps::kUnresolvedMarker,
+ entry.GetDeclaringClassIndex()));
+ return;
+ }
+ }
+ LOG(FATAL) << "Could not find any resolved methods";
+ UNREACHABLE();
+ }, buffer, &error_msg));
+ ASSERT_FALSE(RunValidation([](VerifierDeps::DexFileDeps& deps) {
+ std::set<VerifierDeps::MethodResolution>* methods = &deps.methods_;
+ for (const auto& entry : *methods) {
+ if (!entry.IsResolved()) {
+ constexpr dex::StringIndex kStringIndexZero(0); // We know there is a class there.
+ methods->insert(VerifierDeps::MethodResolution(0 /* we know there is a method there */,
+ VerifierDeps::kUnresolvedMarker - 1,
+ kStringIndexZero));
+ return;
+ }
+ }
+ LOG(FATAL) << "Could not find any unresolved methods";
+ UNREACHABLE();
+ }, buffer, &error_msg));
+ ASSERT_FALSE(RunValidation([](VerifierDeps::DexFileDeps& deps) {
+ std::set<VerifierDeps::MethodResolution>* methods = &deps.methods_;
+ for (const auto& entry : *methods) {
+ if (entry.IsResolved()) {
+ methods->insert(VerifierDeps::MethodResolution(entry.GetDexMethodIndex(),
+ entry.GetAccessFlags() - 1,
+ entry.GetDeclaringClassIndex()));
+ return;
+ }
+ }
+ LOG(FATAL) << "Could not find any resolved methods";
+ UNREACHABLE();
+ }, buffer, &error_msg));
+ ASSERT_FALSE(RunValidation([](VerifierDeps::DexFileDeps& deps) {
+ std::set<VerifierDeps::MethodResolution>* methods = &deps.methods_;
+ for (const auto& entry : *methods) {
+ constexpr dex::StringIndex kNewTypeIndex(0);
+ if (entry.IsResolved() && entry.GetDeclaringClassIndex() != kNewTypeIndex) {
+ methods->insert(VerifierDeps::MethodResolution(entry.GetDexMethodIndex(),
+ entry.GetAccessFlags(),
+ kNewTypeIndex));
+ return;
+ }
+ }
+ LOG(FATAL) << "Could not find any suitable methods";
+ UNREACHABLE();
+ }, buffer, &error_msg));
}
TEST_F(VerifierDepsTest, CompilerDriver) {