From: Julian S. <se...@so...> - 2018-01-03 10:59:26
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=0f18cfc986f800b107c7eee063b8b7c04617e0b8 commit 0f18cfc986f800b107c7eee063b8b7c04617e0b8 Author: Julian Seward <js...@ac...> Date: Wed Jan 3 11:55:44 2018 +0100 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: --- memcheck/tests/vbit-test/binary.c | 17 +++++ memcheck/tests/vbit-test/irops.c | 24 +++--- memcheck/tests/vbit-test/util.c | 16 ++-- memcheck/tests/vbit-test/vbit-test.vgtest | 2 +- memcheck/tests/vbit-test/vbits.c | 118 ++++++++++++++++++++++++++++++ memcheck/tests/vbit-test/vbits.h | 5 ++ memcheck/tests/vbit-test/vtest.h | 7 ++ 7 files changed, 170 insertions(+), 19 deletions(-) diff --git a/memcheck/tests/vbit-test/binary.c b/memcheck/tests/vbit-test/binary.c index 89a7237..22c3ec5 100644 --- 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 242184b..7c5fa14 100644 --- 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 d829d74..d64268d 100644 --- 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 08a8b06..a058905 100644 --- 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 8ba6532..5a5cd66 100644 --- 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 0782e2d..7a332c7 100644 --- 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 540e783..ccb982a 100644 --- 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. * |