From 680fc8f17e267394e6770e6b0d09a82afdddd346 Mon Sep 17 00:00:00 2001 From: Disconnect3d Date: Sat, 17 Feb 2018 17:51:58 +0100 Subject: [PATCH] Armv7 code refactor (#749) * Armv7 code refactor Removes dead imports, makes code style more consistent and a bit more compliant with pep8 (yeah i seems we don't enforce any particular style, but this seems to be a reasonable default for me). * Review fix: add comment for armv7 _shift --- manticore/core/cpu/arm.py | 370 ++++++++++++++++++++------------------ tests/test_armv7cpu.py | 2 +- tests/test_unicorn.py | 2 +- 3 files changed, 193 insertions(+), 181 deletions(-) diff --git a/manticore/core/cpu/arm.py b/manticore/core/cpu/arm.py index 2e0be10..47925af 100644 --- a/manticore/core/cpu/arm.py +++ b/manticore/core/cpu/arm.py @@ -18,15 +18,17 @@ OP_NAME_MAP = { 'MOVW': 'MOV' } + def HighBit(n): return Bit(n, 31) + def instruction(body): @wraps(body) def instruction_implementation(cpu, *args, **kwargs): ret = None - should_execute = cpu.shouldExecuteConditional() + should_execute = cpu.should_execute_conditional() if cpu._at_symbolic_conditional: cpu._at_symbolic_conditional = False @@ -36,20 +38,21 @@ def instruction(body): # Let's remember next time we get here we should not do this again cpu._at_symbolic_conditional = True i_size = cpu.address_bit_size / 8 - cpu.PC = Operators.ITEBV(cpu.address_bit_size, should_execute, cpu.PC-i_size, - cpu.PC) + cpu.PC = Operators.ITEBV(cpu.address_bit_size, should_execute, cpu.PC - i_size, + cpu.PC) return if should_execute: ret = body(cpu, *args, **kwargs) - if cpu.shouldCommitFlags(): - cpu.commitFlags() + if cpu.should_commit_flags(): + cpu.commit_flags() return ret return abstract_instruction(instruction_implementation) + class Armv7Operand(Operand): def __init__(self, cpu, op, **kwargs): super(Armv7Operand, self).__init__(cpu, op, **kwargs) @@ -60,8 +63,8 @@ class Armv7Operand(Operand): cs.arm.ARM_OP_REG: 'register', cs.arm.ARM_OP_MEM: 'memory', cs.arm.ARM_OP_IMM: 'immediate', - cs.arm.ARM_OP_PIMM:'coprocessor', - cs.arm.ARM_OP_CIMM:'immediate' + cs.arm.ARM_OP_PIMM: 'coprocessor', + cs.arm.ARM_OP_CIMM: 'immediate' } return type_map[self.op.type] @@ -69,13 +72,13 @@ class Armv7Operand(Operand): @property def size(self): assert self.type == 'register' - if self.op.reg >= cs.arm.ARM_REG_D0 and self.op.reg <= cs.arm.ARM_REG_D31: + if cs.arm.ARM_REG_D0 <= self.op.reg <= cs.arm.ARM_REG_D31: return 64 else: - #FIXME check other types of operand sizes + # FIXME check other types of operand sizes return 32 - def read(self, nbits=None, withCarry=False): + def read(self, nbits=None, with_carry=False): carry = self.cpu.regfile.read('APSR_C') if self.type == 'register': value = self.cpu.regfile.read(self.reg) @@ -85,26 +88,26 @@ class Armv7Operand(Operand): value += cpu.instruction.size if self.is_shifted(): shift = self.op.shift - value, carry = self.cpu._Shift(value, shift.type, shift.value, carry) + value, carry = self.cpu._shift(value, shift.type, shift.value, carry) if self.op.subtracted: value = -value - if withCarry: + if with_carry: return value, carry return value elif self.type == 'immediate': imm = self.op.imm if self.op.subtracted: imm = -imm - if withCarry: - return imm, self._getExpandImmCarry(carry) + if with_carry: + return imm, self._get_expand_imm_carry(carry) return imm elif self.type == 'coprocessor': imm = self.op.imm return imm elif self.type == 'memory': val = self.cpu.read_int(self.address(), nbits) - if withCarry: - return (val, carry) + if with_carry: + return val, carry return val else: raise NotImplementedError("readOperand unknown type", self.op.type) @@ -142,7 +145,7 @@ class Armv7Operand(Operand): carry = self.cpu.regfile.read('APSR_C') if self.is_shifted(): shift = self.op.shift - idx, carry = self.cpu._Shift(idx, shift.type, shift.value, carry) + idx, carry = self.cpu._shift(idx, shift.type, shift.value, carry) off = -idx if self.op.subtracted else idx else: off = self.mem.disp @@ -174,58 +177,57 @@ class Armv7Operand(Operand): else: return base - def _getExpandImmCarry(self, carryIn): - '''Manually compute the carry bit produced by expanding an immediate - operand (see ARMExpandImm_C) - ''' + def _get_expand_imm_carry(self, carryIn): + """Manually compute the carry bit produced by expanding an immediate operand (see ARMExpandImm_C)""" insn = struct.unpack(' cs.arm.ARM_SFT_INVALID and _type <= cs.arm.ARM_SFT_RRX_REG) + def _shift(cpu, value, _type, amount, carry): + """See Shift() and Shift_C() in the ARM manual""" + assert(cs.arm.ARM_SFT_INVALID < _type <= cs.arm.ARM_SFT_RRX_REG) # XXX: Capstone should set the value of an RRX shift to 1, which is # asserted in the manual, but it sets it to 0, so we have to check @@ -440,7 +445,7 @@ class Armv7Cpu(Cpu): width = cpu.address_bit_size - if _type in (cs.arm.ARM_SFT_ASR, cs.arm.ARM_SFT_ASR_REG): + if _type in (cs.arm.ARM_SFT_ASR, cs.arm.ARM_SFT_ASR_REG): return ASR_C(value, amount, width) elif _type in (cs.arm.ARM_SFT_LSL, cs.arm.ARM_SFT_LSL_REG): return LSL_C(value, amount, width) @@ -456,11 +461,11 @@ class Armv7Cpu(Cpu): # TODO add to abstract cpu, and potentially remove stacksub/add from it? def stack_push(self, data, nbytes=None): if isinstance(data, (int, long)): - nbytes = nbytes or self.address_bit_size/8 + nbytes = nbytes or self.address_bit_size / 8 self.SP -= nbytes self.write_int(self.SP, data, nbytes * 8) elif isinstance(data, BitVec): - self.SP -= data.size/8 + self.SP -= data.size / 8 self.write_int(self.SP, data, data.size) elif isinstance(data, str): self.SP -= len(data) @@ -507,21 +512,21 @@ class Armv7Cpu(Cpu): def _wrap_operands(self, ops): return [Armv7Operand(self, op) for op in ops] - def shouldCommitFlags(cpu): - #workaround for a capstone bug (issue #980); - #the bug has been fixed the 'master' and 'next' branches of capstone as of 2017-07-31 + def should_commit_flags(cpu): + # workaround for a capstone bug (issue #980); + # the bug has been fixed the 'master' and 'next' branches of capstone as of 2017-07-31 if cpu.instruction.id == cs.arm.ARM_INS_UADD8: return True return cpu.instruction.update_flags - def shouldExecuteConditional(cpu): - #for the IT instuction, the cc applies to the subsequent instructions, - #so the IT instruction should be executed regardless of its cc + def should_execute_conditional(cpu): + # for the IT instruction, the cc applies to the subsequent instructions, + # so the IT instruction should be executed regardless of its cc if cpu.instruction.id == cs.arm.ARM_INS_IT: return True - #support for the it[x[y[z]]] instructions + # support for the it[x[y[z]]] instructions if cpu._it_conditional: return cpu._it_conditional.pop(0) @@ -534,21 +539,32 @@ class Armv7Cpu(Cpu): V = cpu.regfile.read('APSR_V') Z = cpu.regfile.read('APSR_Z') - if cc == cs.arm.ARM_CC_AL: ret = True - elif cc == cs.arm.ARM_CC_EQ: ret = Z - elif cc == cs.arm.ARM_CC_NE: ret = Operators.NOT(Z) - elif cc == cs.arm.ARM_CC_HS: ret = C - elif cc == cs.arm.ARM_CC_LO: ret = Operators.NOT(C) - elif cc == cs.arm.ARM_CC_MI: ret = N - elif cc == cs.arm.ARM_CC_PL: ret = Operators.NOT(N) - elif cc == cs.arm.ARM_CC_VS: ret = V - elif cc == cs.arm.ARM_CC_VC: ret = Operators.NOT(V) + if cc == cs.arm.ARM_CC_AL: + ret = True + elif cc == cs.arm.ARM_CC_EQ: + ret = Z + elif cc == cs.arm.ARM_CC_NE: + ret = Operators.NOT(Z) + elif cc == cs.arm.ARM_CC_HS: + ret = C + elif cc == cs.arm.ARM_CC_LO: + ret = Operators.NOT(C) + elif cc == cs.arm.ARM_CC_MI: + ret = N + elif cc == cs.arm.ARM_CC_PL: + ret = Operators.NOT(N) + elif cc == cs.arm.ARM_CC_VS: + ret = V + elif cc == cs.arm.ARM_CC_VC: + ret = Operators.NOT(V) elif cc == cs.arm.ARM_CC_HI: ret = Operators.AND(C, Operators.NOT(Z)) elif cc == cs.arm.ARM_CC_LS: ret = Operators.OR(Operators.NOT(C), Z) - elif cc == cs.arm.ARM_CC_GE: ret = N == V - elif cc == cs.arm.ARM_CC_LT: ret = N != V + elif cc == cs.arm.ARM_CC_GE: + ret = N == V + elif cc == cs.arm.ARM_CC_LT: + ret = N != V elif cc == cs.arm.ARM_CC_GT: ret = Operators.AND(Operators.NOT(Z), N == V) elif cc == cs.arm.ARM_CC_LE: @@ -562,8 +578,8 @@ class Armv7Cpu(Cpu): def IT(cpu): cc = cpu.instruction.cc true_case = cpu._evaluate_conditional(cc) - #this is incredibly hacky--how else does capstone expose this? - #TODO: find a better way than string parsing the mnemonic -GR, 2017-07-13 + # this is incredibly hacky--how else does capstone expose this? + # TODO: find a better way than string parsing the mnemonic -GR, 2017-07-13 for c in cpu.instruction.mnemonic[1:]: if c == 't': cpu._it_conditional.append(true_case) @@ -577,14 +593,14 @@ class Armv7Cpu(Cpu): sums = list() carries = list() for i in range(4): - uo1 = UInt(Operators.ZEXTEND(Operators.EXTRACT(op1, (8*i), 8), 9), 9) - uo2 = UInt(Operators.ZEXTEND(Operators.EXTRACT(op2, (8*i), 8), 9), 9) + uo1 = UInt(Operators.ZEXTEND(Operators.EXTRACT(op1, (8 * i), 8), 9), 9) + uo2 = UInt(Operators.ZEXTEND(Operators.EXTRACT(op2, (8 * i), 8), 9), 9) byte = uo1 + uo2 carry = Operators.EXTRACT(byte, 8, 1) sums.append(Operators.EXTRACT(byte, 0, 8)) carries.append(carry) dest.write(Operators.CONCAT(32, *reversed(sums))) - cpu.setFlags(GE=Operators.CONCAT(4, *reversed(carries))) + cpu.set_flags(GE=Operators.CONCAT(4, *reversed(carries))) @instruction def SEL(cpu, dest, op1, op2): @@ -594,12 +610,13 @@ class Armv7Cpu(Cpu): GE = cpu.regfile.read('APSR_GE') for i in range(4): bit = Operators.EXTRACT(GE, i, 1) - result.append(Operators.ITEBV(8, bit, Operators.EXTRACT(op1val, i*8, 8), Operators.EXTRACT(op2val, i*8, 8))) + result.append( + Operators.ITEBV(8, bit, Operators.EXTRACT(op1val, i * 8, 8), Operators.EXTRACT(op2val, i * 8, 8))) dest.write(Operators.CONCAT(32, *reversed(result))) @instruction def MOV(cpu, dest, src): - ''' + """ Implement the MOV{S} instruction. Note: If src operand is PC, temporarily release our logical PC @@ -607,25 +624,25 @@ class Armv7Cpu(Cpu): :param Armv7Operand dest: The destination operand; register. :param Armv7Operand src: The source operand; register or immediate. - ''' + """ if cpu.mode == cs.CS_MODE_ARM: - result, carry_out = src.read(withCarry=True) + result, carry_out = src.read(with_carry=True) dest.write(result) - cpu.setFlags(C=carry_out, N=HighBit(result), Z=(result == 0)) + cpu.set_flags(C=carry_out, N=HighBit(result), Z=(result == 0)) else: # thumb mode cannot do wonky things to the operand, so no carry calculation result = src.read() dest.write(result) - cpu.setFlags(N=HighBit(result), Z=(result == 0)) + cpu.set_flags(N=HighBit(result), Z=(result == 0)) @instruction def MOVT(cpu, dest, src): - ''' + """ MOVT writes imm16 to Rd[31:16]. The write does not affect Rd[15:0]. :param Armv7Operand dest: The destination operand; register :param Armv7Operand src: The source operand; 16-bit immediate - ''' + """ assert src.type == 'immediate' imm = src.read() low_halfword = dest.read() & Mask(16) @@ -633,7 +650,7 @@ class Armv7Cpu(Cpu): @instruction def MRC(cpu, coprocessor, opcode1, dest, coprocessor_reg_n, coprocessor_reg_m, opcode2): - ''' + """ MRC moves to ARM register from coprocessor. :param Armv7Operand coprocessor: The name of the coprocessor; immediate @@ -642,7 +659,7 @@ class Armv7Cpu(Cpu): :param Armv7Operand coprocessor_reg_n: the coprocessor register; immediate :param Armv7Operand coprocessor_reg_m: the coprocessor register; immediate :param Armv7Operand opcode2: coprocessor specific opcode; 3-bit immediate - ''' + """ assert coprocessor.type == 'coprocessor' assert opcode1.type == 'immediate' assert opcode2.type == 'immediate' @@ -653,7 +670,7 @@ class Armv7Cpu(Cpu): coprocessor_n_name = coprocessor_reg_n.read() coprocessor_m_name = coprocessor_reg_m.read() - if 15 == imm_coprocessor: #MMU + if 15 == imm_coprocessor: # MMU if 0 == imm_opcode1: if 13 == coprocessor_n_name: if 3 == imm_opcode2: @@ -663,14 +680,12 @@ class Armv7Cpu(Cpu): @instruction def LDRD(cpu, dest1, dest2, src, offset=None): - ''' - Loads double width data from memory. - ''' + """Loads double width data from memory.""" assert dest1.type == 'register' assert dest2.type == 'register' assert src.type == 'memory' mem1 = cpu.read_int(src.address(), 32) - mem2 = cpu.read_int(src.address()+4, 32) + mem2 = cpu.read_int(src.address() + 4, 32) writeback = cpu._compute_writeback(src, offset) dest1.write(mem1) dest2.write(mem2) @@ -678,9 +693,7 @@ class Armv7Cpu(Cpu): @instruction def STRD(cpu, src1, src2, dest, offset=None): - ''' - Writes the contents of two registers to memory. - ''' + """Writes the contents of two registers to memory.""" assert src1.type == 'register' assert src2.type == 'register' assert dest.type == 'memory' @@ -688,12 +701,12 @@ class Armv7Cpu(Cpu): val2 = src2.read() writeback = cpu._compute_writeback(dest, offset) cpu.write_int(dest.address(), val1, 32) - cpu.write_int(dest.address()+4, val2, 32) + cpu.write_int(dest.address() + 4, val2, 32) cpu._cs_hack_ldr_str_writeback(dest, offset, writeback) @instruction def LDREX(cpu, dest, src, offset=None): - ''' + """ LDREX loads data from memory. * If the physical address has the shared TLB attribute, LDREX tags the physical address as exclusive access for the current @@ -704,38 +717,36 @@ class Armv7Cpu(Cpu): :param Armv7Operand dest: the destination register; register :param Armv7Operand src: the source operand: register - ''' - #TODO: add lock mechanism to underlying memory --GR, 2017-06-06 + """ + # TODO: add lock mechanism to underlying memory --GR, 2017-06-06 cpu._LDR(dest, src, 32, False, offset) @instruction def STREX(cpu, status, *args): - ''' + """ STREX performs a conditional store to memory. :param Armv7Operand status: the destination register for the returned status; register - ''' - #TODO: implement conditional return with appropriate status --GR, 2017-06-06 + """ + # TODO: implement conditional return with appropriate status --GR, 2017-06-06 status.write(0) return cpu._STR(cpu.address_bit_size, *args) @instruction def UXTB(cpu, dest, src): - ''' + """ UXTB extracts an 8-bit value from a register, zero-extends it to the size of the register, and writes the result to the destination register. :param ARMv7Operand dest: the destination register; register :param ARMv7Operand dest: the source register; register - ''' + """ val = GetNBits(src.read(), 8) word = Operators.ZEXTEND(val, cpu.address_bit_size) dest.write(word) @instruction def PLD(cpu, addr, offset=None): - ''' - PLD instructs the cpu that the address at addr might be loaded soon. - ''' + """PLD instructs the cpu that the address at addr might be loaded soon.""" pass def _compute_writeback(cpu, operand, offset): @@ -758,13 +769,16 @@ class Armv7Cpu(Cpu): cpu._cs_hack_ldr_str_writeback(dest, offset, writeback) @instruction - def STR(cpu, *args): return cpu._STR(cpu.address_bit_size, *args) + def STR(cpu, *args): + return cpu._STR(cpu.address_bit_size, *args) @instruction - def STRB(cpu, *args): return cpu._STR(8, *args) + def STRB(cpu, *args): + return cpu._STR(8, *args) @instruction - def STRH(cpu, *args): return cpu._STR(16, *args) + def STRH(cpu, *args): + return cpu._STR(16, *args) def _LDR(cpu, dest, src, width, is_signed, offset): mem = cpu.read_int(src.address(), width) @@ -806,24 +820,24 @@ class Armv7Cpu(Cpu): # this converts it back to unsigned _op2 = Operators.ZEXTEND(_op2, W) - uo1 = UInt(_op1, W*2) - uo2 = UInt(_op2, W*2) - c = UInt(carry, W*2) + uo1 = UInt(_op1, W * 2) + uo2 = UInt(_op2, W * 2) + c = UInt(carry, W * 2) unsigned_sum = uo1 + uo2 + c - so1 = SInt(Operators.SEXTEND(_op1, W, W*2), W*2) - so2 = SInt(Operators.SEXTEND(_op2, W, W*2), W*2) + so1 = SInt(Operators.SEXTEND(_op1, W, W * 2), W * 2) + so2 = SInt(Operators.SEXTEND(_op2, W, W * 2), W * 2) signed_sum = so1 + so2 + c result = GetNBits(unsigned_sum, W) - carry_out = UInt(result, W*2) != unsigned_sum - overflow = SInt(Operators.SEXTEND(result,W,W*2), W*2) != signed_sum + carry_out = UInt(result, W * 2) != unsigned_sum + overflow = SInt(Operators.SEXTEND(result, W, W * 2), W * 2) != signed_sum - cpu.setFlags(C=carry_out, - V=overflow, - N=HighBit(result), - Z=result == 0) + cpu.set_flags(C=carry_out, + V=overflow, + N=HighBit(result), + Z=result == 0) return result, carry_out, overflow @@ -842,7 +856,7 @@ class Armv7Cpu(Cpu): if add is not None: result, carry, overflow = cpu._ADD(src.read(), add.read()) else: - #support for the thumb mode version of adds , + # support for the thumb mode version of adds , result, carry, overflow = cpu._ADD(dest.read(), src.read()) dest.write(result) return result, carry, overflow @@ -867,8 +881,9 @@ class Armv7Cpu(Cpu): if add is not None: result, carry, overflow = cpu._ADD(src.read(), ~add.read(), 1) else: - #support for the thumb mode version of sub , + # support for the thumb mode version of sub , result, carry, overflow = cpu._ADD(dest.read(), ~src.read(), 1) + dest.write(result) return result, carry, overflow @@ -924,9 +939,9 @@ class Armv7Cpu(Cpu): cpu.regfile.write('LR', next_instr_addr) cpu.regfile.write('PC', target & ~1) - ## The `blx