summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--dex2oat/dex2oat_test.cc9
-rw-r--r--libdexfile/dex/dex_instruction.cc7
-rw-r--r--libdexfile/dex/dex_instruction_iterator.h77
-rw-r--r--runtime/verifier/method_verifier.cc84
4 files changed, 73 insertions, 104 deletions
diff --git a/dex2oat/dex2oat_test.cc b/dex2oat/dex2oat_test.cc
index f81345b345..7013bbce0c 100644
--- a/dex2oat/dex2oat_test.cc
+++ b/dex2oat/dex2oat_test.cc
@@ -1816,13 +1816,6 @@ TEST_F(Dex2oatTest, AppImageResolveStrings) {
const_cast<Instruction&>(last_instruction.Inst())
.SetOpcode(Instruction::CONST_STRING_JUMBO);
mutated_successfully = true;
- // Test that the safe iterator doesn't go past the end.
- SafeDexInstructionIterator it2(instructions.begin(), instructions.end());
- while (!it2.IsErrorState()) {
- ++it2;
- }
- EXPECT_TRUE(it2 == last_instruction);
- EXPECT_TRUE(it2 < instructions.end());
methods.push_back(method.GetIndex());
mutated_successfully = true;
} else if (method_name == "startUpMethod") {
@@ -1890,8 +1883,6 @@ TEST_F(Dex2oatTest, AppImageResolveStrings) {
// Classes initializers
EXPECT_TRUE(seen.find("Startup init") != seen.end());
EXPECT_TRUE(seen.find("Other class init") == seen.end());
- // Expect the sets match.
- EXPECT_GE(seen.size(), seen.size());
// Verify what strings are marked as boot image.
std::set<std::string> boot_image_strings;
diff --git a/libdexfile/dex/dex_instruction.cc b/libdexfile/dex/dex_instruction.cc
index 67147bba11..c4db658ae1 100644
--- a/libdexfile/dex/dex_instruction.cc
+++ b/libdexfile/dex/dex_instruction.cc
@@ -74,7 +74,12 @@ size_t Instruction::SizeInCodeUnitsComplexOpcode() const {
uint16_t element_size = insns[1];
uint32_t length = insns[2] | (((uint32_t)insns[3]) << 16);
// The plus 1 is to round up for odd size and width.
- return (4 + (element_size * length + 1) / 2);
+ uint32_t result = (4 + (element_size * length + 1) / 2);
+ // This function is used only after the `MethodVerifier` checked that the 32-bit calculation
+ // does not overflow. Let's `DCHECK()` the result against a 64-bit calculation.
+ DCHECK_EQ(result,
+ 4 + (static_cast<uint64_t>(element_size) * static_cast<uint64_t>(length) + 1) / 2);
+ return result;
}
default:
if ((*insns & 0xFF) == 0) {
diff --git a/libdexfile/dex/dex_instruction_iterator.h b/libdexfile/dex/dex_instruction_iterator.h
index da494e1f08..9f4e39030c 100644
--- a/libdexfile/dex/dex_instruction_iterator.h
+++ b/libdexfile/dex/dex_instruction_iterator.h
@@ -156,83 +156,6 @@ class DexInstructionIterator : public DexInstructionIteratorBase {
}
};
-// A safe version of DexInstructionIterator that is guaranteed to not go past the end of the code
-// item.
-class SafeDexInstructionIterator : public DexInstructionIteratorBase {
- public:
- explicit SafeDexInstructionIterator(const DexInstructionIteratorBase& start,
- const DexInstructionIteratorBase& end)
- : DexInstructionIteratorBase(&start.Inst(), start.DexPc())
- , num_code_units_(end.DexPc()) {
- DCHECK_EQ(start.Instructions(), end.Instructions())
- << "start and end must be in the same code item.";
- }
-
- // Value after modification, does not read past the end of the allowed region. May increment past
- // the end of the code item though.
- SafeDexInstructionIterator& operator++() {
- AssertValid();
- const size_t size_code_units = Inst().CodeUnitsRequiredForSizeComputation();
- const size_t available = NumCodeUnits() - DexPc();
- if (UNLIKELY(size_code_units > available)) {
- error_state_ = true;
- return *this;
- }
- const size_t instruction_code_units = Inst().SizeInCodeUnits();
- if (UNLIKELY(instruction_code_units > available)) {
- error_state_ = true;
- return *this;
- }
- data_.dex_pc_ += instruction_code_units;
- return *this;
- }
-
- // Value before modification.
- SafeDexInstructionIterator operator++(int) {
- SafeDexInstructionIterator temp = *this;
- ++*this;
- return temp;
- }
-
- const value_type& operator*() const {
- AssertValid();
- return data_;
- }
-
- const Instruction* operator->() const {
- AssertValid();
- return &data_.Inst();
- }
-
- // Return the current instruction of the iterator.
- ALWAYS_INLINE const Instruction& Inst() const {
- return data_.Inst();
- }
-
- const uint16_t* Instructions() const {
- return data_.Instructions();
- }
-
- // Returns true if the iterator is in an error state. This occurs when an instruction couldn't
- // have its size computed without reading past the end iterator.
- bool IsErrorState() const {
- return error_state_;
- }
-
- private:
- ALWAYS_INLINE void AssertValid() const {
- DCHECK(!IsErrorState());
- DCHECK_LT(DexPc(), NumCodeUnits());
- }
-
- ALWAYS_INLINE uint32_t NumCodeUnits() const {
- return num_code_units_;
- }
-
- const uint32_t num_code_units_ = 0;
- bool error_state_ = false;
-};
-
} // namespace art
#endif // ART_LIBDEXFILE_DEX_DEX_INSTRUCTION_ITERATOR_H_
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 68c0268720..42f694c62f 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -1619,29 +1619,79 @@ template <bool kVerifierDebug>
bool MethodVerifier<kVerifierDebug>::ComputeWidthsAndCountOps() {
// We can't assume the instruction is well formed, handle the case where calculating the size
// goes past the end of the code item.
- SafeDexInstructionIterator it(code_item_accessor_.begin(), code_item_accessor_.end());
- if (it == code_item_accessor_.end()) {
+ const uint32_t insns_size = code_item_accessor_.InsnsSizeInCodeUnits();
+ if (insns_size == 0u) {
Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "code item has no opcode";
return false;
}
- for ( ; !it.IsErrorState() && it < code_item_accessor_.end(); ++it) {
- // In case the instruction goes past the end of the code item, make sure to not process it.
- SafeDexInstructionIterator next = it;
- ++next;
- if (next.IsErrorState()) {
- break;
+ const Instruction* inst = &code_item_accessor_.InstructionAt(0u);
+ uint32_t dex_pc = 0u;
+ while (dex_pc != insns_size) {
+ const uint32_t remaining_code_units = insns_size - dex_pc;
+ const uint16_t inst_data = inst->Fetch16(0);
+ const Instruction::Code opcode = inst->Opcode(inst_data);
+ uint32_t instruction_size = 0u;
+ bool ok;
+ if (opcode == Instruction::NOP) {
+ auto check_switch = [&](uint32_t base_size, uint32_t entry_size) ALWAYS_INLINE {
+ if (UNLIKELY(base_size > remaining_code_units)) {
+ return false;
+ }
+ // This 32-bit calculation cannot overflow because `num_entries` starts as 16-bit.
+ uint32_t num_entries = inst->Fetch16(1);
+ instruction_size = base_size + num_entries * entry_size;
+ if (UNLIKELY(instruction_size > remaining_code_units)) {
+ return false;
+ }
+ return true;
+ };
+ switch (inst_data) {
+ case Instruction::kPackedSwitchSignature:
+ ok = check_switch(4u, 2u);
+ break;
+ case Instruction::kSparseSwitchSignature:
+ ok = check_switch(2u, 4u);
+ break;
+ case Instruction::kArrayDataSignature:
+ if (UNLIKELY(remaining_code_units < 4u)) {
+ ok = false;
+ } else {
+ uint16_t element_size = inst->Fetch16(1);
+ uint32_t length = inst->Fetch16(2) | (((uint32_t)inst->Fetch16(3)) << 16);
+ // Use 64-bit calculation to avoid arithmetic overflow.
+ uint64_t bytes = static_cast<uint64_t>(element_size) * static_cast<uint64_t>(length);
+ uint64_t code_units = UINT64_C(4) + (bytes + /* round up */ UINT64_C(1)) / UINT64_C(2);
+ if (UNLIKELY(code_units > remaining_code_units)) {
+ ok = false;
+ } else {
+ instruction_size = dchecked_integral_cast<uint32_t>(code_units);
+ ok = true;
+ }
+ }
+ break;
+ default:
+ instruction_size = 1u;
+ ok = true;
+ break;
+ }
+ } else {
+ instruction_size = Instruction::SizeInCodeUnits(Instruction::FormatOf(opcode));
+ DCHECK_EQ(instruction_size, inst->SizeInCodeUnits());
+ ok = LIKELY(instruction_size <= remaining_code_units);
}
- GetModifiableInstructionFlags(it.DexPc()).SetIsOpcode();
- }
-
- if (it != code_item_accessor_.end()) {
- const size_t insns_size = code_item_accessor_.InsnsSizeInCodeUnits();
- Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "code did not end where expected ("
- << it.DexPc() << " vs. " << insns_size << ")";
- return false;
+ if (!ok) {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "code did not end where expected ("
+ << dex_pc << " vs. " << insns_size << ")";
+ return false;
+ }
+ GetModifiableInstructionFlags(dex_pc).SetIsOpcode();
+ DCHECK_NE(instruction_size, 0u);
+ DCHECK_EQ(instruction_size, inst->SizeInCodeUnits());
+ DCHECK_LE(instruction_size, remaining_code_units);
+ dex_pc += instruction_size;
+ inst = inst->RelativeAt(instruction_size);
}
DCHECK(GetInstructionFlags(0).IsOpcode());
-
return true;
}