[llvm-dev] Conditional jump or move depends on uninitialised value(s)
John Regehr via llvm-dev
llvm-dev at lists.llvm.org
Mon Nov 21 23:01:53 PST 2016
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
>
More information about the llvm-dev
mailing list