<div style="font-family: arial, helvetica, sans-serif"><font size="2">I discovered that <a href="http://llvm.org/bugs/show_bug.cgi?id=13168">http://llvm.org/bugs/show_bug.cgi?id=13168</a> was caused by incorrectly deciding that variadic functions without FunctionDecl's were not, in fact, variadic. This has been fixed in the attached patch, which will look nearly identical as the only change was to move a return statement out of one enclosing if-else block. I also tested this patch more extensively, in the hopes of not breaking anything this time :)<div>
<br><div class="gmail_quote">On Wed, Jun 20, 2012 at 6:10 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thanks, committed as r158887.<br>
<div class="HOEnZb"><div class="h5"><br>
On Wed, Jun 20, 2012 at 5:34 PM, Sam Panzer <<a href="mailto:panzer@google.com">panzer@google.com</a>> wrote:<br>
> Hello again,<br>
><br>
> Attached is the newest-and-improved patch.<br>
><br>
> On Wed, Jun 20, 2012 at 4:46 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
> wrote:<br>
>><br>
>> Hi Sam,<br>
>><br>
>> On Wed, Jun 20, 2012 at 1:31 PM, Sam Panzer <<a href="mailto:panzer@google.com">panzer@google.com</a>> wrote:<br>
>> > Here is the next version of this patch. Changes over the last one<br>
>> > include<br>
>> > Richard's suggestions for variadicArgumentPODCheck, some cleanups that<br>
>> > were<br>
>> > available after this adjustment, and more unification of<br>
>> > Check{Constructor,Function,Block,ObjCMethod}Call(), to make sure that<br>
>> > the<br>
>> > warning is issued in a uniform way for various variadic function-like<br>
>> > things.<br>
>><br>
>> I really like this patch. A handful of quick things, then I think this<br>
>> is ready to be checked in:<br>
>><br>
>> > +  void checkSecurityProperties(NamedDecl *FDecl, Expr **Args,<br>
>><br>
>> Since this is also checking PODness of vararg types, perhaps checkCall<br>
>> would be a better name?<br>
><br>
><br>
> Good idea; done.<br>
><br>
>><br>
>><br>
>> > +  StringLiteralCheckType isFormatStringLiteral(const Expr *E,<br>
>><br>
>> Since this actually does non-trivial checking work, perhaps<br>
>> checkFormatStringExpr?<br>
><br>
><br>
> Done.<br>
><br>
>><br>
>><br>
>> > +  VariadicCallType CallType = //VariadicDoesNotApply;<br>
>><br>
>> Remove this comment.<br>
><br>
><br>
> Done.<br>
><br>
>><br>
>><br>
>> > +    StringLiteralCheckType left =<br>
>> > +    StringLiteralCheckType right =<br>
>><br>
>> Left, Right.<br>
><br>
><br>
> Fixed.<br>
><br>
>><br>
>><br>
>> > +  if (isValidVarArgType(Ty) != VAK_Valid) {<br>
>><br>
>> This should be == VAK_Invalid, I think.<br>
><br>
><br>
> Yep. It was actually correct in my local source...<br>
><br>
>><br>
>><br>
>> > +  const FunctionProtoType *Proto = NULL;<br>
>> > +  if ((Proto = dyn_cast<FunctionProtoType>(FuncT))) {<br>
>><br>
>> would be clearer with the dyn_cast in the initialization rather than<br>
>> assigning in the condition.<br>
><br>
><br>
> Done.<br>
</div></div></blockquote></div><br></div></font></div>