regehr via llvm-dev
2016-Nov-22 06:06 UTC
[llvm-dev] Conditional jump or move depends on uninitialised value(s)
Just want to emphasize that on x86-64 and using Valgrind: LLVM compiled with LLVM gets 360 unexpected test fails LLVM compiled with GCC gets 22 unexpected test fails Of course I don't know how many of these are caused by this bitfield speculation issue. John On 11/21/2016 10:48 PM, regehr via llvm-dev wrote:> Alright, here's what seems to be happening... > > The testcase mentioned below builds a MachineOperand that prints like this: > > <BB#2> > > The bottom word of this MachineOperand now looks like this, with > (according to Valgrind) the x's corresponding to uninitialized bits: > > xxxx xxxx xxxx 0000 0000 0000 0000 0100 > > At this point isReg() can be called safely since it looks only at the > lower bits. isDef() cannot be called safely because it looks at bit 25. > However it is clear that the C++ code (below) never calls isDef() when > isReg() returns false, as it does here. > > So now back to the asm: > > 0000000000000000 > <_Z6xfuncxPKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoEPNS_9BitVectorE>: > > 0: b8 ff 00 00 01 mov $0x10000ff,%eax > 5: 23 07 and (%rdi),%eax > 7: 3d 00 00 00 01 cmp $0x1000000,%eax > c: 75 05 jne 13 > <_Z6xfuncxPKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoEPNS_9BitVectorE+0x13> > > e: e9 00 00 00 00 jmpq 13 > <_Z6xfuncxPKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoEPNS_9BitVectorE+0x13> > > 13: 48 89 d6 mov %rdx,%rsi > 16: e9 00 00 00 00 jmpq 1b <.LCPI5_1+0xb> > > It grabs the low word of the MO and uses a mask to grab bit 25 and also > the low 8 bits. Next, it branches on bit 25, which isn't initialized. > The code is clever but -- I think -- wrong. > > GCC does the right thing here, first branching on the low bits and only > then looking at bit 25. > > Sorry if I got anything wrong here! > > John > > > > On 11/21/2016 04:38 PM, regehr via llvm-dev wrote: >> I spent some time digging into a Valgrind report of uninitialized values >> in LLVM r287520 built using itself. (One of quite a few such reports >> that comes up during a "make check".) >> >> I could use another set of eyes on the issue if someone has time. >> >> This command gives me an error: >> >> valgrind -q ./bin/llc < >> /home/regehr/llvm/test/CodeGen/Hexagon/hwloop-dbg.ll -march=hexagon >> -mcpu=hexagonv4 >> >> The error is at this line: >> >> https://github.com/llvm-mirror/llvm/blob/master/lib/CodeGen/DeadMachineInstructionElim.cpp#L142 >> >> >> >> Here I've refactored the code into a minimal (noinline) function that >> still triggers the problem. xfunc2() and xfunc3() are also noinline. >> The problem goes away if either isReg() or isDef() is marked noinline. >> >> void xfuncx(const MachineOperand &MO, >> const TargetRegisterInfo *TRI, >> BitVector &LivePhysRegs) { >> if (MO.isReg() && // <<<<------ problem reported here >> MO.isDef()) { >> xfunc2(MO, TRI, LivePhysRegs); >> } else { >> xfunc3(MO, LivePhysRegs); >> } >> } >> >> The asm is below. Maybe I've been staring too long but I don't see the >> problem Valgrind is talking about. >> >> John >> >> >> .section >> .text._Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE,"ax", at progbits >> >> >> .globl >> _Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE >> >> .p2align 4, 0x90 >> .type >> _Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE, at function >> >> >> _Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE: >> >> # >> @_Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE >> >> >> .Lfunc_begin4: >> .loc 2 126 0 # >> ../lib/CodeGen/DeadMachineInstructionElim.cpp:126:0 >> .cfi_startproc >> # BB#0: # %entry >> #DEBUG_VALUE: xfuncx:MO <- %RDI >> #DEBUG_VALUE: xfuncx:TRI <- %RSI >> #DEBUG_VALUE: xfuncx:LivePhysRegs <- %RDX >> #DEBUG_VALUE: isReg:this <- %RDI >> .loc 2 127 18 prologue_end # >> ../lib/CodeGen/DeadMachineInstructionElim.cpp:127:18 >> movl $16777471, %eax # imm = 0x10000FF >> andl (%rdi), %eax >> .Ltmp128: >> #DEBUG_VALUE: isReg:this <- %RDI >> cmpl $16777216, %eax # imm = 0x1000000 >> jne .LBB4_2 >> # BB#1: # %if.then >> #DEBUG_VALUE: xfuncx:LivePhysRegs <- %RDX >> #DEBUG_VALUE: xfuncx:TRI <- %RSI >> #DEBUG_VALUE: xfuncx:MO <- %RDI >> .Ltmp129: >> .loc 2 129 5 # >> ../lib/CodeGen/DeadMachineInstructionElim.cpp:129:5 >> jmp >> _Z6xfunc2RKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE at PLT >> >> # TAILCALL >> .Ltmp130: >> .LBB4_2: # %if.else >> #DEBUG_VALUE: xfuncx:LivePhysRegs <- %RDX >> #DEBUG_VALUE: xfuncx:TRI <- %RSI >> #DEBUG_VALUE: xfuncx:MO <- %RDI >> .loc 2 131 5 # >> ../lib/CodeGen/DeadMachineInstructionElim.cpp:131:5 >> movq %rdx, %rsi >> jmp _Z6xfunc3RKN4llvm14MachineOperandERNS_9BitVectorE at PLT # >> TAILCALL >> .Ltmp131: >> .Lfunc_end4: >> .size >> _Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE, >> >> .Lfunc_end4-_Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE >> >> >> .cfi_endproc >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Chandler Carruth via llvm-dev
2016-Nov-22 06:50 UTC
[llvm-dev] Conditional jump or move depends on uninitialised value(s)
I think this is a Valgrind false-positive. Reading uninitialized memory is safe provided the result isn't observed. The isReg() test and the isDef() test are OK because whether bit 25 is set or not is irrelevant if the low bits are not zero (the comparison will be false no matter what value bit 25 holds). So there is only an observable effect of examining bit 25 if the low bits are all zero, satisfying the abstract requirement. Valgrind can't know this sadly, so it flags this as a bug. This is something that I would expect MSan to do a better job of by helping the compiler not merge these two tests by showing a MSan check that differentiates them. -Chandler On Mon, Nov 21, 2016 at 10:06 PM regehr via llvm-dev < llvm-dev at lists.llvm.org> wrote:> Just want to emphasize that on x86-64 and using Valgrind: > > LLVM compiled with LLVM gets 360 unexpected test fails > > LLVM compiled with GCC gets 22 unexpected test fails > > Of course I don't know how many of these are caused by this bitfield > speculation issue. > > John > > > On 11/21/2016 10:48 PM, regehr via llvm-dev wrote: > > Alright, here's what seems to be happening... > > > > The testcase mentioned below builds a MachineOperand that prints like > this: > > > > <BB#2> > > > > The bottom word of this MachineOperand now looks like this, with > > (according to Valgrind) the x's corresponding to uninitialized bits: > > > > xxxx xxxx xxxx 0000 0000 0000 0000 0100 > > > > At this point isReg() can be called safely since it looks only at the > > lower bits. isDef() cannot be called safely because it looks at bit 25. > > However it is clear that the C++ code (below) never calls isDef() when > > isReg() returns false, as it does here. > > > > So now back to the asm: > > > > 0000000000000000 > > > <_Z6xfuncxPKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoEPNS_9BitVectorE>: > > > > 0: b8 ff 00 00 01 mov $0x10000ff,%eax > > 5: 23 07 and (%rdi),%eax > > 7: 3d 00 00 00 01 cmp $0x1000000,%eax > > c: 75 05 jne 13 > > > <_Z6xfuncxPKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoEPNS_9BitVectorE+0x13> > > > > e: e9 00 00 00 00 jmpq 13 > > > <_Z6xfuncxPKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoEPNS_9BitVectorE+0x13> > > > > 13: 48 89 d6 mov %rdx,%rsi > > 16: e9 00 00 00 00 jmpq 1b <.LCPI5_1+0xb> > > > > It grabs the low word of the MO and uses a mask to grab bit 25 and also > > the low 8 bits. Next, it branches on bit 25, which isn't initialized. > > The code is clever but -- I think -- wrong. > > > > GCC does the right thing here, first branching on the low bits and only > > then looking at bit 25. > > > > Sorry if I got anything wrong here! > > > > John > > > > > > > > On 11/21/2016 04:38 PM, regehr via llvm-dev wrote: > >> I spent some time digging into a Valgrind report of uninitialized values > >> in LLVM r287520 built using itself. (One of quite a few such reports > >> that comes up during a "make check".) > >> > >> I could use another set of eyes on the issue if someone has time. > >> > >> This command gives me an error: > >> > >> valgrind -q ./bin/llc < > >> /home/regehr/llvm/test/CodeGen/Hexagon/hwloop-dbg.ll -march=hexagon > >> -mcpu=hexagonv4 > >> > >> The error is at this line: > >> > >> > https://github.com/llvm-mirror/llvm/blob/master/lib/CodeGen/DeadMachineInstructionElim.cpp#L142 > >> > >> > >> > >> Here I've refactored the code into a minimal (noinline) function that > >> still triggers the problem. xfunc2() and xfunc3() are also noinline. > >> The problem goes away if either isReg() or isDef() is marked noinline. > >> > >> void xfuncx(const MachineOperand &MO, > >> const TargetRegisterInfo *TRI, > >> BitVector &LivePhysRegs) { > >> if (MO.isReg() && // <<<<------ problem reported here > >> MO.isDef()) { > >> xfunc2(MO, TRI, LivePhysRegs); > >> } else { > >> xfunc3(MO, LivePhysRegs); > >> } > >> } > >> > >> The asm is below. Maybe I've been staring too long but I don't see the > >> problem Valgrind is talking about. > >> > >> John > >> > >> > >> .section > >> > .text._Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE,"ax", at progbits > >> > >> > >> .globl > >> > _Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE > >> > >> .p2align 4, 0x90 > >> .type > >> > _Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE, at function > >> > >> > >> > _Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE: > >> > >> # > >> > @_Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE > >> > >> > >> .Lfunc_begin4: > >> .loc 2 126 0 # > >> ../lib/CodeGen/DeadMachineInstructionElim.cpp:126:0 > >> .cfi_startproc > >> # BB#0: # %entry > >> #DEBUG_VALUE: xfuncx:MO <- %RDI > >> #DEBUG_VALUE: xfuncx:TRI <- %RSI > >> #DEBUG_VALUE: xfuncx:LivePhysRegs <- %RDX > >> #DEBUG_VALUE: isReg:this <- %RDI > >> .loc 2 127 18 prologue_end # > >> ../lib/CodeGen/DeadMachineInstructionElim.cpp:127:18 > >> movl $16777471, %eax # imm = 0x10000FF > >> andl (%rdi), %eax > >> .Ltmp128: > >> #DEBUG_VALUE: isReg:this <- %RDI > >> cmpl $16777216, %eax # imm = 0x1000000 > >> jne .LBB4_2 > >> # BB#1: # %if.then > >> #DEBUG_VALUE: xfuncx:LivePhysRegs <- %RDX > >> #DEBUG_VALUE: xfuncx:TRI <- %RSI > >> #DEBUG_VALUE: xfuncx:MO <- %RDI > >> .Ltmp129: > >> .loc 2 129 5 # > >> ../lib/CodeGen/DeadMachineInstructionElim.cpp:129:5 > >> jmp > >> > _Z6xfunc2RKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE at PLT > >> > >> # TAILCALL > >> .Ltmp130: > >> .LBB4_2: # %if.else > >> #DEBUG_VALUE: xfuncx:LivePhysRegs <- %RDX > >> #DEBUG_VALUE: xfuncx:TRI <- %RSI > >> #DEBUG_VALUE: xfuncx:MO <- %RDI > >> .loc 2 131 5 # > >> ../lib/CodeGen/DeadMachineInstructionElim.cpp:131:5 > >> movq %rdx, %rsi > >> jmp _Z6xfunc3RKN4llvm14MachineOperandERNS_9BitVectorE at PLT # > >> TAILCALL > >> .Ltmp131: > >> .Lfunc_end4: > >> .size > >> > _Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE, > >> > >> > .Lfunc_end4-_Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE > >> > >> > >> .cfi_endproc > >> _______________________________________________ > >> LLVM Developers mailing list > >> llvm-dev at lists.llvm.org > >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161122/20c71bf9/attachment.html>
John Regehr via llvm-dev
2016-Nov-22 07:01 UTC
[llvm-dev] Conditional jump or move depends on uninitialised value(s)
Argh, thanks. It's a bummer to have to throw out Valgrind. Something is messing up Souper when we use it to build LLVM. It could easily be a Souper bug, but in the meantime it makes a good excuse to track down and eradicate UBs in LLVM. I'll go back to UBSan/ASan/MSan. John On 11/21/16 11:50 PM, Chandler Carruth wrote:> I think this is a Valgrind false-positive. > > Reading uninitialized memory is safe provided the result isn't observed. > > The isReg() test and the isDef() test are OK because whether bit 25 is > set or not is irrelevant if the low bits are not zero (the comparison > will be false no matter what value bit 25 holds). So there is only an > observable effect of examining bit 25 if the low bits are all zero, > satisfying the abstract requirement. > > Valgrind can't know this sadly, so it flags this as a bug. This is > something that I would expect MSan to do a better job of by helping the > compiler not merge these two tests by showing a MSan check that > differentiates them. > > -Chandler > > On Mon, Nov 21, 2016 at 10:06 PM regehr via llvm-dev > <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: > > Just want to emphasize that on x86-64 and using Valgrind: > > LLVM compiled with LLVM gets 360 unexpected test fails > > LLVM compiled with GCC gets 22 unexpected test fails > > Of course I don't know how many of these are caused by this bitfield > speculation issue. > > John > > > On 11/21/2016 10:48 PM, regehr via llvm-dev wrote: > > Alright, here's what seems to be happening... > > > > The testcase mentioned below builds a MachineOperand that prints > like this: > > > > <BB#2> > > > > The bottom word of this MachineOperand now looks like this, with > > (according to Valgrind) the x's corresponding to uninitialized bits: > > > > xxxx xxxx xxxx 0000 0000 0000 0000 0100 > > > > At this point isReg() can be called safely since it looks only at the > > lower bits. isDef() cannot be called safely because it looks at > bit 25. > > However it is clear that the C++ code (below) never calls isDef() > when > > isReg() returns false, as it does here. > > > > So now back to the asm: > > > > 0000000000000000 > > > <_Z6xfuncxPKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoEPNS_9BitVectorE>: > > > > 0: b8 ff 00 00 01 mov $0x10000ff,%eax > > 5: 23 07 and (%rdi),%eax > > 7: 3d 00 00 00 01 cmp $0x1000000,%eax > > c: 75 05 jne 13 > > > <_Z6xfuncxPKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoEPNS_9BitVectorE+0x13> > > > > e: e9 00 00 00 00 jmpq 13 > > > <_Z6xfuncxPKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoEPNS_9BitVectorE+0x13> > > > > 13: 48 89 d6 mov %rdx,%rsi > > 16: e9 00 00 00 00 jmpq 1b <.LCPI5_1+0xb> > > > > It grabs the low word of the MO and uses a mask to grab bit 25 and > also > > the low 8 bits. Next, it branches on bit 25, which isn't initialized. > > The code is clever but -- I think -- wrong. > > > > GCC does the right thing here, first branching on the low bits and > only > > then looking at bit 25. > > > > Sorry if I got anything wrong here! > > > > John > > > > > > > > On 11/21/2016 04:38 PM, regehr via llvm-dev wrote: > >> I spent some time digging into a Valgrind report of uninitialized > values > >> in LLVM r287520 built using itself. (One of quite a few such reports > >> that comes up during a "make check".) > >> > >> I could use another set of eyes on the issue if someone has time. > >> > >> This command gives me an error: > >> > >> valgrind -q ./bin/llc < > >> /home/regehr/llvm/test/CodeGen/Hexagon/hwloop-dbg.ll -march=hexagon > >> -mcpu=hexagonv4 > >> > >> The error is at this line: > >> > >> > https://github.com/llvm-mirror/llvm/blob/master/lib/CodeGen/DeadMachineInstructionElim.cpp#L142 > >> > >> > >> > >> Here I've refactored the code into a minimal (noinline) function that > >> still triggers the problem. xfunc2() and xfunc3() are also noinline. > >> The problem goes away if either isReg() or isDef() is marked > noinline. > >> > >> void xfuncx(const MachineOperand &MO, > >> const TargetRegisterInfo *TRI, > >> BitVector &LivePhysRegs) { > >> if (MO.isReg() && // <<<<------ problem reported here > >> MO.isDef()) { > >> xfunc2(MO, TRI, LivePhysRegs); > >> } else { > >> xfunc3(MO, LivePhysRegs); > >> } > >> } > >> > >> The asm is below. Maybe I've been staring too long but I don't > see the > >> problem Valgrind is talking about. > >> > >> John > >> > >> > >> .section > >> > .text._Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE,"ax", at progbits > >> > >> > >> .globl > >> > _Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE > >> > >> .p2align 4, 0x90 > >> .type > >> > _Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE, at function > >> > >> > >> > _Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE: > >> > >> # > >> > @_Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE > >> > >> > >> .Lfunc_begin4: > >> .loc 2 126 0 # > >> ../lib/CodeGen/DeadMachineInstructionElim.cpp:126:0 > >> .cfi_startproc > >> # BB#0: # %entry > >> #DEBUG_VALUE: xfuncx:MO <- %RDI > >> #DEBUG_VALUE: xfuncx:TRI <- %RSI > >> #DEBUG_VALUE: xfuncx:LivePhysRegs <- %RDX > >> #DEBUG_VALUE: isReg:this <- %RDI > >> .loc 2 127 18 prologue_end # > >> ../lib/CodeGen/DeadMachineInstructionElim.cpp:127:18 > >> movl $16777471, %eax # imm = 0x10000FF > >> andl (%rdi), %eax > >> .Ltmp128: > >> #DEBUG_VALUE: isReg:this <- %RDI > >> cmpl $16777216, %eax # imm = 0x1000000 > >> jne .LBB4_2 > >> # BB#1: # %if.then > >> #DEBUG_VALUE: xfuncx:LivePhysRegs <- %RDX > >> #DEBUG_VALUE: xfuncx:TRI <- %RSI > >> #DEBUG_VALUE: xfuncx:MO <- %RDI > >> .Ltmp129: > >> .loc 2 129 5 # > >> ../lib/CodeGen/DeadMachineInstructionElim.cpp:129:5 > >> jmp > >> > _Z6xfunc2RKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE at PLT > >> > >> # TAILCALL > >> .Ltmp130: > >> .LBB4_2: # %if.else > >> #DEBUG_VALUE: xfuncx:LivePhysRegs <- %RDX > >> #DEBUG_VALUE: xfuncx:TRI <- %RSI > >> #DEBUG_VALUE: xfuncx:MO <- %RDI > >> .loc 2 131 5 # > >> ../lib/CodeGen/DeadMachineInstructionElim.cpp:131:5 > >> movq %rdx, %rsi > >> jmp _Z6xfunc3RKN4llvm14MachineOperandERNS_9BitVectorE at PLT # > >> TAILCALL > >> .Ltmp131: > >> .Lfunc_end4: > >> .size > >> > _Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE, > >> > >> > .Lfunc_end4-_Z6xfuncxRKN4llvm14MachineOperandEPKNS_18TargetRegisterInfoERNS_9BitVectorE > >> > >> > >> .cfi_endproc > >> _______________________________________________ > >> LLVM Developers mailing list > >> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >