Fixed TEST instruction handlers and tests. Updated TestImmWithRm8Handler and TestImmWithRm32Handler to properly check opcode in CanHandle and validate reg field in Decode. Improved test cases to use InstructionDecoder directly.

X86Disassembler/X86/Handlers/Group3/TestImmWithRm32Handler.cs

@@ -23,18 +23,9 @@ public class TestImmWithRm32Handler : Group3BaseHandler
     /// <returns>True if this handler can decode the opcode</returns>
     public override bool CanHandle(byte opcode)
     {
-        if (opcode != 0xF7)
+        // This handler only handles opcode 0xF7
-            return false;
+        // The reg field check (for TEST operation) will be done in the Decode method
-            
+        return opcode == 0xF7;
-        // Check if the reg field of the ModR/M byte is 0 (TEST)
-        int position = Decoder.GetPosition();
-        if (position >= Length)
-            return false;
-            
-        byte modRM = CodeBuffer[position];
-        byte reg = (byte)((modRM & 0x38) >> 3);
-        
-        return reg == 0; // 0 = TEST
     }
     
     /// <summary>
@@ -45,9 +36,6 @@ public class TestImmWithRm32Handler : Group3BaseHandler
     /// <returns>True if the instruction was successfully decoded</returns>
     public override bool Decode(byte opcode, Instruction instruction)
     {
-        // Set the mnemonic
-        instruction.Mnemonic = "test";
-        
         int position = Decoder.GetPosition();
         
         if (position >= Length)
@@ -57,15 +45,36 @@ public class TestImmWithRm32Handler : Group3BaseHandler
         
         // Read the ModR/M byte
         byte modRM = CodeBuffer[position++];
-        Decoder.SetPosition(position);
         
         // Extract the fields from the ModR/M byte
         byte mod = (byte)((modRM & 0xC0) >> 6);
         byte reg = (byte)((modRM & 0x38) >> 3); // Should be 0 for TEST
         byte rm = (byte)(modRM & 0x07);
         
-        // Decode the destination operand
+        // Check if the reg field is 0 (TEST operation)
-        string destOperand = _modRMDecoder.DecodeModRM(mod, rm, false);
+        if (reg != 0)
+        {
+            return false; // Not a TEST instruction
+        }
+        
+        // Set the mnemonic
+        instruction.Mnemonic = "test";
+        
+        Decoder.SetPosition(position);
+        
+        // Get the operand based on the addressing mode
+        string destOperand;
+        
+        // For direct register addressing (mod == 3), the r/m field specifies a register
+        if (mod == 3)
+        {
+            destOperand = GetRegister32(rm);
+        }
+        else
+        {
+            // Use the ModR/M decoder for memory addressing
+            destOperand = _modRMDecoder.DecodeModRM(mod, rm, false);
+        }
         
         // Read the immediate value
         if (position + 3 >= Length)
@@ -73,6 +82,7 @@ public class TestImmWithRm32Handler : Group3BaseHandler
             return false;
         }
         
+        // Read the immediate value using BitConverter
         uint imm32 = BitConverter.ToUInt32(CodeBuffer, position);
         Decoder.SetPosition(position + 4);
         

X86Disassembler/X86/Handlers/Group3/TestImmWithRm8Handler.cs

@@ -23,18 +23,9 @@ public class TestImmWithRm8Handler : Group3BaseHandler
     /// <returns>True if this handler can decode the opcode</returns>
     public override bool CanHandle(byte opcode)
     {
-        if (opcode != 0xF6)
+        // This handler only handles opcode 0xF6
-            return false;
+        // The reg field check (for TEST operation) will be done in the Decode method
-            
+        return opcode == 0xF6;
-        // Check if the reg field of the ModR/M byte is 0 (TEST)
-        int position = Decoder.GetPosition();
-        if (position >= Length)
-            return false;
-            
-        byte modRM = CodeBuffer[position];
-        byte reg = (byte)((modRM & 0x38) >> 3);
-        
-        return reg == 0; // 0 = TEST
     }
     
     /// <summary>
