Fix s390_tchain_patch_load64; some bytes were mixed up.

Fix unchainXDirect_S390; modified place_to_unchain address
before patching the code there.
Add some convenience functions for insn verification in
chain/unchain machinery.
Avoid magic constants.

git-svn-id: svn://svn.valgrind.org/vex/branches/TCHAIN@2286

diff --git a/VEX/priv/host_s390_defs.c b/VEX/priv/host_s390_defs.c
index 30a2bdbe57f0b426f14deb6ef2bc815ae54c2863..6d420c9196a928e28422b60e486037ee68aba1f4 100644 (file)
--- a/VEX/priv/host_s390_defs.c
+++ b/VEX/priv/host_s390_defs.c
@@ -59,6 +59,7 @@ const VexArchInfo *s390_archinfo_host;
 static Bool s390_insn_is_reg_reg_move(const s390_insn *, HReg *src, HReg *dst);
 static void s390_insn_map_regs(HRegRemap *, s390_insn *);
 static void s390_insn_get_reg_usage(HRegUsage *u, const s390_insn *);
+static UInt s390_tchain_load64_len(void);
 
 
 /*------------------------------------------------------------*/
@@ -7206,6 +7207,24 @@ s390_insn_gadd_emit(UChar *buf, const s390_insn *insn)
 /* Define convenience functions needed for translation chaining.
    Any changes need to be applied to the functions in concert. */
 
+static __inline__ Bool
+s390_insn_is_BRCL(const UChar *p, UChar condition)
+{
+   return p[0] == 0xc0 && p[1] == ((condition << 4) | 0x04);
+}
+
+static __inline__ Bool
+s390_insn_is_BR(const UChar *p, UChar reg)
+{
+   return p[0] == 0x07 && p[1] == (0xF0 | reg);  /* BCR 15,reg */
+}
+
+static __inline__ Bool
+s390_insn_is_BASR(const UChar *p, UChar link_reg, UChar other_reg)
+{
+   return p[0] == 0x0D && p[1] == ((link_reg << 4) | other_reg);
+}
+
 /* Load the 64-bit VALUE into REG. Note that this function must NOT
    optimise the generated code by looking at the value. I.e. using
    LGHI if value == 0 would be very wrong.
@@ -7213,6 +7232,8 @@ s390_insn_gadd_emit(UChar *buf, const s390_insn *insn)
 static UChar *
 s390_tchain_load64(UChar *buf, UChar regno, ULong value)
 {
+   UChar *begin = buf;
+
    buf = s390_emit_IILL(buf, regno, value & 0xFFFF);
    value >>= 16;
    buf = s390_emit_IILH(buf, regno, value & 0xFFFF);
@@ -7221,6 +7242,8 @@ s390_tchain_load64(UChar *buf, UChar regno, ULong value)
    value >>= 16;
    buf = s390_emit_IIHH(buf, regno, value & 0xFFFF);
 
+   vassert(buf - begin == s390_tchain_load64_len());
+
    return buf;
 }
 
@@ -7228,8 +7251,7 @@ s390_tchain_load64(UChar *buf, UChar regno, ULong value)
 static UInt
 s390_tchain_load64_len(void)
 {
-   vassert(S390_TCHAIN_LOAD64_LEN == 16);
-   return 16;
+   return S390_TCHAIN_LOAD64_LEN;
 }
 
 /* Verify that CODE is the code sequence generated by s390_tchain_load64
@@ -7269,7 +7291,7 @@ s390_tchain_verify_load64(const UChar *code, UChar regno, ULong value)
    vassert(code[14] == (hw >> 8));
    vassert(code[15] == (hw & 0xFF));
 
-   return code + 16;
+   return code + s390_tchain_load64_len();
 }
 
 /* CODE points to the code sequence as generated by s390_tchain_load64.
@@ -7278,16 +7300,16 @@ s390_tchain_verify_load64(const UChar *code, UChar regno, ULong value)
 static UChar *
 s390_tchain_patch_load64(UChar *code, ULong imm64)
 {
-   code[2]  = imm64 & 0xFF; imm64 >>= 8;
    code[3]  = imm64 & 0xFF; imm64 >>= 8;
-   code[6]  = imm64 & 0xFF; imm64 >>= 8;
+   code[2]  = imm64 & 0xFF; imm64 >>= 8;
    code[7]  = imm64 & 0xFF; imm64 >>= 8;
-   code[10] = imm64 & 0xFF; imm64 >>= 8;
+   code[6]  = imm64 & 0xFF; imm64 >>= 8;
    code[11] = imm64 & 0xFF; imm64 >>= 8;
-   code[14] = imm64 & 0xFF; imm64 >>= 8;
+   code[10] = imm64 & 0xFF; imm64 >>= 8;
    code[15] = imm64 & 0xFF; imm64 >>= 8;
+   code[14] = imm64 & 0xFF; imm64 >>= 8;
 
-   return code + 16;
+   return code + s390_tchain_load64_len();
 }
 
 
@@ -7372,7 +7394,7 @@ s390_insn_xdirect_emit(UChar *buf, const s390_insn *insn,
 static UInt
 s390_xdirect_patchable_len(void)
 {
-   return s390_tchain_load64_len() + 2;  /* 2 == BASR */
+   return s390_tchain_load64_len() + S390_TCHAIN_CALL_LEN;
 }
 
 
