Bug 502324 - s390x: Fix memcheck false positives with TMxx

The 16-bit "test under mask" instructions TMLL, TMLH, TMHL, and TMHH can
yield memcheck false positives when:

* some of the operand bits in the tested 16-bit chunk (but outside the
  mask) are undefined

* the resulting condition code is used in a different block

In this case the condition code is computed by the helper
s390_call_calculate_cond, with the full 16-bit chunk and the given mask as
arguments.  Since the 16-bit chunk is not fully defined, memcheck
complains.

To fix this, AND the operand with the given mask before storing it for use
as a helper argument.  Also, optimize the frequent case of testing a
single bit.  For this purpose, add S390_CC_OP_BITWISE2 as a new way of
computing the condition code.

diff --git a/VEX/priv/guest_s390_defs.h b/VEX/priv/guest_s390_defs.h
index 0039f5e78483ef6ff112a4a99fd8a33866fdeedf..0ec242a99b44156591a63c0ce33cb6f0992c89b7 100644 (file)
--- a/VEX/priv/guest_s390_defs.h
+++ b/VEX/priv/guest_s390_defs.h
@@ -155,7 +155,8 @@ enum {
    S390_CC_OP_PFPO_64 = 59,
    S390_CC_OP_PFPO_128 = 60,
    S390_CC_OP_MUL_32 = 61,
-   S390_CC_OP_MUL_64 = 62
+   S390_CC_OP_MUL_64 = 62,
+   S390_CC_OP_BITWISE2 = 63
 };
 
 /*------------------------------------------------------------*/

