From abbd80e7e23deb1f27fb50410c8b3a3f7cdfb646 Mon Sep 17 00:00:00 2001

From: Andreas Arnez <arnez@linux.ibm.com>

Date: Wed, 12 Feb 2020 14:13:55 +0100

Subject: [PATCH] Bug 417452 - s390_insn_store_emit: dst->tag for HRcVec128

It was seen that the s390 instruction selector chose a wrong addressing

mode for storing a vector register.  The VST instruction only handles

short (12-bit unsigned) displacements, but a long (20-bit signed)

displacement was generated instead, resulting in a panic:

vex: the `impossible' happened:

   s390_insn_store_emit: unknown dst->tag for HRcVec128

The fix prevents long displacements for vector store operations.  It also

optimizes vector store operations from an Iex_Get, by converting them to a

memory copy.  This optimization was already performed for integer

registers.

---

 VEX/priv/host_s390_defs.c |  2 +-

 VEX/priv/host_s390_isel.c | 67 +++++++++++++++++++++++++++------------

 2 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/VEX/priv/host_s390_defs.c b/VEX/priv/host_s390_defs.c

index 9ad7240c4..47928cbe1 100644

--- a/VEX/priv/host_s390_defs.c

+++ b/VEX/priv/host_s390_defs.c

@@ -6314,7 +6314,7 @@ s390_insn_memcpy(UChar size, s390_amode *dst, s390_amode *src)

    insn->variant.memcpy.src = src;

    insn->variant.memcpy.dst = dst;

-   vassert(size == 1 || size == 2 || size == 4 || size == 8);

+   vassert(size == 1 || size == 2 || size == 4 || size == 8 || size == 16);

    return insn;

 }

diff --git a/VEX/priv/host_s390_isel.c b/VEX/priv/host_s390_isel.c

index 97614c873..fff81fe0f 100644

--- a/VEX/priv/host_s390_isel.c

+++ b/VEX/priv/host_s390_isel.c

@@ -302,12 +302,14 @@ ulong_fits_signed_8bit(ULong val)

    return val == v;

 }

-/* EXPR is an expression that is used as an address. Return an s390_amode

-   for it. If select_b12_b20_only is true the returned amode must be either

-   S390_AMODE_B12 or S390_AMODE_B20. */

+/* EXPR is an expression that is used as an address. Return an s390_amode for

+   it. If no_index is true the returned amode must be either S390_AMODE_B12 or

+   S390_AMODE_B20. If short_displacement is true it must be either

+   S390_AMODE_B12 or S390_AMODE_BX12. */

 static s390_amode *

 s390_isel_amode_wrk(ISelEnv *env, IRExpr *expr,

-                    Bool select_b12_b20_only __attribute__((unused)))

+                    Bool no_index __attribute__((unused)),

+                    Bool short_displacement)

 {

    if (expr->tag == Iex_Binop && expr->Iex.Binop.op == Iop_Add64) {

       IRExpr *arg1 = expr->Iex.Binop.arg1;

@@ -328,7 +330,7 @@ s390_isel_amode_wrk(ISelEnv *env, IRExpr *expr,

          if (ulong_fits_unsigned_12bit(value)) {

             return s390_amode_b12((Int)value, s390_isel_int_expr(env, arg1));

          }

-         if (ulong_fits_signed_20bit(value)) {

+         if (!short_displacement && ulong_fits_signed_20bit(value)) {

             return s390_amode_b20((Int)value, s390_isel_int_expr(env, arg1));

          }

       }

@@ -348,7 +350,25 @@ s390_isel_amode(ISelEnv *env, IRExpr *expr)

    /* Address computation should yield a 64-bit value */

    vassert(typeOfIRExpr(env->type_env, expr) == Ity_I64);

-   am = s390_isel_amode_wrk(env, expr, /* B12, B20 only */ False);

+   am = s390_isel_amode_wrk(env, expr, False, False);

+

+   /* Check post-condition */

+   vassert(s390_amode_is_sane(am));

+

+   return am;

+}

+

+/* Sometimes we need an amode with short (12-bit) displacement. An example is

+   the vector-store opcode. */

+static s390_amode *

+s390_isel_amode_short(ISelEnv *env, IRExpr *expr)

+{

+   s390_amode *am;

+

+   /* Address computation should yield a 64-bit value */

+   vassert(typeOfIRExpr(env->type_env, expr) == Ity_I64);

+

+   am = s390_isel_amode_wrk(env, expr, False, True);

    /* Check post-condition */

    vassert(s390_amode_is_sane(am));

@@ -379,7 +399,7 @@ s390_isel_amode_b12_b20(ISelEnv *env, IRExpr *expr)

    /* Address computation should yield a 64-bit value */

    vassert(typeOfIRExpr(env->type_env, expr) == Ity_I64);

-   am = s390_isel_amode_wrk(env, expr, /* B12, B20 only */ True);

+   am = s390_isel_amode_wrk(env, expr, True, False);

    /* Check post-condition */

    vassert(s390_amode_is_sane(am) &&

@@ -4727,7 +4747,26 @@ s390_isel_stmt(ISelEnv *env, IRStmt *stmt)

       if (stmt->Ist.Store.end != Iend_BE) goto stmt_fail;

-      am = s390_isel_amode(env, stmt->Ist.Store.addr);

+      if (tyd == Ity_V128) {

+         am = s390_isel_amode_short(env, stmt->Ist.Store.addr);

+      } else {

+         am = s390_isel_amode(env, stmt->Ist.Store.addr);

+      }

+

+      /* Check whether we can use a memcpy. Currently, the restriction

+         is that both amodes need to be B12, so MVC can be emitted.

+         We do not consider a store whose data expression is a load because

+         we don't want to deal with overlapping locations. */

+      /* store(get) never overlaps*/

+      if (am->tag == S390_AMODE_B12 &&

+          stmt->Ist.Store.data->tag == Iex_Get) {

+         UInt offset = stmt->Ist.Store.data->Iex.Get.offset;

+         s390_amode *from = s390_amode_for_guest_state(offset);

+         if (from->tag == S390_AMODE_B12) {

+            addInstr(env, s390_insn_memcpy(sizeofIRType(tyd), am, from));

+            return;

+         }

+      }

       switch (tyd) {

       case Ity_I8:

@@ -4742,18 +4781,6 @@ s390_isel_stmt(ISelEnv *env, IRStmt *stmt)

             addInstr(env, s390_insn_mimm(sizeofIRType(tyd), am, value));

             return;

          }

-         /* Check whether we can use a memcpy here. Currently, the restriction

-            is that both amodes need to be B12, so MVC can be emitted.

-            We do not consider a store whose data expression is a load because

-            we don't want to deal with overlapping locations. */

-         /* store(get) never overlaps*/

-         if (am->tag == S390_AMODE_B12 &&

-             stmt->Ist.Store.data->tag == Iex_Get) {

-            UInt offset = stmt->Ist.Store.data->Iex.Get.offset;

-            s390_amode *from = s390_amode_for_guest_state(offset);

-            addInstr(env, s390_insn_memcpy(sizeofIRType(tyd), am, from));

-            return;

-         }

          /* General case: compile data into a register */

          src = s390_isel_int_expr(env, stmt->Ist.Store.data);

          break;

-- 

2.23.0