From 189cbcfdad097363e66f41daf4d6a76acbf3661c Mon Sep 17 00:00:00 2001 From: Mitchell Johnson Date: Sun, 4 Sep 2022 17:33:39 -0400 Subject: Fix 32-bit Thumb unconditional branch decoding T4 encodings of unconditional branches were not being interpreted correctly (#6), and 32-bit bl/blx instructions were similarly incorrect. Correct the bits selected for op1 and op2 and handle the slightly-unusual i1/i2 sign bit xor for these instructions. --- src/armv7/thumb.rs | 98 ++++++++++++++++++++-------------------------------- tests/armv7/thumb.rs | 13 +++++++ 2 files changed, 50 insertions(+), 61 deletions(-) diff --git a/src/armv7/thumb.rs b/src/armv7/thumb.rs index 1545a36..baa2352 100644 --- a/src/armv7/thumb.rs +++ b/src/armv7/thumb.rs @@ -125,7 +125,7 @@ pub fn decode_into::Address, ::Word>>(d let op2 = &instr2[4..11]; - if opword < 0b11110 { + if opword == 0b11101 { // op1 == 0b01 // interpret `op1 == 0b01` lines of table `A6-9` if !op2[6] { @@ -1022,10 +1022,10 @@ pub fn decode_into::Address, ::Word>>(d // this means `assert!(instr2[10])` return decode_table_a6_30(decoder, inst, instr2, lower2); } - } else if opword < 0b11111 { + } else if opword == 0b11110 { // op1 == 0b10 // interpret `op1 == 0b10` lines in table `A6-9` on `A6-228`: - if lower & 0x80 == 0 { + if !lower2[15] { // op == 0 if !op2[5] { // `A6.3.1` `Data-processing (modified immediate)` (`A6-229`) @@ -1473,11 +1473,11 @@ pub fn decode_into::Address, ::Word>>(d } } } else { - // op == 1 + // A6.3 op == 1 // `Branches and miscellaneous control` (`A6-233`) let imm8 = lower2[0..8].load::(); - let op2 = lower2[8..11].load::(); - let op1 = instr2[12..15].load::(); + let op2 = lower2[8..12].load::(); + let op1 = lower2[12..15].load::(); let op = instr2[4..11].load::(); if op1 & 0b101 == 0b000 { // TODO: This entire section appears wrong? what encoding is the conditional @@ -1915,74 +1915,50 @@ pub fn decode_into::Address, ::Word>>(d } } } - } else if op1 & 0b101 == 0b001 { - // `Branch` (`A8-332`) - // v6T2 - // TODO: encoding T3? unclear - let imm11 = lower2[0..11].load::(); - let imm6 = instr2[0..6].load::(); - let j1 = lower2[13..14].load::(); - let j2 = lower2[11..12].load::(); - let s = instr2[10..11].load::(); - let imm = - (imm11 as u32) | - ((imm6 as u32) << 11) | - ((j1 as u32) << 17) | - ((j2 as u32) << 18) | - ((s as u32) << 19); - let imm = (imm << 12) >> 12; - inst.opcode = Opcode::B; - inst.operands = [ - Operand::Imm32(imm as u32), - Operand::Nothing, - Operand::Nothing, - Operand::Nothing, - ]; - } else if op1 & 0b101 == 0b100 { - // `Branch with Link and Exchange` (`A8-346`) - // `UNDEFINED` in v4T - // v5T - let imm11 = lower2[0..11].load::(); - let imm10 = instr2[0..10].load::(); - let j1 = lower2[13..14].load::(); - let j2 = lower2[11..12].load::(); - let s = instr2[10..11].load::(); - let imm = - (imm11 as u32) | - ((imm10 as u32) << 11) | - ((j1 as u32) << 21) | - ((j2 as u32) << 22) | - ((s as u32) << 23); - let imm = (imm << 8) >> 8; - inst.opcode = Opcode::BLX; - inst.operands = [ - Operand::Imm32(imm as u32), - Operand::Nothing, - Operand::Nothing, - Operand::Nothing, - ]; } else { - // `Brach with Link` (`A8-346`) - // v4T let imm11 = lower2[0..11].load::(); let imm10 = instr2[0..10].load::(); let j1 = lower2[13..14].load::(); let j2 = lower2[11..12].load::(); let s = instr2[10..11].load::(); + let i1 = 0x1 ^ s ^ j1; + let i2 = 0x1 ^ s ^ j2; let imm = - (imm11 as u32) | - ((imm10 as u32) << 11) | - ((j1 as u32) << 21) | - ((j2 as u32) << 22) | - ((s as u32) << 23); + (imm11 as i32) | + ((imm10 as i32) << 11) | + ((i2 as i32) << 21) | + ((i1 as i32) << 22) | + ((s as i32) << 23); let imm = (imm << 8) >> 8; - inst.opcode = Opcode::BL; inst.operands = [ - Operand::Imm32(imm as u32), + Operand::BranchThumbOffset(imm), Operand::Nothing, Operand::Nothing, Operand::Nothing, ]; + + if op1 & 0b101 == 0b001 { + // `Branch` (`A8-332`) + // T4 encoding + // v6T2 + inst.opcode = Opcode::B; + } else if op1 & 0b101 == 0b100 { + // `Branch with Link and Exchange` (`A8-346`) + // `UNDEFINED` in v4T + // v5T + // Undefined if low bit of imm10 is set ("H") + if imm11 & 0x1 != 0 { + return Err(DecodeError::Undefined); + } + inst.opcode = Opcode::BLX; + } else if op1 & 0b101 == 0b101 { + // `Brach with Link` (`A8-346`) + // v4T + inst.opcode = Opcode::BL; + } else { + // Permanently undefined by A6-13 + return Err(DecodeError::Undefined); + } } } } else { diff --git a/tests/armv7/thumb.rs b/tests/armv7/thumb.rs index 9182f6e..be5434f 100644 --- a/tests/armv7/thumb.rs +++ b/tests/armv7/thumb.rs @@ -723,6 +723,19 @@ fn test_decode_bcc_cases() { ); } #[test] +fn test_decode_32b_branch_cases() { + // encoding t4 + test_display( + &[0xf3, 0xf7, 0x7c, 0xbe], + "b.w $-0xc308" + ); + // encoding t4 + test_display( + &[0x0c, 0xf0, 0x84, 0xb9], + "b.w $+0xc308" + ); +} +#[test] fn test_decode_bkpt_cases() { test_display( &[0x00, 0xbe], -- cgit v1.1