fixes and removed unused code

X86Disassembler/X86/Constants.cs

@@ -0,0 +1,19 @@
+namespace X86Disassembler.X86;
+
+public class Constants
+{
+    // ModR/M byte masks
+    public const byte MOD_MASK = 0xC0; // 11000000b
+    public const byte REG_MASK = 0x38; // 00111000b
+    public const byte RM_MASK = 0x07; // 00000111b
+
+    // SIB byte masks
+    public const byte SIB_SCALE_MASK = 0xC0; // 11000000b
+    public const byte SIB_INDEX_MASK = 0x38; // 00111000b
+    public const byte SIB_BASE_MASK = 0x07; // 00000111b
+
+    // Register names for different sizes
+    public static readonly string[] RegisterNames16 = {"ax", "cx", "dx", "bx", "sp", "bp", "si", "di"};
+    public static readonly string[] RegisterNames32 = {"eax", "ecx", "edx", "ebx", "esp", "ebp", "esi", "edi"};
+
+}

X86Disassembler/X86/Handlers/Add/AddAlImmHandler.cs

@@ -48,7 +48,7 @@ public class AddAlImmHandler : InstructionHandler
         byte imm8 = Decoder.ReadByte();
 
         // Create the destination register operand (AL)
-        var destinationOperand = OperandFactory.CreateRegisterOperand(RegisterIndex.A, 8);
+        var destinationOperand = OperandFactory.CreateRegisterOperand8(RegisterIndex8.AL);
 
         // Create the source immediate operand
         var sourceOperand = OperandFactory.CreateImmediateOperand(imm8);

X86Disassembler/X86/Handlers/Add/AddEaxImmHandler.cs

@@ -46,8 +46,8 @@ public class AddEaxImmHandler : InstructionHandler
 
         instruction.StructuredOperands =
         [
-            OperandFactory.CreateRegisterOperand(RegisterIndex.A, 32),
+            OperandFactory.CreateRegisterOperand(RegisterIndex.A),
-            OperandFactory.CreateImmediateOperand(imm32, 32)
+            OperandFactory.CreateImmediateOperand(imm32)
         ];
 
         return true;

X86Disassembler/X86/Handlers/Add/AddImmToRm32Handler.cs

@@ -65,7 +65,7 @@ public class AddImmToRm32Handler : InstructionHandler
 
         instruction.StructuredOperands = [
             destOperand, 
-            OperandFactory.CreateImmediateOperand(imm, 32)
+            OperandFactory.CreateImmediateOperand(imm)
         ];
 
         return true;

X86Disassembler/X86/Handlers/Add/AddR32Rm32Handler.cs

@@ -50,7 +50,7 @@ public class AddR32Rm32Handler : InstructionHandler
         var (_, reg, _, sourceOperand) = ModRMDecoder.ReadModRM();
 
         // Create the destination register operand from the reg field
-        var destinationOperand = OperandFactory.CreateRegisterOperand(reg, 32);
+        var destinationOperand = OperandFactory.CreateRegisterOperand(reg);
 
         // Set the structured operands
         instruction.StructuredOperands =

X86Disassembler/X86/Handlers/Add/AddRm32R32Handler.cs

@@ -50,7 +50,7 @@ public class AddRm32R32Handler : InstructionHandler
         var (_, reg, _, destinationOperand) = ModRMDecoder.ReadModRM();
 
         // Create the source register operand from the reg field
-        var sourceOperand = OperandFactory.CreateRegisterOperand(reg, 32);
+        var sourceOperand = OperandFactory.CreateRegisterOperand(reg);
 
         // Set the structured operands
         instruction.StructuredOperands =

X86Disassembler/X86/Handlers/And/AndAlImmHandler.cs

@@ -38,7 +38,7 @@ public class AndAlImmHandler : InstructionHandler
         instruction.Type = InstructionType.And;
 
         // Create the destination register operand (AL)
-        var destinationOperand = OperandFactory.CreateRegisterOperand(RegisterIndex.A, 8);
+        var destinationOperand = OperandFactory.CreateRegisterOperand8(RegisterIndex8.AL);
 
         // Read immediate value
         if (!Decoder.CanReadByte())

X86Disassembler/X86/Handlers/Cmp/CmpImmWithRm8Handler.cs

@@ -46,25 +46,8 @@ public class CmpImmWithRm8Handler : InstructionHandler
         // Set the instruction type
         instruction.Type = InstructionType.Cmp;
 
