[Valgrind-developers] [valgrind] s390x: Final change to fix BZ 495817

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 422-6466

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=78b3e5447172090387a981a6d9d6a0aa98455b9b

commit 78b3e5447172090387a981a6d9d6a0aa98455b9b
Author: Florian Krohm <fl...@ei...>
Date:   Sun Apr 13 11:05:08 2025 +0000

    s390x: Final change to fix BZ 495817
    
    This patch makes VEX disassembly match objdump disassembly for
    objdump versions >= 2.44. Prior objdump versions did not handle the
    nuances of the "rotate and ..." set of opcodes and there was also a
    bug disassembling BC insns.
    
    Fixes https://bugs.kde.org/show_bug.cgi?id=495817

Diff:
---
 NEWS                                  |  1 +
 VEX/priv/guest_s390_toIR.c            |  2 +-
 VEX/priv/host_s390_defs.c             |  2 +-
 VEX/priv/s390_disasm.c                | 42 ++++++++++++++++++++++++++++++-----
 VEX/priv/s390_disasm.h                |  1 +
 none/tests/s390x/disasm-test/opcode.c | 32 ++++++++++++++++++--------
 6 files changed, 64 insertions(+), 16 deletions(-)

diff --git a/NEWS b/NEWS
index 98270e95e6..fac28e7984 100644
--- a/NEWS
+++ b/NEWS
@@ -44,6 +44,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 494337  All threaded applications cause still holding lock errors
 495488  Add FreeBSD getrlimitusage syscall wrapper
 495816  s390x: Fix disassembler segfault for C[G]RT and CL[G]RT
+495817  s390x: Disassembly to match objdump -d output
 496370  Illumos: signal handling is broken
 496571  False positive for null key passed to bpf_map_get_next_key syscall.
 496950  s390x: Fix hardware capabilities and EmFail codes
