[llvm-commits] [llvm] r138636 - /llvm/trunk/lib/Target/ARM/Disassembler/ARMDisassembler.cpp

[Benjamin Kramer]

On Fri, Aug 26, 2011 at 11:37, Jim Grosbach <grosbach at apple.com> wrote:
>
> On Aug 26, 2011, at 11:30 AM, Benjamin Kramer wrote:
>
>> On Fri, Aug 26, 2011 at 11:26, Jim Grosbach <grosbach at apple.com> wrote:
>>> Testcase?
>>
>> It's not directly testable from llvm-mc, but the valgrind buildbot
>> complains about uninitialized variables without this patch.
>
> That's odd. Along what execution path? The MC dissassembler and the Enhanced disassembler both early exit when failure is returned and never reference the Size value. At least according to my reading the code, anyway. What am I missing?

The relevant code in llvm-mc's Disassembler.cpp:62 looks like this:

>  uint64_t Size;
>  uint64_t Index;
>
>  for (Index = 0; Index < Bytes.size(); Index += Size) {
>    MCInst Inst;
>
>    MCDisassembler::DecodeStatus S;
>    S = DisAsm.getInstruction(Inst, Size, memoryObject, Index,
>                              /*REMOVE*/ nulls());
>    switch (S) {
>    case MCDisassembler::Fail:
>      SM.PrintMessage(SMLoc::getFromPointer(Bytes[Index].second),
>                      "invalid instruction encoding", "warning");
>      if (Size == 0)
>        Size = 1; // skip illegible bytes
>      break;

Size is passed by reference to DisAsm.getInstruction. When
getInstruction doesn't set Size in the first iteration then Index
becomes an undefined value in the next iteration. Some of our tests
contain only invalid bytes and valgrind will complain.