Add a format warning for "%p" with non-void* args

GCC -pedantic produces a format warning when the "%p" specifier is used with
arguments that are not void*. It's useful for portability to be able to
catch such warnings with clang as well. The warning is off by default in
both gcc and with this patch. This patch enables it either when extensions
are disabled with -pedantic, or with the specific flag -Wformat-pedantic.

The C99 and C11 specs do appear to require arguments corresponding to 'p'
specifiers to be void*: "If any argument is not the correct type for the
corresponding conversion specification, the behavior is undefined."
[7.19.6.1 p9], and of the 'p' format specifier "The argument shall be a
pointer to void." [7.19.6.1 p8]

Both printf and scanf format checking are covered.

llvm-svn: 231211

1 files changed, 41 insertions, 31 deletions

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 01d82155f73..72cb6003f5a 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3669,8 +3669,11 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
     ExprTy = TET->getUnderlyingExpr()->getType();
   }
 
-  if (AT.matchesType(S.Context, ExprTy))
+  analyze_printf::ArgType::MatchKind match = AT.matchesType(S.Context, ExprTy);
+
+  if (match == analyze_printf::ArgType::Match) {
     return true;
+  }
 
   // Look through argument promotions for our error message's reported type.
   // This includes the integral and floating promotions, but excludes array
@@ -3848,15 +3851,18 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
     // arguments here.
     switch (S.isValidVarArgType(ExprTy)) {
     case Sema::VAK_Valid:
-    case Sema::VAK_ValidInCXX11:
+    case Sema::VAK_ValidInCXX11: {
+      unsigned diag = diag::warn_format_conversion_argument_type_mismatch;
+      if (match == analyze_printf::ArgType::NoMatchPedantic) {
+        diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
+      }
+
       EmitFormatDiagnostic(
-        S.PDiag(diag::warn_format_conversion_argument_type_mismatch)
-          << AT.getRepresentativeTypeName(S.Context) << ExprTy << IsEnum
-          << CSR
-          << E->getSourceRange(),
-        E->getLocStart(), /*IsStringLocation*/false, CSR);
+          S.PDiag(diag) << AT.getRepresentativeTypeName(S.Context) << ExprTy
+                        << IsEnum << CSR << E->getSourceRange(),
+          E->getLocStart(), /*IsStringLocation*/ false, CSR);
       break;
-
+    }
     case Sema::VAK_Undefined:
     case Sema::VAK_MSVCUndefined:
       EmitFormatDiagnostic(
@@ -3988,13 +3994,13 @@ bool CheckScanfHandler::HandleScanfSpecifier(
                            FixItHint::CreateRemoval(R));
     }
   }
-  
+
   if (!FS.consumesDataArgument()) {
     // FIXME: Technically specifying a precision or field width here
     // makes no sense.  Worth issuing a warning at some point.
     return true;
   }
-  
+
   // Consume the argument.
   unsigned argIndex = FS.getArgIndex();
   if (argIndex < NumDataArgs) {
@@ -4003,7 +4009,7 @@ bool CheckScanfHandler::HandleScanfSpecifier(
       // function if we encounter some other error.
     CoveredArgs.set(argIndex);
   }
-  
+
   // Check the length modifier is valid with the given conversion specifier.
   if (!FS.hasValidLengthModifier(S.getASTContext().getTargetInfo()))
     HandleInvalidLengthModifier(FS, CS, startSpecifier, specifierLen,
@@ -4020,21 +4026,28 @@ bool CheckScanfHandler::HandleScanfSpecifier(
   // The remaining checks depend on the data arguments.
   if (HasVAListArg)
     return true;
-  
+
   if (!CheckNumArgs(FS, CS, startSpecifier, specifierLen, argIndex))
     return false;
-  
+
   // Check that the argument type matches the format specifier.
   const Expr *Ex = getDataArg(argIndex);
   if (!Ex)
     return true;
 
   const analyze_format_string::ArgType &AT = FS.getArgType(S.Context);
-  if (AT.isValid() && !AT.matchesType(S.Context, Ex->getType())) {
+  analyze_format_string::ArgType::MatchKind match =
+      AT.matchesType(S.Context, Ex->getType());
+  if (AT.isValid() && match != analyze_format_string::ArgType::Match) {
     ScanfSpecifier fixedFS = FS;
-    bool success = fixedFS.fixType(Ex->getType(),
-                                   Ex->IgnoreImpCasts()->getType(),
-                                   S.getLangOpts(), S.Context);
+    bool success =
+        fixedFS.fixType(Ex->getType(), Ex->IgnoreImpCasts()->getType(),
+                        S.getLangOpts(), S.Context);
+
+    unsigned diag = diag::warn_format_conversion_argument_type_mismatch;
+    if (match == analyze_format_string::ArgType::NoMatchPedantic) {
+      diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
+    }
 
     if (success) {
       // Get the fix string from the fixed format specifier.
@@ -4043,23 +4056,20 @@ bool CheckScanfHandler::HandleScanfSpecifier(
       fixedFS.toString(os);
 
       EmitFormatDiagnostic(
-        S.PDiag(diag::warn_format_conversion_argument_type_mismatch)
-          << AT.getRepresentativeTypeName(S.Context) << Ex->getType() << false
-          << Ex->getSourceRange(),
-        Ex->getLocStart(),
-        /*IsStringLocation*/false,
-        getSpecifierRange(startSpecifier, specifierLen),
-        FixItHint::CreateReplacement(
+          S.PDiag(diag) << AT.getRepresentativeTypeName(S.Context)
+                        << Ex->getType() << false << Ex->getSourceRange(),
+          Ex->getLocStart(),
+          /*IsStringLocation*/ false,
           getSpecifierRange(startSpecifier, specifierLen),
-          os.str()));
+          FixItHint::CreateReplacement(
+              getSpecifierRange(startSpecifier, specifierLen), os.str()));
     } else {
       EmitFormatDiagnostic(
-        S.PDiag(diag::warn_format_conversion_argument_type_mismatch)
-          << AT.getRepresentativeTypeName(S.Context) << Ex->getType() << false
-          << Ex->getSourceRange(),
-        Ex->getLocStart(),
-        /*IsStringLocation*/false,
-        getSpecifierRange(startSpecifier, specifierLen));
+          S.PDiag(diag) << AT.getRepresentativeTypeName(S.Context)
+                        << Ex->getType() << false << Ex->getSourceRange(),
+          Ex->getLocStart(),
+          /*IsStringLocation*/ false,
+          getSpecifierRange(startSpecifier, specifierLen));
     }
   }