Mark methods of unverified classes as uncompilable.
We were forgetting to mark them during FastVerify.
Bug: 247097413
Test: 842-vdex-hard-failure
Change-Id: I4e24e1c64f6a50f87eb4ce1d85face1619169915
(cherry picked from commit 57ff705fa56e66ceab13ad1ab534ff708dafad62)
Merged-In: I4e24e1c64f6a50f87eb4ce1d85face1619169915
diff --git a/compiler/dex/verification_results.cc b/compiler/dex/verification_results.cc
index b819d0e..bc0329a 100644
--- a/compiler/dex/verification_results.cc
+++ b/compiler/dex/verification_results.cc
@@ -20,6 +20,7 @@
#include "base/mutex-inl.h"
#include "base/stl_util.h"
+#include "dex/class_accessor-inl.h"
#include "runtime.h"
#include "thread-current-inl.h"
#include "thread.h"
@@ -55,6 +56,17 @@
DCHECK(IsUncompilableMethod(ref));
}
+void VerificationResults::AddUncompilableClass(ClassReference ref) {
+ const DexFile& dex_file = *ref.dex_file;
+ const dex::ClassDef& class_def = dex_file.GetClassDef(ref.ClassDefIdx());
+ WriterMutexLock mu(Thread::Current(), uncompilable_methods_lock_);
+ ClassAccessor accessor(dex_file, class_def);
+ for (const ClassAccessor::Method& method : accessor.GetMethods()) {
+ MethodReference method_ref(&dex_file, method.GetIndex());
+ uncompilable_methods_.insert(method_ref);
+ }
+}
+
bool VerificationResults::IsUncompilableMethod(MethodReference ref) const {
ReaderMutexLock mu(Thread::Current(), uncompilable_methods_lock_);
return uncompilable_methods_.find(ref) != uncompilable_methods_.end();
diff --git a/compiler/dex/verification_results.h b/compiler/dex/verification_results.h
index b294ed3..7c5595e 100644
--- a/compiler/dex/verification_results.h
+++ b/compiler/dex/verification_results.h
@@ -39,6 +39,7 @@
void AddRejectedClass(ClassReference ref) REQUIRES(!rejected_classes_lock_);
bool IsClassRejected(ClassReference ref) const REQUIRES(!rejected_classes_lock_);
+ void AddUncompilableClass(ClassReference ref) REQUIRES(!uncompilable_methods_lock_);
void AddUncompilableMethod(MethodReference ref) REQUIRES(!uncompilable_methods_lock_);
bool IsUncompilableMethod(MethodReference ref) const REQUIRES(!uncompilable_methods_lock_);
diff --git a/dex2oat/dex/quick_compiler_callbacks.cc b/dex2oat/dex/quick_compiler_callbacks.cc
index 11223ff..7611374 100644
--- a/dex2oat/dex/quick_compiler_callbacks.cc
+++ b/dex2oat/dex/quick_compiler_callbacks.cc
@@ -28,6 +28,12 @@
}
}
+void QuickCompilerCallbacks::AddUncompilableClass(ClassReference ref) {
+ if (verification_results_ != nullptr) {
+ verification_results_->AddUncompilableClass(ref);
+ }
+}
+
void QuickCompilerCallbacks::ClassRejected(ClassReference ref) {
if (verification_results_ != nullptr) {
verification_results_->AddRejectedClass(ref);
diff --git a/dex2oat/dex/quick_compiler_callbacks.h b/dex2oat/dex/quick_compiler_callbacks.h
index 4447e02..a7a482b 100644
--- a/dex2oat/dex/quick_compiler_callbacks.h
+++ b/dex2oat/dex/quick_compiler_callbacks.h
@@ -34,6 +34,7 @@
~QuickCompilerCallbacks() { }
void AddUncompilableMethod(MethodReference ref) override;
+ void AddUncompilableClass(ClassReference ref) override;
void ClassRejected(ClassReference ref) override;
diff --git a/dex2oat/driver/compiler_driver.cc b/dex2oat/driver/compiler_driver.cc
index 5a72178..862062f 100644
--- a/dex2oat/driver/compiler_driver.cc
+++ b/dex2oat/driver/compiler_driver.cc
@@ -1654,8 +1654,8 @@
bool CompilerDriver::FastVerify(jobject jclass_loader,
const std::vector<const DexFile*>& dex_files,
TimingLogger* timings) {
- verifier::VerifierDeps* verifier_deps =
- Runtime::Current()->GetCompilerCallbacks()->GetVerifierDeps();
+ CompilerCallbacks* callbacks = Runtime::Current()->GetCompilerCallbacks();
+ verifier::VerifierDeps* verifier_deps = callbacks->GetVerifierDeps();
// If there exist VerifierDeps that aren't the ones we just created to output, use them to verify.
if (verifier_deps == nullptr || verifier_deps->OutputOnly()) {
return false;
@@ -1691,28 +1691,32 @@
const std::vector<bool>& verified_classes = verifier_deps->GetVerifiedClasses(*dex_file);
DCHECK_EQ(verified_classes.size(), dex_file->NumClassDefs());
for (ClassAccessor accessor : dex_file->GetClasses()) {
- if (verified_classes[accessor.GetClassDefIndex()]) {
- if (compiler_only_verifies) {
- // Just update the compiled_classes_ map. The compiler doesn't need to resolve
- // the type.
- ClassReference ref(dex_file, accessor.GetClassDefIndex());
- const ClassStatus existing = ClassStatus::kNotReady;
- ClassStateTable::InsertResult result =
- compiled_classes_.Insert(ref, existing, ClassStatus::kVerifiedNeedsAccessChecks);
- CHECK_EQ(result, ClassStateTable::kInsertResultSuccess) << ref.dex_file->GetLocation();
- } else {
- // Update the class status, so later compilation stages know they don't need to verify
- // the class.
- LoadAndUpdateStatus(
- accessor, ClassStatus::kVerifiedNeedsAccessChecks, class_loader, soa.Self());
- }
- } else if (!compiler_only_verifies) {
- // Make sure later compilation stages know they should not try to verify
- // this class again.
- LoadAndUpdateStatus(accessor,
- ClassStatus::kRetryVerificationAtRuntime,
- class_loader,
- soa.Self());
+ ClassStatus status = verified_classes[accessor.GetClassDefIndex()]
+ ? ClassStatus::kVerifiedNeedsAccessChecks
+ : ClassStatus::kRetryVerificationAtRuntime;
+ if (compiler_only_verifies) {
+ // Just update the compiled_classes_ map. The compiler doesn't need to resolve
+ // the type.
+ ClassReference ref(dex_file, accessor.GetClassDefIndex());
+ const ClassStatus existing = ClassStatus::kNotReady;
+ ClassStateTable::InsertResult result = compiled_classes_.Insert(ref, existing, status);
+ CHECK_EQ(result, ClassStateTable::kInsertResultSuccess) << ref.dex_file->GetLocation();
+ } else {
+ // Update the class status, so later compilation stages know they don't need to verify
+ // the class.
+ LoadAndUpdateStatus(accessor, status, class_loader, soa.Self());
+ }
+
+ // Vdex marks class as unverified for two reasons only:
+ // 1. It has a hard failure, or
+ // 2. Once of its method needs lock counting.
+ //
+ // The optimizing compiler expects a method to not have a hard failure before
+ // compiling it, so for simplicity just disable any compilation of methods
+ // of these classes.
+ if (status == ClassStatus::kRetryVerificationAtRuntime) {
+ ClassReference ref(dex_file, accessor.GetClassDefIndex());
+ callbacks->AddUncompilableClass(ref);
}
}
}
diff --git a/dex2oat/verifier_deps_test.cc b/dex2oat/verifier_deps_test.cc
index 27eadec..6a2deba 100644
--- a/dex2oat/verifier_deps_test.cc
+++ b/dex2oat/verifier_deps_test.cc
@@ -47,6 +47,7 @@
deps_(nullptr) {}
void AddUncompilableMethod(MethodReference ref ATTRIBUTE_UNUSED) override {}
+ void AddUncompilableClass(ClassReference ref ATTRIBUTE_UNUSED) override {}
void ClassRejected(ClassReference ref ATTRIBUTE_UNUSED) override {}
verifier::VerifierDeps* GetVerifierDeps() const override { return deps_; }
diff --git a/runtime/compiler_callbacks.h b/runtime/compiler_callbacks.h
index c71d4ac..f76ee66 100644
--- a/runtime/compiler_callbacks.h
+++ b/runtime/compiler_callbacks.h
@@ -48,6 +48,7 @@
virtual ~CompilerCallbacks() { }
virtual void AddUncompilableMethod(MethodReference ref) = 0;
+ virtual void AddUncompilableClass(ClassReference ref) = 0;
virtual void ClassRejected(ClassReference ref) = 0;
virtual verifier::VerifierDeps* GetVerifierDeps() const = 0;
diff --git a/runtime/noop_compiler_callbacks.h b/runtime/noop_compiler_callbacks.h
index f415831..aed0014 100644
--- a/runtime/noop_compiler_callbacks.h
+++ b/runtime/noop_compiler_callbacks.h
@@ -27,6 +27,7 @@
~NoopCompilerCallbacks() {}
void AddUncompilableMethod(MethodReference ref ATTRIBUTE_UNUSED) override {}
+ void AddUncompilableClass(ClassReference ref ATTRIBUTE_UNUSED) override {}
void ClassRejected(ClassReference ref ATTRIBUTE_UNUSED) override {}
verifier::VerifierDeps* GetVerifierDeps() const override { return nullptr; }
diff --git a/test/842-vdex-hard-failure/expected-stderr.txt b/test/842-vdex-hard-failure/expected-stderr.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/842-vdex-hard-failure/expected-stderr.txt
diff --git a/test/842-vdex-hard-failure/expected-stdout.txt b/test/842-vdex-hard-failure/expected-stdout.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/842-vdex-hard-failure/expected-stdout.txt
diff --git a/test/842-vdex-hard-failure/info.txt b/test/842-vdex-hard-failure/info.txt
new file mode 100644
index 0000000..21412c7
--- /dev/null
+++ b/test/842-vdex-hard-failure/info.txt
@@ -0,0 +1,2 @@
+Regression test for vdex, where we were compiling methods that had hard
+failures.
diff --git a/test/842-vdex-hard-failure/run b/test/842-vdex-hard-failure/run
new file mode 100644
index 0000000..4a4ee37
--- /dev/null
+++ b/test/842-vdex-hard-failure/run
@@ -0,0 +1,18 @@
+#!/bin/bash
+#
+# Copyright (C) 2022 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.
+
+# This test is for testing vdex when calling FastVerify and doing compilation.
+exec ${RUN} --vdex --vdex-filter speed "${@}"
diff --git a/test/842-vdex-hard-failure/smali/HardFail.smali b/test/842-vdex-hard-failure/smali/HardFail.smali
new file mode 100644
index 0000000..52aefe0
--- /dev/null
+++ b/test/842-vdex-hard-failure/smali/HardFail.smali
@@ -0,0 +1,26 @@
+# /*
+# * Copyright (C) 2022 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 LHardFail;
+
+.super Ljava/lang/Object;
+
+.method public static foo()V
+ .locals 1
+ # smali requires at least one instruction
+ new-instance v0, Ljava/lang/Object;
+ # No return on purprose to hard fail the class and crash the compiler.
+.end method
diff --git a/test/842-vdex-hard-failure/src/Main.java b/test/842-vdex-hard-failure/src/Main.java
new file mode 100644
index 0000000..c6f1f68
--- /dev/null
+++ b/test/842-vdex-hard-failure/src/Main.java
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2022 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.
+ */
+
+public class Main {
+
+ public static void main(String[] args) throws ClassNotFoundException {
+ try {
+ Class.forName("HardFail");
+ throw new Error("Expected VerifyError");
+ } catch (VerifyError e) {
+ // expected
+ }
+ }
+}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 3e485f6..5efb09d 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1030,6 +1030,7 @@
"816-illegal-new-array",
"819-verification-runtime",
"823-cha-inlining",
+ "842-vdex-hard-failure",
"900-hello-plugin",
"901-hello-ti-agent",
"903-hello-tagging",