Don't assume the class has never been looked at in FastVerify.
When we compile dex files individually and one FastVerify fails, we may
verify more classes that in the one dex file.
Also fix a bug in VerifierDeps when verifying after failing fast
verification: we need to clear the existing contents to repopulate it.
Bug: 257335641
Test: 845-fast-verify
Change-Id: I92f64daff1653ce3d58cee81ad3b461fa152cb84
diff --git a/dex2oat/driver/compiler_driver.cc b/dex2oat/driver/compiler_driver.cc
index 31ba701..78fdc31 100644
--- a/dex2oat/driver/compiler_driver.cc
+++ b/dex2oat/driver/compiler_driver.cc
@@ -1738,6 +1738,9 @@
class_loader,
dex_files,
&error_msg)) {
+ // Clear the information we have as we are going to re-verify and we do not
+ // want to keep that a class is verified.
+ verifier_deps->ClearData(dex_files);
LOG(WARNING) << "Fast verification failed: " << error_msg;
return false;
}
@@ -1764,8 +1767,10 @@
// 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();
+ // Note: when dex files are compiled inidividually, the class may have
+ // been verified in a previous stage. This means this insertion can
+ // fail, but that's OK.
+ compiled_classes_.Insert(ref, existing, status);
} else {
// Update the class status, so later compilation stages know they don't need to verify
// the class.
diff --git a/runtime/verifier/verifier_deps.cc b/runtime/verifier/verifier_deps.cc
index db5fa0f..781cf7b 100644
--- a/runtime/verifier/verifier_deps.cc
+++ b/runtime/verifier/verifier_deps.cc
@@ -760,6 +760,18 @@
return true;
}
+void VerifierDeps::ClearData(const std::vector<const DexFile*>& dex_files) {
+ for (const DexFile* dex_file : dex_files) {
+ auto it = dex_deps_.find(dex_file);
+ if (it == dex_deps_.end()) {
+ continue;
+ }
+ std::unique_ptr<DexFileDeps> deps(new DexFileDeps(dex_file->NumClassDefs()));
+ it->second.swap(deps);
+ }
+}
+
+
bool VerifierDeps::VerifyDexFile(Handle<mirror::ClassLoader> class_loader,
const DexFile& dex_file,
const DexFileDeps& deps,
diff --git a/runtime/verifier/verifier_deps.h b/runtime/verifier/verifier_deps.h
index 8e3b8a7..46b3554 100644
--- a/runtime/verifier/verifier_deps.h
+++ b/runtime/verifier/verifier_deps.h
@@ -137,6 +137,9 @@
return GetDexFileDeps(dex_file) != nullptr;
}
+ // Resets the data related to the given dex files.
+ void ClearData(const std::vector<const DexFile*>& dex_files);
+
// Parses raw VerifierDeps data to extract bitvectors of which class def indices
// were verified or not. The given `dex_files` must match the order and count of
// dex files used to create the VerifierDeps.
diff --git a/test/845-fast-verify/845-fast-verify.jar b/test/845-fast-verify/845-fast-verify.jar
new file mode 100644
index 0000000..d94777a
--- /dev/null
+++ b/test/845-fast-verify/845-fast-verify.jar
Binary files differ
diff --git a/test/845-fast-verify/build.py b/test/845-fast-verify/build.py
new file mode 100644
index 0000000..f304d95
--- /dev/null
+++ b/test/845-fast-verify/build.py
@@ -0,0 +1,18 @@
+#
+# 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.
+
+
+def build(ctx):
+ pass # Nothing to do.
diff --git a/test/845-fast-verify/classes.dm b/test/845-fast-verify/classes.dm
new file mode 100644
index 0000000..c2c8559
--- /dev/null
+++ b/test/845-fast-verify/classes.dm
Binary files differ
diff --git a/test/845-fast-verify/expected-stderr.txt b/test/845-fast-verify/expected-stderr.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/845-fast-verify/expected-stderr.txt
diff --git a/test/845-fast-verify/expected-stdout.txt b/test/845-fast-verify/expected-stdout.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/845-fast-verify/expected-stdout.txt
diff --git a/test/845-fast-verify/info.txt b/test/845-fast-verify/info.txt
new file mode 100644
index 0000000..678ec3f
--- /dev/null
+++ b/test/845-fast-verify/info.txt
@@ -0,0 +1,46 @@
+Regression test for the combination of dex2oat using:
+- jar with multidex
+- vdex file where one dex file fails to fast verify (for example because of a
+ boot classpath change)
+- dex files being compiled individually
+
+We used to crash in CompilerDriver::FastVerify, assuming that only FastVerify
+can update the compiled_classes_ map. However, this isn't the case if one of the
+dex file ended up needing full verification.
+
+We need prebuilts of the .jar and .dm file as we rely on the bootclasspath to
+change which isn't expressable in a run-test. So we locally modified
+android.system.Int32Ref to inherit java.util.HashMap.
+
+The code that was used to generate the prebuilts is as follows:
+
+
+file Main.java in classes.dex:
+
+import java.util.HashMap;
+import android.system.Int32Ref;
+
+public class Main {
+ public static void main(String[] args) throws Exception {
+ try {
+ FailVerification.foo();
+ throw new Exception("Expected error");
+ } catch (Error expected) {
+ }
+ }
+}
+
+class FailVerification extends Foo {
+
+ public static void foo() {
+ Int32Ref ref = new Int32Ref(42);
+ takeHashMap(ref);
+ }
+
+ public static void takeHashMap(HashMap m) {}
+}
+
+file Foo.java in classes2.dex:
+
+public class Foo {
+}
diff --git a/test/845-fast-verify/run.py b/test/845-fast-verify/run.py
new file mode 100644
index 0000000..f27ec80
--- /dev/null
+++ b/test/845-fast-verify/run.py
@@ -0,0 +1,25 @@
+#!/bin/bash
+#
+# Copyright 2016 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.
+
+
+def run(ctx, args):
+ ctx.default_run(
+ args,
+ # Disable app image to make sure we compile dex files individually.
+ app_image=False,
+ # Pass a .dm file to run FastVerify and ask to compile dex files
+ # individually in order to run the problematic code.
+ Xcompiler_option=[f"--dm-file={ctx.env.DEX_LOCATION}/classes.dm", "--compile-individually"])
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 82b3a3d..bbd20e9 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1166,6 +1166,7 @@
"837-deopt",
"844-exception",
"844-exception2",
+ "845-fast-verify",
"999-redefine-hiddenapi",
"1000-non-moving-space-stress",
"1001-app-image-regions",
diff --git a/test/run_test_build.py b/test/run_test_build.py
index 8a34e21..f2656f0 100755
--- a/test/run_test_build.py
+++ b/test/run_test_build.py
@@ -335,6 +335,10 @@
zip(TEST_NAME + ".jar", "classes.dex")
return
+ if path.exists("classes.dm"):
+ zip(TEST_NAME + ".jar", "classes.dm")
+ return
+
def has_multidex():
return HAS_SRC_MULTIDEX or HAS_JASMIN_MULTIDEX or HAS_SMALI_MULTIDEX