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

[Richard Trieu]

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.

> > >       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
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130628/df8a8a5a/attachment.html>