@@ -45,9 +36,6 @@ public class TestImmWithRm8Handler : Group3BaseHandler
     /// <returns>True if the instruction was successfully decoded</returns>
     public override bool Decode(byte opcode, Instruction instruction)
     {
-        // Set the mnemonic
-        instruction.Mnemonic = "test";
-        
         int position = Decoder.GetPosition();
         
         if (position >= Length)
@@ -57,20 +45,29 @@ public class TestImmWithRm8Handler : Group3BaseHandler
         
         // Read the ModR/M byte
         byte modRM = CodeBuffer[position++];
-        Decoder.SetPosition(position);
         
         // Extract the fields from the ModR/M byte
         byte mod = (byte)((modRM & 0xC0) >> 6);
         byte reg = (byte)((modRM & 0x38) >> 3); // Should be 0 for TEST
         byte rm = (byte)(modRM & 0x07);
         
-        // Decode the destination operand
+        // Check if the reg field is 0 (TEST operation)
+        if (reg != 0)
+        {
+            return false; // Not a TEST instruction
+        }
+        
+        // Set the mnemonic
+        instruction.Mnemonic = "test";
+        
+        Decoder.SetPosition(position);
+        
+        // Get the operand based on the addressing mode
         string destOperand;
         
-        // Special case for direct register addressing (mod == 3)
+        // For direct register addressing (mod == 3), the r/m field specifies a register
         if (mod == 3)
         {
-            // Get the register name based on the rm field
             destOperand = GetRegister8(rm);
         }
         else
@@ -93,15 +90,4 @@ public class TestImmWithRm8Handler : Group3BaseHandler
         
         return true;
     }
-    
-    /// <summary>
-    /// Gets the 8-bit register name for the given register index
-    /// </summary>
-    /// <param name="reg">The register index</param>
-    /// <returns>The register name</returns>
-    private static new string GetRegister8(byte reg)
-    {
-        string[] registerNames = { "al", "cl", "dl", "bl", "ah", "ch", "dh", "bh" };
-        return registerNames[reg & 0x07];
-    }
 }

X86Disassembler/X86/Handlers/TestEaxImmHandler.cs

@@ -39,13 +39,20 @@ public class TestEaxImmHandler : InstructionHandler
         
         int position = Decoder.GetPosition();
         
-        if (position + 4 > Length)
+        if (position + 3 >= Length)
         {
             return false;
         }
         
-        // Read the immediate value
+        // Read the immediate value - x86 is little-endian, so we need to read the bytes in the correct order
-        uint imm32 = BitConverter.ToUInt32(CodeBuffer, position);
+        byte b0 = CodeBuffer[position];
+        byte b1 = CodeBuffer[position + 1];
+        byte b2 = CodeBuffer[position + 2];
+        byte b3 = CodeBuffer[position + 3];
+        
+        // Combine the bytes to form a 32-bit immediate value
+        uint imm32 = (uint)(b0 | (b1 << 8) | (b2 << 16) | (b3 << 24));
+        
         Decoder.SetPosition(position + 4);
         
         // Set the operands

X86Disassembler/X86/Handlers/TestRegMem8Handler.cs

@@ -57,14 +57,30 @@ public class TestRegMem8Handler : InstructionHandler
         byte reg = (byte)((modRM & 0x38) >> 3);
         byte rm = (byte)(modRM & 0x07);
         
-        // Decode the destination operand
+        // For direct register addressing (mod == 3), the r/m field specifies a register
-        string destOperand = _modRMDecoder.DecodeModRM(mod, rm, true);
+        if (mod == 3)
+        {
+            // Get the register names
+            string rmReg = GetRegister8(rm);
+            string regReg = GetRegister8(reg);
             
-        // Get the source register
+            // Set the operands (TEST r/m8, r8)
-        string srcReg = GetRegister8(reg);
+            // In x86 assembly, the TEST instruction has the operand order r/m8, r8
+            // According to Ghidra and standard x86 assembly convention, it should be TEST CL,AL
+            // where CL is the r/m operand and AL is the reg operand
+            instruction.Operands = $"{rmReg}, {regReg}";
+        }
+        else
+        {
+            // Decode the memory operand
+            string memOperand = _modRMDecoder.DecodeModRM(mod, rm, true);
             
-        // Set the operands
+            // Get the register name
-        instruction.Operands = $"{destOperand}, {srcReg}";
+            string regReg = GetRegister8(reg);
+            
+            // Set the operands (TEST r/m8, r8)
+            instruction.Operands = $"{memOperand}, {regReg}";
+        }
         
         return true;
     }

X86Disassembler/X86/Handlers/TestRegMemHandler.cs

