Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Support lsr.w on ARMv7 THUMB #1363

Merged
merged 3 commits into from
Jul 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions manticore/native/cpu/arm.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,16 @@ def read(self, nbits=None, with_carry=False):
value += self.cpu.instruction.size
if self.is_shifted():
shift = self.op.shift
value, carry = self.cpu._shift(value, shift.type, shift.value, carry)
# XXX: This is unnecessary repetition.
if shift.type in range(cs.arm.ARM_SFT_ASR_REG, cs.arm.ARM_SFT_RRX_REG + 1):
if self.cpu.mode == cs.CS_MODE_THUMB:
amount = shift.value.read()
else:
src_reg = self.cpu.instruction.reg_name(shift.value).upper()
amount = self.cpu.regfile.read(src_reg)
else:
amount = shift.value
value, carry = self.cpu._shift(value, shift.type, amount, carry)
if self.op.subtracted:
value = -value
if with_carry:
Expand Down Expand Up @@ -580,12 +589,7 @@ def _shift(cpu, value, _type, amount, carry):
amount = 1
Copy link
Member

@disconnect3d disconnect3d Jan 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# 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
if _type in (cs.arm.ARM_SFT_RRX, cs.arm.ARM_SFT_RRX_REG) and amount != 1:

While we are here, could you check if this is still the case in latest Capstone (4.0.1), please?

This way maybe we can remove this if/elif here.


elif _type in range(cs.arm.ARM_SFT_ASR_REG, cs.arm.ARM_SFT_RRX_REG + 1):
if cpu.mode == cs.CS_MODE_THUMB:
src = amount.read()
else:
src_reg = cpu.instruction.reg_name(amount).upper()
src = cpu.regfile.read(src_reg)
amount = Operators.EXTRACT(src, 0, 8)
amount = Operators.EXTRACT(amount, 0, 8)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the amount would be a register that stores the shift value now?

Copy link
Member

@disconnect3d disconnect3d Jan 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that below we use e.g. ASR_C from bitwise.py and this expects amount to be a number; is it always fine?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I won't have much time to look into this in the near future due to other priorities, but to address your point about bitwise: there are tests for ASR and ROR in the testsuite, which pass with this change. That's why the change at manticore/native/cpu/arm.py:105 was necessary. But it's hard to say for sure whether this would work for all inputs.


if amount == 0:
return value, carry
Expand Down Expand Up @@ -1494,12 +1498,15 @@ def _SR(cpu, insn_id, dest, op, *rest):
carry = cpu.regfile.read("APSR_C")
if rest and rest[0].type == "register":
# FIXME we should make Operand.op private (and not accessible)
result, carry = cpu._shift(op.read(), srtype, rest[0].op.reg, carry)
src_reg = cpu.instruction.reg_name(rest[0].op.reg).upper()
amount = cpu.regfile.read(src_reg)
result, carry = cpu._shift(op.read(), srtype, amount, carry)
elif rest and rest[0].type == "immediate":
amount = rest[0].read()
result, carry = cpu._shift(op.read(), srtype, amount, carry)
elif cpu.mode == cs.CS_MODE_THUMB:
result, carry = cpu._shift(dest.read(), srtype, op, carry)
amount = op.read()
result, carry = cpu._shift(dest.read(), srtype, amount, carry)
else:
result, carry = op.read(with_carry=True)
dest.write(result)
Expand Down
5 changes: 5 additions & 0 deletions tests/native/test_armv7cpu.py
Original file line number Diff line number Diff line change
Expand Up @@ -1666,6 +1666,11 @@ def test_thumb_lsrs(self):
def test_lsrw_thumb(self):
self.assertEqual(self.cpu.R5, 16 >> 3)

@itest_setregs("R0=11", "R2=2")
@itest_thumb("lsr.w R0, R0, R2")
def test_lsrw_thumb_reg(self):
self.assertEqual(self.cpu.R0, 11 >> 2)

@itest_setregs("R5=0", "R6=16")
@itest_thumb("asr.w R5, R6, #3")
def test_asrw_thumb(self):
Expand Down