diff options
| author | 2017-01-20 10:29:59 +0000 | |
|---|---|---|
| committer | 2017-01-20 10:30:00 +0000 | |
| commit | 3eede0720f784b7e8a12cbf0bd8626c0e819a96d (patch) | |
| tree | 0df4180ec2cfb2ae06078d2d26abed91c00b519f | |
| parent | 5e821602426718bf971c3d693c3f8ff15d85017d (diff) | |
| parent | 865cf901e9f16680ca3594a5ab8d8e17b0b6f9d4 (diff) | |
Merge "vdex optimization: avoid doing application type resolution."
| -rw-r--r-- | compiler/verifier_deps_test.cc | 12 | ||||
| -rw-r--r-- | runtime/verifier/verifier_deps.cc | 39 |
2 files changed, 34 insertions, 17 deletions
diff --git a/compiler/verifier_deps_test.cc b/compiler/verifier_deps_test.cc index 4f06a91448..5fc9972d09 100644 --- a/compiler/verifier_deps_test.cc +++ b/compiler/verifier_deps_test.cc @@ -1414,7 +1414,14 @@ TEST_F(VerifierDepsTest, VerifyDeps) { ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self())); } - { + // The two tests below make sure that fiddling with the method kind + // (static, virtual, interface) is detected by `ValidateDependencies`. + + // An interface method lookup can succeed with a virtual method lookup on the same class. + // That's OK, as we only want to make sure there is a method being defined with the right + // flags. Therefore, polluting the interface methods with virtual methods does not have + // to fail verification. + if (resolution_kind != kVirtualMethodResolution) { VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer)); VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_); bool found = false; @@ -1433,7 +1440,8 @@ TEST_F(VerifierDepsTest, VerifyDeps) { ASSERT_FALSE(decoded_deps.ValidateDependencies(new_class_loader, soa.Self())); } - { + // See comment above that applies the same way. + if (resolution_kind != kInterfaceMethodResolution) { VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer)); VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_); bool found = false; diff --git a/runtime/verifier/verifier_deps.cc b/runtime/verifier/verifier_deps.cc index 15cc566cc6..113160785c 100644 --- a/runtime/verifier/verifier_deps.cc +++ b/runtime/verifier/verifier_deps.cc @@ -963,20 +963,25 @@ bool VerifierDeps::VerifyFields(Handle<mirror::ClassLoader> class_loader, // Check recorded fields are resolved the same way, have the same recorded class, // and have the same recorded flags. ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); - StackHandleScope<1> hs(self); - Handle<mirror::DexCache> dex_cache( - hs.NewHandle(class_linker->FindDexCache(self, dex_file, /* allow_failure */ false))); for (const auto& entry : fields) { - ArtField* field = class_linker->ResolveFieldJLS( - dex_file, entry.GetDexFieldIndex(), dex_cache, class_loader); - - if (field == nullptr) { - DCHECK(self->IsExceptionPending()); - self->ClearException(); + const DexFile::FieldId& field_id = dex_file.GetFieldId(entry.GetDexFieldIndex()); + StringPiece name(dex_file.StringDataByIdx(field_id.name_idx_)); + StringPiece type(dex_file.StringDataByIdx(dex_file.GetTypeId(field_id.type_idx_).descriptor_idx_)); + // Only use field_id.class_idx_ when the entry is unresolved, which is rare. + // Otherwise, we might end up resolving an application class, which is expensive. + std::string expected_decl_klass = entry.IsResolved() + ? GetStringFromId(dex_file, entry.GetDeclaringClassIndex()) + : dex_file.StringByTypeIdx(field_id.class_idx_); + mirror::Class* cls = FindClassAndClearException( + class_linker, self, expected_decl_klass.c_str(), class_loader); + if (cls == nullptr) { + LOG(INFO) << "VerifierDeps: Could not resolve class " << expected_decl_klass; + return false; } + DCHECK(cls->IsResolved()); + ArtField* field = mirror::Class::FindField(self, cls, name, type); if (entry.IsResolved()) { - std::string expected_decl_klass = GetStringFromId(dex_file, entry.GetDeclaringClassIndex()); std::string temp; if (field == nullptr) { LOG(INFO) << "VerifierDeps: Could not resolve field " @@ -1025,11 +1030,16 @@ bool VerifierDeps::VerifyMethods(Handle<mirror::ClassLoader> class_loader, const char* name = dex_file.GetMethodName(method_id); const Signature signature = dex_file.GetMethodSignature(method_id); - const char* descriptor = dex_file.GetMethodDeclaringClassDescriptor(method_id); - - mirror::Class* cls = FindClassAndClearException(class_linker, self, descriptor, class_loader); + // Only use method_id.class_idx_ when the entry is unresolved, which is rare. + // Otherwise, we might end up resolving an application class, which is expensive. + std::string expected_decl_klass = entry.IsResolved() + ? GetStringFromId(dex_file, entry.GetDeclaringClassIndex()) + : dex_file.StringByTypeIdx(method_id.class_idx_); + + mirror::Class* cls = FindClassAndClearException( + class_linker, self, expected_decl_klass.c_str(), class_loader); if (cls == nullptr) { - LOG(INFO) << "VerifierDeps: Could not resolve class " << descriptor; + LOG(INFO) << "VerifierDeps: Could not resolve class " << expected_decl_klass; return false; } DCHECK(cls->IsResolved()); @@ -1045,7 +1055,6 @@ bool VerifierDeps::VerifyMethods(Handle<mirror::ClassLoader> class_loader, if (entry.IsResolved()) { std::string temp; - std::string expected_decl_klass = GetStringFromId(dex_file, entry.GetDeclaringClassIndex()); if (method == nullptr) { LOG(INFO) << "VerifierDeps: Could not resolve " << kind |