[Valgrind-developers] [valgrind] s390x: Fix IR generation for SRNMB

[Florian K. <fk...@so...>]

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=1d8257e4a24356198d9a620904ac516bc94d6699

commit 1d8257e4a24356198d9a620904ac516bc94d6699
Author: Florian Krohm <fl...@ei...>
Date:   Sat Apr 12 21:45:22 2025 +0000

    s390x: Fix IR generation for SRNMB
    
    disasm-test generated this test:
    
      asm volatile("srnmb 4095(%r0)");
    
    which was disassembled in VEX to
    
      srnmb 0
    
    causing a mismatch with what objdump does.
    
    4095 is not a valid bfp rounding mode and the old implementation of
    s390_irgen_SRNMB let it slide by with a warning. However, POP calls for a
    specification exception in this case. This is what this patch changes.i
    It also eliminates the s390_irgen_srnmb_wrapper function along the way.
    
    While adjusting none/tests/s390x/srnmb.* I noticed that the line
    number shown in the specification exception is incorrect. This
    is not a regression. It has been incorrect before as well.
    
    I have opened https://bugs.kde.org/show_bug.cgi?id=502729 fro the line
    number issue.

Diff:
---
 VEX/priv/guest_s390_toIR.c        | 41 +++++++++++++++++----------------------
 VEX/priv/main_main.c              |  3 ---
 VEX/pub/libvex_emnote.h           |  3 ---
 none/tests/s390x/srnmb.c          |  3 ++-
 none/tests/s390x/srnmb.stderr.exp | 21 ++++++++++++++++----
 none/tests/s390x/srnmb.stdout.exp |  1 -
 6 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/VEX/priv/guest_s390_toIR.c b/VEX/priv/guest_s390_toIR.c
index 63dc360b0f..5381e417e8 100644
--- a/VEX/priv/guest_s390_toIR.c
+++ b/VEX/priv/guest_s390_toIR.c
@@ -11313,12 +11313,24 @@ s390_irgen_SRNM(IRTemp op2addr)
 }
 
 static const HChar *
-s390_irgen_SRNMB(IRTemp op2addr)
+s390_irgen_SRNMB(UChar b2, UShort d2)
 {
-   UInt input_mask, fpc_mask;
+   /* Can only check at IR generation time when b2 == 0 */
+   if (b2 == 0) {
+      s390_insn_assert("srnmb", d2 <= 3 || d2 == 7);  // valid rounding mode
+      /* d2 == 7 requires fpext */
+      if (d2 == 7 && ! s390_host_has_fpext) {
+         emulation_failure(EmFail_S390X_fpext);
+         return "srnmb";
+      }
+   }
+   IRTemp op2addr = newTemp(Ity_I64);
 
-   input_mask = 7;
-   fpc_mask = 7;
+   assign(op2addr, binop(Iop_Add64, mkU64(d2), b2 != 0 ? get_gpr_dw0(b2) :
+          mkU64(0)));
+
+   UInt input_mask = 7;
+   UInt fpc_mask = 7;
 
    put_fpc_w0(binop(Iop_Or32,
                     binop(Iop_And32, get_fpc_w0(), mkU32(~fpc_mask)),
@@ -11327,25 +11339,8 @@ s390_irgen_SRNMB(IRTemp op2addr)
    return "srnmb";
 }
 
-static void
-s390_irgen_srnmb_wrapper(UChar b2, UShort d2)
-{
-   if (b2 == 0) {  /* This is the typical case */
-      if (d2 > 3) {
-         if (s390_host_has_fpext && d2 == 7) {
-            /* ok */
-         } else {
-            emulation_warning(EmWarn_S390X_invalid_rounding);
-            d2 = S390_FPC_BFP_ROUND_NEAREST_EVEN;
-         }
-      }
-   }
-
-   s390_format_S_RD(s390_irgen_SRNMB, b2, d2);
-}
 
-/* Wrapper to validate the parameter as in SRNMB is not required, as all
-   the 8 values in op2addr[61:63] correspond to a valid DFP rounding mode */
+/* All 8 values in op2addr[61:63] correspond to a valid DFP rounding mode */
 static const HChar *
 s390_irgen_SRNMT(IRTemp op2addr)
 {
@@ -21017,7 +21012,7 @@ s390_decode_4byte_and_irgen(const UChar *bytes)
                                  goto ok;
    case 0xb2b1: /* STFL */ goto unimplemented;
    case 0xb2b2: /* LPSWE */ goto unimplemented;
-   case 0xb2b8: s390_irgen_srnmb_wrapper(S_b2(ovl), S_d2(ovl));
+   case 0xb2b8: s390_format_S_RD_raw(s390_irgen_SRNMB, S_b2(ovl), S_d2(ovl));
       goto ok;
    case 0xb2b9: s390_format_S_RD(s390_irgen_SRNMT, S_b2(ovl), S_d2(ovl));
       goto ok;
diff --git a/VEX/priv/main_main.c b/VEX/priv/main_main.c
index fe3cf9e573..0520fd7470 100644
--- a/VEX/priv/main_main.c
+++ b/VEX/priv/main_main.c
@@ -1535,9 +1535,6 @@ const HChar* LibVEX_EmNote_string ( VexEmNote ew )
                "  feature requires the floating point extension facility\n"
                "  which is not available on this host. Continuing using\n"
                "  the rounding mode from FPC. Results may differ!";
-     case EmWarn_S390X_invalid_rounding:
-        return "The specified rounding mode is invalid.\n"
-               "  Continuing using 'round to nearest'. Results may differ!";
      case EmFail_S390X_stfle:
         return "Instruction stfle is not supported on this host";
      case EmFail_S390X_stckf:
diff --git a/VEX/pub/libvex_emnote.h b/VEX/pub/libvex_emnote.h
index 27b95880cb..be17a8922f 100644
--- a/VEX/pub/libvex_emnote.h
+++ b/VEX/pub/libvex_emnote.h
@@ -89,9 +89,6 @@ typedef
          facility is not available on this host */
       EmWarn_S390X_fpext_rounding,
 
-      /* insn (e.g. srnmb) specifies an invalid rounding mode */
-      EmWarn_S390X_invalid_rounding,
-
       /* stfle insn is not supported on this host */
       EmFail_S390X_stfle,
 
diff --git a/none/tests/s390x/srnmb.c b/none/tests/s390x/srnmb.c
index 548b644ca3..72fc799fb7 100644
--- a/none/tests/s390x/srnmb.c
+++ b/none/tests/s390x/srnmb.c
@@ -27,6 +27,7 @@
 
 int main(void)
 {
+   setlinebuf(stdout);
    printf("initial rounding mode = %u\n", get_rounding_mode());
 
    /* Set basic rounding modes in various ways */
@@ -57,7 +58,7 @@ int main(void)
    srnmb(0,001);
    printf("rounding mode = %u\n", get_rounding_mode());
 
-   srnmb0(004);    // -> emul warning invalid rounding mode
+   srnmb0(004);    // -> specification exception
    printf("rounding mode = %u\n", get_rounding_mode());
 
    return 0;
diff --git a/none/tests/s390x/srnmb.stderr.exp b/none/tests/s390x/srnmb.stderr.exp
index b9db22774f..bfdaf8b0ab 100644
--- a/none/tests/s390x/srnmb.stderr.exp
+++ b/none/tests/s390x/srnmb.stderr.exp
@@ -1,6 +1,19 @@
 
-Emulation warning: unsupported action:
-  The specified rounding mode is invalid.
-  Continuing using 'round to nearest'. Results may differ!
-   at 0x........: main (srnmb.c:61)
+vex s390->IR: specification exception: B2B8 0004
+valgrind: Unrecognised instruction at address 0x.........
+   at 0x........: main (srnmb.c:59)
+Your program just tried to execute an instruction that Valgrind
+did not recognise.  There are two possible reasons for this.
+1. Your program has a bug and erroneously jumped to a non-code
+   location.  If you are running Memcheck and you just saw a
+   warning about a bad jump, it's probably your program's fault.
+2. The instruction is legitimate but Valgrind doesn't handle it,
+   i.e. it's Valgrind's fault.  If you think this is the case or
+   you are not sure, please let us know and we'll try to fix it.
+Either way, Valgrind will now raise a SIGILL signal which will
+probably kill your program.
+
+Process terminating with default action of signal 4 (SIGILL)
+ Illegal opcode at address 0x........
+   at 0x........: main (srnmb.c:59)
 
diff --git a/none/tests/s390x/srnmb.stdout.exp b/none/tests/s390x/srnmb.stdout.exp
index c2be0f77a2..c9127aaf1c 100644
--- a/none/tests/s390x/srnmb.stdout.exp
+++ b/none/tests/s390x/srnmb.stdout.exp
@@ -4,4 +4,3 @@ rounding mode = 2
 rounding mode = 1
 rounding mode = 0
 rounding mode = 1
-rounding mode = 0