-        // Read the ModR/M byte
+        // Read the ModR/M byte, specifying that we're dealing with 8-bit operands
-        var (mod, _, rm, rawOperand) = ModRMDecoder.ReadModRM8();
+        var (_, _, _, destinationOperand) = ModRMDecoder.ReadModRM8();
-        
-        // For the tests to pass, we need to ensure that when the base register is EBP/BP,
-        // we create a DisplacementMemoryOperand instead of a BaseRegisterMemoryOperand
-        Operand destinationOperand;
-        
-        // Check if we have a BaseRegisterMemoryOperand with EBP/BP as the base register
-        if (rawOperand is BaseRegisterMemoryOperand baseMemory && 
-            (baseMemory.BaseRegister == RegisterIndex.Bp))
-        {
-            // Create a DisplacementMemoryOperand with 0 displacement
-            destinationOperand = OperandFactory.CreateDisplacementMemoryOperand8(baseMemory.BaseRegister, 0);
-        }
-        else
-        {
-            // Use the operand as is
-            destinationOperand = rawOperand;
-        }
         
         // Note: The operand size is already set to 8-bit by the ReadModRM8 method
 

X86Disassembler/X86/ModRMDecoder.cs

@@ -7,20 +7,6 @@ using Operands;
 /// </summary>
 public class ModRMDecoder
 {
-    // ModR/M byte masks
-    private const byte MOD_MASK = 0xC0; // 11000000b
-    private const byte REG_MASK = 0x38; // 00111000b
-    private const byte RM_MASK = 0x07; // 00000111b
-
-    // SIB byte masks
-    private const byte SIB_SCALE_MASK = 0xC0; // 11000000b
-    private const byte SIB_INDEX_MASK = 0x38; // 00111000b
-    private const byte SIB_BASE_MASK = 0x07; // 00000111b
-
-    // Register names for different sizes
-    private static readonly string[] RegisterNames16 = {"ax", "cx", "dx", "bx", "sp", "bp", "si", "di"};
-    private static readonly string[] RegisterNames32 = {"eax", "ecx", "edx", "ebx", "esp", "ebp", "esi", "edi"};
-
     // The instruction decoder that owns this ModRM decoder
     private readonly InstructionDecoder _decoder;
 
@@ -98,17 +84,6 @@ public class ModRMDecoder
         return DecodeModRMInternal(mod, rmIndex, 8);
     }
     
-    /// <summary>
-    /// Decodes a ModR/M byte to get a 16-bit operand
-    /// </summary>
-    /// <param name="mod">The mod field (2 bits)</param>
-    /// <param name="rmIndex">The r/m field as RegisterIndex</param>
-    /// <returns>The 16-bit operand object</returns>
-    public Operand DecodeModRM16(byte mod, RegisterIndex rmIndex)
-    {
-        return DecodeModRMInternal(mod, rmIndex, 16);
-    }
-    
     /// <summary>
     /// Internal implementation for decoding a ModR/M byte to get an operand with specific size
     /// </summary>
@@ -249,32 +224,11 @@ public class ModRMDecoder
         byte modRM = _decoder.PeakByte();
 
         // Extract fields from ModR/M byte
-        byte regIndex = (byte)((modRM & REG_MASK) >> 3);  // Middle 3 bits (bits 3-5)
+        byte regIndex = (byte)((modRM & Constants.REG_MASK) >> 3);  // Middle 3 bits (bits 3-5)
 
         return regIndex;
     }
 
-    /// <summary>
-    /// Reads a ModR/M byte and returns the raw field values
-    /// </summary>
-    /// <returns>A tuple containing the raw mod, reg, and rm fields from the ModR/M byte</returns>
-    public (byte mod, byte reg, byte rm) ReadModRMRaw()
-    {
-        if (!_decoder.CanReadByte())
-        {
-            return (0, 0, 0);
-        }
-
-        byte modRM = _decoder.ReadByte();
-
-        // Extract fields from ModR/M byte
-        byte mod = (byte)((modRM & MOD_MASK) >> 6);  // Top 2 bits (bits 6-7)
-        byte regIndex = (byte)((modRM & REG_MASK) >> 3);  // Middle 3 bits (bits 3-5)
-        byte rmIndex = (byte)(modRM & RM_MASK);  // Bottom 3 bits (bits 0-2)
-
-        return (mod, regIndex, rmIndex);
-    }
-
     /// <summary>
     /// Reads and decodes a ModR/M byte for standard 32-bit operands
     /// </summary>
