Fix memcheck/tests/vbit-test (the vbit test program) to track changes in bug 387664.

Bug 387664 changes the default settings for accurate definedness checking
for {Add,Sub}{32,64} and {CmpEQ,CmpNE}{8,16,32,64}.  This fix updates the
vbit tester (memcheck/tests/vbit-test) to test the accurate versions of
these, and thereby fixes a regression caused by
e847cb5429927317023d8410c3c56952aa47fb08 as committed for bug 387664.

diff --git a/memcheck/tests/vbit-test/binary.c b/memcheck/tests/vbit-test/binary.c
index 89a723751471dc69b176ce8702bf9b9718473b7a..22c3ec566d7bb18a2bf0b0e32d5c6b4ef326f1ef 100644 (file)
--- a/memcheck/tests/vbit-test/binary.c
+++ b/memcheck/tests/vbit-test/binary.c
@@ -191,6 +191,23 @@ check_result_for_binary(const irop_t *op, const test_data_t *data)
                                     opnd2->vbits.num_bits);
       break;
 
+   case UNDEF_CMP_EQ_NE:
+      expected_vbits = cmp_eq_ne_vbits(opnd1->vbits, opnd2->vbits,
+                                       opnd1->value, opnd2->value);
+      break;
+
+   case UNDEF_INT_ADD:
+      expected_vbits = int_add_or_sub_vbits(1/*isAdd*/,
+                                            opnd1->vbits, opnd2->vbits,
+                                            opnd1->value, opnd2->value);
+      break;
+
+   case UNDEF_INT_SUB:
+      expected_vbits = int_add_or_sub_vbits(0/*!isAdd*/,
+                                            opnd1->vbits, opnd2->vbits,
+                                            opnd1->value, opnd2->value);
+      break;
+
    case UNDEF_ALL_64x2:
       assert(opnd1->vbits.num_bits == opnd2->vbits.num_bits);
       expected_vbits =

