test that instruction lengths are correct

fix several instances of incorrect instruction lengths

* immediates for `mov reg, imm` and some other instructions were double-counted
* lengths for vex prefixes were wrong all over the place

3 files changed, 8 insertions, 4 deletions

diff --git a/src/lib.rs b/src/lib.rs
index 9409ca9..500d59d 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -4757,7 +4757,6 @@ fn read_operands<T: Iterator<Item=u8>>(decoder: &InstDecoder, mut bytes_iter: T,
                 RegSpec::gp_from_parts(reg, instruction.prefixes.rex().b(), 1, instruction.prefixes.rex().present());
             instruction.imm =
                 read_imm_unsigned(&mut bytes_iter, 1, length)?;
-            *length += 1;
             instruction.operands[1] = OperandSpec::ImmU8;
             instruction.operand_count = 2;
         } else {
@@ -4774,7 +4773,6 @@ fn read_operands<T: Iterator<Item=u8>>(decoder: &InstDecoder, mut bytes_iter: T,
                 RegSpec::from_parts(reg, instruction.prefixes.rex().b(), bank);
             instruction.imm =
                 read_imm_ivq(&mut bytes_iter, opwidth, length)?;
-            *length += opwidth;
             instruction.operands[1] = match opwidth {
                 1 => OperandSpec::ImmI8,
                 2 => OperandSpec::ImmI16,
@@ -4947,7 +4945,6 @@ fn read_operands<T: Iterator<Item=u8>>(decoder: &InstDecoder, mut bytes_iter: T,
             instruction.operands[0] = mem_oper;
             let numwidth = if opwidth == 8 { 4 } else { opwidth };
             instruction.imm = read_imm_signed(&mut bytes_iter, numwidth, length)? as u64;
-            *length = numwidth;
             instruction.opcode = base_opcode_map((modrm >> 3) & 7);
             instruction.operands[1] = match opwidth {
                 1 => OperandSpec::ImmI8,
diff --git a/src/vex.rs b/src/vex.rs
index 99cee01..8a7434f 100644
--- a/src/vex.rs
+++ b/src/vex.rs
@@ -89,6 +89,7 @@ enum VEXOperandCode {
 pub(crate) fn three_byte_vex<T: Iterator<Item=u8>>(bytes: &mut T, instruction: &mut Instruction, mut length: u8) -> Result<(), DecodeError> {
     let vex_byte_one = bytes.next().ok_or(DecodeError::ExhaustedInput)?;
     let vex_byte_two = bytes.next().ok_or(DecodeError::ExhaustedInput)?;
+    length += 2;
     let p = vex_byte_two & 0x03;
     let p = match p {
         0x00 => VEXOpcodePrefix::None,
@@ -121,6 +122,7 @@ pub(crate) fn three_byte_vex<T: Iterator<Item=u8>>(bytes: &mut T, instruction: &
 
 pub(crate) fn two_byte_vex<T: Iterator<Item=u8>>(bytes: &mut T, instruction: &mut Instruction, mut length: u8) -> Result<(), DecodeError> {
     let vex_byte = bytes.next().ok_or(DecodeError::ExhaustedInput)?;
+    length += 1;
     let p = vex_byte & 0x03;
     let p = match p {
         0x00 => VEXOpcodePrefix::None,
@@ -553,6 +555,7 @@ fn read_vex_operands<T: Iterator<Item=u8>>(bytes: &mut T, instruction: &mut Inst
 
 fn read_vex_instruction<T: Iterator<Item=u8>>(opcode_map: VEXOpcodeMap, bytes: &mut T, instruction: &mut Instruction, length: &mut u8, p: VEXOpcodePrefix) -> Result<(), DecodeError> {
     let opc = bytes.next().ok_or(DecodeError::ExhaustedInput)?;
+    *length += 1;
 
     // the name of this bit is `L` in the documentation, so use the same name here.
     #[allow(non_snake_case)]
diff --git a/test/test.rs b/test/test.rs
index 0ffc1c4..07d715c 100644
--- a/test/test.rs
+++ b/test/test.rs
@@ -39,6 +39,9 @@ fn test_display_under(decoder: &InstDecoder, data: &[u8], expected: &'static str
                 text,
                 expected
             );
+            // while we're at it, test that the instruction is as long, and no longer, than its
+            // input
+            assert_eq!(instr.len() as usize, data.len(), "instruction length is incorrect, wanted instruction {}", expected);
         },
         Err(e) => {
             assert!(false, "decode error ({}) for {} under decoder {}:\n  expected: {}\n", e, hex, decoder, expected);
@@ -341,6 +344,7 @@ fn test_mov() {
     // test_display(&[0xa1, 0x93, 0x62, 0xc4, 0x00, 0x12, 0x34, 0x12, 0x34], "mov eax, [0x3412341200c46293]");
     // RCT.exe 32bit version, TODO: FIX
     test_display(&[0xa1, 0x93, 0x62, 0xc4, 0x00], "mov eax, [0xc46293]");
+    test_display(&[0xba, 0x01, 0x00, 0x00, 0x00], "mov edx, 0x1");
     test_display(&[0x48, 0xc7, 0x04, 0x24, 0x00, 0x00, 0x00, 0x00], "mov [rsp], 0x0");
     test_display(&[0x48, 0x89, 0x44, 0x24, 0x08], "mov [rsp + 0x8], rax");
     test_display(&[0x48, 0x89, 0x43, 0x18], "mov [rbx + 0x18], rax");
@@ -348,7 +352,7 @@ fn test_mov() {
     test_display(&[0x49, 0x89, 0x4e, 0x08], "mov [r14 + 0x8], rcx");
     test_display(&[0x48, 0x8b, 0x32], "mov rsi, [rdx]");
     test_display(&[0x49, 0x89, 0x46, 0x10], "mov [r14 + 0x10], rax");
-    test_display(&[0x4d, 0x0f, 0x43, 0xec, 0x49], "cmovnb r13, r12");
+    test_display(&[0x4d, 0x0f, 0x43, 0xec], "cmovnb r13, r12");
     test_display(&[0x0f, 0xb6, 0x06], "movzx eax, byte [rsi]");
     test_display(&[0x0f, 0xb7, 0x06], "movzx eax, word [rsi]");
     test_display(&[0x89, 0x55, 0x94], "mov [rbp - 0x6c], edx");