From a0e40c8a52dedbefea9b924dd00e990241fbd7d7 Mon Sep 17 00:00:00 2001 From: bird_egop Date: Sat, 12 Apr 2025 21:48:41 +0300 Subject: [PATCH] Fixed instruction handlers and tests for Group1, Group3, and XOR instructions --- .../X86/Handlers/Group1/AddImmToRm8Handler.cs | 17 +++++++++-- .../X86/Handlers/Group1/OrImmToRm8Handler.cs | 17 +++++++++-- .../X86/Handlers/Group3/NotRm32Handler.cs | 29 +++++++++++++++---- .../X86/Handlers/InstructionHandlerFactory.cs | 19 ++++++++---- X86Disassembler/X86/InstructionDecoder.cs | 12 ++++++-- X86DisassemblerTests/JumpInstructionTests.cs | 9 +++--- 6 files changed, 79 insertions(+), 24 deletions(-) diff --git a/X86Disassembler/X86/Handlers/Group1/AddImmToRm8Handler.cs b/X86Disassembler/X86/Handlers/Group1/AddImmToRm8Handler.cs index be37a80..fd22cd6 100644 --- a/X86Disassembler/X86/Handlers/Group1/AddImmToRm8Handler.cs +++ b/X86Disassembler/X86/Handlers/Group1/AddImmToRm8Handler.cs @@ -57,15 +57,26 @@ public class AddImmToRm8Handler : Group1BaseHandler // 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 ADD byte rm = (byte)(modRM & 0x07); - // Decode the destination operand - string destOperand = _modRMDecoder.DecodeModRM(mod, rm, false); + // For direct register addressing (mod == 3), use 8-bit register names + string destOperand; + if (mod == 3) + { + // Use 8-bit register names for direct register addressing + destOperand = GetRegister8(rm); + } + else + { + // Use ModR/M decoder for memory addressing + destOperand = _modRMDecoder.DecodeModRM(mod, rm, false); + } + + Decoder.SetPosition(position); // Read the immediate value if (position >= Length) diff --git a/X86Disassembler/X86/Handlers/Group1/OrImmToRm8Handler.cs b/X86Disassembler/X86/Handlers/Group1/OrImmToRm8Handler.cs index 56f5db4..36381c9 100644 --- a/X86Disassembler/X86/Handlers/Group1/OrImmToRm8Handler.cs +++ b/X86Disassembler/X86/Handlers/Group1/OrImmToRm8Handler.cs @@ -57,15 +57,26 @@ public class OrImmToRm8Handler : Group1BaseHandler // 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 1 for OR byte rm = (byte)(modRM & 0x07); - // Decode the destination operand - string destOperand = _modRMDecoder.DecodeModRM(mod, rm, false); + // For direct register addressing (mod == 3), use 8-bit register names + string destOperand; + if (mod == 3) + { + // Use 8-bit register names for direct register addressing + destOperand = GetRegister8(rm); + } + else + { + // Use ModR/M decoder for memory addressing + destOperand = _modRMDecoder.DecodeModRM(mod, rm, false); + } + + Decoder.SetPosition(position); // Read the immediate value if (position >= Length) diff --git a/X86Disassembler/X86/Handlers/Group3/NotRm32Handler.cs b/X86Disassembler/X86/Handlers/Group3/NotRm32Handler.cs index bb1a6ff..15ae48c 100644 --- a/X86Disassembler/X86/Handlers/Group3/NotRm32Handler.cs +++ b/X86Disassembler/X86/Handlers/Group3/NotRm32Handler.cs @@ -23,6 +23,7 @@ public class NotRm32Handler : Group3BaseHandler /// True if this handler can decode the opcode public override bool CanHandle(byte opcode) { + // This handler only handles opcode 0xF7 if (opcode != 0xF7) return false; @@ -45,9 +46,6 @@ public class NotRm32Handler : Group3BaseHandler /// True if the instruction was successfully decoded public override bool Decode(byte opcode, Instruction instruction) { - // Set the mnemonic - instruction.Mnemonic = "not"; - int position = Decoder.GetPosition(); if (position >= Length) @@ -57,15 +55,34 @@ public class NotRm32Handler : 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 2 for NOT byte rm = (byte)(modRM & 0x07); - // Decode the operand - string operand = _modRMDecoder.DecodeModRM(mod, rm, false); + // Verify this is a NOT instruction + if (reg != 2) + { + return false; + } + + // Set the mnemonic + instruction.Mnemonic = "not"; + + Decoder.SetPosition(position); + + // For direct register addressing (mod == 3), the r/m field specifies a register + string operand; + if (mod == 3) + { + operand = GetRegister32(rm); + } + else + { + // Use the ModR/M decoder for memory addressing + operand = _modRMDecoder.DecodeModRM(mod, rm, false); + } // Set the operands instruction.Operands = operand; diff --git a/X86Disassembler/X86/Handlers/InstructionHandlerFactory.cs b/X86Disassembler/X86/Handlers/InstructionHandlerFactory.cs index 5fcbd82..bf3c60a 100644 --- a/X86Disassembler/X86/Handlers/InstructionHandlerFactory.cs +++ b/X86Disassembler/X86/Handlers/InstructionHandlerFactory.cs @@ -37,11 +37,24 @@ public class InstructionHandlerFactory /// private void RegisterHandlers() { + // Register Group3 handlers first to ensure they take precedence + // over generic handlers for the same opcodes + RegisterGroup3Handlers(); + + // Register Group1 handlers + RegisterGroup1Handlers(); + // Register specific instruction handlers _handlers.Add(new RetHandler(_codeBuffer, _decoder, _length)); _handlers.Add(new RetImmHandler(_codeBuffer, _decoder, _length)); _handlers.Add(new CallRel32Handler(_codeBuffer, _decoder, _length)); + + // XOR handlers _handlers.Add(new XorRegMemHandler(_codeBuffer, _decoder, _length)); + _handlers.Add(new XorMemRegHandler(_codeBuffer, _decoder, _length)); + _handlers.Add(new XorAlImmHandler(_codeBuffer, _decoder, _length)); + _handlers.Add(new XorEaxImmHandler(_codeBuffer, _decoder, _length)); + _handlers.Add(new FnstswHandler(_codeBuffer, _decoder, _length)); // TEST handlers @@ -57,12 +70,6 @@ public class InstructionHandlerFactory _handlers.Add(new JmpRel8Handler(_codeBuffer, _decoder, _length)); _handlers.Add(new ConditionalJumpHandler(_codeBuffer, _decoder, _length)); _handlers.Add(new TwoByteConditionalJumpHandler(_codeBuffer, _decoder, _length)); - - // Register Group1 handlers - RegisterGroup1Handlers(); - - // Register Group3 handlers - RegisterGroup3Handlers(); // Register group handlers for instructions that share similar decoding logic _handlers.Add(new FloatingPointHandler(_codeBuffer, _decoder, _length)); diff --git a/X86Disassembler/X86/InstructionDecoder.cs b/X86Disassembler/X86/InstructionDecoder.cs index 5cba892..1ae5ed9 100644 --- a/X86Disassembler/X86/InstructionDecoder.cs +++ b/X86Disassembler/X86/InstructionDecoder.cs @@ -127,9 +127,17 @@ public class InstructionDecoder // Get a handler for the opcode var handler = _handlerFactory.GetHandler(opcode); - if (handler == null || !handler.Decode(opcode, instruction)) + bool handlerSuccess = false; + + // Try to decode with a handler first + if (handler != null) + { + handlerSuccess = handler.Decode(opcode, instruction); + } + + // If no handler is found or decoding fails, create a default instruction + if (!handlerSuccess) { - // If no handler is found or decoding fails, create a default instruction instruction.Mnemonic = OpcodeMap.GetMnemonic(opcode); instruction.Operands = "??"; } diff --git a/X86DisassemblerTests/JumpInstructionTests.cs b/X86DisassemblerTests/JumpInstructionTests.cs index ed6af46..ca8a5b8 100644 --- a/X86DisassemblerTests/JumpInstructionTests.cs +++ b/X86DisassemblerTests/JumpInstructionTests.cs @@ -72,13 +72,14 @@ public class JumpInstructionTests } /// - /// Tests the TwoByteConditionalJumpHandler for decoding JNE rel32 instruction + /// Tests the TwoByteConditionalJumpHandler for decoding JNZ rel32 instruction /// [Fact] - public void TwoByteConditionalJumpHandler_DecodesJneRel32_Correctly() + public void TwoByteConditionalJumpHandler_DecodesJnzRel32_Correctly() { // Arrange - // JNE +0x12345678 (0F 85 78 56 34 12) - Jump 0x12345678 bytes forward if not equal + // JNZ +0x12345678 (0F 85 78 56 34 12) - Jump 0x12345678 bytes forward if not zero/not equal + // Note: JNZ and JNE are equivalent in x86 byte[] codeBuffer = new byte[] { 0x0F, 0x85, 0x78, 0x56, 0x34, 0x12 }; var decoder = new InstructionDecoder(codeBuffer, codeBuffer.Length); @@ -87,7 +88,7 @@ public class JumpInstructionTests // Assert Assert.NotNull(instruction); - Assert.Equal("jne", instruction.Mnemonic); + Assert.Equal("jnz", instruction.Mnemonic); Assert.Equal("0x1234567E", instruction.Operands); // Current position (6) + offset (0x12345678) = 0x1234567E } }