cpu: fix bug in Movzx16

martinlindhe · Jan 8, 2020 · bc06e5c · bc06e5c
1 parent fdc1d2d
commit bc06e5c
Show file tree

Hide file tree

Showing 10 changed files with 45 additions and 17 deletions.
diff --git a/docs/render/demo_com_16bit/13_lava.png b/docs/render/demo_com_16bit/13_lava.png
diff --git a/docs/render/demo_com_16bit/13_proto256.png b/docs/render/demo_com_16bit/13_proto256.png
diff --git a/docs/render/demo_com_32bit/13_voronoy.png b/docs/render/demo_com_32bit/13_voronoy.png
diff --git a/docs/render/demo_com_32bit/13_xwater.png b/docs/render/demo_com_32bit/13_xwater.png
diff --git a/dustbox/src/cpu/encoder.rs b/dustbox/src/cpu/encoder.rs
@@ -215,6 +215,26 @@ impl Encoder {
                 out.push(0xAC);
                 out.extend(self.encode_rm_r_imm(&op.params));
             }
+            Op::Movsx16 => {
+               // MOVSX r16, r/m8
+               out.push(0x0F);
+               out.push(0xBE);
+               if let Parameter::Reg16(r) = op.params.dst {
+                   out.extend(self.encode_rm(&op.params.src, r.u8()));
+               } else {
+                   return Err(EncodeError::UnhandledParameter(op.params.dst.clone()));
+               }
+            }
+            Op::Movzx16 => {
+                // MOVZX r16, r/m8
+                out.push(0x0F);
+                out.push(0xB6);
+                if let Parameter::Reg16(r) = op.params.dst {
+                    out.extend(self.encode_rm(&op.params.src, r.u8()));
+                } else {
+                    return Err(EncodeError::UnhandledParameter(op.params.dst.clone()));
+                }
+            }
             Op::Mov8 => {
                 match op.params.dst {
                     Parameter::Reg8(r) => {

diff --git a/dustbox/src/cpu/encoder_test.rs b/dustbox/src/cpu/encoder_test.rs
@@ -566,6 +566,13 @@ fn can_encode_mov32() {
     assert_encdec(&op, "mov ebx,0x11228844", vec!(0x66, 0xBB, 0x44, 0x88, 0x22, 0x11));
 }
 
+#[test]
+fn can_encode_movzx16() {
+    // r16, imm16
+    let op = Instruction::new2(Op::Movzx16, Parameter::Reg16(R::AX), Parameter::Reg8(R::BL));
+    assert_encdec(&op, "movzx ax,bl", vec!(0x0F, 0xB6, 0xC3));
+}
+
 // TODO make this into a macro to retain caller line numbers in the asserts
 fn assert_encdec(op :&Instruction, expected_ndisasm: &str, expected_bytes: Vec<u8>) {
     let encoder = Encoder::new();

diff --git a/dustbox/src/cpu/register.rs b/dustbox/src/cpu/register.rs
@@ -109,6 +109,10 @@ impl R {
         }
     }
 
+    pub fn u8(self) -> u8 {
+        self.index() as u8
+    }
+
     pub fn is_gpr(self) -> bool {
         match self {
             R::AL | R::CL | R::DL | R::BL | R::AH | R::CH | R::DH | R::BH |

diff --git a/dustbox/src/machine.rs b/dustbox/src/machine.rs
@@ -1596,7 +1596,6 @@ impl Machine {
                 let src = self.cpu.read_parameter_value(&self.mmu, &op.params.src) as u8;
 
                 let mut data = u16::from(src);
-                // XXX should not work identical as Movzx16
                 if src & 0x80 != 0 {
                     data += 0xFF00;
                 }
@@ -1609,33 +1608,24 @@ impl Machine {
                 let src = self.cpu.read_parameter_value(&self.mmu, &op.params.src) as u8;
 
                 let mut data = u32::from(src);
-                // XXX should not work identical as Movzx16
                 if src & 0x80 != 0 {
                     data += 0xFFFF_FF00;
                 }
                 self.cpu.write_parameter_u32(&mut self.mmu, op.segment_prefix, &op.params.dst, data);
             }
             Op::Movzx16 => {
                 // 80386+
-                // moves an unsigned value into a register and zero-extends it with zero.
+                // moves an unsigned value into a register and zero-extends it.
                 // two arguments (dst=reg)
-                let src = self.cpu.read_parameter_value(&self.mmu, &op.params.src) as u8;
-                let mut data = u16::from(src);
-                if src & 0x80 != 0 {
-                    data += 0xFF00;
-                }
-                self.cpu.write_parameter_u16(&mut self.mmu, op.segment_prefix, &op.params.dst, data);
+                let val = self.cpu.read_parameter_value(&self.mmu, &op.params.src) as u16;
+                self.cpu.write_parameter_u16(&mut self.mmu, op.segment_prefix, &op.params.dst, val);
             }
             Op::Movzx32 => {
                 // 80386+
-                // moves an unsigned value into a register and zero-extends it with zero.
+                // moves an unsigned value into a register and zero-extends it.
                 // two arguments (dst=reg)
-                let src = self.cpu.read_parameter_value(&self.mmu, &op.params.src) as u8;
-                let mut data = u32::from(src);
-                if src & 0x80 != 0 {
-                    data += 0xFFFF_FF00;
-                }
-                self.cpu.write_parameter_u32(&mut self.mmu, op.segment_prefix, &op.params.dst, data);
+                let val = self.cpu.read_parameter_value(&self.mmu, &op.params.src) as u32;
+                self.cpu.write_parameter_u32(&mut self.mmu, op.segment_prefix, &op.params.dst, val);
             }
             Op::Mul8 => {
                 // Unsigned multiply (AX ← AL ∗ r/m8).

diff --git a/fuzzer/src/bin/fuzzer-main.rs b/fuzzer/src/bin/fuzzer-main.rs
@@ -60,6 +60,7 @@ fn main() {
         Op::Rcl8, Op::Rcr8, Op::Ror8, Op::Shl8, Op::Rol8,
 
         // SEEMS ALL OK:
+        Op::Movzx16, Op::Movsx16,
         Op::Shr8, Op::Sar8, // OK !
         Op::Div8, Op::Div16, Op::Idiv8, Op::Idiv16, // seems correct. NOTE that winxp crashes with "Divide overflow" on some input
         Op::Bt, Op::Bsf,

diff --git a/fuzzer/src/fuzzer.rs b/fuzzer/src/fuzzer.rs
@@ -209,7 +209,8 @@ impl AffectedFlags {
     /// returns a flag mask for affected flag registers by op
     pub fn for_op(op: &Op) -> u16 {
         match *op {
-            Op::Nop | Op::Mov8 | Op::Mov16 | Op::Mov32 | Op::Not8 | Op::Not16 |
+            Op::Nop | Op::Mov8 | Op::Mov16 | Op::Mov32 | Op::Movzx16 | Op::Movsx16 |
+            Op::Not8 | Op::Not16 |
             Op::Div8 | Op::Div16 | Op::Idiv8 | Op::Idiv16 | Op::Xchg8 | Op::Xchg16 |
             Op::Salc | Op::Cbw | Op::Cwd16 | Op::Lahf | Op::Lea16 | Op::Xlatb =>
                 AffectedFlags{s:0, z:0, p:0, c:0, a:0, o:0, d:0, i:0}.mask(), // none
@@ -619,6 +620,11 @@ fn get_mutator_snippet<RNG: Rng + ?Sized>(op: &Op, rng: &mut RNG) -> Vec<Instruc
             Instruction::new2(Op::Mov16, Parameter::Reg16(R::AX), Parameter::Imm16(rng.gen())),
             Instruction::new2(op.clone(), Parameter::Reg16(R::AX), Parameter::Imm16(rng.gen())),
         )}
+        Op::Movzx16 | Op::Movsx16 => { vec!(
+            // movzx AX, bl
+            Instruction::new2(Op::Mov8, Parameter::Reg8(R::BL), Parameter::Imm8(rng.gen())),
+            Instruction::new2(op.clone(), Parameter::Reg16(R::AX), Parameter::Reg8(R::BL)),
+        )}
         Op::Inc16 | Op::Dec16 | Op::Not16 | Op::Neg16 => { vec!(
             // mutate ax: r/m16
             Instruction::new2(Op::Mov16, Parameter::Reg16(R::AX), Parameter::Imm16(rng.gen())),