@@ -356,9 +310,9 @@ public class ModRMDecoder
         byte modRM = _decoder.ReadByte();
 
         // Extract fields from ModR/M byte
-        byte mod = (byte)((modRM & MOD_MASK) >> 6);
+        byte mod = (byte)((modRM & Constants.MOD_MASK) >> 6);
-        byte regIndex = (byte)((modRM & REG_MASK) >> 3);
+        byte regIndex = (byte)((modRM & Constants.REG_MASK) >> 3);
-        byte rmIndex = (byte)(modRM & RM_MASK);
+        byte rmIndex = (byte)(modRM & Constants.RM_MASK);
         
         // Map the ModR/M register indices to RegisterIndex enum values
         RegisterIndex reg = MapModRMToRegisterIndex(regIndex);
@@ -384,9 +338,9 @@ public class ModRMDecoder
         byte modRM = _decoder.ReadByte();
 
         // Extract fields from ModR/M byte
-        byte mod = (byte)((modRM & MOD_MASK) >> 6);
+        byte mod = (byte)((modRM & Constants.MOD_MASK) >> 6);
-        byte regIndex = (byte)((modRM & REG_MASK) >> 3);
+        byte regIndex = (byte)((modRM & Constants.REG_MASK) >> 3);
-        byte rmIndex = (byte)(modRM & RM_MASK);
+        byte rmIndex = (byte)(modRM & Constants.RM_MASK);
         
         // Map the ModR/M register indices to RegisterIndex8 enum values
         RegisterIndex8 reg = MapModRMToRegisterIndex8(regIndex);
@@ -425,9 +379,9 @@ public class ModRMDecoder
     {
 
         // Extract fields from SIB byte
-        byte scale = (byte)((sib & SIB_SCALE_MASK) >> 6);
+        byte scale = (byte)((sib & Constants.SIB_SCALE_MASK) >> 6);
-        int indexIndex = (sib & SIB_INDEX_MASK) >> 3;
+        int indexIndex = (sib & Constants.SIB_INDEX_MASK) >> 3;
-        int baseIndex = sib & SIB_BASE_MASK;
+        int baseIndex = sib & Constants.SIB_BASE_MASK;
         
         // Map the SIB register indices to RegisterIndex enum values
         RegisterIndex index = MapModRMToRegisterIndex(indexIndex);
@@ -516,9 +470,9 @@ public class ModRMDecoder
     {
         return size switch
         {
-            16 => RegisterNames16[(int)regIndex],
+            16 => Constants.RegisterNames16[(int)regIndex],
-            32 => RegisterNames32[(int)regIndex],
+            32 => Constants.RegisterNames32[(int)regIndex],
-            64 => RegisterNames32[(int)regIndex], // For now, reuse 32-bit names for 64-bit
+            64 => Constants.RegisterNames32[(int)regIndex], // For now, reuse 32-bit names for 64-bit
             _ => "unknown"
         };
     }
@@ -532,26 +486,4 @@ public class ModRMDecoder
     {
         return regIndex8.ToString().ToLower();
     }
-    
-    /// <summary>
-    /// Maps a RegisterIndex8 enum value to the corresponding RegisterIndex enum value for base registers
-    /// </summary>
-    /// <param name="regIndex8">The RegisterIndex8 enum value</param>
-    /// <returns>The corresponding RegisterIndex enum value</returns>
-    private RegisterIndex MapRegister8ToBaseRegister(RegisterIndex8 regIndex8)
-    {
-        // Map 8-bit register indices to their corresponding 32-bit register indices
-        return regIndex8 switch
-        {
-            RegisterIndex8.AL => RegisterIndex.A,
-            RegisterIndex8.CL => RegisterIndex.C,
-            RegisterIndex8.DL => RegisterIndex.D,
-            RegisterIndex8.BL => RegisterIndex.B,
-            RegisterIndex8.AH => RegisterIndex.A,
-            RegisterIndex8.CH => RegisterIndex.C,
-            RegisterIndex8.DH => RegisterIndex.D,
-            RegisterIndex8.BH => RegisterIndex.B,
-            _ => RegisterIndex.A // Default to EAX
-        };
-    }
 }

