]> git.ipfire.org Git - thirdparty/valgrind.git/commitdiff
Bug 502324 - s390x: Fix memcheck false positives with TM/TMY
authorAndreas Arnez <arnez@linux.ibm.com>
Wed, 2 Apr 2025 17:52:26 +0000 (19:52 +0200)
committerAndreas Arnez <arnez@linux.ibm.com>
Wed, 2 Apr 2025 17:52:26 +0000 (19:52 +0200)
If the condition code of TM/TMY is generated in a different block than it
is used, memcheck can yield false positives for a partially initialized
value even if the checked bits are all defined.

Fix this by storing the operand ANDed with the mask in the flags thunk,
instead of the unmodified operand.  This enables memcheck to track the
defined bits correctly.

VEX/priv/guest_s390_helpers.c
VEX/priv/guest_s390_toIR.c

index 8f3f1a172fb76a70bdfd40b3ae55bd32bde7367b..b99d71a5af20aea95a6b4c8f69af8d6b1c202519 100644 (file)
@@ -2140,8 +2140,8 @@ guest_s390x_spechelper(const HChar *function_name, IRExpr **args,
       }
 
       /* S390_CC_OP_TEST_UNDER_MASK_8
-         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 = an 8-bit mask; expected to be a constant here */
       if (cc_op == S390_CC_OP_TEST_UNDER_MASK_8) {
          ULong mask16;
 
@@ -2149,33 +2149,23 @@ guest_s390x_spechelper(const HChar *function_name, IRExpr **args,
 
          mask16 = 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 (cond & 0x8) return mkU32(1);
             return mkU32(0);
          }
 
          /* cc == 2 is a don't care */
-         if (cond == 8 || cond == 8 + 2) {
-            return unop(Iop_1Uto32, binop(Iop_CmpEQ64,
-                                          binop(Iop_And64, cc_dep1, cc_dep2),
-                                          mkU64(0)));
+         if (cond == 8 || cond == 8 + 2) { /* all bits zero */
+            return unop(Iop_1Uto32, binop(Iop_CmpEQ64, cc_dep1, mkU64(0)));
          }
-         if (cond == 7 || cond == 7 - 2) {
-            return unop(Iop_1Uto32, binop(Iop_CmpNE64,
-                                          binop(Iop_And64, cc_dep1, cc_dep2),
-                                          mkU64(0)));
+         if (cond == 7 || cond == 7 - 2) { /* not all bits zero */
+            return unop(Iop_1Uto32, binop(Iop_CmpNE64, cc_dep1, mkU64(0)));
          }
-         if (cond == 1 || cond == 1 + 2) {
-            return unop(Iop_1Uto32, binop(Iop_CmpEQ64,
-                                          binop(Iop_And64, cc_dep1, cc_dep2),
-                                          cc_dep2));
+         if (cond == 1 || cond == 1 + 2) { /* all bits set */
+            return unop(Iop_1Uto32, binop(Iop_CmpEQ64, cc_dep1, cc_dep2));
          }
-         if (cond == 14 || cond == 14 - 2) {  /* ! OVFL */
-            return unop(Iop_1Uto32, binop(Iop_CmpNE64,
-                                          binop(Iop_And64, cc_dep1, cc_dep2),
-                                          cc_dep2));
+         if (cond == 14 || cond == 14 - 2) { /* not all bits set */
+            return unop(Iop_1Uto32, binop(Iop_CmpNE64, cc_dep1, cc_dep2));
          }
          goto missed;
       }
index e95f28bfda677d003f214a2244dda33bf052c7c5..a5b1fcae6a09411c73bcf4fe60e880c76eb1c73c 100644 (file)
@@ -11066,31 +11066,26 @@ s390_irgen_SVC(UChar i)
 }
 
 static const HChar *
-s390_irgen_TM(UChar i2, IRTemp op1addr)
+s390_irgen_TMx(const HChar *mnem, UChar mask, IRTemp op1addr)
 {
-   UChar mask;
-   IRTemp value = newTemp(Ity_I8);
+   IRTemp masked = newTemp(Ity_I8);
 
-   mask = i2;
-   assign(value, load(Ity_I8, mkexpr(op1addr)));
-   s390_cc_thunk_putZZ(S390_CC_OP_TEST_UNDER_MASK_8, value, mktemp(Ity_I8,
+   assign(masked, binop(Iop_And8, load(Ity_I8, mkexpr(op1addr)), mkU8(mask)));
+   s390_cc_thunk_putZZ(S390_CC_OP_TEST_UNDER_MASK_8, masked, mktemp(Ity_I8,
                        mkU8(mask)));
+   return mnem;
+}
 
-   return "tm";
+static const HChar *
+s390_irgen_TM(UChar i2, IRTemp op1addr)
+{
+   return s390_irgen_TMx("tm", i2, op1addr);
 }
 
 static const HChar *
 s390_irgen_TMY(UChar i2, IRTemp op1addr)
 {
-   UChar mask;
-   IRTemp value = newTemp(Ity_I8);
-
-   mask = i2;
-   assign(value, load(Ity_I8, mkexpr(op1addr)));
-   s390_cc_thunk_putZZ(S390_CC_OP_TEST_UNDER_MASK_8, value, mktemp(Ity_I8,
-                       mkU8(mask)));
-
-   return "tmy";
+   return s390_irgen_TMx("tmy", i2, op1addr);
 }
 
 static const HChar *