]> git.ipfire.org Git - thirdparty/valgrind.git/commitdiff
'grail' fixes for MIPS: grail
authorJulian Seward <jseward@acm.org>
Sun, 15 Dec 2019 19:14:37 +0000 (20:14 +0100)
committerJulian Seward <jseward@acm.org>
Sun, 15 Dec 2019 19:14:37 +0000 (20:14 +0100)
This isn't a good result.  It merely disables the new functionality on MIPS
because enabling it causes segfaults, even with --tool=none, the cause of
which are not obvious.  It is only chasing through conditional branches that
is disabled, though.  Chasing through unconditional branches (jumps and calls
to known destinations) is still enabled.

* guest_generic_bb_to_IR.c bb_to_IR(): Disable, hopefully temporarily, the key
  &&-recovery transformation on MIPS.

* VEX/priv/host_mips_isel.c iselWordExpr_R_wrk(), iselCondCode_wrk():

  - add support for Iop_And1, Iop_Or1, and IRConst_U1.  This code is my best
    guess about what is correct, but is #if 0'd for now.

  - Properly guard some Iex_Binop cases that lacked a leading check that the
    expression actually was a Binop.

VEX/priv/guest_generic_bb_to_IR.c
VEX/priv/host_mips_isel.c

index 0b8f852ec0195887eda27502386cc2fb01163e72..507c75e530c8b395121c2c4f5ff0cb11cdd64241 100644 (file)
@@ -1451,7 +1451,10 @@ IRSB* bb_to_IR (
              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)
+          && arch_guest != VexArchS390X
+          /* sewardj 2019Dec14: It also causes crashing on MIPS, even for
+             --tool=none. */
+          && arch_guest != VexArchMIPS64 && arch_guest != VexArchMIPS32)
       {
          if (debug_print) {
             vex_printf("\n-+-+ (ext# %d) Considering cbranch to"
index c49d152b72d1948cc691839e2a3b67f18f4a7320..f14f6543486ffa56df49cd5a251b9146788ac61d 100644 (file)
@@ -2364,6 +2364,13 @@ static HReg iselWordExpr_R_wrk(ISelEnv * env, IRExpr * e)
          case Ico_U8:
             l = (Long) (Int) (Char) con->Ico.U8;
             break;
+#if 0
+         // Not needed until chasing cond branches in bb_to_IR is enabled on
+         // MIPS.  See comment on And1/Or1 below.
+         case Ico_U1:
+            l = con->Ico.U1 ? 1 : 0;
+            break;
+#endif
          default:
             vpanic("iselIntExpr_R.const(mips)");
       }
@@ -2644,18 +2651,19 @@ static MIPSCondCode iselCondCode_wrk(ISelEnv * env, IRExpr * e)
    vassert(e);
    vassert(typeOfIRExpr(env->type_env, e) == Ity_I1);
    /* Cmp*32*(x,y) ? */
-   if (e->Iex.Binop.op == Iop_CmpEQ32
-       || e->Iex.Binop.op == Iop_CmpNE32
-       || e->Iex.Binop.op == Iop_CmpNE64
-       || e->Iex.Binop.op == Iop_CmpLT32S
-       || e->Iex.Binop.op == Iop_CmpLT32U
-       || e->Iex.Binop.op == Iop_CmpLT64U
-       || e->Iex.Binop.op == Iop_CmpLE32S
-       || e->Iex.Binop.op == Iop_CmpLE64S
-       || e->Iex.Binop.op == Iop_CmpLT64S
-       || e->Iex.Binop.op == Iop_CmpEQ64
-       || e->Iex.Binop.op == Iop_CasCmpEQ32
-       || e->Iex.Binop.op == Iop_CasCmpEQ64) {
+   if (e->tag == Iex_Binop
+       && (e->Iex.Binop.op == Iop_CmpEQ32
+           || e->Iex.Binop.op == Iop_CmpNE32
+           || e->Iex.Binop.op == Iop_CmpNE64
+           || e->Iex.Binop.op == Iop_CmpLT32S
+           || e->Iex.Binop.op == Iop_CmpLT32U
+           || e->Iex.Binop.op == Iop_CmpLT64U
+           || e->Iex.Binop.op == Iop_CmpLE32S
+           || e->Iex.Binop.op == Iop_CmpLE64S
+           || e->Iex.Binop.op == Iop_CmpLT64S
+           || e->Iex.Binop.op == Iop_CmpEQ64
+           || e->Iex.Binop.op == Iop_CasCmpEQ32
+           || e->Iex.Binop.op == Iop_CasCmpEQ64)) {
 
       Bool syned = (e->Iex.Binop.op == Iop_CmpLT32S
                    || e->Iex.Binop.op == Iop_CmpLE32S
@@ -2726,7 +2734,7 @@ static MIPSCondCode iselCondCode_wrk(ISelEnv * env, IRExpr * e)
                dst, mode64));
       return cc;
    }
-   if (e->Iex.Binop.op == Iop_Not1) {
+   if (e->tag == Iex_Unop && e->Iex.Binop.op == Iop_Not1) {
       HReg r_dst = newVRegI(env);
       HReg r_srcL = iselWordExpr_R(env, e->Iex.Unop.arg);
       MIPSRH *r_srcR = MIPSRH_Reg(r_srcL);
@@ -2742,7 +2750,33 @@ static MIPSCondCode iselCondCode_wrk(ISelEnv * env, IRExpr * e)
                r_dst, mode64));
       return MIPScc_NE;
    }
-   if (e->tag == Iex_RdTmp || e->tag == Iex_Unop) {
+#if 0
+   // sewardj 2019Dec14: this is my best attempt at And1/Or1, but I am not
+   // sure if it is correct.  In any case it is not needed until chasing cond
+   // branches is enabled on MIPS.  Currently it is disabled, in function bb_to_IR
+   // (see comments there).
+   if (e->tag == Iex_Binop
+       && (e->Iex.Binop.op == Iop_And1 || e->Iex.Binop.op == Iop_Or1)) {
+      HReg r_argL = iselWordExpr_R(env, e->Iex.Binop.arg1);
+      HReg r_argR = iselWordExpr_R(env, e->Iex.Binop.arg2);
+      HReg r_dst  = newVRegI(env);
+      addInstr(env, MIPSInstr_Alu(e->Iex.Binop.op == Iop_And1 ? Malu_AND : Malu_OR,
+                                  r_dst, r_argL, MIPSRH_Reg(r_argR)));
+      addInstr(env, MIPSInstr_Alu(Malu_AND, r_dst, r_dst, MIPSRH_Imm(False, 1)));
+      /* Store result to guest_COND */
+      /* sewardj 2019Dec13: this seems wrong to me.  The host-side instruction
+         selector shouldn't touch the guest-side state, except in response to
+         Iex_Get and Ist_Put. */
+      MIPSAMode *am_addr = MIPSAMode_IR(0, GuestStatePointer(mode64));
+
+      addInstr(env, MIPSInstr_Store(4,
+               MIPSAMode_IR(am_addr->Mam.IR.index + COND_OFFSET(mode64),
+                            am_addr->Mam.IR.base),
+               r_dst, mode64));
+      return MIPScc_EQ;
+   }
+#endif
+   if (e->tag == Iex_RdTmp) {
       HReg r_dst = iselWordExpr_R_wrk(env, e);
       /* Store result to guest_COND */
       MIPSAMode *am_addr = MIPSAMode_IR(0, GuestStatePointer(mode64));