Created attachment 119496 [details] full disassembly SUMMARY I'm getting a lot of false positives for accessing bitfield. This is the minified code: $ cat test_bitfields1.cpp typedef int BitfieldType; //typedef unsigned BitfieldType; struct A { BitfieldType field : 4; void *buf; }; __attribute__((noinline)) void init(A *a) { a->field=0; a->buf=nullptr; } int main() { A a; init(&a); if (a.field) { if (a.buf) { if (a.field!=1 && a.field!=2) { asm volatile("" ::: "memory"); } a.buf = nullptr; } } return 0; } This is the compile command: <path-to-clang++> -std=c++11 test_bitfields1.cpp -Ofast -g This is the result of running valgrind: $ ~/software/valgrind-3.15.0/bin/valgrind --expensive-definedness-checks=yes ./a.out ==12654== Memcheck, a memory error detector ==12654== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==12654== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info ==12654== Command: ./a.out ==12654== ==12654== Conditional jump or move depends on uninitialised value(s) ==12654== at 0x4005E5: main (test_bitfields1.cpp:19) ==12654== ==12654== ==12654== HEAP SUMMARY: ==12654== in use at exit: 0 bytes in 0 blocks ==12654== total heap usage: 0 allocs, 0 frees, 0 bytes allocated ==12654== ==12654== All heap blocks were freed -- no leaks are possible ==12654== ==12654== Use --track-origins=yes to see where uninitialised values come from ==12654== For lists of detected and suppressed errors, rerun with: -s ==12654== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0) Passing or not --expensive-definedness-checks=yes change anything. This is the disassembly: 00000000004005c0 <init(A*)>: 4005c0: 80 27 f0 andb $0xf0,(%rdi) 4005c3: 48 c7 47 08 00 00 00 movq $0x0,0x8(%rdi) 4005ca: 00 4005cb: c3 retq 4005cc: 0f 1f 40 00 nopl 0x0(%rax) 00000000004005d0 <main>: 4005d0: 48 83 ec 18 sub $0x18,%rsp 4005d4: 48 8d 7c 24 08 lea 0x8(%rsp),%rdi 4005d9: e8 e2 ff ff ff callq 4005c0 <init(A*)> 4005de: 8a 44 24 08 mov 0x8(%rsp),%al 4005e2: c0 e0 04 shl $0x4,%al 4005e5: 74 19 je 400600 <main+0x30> 4005e7: 48 83 7c 24 10 00 cmpq $0x0,0x10(%rsp) 4005ed: 74 11 je 400600 <main+0x30> 4005ef: 3c 10 cmp $0x10,%al 4005f1: 74 04 je 4005f7 <main+0x27> 4005f3: 3c 20 cmp $0x20,%al 4005f5: 74 00 je 4005f7 <main+0x27> 4005f7: 48 c7 44 24 10 00 00 movq $0x0,0x10(%rsp) 4005fe: 00 00 400600: 31 c0 xor %eax,%eax 400602: 48 83 c4 18 add $0x18,%rsp 400606: c3 retq 400607: 66 0f 1f 84 00 00 00 nopw 0x0(%rax,%rax,1) 40060e: 00 00 (see the attached file for the full disassembly) SOFTWARE/OS VERSIONS Linux: CentOS 7 Tested valgrind 3.14 from devtoolset-8 and selfcompiled valgrind 3.15 Clang 7.0.0 build against the GCC 4.8 on the system
"Passing or not --expensive-definedness-checks=yes change anything." should have been "Passing --expensive-definedness-checks=yes doesn't change anything." :(
Sigh. Looks like we need yet another spec rule, for this: 4005e2: c0 e0 04 shl $0x4,%al 4005e5: 74 19 je 400600 <main+0x30> I'll try to hack one up later, but if you want to try now, in the function guest_amd64_spechelper(), find this if (isU64(cc_op, AMD64G_CC_OP_SHRL) && isU64(cond, AMD64CondZ)) { /* SHRL, then Z --> test dep1 == 0 */ return unop(Iop_1Uto64, binop(Iop_CmpEQ32, unop(Iop_64to32, cc_dep1), mkU32(0))); } and add an 8-bit version of it (the above example is for shrl; je/jz).
"for shl ; je/jz", I meant.
This: if (isU64(cc_op, AMD64G_CC_OP_SHRB) && isU64(cond, AMD64CondZ)) { /* SHRL, then Z --> test dep1 == 0 */ return unop(Iop_1Uto64, binop(Iop_CmpEQ8, unop(Iop_64to8, cc_dep1), mkU8(0))); }
Created attachment 119498 [details] patch which fixes it against 3.15 See the patch I've uploaded for the change which worked for me. Thanks for the quick reply.
Julian, this looks correct. Should we land this?