Add a -Wclass-varargs to warn on objects of any class type being passed through an ellipsis. Since C++11 relaxed the rules on this, we allow a lot more bad code through silently, such as:

  const char *format = "%s";
  std::experimental::string_view view = "foo";
  printf(format, view);

In this case, not only warn about a class type being used here, but also suggest that calling c_str() might be a good idea.

llvm-svn: 202461

Author: zygoloid

clang/include/clang/Basic/DiagnosticGroups.td

@@ -219,6 +219,7 @@ def NullDereference : DiagGroup<"null-dereference">;
 def InitializerOverrides : DiagGroup<"initializer-overrides">;
 def NonNull : DiagGroup<"nonnull">;
 def NonPODVarargs : DiagGroup<"non-pod-varargs">;
+def ClassVarargs : DiagGroup<"class-varargs", [NonPODVarargs]>;
 def : DiagGroup<"nonportable-cfstrings">;
 def NonVirtualDtor : DiagGroup<"non-virtual-dtor">;
 def OveralignedType : DiagGroup<"over-aligned">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -5725,6 +5725,11 @@ def warn_cxx98_compat_pass_non_pod_arg_to_vararg : Warning<
   "passing object of trivial but non-POD type %0 through variadic"
   " %select{function|block|method|constructor}1 is incompatible with C++98">,
   InGroup<CXX98Compat>, DefaultIgnore;
+def warn_pass_class_arg_to_vararg : Warning<
+  "passing object of class type %0 through variadic "
+  "%select{function|block|method|constructor}1"
+  "%select{|; did you mean to call '%3'?}2">,
+  InGroup<ClassVarargs>, DefaultIgnore;
 def err_cannot_pass_to_vararg : Error<
   "cannot pass %select{expression of type %1|initializer list}0 to variadic "
   "%select{function|block|method|constructor}2">;

clang/include/clang/Sema/Sema.h

@@ -7249,6 +7249,9 @@ class Sema {
   /// function, issuing a diagnostic if not.
   void checkVariadicArgument(const Expr *E, VariadicCallType CT);
 
+  /// Check to see if a given expression could have '.c_str()' called on it.
+  bool hasCStrMethod(const Expr *E);
+
   /// GatherArgumentsForCall - Collector argument expressions for various
   /// form of call prototypes.
   bool GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl,

clang/lib/Sema/SemaChecking.cpp

@@ -2765,7 +2765,7 @@ class CheckPrintfHandler : public CheckFormatHandler {
                          const analyze_printf::OptionalFlag &flag,
                          const char *startSpecifier, unsigned specifierLen);
   bool checkForCStrMembers(const analyze_printf::ArgType &AT,
-                           const Expr *E, const CharSourceRange &CSR);
+                           const Expr *E);
 
 };  
 }
