<div dir="ltr">On Fri, Jun 28, 2013 at 3:46 PM, Bill Wendling <span dir="ltr"><<a href="mailto:wendling@apple.com" target="_blank">wendling@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
On Jun 28, 2013, at 3:39 PM, Richard Trieu <<a href="mailto:rtrieu@google.com">rtrieu@google.com</a>> wrote:<br>
<br>
><br>
><br>
><br>
> On Fri, Jun 28, 2013 at 3:23 PM, Bill Wendling <<a href="mailto:wendling@apple.com">wendling@apple.com</a>> wrote:<br>
><br>
> On Jun 28, 2013, at 3:20 PM, Richard Trieu <<a href="mailto:rtrieu@google.com">rtrieu@google.com</a>> wrote:<br>
><br>
> > This time, reply to the mailing list as well.<br>
> ><br>
> > On Fri, Jun 28, 2013 at 3:00 PM, Bill Wendling <<a href="mailto:wendling@apple.com">wendling@apple.com</a>> wrote:<br>
> ><br>
> > On Jun 28, 2013, at 2:54 PM, Richard Trieu <<a href="mailto:rtrieu@google.com">rtrieu@google.com</a>> wrote:<br>
> ><br>
> > > Author: rtrieu<br>
> > > Date: Fri Jun 28 16:54:25 2013<br>
> > > New Revision: 185227<br>
> > ><br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=185227&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=185227&view=rev</a><br>
> > > Log:<br>
> > > Fix broken asserts that never fire.<br>
> > ><br>
> > > Change assert("text") to assert(0 && "text").  The first case is a const char *<br>
> > > to bool conversion, which always evaluates to true, never triggering the<br>
> > > assert.  The second case will always trigger the assert.<br>
> > ><br>
> > > Modified:<br>
> > >    llvm/trunk/include/llvm/TableGen/Record.h<br>
> > >    llvm/trunk/lib/Target/Hexagon/InstPrinter/HexagonInstPrinter.cpp<br>
> > ><br>
> > > Modified: llvm/trunk/include/llvm/TableGen/Record.h<br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/TableGen/Record.h?rev=185227&r1=185226&r2=185227&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/TableGen/Record.h?rev=185227&r1=185226&r2=185227&view=diff</a><br>

> > > ==============================================================================<br>
> > > --- llvm/trunk/include/llvm/TableGen/Record.h (original)<br>
> > > +++ llvm/trunk/include/llvm/TableGen/Record.h Fri Jun 28 16:54:25 2013<br>
> > > @@ -1799,9 +1799,9 @@ struct LessRecordRegister {<br>
> > ><br>
> > >       unsigned LHSVal, RHSVal;<br>
> > >       if (LHSPart.second.getAsInteger(10, LHSVal))<br>
> > > -        assert("Unable to convert LHS to integer.");<br>
> > > +        assert(0 && "Unable to convert LHS to integer.");<br>
> > >       if (RHSPart.second.getAsInteger(10, RHSVal))<br>
> > > -        assert("Unable to convert RHS to integer.");<br>
> > > +        assert(0 && "Unable to convert RHS to integer.");<br>
> ><br>
> > Why do it like this?<br>
> ><br>
> >      assert(LHSPart.second.getAsInteger(10, LHSVal) && "Unable to convert LHS to integer.");<br>
> >      assert(RHSPart.second.getAsInteger(10, RHSVal) &&"Unable to convert RHS to integer.");<br>
> ><br>
> > -bw<br>
> ><br>
> > On non-assert builds, getAsInteger wouldn't run, so LHSVal and RHSVal, which is used later, will not get computed.<br>
><br>
><br>
> Then do this. :-)<br>
><br>
>     LHSPart.second.getAsInteger(10, LHSVal);<br>
>     RHSPart.second.getAsInteger(10, LHSVal)<br>
>     assert(LHSVal && "Unable to convert LHS to integer.");<br>
>     assert(RHSVal && "Unable to convert RHS to integer.");<br>
><br>
> -bw<br>
><br>
> 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.<br>

<br>
</div></div>Hmm...if only there were an easy solution to that. ;-)<br>
<span class="HOEnZb"><font color="#888888"><br>
-bw<br>
</font></span><div class="HOEnZb"><div class="h5"><br></div></div></blockquote><div>I'm a bit hesitant to go changing code I'm not that familiar with.  It seems like you are suggesting to zero initialize the variables, then check that they are unchanged.  However, that assumes that getAsInteger will never set the variable to 0.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
> > >       if (LHSVal != RHSVal)<br>
> > >         return LHSVal < RHSVal;<br>
> > >     }<br>
> > ><br>
> > > Modified: llvm/trunk/lib/Target/Hexagon/InstPrinter/HexagonInstPrinter.cpp<br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/InstPrinter/HexagonInstPrinter.cpp?rev=185227&r1=185226&r2=185227&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/InstPrinter/HexagonInstPrinter.cpp?rev=185227&r1=185226&r2=185227&view=diff</a><br>

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