diff --git a/VEX/priv/guest_s390_helpers.c b/VEX/priv/guest_s390_helpers.c
index bbba22b4784037d5781918cc6b9c4907c40368ae..8f3f1a172fb76a70bdfd40b3ae55bd32bde7367b 100644 (file)
--- a/VEX/priv/guest_s390_helpers.c
+++ b/VEX/priv/guest_s390_helpers.c
@@ -1454,7 +1454,10 @@ s390_calculate_cc(ULong cc_op, ULong cc_dep1, ULong cc_dep2, ULong cc_ndep)
    switch (cc_op) {
 
    case S390_CC_OP_BITWISE:
-      return S390_CC_FOR_BINARY("ogr", cc_dep1, (ULong)0);
+      return cc_dep1 == 0 ? 0 : 1;
+
+   case S390_CC_OP_BITWISE2:
+      return cc_dep1 == 0 ? 0 : 3;
 
    case S390_CC_OP_SIGNED_COMPARE:
       return S390_CC_FOR_BINARY("cgr", cc_dep1, cc_dep2);
@@ -2045,6 +2048,21 @@ guest_s390x_spechelper(const HChar *function_name, IRExpr **args,
          return mkU32(0);
       }
 
+      /* S390_CC_OP_BITWISE2
+         Like S390_CC_OP_BITWISE, but yielding cc = 3 for nonzero result */
+      if (cc_op == S390_CC_OP_BITWISE2) {
+         if ((cond & (8 + 1)) == 8 + 1) {
+            return mkU32(1);
+         }
+         if (cond & 8) {
+            return unop(Iop_1Uto32, binop(Iop_CmpEQ64, cc_dep1, mkU64(0)));
+         }
+         if (cond & 1) {
+            return unop(Iop_1Uto32, binop(Iop_CmpNE64, cc_dep1, mkU64(0)));
+         }
+         return mkU32(0);
+      }
+
       /* S390_CC_OP_INSERT_CHAR_MASK_32
          Since the mask comes from an immediate field in the opcode, we
          expect the mask to be a constant here. That simplifies matters. */
@@ -2163,110 +2181,94 @@ guest_s390x_spechelper(const HChar *function_name, IRExpr **args,
       }
 
       /* S390_CC_OP_TEST_UNDER_MASK_16
-         Since the mask comes from an immediate field in the opcode, we
-         expect the mask to be a constant here. That simplifies matters. */
+         cc_dep1 = the value to be tested, ANDed with the mask
+         cc_dep2 = a 16-bit mask; expected to be a constant here */
       if (cc_op == S390_CC_OP_TEST_UNDER_MASK_16) {
-         ULong mask16;
-         UInt msb;
+         IRExpr* val = cc_dep1;
+         ULong mask;
+         ULong msb;
 
          if (! isC64(cc_dep2)) goto missed;
 
-         mask16 = cc_dep2->Iex.Const.con->Ico.U64;
+         mask = cc_dep2->Iex.Const.con->Ico.U64;
 
-         /* Get rid of the mask16 == 0 case first. Some of the simplifications
-            below (e.g. for OVFL) only hold if mask16 == 0.  */
-         if (mask16 == 0) {   /* cc == 0 */
+         if (mask == 0) {   /* cc == 0 */
             if (cond & 0x8) return mkU32(1);
             return mkU32(0);
          }
 
+         /* Find MSB in mask */
+         msb = 0x8000;
+         while (msb > mask)
+            msb >>= 1;
+
+         /* If cc_dep1 results from a shift, avoid the shift operation */
+         if (val->tag == Iex_Binop && val->Iex.Binop.op == Iop_Shr64 &&
+             val->Iex.Binop.arg2->tag == Iex_Const &&
+             val->Iex.Binop.arg2->Iex.Const.con->tag == Ico_U8) {
+            UInt n_bits = val->Iex.Binop.arg2->Iex.Const.con->Ico.U8;
+            mask <<= n_bits;
+            msb <<= n_bits;
+            val = val->Iex.Binop.arg1;
+         }
+
          if (cond == 15) return mkU32(1);
 
-         if (cond == 8) {
-            return unop(Iop_1Uto32, binop(Iop_CmpEQ64,
-                                          binop(Iop_And64, cc_dep1, cc_dep2),
-                                          mkU64(0)));
+         if (cond == 8) { /* all bits zero */
+            return unop(Iop_1Uto32, binop(Iop_CmpEQ64, val, mkU64(0)));
          }
-         if (cond == 7) {
-            return unop(Iop_1Uto32, binop(Iop_CmpNE64,
-                                          binop(Iop_And64, cc_dep1, cc_dep2),
-                                          mkU64(0)));
+         if (cond == 7) { /* not all bits zero */
+            return unop(Iop_1Uto32, binop(Iop_CmpNE64, val, mkU64(0)));
          }
-         if (cond == 1) {
-            return unop(Iop_1Uto32, binop(Iop_CmpEQ64,
-                                          binop(Iop_And64, cc_dep1, cc_dep2),
-                                          mkU64(mask16)));
+         if (cond == 1) { /* all bits set */
+            return unop(Iop_1Uto32, binop(Iop_CmpEQ64, val, mkU64(mask)));
          }
-         if (cond == 14) {  /* ! OVFL */
-            return unop(Iop_1Uto32, binop(Iop_CmpNE64,
-                                          binop(Iop_And64, cc_dep1, cc_dep2),
-                                          mkU64(mask16)));
+         if (cond == 14) { /* not all bits set */
+            return unop(Iop_1Uto32, binop(Iop_CmpNE64, val, mkU64(mask)));
          }
 
-         /* Find MSB in mask */
-         msb = 0x8000;
-         while (msb > mask16)
-            msb >>= 1;
+         IRExpr *masked_msb = binop(Iop_And64, val, mkU64(msb));
 
          if (cond == 2) {  /* cc == 2 */
-            IRExpr *c1, *c2;
-
-            /* (cc_dep & msb) != 0 && (cc_dep & mask16) != mask16 */
-            c1 = binop(Iop_CmpNE64,
-                       binop(Iop_And64, cc_dep1, mkU64(msb)), mkU64(0));
-            c2 = binop(Iop_CmpNE64,
-                       binop(Iop_And64, cc_dep1, cc_dep2),
-                       mkU64(mask16));
-            return binop(Iop_And32, unop(Iop_1Uto32, c1),
-                         unop(Iop_1Uto32, c2));
+            /* mixed, and leftmost bit set */
+            return unop(Iop_1Uto32,
+                        binop(Iop_And1,
+                              binop(Iop_CmpNE64, masked_msb, mkU64(0)),
+                              binop(Iop_CmpNE64, val, mkU64(mask))));
          }
 
          if (cond == 4) {  /* cc == 1 */
-            IRExpr *c1, *c2;
-
-            /* (cc_dep & msb) == 0 && (cc_dep & mask16) != 0 */
-            c1 = binop(Iop_CmpEQ64,
-                       binop(Iop_And64, cc_dep1, mkU64(msb)), mkU64(0));
-            c2 = binop(Iop_CmpNE64,
-                       binop(Iop_And64, cc_dep1, cc_dep2),
-                       mkU64(0));
-            return binop(Iop_And32, unop(Iop_1Uto32, c1),
-                         unop(Iop_1Uto32, c2));
+            /* mixed, and leftmost bit zero */
+            return unop(Iop_1Uto32,
+                        binop(Iop_And1,
+                              binop(Iop_CmpEQ64, masked_msb, mkU64(0)),
+                              binop(Iop_CmpNE64, val, mkU64(0))));
          }
 
          if (cond == 11) {  /* cc == 0,2,3 */
-            IRExpr *c1, *c2;
-
-            c1 = binop(Iop_CmpNE64,
-                       binop(Iop_And64, cc_dep1, mkU64(msb)), mkU64(0));
-            c2 = binop(Iop_CmpEQ64,
-                       binop(Iop_And64, cc_dep1, cc_dep2),
-                       mkU64(0));
-            return binop(Iop_Or32, unop(Iop_1Uto32, c1),
-                         unop(Iop_1Uto32, c2));
+            /* leftmost bit set, or all bits zero */
+            return unop(Iop_1Uto32,
+                        binop(Iop_Or1,
+                              binop(Iop_CmpNE64, masked_msb, mkU64(0)),
+                              binop(Iop_CmpEQ64, val, mkU64(0))));
          }
 
          if (cond == 3) {  /* cc == 2 || cc == 3 */
+            /* leftmost bit set, rest don't care */
             return unop(Iop_1Uto32,
-                        binop(Iop_CmpNE64,
-                              binop(Iop_And64, cc_dep1, mkU64(msb)),
-                              mkU64(0)));
+                        binop(Iop_CmpNE64, masked_msb, mkU64(0)));
          }
          if (cond == 12) { /* cc == 0 || cc == 1 */
+            /* leftmost bit zero, rest don't care */
             return unop(Iop_1Uto32,
-                        binop(Iop_CmpEQ64,
-                              binop(Iop_And64, cc_dep1, mkU64(msb)),
-                              mkU64(0)));
+                        binop(Iop_CmpEQ64, masked_msb, mkU64(0)));
          }
          if (cond == 13) { /* cc == 0 || cc == 1 || cc == 3 */
-            IRExpr *c01, *c3;
-
-            c01 = binop(Iop_CmpEQ64, binop(Iop_And64, cc_dep1, mkU64(msb)),
-                        mkU64(0));
-            c3 = binop(Iop_CmpEQ64, binop(Iop_And64, cc_dep1, cc_dep2),
-                       mkU64(mask16));
-            return binop(Iop_Or32, unop(Iop_1Uto32, c01),
-                         unop(Iop_1Uto32, c3));
+            /* leftmost bit zero, or all bits set */
+            return unop(Iop_1Uto32,
+                        binop(Iop_Or1,
+                              binop(Iop_CmpEQ64, masked_msb, mkU64(0)),
+                              binop(Iop_CmpEQ64, val, mkU64(mask))));
          }
          // fixs390: handle cond = 5,6,9,10 (the missing cases)
          // vex_printf("TUM mask = 0x%llx\n", mask16);

diff --git a/VEX/priv/guest_s390_toIR.c b/VEX/priv/guest_s390_toIR.c
index 5b160021032d4db6c4816b3a6af16bcc5446cd73..e95f28bfda677d003f214a2244dda33bf052c7c5 100644 (file)
--- a/VEX/priv/guest_s390_toIR.c
+++ b/VEX/priv/guest_s390_toIR.c
@@ -11094,59 +11094,52 @@ s390_irgen_TMY(UChar i2, IRTemp op1addr)
 }
 
 static const HChar *
-s390_irgen_TMHH(UChar r1, UShort i2)
+s390_irgen_TMxx(const HChar *mnem, UChar r1, UShort mask, UChar offs)
 {
-   UShort mask;
-   IRTemp value = newTemp(Ity_I16);
+   if (mask == 0) {
+      s390_cc_set_val(0);
+      return mnem;
+   }
 
-   mask = i2;
-   assign(value, get_gpr_hw0(r1));
-   s390_cc_thunk_putZZ(S390_CC_OP_TEST_UNDER_MASK_16, value, mktemp(Ity_I16,
-                       mkU16(mask)));
+   IRExpr* masked;
+   masked = binop(Iop_And64, get_gpr_dw0(r1), mkU64((ULong)mask << offs));
 
-   return "tmhh";
+   if ((mask & (mask - 1)) == 0) {
+      /* Single-bit mask */
+      s390_cc_thunk_put1(S390_CC_OP_BITWISE2, mktemp(Ity_I64, masked), False);
+   } else {
+      if (offs) {
+         masked = binop(Iop_Shr64, masked, mkU8(offs));
+      }
+      s390_cc_thunk_putZZ(S390_CC_OP_TEST_UNDER_MASK_16,
+                          mktemp(Ity_I64, masked),
+                          mktemp(Ity_I64, mkU64(mask)));
+   }
+   return mnem;
 }
 
 static const HChar *
-s390_irgen_TMHL(UChar r1, UShort i2)
+s390_irgen_TMHH(UChar r1, UShort i2)
 {
-   UShort mask;
-   IRTemp value = newTemp(Ity_I16);
-
-   mask = i2;
-   assign(value, get_gpr_hw1(r1));
-   s390_cc_thunk_putZZ(S390_CC_OP_TEST_UNDER_MASK_16, value, mktemp(Ity_I16,
-                       mkU16(mask)));
+   return s390_irgen_TMxx("tmhh", r1, i2, 48);
+}
 
-   return "tmhl";
+static const HChar *
+s390_irgen_TMHL(UChar r1, UShort i2)
+{
+   return s390_irgen_TMxx("tmhl", r1, i2, 32);
 }
 
 static const HChar *
 s390_irgen_TMLH(UChar r1, UShort i2)
 {
-   UShort mask;
-   IRTemp value = newTemp(Ity_I16);
-
-   mask = i2;
-   assign(value, get_gpr_hw2(r1));
-   s390_cc_thunk_putZZ(S390_CC_OP_TEST_UNDER_MASK_16, value, mktemp(Ity_I16,
-                       mkU16(mask)));
-
-   return "tmlh";
+   return s390_irgen_TMxx("tmlh", r1, i2, 16);
 }
 
 static const HChar *
 s390_irgen_TMLL(UChar r1, UShort i2)
 {
-   UShort mask;
-   IRTemp value = newTemp(Ity_I16);
-
-   mask = i2;
-   assign(value, get_gpr_hw3(r1));
-   s390_cc_thunk_putZZ(S390_CC_OP_TEST_UNDER_MASK_16, value, mktemp(Ity_I16,
-                       mkU16(mask)));
-
-   return "tmll";
+   return s390_irgen_TMxx("tmll", r1, i2, 0);
 }
 
 static const HChar *