From 82df9f3af144814889afb7e289d04dcc1ffa5dbf Mon Sep 17 00:00:00 2001 From: Brandon Ros Date: Thu, 2 Jul 2026 23:20:56 -0400 Subject: fix several thumb2 decode bugs - 32-bit store encodings with the imm12 bit (bit 23) set were decoded as the register-offset form when imm12 < 64, or as strbt/strht/strt when imm12 was 0xExx. bit 23 set always means the imm12 form. - movw/movt packed imm4 at bit 16 instead of bit 12, producing 20-bit immediates (e.g. 0xf0005 instead of 0xf005) - mvn (immediate) was decoded as mov, dropping the bitwise not - ror (immediate) was decoded as asr all cases verified against binutils objdump --- CHANGELOG | 7 ++++ src/armv7/thumb.rs | 22 +++++----- tests/armv7/thumb.rs | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 10 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 2c08422..5a6dc75 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -22,6 +22,13 @@ several fixes from @Grond66: * TST/TEQ decoded "three operands" in some cases when they only have two. * ARMv7: do not print the "s" suffix for instructions that unconditionally set flags. +as well as thumb2 fixes from @brandonros: +* thumb2: wide store encodings were very confused about offset widths. +* thumb2: movw/movt incorrectly reassembled 16-bit immediates into 20-bit immediates. +* thumb2: some instructions had outright incorrect opcodes: + * ror (immediate) was decoded as asr, + * mvn (immediate) was decoded as mov, + thank you for the patches! ## 0.4.0 diff --git a/src/armv7/thumb.rs b/src/armv7/thumb.rs index a2809eb..99ab46b 100644 --- a/src/armv7/thumb.rs +++ b/src/armv7/thumb.rs @@ -838,7 +838,7 @@ pub fn decode_into::Address, ::Word>>(d inst.set_w(false); let rm = lower2[..4].load::(); let rd = lower2[8..12].load::(); - inst.opcode = Opcode::ASR; + inst.opcode = Opcode::ROR; inst.operands = [ Operand::Reg(Reg::from_u8(rd)), Operand::Reg(Reg::from_u8(rm)), @@ -1111,7 +1111,7 @@ pub fn decode_into::Address, ::Word>>(d if rn == 0b1111 { // `MVN` (`A8-505`) // v6T2 - inst.opcode = Opcode::MOV; + inst.opcode = Opcode::MVN; inst.operands = [ Operand::Reg(Reg::from_u8(rd)), Operand::Imm32(imm as u32), @@ -1292,7 +1292,7 @@ pub fn decode_into::Address, ::Word>>(d inst.opcode = Opcode::MOV; inst.operands = [ Operand::Reg(Reg::from_u8(rd)), - Operand::Imm32(imm as u32 | ((rn as u32) << 16)), + Operand::Imm32(imm as u32 | ((rn as u32) << 12)), Operand::Nothing, Operand::Nothing, ]; @@ -1330,7 +1330,7 @@ pub fn decode_into::Address, ::Word>>(d inst.opcode = Opcode::MOVT; inst.operands = [ Operand::Reg(Reg::from_u8(rd)), - Operand::Imm32(imm as u32 | ((rn as u32) << 16)), + Operand::Imm32(imm as u32 | ((rn as u32) << 12)), Operand::Nothing, Operand::Nothing, ]; @@ -2023,7 +2023,9 @@ pub fn decode_into::Address, ::Word>>(d match size_bits { 0b00 => { // `STRB_` - if op2 == 0 { + // op2 only selects a form when the imm12 bit is clear; + // otherwise it's just the high bits of imm12. + if !has_imm12 && op2 == 0 { // `STRB (register)` (`A8-683`) // encoding T2 // v6T2 @@ -2048,7 +2050,7 @@ pub fn decode_into::Address, ::Word>>(d Operand::Nothing, Operand::Nothing, ]; - } else if (op2 & 0b111100) == 0b111000 { + } else if !has_imm12 && (op2 & 0b111100) == 0b111000 { // `STRBT` (`A8-685`) // v6T2 let imm8 = lower & 0b1111_1111; @@ -2112,7 +2114,7 @@ pub fn decode_into::Address, ::Word>>(d 0b01 => { // `STRH_` // v6T2 - if op2 == 0 { + if !has_imm12 && op2 == 0 { // `STRH (register)` (`A8-703`) let rm = (lower & 0b1111) as u8; let imm2 = (lower >> 4) & 0b11; @@ -2135,7 +2137,7 @@ pub fn decode_into::Address, ::Word>>(d Operand::Nothing, Operand::Nothing, ]; - } else if (op2 & 0b111100) == 0b111000 { + } else if !has_imm12 && (op2 & 0b111100) == 0b111000 { // `STRHT` (`A8-705`) let imm8 = lower & 0b1111_1111; let rt = ((lower >> 12) & 0b1111) as u8; @@ -2197,7 +2199,7 @@ pub fn decode_into::Address, ::Word>>(d } 0b10 => { // `STR_` - if op2 == 0 { + if !has_imm12 && op2 == 0 { // `STR (register)` (`A8-677`) // v6T2 let rm = (lower & 0b1111) as u8; @@ -2221,7 +2223,7 @@ pub fn decode_into::Address, ::Word>>(d Operand::Nothing, Operand::Nothing, ]; - } else if (op2 & 0b111100) == 0b111000 { + } else if !has_imm12 && (op2 & 0b111100) == 0b111000 { // `STRT` (`A8-707`) let imm8 = lower & 0b1111_1111; let rt = ((lower >> 12) & 0b1111) as u8; diff --git a/tests/armv7/thumb.rs b/tests/armv7/thumb.rs index fb9120b..473f17c 100644 --- a/tests/armv7/thumb.rs +++ b/tests/armv7/thumb.rs @@ -4136,3 +4136,114 @@ fn test_decode_simd_32b_cases() { "vstmdb r3!, {s11, s12, s13, s14, s15, s16, s17, s18, s19, s20, s21, s22, s23, s24, s25, s26, s27, s28, s29, s30, s31}" ); } + +#[test] +fn test_decode_str_32b_imm12_cases() { + // bit 23 selects the imm12 form; low imm12 bits are not the register-offset + // or strbt/strht/strt forms. + test_display( + &[0x83, 0xf8, 0x21, 0x20], + "strb.w r2, [r3, 0x21]" + ); + test_display( + &[0x83, 0xf8, 0x3f, 0x20], + "strb.w r2, [r3, 0x3f]" + ); + test_display( + &[0xa3, 0xf8, 0x21, 0x20], + "strh.w r2, [r3, 0x21]" + ); + test_display( + &[0xc3, 0xf8, 0x21, 0x20], + "str.w r2, [r3, 0x21]" + ); + test_display( + &[0x83, 0xf8, 0x21, 0x2e], + "strb.w r2, [r3, 0xe21]" + ); + test_display( + &[0x83, 0xf8, 0x40, 0x20], + "strb.w r2, [r3, 0x40]" + ); + test_display( + &[0xcd, 0xf8, 0x0c, 0xc0], + "str.w ip, [sp, 0xc]" + ); + test_display( + &[0xd3, 0xf8, 0x21, 0x20], + "ldr.w r2, [r3, 0x21]" + ); + test_display( + &[0x93, 0xf8, 0x05, 0x10], + "ldrb.w r1, [r3, 0x5]" + ); + test_display( + &[0xb3, 0xf8, 0x08, 0x10], + "ldrh.w r1, [r3, 0x8]" + ); +} + +#[test] +fn test_decode_movw_movt_cases() { + // imm16 is imm4:i:imm3:imm8 + test_display( + &[0xcf, 0xf2, 0x05, 0x04], + "movt r4, 0xf005" + ); + test_display( + &[0x4f, 0xf2, 0x05, 0x00], + "mov r0, 0xf005" + ); + test_display( + &[0x40, 0xf2, 0x34, 0x12], + "mov r2, 0x134" + ); + test_display( + &[0xc0, 0xf2, 0x01, 0x00], + "movt r0, 0x1" + ); +} + +#[test] +fn test_decode_mvn_imm_cases() { + test_display( + &[0x6f, 0xf0, 0x40, 0x41], + "mvn.w r1, 0xc0000000" + ); + test_display( + &[0x6f, 0xf0, 0x7f, 0x40], + "mvn.w r0, 0xff000000" + ); + test_display( + &[0x6f, 0xf0, 0x01, 0x01], + "mvn.w r1, 0x1" + ); + test_display( + &[0x7f, 0xf0, 0x40, 0x41], + "mvns.w r1, 0xc0000000" + ); +} + +#[test] +fn test_decode_mov_reg_shift_cases() { + test_display( + &[0x4f, 0xea, 0xfc, 0x49], + "ror sb, ip, 0x13" + ); + test_display( + &[0x4f, 0xea, 0x31, 0x42], + "ror r2, r1, 0x10" + ); + test_display( + &[0x4f, 0xea, 0x72, 0x12], + "ror r2, r2, 0x5" + ); + test_display( + &[0x4f, 0xea, 0x51, 0x01], + "lsr.w r1, r1, 0x1" + ); + test_display( + &[0x4f, 0xea, 0x32, 0x03], + "rrx r3, r2" + ); +} -- cgit v1.1