@@ -2899,11 +2899,12 @@ CXXRecordMembersNamed(StringRef Name, Sema &S, QualType Ty) {
   if (!RT)
     return Results;
   const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(RT->getDecl());
-  if (!RD)
+  if (!RD || !RD->getDefinition())
     return Results;
 
   LookupResult R(S, &S.PP.getIdentifierTable().get(Name), SourceLocation(),
                  Sema::LookupMemberName);
+  R.suppressDiagnostics();
 
   // We just need to include all members of the right kind turned up by the
   // filter, at this point.
@@ -2916,12 +2917,26 @@ CXXRecordMembersNamed(StringRef Name, Sema &S, QualType Ty) {
   return Results;
 }
 
+/// Check if we could call '.c_str()' on an object.
+///
+/// FIXME: This returns the wrong results in some cases (if cv-qualifiers don't
+/// allow the call, or if it would be ambiguous).
+bool Sema::hasCStrMethod(const Expr *E) {
+  typedef llvm::SmallPtrSet<CXXMethodDecl*, 1> MethodSet;
+  MethodSet Results =
+      CXXRecordMembersNamed<CXXMethodDecl>("c_str", *this, E->getType());
+  for (MethodSet::iterator MI = Results.begin(), ME = Results.end();
+       MI != ME; ++MI)
+    if ((*MI)->getMinRequiredArguments() == 0)
+      return true;
+  return false;
+}
+
 // Check if a (w)string was passed when a (w)char* was needed, and offer a
 // better diagnostic if so. AT is assumed to be valid.
 // Returns true when a c_str() conversion method is found.
 bool CheckPrintfHandler::checkForCStrMembers(
-    const analyze_printf::ArgType &AT, const Expr *E,
-    const CharSourceRange &CSR) {
+    const analyze_printf::ArgType &AT, const Expr *E) {
   typedef llvm::SmallPtrSet<CXXMethodDecl*, 1> MethodSet;
 
   MethodSet Results =
@@ -2930,7 +2945,7 @@ bool CheckPrintfHandler::checkForCStrMembers(
   for (MethodSet::iterator MI = Results.begin(), ME = Results.end();
        MI != ME; ++MI) {
     const CXXMethodDecl *Method = *MI;
-    if (Method->getNumParams() == 0 &&
+    if (Method->getMinRequiredArguments() == 0 &&
         AT.matchesType(S.Context, Method->getReturnType())) {
       // FIXME: Suggest parens if the expression needs them.
       SourceLocation EndLoc =
@@ -3317,7 +3332,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
           << CSR
           << E->getSourceRange(),
         E->getLocStart(), /*IsStringLocation*/false, CSR);
-      checkForCStrMembers(AT, E, CSR);
+      checkForCStrMembers(AT, E);
       break;
 
     case Sema::VAK_Invalid:

clang/lib/Sema/SemaExpr.cpp

@@ -806,14 +806,20 @@ void Sema::checkVariadicArgument(const Expr *E, VariadicCallType CT) {
 
   // Complain about passing non-POD types through varargs.
   switch (VAK) {
-  case VAK_Valid:
-    break;
-
   case VAK_ValidInCXX11:
     DiagRuntimeBehavior(
         E->getLocStart(), 0,
         PDiag(diag::warn_cxx98_compat_pass_non_pod_arg_to_vararg)
-          << E->getType() << CT);
+          << Ty << CT);
+    // Fall through.
+  case VAK_Valid:
+    if (Ty->isRecordType()) {
+      // This is unlikely to be what the user intended. If the class has a
+      // 'c_str' member function, the user probably meant to call that.
+      DiagRuntimeBehavior(E->getLocStart(), 0,
+                          PDiag(diag::warn_pass_class_arg_to_vararg)
+                            << Ty << CT << hasCStrMethod(E) << ".c_str()");
+    }
     break;
 
   case VAK_Undefined:

clang/test/SemaCXX/vararg-class.cpp

@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -verify -Wclass-varargs -std=c++98 %s
+// RUN: %clang_cc1 -verify -Wclass-varargs -std=c++11 %s
+
+struct A {};
+struct B { ~B(); };
+class C { char *c_str(); };
+struct D { char *c_str(); };
+struct E { E(); };
+struct F { F(); char *c_str(); };
+
+void v(...);
+void w(const char*, ...) __attribute__((format(printf, 1, 2)));
+
+void test(A a, B b, C c, D d, E e, F f) {
+  v(a); // expected-warning-re {{passing object of class type 'A' through variadic function{{$}}}}
+  v(b); // expected-error-re {{cannot pass object of non-{{POD|trivial}} type 'B' through variadic function; call will abort at runtime}}
+  v(c); // expected-warning {{passing object of class type 'C' through variadic function; did you mean to call '.c_str()'?}}
+  v(d); // expected-warning {{passing object of class type 'D' through variadic function; did you mean to call '.c_str()'?}}
+  v(e);
+  v(f);
+#if __cplusplus < 201103L
+  // expected-error@-3 {{cannot pass object of non-POD type 'E' through variadic function; call will abort at runtime}}
+  // expected-error@-3 {{cannot pass object of non-POD type 'F' through variadic function; call will abort at runtime}}
+#else
+  // expected-warning-re@-6 {{passing object of class type 'E' through variadic function{{$}}}}
+  // expected-warning@-6 {{passing object of class type 'F' through variadic function; did you mean to call '.c_str()'?}}
+#endif
+
+  v(d.c_str());
+  v(f.c_str());
+  v(0);
+  v('x');
+
+  w("%s", a); // expected-warning {{format specifies type 'char *' but the argument has type 'A'}}
+  w("%s", b); // expected-error-re {{cannot pass non-{{POD|trivial}} object of type 'B' to variadic function; expected type from format string was 'char *'}}
+  w("%s", c); // expected-warning {{format specifies type 'char *' but the argument has type 'C'}}
+  w("%s", d); // expected-warning {{format specifies type 'char *' but the argument has type 'D'}}
+  w("%s", e);
+  w("%s", f);
+#if __cplusplus < 201103L
+  // expected-error@-3 {{cannot pass non-POD object of type 'E' to variadic function; expected type from format string was 'char *'}}
+  // expected-error@-3 {{cannot pass non-POD object of type 'F' to variadic function; expected type from format string was 'char *'}}
+  // expected-note@-4 {{did you mean to call the c_str() method?}}
+#else
+  // expected-warning@-7 {{format specifies type 'char *' but the argument has type 'E'}}
+  // expected-warning@-7 {{format specifies type 'char *' but the argument has type 'F'}}
+#endif
+}