From fe0b04f5a12611ec3e5ab9ef918390224ae90501 Mon Sep 17 00:00:00 2001 From: bird_egop Date: Sat, 12 Apr 2025 21:21:03 +0300 Subject: [PATCH] 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. --- .../Handlers/Group3/TestImmWithRm32Handler.cs | 46 +++-- .../Handlers/Group3/TestImmWithRm8Handler.cs | 46 ++--- .../X86/Handlers/TestEaxImmHandler.cs | 13 +- .../X86/Handlers/TestRegMem8Handler.cs | 32 ++- .../X86/Handlers/TestRegMemHandler.cs | 32 ++- X86DisassemblerTests/GlobalUsings.cs | 1 + .../InstructionDecoderTests.cs | 12 +- .../TestInstructionHandlerTests.cs | 183 ++++++++++++++++++ X86DisassemblerTests/UnitTest1.cs | 10 + .../X86DisassemblerTests.csproj | 29 +++ 10 files changed, 330 insertions(+), 74 deletions(-) create mode 100644 X86DisassemblerTests/GlobalUsings.cs create mode 100644 X86DisassemblerTests/TestInstructionHandlerTests.cs create mode 100644 X86DisassemblerTests/UnitTest1.cs create mode 100644 X86DisassemblerTests/X86DisassemblerTests.csproj diff --git a/X86Disassembler/X86/Handlers/Group3/TestImmWithRm32Handler.cs b/X86Disassembler/X86/Handlers/Group3/TestImmWithRm32Handler.cs index 2c9832e..22d0894 100644 --- a/X86Disassembler/X86/Handlers/Group3/TestImmWithRm32Handler.cs +++ b/X86Disassembler/X86/Handlers/Group3/TestImmWithRm32Handler.cs @@ -23,18 +23,9 @@ public class TestImmWithRm32Handler : Group3BaseHandler /// True if this handler can decode the opcode public override bool CanHandle(byte opcode) { - if (opcode != 0xF7) - return false; - - // 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 + // This handler only handles opcode 0xF7 + // The reg field check (for TEST operation) will be done in the Decode method + return opcode == 0xF7; } /// @@ -45,9 +36,6 @@ public class TestImmWithRm32Handler : Group3BaseHandler /// True if the instruction was successfully decoded 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 - string destOperand = _modRMDecoder.DecodeModRM(mod, rm, false); + // 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; + + // 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); diff --git a/X86Disassembler/X86/Handlers/Group3/TestImmWithRm8Handler.cs b/X86Disassembler/X86/Handlers/Group3/TestImmWithRm8Handler.cs index e46a025..1274e6a 100644 --- a/X86Disassembler/X86/Handlers/Group3/TestImmWithRm8Handler.cs +++ b/X86Disassembler/X86/Handlers/Group3/TestImmWithRm8Handler.cs @@ -23,18 +23,9 @@ public class TestImmWithRm8Handler : Group3BaseHandler /// True if this handler can decode the opcode public override bool CanHandle(byte opcode) { - if (opcode != 0xF6) - return false; - - // 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 + // This handler only handles opcode 0xF6 + // The reg field check (for TEST operation) will be done in the Decode method + return opcode == 0xF6; } /// @@ -45,9 +36,6 @@ public class TestImmWithRm8Handler : Group3BaseHandler /// True if the instruction was successfully decoded 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; } - - /// - /// Gets the 8-bit register name for the given register index - /// - /// The register index - /// The register name - private static new string GetRegister8(byte reg) - { - string[] registerNames = { "al", "cl", "dl", "bl", "ah", "ch", "dh", "bh" }; - return registerNames[reg & 0x07]; - } } diff --git a/X86Disassembler/X86/Handlers/TestEaxImmHandler.cs b/X86Disassembler/X86/Handlers/TestEaxImmHandler.cs index 615bb81..2c5efb4 100644 --- a/X86Disassembler/X86/Handlers/TestEaxImmHandler.cs +++ b/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 - uint imm32 = BitConverter.ToUInt32(CodeBuffer, position); + // Read the immediate value - x86 is little-endian, so we need to read the bytes in the correct order + 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 diff --git a/X86Disassembler/X86/Handlers/TestRegMem8Handler.cs b/X86Disassembler/X86/Handlers/TestRegMem8Handler.cs index 1f72c32..37cefe9 100644 --- a/X86Disassembler/X86/Handlers/TestRegMem8Handler.cs +++ b/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 - string destOperand = _modRMDecoder.DecodeModRM(mod, rm, true); - - // Get the source register - string srcReg = GetRegister8(reg); - - // Set the operands - instruction.Operands = $"{destOperand}, {srcReg}"; + // For direct register addressing (mod == 3), the r/m field specifies a register + if (mod == 3) + { + // Get the register names + string rmReg = GetRegister8(rm); + string regReg = GetRegister8(reg); + + // Set the operands (TEST r/m8, r8) + // 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); + + // Get the register name + string regReg = GetRegister8(reg); + + // Set the operands (TEST r/m8, r8) + instruction.Operands = $"{memOperand}, {regReg}"; + } return true; } diff --git a/X86Disassembler/X86/Handlers/TestRegMemHandler.cs b/X86Disassembler/X86/Handlers/TestRegMemHandler.cs index 96c9eee..b2bb77a 100644 --- a/X86Disassembler/X86/Handlers/TestRegMemHandler.cs +++ b/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 - string destOperand = _modRMDecoder.DecodeModRM(mod, rm, false); - - // Get the source register - string srcReg = GetRegister32(reg); - - // Set the operands - instruction.Operands = $"{destOperand}, {srcReg}"; + // For direct register addressing (mod == 3), the r/m field specifies a register + if (mod == 3) + { + // Get the register names + string rmReg = GetRegister32(rm); + string regReg = GetRegister32(reg); + + // Set the operands (TEST r/m32, r32) + // 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); + + // Get the register name + string regReg = GetRegister32(reg); + + // Set the operands (TEST r/m32, r32) + instruction.Operands = $"{memOperand}, {regReg}"; + } return true; } diff --git a/X86DisassemblerTests/GlobalUsings.cs b/X86DisassemblerTests/GlobalUsings.cs new file mode 100644 index 0000000..8c927eb --- /dev/null +++ b/X86DisassemblerTests/GlobalUsings.cs @@ -0,0 +1 @@ +global using Xunit; \ No newline at end of file diff --git a/X86DisassemblerTests/InstructionDecoderTests.cs b/X86DisassemblerTests/InstructionDecoderTests.cs index feb9e0c..0d55d77 100644 --- a/X86DisassemblerTests/InstructionDecoderTests.cs +++ b/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 - Assert.Equal("al, cl", instruction.Operands); + // The correct operand order is TEST r/m8, r8 (TEST CL, AL) + 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 - Assert.Equal("eax, ecx", instruction.Operands); + // The correct operand order is TEST r/m32, r32 (TEST ECX, EAX) + 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); } } diff --git a/X86DisassemblerTests/TestInstructionHandlerTests.cs b/X86DisassemblerTests/TestInstructionHandlerTests.cs new file mode 100644 index 0000000..2893a45 --- /dev/null +++ b/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; + +/// +/// Tests for TEST instruction handlers +/// +public class TestInstructionHandlerTests +{ + /// + /// Tests the TestRegMemHandler for decoding TEST r/m32, r32 instructions + /// + [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); + } + + /// + /// Tests the TestRegMem8Handler for decoding TEST r/m8, r8 instructions + /// + [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); + } + + /// + /// Tests the TestAlImmHandler for decoding TEST AL, imm8 instructions + /// + [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); + } + + /// + /// Tests the TestEaxImmHandler for decoding TEST EAX, imm32 instructions + /// + [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); + } + + /// + /// Tests that the TestImmWithRm8Handler can handle the correct opcode + /// + [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); + } + + /// + /// Tests that the TestImmWithRm8Handler cannot handle an incorrect opcode + /// + [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); + } + + /// + /// Tests the TestImmWithRm8Handler for decoding TEST r/m8, imm8 instructions + /// + [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); + } + + /// + /// Tests the TestImmWithRm32Handler for decoding TEST r/m32, imm32 instructions + /// + [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); + } +} diff --git a/X86DisassemblerTests/UnitTest1.cs b/X86DisassemblerTests/UnitTest1.cs new file mode 100644 index 0000000..be011a5 --- /dev/null +++ b/X86DisassemblerTests/UnitTest1.cs @@ -0,0 +1,10 @@ +namespace X86DisassemblerTests; + +public class UnitTest1 +{ + [Fact] + public void Test1() + { + + } +} \ No newline at end of file diff --git a/X86DisassemblerTests/X86DisassemblerTests.csproj b/X86DisassemblerTests/X86DisassemblerTests.csproj new file mode 100644 index 0000000..1960604 --- /dev/null +++ b/X86DisassemblerTests/X86DisassemblerTests.csproj @@ -0,0 +1,29 @@ + + + + net8.0 + enable + enable + + false + true + + + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + + + + + +