[llvm] r185227 - Fix broken asserts that never fire.

[Bill Wendling]

On Jun 28, 2013, at 3:39 PM, Richard Trieu <rtrieu at google.com> wrote:

> 
> 
> 
> On Fri, Jun 28, 2013 at 3:23 PM, Bill Wendling <wendling at apple.com> wrote:
> 
> On Jun 28, 2013, at 3:20 PM, Richard Trieu <rtrieu at google.com> wrote:
> 
> > This time, reply to the mailing list as well.
> >
> > On Fri, Jun 28, 2013 at 3:00 PM, Bill Wendling <wendling at apple.com> wrote:
> >
> > On Jun 28, 2013, at 2:54 PM, Richard Trieu <rtrieu at google.com> wrote:
> >
> > > Author: rtrieu
> > > Date: Fri Jun 28 16:54:25 2013
> > > New Revision: 185227
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=185227&view=rev
> > > Log:
> > > Fix broken asserts that never fire.
> > >
> > > Change assert("text") to assert(0 && "text").  The first case is a const char *
> > > to bool conversion, which always evaluates to true, never triggering the
> > > assert.  The second case will always trigger the assert.
> > >
> > > Modified:
> > >    llvm/trunk/include/llvm/TableGen/Record.h
> > >    llvm/trunk/lib/Target/Hexagon/InstPrinter/HexagonInstPrinter.cpp
> > >
> > > Modified: llvm/trunk/include/llvm/TableGen/Record.h
> > > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/TableGen/Record.h?rev=185227&r1=185226&r2=185227&view=diff
> > > ==============================================================================
> > > --- llvm/trunk/include/llvm/TableGen/Record.h (original)
> > > +++ llvm/trunk/include/llvm/TableGen/Record.h Fri Jun 28 16:54:25 2013
> > > @@ -1799,9 +1799,9 @@ struct LessRecordRegister {
> > >
> > >       unsigned LHSVal, RHSVal;
> > >       if (LHSPart.second.getAsInteger(10, LHSVal))
> > > -        assert("Unable to convert LHS to integer.");
> > > +        assert(0 && "Unable to convert LHS to integer.");
> > >       if (RHSPart.second.getAsInteger(10, RHSVal))
> > > -        assert("Unable to convert RHS to integer.");
> > > +        assert(0 && "Unable to convert RHS to integer.");
> >
> > Why do it like this?
> >
> >      assert(LHSPart.second.getAsInteger(10, LHSVal) && "Unable to convert LHS to integer.");
> >      assert(RHSPart.second.getAsInteger(10, RHSVal) &&"Unable to convert RHS to integer.");
> >
> > -bw
> >
> > On non-assert builds, getAsInteger wouldn't run, so LHSVal and RHSVal, which is used later, will not get computed.
> 
> 
> Then do this. :-)
> 
>     LHSPart.second.getAsInteger(10, LHSVal);
>     RHSPart.second.getAsInteger(10, LHSVal)
>     assert(LHSVal && "Unable to convert LHS to integer.");
>     assert(RHSVal && "Unable to convert RHS to integer.");
> 
> -bw
> 
> LHSVal and RHSVal are type unsigned and uninitialized when getAsInteger is called.  When getAsInteger fails, LHSVal and RHSVal will remain uninitialized.  So it is possible that the assert condition will be evaluated on uninitialized values.

Hmm...if only there were an easy solution to that. ;-)

-bw

> > >       if (LHSVal != RHSVal)
> > >         return LHSVal < RHSVal;
> > >     }
> > >
> > > Modified: llvm/trunk/lib/Target/Hexagon/InstPrinter/HexagonInstPrinter.cpp
> > > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/InstPrinter/HexagonInstPrinter.cpp?rev=185227&r1=185226&r2=185227&view=diff
> > > ==============================================================================
> > > --- llvm/trunk/lib/Target/Hexagon/InstPrinter/HexagonInstPrinter.cpp (original)
> > > +++ llvm/trunk/lib/Target/Hexagon/InstPrinter/HexagonInstPrinter.cpp Fri Jun 28 16:54:25 2013
> > > @@ -179,7 +179,7 @@ void HexagonInstPrinter::printBranchOper
> > >                                             raw_ostream &O) const {
> > >   // Branches can take an immediate operand.  This is used by the branch
> > >   // selection pass to print $+8, an eight byte displacement from the PC.
> > > -  assert("Unknown branch operand.");
> > > +  assert(0 && "Unknown branch operand.");
> > > }
> > >
> > > void HexagonInstPrinter::printCallOperand(const MCInst *MI, unsigned OpNo,
> > > @@ -203,7 +203,7 @@ void HexagonInstPrinter::printSymbol(con
> > >     O << '#';
> > >     printOperand(MI, OpNo, O);
> > >   } else {
> > > -    assert("Unknown symbol operand");
> > > +    assert(0 && "Unknown symbol operand");
> > >     printOperand(MI, OpNo, O);
> >
> > The 'printOperand' instruction is now dead.
> >
> > I'd prefer someone who knows this code to make that change.  I'm only here to fix the string literal to bool conversions.
> >
> > >   }
> > >   O << ')';
> > >
> > >
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
> 
>