Verifier cleanups.
- For apps <= S, keep the behavior of not analyzing an unreachable
handler. If >= T, we analyze it to simplify handling in the compiler.
- Remove VERIFY_ERROR_SKIP_COMPILER and fold uncompilable methods into
checking HasInstructionThatWillThrow.
Test: test.py
Bug: 28313047
Change-Id: I20b65cf50def2a4a95617a03142575b8591ae0ec
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 1b37c66..41ca523 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -752,7 +752,8 @@
// For app-compatibility, code after a runtime throw is treated as dead code
// for apps targeting <= S.
- void PotentiallyMarkRuntimeThrow() override;
+ // Returns whether the current instruction was marked as throwing.
+ bool PotentiallyMarkRuntimeThrow() override;
// Dump the failures encountered by the verifier.
std::ostream& DumpFailures(std::ostream& os) {
@@ -3773,23 +3774,16 @@
handlers_ptr = iterator.EndDataPointer();
}
if (unresolved != nullptr) {
- if (!IsAotMode() && common_super == nullptr) {
- // This is an unreachable handler.
-
- // We need to post a failure. The compiler currently does not handle unreachable
- // code correctly.
- Fail(VERIFY_ERROR_SKIP_COMPILER, /*pending_exc=*/ false)
- << "Unresolved catch handler, fail for compiler";
-
- return std::make_pair(false, unresolved);
- }
// Soft-fail, but do not handle this with a synthetic throw.
Fail(VERIFY_ERROR_UNRESOLVED_TYPE_CHECK, /*pending_exc=*/ false)
<< "Unresolved catch handler";
+ bool should_continue = true;
if (common_super != nullptr) {
unresolved = &unresolved->Merge(*common_super, ®_types_, this);
+ } else {
+ should_continue = !PotentiallyMarkRuntimeThrow();
}
- return std::make_pair(true, unresolved);
+ return std::make_pair(should_continue, unresolved);
}
}
if (common_super == nullptr) {
@@ -4982,18 +4976,25 @@
}
template <bool kVerifierDebug>
-void MethodVerifier<kVerifierDebug>::PotentiallyMarkRuntimeThrow() {
+bool MethodVerifier<kVerifierDebug>::PotentiallyMarkRuntimeThrow() {
if (IsAotMode() || IsSdkVersionSetAndAtLeast(api_level_, SdkVersion::kT)) {
- return;
+ return false;
}
- flags_.have_pending_runtime_throw_failure_ = true;
- // How to handle runtime failures for instructions that are not flagged kThrow.
- //
+ // Compatibility mode: we treat the following code unreachable and the verifier
+ // will not analyze it.
// The verifier may fail before we touch any instruction, for the signature of a method. So
// add a check.
if (work_insn_idx_ < dex::kDexNoIndex) {
const Instruction& inst = code_item_accessor_.InstructionAt(work_insn_idx_);
Instruction::Code opcode = inst.Opcode();
+ if (opcode == Instruction::MOVE_EXCEPTION) {
+ // This is an unreachable handler. The instruction doesn't throw, but we
+ // mark the method as having a pending runtime throw failure so that
+ // the compiler does not try to compile it.
+ flags_.have_any_pending_runtime_throw_failure_ = true;
+ return true;
+ }
+ // How to handle runtime failures for instructions that are not flagged kThrow.
if ((Instruction::FlagsOf(opcode) & Instruction::kThrow) == 0 &&
!impl::IsCompatThrow(opcode) &&
GetInstructionFlags(work_insn_idx_).IsInTry()) {
@@ -5007,6 +5008,8 @@
saved_line_->CopyFromLine(work_line_.get());
}
}
+ flags_.have_pending_runtime_throw_failure_ = true;
+ return true;
}
} // namespace
@@ -5205,7 +5208,8 @@
} else {
result.kind = FailureKind::kSoftFailure;
}
- if (!CanCompilerHandleVerificationFailure(verifier.encountered_failure_types_)) {
+ if (!CanCompilerHandleVerificationFailure(verifier.encountered_failure_types_) ||
+ verifier.HasInstructionThatWillThrow()) {
set_dont_compile = true;
}
if ((verifier.encountered_failure_types_ & VerifyError::VERIFY_ERROR_LOCKING) != 0) {
@@ -5498,10 +5502,6 @@
flags_.have_pending_hard_failure_ = true;
break;
}
-
- case VERIFY_ERROR_SKIP_COMPILER:
- // Nothing to do, just remember the failure type.
- break;
}
} else if (kIsDebugBuild) {
CHECK_NE(error, VERIFY_ERROR_BAD_CLASS_SOFT);
diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h
index 32ce065..1849f6b 100644
--- a/runtime/verifier/method_verifier.h
+++ b/runtime/verifier/method_verifier.h
@@ -324,7 +324,7 @@
uint32_t api_level)
REQUIRES_SHARED(Locks::mutator_lock_);
- virtual void PotentiallyMarkRuntimeThrow() = 0;
+ virtual bool PotentiallyMarkRuntimeThrow() = 0;
// The thread we're verifying on.
Thread* const self_;
diff --git a/runtime/verifier/verifier_enums.h b/runtime/verifier/verifier_enums.h
index 8c0d739..6dc8e29 100644
--- a/runtime/verifier/verifier_enums.h
+++ b/runtime/verifier/verifier_enums.h
@@ -89,11 +89,6 @@
VERIFY_ERROR_INSTANTIATION = 1 << 9, // InstantiationError.
VERIFY_ERROR_LOCKING = 1 << 10, // Could not guarantee balanced locking. This should be
// punted to the interpreter with access checks.
- VERIFY_ERROR_SKIP_COMPILER = 1u << 31, // Flag to note that the failure should preclude
- // optimization. Meant as a signal from the verifier
- // to the compiler that there is unreachable unverified
- // code. May be removed once the compiler handles
- // unreachable code correctly.
};
std::ostream& operator<<(std::ostream& os, VerifyError rhs);
diff --git a/test/800-smali/run b/test/800-smali/run
new file mode 100644
index 0000000..c8ce0bc
--- /dev/null
+++ b/test/800-smali/run
@@ -0,0 +1,18 @@
+#!/bin/bash
+#
+# Copyright 2021 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.
+
+# Target 31 to have the verifier behavior the test expects.
+./default-run "$@" --runtime-option -Xtarget-sdk-version:31