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

Jean-Daniel Dupas devlists at shadowlab.org
Mon Jan 30 00:46:47 PST 2012


Author: jddupas
Date: Mon Jan 30 02:46:47 2012
New Revision: 149236

URL: http://llvm.org/viewvc/llvm-project?rev=149236&view=rev
Log:
Update on format attribute handling.
- Remove the printf0 special handling as we treat it as printf anyway.
- Perform basic checks (non-literal, empty) for all formats and not only printf/scanf.


Modified:
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/test/Sema/format-strings.c
    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=149236&r1=149235&r2=149236&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Jan 30 02:46:47 2012
@@ -6278,8 +6278,6 @@
                            Expr **Args, unsigned NumArgs);
   bool CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall);
 
-  bool CheckablePrintfAttr(const FormatAttr *Format, Expr **Args, 
-                           unsigned NumArgs, bool IsCXXMemberCall);
   bool CheckObjCString(Expr *Arg);
 
   ExprResult CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
@@ -6303,28 +6301,38 @@
   bool SemaBuiltinConstantArg(CallExpr *TheCall, int ArgNum,
                               llvm::APSInt &Result);
 
+  enum FormatStringType {
+    FST_Scanf,
+    FST_Printf,
+    FST_NSString,
+    FST_Strftime,
+    FST_Strfmon,
+    FST_Kprintf,
+    FST_Unknown
+  };
+  static FormatStringType GetFormatStringType(const FormatAttr *Format);
   bool SemaCheckStringLiteral(const Expr *E, Expr **Args, unsigned NumArgs,
                               bool HasVAListArg, unsigned format_idx,
-                              unsigned firstDataArg, bool isPrintf,
+                              unsigned firstDataArg, FormatStringType Type,
                               bool inFunctionCall = true);
 
   void CheckFormatString(const StringLiteral *FExpr, const Expr *OrigFormatExpr,
                          Expr **Args, unsigned NumArgs, bool HasVAListArg,
                          unsigned format_idx, unsigned firstDataArg,
-                         bool isPrintf, bool inFunctionCall);
-
-  void CheckNonNullArguments(const NonNullAttr *NonNull,
-                             const Expr * const *ExprArgs,
-                             SourceLocation CallSiteLoc);
+                         FormatStringType Type, bool inFunctionCall);
 
   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 CheckFormatArguments(Expr **Args, unsigned NumArgs,
+                            bool HasVAListArg, unsigned format_idx,
+                            unsigned firstDataArg, FormatStringType Type,
+                            SourceLocation Loc, SourceRange range);
+
+  void CheckNonNullArguments(const NonNullAttr *NonNull,
+                             const Expr * const *ExprArgs,
+                             SourceLocation CallSiteLoc);
 
   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=149236&r1=149235&r2=149236&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Jan 30 02:46:47 2012
@@ -45,31 +45,6 @@
                                PP.getLangOptions(), PP.getTargetInfo());
 }
 
-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 (IsCXXMemberCall) {
-      if (format_idx == 0)
-        return false;
-      --format_idx;
-    }
-    if (format_idx < NumArgs) {
-      Expr *Format = Args[format_idx]->IgnoreParenCasts();
-      if (!Format->isNullPointerConstant(Context,
-                                         Expr::NPC_ValueDependentIsNull))
-        return true;
-    }
-  }
-  return false;
-}
-
 /// Checks that a call expression's argument count is the desired number.
 /// This is useful when doing custom type-checking.  Returns true on error.
 static bool checkArgCount(Sema &S, CallExpr *call, unsigned desiredArgCount) {
@@ -1384,23 +1359,23 @@
 bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args,
                                   unsigned NumArgs, bool HasVAListArg,
                                   unsigned format_idx, unsigned firstDataArg,
-                                  bool isPrintf, bool inFunctionCall) {
+                                  FormatStringType Type, bool inFunctionCall) {
  tryAgain:
   if (E->isTypeDependent() || E->isValueDependent())
     return false;
 
-  E = E->IgnoreParens();
+  E = E->IgnoreParenCasts();
 
   switch (E->getStmtClass()) {
   case Stmt::BinaryConditionalOperatorClass:
   case Stmt::ConditionalOperatorClass: {
     const AbstractConditionalOperator *C = cast<AbstractConditionalOperator>(E);
     return SemaCheckStringLiteral(C->getTrueExpr(), Args, NumArgs, HasVAListArg,
-                                  format_idx, firstDataArg, isPrintf,
+                                  format_idx, firstDataArg, Type,
                                   inFunctionCall)
-        && SemaCheckStringLiteral(C->getFalseExpr(), Args, NumArgs, HasVAListArg,
-                                  format_idx, firstDataArg, isPrintf,
-                                  inFunctionCall);
+       && SemaCheckStringLiteral(C->getFalseExpr(), Args, NumArgs, HasVAListArg,
+                                 format_idx, firstDataArg, Type,
+                                 inFunctionCall);
   }
 
   case Stmt::IntegerLiteralClass:
@@ -1452,7 +1427,7 @@
         if (const Expr *Init = VD->getAnyInitializer())
           return SemaCheckStringLiteral(Init, Args, NumArgs,
                                         HasVAListArg, format_idx, firstDataArg,
-                                        isPrintf, /*inFunctionCall*/false);
+                                        Type, /*inFunctionCall*/false);
       }
 
       // For vprintf* functions (i.e., HasVAListArg==true), we add a
@@ -1492,7 +1467,7 @@
             const Expr *Arg = CE->getArg(ArgIndex - 1);
 
             return SemaCheckStringLiteral(Arg, Args, NumArgs, HasVAListArg,
-                                          format_idx, firstDataArg, isPrintf,
+                                          format_idx, firstDataArg, Type,
                                           inFunctionCall);
           }
         }
