[cfe-commits] r160517 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp lib/Sema/SemaExpr.cpp test/SemaCXX/printf-block.cpp test/SemaCXX/printf-cstr.cpp test/SemaObjC/format-strings-objc.m test/SemaObjC/method-bad-param.m

Jordan Rose jordan_rose at apple.com
Thu Jul 19 11:10:24 PDT 2012


Author: jrose
Date: Thu Jul 19 13:10:23 2012
New Revision: 160517

URL: http://llvm.org/viewvc/llvm-project?rev=160517&view=rev
Log:
For varargs, diagnose passing ObjC objects by value like other non-POD types.

While we still want to consider this a hard error (non-POD variadic args are
normally a DefaultError warning), delaying the diagnostic allows us to give
better error messages, which also match the usual non-POD errors more closely.

In addition, this change improves the diagnostic messages for format string
argument type mismatches by passing down the type of the callee, so we can
say "variadic method" or "variadic function" appropriately.

<rdar://problem/11825593>

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/SemaCXX/printf-block.cpp
    cfe/trunk/test/SemaCXX/printf-cstr.cpp
    cfe/trunk/test/SemaObjC/format-strings-objc.m
    cfe/trunk/test/SemaObjC/method-bad-param.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=160517&r1=160516&r2=160517&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Jul 19 13:10:23 2012
@@ -4891,16 +4891,19 @@
   "reference to %select{__device__|__global__|__host__|__host__ __device__}0 "
   "function %1 in %select{__device__|__global__|__host__|__host__ __device__}2 function">;
 
-
-def err_cannot_pass_objc_interface_to_vararg : Error<
-  "cannot pass object with interface type %0 by-value through variadic "
-  "%select{function|block|method}1">;
-
 def warn_non_pod_vararg_with_format_string : Warning<
   "cannot pass %select{non-POD|non-trivial}0 object of type %1 to variadic "
-  "function; expected type from format string was %2">,
-  InGroup<DiagGroup<"non-pod-varargs">>, DefaultError;
+  "%select{function|block|method|constructor}2; expected type from format "
+  "string was %3">, InGroup<DiagGroup<"non-pod-varargs">>, DefaultError;
+// The arguments to this diagnostic should match the warning above.
+def err_cannot_pass_objc_interface_to_vararg_format : Error<
+  "cannot pass object with interface type %1 by value to variadic "
+  "%select{function|block|method|constructor}2; expected type from format "
+  "string was %3">;
 
+def err_cannot_pass_objc_interface_to_vararg : Error<
+  "cannot pass object with interface type %0 by value through variadic "
+  "%select{function|block|method|constructor}1">;
 def warn_cannot_pass_non_pod_arg_to_vararg : Warning<
   "cannot pass object of %select{non-POD|non-trivial}0 type %1 through variadic"
   " %select{function|block|method|constructor}2; call will abort at runtime">,

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=160517&r1=160516&r2=160517&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Jul 19 13:10:23 2012
@@ -6455,7 +6455,7 @@
                               bool AllowExplicit = false);
 
   // DefaultVariadicArgumentPromotion - Like DefaultArgumentPromotion, but
-  // will return ExprError() if the resulting type is not a POD type.
+  // will create a runtime trap if the resulting type is not a POD type.
   ExprResult DefaultVariadicArgumentPromotion(Expr *E, VariadicCallType CT,
                                               FunctionDecl *FDecl);
 
@@ -7117,20 +7117,23 @@
                                                unsigned format_idx,
                                                unsigned firstDataArg,
                                                FormatStringType Type,
+                                               VariadicCallType CallType,
                                                bool inFunctionCall = true);
 
   void CheckFormatString(const StringLiteral *FExpr, const Expr *OrigFormatExpr,
                          Expr **Args, unsigned NumArgs, bool HasVAListArg,
                          unsigned format_idx, unsigned firstDataArg,
-                         FormatStringType Type, bool inFunctionCall);
+                         FormatStringType Type, bool inFunctionCall,
+                         VariadicCallType CallType);
 
