s390x: Fix memcheck false positives with certain comparisons

Consider this structure definition:

  struct s {
    unsigned b : 1;
    unsigned c : 1;
  } x;

Then certain compiler optimizations for a big-endian system may transform
the test

  if (x.b || x.c)
     ...

into a comparison `> 0x3f' of the structure's first byte.  Indeed, the
result of this comparison only depends on the two highest bits of the
byte.  Thus, even if the lower bits are undefined, memcheck shouldn't
complain, but it does.

For certain cases this can be fixed.  Do this by detecting comparisons
like this in the condition code helper for S390_CC_OP_UNSIGNED_COMPARE and
transforming them to a test for the selected bits instead.  Add a small
test to verify the fix.

diff --git a/VEX/priv/guest_s390_helpers.c b/VEX/priv/guest_s390_helpers.c
index df565ffa7fe2dec7b7a9133f3bc6a1745c1352b4..693e96bf6f9afda7315a450a5936dd08c1f04122 100644 (file)
--- a/VEX/priv/guest_s390_helpers.c
+++ b/VEX/priv/guest_s390_helpers.c
@@ -2043,6 +2043,29 @@ guest_s390x_spechelper(const HChar *function_name, IRExpr **args,
             Because cc == 3 cannot occur the rightmost bit of cond is
             a don't care.
          */
+         if (isC64(cc_dep2)) {
+            /* Avoid memcheck false positives for comparisons like `<= 0x1f',
+               `> 0x1f', `< 0x20', or `>= 0x20', where the lower bits don't
+               matter.  Some compiler optimizations yield such comparisons when
+               testing if any (or none) of the upper bits are set. */
+
+            ULong mask = cc_dep2->Iex.Const.con->Ico.U64;
+            ULong c    = cond & (8 + 4 + 2);
+
+            if ((mask & (mask - 1)) == 0) {
+               /* Transform `<  0x20' to `<= 0x1f' and
+                            `>= 0x20' to `>  0x1f' */
+               mask -= 1;
+               c ^= 8;
+            }
+            if (mask != 0 && (mask + 1) != 0 && (mask & (mask + 1)) == 0 &&
+                (c == 8 + 4 || c == 2)) {
+               IROp cmp = c == 8 + 4 ? Iop_CmpEQ64 : Iop_CmpNE64;
+               return unop(Iop_1Uto32,
+                           binop(cmp, binop(Iop_And64, cc_dep1, mkU64(~mask)),
+                                 mkU64(0)));
+            }
+         }
          if (cond == 8 || cond == 8 + 1) {
             return unop(Iop_1Uto32, binop(Iop_CmpEQ64, cc_dep1, cc_dep2));
          }

diff --git a/memcheck/tests/s390x/Makefile.am b/memcheck/tests/s390x/Makefile.am
index 668fd9933cb56183fd6347d38990d3b64169173a..095c07c7f35c6204d818f61d601d85907f4bdac7 100644 (file)
--- a/memcheck/tests/s390x/Makefile.am
+++ b/memcheck/tests/s390x/Makefile.am
@@ -2,7 +2,7 @@ include $(top_srcdir)/Makefile.tool-tests.am
 
 dist_noinst_SCRIPTS = filter_stderr
 
-INSN_TESTS = cdsg cu21 cu42 ltgjhe vstrc vfae vistr vstrs
+INSN_TESTS = cdsg cli cu21 cu42 ltgjhe vstrc vfae vistr vstrs
 
 check_PROGRAMS = $(INSN_TESTS) 
 

diff --git a/memcheck/tests/s390x/cli.c b/memcheck/tests/s390x/cli.c
new file mode 100644 (file)
index 0000000..479ad89
--- /dev/null
+++ b/memcheck/tests/s390x/cli.c
@@ -0,0 +1,41 @@
+#include <stdlib.h>
+
+volatile unsigned long tmp;
+static const char      use_idx[] = "01234567890abcdefghijklmnopqrstuvwxyz";
+
+static void depend_on(int val) { tmp = use_idx[val]; }
+
+static void pretend_write(unsigned long* val) { __asm__("" : "=m"(*val) : :); }
+
+static void do_compares(unsigned long value)
+{
+   unsigned char val1 = value & 0xff;
+   int result = 0;
+
+   __asm__("cli  %[val],0x20\n\t"
+           "jl   1f\n\t"
+           "lghi %[res],1\n\t"
+           "1:   nopr 0"
+           : [res] "+d"(result)
+           : [val] "Q"(val1)
+           :);
+   depend_on(result);
+
+   __asm__("cli  %[val],0x40\n\t"
+           "jnl  1f\n\t"
+           "lghi %[res],2\n\t"
+           "1:   nopr 0"
+           : [res] "+d"(result)
+           : [val] "Q"(val1)
+           :);
+   depend_on(result);
+}
+
+int main()
+{
+   unsigned long* buf = malloc(sizeof(unsigned long));
+   pretend_write(buf);
+   do_compares(*buf & 0x3f);
+   free(buf);
+   return 0;
+}

diff --git a/memcheck/tests/s390x/cli.stderr.exp b/memcheck/tests/s390x/cli.stderr.exp
new file mode 100644 (file)
index 0000000..54a7a4c
--- /dev/null
+++ b/memcheck/tests/s390x/cli.stderr.exp
@@ -0,0 +1,4 @@
+Conditional jump or move depends on uninitialised value(s)
+   at 0x........: do_compares (cli.c:15)
+   by 0x........: main (cli.c:38)
+

diff --git a/memcheck/tests/s390x/cli.stdout.exp b/memcheck/tests/s390x/cli.stdout.exp
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/memcheck/tests/s390x/cli.vgtest b/memcheck/tests/s390x/cli.vgtest
new file mode 100644 (file)
index 0000000..89db99e
--- /dev/null
+++ b/memcheck/tests/s390x/cli.vgtest
@@ -0,0 +1,2 @@
+prog: cli
+vgopts: -q