From 8d510c468a20b7528e2fca870f3236b4e2cb965f Mon Sep 17 00:00:00 2001
From: Julian Seward <jseward@acm.org>
Date: Sun, 1 Dec 2019 07:01:20 +0100
Subject: [PATCH] 'grail' fixes for s390x:

This isn't a good result.  It merely disables the new functionality on s390x,
for the reason stated below.

* guest_generic_bb_to_IR.c bb_to_IR(): Disable, hopefully temporarily, the key
  &&-recovery transformation on s390x, since it causes Memcheck to crash for
  reasons I couldn't figure out.  It also exposes some missing Iex_ITE cases
  in the s390x insn selector, although those shouldn't be a big deal to fix.

  Maybe it's some strangeness to do with the s390x "ex" instruction.  I don't
  exactly understand how that trickery works, but from some study of it, I
  didn't see anything obviously wrong.

  It is only chasing through conditional branches that is disabled for s390x.
  Chasing through unconditional branches (jumps and calls to known
  destinations) is still enabled.

* host_s390_isel.c s390_isel_cc(): No functional change.  Code has been added
  here to handle the new Iop_And1 and Iop_Or1, and it is somewhat tested, but
  is not needed until conditional branch chasing is enabled on s390x.
---
 VEX/priv/guest_generic_bb_to_IR.c |  8 +++++++-
 VEX/priv/host_s390_isel.c         | 40 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/VEX/priv/guest_generic_bb_to_IR.c b/VEX/priv/guest_generic_bb_to_IR.c
index 6a1d4dc04..0b8f852ec 100644
--- a/VEX/priv/guest_generic_bb_to_IR.c
+++ b/VEX/priv/guest_generic_bb_to_IR.c
@@ -1446,7 +1446,13 @@ IRSB* bb_to_IR (
       // Try for an extend based on a conditional branch, specifically in the
       // hope of identifying and recovering, an "A && B" condition spread across
       // two basic blocks.
-      if (irsb_be.tag == Be_Cond) {
+      if (irsb_be.tag == Be_Cond
+          /* sewardj 2019Nov30: Alas, chasing cond branches on s390 causes
+             Memcheck to crash, for as-yet unknown reasons.  It also exposes
+             some unhandled Iex_ITE cases in the s390x instruction selector.
+             For now, disable. */
+          && arch_guest != VexArchS390X)
+      {
          if (debug_print) {
             vex_printf("\n-+-+ (ext# %d) Considering cbranch to"
                        " SX=0x%llx FT=0x%llx -+-+\n\n",
diff --git a/VEX/priv/host_s390_isel.c b/VEX/priv/host_s390_isel.c
index 30e5c7620..97614c873 100644
--- a/VEX/priv/host_s390_isel.c
+++ b/VEX/priv/host_s390_isel.c
@@ -3535,6 +3535,46 @@ s390_isel_cc(ISelEnv *env, IRExpr *cond)
       IRExpr *arg2 = cond->Iex.Binop.arg2;
       HReg reg1, reg2;
 
+      /* sewardj 2019Nov30: This will be needed when chasing through conditional
+         branches in guest_generic_bb_to_IR.c is enabled on s390x.
+         Unfortunately that is currently disabled on s390x as it causes
+         mysterious segfaults and also exposes some unhandled Iex_ITE cases in
+         this instruction selector.  The following Iop_And1/Iop_Or1 cases are
+         also needed when enabled.  The code below is *believed* to be correct,
+         and has been lightly tested, but it is #if 0'd until such time as we
+         need it. */
+#     if 0
+      /* FIXME: We could (and probably should) do a lot better here, by using
+         the iselCondCode_C/_R scheme used in the amd64 insn selector. */
+      if (cond->Iex.Binop.op == Iop_And1 || cond->Iex.Binop.op == Iop_Or1) {
+         /* In short: force both operands into registers, AND or OR them, mask
+            off all but the lowest bit, then convert the result back into a
+            condition code. */
+         const s390_opnd_RMI one = s390_opnd_imm(1);
+
+         HReg x_as_64 = newVRegI(env);
+         s390_cc_t cc_x = s390_isel_cc(env, arg1);
+         addInstr(env, s390_insn_cc2bool(x_as_64, cc_x));
+         addInstr(env, s390_insn_alu(8, S390_ALU_AND, x_as_64, one));
+
+         HReg y_as_64 = newVRegI(env);
+         s390_cc_t cc_y = s390_isel_cc(env, arg2);
+         addInstr(env, s390_insn_cc2bool(y_as_64, cc_y));
+         addInstr(env, s390_insn_alu(8, S390_ALU_AND, y_as_64, one));
+
+         s390_alu_t opkind
+            = cond->Iex.Binop.op == Iop_And1 ? S390_ALU_AND : S390_ALU_OR;
+         addInstr(env, s390_insn_alu(/*size=*/8,
+                                     opkind, x_as_64, s390_opnd_reg(y_as_64)));
+
+         addInstr(env, s390_insn_alu(/*size=*/8, S390_ALU_AND, x_as_64, one));
+         addInstr(env, s390_insn_test(/*size=*/8, s390_opnd_reg(x_as_64)));
+         return S390_CC_NE;
+      }
+#     endif /* 0 */
+
+      // |sizeofIRType| asserts on Ity_I1, so we can't do it until after we're
+      // sure that Iop_And1 and Iop_Or1 can't make it this far.
       size = sizeofIRType(typeOfIRExpr(env->type_env, arg1));
 
       switch (cond->Iex.Binop.op) {
-- 
2.11.4.GIT