-  bool CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall);
   bool CheckFormatArguments(const FormatAttr *Format, Expr **Args,
                             unsigned NumArgs, bool IsCXXMember,
+                            VariadicCallType CallType,
                             SourceLocation Loc, SourceRange Range);
   bool CheckFormatArguments(Expr **Args, unsigned NumArgs,
                             bool HasVAListArg, unsigned format_idx,
                             unsigned firstDataArg, FormatStringType Type,
+                            VariadicCallType CallType,
                             SourceLocation Loc, SourceRange range);
 
   void CheckNonNullArguments(const NonNullAttr *NonNull,

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=160517&r1=160516&r2=160517&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Jul 19 13:10:23 2012
@@ -499,7 +499,8 @@
   for (specific_attr_iterator<FormatAttr>
          I = FDecl->specific_attr_begin<FormatAttr>(),
          E = FDecl->specific_attr_end<FormatAttr>(); I != E ; ++I)
-    if (CheckFormatArguments(*I, Args, NumArgs, IsMemberFunction, Loc, Range))
+    if (CheckFormatArguments(*I, Args, NumArgs, IsMemberFunction, CallType,
+                             Loc, Range))
         HandledFormatString = true;
 
   // Refuse POD arguments that weren't caught by the format string
@@ -1610,7 +1611,8 @@
 Sema::checkFormatStringExpr(const Expr *E, Expr **Args,
                             unsigned NumArgs, bool HasVAListArg,
                             unsigned format_idx, unsigned firstDataArg,
-                            FormatStringType Type, bool inFunctionCall) {
+                            FormatStringType Type, VariadicCallType CallType,
+                            bool inFunctionCall) {
  tryAgain:
   if (E->isTypeDependent() || E->isValueDependent())
     return SLCT_NotALiteral;
@@ -1634,13 +1636,13 @@
     StringLiteralCheckType Left =
         checkFormatStringExpr(C->getTrueExpr(), Args, NumArgs,
                               HasVAListArg, format_idx, firstDataArg,
-                              Type, inFunctionCall);
+                              Type, CallType, inFunctionCall);
     if (Left == SLCT_NotALiteral)
       return SLCT_NotALiteral;
     StringLiteralCheckType Right =
         checkFormatStringExpr(C->getFalseExpr(), Args, NumArgs,
                               HasVAListArg, format_idx, firstDataArg,
-                              Type, inFunctionCall);
+                              Type, CallType, inFunctionCall);
     return Left < Right ? Left : Right;
   }
 
@@ -1691,7 +1693,7 @@
           }
           return checkFormatStringExpr(Init, Args, NumArgs,
                                        HasVAListArg, format_idx,
-                                       firstDataArg, Type,
+                                       firstDataArg, Type, CallType,
                                        /*inFunctionCall*/false);
         }
       }
@@ -1749,7 +1751,7 @@
 
         return checkFormatStringExpr(Arg, Args, NumArgs,
                                      HasVAListArg, format_idx, firstDataArg,
-                                     Type, inFunctionCall);
+                                     Type, CallType, inFunctionCall);
       } else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(ND)) {
         unsigned BuiltinID = FD->getBuiltinID();
         if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
@@ -1757,7 +1759,8 @@
           const Expr *Arg = CE->getArg(0);
           return checkFormatStringExpr(Arg, Args, NumArgs,
                                        HasVAListArg, format_idx,
-                                       firstDataArg, Type, inFunctionCall);
+                                       firstDataArg, Type, CallType,
+                                       inFunctionCall);
         }
       }
     }
@@ -1775,7 +1778,7 @@
 
     if (StrE) {
       CheckFormatString(StrE, E, Args, NumArgs, HasVAListArg, format_idx,
-                        firstDataArg, Type, inFunctionCall);
+                        firstDataArg, Type, inFunctionCall, CallType);
       return SLCT_CheckedLiteral;
     }
 
@@ -1812,31 +1815,25 @@
   .Default(FST_Unknown);
 }
 
-/// CheckPrintfScanfArguments - Check calls to printf and scanf (and similar
+/// CheckFormatArguments - Check calls to printf and scanf (and similar
 /// functions) for correct use of format strings.
 /// Returns true if a format string has been fully checked.
