From b480296e6cb4a2cc5d5f588e35d4148f8770fea9 Mon Sep 17 00:00:00 2001 From: Seth Cantrell Date: Wed, 4 Mar 2015 03:12:10 +0000 Subject: 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 --- clang/lib/Sema/SemaChecking.cpp | 72 +++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 31 deletions(-) (limited to 'clang/lib/Sema/SemaChecking.cpp') 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)); } } -- cgit v1.2.3