From 29f8a0ad4ed17d9cdf4a5d6661a51d25db6fc472 Mon Sep 17 00:00:00 2001 From: Andreas Arnez Date: Wed, 2 Apr 2025 19:52:26 +0200 Subject: [PATCH] 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. --- VEX/priv/guest_s390_helpers.c | 30 ++++++++++-------------------- VEX/priv/guest_s390_toIR.c | 27 +++++++++++---------------- 2 files changed, 21 insertions(+), 36 deletions(-) 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 * -- 2.47.2