-bool Sema::CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall) {
-  bool IsCXXMember = isa<CXXMemberCallExpr>(TheCall);
-  return CheckFormatArguments(Format, TheCall->getArgs(),
-                              TheCall->getNumArgs(),
-                              IsCXXMember, TheCall->getRParenLoc(), 
-                              TheCall->getCallee()->getSourceRange());
-}
-
 bool Sema::CheckFormatArguments(const FormatAttr *Format, Expr **Args,
                                 unsigned NumArgs, bool IsCXXMember,
+                                VariadicCallType CallType,
                                 SourceLocation Loc, SourceRange Range) {
   FormatStringInfo FSI;
   if (getFormatStringInfo(Format, IsCXXMember, &FSI))
     return CheckFormatArguments(Args, NumArgs, FSI.HasVAListArg, FSI.FormatIdx,
                                 FSI.FirstDataArg, GetFormatStringType(Format),
-                                Loc, Range);
+                                CallType, Loc, Range);
   return false;
 }
 
 bool Sema::CheckFormatArguments(Expr **Args, unsigned NumArgs,
                                 bool HasVAListArg, unsigned format_idx,
                                 unsigned firstDataArg, FormatStringType Type,
+                                VariadicCallType CallType,
                                 SourceLocation Loc, SourceRange Range) {
   // CHECK: printf/scanf-like function is called with no format string.
   if (format_idx >= NumArgs) {
@@ -1860,7 +1857,7 @@
   // the same format string checking logic for both ObjC and C strings.
   StringLiteralCheckType CT =
       checkFormatStringExpr(OrigFormatExpr, Args, NumArgs, HasVAListArg,
-                            format_idx, firstDataArg, Type);
+                            format_idx, firstDataArg, Type, CallType);
   if (CT != SLCT_NotALiteral)
     // Literal format string found, check done!
     return CT == SLCT_CheckedLiteral;
@@ -1908,18 +1905,20 @@
   bool usesPositionalArgs;
   bool atFirstArg;
   bool inFunctionCall;
+  Sema::VariadicCallType CallType;
 public:
   CheckFormatHandler(Sema &s, const StringLiteral *fexpr,
                      const Expr *origFormatExpr, unsigned firstDataArg,
                      unsigned numDataArgs, const char *beg, bool hasVAListArg,
                      Expr **args, unsigned numArgs,
-                     unsigned formatIdx, bool inFunctionCall)
+                     unsigned formatIdx, bool inFunctionCall,
+                     Sema::VariadicCallType callType)
     : S(s), FExpr(fexpr), OrigFormatExpr(origFormatExpr),
       FirstDataArg(firstDataArg), NumDataArgs(numDataArgs),
       Beg(beg), HasVAListArg(hasVAListArg),
       Args(args), NumArgs(numArgs), FormatIdx(formatIdx),
       usesPositionalArgs(false), atFirstArg(true),
-      inFunctionCall(inFunctionCall) {
+      inFunctionCall(inFunctionCall), CallType(callType) {
         CoveredArgs.resize(numDataArgs);
         CoveredArgs.reset();
       }
@@ -2238,11 +2237,13 @@
                      unsigned numDataArgs, bool isObjC,
                      const char *beg, bool hasVAListArg,
                      Expr **Args, unsigned NumArgs,
-                     unsigned formatIdx, bool inFunctionCall)
+                     unsigned formatIdx, bool inFunctionCall,
+                     Sema::VariadicCallType CallType)
   : CheckFormatHandler(s, fexpr, origFormatExpr, firstDataArg,
                        numDataArgs, beg, hasVAListArg, Args, NumArgs,
-                       formatIdx, inFunctionCall), ObjCContext(isObjC) {}
-  
+                       formatIdx, inFunctionCall, CallType), ObjCContext(isObjC)
+  {}
+
   
   bool HandleInvalidPrintfConversionSpecifier(
                                       const analyze_printf::PrintfSpecifier &FS,
@@ -2646,10 +2647,17 @@
       // was deferred until now, we emit a warning for non-POD
       // arguments here.
       if (S.isValidVarArgType(E->getType()) == Sema::VAK_Invalid) {
+        unsigned DiagKind;
+        if (E->getType()->isObjCObjectType())
+          DiagKind = diag::err_cannot_pass_objc_interface_to_vararg_format;
+        else
+          DiagKind = diag::warn_non_pod_vararg_with_format_string;
+
         EmitFormatDiagnostic(
-          S.PDiag(diag::warn_non_pod_vararg_with_format_string)
+          S.PDiag(DiagKind)
             << S.getLangOpts().CPlusPlus0x
             << E->getType()
+            << CallType
             << ATR.getRepresentativeTypeName(S.Context)
             << CSR
             << E->getSourceRange(),
@@ -2678,10 +2686,12 @@
                     const Expr *origFormatExpr, unsigned firstDataArg,
                     unsigned numDataArgs, const char *beg, bool hasVAListArg,
                     Expr **Args, unsigned NumArgs,
-                    unsigned formatIdx, bool inFunctionCall)
+                    unsigned formatIdx, bool inFunctionCall,
+                    Sema::VariadicCallType CallType)
   : CheckFormatHandler(s, fexpr, origFormatExpr, firstDataArg,
                        numDataArgs, beg, hasVAListArg,
-                       Args, NumArgs, formatIdx, inFunctionCall) {}
+                       Args, NumArgs, formatIdx, inFunctionCall, CallType)
+  {}
   
   bool HandleScanfSpecifier(const analyze_scanf::ScanfSpecifier &FS,
                             const char *startSpecifier,
@@ -2842,7 +2852,7 @@
                              Expr **Args, unsigned NumArgs,
                              bool HasVAListArg, unsigned format_idx,
                              unsigned firstDataArg, FormatStringType Type,
-                             bool inFunctionCall) {
+                             bool inFunctionCall, VariadicCallType CallType) {
   
   // CHECK: is the format string a wide literal?
   if (!FExpr->isAscii() && !FExpr->isUTF8()) {
@@ -2872,7 +2882,7 @@
     CheckPrintfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg,
                          numDataArgs, (Type == FST_NSString),
                          Str, HasVAListArg, Args, NumArgs, format_idx,
-                         inFunctionCall);
+                         inFunctionCall, CallType);
   
     if (!analyze_format_string::ParsePrintfString(H, Str, Str + StrLen,
                                                   getLangOpts()))
