diff options
author | Calin Juravle <calin@google.com> | 2015-04-13 18:42:21 +0100 |
---|---|---|
committer | Calin Juravle <calin@google.com> | 2015-04-16 16:28:11 +0100 |
commit | f1c6d9e87cbfd27702103ccc7c7f08ce784dc872 (patch) | |
tree | 45ad9f5bb52eb0db3857e344ab67b5aab2309472 | |
parent | e015a31e509c3f4de8a90b57b77329ba6609ce2f (diff) | |
download | art-f1c6d9e87cbfd27702103ccc7c7f08ce784dc872.zip art-f1c6d9e87cbfd27702103ccc7c7f08ce784dc872.tar.gz art-f1c6d9e87cbfd27702103ccc7c7f08ce784dc872.tar.bz2 |
Fallback to quick in case of soft verification errors
Add a regression test: using uninitialized values triggers a soft
verification error and optimizing should not crash.
Thanks to Stephen Kyle (stephenckyle@googlemail.com) for the bug report.
Bug: 19988704
Change-Id: I67174538eed853baff735694b3ae8eb34afe2a39
-rw-r--r-- | compiler/dex/verified_method.cc | 1 | ||||
-rw-r--r-- | compiler/dex/verified_method.h | 7 | ||||
-rw-r--r-- | compiler/driver/compiler_driver.cc | 25 | ||||
-rw-r--r-- | compiler/driver/compiler_driver.h | 6 | ||||
-rw-r--r-- | compiler/optimizing/inliner.cc | 8 | ||||
-rw-r--r-- | compiler/optimizing/optimizing_compiler.cc | 23 | ||||
-rw-r--r-- | compiler/optimizing/optimizing_compiler_stats.h | 4 | ||||
-rw-r--r-- | test/471-uninitialized-locals/expected.txt | 0 | ||||
-rw-r--r-- | test/471-uninitialized-locals/info.txt | 2 | ||||
-rw-r--r-- | test/471-uninitialized-locals/smali/Test.smali | 23 | ||||
-rw-r--r-- | test/471-uninitialized-locals/src/Main.java | 37 |
11 files changed, 129 insertions, 7 deletions
diff --git a/compiler/dex/verified_method.cc b/compiler/dex/verified_method.cc index ae814b4..977757f 100644 --- a/compiler/dex/verified_method.cc +++ b/compiler/dex/verified_method.cc @@ -40,6 +40,7 @@ namespace art { const VerifiedMethod* VerifiedMethod::Create(verifier::MethodVerifier* method_verifier, bool compile) { std::unique_ptr<VerifiedMethod> verified_method(new VerifiedMethod); + verified_method->has_verification_failures_ = method_verifier->HasFailures(); if (compile) { /* Generate a register map. */ if (!verified_method->GenerateGcMap(method_verifier)) { diff --git a/compiler/dex/verified_method.h b/compiler/dex/verified_method.h index 954cbf4..437ae52 100644 --- a/compiler/dex/verified_method.h +++ b/compiler/dex/verified_method.h @@ -70,6 +70,11 @@ class VerifiedMethod { // by using the check-cast elision peephole optimization in the verifier. bool IsSafeCast(uint32_t pc) const; + // Returns true if there were any errors during verification. + bool HasVerificationFailures() const { + return has_verification_failures_; + } + private: VerifiedMethod() = default; @@ -107,6 +112,8 @@ class VerifiedMethod { // dex PC to dex method index or dex field index based on the instruction. DequickenMap dequicken_map_; SafeCastSet safe_cast_set_; + + bool has_verification_failures_; }; } // namespace art diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index ef47377..641d174 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -2344,6 +2344,31 @@ CompiledMethod* CompilerDriver::GetCompiledMethod(MethodReference ref) const { return it->second; } +bool CompilerDriver::IsMethodVerifiedWithoutFailures(uint32_t method_idx, + uint16_t class_def_idx, + const DexFile& dex_file) const { + const VerifiedMethod* verified_method = GetVerifiedMethod(&dex_file, method_idx); + if (verified_method != nullptr) { + return !verified_method->HasVerificationFailures(); + } + + // If we can't find verification metadata, check if this is a system class (we trust that system + // classes have their methods verified). If it's not, be conservative and assume the method + // has not been verified successfully. + + // TODO: When compiling the boot image it should be safe to assume that everything is verified, + // even if methods are not found in the verification cache. + const char* descriptor = dex_file.GetClassDescriptor(dex_file.GetClassDef(class_def_idx)); + ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); + Thread* self = Thread::Current(); + ScopedObjectAccess soa(self); + bool is_system_class = class_linker->FindSystemClass(self, descriptor) != nullptr; + if (!is_system_class) { + self->ClearException(); + } + return is_system_class; +} + size_t CompilerDriver::GetNonRelativeLinkerPatchCount() const { MutexLock mu(Thread::Current(), compiled_methods_lock_); return non_relative_linker_patch_count_; diff --git a/compiler/driver/compiler_driver.h b/compiler/driver/compiler_driver.h index f1066a5..1a4ae13 100644 --- a/compiler/driver/compiler_driver.h +++ b/compiler/driver/compiler_driver.h @@ -425,6 +425,12 @@ class CompilerDriver { void RecordClassStatus(ClassReference ref, mirror::Class::Status status) LOCKS_EXCLUDED(compiled_classes_lock_); + // Checks if the specified method has been verified without failures. Returns + // false if the method is not in the verification results (GetVerificationResults). + bool IsMethodVerifiedWithoutFailures(uint32_t method_idx, + uint16_t class_def_idx, + const DexFile& dex_file) const; + SwapVector<uint8_t>* DeduplicateCode(const ArrayRef<const uint8_t>& code); SwapSrcMap* DeduplicateSrcMappingTable(const ArrayRef<SrcMapElem>& src_map); SwapVector<uint8_t>* DeduplicateMappingTable(const ArrayRef<const uint8_t>& code); diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index 2c17a67..87a8d33 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -31,6 +31,8 @@ #include "ssa_phi_elimination.h" #include "scoped_thread_state_change.h" #include "thread.h" +#include "dex/verified_method.h" +#include "dex/verification_results.h" namespace art { @@ -114,9 +116,11 @@ bool HInliner::TryInline(HInvoke* invoke_instruction, return false; } - if (!resolved_method->GetDeclaringClass()->IsVerified()) { + uint16_t class_def_idx = resolved_method->GetDeclaringClass()->GetDexClassDefIndex(); + if (!compiler_driver_->IsMethodVerifiedWithoutFailures( + resolved_method->GetDexMethodIndex(), class_def_idx, *resolved_method->GetDexFile())) { VLOG(compiler) << "Method " << PrettyMethod(method_index, caller_dex_file) - << " is not inlined because its class could not be verified"; + << " couldn't be verified, so it cannot be inlined"; return false; } diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc index 56cea8a..64a066d 100644 --- a/compiler/optimizing/optimizing_compiler.cc +++ b/compiler/optimizing/optimizing_compiler.cc @@ -31,6 +31,8 @@ #include "constant_folding.h" #include "dead_code_elimination.h" #include "dex/quick/dex_file_to_method_inliner_map.h" +#include "dex/verified_method.h" +#include "dex/verification_results.h" #include "driver/compiler_driver.h" #include "driver/compiler_options.h" #include "driver/dex_compilation_unit.h" @@ -45,13 +47,13 @@ #include "mirror/art_method-inl.h" #include "nodes.h" #include "prepare_for_register_allocation.h" +#include "reference_type_propagation.h" #include "register_allocator.h" #include "side_effects_analysis.h" #include "ssa_builder.h" #include "ssa_phi_elimination.h" #include "ssa_liveness_analysis.h" #include "utils/assembler.h" -#include "reference_type_propagation.h" namespace art { @@ -592,15 +594,26 @@ CompiledMethod* OptimizingCompiler::Compile(const DexFile::CodeItem* code_item, InvokeType invoke_type, uint16_t class_def_idx, uint32_t method_idx, - jobject class_loader, + jobject jclass_loader, const DexFile& dex_file) const { - CompiledMethod* method = TryCompile(code_item, access_flags, invoke_type, class_def_idx, - method_idx, class_loader, dex_file); + CompilerDriver* compiler_driver = GetCompilerDriver(); + CompiledMethod* method = nullptr; + if (compiler_driver->IsMethodVerifiedWithoutFailures(method_idx, class_def_idx, dex_file)) { + method = TryCompile(code_item, access_flags, invoke_type, class_def_idx, + method_idx, jclass_loader, dex_file); + } else { + if (compiler_driver->GetCompilerOptions().VerifyAtRuntime()) { + compilation_stats_.RecordStat(MethodCompilationStat::kNotCompiledVerifyAtRuntime); + } else { + compilation_stats_.RecordStat(MethodCompilationStat::kNotCompiledClassNotVerified); + } + } + if (method != nullptr) { return method; } method = delegate_->Compile(code_item, access_flags, invoke_type, class_def_idx, method_idx, - class_loader, dex_file); + jclass_loader, dex_file); if (method != nullptr) { compilation_stats_.RecordStat(MethodCompilationStat::kCompiledQuick); diff --git a/compiler/optimizing/optimizing_compiler_stats.h b/compiler/optimizing/optimizing_compiler_stats.h index 4d5b8d0..d4a936d 100644 --- a/compiler/optimizing/optimizing_compiler_stats.h +++ b/compiler/optimizing/optimizing_compiler_stats.h @@ -45,6 +45,8 @@ enum MethodCompilationStat { kNotCompiledCantAccesType, kNotOptimizedRegisterAllocator, kNotCompiledUnhandledInstruction, + kNotCompiledVerifyAtRuntime, + kNotCompiledClassNotVerified, kRemovedCheckedCast, kRemovedNullCheck, kInstructionSimplifications, @@ -109,6 +111,8 @@ class OptimizingCompilerStats { case kNotCompiledSpaceFilter : return "kNotCompiledSpaceFilter"; case kNotOptimizedRegisterAllocator : return "kNotOptimizedRegisterAllocator"; case kNotCompiledUnhandledInstruction : return "kNotCompiledUnhandledInstruction"; + case kNotCompiledVerifyAtRuntime : return "kNotCompiledVerifyAtRuntime"; + case kNotCompiledClassNotVerified : return "kNotCompiledClassNotVerified"; case kRemovedCheckedCast: return "kRemovedCheckedCast"; case kRemovedNullCheck: return "kRemovedNullCheck"; case kInstructionSimplifications: return "kInstructionSimplifications"; diff --git a/test/471-uninitialized-locals/expected.txt b/test/471-uninitialized-locals/expected.txt new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/test/471-uninitialized-locals/expected.txt diff --git a/test/471-uninitialized-locals/info.txt b/test/471-uninitialized-locals/info.txt new file mode 100644 index 0000000..ebead8e --- /dev/null +++ b/test/471-uninitialized-locals/info.txt @@ -0,0 +1,2 @@ +Regression for the optimizing for crashes during compilation of methods which +use values before initializing them. diff --git a/test/471-uninitialized-locals/smali/Test.smali b/test/471-uninitialized-locals/smali/Test.smali new file mode 100644 index 0000000..17a14bf --- /dev/null +++ b/test/471-uninitialized-locals/smali/Test.smali @@ -0,0 +1,23 @@ +# +# Copyright (C) 2015 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 LTest; + +.super Ljava/lang/Object; + +.method public static ThrowException()V + .registers 1 + throw v0 +.end method diff --git a/test/471-uninitialized-locals/src/Main.java b/test/471-uninitialized-locals/src/Main.java new file mode 100644 index 0000000..a5b1c48 --- /dev/null +++ b/test/471-uninitialized-locals/src/Main.java @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2015 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. + */ + +import java.lang.reflect.Method; + +public class Main { + + // Workaround for b/18051191. + class InnerClass {} + + public static void main(String args[]) throws Exception { + try { + Class<?> c = Class.forName("Test"); + Method m = c.getMethod("ThrowException", (Class[]) null); + m.invoke(null, (Object[]) null); + } catch (VerifyError e) { + // Compilation should go fine but we expect the runtime verification to fail. + return; + } + + throw new Error("Failed to preset verification error!"); + } + +} |