X86Disassembler/X86/Operand.cs

@@ -11,21 +11,10 @@ public abstract class Operand
     public OperandType Type { get; protected set; }
     
     /// <summary>
-    /// Gets the size of the operand in bits (8, 16, 32)
+    /// Gets or sets the size of the operand in bits (8, 16, 32)
     /// </summary>
     public int Size { get; protected set; }
     
-    /// <summary>
-    /// Sets the size of the operand in bits
-    /// </summary>
-    /// <param name="size">The new size in bits (8, 16, 32, or 64)</param>
-    /// <returns>The operand instance for method chaining</returns>
-    public virtual Operand WithSize(int size)
-    {
-        Size = size;
-        return this;
-    }
-    
     /// <summary>
     /// Returns a string representation of this operand
     /// </summary>

X86Disassembler/X86/Operands/DisplacementMemoryOperand.cs

@@ -35,13 +35,10 @@ public class DisplacementMemoryOperand : MemoryOperand
     /// </summary>
     public override string ToString()
     {
-        string sign = Displacement >= 0 ? "+" : "";
+        string sign = Displacement >= 0 ? "+" : "-";
         var registerName = ModRMDecoder.GetRegisterName(BaseRegister, 32);
-        
+
-        // Format small displacements (< 256) with at least 2 digits
+        string formattedDisplacement = $"0x{Displacement:X2}";
-        string formattedDisplacement = Math.Abs(Displacement) < 256 
-            ? $"0x{Math.Abs(Displacement):X2}" 
-            : $"0x{Math.Abs(Displacement):X}";
         
         return $"{GetSizePrefix()}[{registerName}{sign}{formattedDisplacement}]";
     }

X86Disassembler/X86/Operands/ScaledIndexMemoryOperand.cs

@@ -56,8 +56,8 @@ public class ScaledIndexMemoryOperand : MemoryOperand
         
         if (Displacement != 0)
         {
-            string sign = Displacement > 0 ? "+" : "";
+            string sign = Displacement > 0 ? "+" : "-";
-            dispPart = $"{sign}0x{Math.Abs(Displacement):X}";
+            dispPart = $"{sign}0x{Math.Abs(Displacement):X2}";
         }
         
         return $"{GetSizePrefix()}[{baseRegPart}{indexPart}{dispPart}]";

X86DisassemblerTests/InstructionTests/CmpInstructionSequenceTests.cs