@@ -2880,7 +2890,7 @@
   } else if (Type == FST_Scanf) {
     CheckScanfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg, numDataArgs,
                         Str, HasVAListArg, Args, NumArgs, format_idx,
-                        inFunctionCall);
+                        inFunctionCall, CallType);
     
     if (!analyze_format_string::ParseScanfString(H, Str, Str + StrLen,
                                                  getLangOpts()))

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=160517&r1=160516&r2=160517&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Jul 19 13:10:23 2012
@@ -602,14 +602,20 @@
 /// Incomplete types are considered POD, since this check can be performed
 /// when we're in an unevaluated context.
 Sema::VarArgKind Sema::isValidVarArgType(const QualType &Ty) {
-  if (Ty->isIncompleteType() || Ty.isCXX98PODType(Context))
+  if (Ty->isIncompleteType()) {
+    if (Ty->isObjCObjectType())
+      return VAK_Invalid;
     return VAK_Valid;
+  }
+
+  if (Ty.isCXX98PODType(Context))
+    return VAK_Valid;
+
   // C++0x [expr.call]p7:
   //   Passing a potentially-evaluated argument of class type (Clause 9) 
   //   having a non-trivial copy constructor, a non-trivial move constructor,
   //   or a non-trivial destructor, with no corresponding parameter, 
   //   is conditionally-supported with implementation-defined semantics.
-
   if (getLangOpts().CPlusPlus0x && !Ty->isDependentType())
     if (CXXRecordDecl *Record = Ty->getAsCXXRecordDecl())
       if (Record->hasTrivialCopyConstructor() &&
@@ -635,18 +641,23 @@
         PDiag(diag::warn_cxx98_compat_pass_non_pod_arg_to_vararg)
         << E->getType() << CT);
     break;
-  case VAK_Invalid:
+  case VAK_Invalid: {
+    if (Ty->isObjCObjectType())
+      return DiagRuntimeBehavior(E->getLocStart(), 0,
+                          PDiag(diag::err_cannot_pass_objc_interface_to_vararg)
+                            << Ty << CT);
+
     return DiagRuntimeBehavior(E->getLocStart(), 0,
                    PDiag(diag::warn_cannot_pass_non_pod_arg_to_vararg)
                    << getLangOpts().CPlusPlus0x << Ty << CT);
   }
+  }
   // c++ rules are enforced elsewhere.
   return false;
 }
 
 /// DefaultVariadicArgumentPromotion - Like DefaultArgumentPromotion, but
