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
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 @@
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 @@
// 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 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
kNotCompiledCantAccesType,
kNotOptimizedRegisterAllocator,
kNotCompiledUnhandledInstruction,
+ kNotCompiledVerifyAtRuntime,
+ kNotCompiledClassNotVerified,
kRemovedCheckedCast,
kRemovedNullCheck,
kInstructionSimplifications,
@@ -109,6 +111,8 @@
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!");
+ }
+
+}