diff options
author | 2017-03-08 10:07:42 +0000 | |
---|---|---|
committer | 2017-03-08 10:07:43 +0000 | |
commit | 742bc41f754e77a528f859babec4dfea179ca96e (patch) | |
tree | 8a0587040a53b4d42c8aa90350ae93afc91a37d2 | |
parent | 2ec053fa337c1934ccb803136b56b57bcc06a32f (diff) | |
parent | 7cc3ae5705416bd8fc4b7096904e2871aa761e73 (diff) |
Merge "Return the right value in VerifyClass."
-rw-r--r-- | build/Android.gtest.mk | 11 | ||||
-rw-r--r-- | compiler/driver/compiler_driver.cc | 22 | ||||
-rw-r--r-- | compiler/verifier_deps_test.cc | 23 | ||||
-rw-r--r-- | runtime/class_linker.cc | 15 | ||||
-rw-r--r-- | runtime/mirror/class.h | 6 | ||||
-rw-r--r-- | test/VerifierDeps/MySub1SoftVerificationFailure.smali | 16 | ||||
-rw-r--r-- | test/VerifierDeps/MySub2SoftVerificationFailure.smali | 16 | ||||
-rw-r--r-- | test/VerifierDepsMulti/MySoftVerificationFailure.smali | 24 |
8 files changed, 117 insertions, 16 deletions
diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk index b661e001c8..c27f8dbe4a 100644 --- a/build/Android.gtest.mk +++ b/build/Android.gtest.mk @@ -75,8 +75,11 @@ $(ART_TEST_TARGET_GTEST_MainStripped_DEX): $(ART_TEST_TARGET_GTEST_Main_DEX) $(call dexpreopt-remove-classes.dex,$@) ART_TEST_GTEST_VerifierDeps_SRC := $(abspath $(wildcard $(LOCAL_PATH)/VerifierDeps/*.smali)) +ART_TEST_GTEST_VerifierDepsMulti_SRC := $(abspath $(wildcard $(LOCAL_PATH)/VerifierDepsMulti/*.smali)) ART_TEST_HOST_GTEST_VerifierDeps_DEX := $(dir $(ART_TEST_HOST_GTEST_Main_DEX))$(subst Main,VerifierDeps,$(basename $(notdir $(ART_TEST_HOST_GTEST_Main_DEX))))$(suffix $(ART_TEST_HOST_GTEST_Main_DEX)) ART_TEST_TARGET_GTEST_VerifierDeps_DEX := $(dir $(ART_TEST_TARGET_GTEST_Main_DEX))$(subst Main,VerifierDeps,$(basename $(notdir $(ART_TEST_TARGET_GTEST_Main_DEX))))$(suffix $(ART_TEST_TARGET_GTEST_Main_DEX)) +ART_TEST_HOST_GTEST_VerifierDepsMulti_DEX := $(dir $(ART_TEST_HOST_GTEST_Main_DEX))$(subst Main,VerifierDepsMulti,$(basename $(notdir $(ART_TEST_HOST_GTEST_Main_DEX))))$(suffix $(ART_TEST_HOST_GTEST_Main_DEX)) +ART_TEST_TARGET_GTEST_VerifierDepsMulti_DEX := $(dir $(ART_TEST_TARGET_GTEST_Main_DEX))$(subst Main,VerifierDepsMulti,$(basename $(notdir $(ART_TEST_TARGET_GTEST_Main_DEX))))$(suffix $(ART_TEST_TARGET_GTEST_Main_DEX)) $(ART_TEST_HOST_GTEST_VerifierDeps_DEX): $(ART_TEST_GTEST_VerifierDeps_SRC) $(HOST_OUT_EXECUTABLES)/smali $(HOST_OUT_EXECUTABLES)/smali --output=$@ $(filter %.smali,$^) @@ -84,6 +87,12 @@ $(ART_TEST_HOST_GTEST_VerifierDeps_DEX): $(ART_TEST_GTEST_VerifierDeps_SRC) $(HO $(ART_TEST_TARGET_GTEST_VerifierDeps_DEX): $(ART_TEST_GTEST_VerifierDeps_SRC) $(HOST_OUT_EXECUTABLES)/smali $(HOST_OUT_EXECUTABLES)/smali --output=$@ $(filter %.smali,$^) +$(ART_TEST_HOST_GTEST_VerifierDepsMulti_DEX): $(ART_TEST_GTEST_VerifierDepsMulti_SRC) $(HOST_OUT_EXECUTABLES)/smali + $(HOST_OUT_EXECUTABLES)/smali --output=$@ $(filter %.smali,$^) + +$(ART_TEST_TARGET_GTEST_VerifierDepsMulti_DEX): $(ART_TEST_GTEST_VerifierDepsMulti_SRC) $(HOST_OUT_EXECUTABLES)/smali + $(HOST_OUT_EXECUTABLES)/smali --output=$@ $(filter %.smali,$^) + # Dex file dependencies for each gtest. ART_GTEST_dex2oat_environment_tests_DEX_DEPS := Main MainStripped MultiDex MultiDexModifiedSecondary Nested @@ -115,7 +124,7 @@ ART_GTEST_stub_test_DEX_DEPS := AllFields ART_GTEST_transaction_test_DEX_DEPS := Transaction ART_GTEST_type_lookup_table_test_DEX_DEPS := Lookup ART_GTEST_unstarted_runtime_test_DEX_DEPS := Nested -ART_GTEST_verifier_deps_test_DEX_DEPS := VerifierDeps MultiDex +ART_GTEST_verifier_deps_test_DEX_DEPS := VerifierDeps VerifierDepsMulti MultiDex ART_GTEST_dex_to_dex_decompiler_test_DEX_DEPS := VerifierDeps DexToDexDecompiler # The elf writer test has dependencies on core.oat. diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index a5e4cb0877..057e3c9960 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -535,9 +535,8 @@ static optimizer::DexToDexCompilationLevel GetDexToDexCompilationLevel( if (klass->IsVerified()) { // Class is verified so we can enable DEX-to-DEX compilation for performance. return max_level; - } else if (klass->IsCompileTimeVerified()) { + } else if (klass->ShouldVerifyAtRuntime()) { // Class verification has soft-failed. Anyway, ensure at least correctness. - DCHECK_EQ(klass->GetStatus(), mirror::Class::kStatusRetryVerificationAtRuntime); return optimizer::DexToDexCompilationLevel::kRequired; } else { // Class verification has failed: do not run DEX-to-DEX compilation. @@ -964,7 +963,7 @@ static void EnsureVerifiedOrVerifyAtRuntime(jobject jclass_loader, if (cls == nullptr) { soa.Self()->ClearException(); } else if (&cls->GetDexFile() == dex_file) { - DCHECK(cls->IsErroneous() || cls->IsVerified() || cls->IsCompileTimeVerified()) + DCHECK(cls->IsErroneous() || cls->IsVerified() || cls->ShouldVerifyAtRuntime()) << cls->PrettyClass() << " " << cls->GetStatus(); } @@ -2160,6 +2159,14 @@ class VerifyClassVisitor : public CompilationVisitor { LOG(ERROR) << "Verification failed on class " << PrettyDescriptor(descriptor) << " because: " << error_msg; manager_->GetCompiler()->SetHadHardVerifierFailure(); + } else { + // Force a soft failure for the VerifierDeps. This is a sanity measure, as + // the vdex file already records that the class hasn't been resolved. It avoids + // trying to do future verification optimizations when processing the vdex file. + DCHECK(failure_kind == verifier::MethodVerifier::kSoftFailure || + failure_kind == verifier::MethodVerifier::kNoFailure) + << failure_kind; + failure_kind = verifier::MethodVerifier::kSoftFailure; } } else if (!SkipClass(jclass_loader, dex_file, klass.Get())) { CHECK(klass->IsResolved()) << klass->PrettyClass(); @@ -2172,7 +2179,7 @@ class VerifyClassVisitor : public CompilationVisitor { manager_->GetCompiler()->SetHadHardVerifierFailure(); } - CHECK(klass->IsCompileTimeVerified() || klass->IsErroneous()) + CHECK(klass->ShouldVerifyAtRuntime() || klass->IsVerified() || klass->IsErroneous()) << klass->PrettyDescriptor() << ": state=" << klass->GetStatus(); // It is *very* problematic if there are verification errors in the boot classpath. For example, @@ -2186,6 +2193,13 @@ class VerifyClassVisitor : public CompilationVisitor { DCHECK(klass->IsVerified()) << "Boot classpath class " << klass->PrettyClass() << " failed to fully verify: state= " << klass->GetStatus(); } + if (klass->IsVerified()) { + DCHECK_EQ(failure_kind, verifier::MethodVerifier::kNoFailure); + } else if (klass->ShouldVerifyAtRuntime()) { + DCHECK_EQ(failure_kind, verifier::MethodVerifier::kSoftFailure); + } else { + DCHECK_EQ(failure_kind, verifier::MethodVerifier::kHardFailure); + } } } else { // Make the skip a soft failure, essentially being considered as verify at runtime. diff --git a/compiler/verifier_deps_test.cc b/compiler/verifier_deps_test.cc index c892b25ed3..01c33591e5 100644 --- a/compiler/verifier_deps_test.cc +++ b/compiler/verifier_deps_test.cc @@ -246,9 +246,13 @@ class VerifierDepsTest : public CommonCompilerTest { } bool HasUnverifiedClass(const std::string& cls) { - const DexFile::TypeId* type_id = primary_dex_file_->FindTypeId(cls.c_str()); + return HasUnverifiedClass(cls, *primary_dex_file_); + } + + bool HasUnverifiedClass(const std::string& cls, const DexFile& dex_file) { + const DexFile::TypeId* type_id = dex_file.FindTypeId(cls.c_str()); DCHECK(type_id != nullptr); - dex::TypeIndex index = primary_dex_file_->GetIndexForTypeId(*type_id); + dex::TypeIndex index = dex_file.GetIndexForTypeId(*type_id); for (const auto& dex_dep : verifier_deps_->dex_deps_) { for (dex::TypeIndex entry : dex_dep.second->unverified_classes_) { if (index == entry) { @@ -1141,7 +1145,7 @@ TEST_F(VerifierDepsTest, UnverifiedClasses) { // Test that a class with hard failure is recorded. ASSERT_TRUE(HasUnverifiedClass("LMyVerificationFailure;")); // Test that a class with unresolved super is recorded. - ASSERT_FALSE(HasUnverifiedClass("LMyClassWithNoSuper;")); + ASSERT_TRUE(HasUnverifiedClass("LMyClassWithNoSuper;")); // Test that a class with unresolved super and hard failure is recorded. ASSERT_TRUE(HasUnverifiedClass("LMyClassWithNoSuperButFailures;")); } @@ -1511,5 +1515,18 @@ TEST_F(VerifierDepsTest, CompilerDriver) { } } +TEST_F(VerifierDepsTest, MultiDexVerification) { + VerifyDexFile("VerifierDepsMulti"); + ASSERT_EQ(NumberOfCompiledDexFiles(), 2u); + + ASSERT_TRUE(HasUnverifiedClass("LMySoftVerificationFailure;", *dex_files_[1])); + ASSERT_TRUE(HasUnverifiedClass("LMySub1SoftVerificationFailure;", *dex_files_[0])); + ASSERT_TRUE(HasUnverifiedClass("LMySub2SoftVerificationFailure;", *dex_files_[0])); + + std::vector<uint8_t> buffer; + verifier_deps_->Encode(dex_files_, &buffer); + ASSERT_FALSE(buffer.empty()); +} + } // namespace verifier } // namespace art diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index d232714338..360d4aba05 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -3900,8 +3900,10 @@ bool ClassLinker::AttemptSupertypeVerification(Thread* self, if (!supertype->IsVerified() && !supertype->IsErroneous()) { VerifyClass(self, supertype); } - if (supertype->IsCompileTimeVerified()) { - // Either we are verified or we soft failed and need to retry at runtime. + + if (supertype->IsVerified() || supertype->ShouldVerifyAtRuntime()) { + // The supertype is either verified, or we soft failed at AOT time. + DCHECK(supertype->IsVerified() || Runtime::Current()->IsAotCompiler()); return true; } // If we got this far then we have a hard failure. @@ -3967,13 +3969,16 @@ verifier::MethodVerifier::FailureKind ClassLinker::VerifyClass( return verifier::MethodVerifier::kHardFailure; } - // Don't attempt to re-verify if already sufficiently verified. + // Don't attempt to re-verify if already verified. if (klass->IsVerified()) { EnsureSkipAccessChecksMethods(klass, image_pointer_size_); return verifier::MethodVerifier::kNoFailure; } - if (klass->IsCompileTimeVerified() && Runtime::Current()->IsAotCompiler()) { - return verifier::MethodVerifier::kNoFailure; + + // For AOT, don't attempt to re-verify if we have already found we should + // verify at runtime. + if (Runtime::Current()->IsAotCompiler() && klass->ShouldVerifyAtRuntime()) { + return verifier::MethodVerifier::kSoftFailure; } if (klass->GetStatus() == mirror::Class::kStatusResolved) { diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h index d34f09c721..b68eedcb11 100644 --- a/runtime/mirror/class.h +++ b/runtime/mirror/class.h @@ -206,10 +206,10 @@ class MANAGED Class FINAL : public Object { return status >= kStatusResolved || status == kStatusErrorResolved; } - // Returns true if the class was compile-time verified. + // Returns true if the class should be verified at runtime. template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags> - bool IsCompileTimeVerified() REQUIRES_SHARED(Locks::mutator_lock_) { - return GetStatus<kVerifyFlags>() >= kStatusRetryVerificationAtRuntime; + bool ShouldVerifyAtRuntime() REQUIRES_SHARED(Locks::mutator_lock_) { + return GetStatus<kVerifyFlags>() == kStatusRetryVerificationAtRuntime; } // Returns true if the class has been verified. diff --git a/test/VerifierDeps/MySub1SoftVerificationFailure.smali b/test/VerifierDeps/MySub1SoftVerificationFailure.smali new file mode 100644 index 0000000000..8123394e87 --- /dev/null +++ b/test/VerifierDeps/MySub1SoftVerificationFailure.smali @@ -0,0 +1,16 @@ +# Copyright (C) 2017 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +.class public LMySub1SoftVerificationFailure; +.super LMySoftVerificationFailure; diff --git a/test/VerifierDeps/MySub2SoftVerificationFailure.smali b/test/VerifierDeps/MySub2SoftVerificationFailure.smali new file mode 100644 index 0000000000..8d003236c8 --- /dev/null +++ b/test/VerifierDeps/MySub2SoftVerificationFailure.smali @@ -0,0 +1,16 @@ +# Copyright (C) 2017 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +.class public LMySub2SoftVerificationFailure; +.super LMySoftVerificationFailure; diff --git a/test/VerifierDepsMulti/MySoftVerificationFailure.smali b/test/VerifierDepsMulti/MySoftVerificationFailure.smali new file mode 100644 index 0000000000..6b56a3b6dc --- /dev/null +++ b/test/VerifierDepsMulti/MySoftVerificationFailure.smali @@ -0,0 +1,24 @@ +# Copyright (C) 2017 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +.class public LMySoftVerificationFailure; +.super Ljava/lang/Object; + +.method public final foo()V + .registers 1 + sget-object v0, LMySoftVerificationFailure;->error:LUnknownType; + throw v0 +.end method + +.field public static error:LUnknownType; |