-/// will warn if the resulting type is not a POD type, and rejects ObjC
-/// interfaces passed by value.
+/// will create a trap if the resulting type is not a POD type.
 ExprResult Sema::DefaultVariadicArgumentPromotion(Expr *E, VariadicCallType CT,
                                                   FunctionDecl *FDecl) {
   if (const BuiltinType *PlaceholderTy = E->getType()->getAsPlaceholderType()) {
@@ -670,12 +681,6 @@
     return ExprError();
   E = ExprRes.take();
 
-  if (E->getType()->isObjCObjectType() &&
-    DiagRuntimeBehavior(E->getLocStart(), 0,
-                        PDiag(diag::err_cannot_pass_objc_interface_to_vararg)
-                          << E->getType() << CT))
-    return ExprError();
-
   // Diagnostics regarding non-POD argument types are
   // emitted along with format string checking in Sema::CheckFunctionCall().
   if (isValidVarArgType(E->getType()) == VAK_Invalid) {

Modified: cfe/trunk/test/SemaCXX/printf-block.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/printf-block.cpp?rev=160517&r1=160516&r2=160517&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/printf-block.cpp (original)
+++ cfe/trunk/test/SemaCXX/printf-block.cpp Thu Jul 19 13:10:23 2012
@@ -14,5 +14,5 @@
   HasNoCStr hncs(str);
   int n = 4;
   block(n, "%s %d", str, n); // no-warning
-  block(n, "%s %s", hncs, n); // expected-warning{{cannot pass non-POD object of type 'HasNoCStr' to variadic function; expected type from format string was 'char *'}} expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
+  block(n, "%s %s", hncs, n); // expected-warning{{cannot pass non-POD object of type 'HasNoCStr' to variadic block; expected type from format string was 'char *'}} expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
 }

Modified: cfe/trunk/test/SemaCXX/printf-cstr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/printf-cstr.cpp?rev=160517&r1=160516&r2=160517&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/printf-cstr.cpp (original)
+++ cfe/trunk/test/SemaCXX/printf-cstr.cpp Thu Jul 19 13:10:23 2012
@@ -49,5 +49,5 @@
   const char str[] = "test";
   HasCStr hcs(str);
   Printf p("%s %d %s", str, 10, 10); // expected-warning {{format specifies type 'char *' but the argument has type 'int'}}
-  Printf q("%s %d", hcs, 10); // expected-warning {{cannot pass non-POD object of type 'HasCStr' to variadic function; expected type from format string was 'char *'}} expected-note{{did you mean to call the c_str() method?}}
+  Printf q("%s %d", hcs, 10); // expected-warning {{cannot pass non-POD object of type 'HasCStr' to variadic constructor; expected type from format string was 'char *'}} expected-note{{did you mean to call the c_str() method?}}
 }

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=160517&r1=160516&r2=160517&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/format-strings-objc.m (original)
+++ cfe/trunk/test/SemaObjC/format-strings-objc.m Thu Jul 19 13:10:23 2012
@@ -227,3 +227,11 @@
   [Foo fooWithFormat:@"%@ %@", dict[CFSTR("abc")]]; // expected-error{{indexing expression is invalid because subscript type 'CFStringRef' (aka 'const struct __CFString *') is not an integral or Objective-C pointer type}} expected-warning{{more '%' conversions than data arguments}}
 }
 
+
+// <rdar://problem/11825593>
+void testByValueObjectInFormat(Foo *obj) {
+  printf("%d %d %d", 1L, *obj, 1L); // expected-error {{cannot pass object with interface type 'Foo' by value to variadic function; expected type from format string was 'int'}} expected-warning 2 {{format specifies type 'int' but the argument has type 'long'}}
+
+  [Bar log2:@"%d", *obj]; // expected-error {{cannot pass object with interface type 'Foo' by value to variadic method; expected type from format string was 'int'}}
+}
+

Modified: cfe/trunk/test/SemaObjC/method-bad-param.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/method-bad-param.m?rev=160517&r1=160516&r2=160517&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/method-bad-param.m (original)
+++ cfe/trunk/test/SemaObjC/method-bad-param.m Thu Jul 19 13:10:23 2012
@@ -26,7 +26,7 @@
 // rdar://6780761
 void f0(foo *a0) {
   extern void g0(int x, ...);
-  g0(1, *(foo*)a0);  // expected-error {{cannot pass object with interface type 'foo' by-value through variadic function}}
+  g0(1, *(foo*)a0);  // expected-error {{cannot pass object with interface type 'foo' by value through variadic function}}
 }
 
 // rdar://8421082





More information about the cfe-commits mailing list