Detect FPU instructions which set %EFLAGS and mark the resulting

UInstrs accordingly.  Fixes a bug in the simulated CPU in which the
results of f{u}comi{p} FPU insns were ignored, potentially leading to
wrong program behaviour.  This will only have happened to people using
P6/P7/K7 class CPUs.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@30

diff --git a/ChangeLog b/ChangeLog
index 26312081cdc33b751e3d07188e6cacc3300e93cd..195bb4b9b9852f08eaef26a683349df83efb6954 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,9 @@
        * vg_to_ucode.c (disInstr): Implement DAA as well as DAS.
          Byrial Jensen <byrial@image.dk>
 
+        (dis_fpu_no_mem): Detect FPU instructions which set %EFLAGS and
+        mark the resulting UInstrs accordingly.
+
        * vg_errcontext.c (pp_ErrContext): Change message "Use of
        uninitialized CPU condition code" to "Conditional jump or move
        depends on uninitialised value(s)", since that will be more

diff --git a/coregrind/vg_from_ucode.c b/coregrind/vg_from_ucode.c
index 4d042935586e1124e3f29d6bcce1081678ff5c9d..5e320840ee12a2c9e948725c91ff58659587c5e8 100644 (file)
--- a/coregrind/vg_from_ucode.c
+++ b/coregrind/vg_from_ucode.c
@@ -2598,8 +2598,13 @@ static void emitUInstr ( Int i, UInstr* u )
       case FPU:
          vg_assert(u->tag1 == Lit16);
          vg_assert(u->tag2 == NoValue);
+         vg_assert(u->flags_r == FlagsEmpty);
+         if (u->flags_w != FlagsEmpty) 
+            emit_get_eflags();
          synth_fpu_no_mem ( (u->val1 >> 8) & 0xFF,
                             u->val1 & 0xFF );
+         if (u->flags_w != FlagsEmpty) 
+            emit_put_eflags();
          break;
 
       default: 

diff --git a/coregrind/vg_include.h b/coregrind/vg_include.h
index 8b81b6d364b476cc6a5656054368b10b56bf9cc0..cf0237ec9ca599e60b2aebe3e833ea53e64368d1 100644 (file)
--- a/coregrind/vg_include.h
+++ b/coregrind/vg_include.h
@@ -616,6 +616,7 @@ typedef UChar FlagSet;
 #define FlagsOSACP  (FlagO | FlagS |         FlagA | FlagC | FlagP)
 #define FlagsSZACP  (        FlagS | FlagZ | FlagA | FlagC | FlagP)
 #define FlagsSZAP   (        FlagS | FlagZ | FlagA |         FlagP)
+#define FlagsZCP    (                FlagZ         | FlagC | FlagP)
 #define FlagsOC     (FlagO |                         FlagC        )
 #define FlagsAC     (                        FlagA | FlagC        )
 