@@ -57,14 +57,30 @@ public class TestRegMemHandler : InstructionHandler
         byte reg = (byte)((modRM & 0x38) >> 3);
         byte rm = (byte)(modRM & 0x07);
         
-        // Decode the destination operand
+        // For direct register addressing (mod == 3), the r/m field specifies a register
-        string destOperand = _modRMDecoder.DecodeModRM(mod, rm, false);
+        if (mod == 3)
+        {
+            // Get the register names
+            string rmReg = GetRegister32(rm);
+            string regReg = GetRegister32(reg);
             
-        // Get the source register
+            // Set the operands (TEST r/m32, r32)
-        string srcReg = GetRegister32(reg);
+            // In x86 assembly, the TEST instruction has the operand order r/m32, r32
+            // According to Ghidra and standard x86 assembly convention, it should be TEST ECX,EAX
+            // where ECX is the r/m operand and EAX is the reg operand
+            instruction.Operands = $"{rmReg}, {regReg}";
+        }
+        else
+        {
+            // Decode the memory operand
+            string memOperand = _modRMDecoder.DecodeModRM(mod, rm, false);
             
-        // Set the operands
+            // Get the register name
-        instruction.Operands = $"{destOperand}, {srcReg}";
+            string regReg = GetRegister32(reg);
+            
+            // Set the operands (TEST r/m32, r32)
+            instruction.Operands = $"{memOperand}, {regReg}";
+        }
         
         return true;
     }

X86DisassemblerTests/GlobalUsings.cs

@@ -0,0 +1 @@
+global using Xunit;

X86DisassemblerTests/InstructionDecoderTests.cs

@@ -52,8 +52,8 @@ public class InstructionDecoderTests
         // Assert
         Assert.NotNull(instruction);
         Assert.Equal("test", instruction.Mnemonic);
-        // The actual implementation produces "al, cl" as the operands
+        // The correct operand order is TEST r/m8, r8 (TEST CL, AL)
-        Assert.Equal("al, cl", instruction.Operands);
+        Assert.Equal("cl, al", instruction.Operands);
         Assert.Equal(2, instruction.RawBytes.Length);
         Assert.Equal(0x84, instruction.RawBytes[0]);
         Assert.Equal(0xC1, instruction.RawBytes[1]);
@@ -76,8 +76,8 @@ public class InstructionDecoderTests
         // Assert
         Assert.NotNull(instruction);
         Assert.Equal("test", instruction.Mnemonic);
-        // The actual implementation produces "eax, ecx" as the operands
+        // The correct operand order is TEST r/m32, r32 (TEST ECX, EAX)
-        Assert.Equal("eax, ecx", instruction.Operands);
+        Assert.Equal("ecx, eax", instruction.Operands);
         Assert.Equal(2, instruction.RawBytes.Length);
         Assert.Equal(0x85, instruction.RawBytes[0]);
         Assert.Equal(0xC1, instruction.RawBytes[1]);
@@ -176,7 +176,6 @@ public class InstructionDecoderTests
         
         // Act - First instruction
         var instruction1 = decoder.DecodeInstruction();
-        Debug.WriteLine($"After first instruction, decoder position: {decoder.GetPosition()}");
         
         // Assert - First instruction
         Assert.NotNull(instruction1);
@@ -185,12 +184,11 @@ public class InstructionDecoderTests
         
         // Act - Second instruction
         var instruction2 = decoder.DecodeInstruction();
-        Debug.WriteLine($"After second instruction, decoder position: {decoder.GetPosition()}");
         
         // Assert - Second instruction
         Assert.NotNull(instruction2);
         Assert.Equal("jz", instruction2.Mnemonic);
         // The correct target address according to x86 architecture
-        Assert.Equal("0x00000032", instruction2?.Operands ?? string.Empty);
+        Assert.Equal("0x00000032", instruction2.Operands);
     }
 }

X86DisassemblerTests/TestInstructionHandlerTests.cs

