Feature: more work on x86 APX decoding.

Author: uxmal

src/Arch/X86/Disassembler/X86Disassembler.Decoders.cs

@@ -179,6 +179,8 @@ public Rex2Decoder(Decoder[] map0decoders, Decoder[] map1decoders)
 
             public override X86Instruction Decode(uint rex2Prefix, X86Disassembler disasm)
             {
+                if (disasm.decodingContext.RexPrefix)
+                    return disasm.CreateInvalidInstruction();
                 if (!disasm.TryReadByte(out var rex2))
                     return disasm.CreateInvalidInstruction();
                 if (!disasm.TryReadByte(out var op))
@@ -521,13 +523,15 @@ public class EvexDecoder : Decoder
             private readonly Decoder[] decoders0F38;
             private readonly Decoder[] decoders0F3A;
             private readonly Decoder[] decodersLegacy;
+            private readonly bool isApx;
 
             private static readonly Bitfield p0Reserved = new Bitfield(2, 2);
             private static readonly Bitfield p1Reserved = new Bitfield(2, 1);
             private static readonly Bitfield p1Vvvv = new Bitfield(3, 4);
 
-            public EvexDecoder(Decoder[] legacy, Decoder[] decoders0F, Decoder[] decoders0F38, Decoder[] decoders0F3A)
+            public EvexDecoder(bool isApx, Decoder[] legacy, Decoder[] decoders0F, Decoder[] decoders0F38, Decoder[] decoders0F3A)
             {
+                this.isApx = isApx;
                 this.decoders0F = decoders0F;
                 this.decoders0F38 = decoders0F38;
                 this.decoders0F3A = decoders0F3A;
@@ -536,7 +540,7 @@ public EvexDecoder(Decoder[] legacy, Decoder[] decoders0F, Decoder[] decoders0F3
 
             public override X86Instruction Decode(uint op, X86Disassembler disasm)
             {
-                // 62 must be the first byte. The presence of previous
+                // 0x62 must be the first byte. The presence of previous
                 // prefixes is an error (according to Intel manual 2.6, vol 2A).
                 var ctx = disasm.decodingContext;
                 if (ctx.F2Prefix |
@@ -546,26 +550,24 @@ public override X86Instruction Decode(uint op, X86Disassembler disasm)
                     return disasm.CreateInvalidInstruction();
                 // The EVEX prefix consists of a leading 0x62 byte, and three
                 // packed payload bytes P0, P1, and P2.
-                //$TODO: this is incomplete: there are many missing flags.
                 if (!disasm.TryReadByte(out byte p0) ||
                     !disasm.TryReadByte(out byte p1) ||
                     !disasm.TryReadByte(out byte p2) ||
-                    p0Reserved.Read(p0) != 0 ||
-                    p1Reserved.Read(p1) != 1)
+                    (!isApx && p0Reserved.Read(p0) != 0) ||
+                    (!isApx && p1Reserved.Read(p1) != 1))
                 {
                     return disasm.CreateInvalidInstruction();
                 }
-                var mm = p0 & 3;
+                var mmm = p0 & 7;
                 var rxb = ~p0 >> 5;
                 var pp = p1 & 3;
                 var w = p1 >> 7;
-                var vvvv = p1Vvvv.Read(~(uint)p1);
-                if (!Bits.IsBitSet(p2, 3))
-                    vvvv |= 0x10;
+                var vvvvv = p1Vvvv.Read(~(uint)p1);
+                vvvvv |= (uint)(~p2 << 1) & 0x10;
                 ctx.RexPrefix = true;
                 ctx.IsVex = true;
                 ctx.IsEvex = true;
-                ctx.VexRegister = (byte) vvvv;
+                ctx.VexRegister = (byte) vvvvv;
                 ctx.VexLongCode = (byte)((p2 >> 5) & 3);
                 ctx.OpMask = (byte)(p2 & 7);
                 ctx.IsWide = w != 0;
@@ -581,16 +583,27 @@ public override X86Instruction Decode(uint op, X86Disassembler disasm)
                 ctx.SizeOverridePrefix = pp == 1;
 
                 Decoder[] decoders;
-                switch (mm)
+                switch (mmm)
                 {
                 case 2: decoders = decoders0F38; break;
                 case 3: decoders = decoders0F3A; break;
-                case 4: decoders = decodersLegacy; break;
+                case 4:
+                    decoders = decodersLegacy;
+                    ctx.EvexNF = Bits.IsBitSet(p2, 2);
+                    if (Bits.IsBitSet(p2, 4))
+                    {
+                        ctx.NewDataDestination = (int) vvvvv;
+                    }
+                    else if (vvvvv != 0)
+                    {
+                        return disasm.CreateInvalidInstruction();
+                    }
+                    break;
                 default: decoders = decoders0F; break;
                 }
                 if (!disasm.TryReadByte(out byte op2))
                     return disasm.CreateInvalidInstruction();
-                TraceEvex(ctx, mm, op2);
+                TraceEvex(ctx, mmm, op2);
                 return decoders[op2].Decode(op2, disasm);
             }
         }

src/Arch/X86/Disassembler/X86Disassembler.OneByte.cs

@@ -37,7 +37,7 @@ private void CreateOnebyteDecoders(
             {
 				// 00
 				d[0x00] = isEevex ? s_invalid : Instr(Mnemonic.add, InstrClass.Linear|InstrClass.Zero, Eb,Gb);
-				d[0x01] = Instr(Mnemonic.add, Ndd,Ev,Gv);
+				d[0x01] = Instr(Mnemonic.add, Nf, Ndd,Ev,Gv);
 				d[0x02] = Instr(Mnemonic.add, Gb,Eb);
 				d[0x03] = Instr(Mnemonic.add, Gv,Ev);
 				d[0x04] = Instr(Mnemonic.add, AL,Ib);
@@ -181,6 +181,7 @@ private void CreateOnebyteDecoders(
                 d[0x62] = isRex2 ? s_invalid : Amd64Instr(
                     Instr186(Mnemonic.bound, Gv,Mv),
                     new EvexDecoder(
+                        isEevex,
                         eevexLegacy0,
                         s_decoders0F,
                         s_decoders0F38,

src/Arch/X86/Disassembler/X86Disassembler.cs

@@ -97,6 +97,7 @@ internal void Reset()
                 this.EvexMergeMode = 0;
                 this.EvexBroadcast = false;
                 this.NewDataDestination = null;
+                this.NoFlags = false;
 
                 this.ops.Clear();
             }
@@ -196,9 +197,19 @@ internal byte ModRegMemByte
             public bool EvexBroadcast{ get; set; }
 
             /// <summary>
-            /// APX : New data destination register encoding
+            /// APX: New data destination register encoding
             /// </summary>
             public int? NewDataDestination { get; set; }
+
+            /// <summary>
+            /// APX: 'NF' no flags bit present in the EVEX prefix.
+            /// </summary>
+            public bool EvexNF { get; set; }
+
+            /// <summary>
+            /// APX: True if setting the flags should be suppressed.
+            /// </summary>
+            public bool NoFlags { get; set; }
         }
 
         //$REVIEW: Instructions longer than this cause exceptions on modern x86 processors.
@@ -599,7 +610,7 @@ private static Mutator<X86Disassembler> B(OperandType opType)
         private static readonly Mutator<X86Disassembler> By = B(OperandType.y);
 
         /// <summary>
-        /// Floating-point ST(x)
+        /// Floating-point ST(x), extracted from the modrm field.
         /// </summary>
         private static bool F(uint op, X86Disassembler d)
         {
@@ -1386,6 +1397,20 @@ private static bool Ndd(uint op, X86Disassembler dasm)
             return true;
         }
 
+        /// <summary>
+        /// Sets the <see cref="X86Instruction.NoFlags"/> flag if the
+        /// instruction has an EVEX prefix with the 'no flags' bit set.
+        /// </summary>
+        private static bool Nf(uint op, X86Disassembler dasm)
+        {
+            var dc = dasm.decodingContext;
+            if (dc.EvexNF)
+            {
+                dc.NoFlags = true;
+            }
+            return true;
+        }
+
         /// <summary>
         /// Use the <see cref="NyiDecoder{TDasm, TMnemonic, TInstr}"/> to mark instructions for which no decoder has 
         /// been written yet.

src/Arch/X86/X86Instruction.cs

@@ -51,20 +51,18 @@ public class X86Instruction : MachineInstruction
         /// </summary>
         public int RepPrefix { get; set; }
 
-        [Obsolete("Use the `RepPrefix` property", true)]
-        public int repPrefix;                   // 0 = no prefix, 2 = repnz, 3 = repz
-        [Obsolete("Use the `DataWidth` property", true)]
-		public DataType dataWidth = default!;	            // Width of the data (if it's a word).
-        [Obsolete("Use the `AddressWidth` property", true)]
-		public PrimitiveType addrWidth = default!;	        // width of the address mode.	// TODO: belongs in MemoryOperand
-
         public byte OpMask { get; set; }        // EVEX Mask register to use.
         public byte MergingMode { get; set; }   // EVEX merging mode
         public bool Broadcast { get; set; }     // EVEX broadcast flag
         public EvexRoundMode RoundMode { get; set; }
         //$PERF: is it worth it to pack the rarely used bit- and byte-sized fields into a single word?
 
-		public X86Instruction(Mnemonic mnemonic, InstrClass iclass, DataType dataWidth, PrimitiveType addrWidth, params MachineOperand [] ops)
+        /// <summary>
+        /// True if instruction does not modify flags -- the Intel APX "NF" bit.
+        /// </summary>
+        public bool NoFlags { get; set; }
+
+        public X86Instruction(Mnemonic mnemonic, InstrClass iclass, DataType dataWidth, PrimitiveType addrWidth, params MachineOperand [] ops)
 		{
 			this.Mnemonic = mnemonic;
             this.InstructionClass = iclass;

src/Core/Machine/DisassemblerBase.cs

@@ -83,6 +83,7 @@ public bool MoveNext()
                 var instr = dasm.DisassembleInstruction();
                 if (instr is null || instr.Length < 0)
                     return false;
+                Debug.Assert(instr.Length > 0);
                 this.current = instr;
                 return true;
             }

src/Core/Machine/IRewriterHost.cs

@@ -52,8 +52,7 @@ public interface IRewriterHost
         Expression? GetImport(Address addrThunk, Address addrInstr);
 
         /// <summary>
-        /// Given an address <paramref name="addrThunk"/>, returns the possible imported procedure
-        /// 
+        /// Given an address <paramref name="addrThunk"/>, returns the possible imported procedure.
         /// </summary>
         /// <param name="arch">Current <see cref="IProcessorArchitecture"/>.</param>
         /// <param name="addrThunk">Address of a thunk.</param>
@@ -72,7 +71,6 @@ public interface IRewriterHost
         /// <param name="arch">Current <see cref="IProcessorArchitecture"/>.</param>
         /// <param name="addrImportThunk"></param>
         /// <returns></returns>
-
         ExternalProcedure? GetInterceptedCall(IProcessorArchitecture arch, Address addrImportThunk);
 
         /// <summary>

src/UnitTests/Arch/X86/Disassembler/X86DisassemblerTests.cs

@@ -1933,8 +1933,6 @@ public void X86Dis_vmaxpd()
             AssertCode64("vmaxpd\tymm6{k7},ymm5,qword ptr [rax]{1to4}", "62 F1 D5 3F 5F 30");
         }
 
-
-
         [Test]
         public void X86Dis_vpminsq()
         {

src/UnitTests/Arch/X86/Disassembler/X86Disassembler_apx_Tests.cs

@@ -71,6 +71,7 @@ public void X86Dis_rex2_illegal_opcodes()
         AssertCode("illegal", "D508 40 03C4");
         AssertCode("illegal", "D508 72 45");
         AssertCode("illegal", "D508 A0 42");
+        AssertCode("illegal", "40 D50803C4");   // REX prefix before REX2
     }
 }