diff --git a/coregrind/vg_to_ucode.c b/coregrind/vg_to_ucode.c
index ebce94fda83c21bb9251f218699155f5435ace6c..b3bd3c367c28e68d1f2c71f2aa9ad3e3a63482e4 100644 (file)
--- a/coregrind/vg_to_ucode.c
+++ b/coregrind/vg_to_ucode.c
@@ -2244,14 +2244,39 @@ Addr dis_fpu_mem ( UCodeBlock* cb, Int size, Bool is_write,
 static 
 Addr dis_fpu_no_mem ( UCodeBlock* cb, Addr eip, UChar first_byte )
 {
+   Bool  sets_ZCP    = False;
    UChar second_byte = getUChar(eip); eip++;
    vg_assert(second_byte >= 0xC0);
+
+   if (first_byte == 0xDB && second_byte >= 0xF0 && second_byte <= 0xF7) {
+      /* FCOMI */
+      sets_ZCP = True;
+   } else
+   if (first_byte == 0xDF && second_byte >= 0xF0 && second_byte <= 0xF7) {
+      /* FCOMIP */
+      sets_ZCP = True;
+   } else
+   if (first_byte == 0xDB && second_byte >= 0xE8 && second_byte <= 0xEF) {
+      /* FUCOMI */
+      sets_ZCP = True;
+   } else
+   if (first_byte == 0xDF && second_byte >= 0xE8 && second_byte <= 0xEF) {
+      /* FUCOMIP */
+      sets_ZCP = True;
+   } 
+
    uInstr1(cb, FPU, 0,
                Lit16,
                (((UShort)first_byte) << 8) | ((UShort)second_byte)
           );
-   if (dis) VG_(printf)("fpu 0x%x:0x%x\n",
-                        (UInt)first_byte, (UInt)second_byte );
+   if (sets_ZCP) {
+      /* VG_(printf)("!!! --- FPU insn which writes %EFLAGS\n"); */
+      uFlagsRWU(cb, FlagsEmpty, FlagsZCP, FlagsEmpty);
+   }
+
+   if (dis) VG_(printf)("fpu 0x%x:0x%x%s\n",
+                        (UInt)first_byte, (UInt)second_byte,
+                        sets_ZCP ? " -wZCP" : "" );
    return eip;
 }
 

diff --git a/coregrind/vg_translate.c b/coregrind/vg_translate.c
index 430aebab8af58cf29409f7b6a1b56e20912589fb..18749ac40c02e09328a8607ee76472fcbd8dbe8a 100644 (file)
--- a/coregrind/vg_translate.c
+++ b/coregrind/vg_translate.c
@@ -478,6 +478,8 @@ Bool VG_(saneUInstr) ( Bool beforeRA, UInstr* u )
 #  define CC0 (u->flags_r == FlagsEmpty && u->flags_w == FlagsEmpty)
 #  define FLG_RD (u->flags_r == FlagsALL && u->flags_w == FlagsEmpty)
 #  define FLG_WR (u->flags_r == FlagsEmpty && u->flags_w == FlagsALL)
+#  define FLG_WR_MAYBE (u->flags_r == FlagsEmpty && \
+                    (u->flags_w == FlagsEmpty || u->flags_w == FlagsZCP))
 #  define CC1 (!(CC0))
 #  define SZ4_IF_TR1 ((u->tag1 == TempReg || u->tag1 == RealReg) \
                       ? (u->size == 4) : True)
@@ -540,7 +542,7 @@ Bool VG_(saneUInstr) ( Bool beforeRA, UInstr* u )
       case FPU_R:  case FPU_W: 
          return CC0 && Ls1 && TR2 && N3;
       case FPU: 
-         return SZ0 && CC0 && Ls1 && N2 && N3;
+         return SZ0 && FLG_WR_MAYBE && Ls1 && N2 && N3;
       case LOADV:
          return CC0 && TR1 && TR2 && N3;
       case STOREV:
@@ -590,6 +592,7 @@ Bool VG_(saneUInstr) ( Bool beforeRA, UInstr* u )
 #  undef N3
 #  undef FLG_RD
 #  undef FLG_WR
+#  undef FLG_WR_MAYBE
 }
 
 

diff --git a/vg_from_ucode.c b/vg_from_ucode.c
index 4d042935586e1124e3f29d6bcce1081678ff5c9d..5e320840ee12a2c9e948725c91ff58659587c5e8 100644 (file)
--- a/vg_from_ucode.c
+++ b/vg_from_ucode.c
@@ -2598,8 +2598,13 @@ static void emitUInstr ( Int i, UInstr* u )
       case FPU:
          vg_assert(u->tag1 == Lit16);
          vg_assert(u->tag2 == NoValue);
+         vg_assert(u->flags_r == FlagsEmpty);
+         if (u->flags_w != FlagsEmpty) 
+            emit_get_eflags();
          synth_fpu_no_mem ( (u->val1 >> 8) & 0xFF,
                             u->val1 & 0xFF );
+         if (u->flags_w != FlagsEmpty) 
+            emit_put_eflags();
          break;
 
       default: 

diff --git a/vg_include.h b/vg_include.h
index 8b81b6d364b476cc6a5656054368b10b56bf9cc0..cf0237ec9ca599e60b2aebe3e833ea53e64368d1 100644 (file)
--- a/vg_include.h
+++ b/vg_include.h
@@ -616,6 +616,7 @@ typedef UChar FlagSet;
 #define FlagsOSACP  (FlagO | FlagS |         FlagA | FlagC | FlagP)
 #define FlagsSZACP  (        FlagS | FlagZ | FlagA | FlagC | FlagP)
 #define FlagsSZAP   (        FlagS | FlagZ | FlagA |         FlagP)
