Skip to content

arm: set the carry flag of logical instructions with a rotated immediate - #1582

Open
bacescuandrei wants to merge 1 commit into
cea-sec:masterfrom
bacescuandrei:fix/arm-imm-carry
Open

bacescuandrei wants to merge 1 commit into
cea-sec:masterfrom
bacescuandrei:fix/arm-imm-carry

Conversation

@bacescuandrei

Copy link
Copy Markdown
Contributor

Flag-setting logical instructions with a modified immediate (ANDS, EORS, ORRS, BICS, MOVS, MVNS, TST, TEQ) never updated the carry flag. In ARMExpandImm_C and ThumbExpandImm_C, an immediate encoded with a rotation sets C to bit 31 of the expanded value. So ANDS R0, R1, 0xFF000000 should set C, and ANDS R0, R1, 0x00FF0000 should clear it. sem.py had a TODO: set cf if ROT imm in argument for this, and MOVS/MVNS had TODO handle cf.

The operand already holds the expanded value, so the new imm_carry() reads the rotation from the encoding (instr.b):

  • A32: imm12[11:8].
  • 32-bit T32: i:imm3[2].
  • 16-bit T32 immediates are never rotated.

When the immediate is rotated, C is the value's MSB; otherwise C is left unchanged. The flag-setting logical instructions call it for an immediate operand.

test_rotated_imm_carry in test/arch/arm/sem.py covers each instruction, plus an unrotated immediate that keeps C. It fails without the change and passes with it. The rest of test/arch/arm/sem.py and test/arch/arm/arch.py pass.

I also compared these instructions against Unicorn (Cortex-A15) on all their immediate forms and edge values, running miasm's IR through the symbolic execution engine:

  • A32: 130,560 cases. 8 forms differed in C before; none differ after.
  • T32: 118,528 cases. The six carry mismatches are gone (33 forms differed before, 27 after).

The T32 forms that still differ are decoder issues I'll report separately:

  • lsr #32/asr #32 with imm5 = 0 are decoded as a shift of 0.
  • orr{s}.w rd, rn, #imm is decoded with Rd and Rn swapped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant