From: Florian Krohm <flo2030@eich-krohm.de>
Date: Tue, 6 May 2025 21:22:47 +0000 (+0000)
Subject: s390x: BPRP related fixes
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=cb266a0b87b3e6d432b8f1d4d0cbe0351222d783;p=thirdparty%2Fvalgrind.git

s390x: BPRP related fixes

objdump disassembly for BPRP looks like so:

c5 0f ff 00 00 00   bprp  0,88 <main+0x88>,8a <main+0x8a>

But the disasm-test parser assumed there could only be one
address including a symbol name on a given line. It stopped
comparison beyond that point.
The line

c5 0f ff 00 00 00   bprp  0,88 <main+0x88>,fffe <main+0xfffe>

would compare equal to the above -- a false positive.

Once fixed, BPRP testcases began failing. This is because the i3
field is 24 bit wide. So an UShort is not good enough to represent
it.
---

diff --git a/VEX/priv/guest_s390_toIR.c b/VEX/priv/guest_s390_toIR.c
index 405bcfa7f..b3ee122c5 100644
--- a/VEX/priv/guest_s390_toIR.c
+++ b/VEX/priv/guest_s390_toIR.c
@@ -2757,8 +2757,8 @@ s390_format_E(const HChar *(*irgen)(void))
 }
 
 static void
-s390_format_MII_UPP(const HChar *(*irgen)(UChar m1, UShort i2, UShort i3),
-                    UChar m1, UShort i2, UShort i3)
+s390_format_MII_UPP(const HChar *(*irgen)(UChar m1, UShort i2, UInt i3),
+                    UChar m1, UShort i2, UInt i3)
 {
    const HChar *mnm;
 
@@ -2766,7 +2766,7 @@ s390_format_MII_UPP(const HChar *(*irgen)(UChar m1, UShort i2, UShort i3),
 
    if (UNLIKELY(vex_traceflags & VEX_TRACE_FE))
       S390_DISASM(MNM(mnm), UINT(m1), PCREL((Int)((Short)(i2 << 4) >> 4)),
-                  PCREL((Int)(Short)i3));
+                  PCREL((Int)(i3 << 8) >> 8));
 }
 
 static void
@@ -20621,7 +20621,7 @@ s390_irgen_BPP(UChar m1, UShort i2, IRTemp op3addr)
 }
 
 static const HChar *
-s390_irgen_BPRP(UChar m1, UShort i2, UShort i3)
+s390_irgen_BPRP(UChar m1, UShort i2, UInt i3)
 {
    /* Treat as a no-op */
    return "bprp";
diff --git a/none/tests/s390x/disasm-test/objdump.c b/none/tests/s390x/disasm-test/objdump.c
index 7100e4742..1646fe9ba 100644
--- a/none/tests/s390x/disasm-test/objdump.c
+++ b/none/tests/s390x/disasm-test/objdump.c
@@ -153,17 +153,6 @@ read_objdump(const char *file)
 
       char *dis_insn = p;
 
-      /* Remove symbolic jump targets, if any. E.g. change
-         1b68:    c0 e5 ff ff fd a4    brasl   %r14,16b0 <puts@plt>  to
-         1b68:    c0 e5 ff ff fd a4    brasl   %r14,16b0
-      */
-      p = strchr(p, '<');
-      if (p) {
-         *p-- = '\0';
-         while (isspace(*p))   // remove trailing white space
-            *p-- = '\0';
-      }
-
       if (strncmp(dis_insn, mark, strlen(mark)) == 0) {
          if (marker_seen)
             break;          // we're done
diff --git a/none/tests/s390x/disasm-test/verify.c b/none/tests/s390x/disasm-test/verify.c
index 2d6fe48c0..8dee9f596 100644
--- a/none/tests/s390x/disasm-test/verify.c
+++ b/none/tests/s390x/disasm-test/verify.c
@@ -121,27 +121,63 @@ disasm_same(const char *from_objdump, const char *from_vex,
    const char *p2 = from_vex;
 
    while (42) {
+      while (isspace(*p1))
+         ++p1;
+      while (isspace(*p2))
+         ++p2;
       if (*p1 == '\0' && *p2 == '\0')
          return 1;
       if (*p1 == '\0' || *p2 == '\0')
          return 0;
+
+      if (*p1 == *p2) {
+         ++p1;
+         ++p2;
+         continue;
+      }
+
+      /* Consider the case where the VEX disassembly has ".+integer"
+         or ".-integer" and the objdump disassembly has an hex address
+         possibly followed by a symbolic address, e.g. <main+0xe>. */
+      if (*p2++ != '.') return 0;
+
+      long long offset_in_bytes = 0;
+      unsigned long long target_address = 0;
+
+      while (isxdigit(*p1)) {
+         target_address *= 16;
+         if (isdigit(*p1))
+            target_address += *p1 - '0';
+         else {
+            int c = tolower(*p1);
+            if (c >= 'a' && c <= 'f')
+               target_address += 10 + c - 'a';
+            else
+               return 0;  // error
+         }
+         ++p1;
+      }
       while (isspace(*p1))
          ++p1;
-      while (isspace(*p2))
+      if (*p1 == '<') {
+         while (*p1++ != '>')
+            ;
+      }
+
+      int is_negative = 0;
+      if (*p2 == '-') {
+         is_negative = 1;
+         ++p2;
+      } else if (*p2 == '+')
+         ++p2;
+      while (isdigit(*p2)) {
+         offset_in_bytes *= 10;
+         offset_in_bytes += *p2 - '0';
          ++p2;
-      if (*p1 != *p2) {
-         long long offset_in_bytes;
-         unsigned long long target_address;
-
-         /* Consider the case where the VEX disassembly has ".+integer"
-            or ".-integer" and the objdump disassembly has an
-            address. */
-         if (*p2++ != '.') return 0;
-         if (sscanf(p2, "%lld", &offset_in_bytes) != 1) return 0;
-         if (sscanf(p1, "%llx", &target_address)  != 1) return 0;
-         return address + offset_in_bytes == target_address;
       }
-      ++p1;
-      ++p2;
+      if (is_negative)
+         offset_in_bytes *= -1;
+
+      if (address + offset_in_bytes != target_address) return 0;
    }
 }