ART: Reject branch/switch/throw to move-result.
move-result* instructions must occur after invoke instructions,
however it was still possible to branch or switch to a move-result*
instruction with creative DEX manipulation. The verifier now rejects
this situation, as well as having a move-result* instruction as the
first instruction in a try block's handler. This now ensures that
move-result* must happen dynamically after an invoke, not just
statically.
Change-Id: Ida97852f4051310fdaf38bed1d6e1c5a541c85c5
Signed-off-by: Stephen Kyle <stephen.kyle@arm.com>
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index f28d488..45e8b6a 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -650,6 +650,11 @@
<< "exception handler starts at bad address (" << dex_pc << ")";
return false;
}
+ if (!CheckNotMoveResult(code_item_->insns_, dex_pc)) {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD)
+ << "exception handler begins with move-result* (" << dex_pc << ")";
+ return false;
+ }
insn_flags_[dex_pc].SetBranchTarget();
// Ensure exception types are resolved so that they don't need resolution to be delivered,
// unresolved exception types will be ignored by exception delivery
@@ -2766,7 +2771,7 @@
return false;
}
DCHECK_EQ(isConditional, (opcode_flags & Instruction::kContinue) != 0);
- if (!CheckNotMoveException(code_item_->insns_, work_insn_idx_ + branch_target)) {
+ if (!CheckNotMoveExceptionOrMoveResult(code_item_->insns_, work_insn_idx_ + branch_target)) {
return false;
}
/* update branch target, set "changed" if appropriate */
@@ -2812,7 +2817,7 @@
(((int32_t) switch_insns[offset_to_targets + targ * 2 + 1]) << 16);
abs_offset = work_insn_idx_ + offset;
DCHECK_LT(abs_offset, code_item_->insns_size_in_code_units_);
- if (!CheckNotMoveException(code_item_->insns_, abs_offset)) {
+ if (!CheckNotMoveExceptionOrMoveResult(code_item_->insns_, abs_offset)) {
return false;
}
if (!UpdateRegisters(abs_offset, work_line_.get(), false)) {
@@ -4001,6 +4006,19 @@
return true;
}
+bool MethodVerifier::CheckNotMoveResult(const uint16_t* insns, int insn_idx) {
+ if (((insns[insn_idx] & 0xff) >= Instruction::MOVE_RESULT) &&
+ ((insns[insn_idx] & 0xff) <= Instruction::MOVE_RESULT_OBJECT)) {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "invalid use of move-result*";
+ return false;
+ }
+ return true;
+}
+
+bool MethodVerifier::CheckNotMoveExceptionOrMoveResult(const uint16_t* insns, int insn_idx) {
+ return (CheckNotMoveException(insns, insn_idx) && CheckNotMoveResult(insns, insn_idx));
+}
+
bool MethodVerifier::UpdateRegisters(uint32_t next_insn, RegisterLine* merge_line,
bool update_merge_line) {
bool changed = true;
diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h
index 87acb20..9f5efe8 100644
--- a/runtime/verifier/method_verifier.h
+++ b/runtime/verifier/method_verifier.h
@@ -612,6 +612,21 @@
bool CheckNotMoveException(const uint16_t* insns, int insn_idx);
/*
+ * Verify that the target instruction is not "move-result". It is important that we cannot
+ * branch to move-result instructions, but we have to make this a distinct check instead of
+ * adding it to CheckNotMoveException, because it is legal to continue into "move-result"
+ * instructions - as long as the previous instruction was an invoke, which is checked elsewhere.
+ */
+ bool CheckNotMoveResult(const uint16_t* insns, int insn_idx);
+
+ /*
+ * Verify that the target instruction is not "move-result" or "move-exception". This is to
+ * be used when checking branch and switch instructions, but not instructions that can
+ * continue.
+ */
+ bool CheckNotMoveExceptionOrMoveResult(const uint16_t* insns, int insn_idx);
+
+ /*
* Control can transfer to "next_insn". Merge the registers from merge_line into the table at
* next_insn, and set the changed flag on the target address if any of the registers were changed.
* In the case of fall-through, update the merge line on a change as its the working line for the
