Changeset 36795 in vbox

Timestamp:

Apr 21, 2011 3:41:39 PM (14 years ago)

Author:

vboxsync

Message:

IEM: Verify memory writes, fixed bugs found doing so.

Location:

trunk/src/VBox/VMM/VMMAll

Files:

• IEMAll.cpp (modified) (13 diffs)
• IEMAllInstructions.cpp.h (modified) (3 diffs)

trunk/src/VBox/VMM/VMMAll/IEMAll.cpp

@@ -190 +190 @@
 /** Temporary hack to disable the double execution.  Will be removed in favor
  * of a dedicated execution mode in EM. */
-//#define IEM_VERIFICATION_MODE_NO_REM
+#define IEM_VERIFICATION_MODE_NO_REM
 
 /** Used to shut up GCC warnings about variables that 'may be used uninitialized'
@@ -3604 +3604 @@
     else
     {
+        GCPtrBottom--;
         uint16_t *pa16Mem = NULL;
         rcStrict = iemMemMap(pIemCpu, (void **)&pa16Mem, 16, X86_SREG_SS, GCPtrBottom, IEM_ACCESS_STACK_W);
@@ -3675 +3676 @@
     else
     {
+        GCPtrBottom--;
         uint32_t *pa32Mem;
         rcStrict = iemMemMap(pIemCpu, (void **)&pa32Mem, 32, X86_SREG_SS, GCPtrBottom, IEM_ACCESS_STACK_W);
@@ -4168 +4170 @@
 
         /* Check stack first - may #SS(0). */
-        rcStrict = iemMemStackPushBeginSpecial(pIemCpu, 4 + (enmOpSize == IEMMODE_32BIT) * 2,
+        rcStrict = iemMemStackPushBeginSpecial(pIemCpu, enmOpSize == IEMMODE_32BIT ? 6 : 4,
                                                &pvRet, &uNewRsp);
         if (rcStrict != VINF_SUCCESS)
@@ -4180 +4182 @@
         if (enmOpSize == IEMMODE_16BIT)
         {
-            ((uint16_t *)pvRet)[0] = pCtx->ip;
+            ((uint16_t *)pvRet)[0] = pCtx->ip + cbInstr;
             ((uint16_t *)pvRet)[1] = pCtx->cs;
         }
         else
         {
-            ((uint32_t *)pvRet)[0] = pCtx->eip;
+            ((uint32_t *)pvRet)[0] = pCtx->eip + cbInstr;
             ((uint16_t *)pvRet)[3] = pCtx->cs;
         }
@@ -4373 +4375 @@
         pu16Frame[2] = (uint16_t)pCtx->eflags.u;
         pu16Frame[1] = (uint16_t)pCtx->cs;
-        pu16Frame[0] = (uint16_t)pCtx->ip;
+        pu16Frame[0] = pCtx->ip + cbInstr;
         rcStrict = iemMemStackPushCommitSpecial(pIemCpu, pu16Frame, uNewRsp);
         if (RT_UNLIKELY(rcStrict != VINF_SUCCESS))
@@ -6188 +6190 @@
 }
 
+
+# ifndef IEM_VERIFICATION_MODE_NO_REM
 /**
  * Allocate an event record.
@@ -6210 +6214 @@
     return pEvtRec;
 }
+# endif
 
 
@@ -6315 +6320 @@
 }
 
+# ifndef IEM_VERIFICATION_MODE_NO_REM
 
 /**
@@ -6441 +6447 @@
 
 /**
+ * Verifies a write record.
+ *
+ * @param   pIemCpu         The IEM per CPU data.
+ * @param   pEvtRec         The write record.
+ */
+static void iemVerifyWriteRecord(PIEMCPU pIemCpu, PIEMVERIFYEVTREC pEvtRec)
+{
+    uint8_t abBuf[sizeof(pEvtRec->u.RamWrite.ab)]; RT_ZERO(abBuf);
+    int rc = PGMPhysSimpleReadGCPhys(IEMCPU_TO_VM(pIemCpu), abBuf, pEvtRec->u.RamWrite.GCPhys, pEvtRec->u.RamWrite.cb);
+    if (   RT_FAILURE(rc)
+        || memcmp(abBuf, pEvtRec->u.RamWrite.ab, pEvtRec->u.RamWrite.cb) )
+    {
+        /* fend off ins */
+        if (   !pIemCpu->cIOReads
+            || pEvtRec->u.RamWrite.ab[0] != 0xcc
+            || (   pEvtRec->u.RamWrite.cb != 1
+                && pEvtRec->u.RamWrite.cb != 2
+                && pEvtRec->u.RamWrite.cb != 4) )
+        {
+            RTAssertMsg1(NULL, __LINE__, __FILE__, __PRETTY_FUNCTION__);
+            RTAssertMsg2Weak("Memory at %RGv differs\n", pEvtRec->u.RamWrite.GCPhys);
+            RTAssertMsg2Add("REM: %.*Rhxs\n"
+                            "IEM: %.*Rhxs\n",
+                            pEvtRec->u.RamWrite.cb, abBuf
+                            pEvtRec->u.RamWrite.cb, pEvtRec->u.RamWrite.ab);
+            iemVerifyAssertAddRecordDump(pEvtRec);
+            RTAssertPanic();
+        }
+    }
+
+}
+
+# endif /* !IEM_VERIFICATION_MODE_NO_REM */
+
+/**
  * Performs the post-execution verfication checks.
  */