diff --git a/VEX/priv/guest_s390_toIR.c b/VEX/priv/guest_s390_toIR.c
index 5381e417e8..102c6a9034 100644
--- a/VEX/priv/guest_s390_toIR.c
+++ b/VEX/priv/guest_s390_toIR.c
@@ -2856,7 +2856,7 @@ s390_format_RIE_RRUUU(const HChar *(*irgen)(UChar r1, UChar r2, UChar i3,
    const HChar *mnm = irgen(r1, r2, i3, i4, i5);
 
    if (UNLIKELY(vex_traceflags & VEX_TRACE_FE))
-      S390_DISASM(MNM(mnm), GPR(r1), GPR(r2), UINT(i3), UINT(i4), UINT(i5));
+      S390_DISASM(XMNM(mnm, rotate_disasm), GPR(r1), GPR(r2), MASK(i3), MASK(i4), MASK(i5));
 }
 
 static void
diff --git a/VEX/priv/host_s390_defs.c b/VEX/priv/host_s390_defs.c
index c503d6ddeb..b7b2f25207 100644
--- a/VEX/priv/host_s390_defs.c
+++ b/VEX/priv/host_s390_defs.c
@@ -5208,7 +5208,7 @@ static UChar *
 s390_emit_RISBG(UChar *p, UChar r1, UChar r2, UChar i3, Char i4, UChar i5)
 {
    if (UNLIKELY(vex_traceflags & VEX_TRACE_ASM))
-      S390_DISASM(MNM("risbg"), GPR(r1), GPR(r2), UINT(i3), UINT(i4), UINT(i5));
+      S390_DISASM(XMNM("risbg", rotate_disasm), GPR(r1), GPR(r2), MASK(i3), MASK(i4), MASK(i5));
 
    return emit_RIEf(p, 0xec0000000055ULL, r1, r2, i3, i4, i5);
 }
diff --git a/VEX/priv/s390_disasm.c b/VEX/priv/s390_disasm.c
index 0b03c14edd..0eb8f922f5 100644
--- a/VEX/priv/s390_disasm.c
+++ b/VEX/priv/s390_disasm.c
@@ -164,7 +164,8 @@ dxb_operand(HChar *p, UInt d, UInt x, UInt b, Bool displacement_is_signed)
       p += vex_sprintf(p, "%u", d);
    }
    if (x != 0) {
-      p += vex_sprintf(p, "(%s,%s)", gpr_operand(x), gpr_operand(b));
+      p += vex_sprintf(p, "(%s,%s)", gpr_operand(x),
+                       b != 0 ? gpr_operand(b) : "0");
    } else {
       if (b != 0) {
          p += vex_sprintf(p, "(%s)", gpr_operand(b));
@@ -182,7 +183,7 @@ udlb_operand(HChar *p, UInt d, UInt length, UInt b)
 {
    p += vex_sprintf(p, "%u", d);
    p += vex_sprintf(p, "(%u", length + 1);  // actual length is +1
-   p += vex_sprintf(p, ",%s", gpr_operand(b));
+   p += vex_sprintf(p, ",%s", b != 0 ? gpr_operand(b) : "0");
    p += vex_sprintf(p, ")");
 
    return p;
@@ -203,7 +204,7 @@ dvb_operand(HChar *p, UInt d, UInt v, UInt b, Bool displacement_is_signed)
       p += vex_sprintf(p, "%u", d);
    }
    p += vex_sprintf(p, "(%s", vr_operand(v));
-   p += vex_sprintf(p, ",%s", gpr_operand(b));
+   p += vex_sprintf(p, ",%s", b != 0 ? gpr_operand(b) : "0");
    p += vex_sprintf(p, ")");
 
    return p;
@@ -282,9 +283,11 @@ bc_disasm(const s390_opnd *opnds, HChar *p)
    const HChar *xmnm;
    UInt mask = opnds[1].mask;
 
-   if (mask == 0)
+   if (mask == 0) {
       xmnm = "nop";
-   else if (mask == 15)
+      if (opnds[2].d == 0 && opnds[2].b == 0 && opnds[2].x == 0)
+         return p += vex_sprintf(p, "nop");
+   } else if (mask == 15)
       xmnm = "b";
    else
       xmnm = construct_mnemonic("b", "", mask);
@@ -1084,6 +1087,35 @@ adtra_like_disasm(const s390_opnd *opnds, HChar *p)
 }
 
 
+static Int
+rotate_mh(UInt ix __attribute__((unused)), UInt mask, UInt *value)
+{
+   *value = mask;
+   if (ix == 5 && mask == 0) return 0;
+   if (ix == 3)   // rosbg, etc
+      *value = mask & ~0x80;
+   if (ix == 4)   // risbg
+      *value = mask & ~0x80;
+   return 1;
+}
+
+
+HChar *
+rotate_disasm(const s390_opnd *opnds, HChar *p)
+{
+   const HChar *base = opnds[0].xmnm.base;
+   UInt len = vex_strlen(base);
+   HChar xmnm[len + 1];
+
+   if (opnds[0].xmnm.base[1] == 'i')
+      vex_sprintf(xmnm, "%s%c", base, (opnds[4].u & 0x80) ? 'z' : '\0');
+   else
+      vex_sprintf(xmnm, "%s%c", base, (opnds[3].u & 0x80) ? 't' : '\0');
+
+   return s390_disasm_aux(opnds, xmnm, p, rotate_mh);
+}
+
+
 /* Write out OPNDS. MH is a mask handler. It decides whether or not a
    MASK operand is written and if so, massages the mask value as needed. */
 static HChar *
diff --git a/VEX/priv/s390_disasm.h b/VEX/priv/s390_disasm.h
index f84f5061f6..14731898c5 100644
--- a/VEX/priv/s390_disasm.h
+++ b/VEX/priv/s390_disasm.h
@@ -138,6 +138,7 @@ HChar *vfmix_like_disasm(const s390_opnd *, HChar *);
 HChar *fp_convf_disasm(const s390_opnd *, HChar *);
 HChar *fp_convt_disasm(const s390_opnd *, HChar *);
 HChar *adtra_like_disasm(const s390_opnd *, HChar *);
+HChar *rotate_disasm(const s390_opnd *, HChar *);
 
 /*---------------------------------------------------------------*/
 /*--- end                                       s390_disasm.h ---*/
diff --git a/none/tests/s390x/disasm-test/opcode.c b/none/tests/s390x/disasm-test/opcode.c
index 24684a613a..e638c973c5 100644
--- a/none/tests/s390x/disasm-test/opcode.c
+++ b/none/tests/s390x/disasm-test/opcode.c
@@ -743,15 +743,29 @@ static const char *opcodes[] = {
    "rll    r1,r3,d20(b2)",
    "rllg   r1,r3,d20(b2)",
 
-   "rnsbg  r1,r2,i3:u8,i4:u8,i5:u8",  // gie FIXME  un/signed  i3/4/5 ? t-bit ? z-bit?
-   "rxsbg  r1,r2,i3:u8,i4:u8,i5:u8",  // gie FIXME  ditto
-   "rosbg  r1,r2,i3:u8,i4:u8,i5:u8",  // gie FIXME  ditto
-
-   "risbg  r1,r2,i3:u8,i4:u8,i5:u8",  // gie FIXME  ditto
-   "risbgn r1,r2,i3:u8,i4:u8,i5:u8",  // mi1 FIXME  ditto
-
-   "risbhg r1,r2,i3:u8,i4:u8,i5:u8",  // hiwo FIXME  ditto
-   "risblg r1,r2,i3:u8,i4:u8,i5:u8",  // hiwo FIXME  ditto
+   // Rotate and .... opcodes require special handling
+   //
+   // For rosbg and friends
+   // - Bit #0 of i3 is the T-bit and bit #1 of i3 ought to be 0.
+   // - i5 is optional and will not be written when 0
+   //
+   // For risbg and friends
+   // - Bit #0 of i4 is the Z-bit and bit #1 of i4 ought to be 0.
+   // - i5 is optional and will not be written when 0
+   //
+   // This implies that we need to model i3, i4 and i5 as masks so
+   // we can manipulate their value when disassembling or suppress
+   // the mask altogether. Note that we limit the set of allowed values
+   // for those masks to avoid excessively large numbers of testcases.
+   "rnsbg  r1,r2,m3:u8{0,1,2,63,128,129,191},m4:u6{0,1,2,63},m5:u6{0,1,2,63}",  // gie
+   "rxsbg  r1,r2,m3:u8{0,1,2,63,128,129,191},m4:u6{0,1,2,63},m5:u6{0,1,2,63}",  // gie
+   "rosbg  r1,r2,m3:u8{0,1,2,63,128,129,191},m4:u6{0,1,2,63},m5:u6{0,1,2,63}",  // gie
+
+   "risbg  r1,r2,m3:u6{0,1,2,63},m4:u8{0,1,2,63,128,129,191},m5:u6{0,1,2,63}",  // gie
+   "risbgn r1,r2,m3:u6{0,1,2,63},m4:u8{0,1,2,63,128,129,191},m5:u6{0,1,2,63}",  // mi1
+
+   "risbhg r1,r2,m3:u5{0,1,2,31},m4:u8{0,1,2,31,128,129,159},m5:u6{0,1,2,63}",  // hiwo
+   "risblg r1,r2,m3:u5{0,1,2,31},m4:u8{0,1,2,31,128,129,159},m5:u6{0,1,2,63}",  // hiwo
 
    "srst   r1,r2",


