[Valgrind-developers] vex: r2768 - in /trunk/priv: guest_amd64_toIR.c guest_x86_toIR.c

[<sv...@va...>]

Author: sewardj
Date: Wed Sep 18 18:27:55 2013
New Revision: 2768

Log:
x86 front ends: tighten up decoding of MOV Ib,Eb and MOV Iv,Ev.  This
failed to check the g-register in the modrm byte, with the result that
it will mis-decode the AVX2 XABORT and XBEGIN instructions as these
instead, with obviously-bizarre consequences.


Modified:
    trunk/priv/guest_amd64_toIR.c
    trunk/priv/guest_x86_toIR.c

Modified: trunk/priv/guest_amd64_toIR.c
==============================================================================
--- trunk/priv/guest_amd64_toIR.c (original)
+++ trunk/priv/guest_amd64_toIR.c Wed Sep 18 18:27:55 2013
@@ -20035,34 +20035,37 @@
       DIP(haveF3(pfx) ? "rep ; ret\n" : "ret\n");
       return delta;
 
-   case 0xC6: /* MOV Ib,Eb */
+   case 0xC6: /* C6 /0 = MOV Ib,Eb */
       sz = 1;
-      goto do_Mov_I_E;
-   case 0xC7: /* MOV Iv,Ev */
-      goto do_Mov_I_E;
-   do_Mov_I_E:
-      if (haveF2orF3(pfx)) goto decode_failure;
+      goto maybe_do_Mov_I_E;
+   case 0xC7: /* C7 /0 = MOV Iv,Ev */
+      goto maybe_do_Mov_I_E;
+   maybe_do_Mov_I_E:
       modrm = getUChar(delta);
-      if (epartIsReg(modrm)) {
-         delta++; /* mod/rm byte */
-         d64 = getSDisp(imin(4,sz),delta); 
-         delta += imin(4,sz);
-         putIRegE(sz, pfx, modrm, 
-                      mkU(szToITy(sz), d64 & mkSizeMask(sz)));
-         DIP("mov%c $%lld, %s\n", nameISize(sz), 
-                                  (Long)d64, 
-                                  nameIRegE(sz,pfx,modrm));
-      } else {
-         addr = disAMode ( &alen, vbi, pfx, delta, dis_buf, 
-                           /*xtra*/imin(4,sz) );
-         delta += alen;
-         d64 = getSDisp(imin(4,sz),delta);
-         delta += imin(4,sz);
-         storeLE(mkexpr(addr), 
-                 mkU(szToITy(sz), d64 & mkSizeMask(sz)));
-         DIP("mov%c $%lld, %s\n", nameISize(sz), (Long)d64, dis_buf);
+      if (gregLO3ofRM(modrm) == 0) {
+         if (haveF2orF3(pfx)) goto decode_failure;
+         if (epartIsReg(modrm)) {
+            delta++; /* mod/rm byte */
+            d64 = getSDisp(imin(4,sz),delta); 
+            delta += imin(4,sz);
+            putIRegE(sz, pfx, modrm, 
+                         mkU(szToITy(sz), d64 & mkSizeMask(sz)));
+            DIP("mov%c $%lld, %s\n", nameISize(sz), 
+                                     (Long)d64, 
+                                     nameIRegE(sz,pfx,modrm));
+         } else {
+            addr = disAMode ( &alen, vbi, pfx, delta, dis_buf, 
+                              /*xtra*/imin(4,sz) );
+            delta += alen;
+            d64 = getSDisp(imin(4,sz),delta);
+            delta += imin(4,sz);
+            storeLE(mkexpr(addr), 
+                    mkU(szToITy(sz), d64 & mkSizeMask(sz)));
+            DIP("mov%c $%lld, %s\n", nameISize(sz), (Long)d64, dis_buf);
+         }
+         return delta;
       }
-      return delta;
+      goto decode_failure;
 
    case 0xC8: /* ENTER */
       /* Same comments re operand size as for LEAVE below apply.

Modified: trunk/priv/guest_x86_toIR.c
==============================================================================
--- trunk/priv/guest_x86_toIR.c (original)
+++ trunk/priv/guest_x86_toIR.c Wed Sep 18 18:27:55 2013
@@ -13503,28 +13503,31 @@
       DIP("mov%c $0x%x,%s\n", nameISize(sz), d32, nameIReg(sz,opc-0xB8));
       break;
 
-   case 0xC6: /* MOV Ib,Eb */
+   case 0xC6: /* C6 /0 = MOV Ib,Eb */
       sz = 1;
-      goto do_Mov_I_E;
-   case 0xC7: /* MOV Iv,Ev */
-      goto do_Mov_I_E;
+      goto maybe_do_Mov_I_E;
+   case 0xC7: /* C7 /0 = MOV Iv,Ev */
+      goto maybe_do_Mov_I_E;
 
-   do_Mov_I_E:
+   maybe_do_Mov_I_E:
       modrm = getIByte(delta);
-      if (epartIsReg(modrm)) {
-         delta++; /* mod/rm byte */
-         d32 = getUDisp(sz,delta); delta += sz;
-         putIReg(sz, eregOfRM(modrm), mkU(szToITy(sz), d32));
-         DIP("mov%c $0x%x, %s\n", nameISize(sz), d32, 
-                                  nameIReg(sz,eregOfRM(modrm)));
-      } else {
-         addr = disAMode ( &alen, sorb, delta, dis_buf );
-         delta += alen;
-         d32 = getUDisp(sz,delta); delta += sz;
-         storeLE(mkexpr(addr), mkU(szToITy(sz), d32));
-         DIP("mov%c $0x%x, %s\n", nameISize(sz), d32, dis_buf);
+      if (gregOfRM(modrm) == 0) {
+         if (epartIsReg(modrm)) {
+            delta++; /* mod/rm byte */
+            d32 = getUDisp(sz,delta); delta += sz;
+            putIReg(sz, eregOfRM(modrm), mkU(szToITy(sz), d32));
+            DIP("mov%c $0x%x, %s\n", nameISize(sz), d32, 
+                                     nameIReg(sz,eregOfRM(modrm)));
+         } else {
+            addr = disAMode ( &alen, sorb, delta, dis_buf );
+            delta += alen;
+            d32 = getUDisp(sz,delta); delta += sz;
+            storeLE(mkexpr(addr), mkU(szToITy(sz), d32));
+            DIP("mov%c $0x%x, %s\n", nameISize(sz), d32, dis_buf);
+         }
+         break;
       }
-      break;
+      goto decode_failure;
 
    /* ------------------------ opl imm, A ----------------- */