+#define FlagsZCP    (                FlagZ         | FlagC | FlagP)
 #define FlagsOC     (FlagO |                         FlagC        )
 #define FlagsAC     (                        FlagA | FlagC        )
 

diff --git a/vg_to_ucode.c b/vg_to_ucode.c
index ebce94fda83c21bb9251f218699155f5435ace6c..b3bd3c367c28e68d1f2c71f2aa9ad3e3a63482e4 100644 (file)
--- a/vg_to_ucode.c
+++ b/vg_to_ucode.c
@@ -2244,14 +2244,39 @@ Addr dis_fpu_mem ( UCodeBlock* cb, Int size, Bool is_write,
 static 
 Addr dis_fpu_no_mem ( UCodeBlock* cb, Addr eip, UChar first_byte )
 {
+   Bool  sets_ZCP    = False;
    UChar second_byte = getUChar(eip); eip++;
    vg_assert(second_byte >= 0xC0);
+
+   if (first_byte == 0xDB && second_byte >= 0xF0 && second_byte <= 0xF7) {
+      /* FCOMI */
+      sets_ZCP = True;
+   } else
+   if (first_byte == 0xDF && second_byte >= 0xF0 && second_byte <= 0xF7) {
+      /* FCOMIP */
+      sets_ZCP = True;
+   } else
+   if (first_byte == 0xDB && second_byte >= 0xE8 && second_byte <= 0xEF) {
+      /* FUCOMI */
+      sets_ZCP = True;
+   } else
+   if (first_byte == 0xDF && second_byte >= 0xE8 && second_byte <= 0xEF) {
+      /* FUCOMIP */
+      sets_ZCP = True;
+   } 
+
    uInstr1(cb, FPU, 0,
                Lit16,
                (((UShort)first_byte) << 8) | ((UShort)second_byte)
           );
-   if (dis) VG_(printf)("fpu 0x%x:0x%x\n",
-                        (UInt)first_byte, (UInt)second_byte );
+   if (sets_ZCP) {
+      /* VG_(printf)("!!! --- FPU insn which writes %EFLAGS\n"); */
+      uFlagsRWU(cb, FlagsEmpty, FlagsZCP, FlagsEmpty);
+   }
+
+   if (dis) VG_(printf)("fpu 0x%x:0x%x%s\n",
+                        (UInt)first_byte, (UInt)second_byte,
+                        sets_ZCP ? " -wZCP" : "" );
    return eip;
 }
 

diff --git a/vg_translate.c b/vg_translate.c
index 430aebab8af58cf29409f7b6a1b56e20912589fb..18749ac40c02e09328a8607ee76472fcbd8dbe8a 100644 (file)
--- a/vg_translate.c
+++ b/vg_translate.c
@@ -478,6 +478,8 @@ Bool VG_(saneUInstr) ( Bool beforeRA, UInstr* u )
 #  define CC0 (u->flags_r == FlagsEmpty && u->flags_w == FlagsEmpty)
 #  define FLG_RD (u->flags_r == FlagsALL && u->flags_w == FlagsEmpty)
 #  define FLG_WR (u->flags_r == FlagsEmpty && u->flags_w == FlagsALL)
+#  define FLG_WR_MAYBE (u->flags_r == FlagsEmpty && \
+                    (u->flags_w == FlagsEmpty || u->flags_w == FlagsZCP))
 #  define CC1 (!(CC0))
 #  define SZ4_IF_TR1 ((u->tag1 == TempReg || u->tag1 == RealReg) \
                       ? (u->size == 4) : True)
@@ -540,7 +542,7 @@ Bool VG_(saneUInstr) ( Bool beforeRA, UInstr* u )
       case FPU_R:  case FPU_W: 
          return CC0 && Ls1 && TR2 && N3;
       case FPU: 
-         return SZ0 && CC0 && Ls1 && N2 && N3;
+         return SZ0 && FLG_WR_MAYBE && Ls1 && N2 && N3;
       case LOADV:
          return CC0 && TR1 && TR2 && N3;
       case STOREV:
@@ -590,6 +592,7 @@ Bool VG_(saneUInstr) ( Bool beforeRA, UInstr* u )
 #  undef N3
 #  undef FLG_RD
 #  undef FLG_WR
+#  undef FLG_WR_MAYBE
 }