From 53de99cd7e863e95179823504335f1f67e03c791 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Mon, 17 Aug 2015 13:43:55 -0700 Subject: [PATCH] ART: Follow-up fixes Addressing comments for CL 166499, commit 5073fedd553afeb6ccdb49c1a1ab2cc2947c0870. Change-Id: I359e5a4c026d58d75cb62b90c495796855302b94 --- runtime/utils.cc | 27 +++++++++++++++------------ runtime/verifier/method_verifier.cc | 27 +++++++++++++++------------ 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/runtime/utils.cc b/runtime/utils.cc index db3f2fecb..8aa1189a9 100644 --- a/runtime/utils.cc +++ b/runtime/utils.cc @@ -1471,7 +1471,7 @@ static void DumpMethodCFGImpl(const DexFile* dex_file, dex_pc_is_branch_target.insert(dex_pc + inst->GetTargetOffset()); } else if (inst->IsSwitch()) { const uint16_t* insns = code_item->insns_ + dex_pc; - int32_t switch_offset = insns[1] | ((int32_t) insns[2]) << 16; + int32_t switch_offset = insns[1] | (static_cast(insns[2]) << 16); const uint16_t* switch_insns = insns + switch_offset; uint32_t switch_count = switch_insns[1]; int32_t targets_offset; @@ -1483,8 +1483,9 @@ static void DumpMethodCFGImpl(const DexFile* dex_file, targets_offset = 2 + 2 * switch_count; } for (uint32_t targ = 0; targ < switch_count; targ++) { - int32_t offset = (int32_t) switch_insns[targets_offset + targ * 2] | - (int32_t) (switch_insns[targets_offset + targ * 2 + 1] << 16); + int32_t offset = + static_cast(switch_insns[targets_offset + targ * 2]) | + static_cast(switch_insns[targets_offset + targ * 2 + 1] << 16); dex_pc_is_branch_target.insert(dex_pc + offset); } } @@ -1499,8 +1500,9 @@ static void DumpMethodCFGImpl(const DexFile* dex_file, const Instruction* inst = Instruction::At(code_item->insns_); bool first_in_block = true; bool force_new_block = false; - for (uint32_t dex_pc = 0; dex_pc < code_item->insns_size_in_code_units_; - dex_pc += inst->SizeInCodeUnits(), inst = inst->Next()) { + for (uint32_t dex_pc = 0; + dex_pc < code_item->insns_size_in_code_units_; + dex_pc += inst->SizeInCodeUnits(), inst = inst->Next()) { if (dex_pc == 0 || (dex_pc_is_branch_target.find(dex_pc) != dex_pc_is_branch_target.end()) || force_new_block) { @@ -1531,7 +1533,7 @@ static void DumpMethodCFGImpl(const DexFile* dex_file, os << " 0x" << std::hex << dex_pc << std::dec << ": "; std::string inst_str = inst->DumpString(dex_file); size_t cur_start = 0; // It's OK to start at zero, instruction dumps don't start with chars - // we need to escape. + // we need to escape. while (cur_start != std::string::npos) { size_t next_escape = inst_str.find_first_of("\"{}<>", cur_start + 1); if (next_escape == std::string::npos) { @@ -1645,7 +1647,7 @@ static void DumpMethodCFGImpl(const DexFile* dex_file, // TODO: Iterate through all switch targets. const uint16_t* insns = code_item->insns_ + dex_pc; /* make sure the start of the switch is in range */ - int32_t switch_offset = insns[1] | ((int32_t) insns[2]) << 16; + int32_t switch_offset = insns[1] | (static_cast(insns[2]) << 16); /* offset to switch table is a relative branch-style offset */ const uint16_t* switch_insns = insns + switch_offset; uint32_t switch_count = switch_insns[1]; @@ -1660,8 +1662,9 @@ static void DumpMethodCFGImpl(const DexFile* dex_file, /* make sure the end of the switch is in range */ /* verify each switch target */ for (uint32_t targ = 0; targ < switch_count; targ++) { - int32_t offset = (int32_t) switch_insns[targets_offset + targ * 2] | - (int32_t) (switch_insns[targets_offset + targ * 2 + 1] << 16); + int32_t offset = + static_cast(switch_insns[targets_offset + targ * 2]) | + static_cast(switch_insns[targets_offset + targ * 2 + 1] << 16); int32_t abs_offset = dex_pc + offset; auto target_it = dex_pc_to_node_id.find(abs_offset); if (target_it != dex_pc_to_node_id.end()) { @@ -1715,7 +1718,7 @@ static void DumpMethodCFGImpl(const DexFile* dex_file, for (uint32_t dex_pc : blocks_with_detailed_exceptions) { const Instruction* inst = Instruction::At(&code_item->insns_[dex_pc]); uint32_t this_node_id = dex_pc_to_incl_id.find(dex_pc)->second; - for (;;) { + while (true) { CatchHandlerIterator catch_it(*code_item, dex_pc); if (catch_it.HasNext()) { std::set handled_targets; @@ -1739,8 +1742,8 @@ static void DumpMethodCFGImpl(const DexFile* dex_file, break; } - // Loop update. In the body to have a late break-out if the next instruction is a branch - // target and thus in another block. + // Loop update. Have a break-out if the next instruction is a branch target and thus in + // another block. dex_pc += inst->SizeInCodeUnits(); if (dex_pc >= code_item->insns_size_in_code_units_) { break; diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index 7c6add1ea..1828b91e2 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -1043,8 +1043,8 @@ bool MethodVerifier::CheckArrayData(uint32_t cur_offset) { DCHECK_LT(cur_offset, insn_count); /* make sure the start of the array data table is in range */ - array_data_offset = insns[1] | (((int32_t) insns[2]) << 16); - if ((int32_t) cur_offset + array_data_offset < 0 || + array_data_offset = insns[1] | (static_cast(insns[2]) << 16); + if (static_cast(cur_offset) + array_data_offset < 0 || cur_offset + array_data_offset + 2 >= insn_count) { Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "invalid array data start: at " << cur_offset << ", data offset " << array_data_offset @@ -1156,8 +1156,9 @@ bool MethodVerifier::CheckSwitchTargets(uint32_t cur_offset) { DCHECK_LT(cur_offset, insn_count); const uint16_t* insns = code_item_->insns_ + cur_offset; /* make sure the start of the switch is in range */ - int32_t switch_offset = insns[1] | ((int32_t) insns[2]) << 16; - if ((int32_t) cur_offset + switch_offset < 0 || cur_offset + switch_offset + 2 > insn_count) { + int32_t switch_offset = insns[1] | (static_cast(insns[2]) << 16); + if (static_cast(cur_offset) + switch_offset < 0 || + cur_offset + switch_offset + 2 > insn_count) { Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "invalid switch start: at " << cur_offset << ", switch offset " << switch_offset << ", count " << insn_count; @@ -1213,8 +1214,9 @@ bool MethodVerifier::CheckSwitchTargets(uint32_t cur_offset) { if (keys_offset > 0 && switch_count > 1) { int32_t last_key = switch_insns[keys_offset] | (switch_insns[keys_offset + 1] << 16); for (uint32_t targ = 1; targ < switch_count; targ++) { - int32_t key = (int32_t) switch_insns[keys_offset + targ * 2] | - (int32_t) (switch_insns[keys_offset + targ * 2 + 1] << 16); + int32_t key = + static_cast(switch_insns[keys_offset + targ * 2]) | + static_cast(switch_insns[keys_offset + targ * 2 + 1] << 16); if (key <= last_key) { Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "invalid packed switch: last key=" << last_key << ", this=" << key; @@ -1225,11 +1227,11 @@ bool MethodVerifier::CheckSwitchTargets(uint32_t cur_offset) { } /* verify each switch target */ for (uint32_t targ = 0; targ < switch_count; targ++) { - int32_t offset = (int32_t) switch_insns[targets_offset + targ * 2] | - (int32_t) (switch_insns[targets_offset + targ * 2 + 1] << 16); + int32_t offset = static_cast(switch_insns[targets_offset + targ * 2]) | + static_cast(switch_insns[targets_offset + targ * 2 + 1] << 16); int32_t abs_offset = cur_offset + offset; if (abs_offset < 0 || - abs_offset >= (int32_t) insn_count || + abs_offset >= static_cast(insn_count) || !insn_flags_[abs_offset].IsOpcode()) { Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "invalid switch target " << offset << " (-> " << reinterpret_cast(abs_offset) << ") at " @@ -2147,7 +2149,8 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) { } else { // Now verify if the element width in the table matches the element width declared in // the array - const uint16_t* array_data = insns + (insns[1] | (((int32_t) insns[2]) << 16)); + const uint16_t* array_data = + insns + (insns[1] | (static_cast(insns[2]) << 16)); if (array_data[0] != Instruction::kArrayDataSignature) { Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "invalid magic for array-data"; } else { @@ -3085,7 +3088,7 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) { * just need to walk through and tag the targets. */ if ((opcode_flags & Instruction::kSwitch) != 0) { - int offset_to_switch = insns[1] | (((int32_t) insns[2]) << 16); + int offset_to_switch = insns[1] | (static_cast(insns[2]) << 16); const uint16_t* switch_insns = insns + offset_to_switch; int switch_count = switch_insns[1]; int offset_to_targets, targ; @@ -3106,7 +3109,7 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) { /* offsets are 32-bit, and only partly endian-swapped */ offset = switch_insns[offset_to_targets + targ * 2] | - (((int32_t) switch_insns[offset_to_targets + targ * 2 + 1]) << 16); + (static_cast(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 (!CheckNotMoveExceptionOrMoveResult(code_item_->insns_, abs_offset)) { -- 2.11.0