@@ -6459 +6500 @@
     int rc = REMR3EmulateInstruction(IEMCPU_TO_VM(pIemCpu), IEMCPU_TO_VMCPU(pIemCpu));
     AssertRC(rc);
-#if 0
-    if (pIemCpu->fPrefixes & (IEM_OP_PRF_REPNZ | IEM_OP_PRF_REPZ))
-    {
-        while (    pOrgCtx->rip == pDebugCtx->rip - pIemCpu->offOpcode
-               &&  pOrgCtx->rcx != pDebugCtx->rcx
-               &&  pOrgCtx->rsi != pDebugCtx->rsi
-               &&  pOrgCtx->rdi != pDebugCtx->rdi)
-        {
-            rc = REMR3EmulateInstruction(IEMCPU_TO_VM(pIemCpu), IEMCPU_TO_VMCPU(pIemCpu));
-            AssertRC(rc);
-        }
-    }
-#endif
 
     /*
@@ -6642 +6670 @@
         while (pIemRec && pOtherRec)
         {
-            /* Since we might miss RAM writes and reads, ignore extra ones
-               made by IEM.  */
+            /* Since we might miss RAM writes and reads, ignore reads and check
+               that any written memory is the same extra ones.  */
             while (   IEMVERIFYEVENT_IS_RAM(pIemRec->enmEvent)
                    && !IEMVERIFYEVENT_IS_RAM(pOtherRec->enmEvent)
                    && pIemRec->pNext)
+            {
+                if (pIemRec->enmEvent == IEMVERIFYEVENT_RAM_WRITE)
+                    iemVerifyWriteRecord(pIemCpu, pIemRec);
                 pIemRec = pIemRec->pNext;
+            }
 
             /* Do the compare. */
@@ -6693 +6725 @@
         /* Ignore extra writes and reads. */
         while (pIemRec && IEMVERIFYEVENT_IS_RAM(pIemRec->enmEvent))
+        {
+            if (pIemRec->enmEvent == IEMVERIFYEVENT_RAM_WRITE)
+                iemVerifyWriteRecord(pIemCpu, pIemRec);
             pIemRec = pIemRec->pNext;
+        }
         if (pIemRec != NULL)
             iemVerifyAssertRecord(pIemRec, "Extra IEM record!");

trunk/src/VBox/VMM/VMMAll/IEMAllInstructions.cpp.h

@@ -8137 +8137 @@
                 IEM_MC_FETCH_MEM_U32(offSeg, pIemCpu->iEffSeg, GCPtrEffSrc);
                 IEM_MC_FETCH_MEM_U16(u16Sel, pIemCpu->iEffSeg, GCPtrEffSrc + 4);
-                IEM_MC_CALL_CIMPL_3(iemCImpl_callf, u16Sel, offSeg, IEMMODE_16BIT);
+                IEM_MC_CALL_CIMPL_3(iemCImpl_callf, u16Sel, offSeg, IEMMODE_32BIT);
                 IEM_MC_END();
             }
@@ -8150 +8150 @@
                 IEM_MC_FETCH_MEM_S32_SX_U64(offSeg, pIemCpu->iEffSeg, GCPtrEffSrc);
                 IEM_MC_FETCH_MEM_U16(u16Sel, pIemCpu->iEffSeg, GCPtrEffSrc + 4);
-                IEM_MC_CALL_CIMPL_3(iemCImpl_callf, u16Sel, offSeg, IEMMODE_16BIT);
+                IEM_MC_CALL_CIMPL_3(iemCImpl_callf, u16Sel, offSeg, IEMMODE_32BIT);
                 IEM_MC_END();
             }
@@ -8164 +8164 @@
             IEM_MC_FETCH_MEM_U64(offSeg, pIemCpu->iEffSeg, GCPtrEffSrc);
             IEM_MC_FETCH_MEM_U16(u16Sel, pIemCpu->iEffSeg, GCPtrEffSrc + 8);
-            IEM_MC_CALL_CIMPL_3(iemCImpl_callf, u16Sel, offSeg, IEMMODE_16BIT);
+            IEM_MC_CALL_CIMPL_3(iemCImpl_callf, u16Sel, offSeg, IEMMODE_64BIT);
             IEM_MC_END();
             return VINF_SUCCESS;