diff --git a/memcheck/tests/vbit-test/irops.c b/memcheck/tests/vbit-test/irops.c
index 242184b4d5ac56f600f139d4efd436f9adc701d7..7c5fa1495179efdf63746c282d5d53fea3c835ed 100644 (file)
--- a/memcheck/tests/vbit-test/irops.c
+++ b/memcheck/tests/vbit-test/irops.c
@@ -38,12 +38,12 @@
 static irop_t irops[] = {
   { DEFOP(Iop_Add8,    UNDEF_LEFT), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
   { DEFOP(Iop_Add16,   UNDEF_LEFT), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
-  { DEFOP(Iop_Add32,   UNDEF_LEFT), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
-  { DEFOP(Iop_Add64,   UNDEF_LEFT), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 = 0, .mips64 = 1 }, // mips asserts
+  { DEFOP(Iop_Add32,   UNDEF_INT_ADD), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
+  { DEFOP(Iop_Add64,   UNDEF_INT_ADD), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 = 0, .mips64 = 1 }, // mips asserts
   { DEFOP(Iop_Sub8,    UNDEF_LEFT), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
   { DEFOP(Iop_Sub16,   UNDEF_LEFT), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 = 1, .mips64 = 1 },
-  { DEFOP(Iop_Sub32,   UNDEF_LEFT), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
-  { DEFOP(Iop_Sub64,   UNDEF_LEFT), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 0, .mips32 = 0, .mips64 = 1 }, // ppc32, mips assert
+  { DEFOP(Iop_Sub32,   UNDEF_INT_SUB), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
+  { DEFOP(Iop_Sub64,   UNDEF_INT_SUB), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 0, .mips32 = 0, .mips64 = 1 }, // ppc32, mips assert
   { DEFOP(Iop_Mul8,    UNDEF_LEFT), .s390x = 0, .amd64 = 0, .x86 = 0, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 = 0, .mips64 = 0 },
   { DEFOP(Iop_Mul16,   UNDEF_LEFT), .s390x = 0, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 = 0, .mips64 = 0 },
   { DEFOP(Iop_Mul32,   UNDEF_LEFT), .s390x = 0, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
@@ -72,14 +72,14 @@ static irop_t irops[] = {
   { DEFOP(Iop_Sar16,   UNDEF_SAR),  .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 = 0, .mips64 = 0 }, // ppc32/64 assert
   { DEFOP(Iop_Sar32,   UNDEF_SAR),  .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
   { DEFOP(Iop_Sar64,   UNDEF_SAR),  .s390x = 1, .amd64 = 1, .x86 = 0, .arm = 1, .ppc64 = 1, .ppc32 = 0, .mips32 = 0, .mips64 = 1 }, // ppc32 asserts
-  { DEFOP(Iop_CmpEQ8,  UNDEF_ALL),  .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 = 0, .mips64 = 0 },
-  { DEFOP(Iop_CmpEQ16, UNDEF_ALL),  .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 = 1, .mips64 = 1 },
-  { DEFOP(Iop_CmpEQ32, UNDEF_ALL),  .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
-  { DEFOP(Iop_CmpEQ64, UNDEF_ALL),  .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 1, .ppc32 = 0, .mips32 = 0, .mips64 = 1 }, // ppc32, mips assert
-  { DEFOP(Iop_CmpNE8,  UNDEF_ALL),  .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 = 0, .mips64 = 0 },
-  { DEFOP(Iop_CmpNE16, UNDEF_ALL),  .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 = 0, .mips64 = 0 },
-  { DEFOP(Iop_CmpNE32, UNDEF_ALL),  .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
-  { DEFOP(Iop_CmpNE64, UNDEF_ALL),  .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 1, .ppc32 = 0, .mips32 = 0, .mips64 = 1 }, // ppc32, mips assert
+  { DEFOP(Iop_CmpEQ8,  UNDEF_CMP_EQ_NE), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 = 0, .mips64 = 0 },
+  { DEFOP(Iop_CmpEQ16, UNDEF_CMP_EQ_NE), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 = 1, .mips64 = 1 },
+  { DEFOP(Iop_CmpEQ32, UNDEF_CMP_EQ_NE), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
+  { DEFOP(Iop_CmpEQ64, UNDEF_CMP_EQ_NE), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 1, .ppc32 = 0, .mips32 = 0, .mips64 = 1 }, // ppc32, mips assert
+  { DEFOP(Iop_CmpNE8,  UNDEF_CMP_EQ_NE), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 = 0, .mips64 = 0 },
+  { DEFOP(Iop_CmpNE16, UNDEF_CMP_EQ_NE), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 0, .ppc32 = 0, .mips32 = 0, .mips64 = 0 },
+  { DEFOP(Iop_CmpNE32, UNDEF_CMP_EQ_NE), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
+  { DEFOP(Iop_CmpNE64, UNDEF_CMP_EQ_NE), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 1, .ppc32 = 0, .mips32 = 0, .mips64 = 1 }, // ppc32, mips assert
   { DEFOP(Iop_Not8,       UNDEF_SAME), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
   { DEFOP(Iop_Not16,      UNDEF_SAME), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 0, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },
   { DEFOP(Iop_Not32,      UNDEF_SAME), .s390x = 1, .amd64 = 1, .x86 = 1, .arm = 1, .ppc64 = 1, .ppc32 = 1, .mips32 = 1, .mips64 = 1 },

diff --git a/memcheck/tests/vbit-test/util.c b/memcheck/tests/vbit-test/util.c
index d829d74fcc5df98a3360e430c31beadd278c4c46..d64268df510120e71732dd1ad1f89655d9a892a2 100644 (file)
--- a/memcheck/tests/vbit-test/util.c
+++ b/memcheck/tests/vbit-test/util.c
@@ -45,6 +45,8 @@
 #include <inttypes.h>
 #include "vtest.h"
 
+#include "memcheck.h" // VALGRIND_MAKE_MEM_DEFINED
+
 
 /* Something bad happened. Cannot continue. */
 void __attribute__((noreturn))
@@ -120,12 +122,14 @@ print_opnd(FILE *fp, const opnd_t *opnd)
 {
    fprintf(fp, "vbits = ");
    print_vbits(fp, opnd->vbits);
-   /* Write the value only if it is defined. Otherwise, there will be error
-      messages about it being undefined */
-   if (equal_vbits(opnd->vbits, defined_vbits(opnd->vbits.num_bits))) {
-      fprintf(fp, "   value = ");
-      print_value(fp, opnd->value, opnd->vbits.num_bits);
-   }
+   /* The value itself might be partially or fully undefined, so take a
+      copy, paint the copy as defined, and print the copied value.  This is
+      so as to avoid error messages from Memcheck, which are correct, but
+      confusing. */
+   volatile value_t value_copy = opnd->value;
+   VALGRIND_MAKE_MEM_DEFINED(&value_copy, sizeof(value_copy));
+   fprintf(fp, "   value = ");
+   print_value(fp, value_copy, opnd->vbits.num_bits);
 }
 
 

diff --git a/memcheck/tests/vbit-test/vbit-test.vgtest b/memcheck/tests/vbit-test/vbit-test.vgtest
index 08a8b06eabc8fb16a42cb4fb6078832870c6019a..a05890566b19589aa464b2d32de74ff7a59e8a0c 100644 (file)
--- a/memcheck/tests/vbit-test/vbit-test.vgtest
+++ b/memcheck/tests/vbit-test/vbit-test.vgtest
@@ -1,2 +1,2 @@
 prog: vbit-test
-vgopts: -q
+vgopts: -q --expensive-definedness-checks=yes

diff --git a/memcheck/tests/vbit-test/vbits.c b/memcheck/tests/vbit-test/vbits.c
index 8ba6532cc769c983b5be83b6de7324ebbd0afd5d..5a5cd663d6b5827bd35b3deb849207e8bd6b1533 100644 (file)
--- a/memcheck/tests/vbit-test/vbits.c
+++ b/memcheck/tests/vbit-test/vbits.c
@@ -45,6 +45,8 @@
 #include "vbits.h"
 #include "vtest.h"
 
+#include "memcheck.h"  // VALGRIND_MAKE_MEM_DEFINED
+
 
 /* Return the bits of V if they fit into 64-bit. If V has fewer than
    64 bits, the bit pattern is zero-extended to the left. */
@@ -1072,3 +1074,119 @@ cmpord_vbits(unsigned v1_num_bits, unsigned v2_num_bits)
 
    return new;
 }
+
+
+/* Deal with precise integer EQ and NE.  Needs some helpers.  The helpers
+   compute the result for 64-bit inputs, but can also be used for the
+   32/16/8 bit cases, because we can zero extend both the vbits and values
+   out to 64 bits and still get the correct result. */
+
+
+/* Get both vbits and values for a binary operation, that has args of the
+   same size (type?), namely 8, 16, 32 or 64 bit.  Unused bits are set to
+   zero in both vbit_ and val_ cases. */
+static
+void get_binary_vbits_and_vals64 ( /*OUT*/uint64_t* varg1,
+                                   /*OUT*/uint64_t* arg1,
+                                   /*OUT*/uint64_t* varg2,
+                                   /*OUT*/uint64_t* arg2,
+                                   vbits_t vbits1, vbits_t vbits2,
+                                   value_t val1, value_t val2)
+{
+   assert(vbits1.num_bits == vbits2.num_bits);
+
+   *varg1 = *arg1 = *varg2 = *arg2 = 0;
+
+   switch (vbits1.num_bits) {
+      case 8:  *arg1 = (uint64_t)val1.u8;  *arg2 = (uint64_t)val2.u8;  break;
+      case 16: *arg1 = (uint64_t)val1.u16; *arg2 = (uint64_t)val2.u16; break;
+      case 32: *arg1 = (uint64_t)val1.u32; *arg2 = (uint64_t)val2.u32; break;
+      case 64: *arg1 = val1.u64;           *arg2 = val2.u64;           break;
+      default: panic(__func__);
+   }
+
+   *varg1 = get_bits64(vbits1);
+   *varg2 = get_bits64(vbits2);
+}
+
+static uint64_t uifu64 ( uint64_t x, uint64_t y ) { return x | y; }
+
+/* Returns 0 (defined) or 1 (undefined). */
+static uint32_t ref_CmpEQ64_with_vbits ( uint64_t arg1, uint64_t varg1,
+                                         uint64_t arg2, uint64_t varg2 )
+{
+   uint64_t naive = uifu64(varg1, varg2);
+   if (naive == 0) {
+      return 0; /* defined */
+   }
+
+   // Mark the two actual arguments as fully defined, else Memcheck will
+   // complain about undefinedness in them, which is correct but confusing
+   // (and pollutes the output of this test program.)
+   VALGRIND_MAKE_MEM_DEFINED(&arg1, sizeof(arg1));
+   VALGRIND_MAKE_MEM_DEFINED(&arg2, sizeof(arg2));
+
+   // if any bit in naive is 1, then the result is undefined.  Except,
+   // if we can find two corresponding bits in arg1 and arg2 such that they
+   // are different but both defined, then the overall result is defined
+   // (because the two bits guarantee that the bit vectors arg1 and arg2
+   // are different.)
+   UInt i;
+   for (i = 0; i < 64; i++) {
+      if ((arg1 & 1) != (arg2 & 1) && (varg1 & 1) == 0 && (varg2 & 1) == 0) {
+         return 0; /* defined */
+      }
+      arg1 >>= 1; arg2 >>= 1; varg1 >>= 1; varg2 >>= 1;
+   }
+
+   return 1; /* undefined */
+}
+
+
+vbits_t
+cmp_eq_ne_vbits(vbits_t vbits1, vbits_t vbits2, value_t val1, value_t val2)
+{
+   uint64_t varg1 = 0, arg1 = 0, varg2 = 0, arg2 = 0;
+   get_binary_vbits_and_vals64(&varg1, &arg1, &varg2, &arg2,
+                               vbits1, vbits2, val1, val2);
+
+   vbits_t res = { .num_bits = 1 };
+   res.bits.u32 = ref_CmpEQ64_with_vbits(arg1, varg1, arg2, varg2);
+
+   return res;
+}
+
+
+/* Deal with precise integer ADD and SUB. */
+vbits_t
+int_add_or_sub_vbits(int isAdd,
+                     vbits_t vbits1, vbits_t vbits2, value_t val1, value_t val2)
+{
+   uint64_t vaa = 0, aa = 0, vbb = 0, bb = 0;
+   get_binary_vbits_and_vals64(&vaa, &aa, &vbb, &bb,
+                               vbits1, vbits2, val1, val2);
+
+   // This is derived from expensiveAddSub() in mc_translate.c.
+   uint64_t a_min = aa & ~vaa;
+   uint64_t b_min = bb & ~vbb;
+   uint64_t a_max = aa | vaa;
+   uint64_t b_max = bb | vbb;
+
+   uint64_t result;
+   if (isAdd) {
+      result = (vaa | vbb) | ((a_min + b_min) ^ (a_max + b_max));
+   } else {
+      result = (vaa | vbb) | ((a_min - b_max) ^ (a_max - b_min));
+   }
+
+   vbits_t res = { .num_bits = vbits1.num_bits };
+   switch (res.num_bits) {
+      case 8:  res.bits.u8  = (uint8_t)result;  break;
+      case 16: res.bits.u16 = (uint16_t)result; break;
+      case 32: res.bits.u32 = (uint32_t)result; break;
+      case 64: res.bits.u64 = (uint64_t)result; break;
+      default: panic(__func__);
+   }
+
+   return res;
+}

diff --git a/memcheck/tests/vbit-test/vbits.h b/memcheck/tests/vbit-test/vbits.h
index 0782e2d3b60b4d45fa11d427f3e0d8c5d33c0fa1..7a332c7e1d49c018b555db8b63ddda688ae6ff9d 100644 (file)
--- a/memcheck/tests/vbit-test/vbits.h
+++ b/memcheck/tests/vbit-test/vbits.h
@@ -92,5 +92,10 @@ vbits_t shr_vbits(vbits_t, unsigned amount);
 vbits_t sar_vbits(vbits_t, unsigned amount);
 int     completely_defined_vbits(vbits_t);
 vbits_t cmpord_vbits(unsigned v1_num_bits, unsigned v2_num_bits);
+vbits_t cmp_eq_ne_vbits(vbits_t vbits1, vbits_t vbits2,
+                        value_t val1, value_t val2);
+vbits_t int_add_or_sub_vbits(int isAdd,
+                             vbits_t vbits1, vbits_t vbits2,
+                             value_t val1, value_t val2);
 
 #endif // VBITS_H

diff --git a/memcheck/tests/vbit-test/vtest.h b/memcheck/tests/vbit-test/vtest.h
index 540e783be0de8a3c5884f80c5e4d70962b87c37a..ccb982ab2af54a11555aad8783c69e2795c2b6e5 100644 (file)
--- a/memcheck/tests/vbit-test/vtest.h
+++ b/memcheck/tests/vbit-test/vtest.h
@@ -79,6 +79,13 @@ typedef enum {
 
    UNDEF_ORD,     // Iop_CmpORD compare 
 
+   // Expensive (exact) integer EQ and NE
+   UNDEF_CMP_EQ_NE,
+
+   // Expensive (exact) integer addition and subtraction
+   UNDEF_INT_ADD,
+   UNDEF_INT_SUB,
+
    /* For each of the following UNDEF_ALL_BxE, E is the number of
     * elements and B is the number of bits in the element.
     *