[cfe-commits] r148324 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp lib/Sema/SemaExprObjC.cpp test/SemaObjC/format-strings-objc.m

Nico Weber thakis at chromium.org
Mon Jan 30 08:37:41 PST 2012


Cool!

One thing that this warned on in the chromium source was:

../../webkit/plugins/npapi/plugin_web_event_converter_mac.mm:213:37:
error: format specifies type 'wchar_t *' (aka 'wchar_t *') but the
argument has type 'const WebUChar *' (aka 'const unsigned short *')
[-Werror,-Wformat]
      [NSString stringWithFormat:@"%S", key_event.text]);
                                   ~^   ~~~~~~~~~~~~~~

It looks like the current %S warning checks if the parameter type is
wchar_t. http://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html#//apple_ref/doc/uid/TP40004265
says that %S is always utf16 (or ucs16, it doesn't say). `man 3
printf` says %S is for wchar_t, but I guess it refers to the wchar_t
type that was effective when libc / AppKit was built. On my OS X 10.6
box, it looks like printf expects a four byte wchar_t, while AppKit
expects a two byte wchar_t. Since that code snippet above is about
AppKit, the code is right and the warning is wrong (chromium isn't
compiled with -fshort-wchar, but AppKit was, and the warning really
needs to check against the wchar_t type that was used to build the
library it's talking to).

Since %S can expect utf32 (lib) or utf16 (AppKit), the %S warning is
currently fairly useless. %S seems to be an Apple extension, so should
we just hardcode %S to look for a 32bit type in c strings and for a
16bit type in @"" strings? Should we disable the warning for %S
altogether?

Nico

On Tue, Jan 17, 2012 at 3:25 PM, Jean-Daniel Dupas
<devlists at shadowlab.org> wrote:
>
> Le 17 janv. 2012 à 21:38, Eli Friedman a écrit :
>
>> On Tue, Jan 17, 2012 at 12:03 PM, Jean-Daniel Dupas
>> <devlists at shadowlab.org> wrote:
>>> Author: jddupas
>>> Date: Tue Jan 17 14:03:31 2012
>>> New Revision: 148324
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=148324&view=rev
>>> Log:
>>> Fix a couples of issues in format strings checking.
>>> PR 10274: format function attribute with the NSString archetype yields no compiler warnings
>>> PR 10275: format function attribute isn't checked in Objective-C methods
>>>
>>>
>>> Modified:
>>>    cfe/trunk/include/clang/Sema/Sema.h
>>>    cfe/trunk/lib/Sema/SemaChecking.cpp
>>>    cfe/trunk/lib/Sema/SemaExprObjC.cpp
>>>    cfe/trunk/test/SemaObjC/format-strings-objc.m
>>>
>>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=148324&r1=148323&r2=148324&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>>> +++ cfe/trunk/include/clang/Sema/Sema.h Tue Jan 17 14:03:31 2012
>>> @@ -6281,9 +6281,12 @@
>>>                         bool AllowOnePastEnd=true, bool IndexNegated=false);
>>>   void CheckArrayAccess(const Expr *E);
>>>   bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall);
>>> +  bool CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation loc,
>>> +                           Expr **Args, unsigned NumArgs);
>>>   bool CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall);
>>>
>>> -  bool CheckablePrintfAttr(const FormatAttr *Format, CallExpr *TheCall);
>>> +  bool CheckablePrintfAttr(const FormatAttr *Format, Expr **Args,
>>> +                           unsigned NumArgs, bool IsCXXMemberCall);
>>>   bool CheckObjCString(Expr *Arg);
>>>
>>>   ExprResult CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
>>> @@ -6307,13 +6310,13 @@
>>>   bool SemaBuiltinConstantArg(CallExpr *TheCall, int ArgNum,
>>>                               llvm::APSInt &Result);
>>>
>>> -  bool SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall,
>>> +  bool SemaCheckStringLiteral(const Expr *E, Expr **Args, unsigned NumArgs,
>>>                               bool HasVAListArg, unsigned format_idx,
>>>                               unsigned firstDataArg, bool isPrintf,
>>>                               bool inFunctionCall = true);
>>>
>>>   void CheckFormatString(const StringLiteral *FExpr, const Expr *OrigFormatExpr,
>>> -                         const CallExpr *TheCall, bool HasVAListArg,
>>> +                         Expr **Args, unsigned NumArgs, bool HasVAListArg,
>>>                          unsigned format_idx, unsigned firstDataArg,
>>>                          bool isPrintf, bool inFunctionCall);
>>>
>>> @@ -6321,9 +6324,14 @@
>>>                              const Expr * const *ExprArgs,
>>>                              SourceLocation CallSiteLoc);
>>>
>>> -  void CheckPrintfScanfArguments(const CallExpr *TheCall, bool HasVAListArg,
>>> -                                 unsigned format_idx, unsigned firstDataArg,
>>> -                                 bool isPrintf);
>>> +  void CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall);
>>> +  void CheckFormatArguments(const FormatAttr *Format, Expr **Args,
>>> +                            unsigned NumArgs, bool IsCXXMember,
>>> +                            SourceLocation Loc, SourceRange Range);
>>> +  void CheckPrintfScanfArguments(Expr **Args, unsigned NumArgs,
>>> +                                 bool HasVAListArg, unsigned format_idx,
>>> +                                 unsigned firstDataArg, bool isPrintf,
>>> +                                 SourceLocation Loc, SourceRange range);
>>>
>>>   void CheckMemaccessArguments(const CallExpr *Call,
>>>                                unsigned BId,
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=148324&r1=148323&r2=148324&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Jan 17 14:03:31 2012
>>> @@ -44,23 +44,24 @@
>>>   return SL->getLocationOfByte(ByteNo, PP.getSourceManager(),
>>>                                PP.getLangOptions(), PP.getTargetInfo());
>>>  }
>>> -
>>>
>>> -/// CheckablePrintfAttr - does a function call have a "printf" attribute
>>> -/// and arguments that merit checking?
>>> -bool Sema::CheckablePrintfAttr(const FormatAttr *Format, CallExpr *TheCall) {
>>> -  if (Format->getType() == "printf") return true;
>>> -  if (Format->getType() == "printf0") {
>>> +bool Sema::CheckablePrintfAttr(const FormatAttr *Format, Expr **Args,
>>> +                               unsigned NumArgs, bool IsCXXMemberCall) {
>>> +  StringRef Type = Format->getType();
>>> +  // FIXME: add support for "CFString" Type. They are not string literal though,
>>> +  // so they need special handling.
>>> +  if (Type == "printf" || Type == "NSString") return true;
>>> +  if (Type == "printf0") {
>>>     // printf0 allows null "format" string; if so don't check format/args
>>>     unsigned format_idx = Format->getFormatIdx() - 1;
>>>     // Does the index refer to the implicit object argument?
>>> -    if (isa<CXXMemberCallExpr>(TheCall)) {
>>> +    if (IsCXXMemberCall) {
>>>       if (format_idx == 0)
>>>         return false;
>>>       --format_idx;
>>>     }
>>> -    if (format_idx < TheCall->getNumArgs()) {
>>> -      Expr *Format = TheCall->getArg(format_idx)->IgnoreParenCasts();
>>> +    if (format_idx < NumArgs) {
>>> +      Expr *Format = Args[format_idx]->IgnoreParenCasts();
>>>       if (!Format->isNullPointerConstant(Context,
>>>                                          Expr::NPC_ValueDependentIsNull))
>>>         return true;
>>> @@ -463,16 +464,7 @@
>>>   for (specific_attr_iterator<FormatAttr>
>>>          i = FDecl->specific_attr_begin<FormatAttr>(),
>>>          e = FDecl->specific_attr_end<FormatAttr>(); i != e ; ++i) {
>>> -
>>> -    const FormatAttr *Format = *i;
>>> -    const bool b = Format->getType() == "scanf";
>>> -    if (b || CheckablePrintfAttr(Format, TheCall)) {
>>> -      bool HasVAListArg = Format->getFirstArg() == 0;
>>> -      CheckPrintfScanfArguments(TheCall, HasVAListArg,
>>> -                                Format->getFormatIdx() - 1,
>>> -                                HasVAListArg ? 0 : Format->getFirstArg() - 1,
>>> -                                !b);
>>> -    }
>>> +    CheckFormatArguments(*i, TheCall);
>>>   }
>>>
>>>   for (specific_attr_iterator<NonNullAttr>
>>> @@ -495,6 +487,26 @@
>>>   return false;
>>>  }
>>>
>>> +bool Sema::CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation lbrac,
>>> +                               Expr **Args, unsigned NumArgs) {
>>> +  for (specific_attr_iterator<FormatAttr>
>>> +       i = Method->specific_attr_begin<FormatAttr>(),
>>> +       e = Method->specific_attr_end<FormatAttr>(); i != e ; ++i) {
>>> +
>>> +    CheckFormatArguments(*i, Args, NumArgs, false, lbrac,
>>> +                         Method->getSourceRange());
>>> +  }
>>> +
>>> +  // diagnose nonnull arguments.
>>> +  for (specific_attr_iterator<NonNullAttr>
>>> +       i = Method->specific_attr_begin<NonNullAttr>(),
>>> +       e = Method->specific_attr_end<NonNullAttr>(); i != e; ++i) {
>>> +    CheckNonNullArguments(*i, Args, lbrac);
>>> +  }
>>> +
>>> +  return false;
>>> +}
>>> +
>>>  bool Sema::CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall) {
>>>   // Printf checking.
>>>   const FormatAttr *Format = NDecl->getAttr<FormatAttr>();
>>> @@ -509,13 +521,7 @@
>>>   if (!Ty->isBlockPointerType())
>>>     return false;
>>>
>>> -  const bool b = Format->getType() == "scanf";
>>> -  if (!b && !CheckablePrintfAttr(Format, TheCall))
>>> -    return false;
>>> -
>>> -  bool HasVAListArg = Format->getFirstArg() == 0;
>>> -  CheckPrintfScanfArguments(TheCall, HasVAListArg, Format->getFormatIdx() - 1,
>>> -                            HasVAListArg ? 0 : Format->getFirstArg() - 1, !b);
>>> +  CheckFormatArguments(Format, TheCall);
>>>
>>>   return false;
>>>  }
>>> @@ -1368,8 +1374,8 @@
>>>  }
>>>
>>>  // Handle i > 1 ? "x" : "y", recursively.
>>> -bool Sema::SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall,
>>> -                                  bool HasVAListArg,
>>> +bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args,
>>> +                                  unsigned NumArgs, bool HasVAListArg,
>>>                                   unsigned format_idx, unsigned firstDataArg,
>>>                                   bool isPrintf, bool inFunctionCall) {
>>>  tryAgain:
>>> @@ -1382,10 +1388,10 @@
>>>   case Stmt::BinaryConditionalOperatorClass:
>>>   case Stmt::ConditionalOperatorClass: {
>>>     const AbstractConditionalOperator *C = cast<AbstractConditionalOperator>(E);
>>> -    return SemaCheckStringLiteral(C->getTrueExpr(), TheCall, HasVAListArg,
>>> +    return SemaCheckStringLiteral(C->getTrueExpr(), Args, NumArgs, HasVAListArg,
>>>                                   format_idx, firstDataArg, isPrintf,
>>>                                   inFunctionCall)
>>> -        && SemaCheckStringLiteral(C->getFalseExpr(), TheCall, HasVAListArg,
>>> +        && SemaCheckStringLiteral(C->getFalseExpr(), Args, NumArgs, HasVAListArg,
>>>                                   format_idx, firstDataArg, isPrintf,
>>>                                   inFunctionCall);
>>>   }
>>> @@ -1433,7 +1439,7 @@
>>>
>>>       if (isConstant) {
>>>         if (const Expr *Init = VD->getAnyInitializer())
>>> -          return SemaCheckStringLiteral(Init, TheCall,
>>> +          return SemaCheckStringLiteral(Init, Args, NumArgs,
>>>                                         HasVAListArg, format_idx, firstDataArg,
>>>                                         isPrintf, /*inFunctionCall*/false);
>>>       }
>>> @@ -1474,7 +1480,7 @@
>>>             unsigned ArgIndex = FA->getFormatIdx();
>>>             const Expr *Arg = CE->getArg(ArgIndex - 1);
>>>
>>> -            return SemaCheckStringLiteral(Arg, TheCall, HasVAListArg,
>>> +            return SemaCheckStringLiteral(Arg, Args, NumArgs, HasVAListArg,
>>>                                           format_idx, firstDataArg, isPrintf,
>>>                                           inFunctionCall);
>>>           }
>>> @@ -1494,7 +1500,7 @@
>>>       StrE = cast<StringLiteral>(E);
>>>
>>>     if (StrE) {
>>> -      CheckFormatString(StrE, E, TheCall, HasVAListArg, format_idx,
>>> +      CheckFormatString(StrE, E, Args, NumArgs, HasVAListArg, format_idx,
>>>                         firstDataArg, isPrintf, inFunctionCall);
>>>       return true;
>>>     }
>>> @@ -1523,37 +1529,52 @@
>>>
>>>  /// CheckPrintfScanfArguments - Check calls to printf and scanf (and similar
>>>  /// functions) for correct use of format strings.
>>> -void
>>> -Sema::CheckPrintfScanfArguments(const CallExpr *TheCall, bool HasVAListArg,
>>> -                                unsigned format_idx, unsigned firstDataArg,
>>> -                                bool isPrintf) {
>>> -
>>> -  const Expr *Fn = TheCall->getCallee();
>>> -
>>> +void Sema::CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall) {
>>> +  bool IsCXXMember = false;
>>>   // The way the format attribute works in GCC, the implicit this argument
>>>   // of member functions is counted. However, it doesn't appear in our own
>>>   // lists, so decrement format_idx in that case.
>>>   if (isa<CXXMemberCallExpr>(TheCall)) {
>>> -    const CXXMethodDecl *method_decl =
>>> -      dyn_cast<CXXMethodDecl>(TheCall->getCalleeDecl());
>>> -    if (method_decl && method_decl->isInstance()) {
>>> -      // Catch a format attribute mistakenly referring to the object argument.
>>> +    const CXXMethodDecl *method_decl =
>>> +    dyn_cast<CXXMethodDecl>(TheCall->getCalleeDecl());
>>> +    IsCXXMember = method_decl && method_decl->isInstance();
>>> +  }
>>> +  CheckFormatArguments(Format, TheCall->getArgs(), TheCall->getNumArgs(),
>>> +                       IsCXXMember, TheCall->getRParenLoc(),
>>> +                       TheCall->getCallee()->getSourceRange());
>>> +}
>>> +
>>> +void Sema::CheckFormatArguments(const FormatAttr *Format, Expr **Args,
>>> +                                unsigned NumArgs, bool IsCXXMember,
>>> +                                SourceLocation Loc, SourceRange Range) {
>>> +  const bool b = Format->getType() == "scanf";
>>> +  if (b || CheckablePrintfAttr(Format, Args, NumArgs, IsCXXMember)) {
>>> +    bool HasVAListArg = Format->getFirstArg() == 0;
>>> +    unsigned format_idx = Format->getFormatIdx() - 1;
>>> +    unsigned firstDataArg = HasVAListArg ? 0 : Format->getFirstArg() - 1;
>>> +    if (IsCXXMember) {
>>>       if (format_idx == 0)
>>>         return;
>>>       --format_idx;
>>>       if(firstDataArg != 0)
>>>         --firstDataArg;
>>>     }
>>> +    CheckPrintfScanfArguments(Args, NumArgs, HasVAListArg, format_idx,
>>> +                              firstDataArg, !b, Loc, Range);
>>>   }
>>> +}
>>>
>>> +void Sema::CheckPrintfScanfArguments(Expr **Args, unsigned NumArgs,
>>> +                                     bool HasVAListArg, unsigned format_idx,
>>> +                                     unsigned firstDataArg, bool isPrintf,
>>> +                                     SourceLocation Loc, SourceRange Range) {
>>>   // CHECK: printf/scanf-like function is called with no format string.
>>> -  if (format_idx >= TheCall->getNumArgs()) {
>>> -    Diag(TheCall->getRParenLoc(), diag::warn_missing_format_string)
>>> -      << Fn->getSourceRange();
>>> +  if (format_idx >= NumArgs) {
>>> +    Diag(Loc, diag::warn_missing_format_string) << Range;
>>>     return;
>>>   }
>>>
>>> -  const Expr *OrigFormatExpr = TheCall->getArg(format_idx)->IgnoreParenCasts();
>>> +  const Expr *OrigFormatExpr = Args[format_idx]->IgnoreParenCasts();
>>>
>>>   // CHECK: format string is not a string literal.
>>>   //
>>> @@ -1567,18 +1588,18 @@
>>>   // C string (e.g. "%d")
>>>   // ObjC string uses the same format specifiers as C string, so we can use
>>>   // the same format string checking logic for both ObjC and C strings.
>>> -  if (SemaCheckStringLiteral(OrigFormatExpr, TheCall, HasVAListArg, format_idx,
>>> -                             firstDataArg, isPrintf))
>>> +  if (SemaCheckStringLiteral(OrigFormatExpr, Args, NumArgs, HasVAListArg,
>>> +                             format_idx, firstDataArg, isPrintf))
>>>     return;  // Literal format string found, check done!
>>>
>>>   // If there are no arguments specified, warn with -Wformat-security, otherwise
>>>   // warn only with -Wformat-nonliteral.
>>> -  if (TheCall->getNumArgs() == format_idx+1)
>>> -    Diag(TheCall->getArg(format_idx)->getLocStart(),
>>> +  if (NumArgs == format_idx+1)
>>> +    Diag(Args[format_idx]->getLocStart(),
>>>          diag::warn_format_nonliteral_noargs)
>>>       << OrigFormatExpr->getSourceRange();
>>>   else
>>> -    Diag(TheCall->getArg(format_idx)->getLocStart(),
>>> +    Diag(Args[format_idx]->getLocStart(),
>>>          diag::warn_format_nonliteral)
>>>            << OrigFormatExpr->getSourceRange();
>>>  }
>>> @@ -1594,7 +1615,8 @@
>>>   const bool IsObjCLiteral;
>>>   const char *Beg; // Start of format string.
>>>   const bool HasVAListArg;
>>> -  const CallExpr *TheCall;
>>> +  const Expr * const *Args;
>>> +  const unsigned NumArgs;
>>>   unsigned FormatIdx;
>>>   llvm::BitVector CoveredArgs;
>>>   bool usesPositionalArgs;
>>> @@ -1605,14 +1627,14 @@
>>>                      const Expr *origFormatExpr, unsigned firstDataArg,
>>>                      unsigned numDataArgs, bool isObjCLiteral,
>>>                      const char *beg, bool hasVAListArg,
>>> -                     const CallExpr *theCall, unsigned formatIdx,
>>> -                     bool inFunctionCall)
>>> +                     Expr **args, unsigned numArgs,
>>> +                     unsigned formatIdx, bool inFunctionCall)
>>>     : S(s), FExpr(fexpr), OrigFormatExpr(origFormatExpr),
>>>       FirstDataArg(firstDataArg),
>>>       NumDataArgs(numDataArgs),
>>>       IsObjCLiteral(isObjCLiteral), Beg(beg),
>>>       HasVAListArg(hasVAListArg),
>>> -      TheCall(theCall), FormatIdx(formatIdx),
>>> +      Args(args), NumArgs(numArgs), FormatIdx(formatIdx),
>>>       usesPositionalArgs(false), atFirstArg(true),
>>>       inFunctionCall(inFunctionCall) {
>>>         CoveredArgs.resize(numDataArgs);
>>> @@ -1727,7 +1749,7 @@
>>>  }
>>>
>>>  const Expr *CheckFormatHandler::getDataArg(unsigned i) const {
>>> -  return TheCall->getArg(FirstDataArg + i);
>>> +  return Args[FirstDataArg + i];
>>>  }
>>>
>>>  void CheckFormatHandler::DoneProcessing() {
>>> @@ -1811,7 +1833,7 @@
>>>                                               bool IsStringLocation,
>>>                                               Range StringRange,
>>>                                               FixItHint FixIt) {
>>> -  EmitFormatDiagnostic(S, inFunctionCall, TheCall->getArg(FormatIdx), PDiag,
>>> +  EmitFormatDiagnostic(S, inFunctionCall, Args[FormatIdx], PDiag,
>>>                        Loc, IsStringLocation, StringRange, FixIt);
>>>  }
>>>
>>> @@ -1870,11 +1892,11 @@
>>>                      const Expr *origFormatExpr, unsigned firstDataArg,
>>>                      unsigned numDataArgs, bool isObjCLiteral,
>>>                      const char *beg, bool hasVAListArg,
>>> -                     const CallExpr *theCall, unsigned formatIdx,
>>> -                     bool inFunctionCall)
>>> +                     Expr **Args, unsigned NumArgs,
>>> +                     unsigned formatIdx, bool inFunctionCall)
>>>   : CheckFormatHandler(s, fexpr, origFormatExpr, firstDataArg,
>>>                        numDataArgs, isObjCLiteral, beg, hasVAListArg,
>>> -                       theCall, formatIdx, inFunctionCall) {}
>>> +                       Args, NumArgs, formatIdx, inFunctionCall) {}
>>>
>>>
>>>   bool HandleInvalidPrintfConversionSpecifier(
>>> @@ -2194,11 +2216,11 @@
>>>                     const Expr *origFormatExpr, unsigned firstDataArg,
>>>                     unsigned numDataArgs, bool isObjCLiteral,
>>>                     const char *beg, bool hasVAListArg,
>>> -                    const CallExpr *theCall, unsigned formatIdx,
>>> -                    bool inFunctionCall)
>>> +                    Expr **Args, unsigned NumArgs,
>>> +                    unsigned formatIdx, bool inFunctionCall)
>>>   : CheckFormatHandler(s, fexpr, origFormatExpr, firstDataArg,
>>>                        numDataArgs, isObjCLiteral, beg, hasVAListArg,
>>> -                       theCall, formatIdx, inFunctionCall) {}
>>> +                       Args, NumArgs, formatIdx, inFunctionCall) {}
>>>
>>>   bool HandleScanfSpecifier(const analyze_scanf::ScanfSpecifier &FS,
>>>                             const char *startSpecifier,
>>> @@ -2341,14 +2363,15 @@
>>>
>>>  void Sema::CheckFormatString(const StringLiteral *FExpr,
>>>                              const Expr *OrigFormatExpr,
>>> -                             const CallExpr *TheCall, bool HasVAListArg,
>>> -                             unsigned format_idx, unsigned firstDataArg,
>>> -                             bool isPrintf, bool inFunctionCall) {
>>> +                             Expr **Args, unsigned NumArgs,
>>> +                             bool HasVAListArg, unsigned format_idx,
>>> +                             unsigned firstDataArg, bool isPrintf,
>>> +                             bool inFunctionCall) {
>>>
>>>   // CHECK: is the format string a wide literal?
>>>   if (!FExpr->isAscii()) {
>>>     CheckFormatHandler::EmitFormatDiagnostic(
>>> -      *this, inFunctionCall, TheCall->getArg(format_idx),
>>> +      *this, inFunctionCall, Args[format_idx],
>>>       PDiag(diag::warn_format_string_is_wide_literal), FExpr->getLocStart(),
>>>       /*IsStringLocation*/true, OrigFormatExpr->getSourceRange());
>>>     return;
>>> @@ -2358,12 +2381,12 @@
>>>   StringRef StrRef = FExpr->getString();
>>>   const char *Str = StrRef.data();
>>>   unsigned StrLen = StrRef.size();
>>> -  const unsigned numDataArgs = TheCall->getNumArgs() - firstDataArg;
>>> +  const unsigned numDataArgs = NumArgs - firstDataArg;
>>>
>>>   // CHECK: empty format string?
>>>   if (StrLen == 0 && numDataArgs > 0) {
>>>     CheckFormatHandler::EmitFormatDiagnostic(
>>> -      *this, inFunctionCall, TheCall->getArg(format_idx),
>>> +      *this, inFunctionCall, Args[format_idx],
>>>       PDiag(diag::warn_empty_format_string), FExpr->getLocStart(),
>>>       /*IsStringLocation*/true, OrigFormatExpr->getSourceRange());
>>>     return;
>>> @@ -2372,7 +2395,7 @@
>>>   if (isPrintf) {
>>>     CheckPrintfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg,
>>>                          numDataArgs, isa<ObjCStringLiteral>(OrigFormatExpr),
>>> -                         Str, HasVAListArg, TheCall, format_idx,
>>> +                         Str, HasVAListArg, Args, NumArgs, format_idx,
>>>                          inFunctionCall);
>>>
>>>     if (!analyze_format_string::ParsePrintfString(H, Str, Str + StrLen,
>>> @@ -2382,7 +2405,7 @@
>>>   else {
>>>     CheckScanfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg,
>>>                         numDataArgs, isa<ObjCStringLiteral>(OrigFormatExpr),
>>> -                        Str, HasVAListArg, TheCall, format_idx,
>>> +                        Str, HasVAListArg, Args, NumArgs, format_idx,
>>>                         inFunctionCall);
>>>
>>>     if (!analyze_format_string::ParseScanfString(H, Str, Str + StrLen,
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=148324&r1=148323&r2=148324&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Tue Jan 17 14:03:31 2012
>>> @@ -453,14 +453,12 @@
>>>                        Args[NumArgs-1]->getLocEnd());
>>>     }
>>>   }
>>> -  // diagnose nonnull arguments.
>>> -  for (specific_attr_iterator<NonNullAttr>
>>> -       i = Method->specific_attr_begin<NonNullAttr>(),
>>> -       e = Method->specific_attr_end<NonNullAttr>(); i != e; ++i) {
>>> -    CheckNonNullArguments(*i, Args, lbrac);
>>> -  }
>>>
>>>   DiagnoseSentinelCalls(Method, lbrac, Args, NumArgs);
>>> +
>>> +  // Do additional checkings on method.
>>> +  IsError |= CheckObjCMethodCall(Method, lbrac, Args, NumArgs);
>>> +
>>>   return IsError;
>>>  }
>>>
>>>
>>> Modified: cfe/trunk/test/SemaObjC/format-strings-objc.m
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/format-strings-objc.m?rev=148324&r1=148323&r2=148324&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/test/SemaObjC/format-strings-objc.m (original)
>>> +++ cfe/trunk/test/SemaObjC/format-strings-objc.m Tue Jan 17 14:03:31 2012
>>> @@ -63,3 +63,22 @@
>>>   printf("%p", y); // no-warning
>>>  }
>>>
>>> +// <rdar://problem/10696348>, PR 10274 - CFString and NSString formats are ignored
>>> +extern void MyNSLog(NSString *format, ...) __attribute__((format(__NSString__, 1, 2)));
>>> +extern void MyCFStringCreateWithFormat(CFStringRef format, ...) __attribute__((format(__CFString__, 1, 2)));
>>> +
>>> +void check_mylog() {
>>> +  MyNSLog(@"%@"); // expected-warning {{more '%' conversions than data arguments}}
>>> +  // FIXME: find a way to test CFString too, but I don't know how to create constant CFString.
>>
>> A constant CFString is written as CFSTR("foo"), which expands to a
>> call to __builtin___CFStringMakeConstantString.
>>
>> -Eli
>
>
> Yes, I'm working on that part, but as __builtin___CFStringMakeConstantString is not interpreted as a string literal, it needs some special handling in the format checker.
>
> -- Jean-Daniel
>
>
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list