@@ -1512,7 +1487,7 @@
 
     if (StrE) {
       CheckFormatString(StrE, E, Args, NumArgs, HasVAListArg, format_idx,
-                        firstDataArg, isPrintf, inFunctionCall);
+                        firstDataArg, Type, inFunctionCall);
       return true;
     }
 
@@ -1538,6 +1513,17 @@
   }
 }
 
+Sema::FormatStringType Sema::GetFormatStringType(const FormatAttr *Format) {
+  return llvm::StringSwitch<FormatStringType>(Format->getType())
+  .Case("scanf", FST_Scanf)
+  .Cases("printf", "printf0", FST_Printf)
+  .Cases("NSString", "CFString", FST_NSString)
+  .Case("strftime", FST_Strftime)
+  .Case("strfmon", FST_Strfmon)
+  .Cases("kprintf", "cmn_err", "vcmn_err", "zcmn_err", FST_Kprintf)
+  .Default(FST_Unknown);
+}
+
 /// CheckPrintfScanfArguments - Check calls to printf and scanf (and similar
 /// functions) for correct use of format strings.
 void Sema::CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall) {
@@ -1558,27 +1544,24 @@
 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);
+  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;
   }
+  CheckFormatArguments(Args, NumArgs, HasVAListArg, format_idx,
+                       firstDataArg, GetFormatStringType(Format), Loc, Range);
 }
 
-void Sema::CheckPrintfScanfArguments(Expr **Args, unsigned NumArgs,
-                                     bool HasVAListArg, unsigned format_idx,
-                                     unsigned firstDataArg, bool isPrintf,
-                                     SourceLocation Loc, SourceRange Range) {
+void Sema::CheckFormatArguments(Expr **Args, unsigned NumArgs,
+                                bool HasVAListArg, unsigned format_idx,
+                                unsigned firstDataArg, FormatStringType Type,
+                                SourceLocation Loc, SourceRange Range) {
   // CHECK: printf/scanf-like function is called with no format string.
   if (format_idx >= NumArgs) {
     Diag(Loc, diag::warn_missing_format_string) << Range;
@@ -1600,7 +1583,7 @@
   // 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, Args, NumArgs, HasVAListArg,
-                             format_idx, firstDataArg, isPrintf))
+                             format_idx, firstDataArg, Type))
     return;  // Literal format string found, check done!
 
   // If there are no arguments specified, warn with -Wformat-security, otherwise
@@ -2376,7 +2359,7 @@
                              const Expr *OrigFormatExpr,
                              Expr **Args, unsigned NumArgs,
                              bool HasVAListArg, unsigned format_idx,
-                             unsigned firstDataArg, bool isPrintf,
+                             unsigned firstDataArg, FormatStringType Type,
                              bool inFunctionCall) {
   
   // CHECK: is the format string a wide literal?
@@ -2403,7 +2386,7 @@
     return;
   }
   
-  if (isPrintf) {
+  if (Type == FST_Printf || Type == FST_NSString) {
     CheckPrintfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg,
                          numDataArgs, isa<ObjCStringLiteral>(OrigFormatExpr),
                          Str, HasVAListArg, Args, NumArgs, format_idx,
@@ -2412,8 +2395,7 @@
     if (!analyze_format_string::ParsePrintfString(H, Str, Str + StrLen,
                                                   getLangOptions()))
       H.DoneProcessing();
-  }
-  else {
+  } else if (Type == FST_Scanf) {
     CheckScanfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg,
                         numDataArgs, isa<ObjCStringLiteral>(OrigFormatExpr),
                         Str, HasVAListArg, Args, NumArgs, format_idx,
@@ -2422,7 +2404,7 @@
     if (!analyze_format_string::ParseScanfString(H, Str, Str + StrLen,
                                                  getLangOptions()))
       H.DoneProcessing();
-  }
+  } // TODO: handle other formats
 }
 
 //===--- CHECK: Standard memory functions ---------------------------------===//

Modified: cfe/trunk/test/Sema/format-strings.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=149236&r1=149235&r2=149236&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings.c (original)
+++ cfe/trunk/test/Sema/format-strings.c Mon Jan 30 02:46:47 2012
@@ -480,3 +480,14 @@
   printf("%Lx", x); // no-warning
   printf("%Ls", "hello"); // expected-warning {{length modifier 'L' results in undefined behavior or no effect with 's' conversion specifier}}
 }
+
+void __attribute__((format(strfmon,1,2))) monformat(const char *fmt, ...);
+void __attribute__((format(strftime,1,0))) dateformat(const char *fmt);
+
+// Other formats
+void test_other_formats() {
+  char *str = "";
+  monformat("", 1); // expected-warning{{format string is empty}}
+  dateformat(""); // expected-warning{{format string is empty}}
+  dateformat(str); // expected-warning{{format string is not a string literal (potentially insecure)}}
+}

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=149236&r1=149235&r2=149236&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/format-strings-objc.m (original)
+++ cfe/trunk/test/SemaObjC/format-strings-objc.m Mon Jan 30 02:46:47 2012
@@ -97,7 +97,7 @@
   printf(s2); // expected-warning {{more '%' conversions than data arguments}}
 
   const char * const s3 = (const char *)0;
-  printf(s3); // expected-warning {{format string is not a string literal}}
+  printf(s3); // no-warning (NULL is a valid format string)
 
   NSString * const ns1 = @"constant string %s"; // expected-note {{format string is defined here}}
   NSLog(ns1); // expected-warning {{more '%' conversions than data arguments}}





More information about the cfe-commits mailing list