@@ -0,0 +1,183 @@
+namespace X86DisassemblerTests;
+
+using System;
+using Xunit;
+using X86Disassembler.X86;
+using X86Disassembler.X86.Handlers;
+using X86Disassembler.X86.Handlers.Group3;
+
+/// <summary>
+/// Tests for TEST instruction handlers
+/// </summary>
+public class TestInstructionHandlerTests
+{
+    /// <summary>
+    /// Tests the TestRegMemHandler for decoding TEST r/m32, r32 instructions
+    /// </summary>
+    [Fact]
+    public void TestRegMemHandler_DecodesTestR32R32_Correctly()
+    {
+        // Arrange
+        // TEST ECX, EAX (85 C1) - ModR/M byte C1 = 11 000 001 (mod=3, reg=0, rm=1)
+        // mod=3 means direct register addressing, reg=0 is EAX, rm=1 is ECX
+        byte[] codeBuffer = new byte[] { 0x85, 0xC1 };
+        var decoder = new InstructionDecoder(codeBuffer, codeBuffer.Length);
+        
+        // Act
+        var instruction = decoder.DecodeInstruction();
+        
+        // Assert
+        Assert.NotNull(instruction);
+        Assert.Equal("test", instruction.Mnemonic);
+        // The ModR/M byte C1 = 11 000 001 (mod=3, reg=0, rm=1) means ECX is the r/m operand
+        // According to x86 assembly convention, the operand order should be TEST r/m32, r32
+        // So the correct operands should be "ecx, eax"
+        Assert.Equal("ecx, eax", instruction.Operands);
+    }
+    
+    /// <summary>
+    /// Tests the TestRegMem8Handler for decoding TEST r/m8, r8 instructions
+    /// </summary>
+    [Fact]
+    public void TestRegMem8Handler_DecodesTestR8R8_Correctly()
+    {
+        // Arrange
+        // TEST CL, AL (84 C1) - ModR/M byte C1 = 11 000 001 (mod=3, reg=0, rm=1)
+        // mod=3 means direct register addressing, reg=0 is AL, rm=1 is CL
+        byte[] codeBuffer = new byte[] { 0x84, 0xC1 };
+        var decoder = new InstructionDecoder(codeBuffer, codeBuffer.Length);
+        
+        // Act
+        var instruction = decoder.DecodeInstruction();
+        
+        // Assert
+        Assert.NotNull(instruction);
+        Assert.Equal("test", instruction.Mnemonic);
+        // The ModR/M byte C1 = 11 000 001 (mod=3, reg=0, rm=1) means CL is the r/m operand
+        // According to x86 assembly convention, the operand order should be TEST r/m8, r8
+        // So the correct operands should be "cl, al"
+        Assert.Equal("cl, al", instruction.Operands);
+    }
+    
+    /// <summary>
+    /// Tests the TestAlImmHandler for decoding TEST AL, imm8 instructions
+    /// </summary>
+    [Fact]
+    public void TestAlImmHandler_DecodesTestAlImm8_Correctly()
+    {
+        // Arrange
+        // TEST AL, 0x42 (A8 42)
+        byte[] codeBuffer = new byte[] { 0xA8, 0x42 };
+        var decoder = new InstructionDecoder(codeBuffer, codeBuffer.Length);
+        
+        // Act
+        var instruction = decoder.DecodeInstruction();
+        
+        // Assert
+        Assert.NotNull(instruction);
+        Assert.Equal("test", instruction.Mnemonic);
+        // The handler should produce "al, 0xXX" as the operands
+        Assert.Equal("al, 0x42", instruction.Operands);
+    }
+    
+    /// <summary>
+    /// Tests the TestEaxImmHandler for decoding TEST EAX, imm32 instructions
+    /// </summary>
+    [Fact]
+    public void TestEaxImmHandler_DecodesTestEaxImm32_Correctly()
+    {
+        // Arrange
+        // TEST EAX, 0x12345678 (A9 78 56 34 12)
+        byte[] codeBuffer = new byte[] { 0xA9, 0x78, 0x56, 0x34, 0x12 };
+        var decoder = new InstructionDecoder(codeBuffer, codeBuffer.Length);
+        
+        // Act
+        var instruction = decoder.DecodeInstruction();
+        
+        // Assert
+        Assert.NotNull(instruction);
+        Assert.Equal("test", instruction.Mnemonic);
+        // The handler should produce "eax, 0xXXXXXXXX" as the operands
+        Assert.Equal("eax, 0x12345678", instruction.Operands);
+    }
+    
+    /// <summary>
+    /// Tests that the TestImmWithRm8Handler can handle the correct opcode
+    /// </summary>
+    [Fact]
+    public void TestImmWithRm8Handler_CanHandle_ReturnsTrueForCorrectOpcode()
+    {
+        // Arrange
+        byte[] codeBuffer = new byte[] { 0xF6, 0xC0 }; // ModR/M byte C0 = 11 000 000 (mod=3, reg=0, rm=0)
+        var decoder = new InstructionDecoder(codeBuffer, codeBuffer.Length);
+        var handler = new TestImmWithRm8Handler(codeBuffer, decoder, codeBuffer.Length);
+        
+        // Act
+        bool result = handler.CanHandle(0xF6);
+        
+        // Assert
+        Assert.True(result);
+    }
+    
+    /// <summary>
+    /// Tests that the TestImmWithRm8Handler cannot handle an incorrect opcode
+    /// </summary>
+    [Fact]
+    public void TestImmWithRm8Handler_CanHandle_ReturnsFalseForIncorrectOpcode()
+    {
+        // Arrange
+        byte[] codeBuffer = new byte[] { 0xF7 };
+        var decoder = new InstructionDecoder(codeBuffer, codeBuffer.Length);
+        var handler = new TestImmWithRm8Handler(codeBuffer, decoder, codeBuffer.Length);
+        
+        // Act
+        bool result = handler.CanHandle(0xF7);
+        
+        // Assert
+        Assert.False(result);
+    }
+    
+    /// <summary>
+    /// Tests the TestImmWithRm8Handler for decoding TEST r/m8, imm8 instructions
+    /// </summary>
+    [Fact]
+    public void TestImmWithRm8Handler_DecodesTestRm8Imm8_Correctly()
+    {
+        // Arrange
+        // TEST AH, 0x01 (F6 C4 01) - ModR/M byte C4 = 11 000 100 (mod=3, reg=0, rm=4)
+        // mod=3 means direct register addressing, reg=0 indicates TEST operation, rm=4 is AH
+        byte[] codeBuffer = new byte[] { 0xF6, 0xC4, 0x01 };
+        var decoder = new InstructionDecoder(codeBuffer, codeBuffer.Length);
+        
+        // Act
+        var instruction = decoder.DecodeInstruction();
+        
+        // Assert
+        Assert.NotNull(instruction);
+        Assert.Equal("test", instruction.Mnemonic);
+        // The handler should produce "ah, 0xXX" as the operands
+        Assert.Equal("ah, 0x01", instruction.Operands);
+    }
+    
+    /// <summary>
+    /// Tests the TestImmWithRm32Handler for decoding TEST r/m32, imm32 instructions
+    /// </summary>
+    [Fact]
+    public void TestImmWithRm32Handler_DecodesTestRm32Imm32_Correctly()
+    {
+        // Arrange
+        // TEST EDI, 0x12345678 (F7 C7 78 56 34 12) - ModR/M byte C7 = 11 000 111 (mod=3, reg=0, rm=7)
+        // mod=3 means direct register addressing, reg=0 indicates TEST operation, rm=7 is EDI
+        byte[] codeBuffer = new byte[] { 0xF7, 0xC7, 0x78, 0x56, 0x34, 0x12 };
+        var decoder = new InstructionDecoder(codeBuffer, codeBuffer.Length);
+        
+        // Act
+        var instruction = decoder.DecodeInstruction();
+        
+        // Assert
+        Assert.NotNull(instruction);
+        Assert.Equal("test", instruction.Mnemonic);
+        // The handler should produce "edi, 0xXXXXXXXX" as the operands
+        Assert.Equal("edi, 0x12345678", instruction.Operands);
+    }
+}

X86DisassemblerTests/UnitTest1.cs

@@ -0,0 +1,10 @@
+namespace X86DisassemblerTests;
+
+public class UnitTest1
+{
+    [Fact]
+    public void Test1()
+    {
+
+    }
+}

X86DisassemblerTests/X86DisassemblerTests.csproj

@@ -0,0 +1,29 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>net8.0</TargetFramework>
+    <ImplicitUsings>enable</ImplicitUsings>
+    <Nullable>enable</Nullable>
+
+    <IsPackable>false</IsPackable>
+    <IsTestProject>true</IsTestProject>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.6.0" />
+    <PackageReference Include="xunit" Version="2.4.2" />
+    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.5">
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+      <PrivateAssets>all</PrivateAssets>
+    </PackageReference>
+    <PackageReference Include="coverlet.collector" Version="6.0.0">
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+      <PrivateAssets>all</PrivateAssets>
+    </PackageReference>
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\X86Disassembler\X86Disassembler.csproj" />
+  </ItemGroup>
+
+</Project>