@@ -7775,14 +7797,13 @@ chainXDirect_S390(void *place_to_chain,
 {
    /* What we're expecting to see @ PLACE_TI_CHAIN is:
 
-        load disp_cp_chain_me_EXPECTED into S390_REGNO_TCHAIN_SCRATCH
+        load tchain-scratch, #disp_cp_chain_me_EXPECTED
         BASR  1,S390_REGNO_TCHAIN_SCRATCH
    */
    const UChar *next;
    next = s390_tchain_verify_load64(place_to_chain, S390_REGNO_TCHAIN_SCRATCH,
                                     Ptr_to_ULong(disp_cp_chain_me_EXPECTED));
-   vassert(next[0] == 0x0D);  // BASR  1,tchain_scratch
-   vassert(next[1] == (0x10 | S390_REGNO_TCHAIN_SCRATCH));
+   vassert(s390_insn_is_BASR(next, 1, S390_REGNO_TCHAIN_SCRATCH));
 
    /* And what we want to change it to is either:
         (general case):
@@ -7828,14 +7849,21 @@ chainXDirect_S390(void *place_to_chain,
    if (shortOK) {
       p = s390_emit_BRCL(p, S390_CC_ALWAYS, delta);  /* 6 bytes */
 
+      /* Make sre that BRCL fits into the patchable part of an xdirect
+         code sequence */
+      vassert(6 < s390_xdirect_patchable_len());
+
       /* Fill remaining bytes with 0x00 (invalid opcode) */
       Int i;
       for (i = 0; i < s390_xdirect_patchable_len() - 6; ++i)
          p[i] = 0x00;
    } else {
-      /* Minimal modifications from the starting sequence. */   
-      p = s390_tchain_patch_load64(p, Ptr_to_ULong(place_to_jump_to));
-      /* Turn the call into a branch: BR *tchain_scratch */
+      /*
+          load tchain_scratch, #place_to_jump_to
+          BR  *tchain_scratch
+      */
+      ULong addr = Ptr_to_ULong(place_to_jump_to);
+      p = s390_tchain_load64(p, S390_REGNO_TCHAIN_SCRATCH, addr);
       s390_emit_BCR(p, S390_CC_ALWAYS, S390_REGNO_TCHAIN_SCRATCH);
    }
 
@@ -7863,18 +7891,17 @@ unchainXDirect_S390(void *place_to_unchain,
           invalid opcodes
    */
    UChar *p = place_to_unchain;
- 
-   if (p[0] == 0xc0 && p[1] == ((S390_CC_ALWAYS << 4) | 0x04)) {
+
+   if (s390_insn_is_BRCL(p, S390_CC_ALWAYS)) {
       /* Looks like the short form */
       Int num_hw = *(Int *)&p[2];
       Int delta = 2 *num_hw;
 
       vassert(p + delta == place_to_jump_to_EXPECTED);
 
-      p += 6;
       Int i;
       for (i = 0; i < s390_xdirect_patchable_len() - 6; ++i)
-         vassert(p[i] == 0x00);
+         vassert(p[6+i] == 0x00);
    } else {
       /* Should be the long form */
       const UChar *next;
@@ -7882,8 +7909,7 @@ unchainXDirect_S390(void *place_to_unchain,
       next = s390_tchain_verify_load64(p, S390_REGNO_TCHAIN_SCRATCH,
                                        Ptr_to_ULong(place_to_jump_to_EXPECTED));
       /* Check for BR *tchain_scratch */
-      vassert(next[0] == 0x07 && /* BCR */
-              (next[1] == (0xF0 | S390_REGNO_TCHAIN_SCRATCH)));
+      vassert(s390_insn_is_BR(next, S390_REGNO_TCHAIN_SCRATCH));
    }
 
    /* And what we want to change it to is:
@@ -7893,8 +7919,6 @@ unchainXDirect_S390(void *place_to_unchain,
    */
    ULong addr = Ptr_to_ULong(disp_cp_chain_me);
    p = s390_tchain_load64(p, S390_REGNO_TCHAIN_SCRATCH, addr);
-
-   /* call *tchain_scratch */
    s390_emit_BASR(p, 1, S390_REGNO_TCHAIN_SCRATCH);
 
    VexInvalRange vir = {0, 0};