From 9e9c7b2663fa52881bf6883e74f4cd8610c38570 Mon Sep 17 00:00:00 2001 From: Brandon Ros Date: Fri, 3 Jul 2026 00:14:45 -0400 Subject: fix more thumb2 register data-processing decode bugs - register-controlled shifts (lsl/lsr/asr/ror by register) all decoded as lsl: the shift type was read from hw1[6:5] (always 0 for this form) instead of hw0[6:5] - 32-bit uxt*/sxt* extends read the ror amount from the wrong field (hw1[2:1] scaled by 4 instead of hw1[5:4] scaled by 8), so a no-rotation extend came back as ror #4; rotation of 0 is now omitted - sxtab was decoded as sxtah (wrong opcode table entry) all cases verified against binutils objdump --- CHANGELOG | 3 ++ src/armv7/thumb.rs | 20 ++++++++++---- tests/armv7/thumb.rs | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 6 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 5a6dc75..5de5ee7 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -28,6 +28,9 @@ as well as thumb2 fixes from @brandonros: * thumb2: some instructions had outright incorrect opcodes: * ror (immediate) was decoded as asr, * mvn (immediate) was decoded as mov, + * sxtab was decoded as sxtah, + * lsl, lsr, asr, ror were determining opcode from an incorrect bitfield, +* thumb2: (un)signed extend-and-rotate loaded an incorrect field for rotate, thank you for the patches! diff --git a/src/armv7/thumb.rs b/src/armv7/thumb.rs index 4424d44..e9427d1 100644 --- a/src/armv7/thumb.rs +++ b/src/armv7/thumb.rs @@ -2695,7 +2695,7 @@ pub fn decode_into::Address, ::Word>>(d Opcode::LSR, Opcode::ASR, Opcode::ROR, - ][op2[1..3].load::()]; + ][op1[1..3].load::()]; let rd = lower2[8..12].load::(); let rm = lower2[0..4].load::(); inst.opcode = op; @@ -2723,14 +2723,18 @@ pub fn decode_into::Address, ::Word>>(d ][op1]; let rm = lower2[..4].load::(); - let rotate = lower2[1..3].load::() << 2; + let rotate = lower2[4..6].load::() << 3; let rd = lower2[8..12].load::(); inst.opcode = op; inst.operands = [ Operand::Reg(Reg::from_u8(rd)), Operand::Reg(Reg::from_u8(rm)), - Operand::Imm32(rotate as u32), + if rotate != 0 { + Operand::Imm32(rotate as u32) + } else { + Operand::Nothing + }, Operand::Nothing, ]; } else { @@ -2739,12 +2743,12 @@ pub fn decode_into::Address, ::Word>>(d Opcode::UXTAH, Opcode::SXTAB16, Opcode::UXTAB16, - Opcode::SXTAH, + Opcode::SXTAB, Opcode::UXTAB, ][op1]; let rm = lower2[..4].load::(); - let rotate = lower2[1..3].load::() << 2; + let rotate = lower2[4..6].load::() << 3; let rd = lower2[8..12].load::(); inst.opcode = op; @@ -2752,7 +2756,11 @@ pub fn decode_into::Address, ::Word>>(d Operand::Reg(Reg::from_u8(rd)), Operand::Reg(Reg::from_u8(rn)), Operand::Reg(Reg::from_u8(rm)), - Operand::Imm32(rotate as u32), + if rotate != 0 { + Operand::Imm32(rotate as u32) + } else { + Operand::Nothing + }, ]; }; } diff --git a/tests/armv7/thumb.rs b/tests/armv7/thumb.rs index 473f17c..8c1885d 100644 --- a/tests/armv7/thumb.rs +++ b/tests/armv7/thumb.rs @@ -4247,3 +4247,81 @@ fn test_decode_mov_reg_shift_cases() { "rrx r3, r2" ); } + +#[test] +fn test_decode_ux_sx_32b_cases() { + // rotation is hw1[5:4] scaled by 8, and omitted when zero + test_display( + &[0x1f, 0xfa, 0x8b, 0xfb], + "uxth.w fp, fp" + ); + test_display( + &[0x5f, 0xfa, 0x8c, 0xfc], + "uxtb.w ip, ip" + ); + test_display( + &[0x5f, 0xfa, 0x9c, 0xfc], + "uxtb.w ip, ip, 0x8" + ); + test_display( + &[0x1f, 0xfa, 0xab, 0xfb], + "uxth.w fp, fp, 0x10" + ); + test_display( + &[0x0f, 0xfa, 0x82, 0xf1], + "sxth.w r1, r2" + ); + test_display( + &[0x52, 0xfa, 0x93, 0xf1], + "uxtab.w r1, r2, r3, 0x8" + ); + test_display( + &[0x42, 0xfa, 0x83, 0xf1], + "sxtab.w r1, r2, r3" + ); + test_display( + &[0x02, 0xfa, 0x83, 0xf1], + "sxtah.w r1, r2, r3" + ); +} + +#[test] +fn test_decode_ux_sx_b16_32b_cases() { + test_display( + &[0x2f, 0xfa, 0x82, 0xf1], + "sxtb16.w r1, r2" + ); + test_display( + &[0x3f, 0xfa, 0x82, 0xf1], + "uxtb16.w r1, r2" + ); + test_display( + &[0x22, 0xfa, 0x83, 0xf1], + "sxtab16.w r1, r2, r3" + ); + test_display( + &[0x32, 0xfa, 0x83, 0xf1], + "uxtab16.w r1, r2, r3" + ); +} + +#[test] +fn test_decode_shift_reg_32b_cases() { + // register-controlled shift: type is in hw0[6:5] + test_display( + &[0x02, 0xfa, 0x03, 0xf1], + "lsl.w r1, r2, r3" + ); + test_display( + &[0x22, 0xfa, 0x03, 0xf1], + "lsr.w r1, r2, r3" + ); + test_display( + &[0x42, 0xfa, 0x03, 0xf1], + "asr.w r1, r2, r3" + ); + test_display( + &[0x62, 0xfa, 0x03, 0xf1], + "ror.w r1, r2, r3" + ); +} -- cgit v1.1