From: Andreas Arnez Date: Wed, 2 Apr 2025 17:52:26 +0000 (+0200) Subject: Bug 502324 - s390x: Fix memcheck false positives with TM/TMY X-Git-Tag: VALGRIND_3_25_0~67 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=29f8a0ad4ed17d9cdf4a5d6661a51d25db6fc472;p=thirdparty%2Fvalgrind.git Bug 502324 - s390x: Fix memcheck false positives with TM/TMY 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. --- diff --git a/VEX/priv/guest_s390_helpers.c b/VEX/priv/guest_s390_helpers.c index 8f3f1a172..b99d71a5a 100644 --- a/VEX/priv/guest_s390_helpers.c +++ b/VEX/priv/guest_s390_helpers.c @@ -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; } diff --git a/VEX/priv/guest_s390_toIR.c b/VEX/priv/guest_s390_toIR.c index e95f28bfd..a5b1fcae6 100644 --- a/VEX/priv/guest_s390_toIR.c +++ b/VEX/priv/guest_s390_toIR.c @@ -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 *