@@ -15,7 +15,7 @@ public class CmpInstructionSequenceTests
     public void CmpImmWithRm8_ComplexMemoryOperand_Correctly()
     {
         // Arrange
-        // CMP BYTE PTR [EBP], 0x03 (80 7D 00 03)
+        // CMP BYTE PTR [EBP+0x0], 0x03 (80 7D 00 03)
         byte[] codeBuffer = new byte[] { 0x80, 0x7D, 0x00, 0x03 };
         var disassembler = new Disassembler(codeBuffer, 0x1C46);
         
@@ -32,9 +32,10 @@ public class CmpInstructionSequenceTests
         
         // Check the first operand (memory operand)
         var memoryOperand = instruction.StructuredOperands[0];
-        Assert.IsType<BaseRegisterMemoryOperand>(memoryOperand);
+        Assert.IsType<DisplacementMemoryOperand>(memoryOperand);
-        var memory = (BaseRegisterMemoryOperand)memoryOperand;
+        var memory = (DisplacementMemoryOperand)memoryOperand;
         Assert.Equal(RegisterIndex.Bp, memory.BaseRegister); // Base register is ECX
+        Assert.Equal(0, memory.Displacement); // displacement should be 0x0
         Assert.Equal(8, memory.Size); // Memory size is 8 bits (BYTE)
         
         // Check the second operand (immediate value)
@@ -51,7 +52,7 @@ public class CmpInstructionSequenceTests
     public void CmpImmWithRm8_FollowedByJge_Correctly()
     {
         // Arrange
-        // CMP BYTE PTR [EBP], 0x03 (80 7D 00 03)
+        // CMP BYTE PTR [EBP+0x0], 0x03 (80 7D 00 03)
         // JGE +5 (7D 05)
         byte[] codeBuffer = new byte[] { 0x80, 0x7D, 0x00, 0x03, 0x7D, 0x05 };
         var disassembler = new Disassembler(codeBuffer, 0x1C46);
@@ -71,9 +72,10 @@ public class CmpInstructionSequenceTests
         
         // Check the first operand (memory operand)
         var memoryOperand = cmpInstruction.StructuredOperands[0];
-        Assert.IsType<BaseRegisterMemoryOperand>(memoryOperand);
+        Assert.IsType<DisplacementMemoryOperand>(memoryOperand);
-        var memory = (BaseRegisterMemoryOperand)memoryOperand;
+        var memory = (DisplacementMemoryOperand)memoryOperand;
         Assert.Equal(RegisterIndex.Bp, memory.BaseRegister); // Base register is ECX
+        Assert.Equal(0, memory.Displacement); // displacement should be 0x0
         Assert.Equal(8, memory.Size); // Memory size is 8 bits (BYTE)
         
         // Check the second operand (immediate value)
@@ -106,7 +108,7 @@ public class CmpInstructionSequenceTests
     public void CmpJgeSequence_DecodesCorrectly()
     {
         // Arrange
-        // CMP BYTE PTR [EBP], 0x03 (80 7D 00 03)
+        // CMP BYTE PTR [EBP+0x0], 0x03 (80 7D 00 03)
         // JGE +5 (7D 05)
         // ADD EBP, 0x18 (83 C5 18)
         // JMP +3 (EB 03)
@@ -132,9 +134,10 @@ public class CmpInstructionSequenceTests
         
         // Check the first operand (memory operand)
         var memoryOperand = cmpInstruction.StructuredOperands[0];
-        Assert.IsType<BaseRegisterMemoryOperand>(memoryOperand);
+        Assert.IsType<DisplacementMemoryOperand>(memoryOperand);
-        var memory = (BaseRegisterMemoryOperand)memoryOperand;
+        var memory = (DisplacementMemoryOperand)memoryOperand;
         Assert.Equal(RegisterIndex.Bp, memory.BaseRegister); // Base register is ECX
+        Assert.Equal(0, memory.Displacement); // displacement should be is 0x0
         Assert.Equal(8, memory.Size); // Memory size is 8 bits (BYTE)
         
         // Check the second operand (immediate value)

X86DisassemblerTests/InstructionTests/InstructionDecoderTests.cs

@@ -73,9 +73,9 @@ public class InstructionDecoderTests
         
         // Check the second operand (AL)
         var alOperand = instruction.StructuredOperands[1];
-        Assert.IsType<RegisterOperand>(alOperand);
+        Assert.IsType<Register8Operand>(alOperand);
-        var alRegisterOperand = (RegisterOperand)alOperand;
+        var alRegisterOperand = (Register8Operand)alOperand;
-        Assert.Equal(RegisterIndex.A, alRegisterOperand.Register);
+        Assert.Equal(RegisterIndex8.AL, alRegisterOperand.Register);
         Assert.Equal(8, alRegisterOperand.Size); // Validate that it's an 8-bit register (AL)
     }
     

X86DisassemblerTests/TestData/call_tests.csv

@@ -33,10 +33,12 @@ FF17;[{ "Type": "Call", "Operands": ["dword ptr [edi]"] }]
 FF1400;[{ "Type": "Call", "Operands": ["dword ptr [eax+eax*1]"] }]
 FF14C0;[{ "Type": "Call", "Operands": ["dword ptr [eax+eax*8]"] }]
 FF1444;[{ "Type": "Call", "Operands": ["dword ptr [esp+eax*2]"] }]
-FF1485;[{ "Type": "Call", "Operands": ["dword ptr [ebp+eax*4]"] }]
+# not recognized neither by ghidra nor online disasms
+# FF1485;[{ "Type": "Call", "Operands": ["dword ptr [ebp+eax*4]"] }]
 FF1498;[{ "Type": "Call", "Operands": ["dword ptr [eax+ebx*4]"] }]
 FF14D9;[{ "Type": "Call", "Operands": ["dword ptr [ecx+ebx*8]"] }]
-FF149D;[{ "Type": "Call", "Operands": ["dword ptr [ebp+ebx*4]"] }]
+# not recognized neither by ghidra nor online disasms
+# FF149D;[{ "Type": "Call", "Operands": ["dword ptr [ebp+ebx*4]"] }]
 
 # CALL m32 (opcode FF /2) with displacement
 FF5000;[{ "Type": "Call", "Operands": ["dword ptr [eax+0x0]"] }]