r231211 - Add a format warning for "%p" with non-void* args
Seth Cantrell
seth.cantrell at gmail.com
Tue Mar 3 19:12:10 PST 2015
Author: socantre
Date: Tue Mar 3 21:12:10 2015
New Revision: 231211
URL: http://llvm.org/viewvc/llvm-project?rev=231211&view=rev
Log:
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.
Modified:
cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Analysis/FormatString.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/SemaCXX/format-strings-0x.cpp
Modified: cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/FormatString.h?rev=231211&r1=231210&r2=231211&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/FormatString.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/FormatString.h Tue Mar 3 21:12:10 2015
@@ -231,6 +231,9 @@ class ArgType {
public:
enum Kind { UnknownTy, InvalidTy, SpecificTy, ObjCPointerTy, CPointerTy,
AnyCharTy, CStrTy, WCStrTy, WIntTy };
+
+ enum MatchKind { NoMatch = 0, Match = 1, NoMatchPedantic };
+
private:
const Kind K;
QualType T;
@@ -254,7 +257,7 @@ public:
return Res;
}
- bool matchesType(ASTContext &C, QualType argTy) const;
+ MatchKind matchesType(ASTContext &C, QualType argTy) const;
QualType getRepresentativeType(ASTContext &C) const;
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=231211&r1=231210&r2=231211&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Mar 3 21:12:10 2015
@@ -551,6 +551,7 @@ def FormatInvalidSpecifier : DiagGroup<"
def FormatSecurity : DiagGroup<"format-security">;
def FormatNonStandard : DiagGroup<"format-non-iso">;
def FormatY2K : DiagGroup<"format-y2k">;
+def FormatPedantic : DiagGroup<"format-pedantic">;
def Format : DiagGroup<"format",
[FormatExtraArgs, FormatZeroLength, NonNull,
FormatSecurity, FormatY2K, FormatInvalidSpecifier]>,
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=231211&r1=231210&r2=231211&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Mar 3 21:12:10 2015
@@ -6644,6 +6644,10 @@ def warn_format_conversion_argument_type
"format specifies type %0 but the argument has "
"%select{type|underlying type}2 %1">,
InGroup<Format>;
+def warn_format_conversion_argument_type_mismatch_pedantic : Extension<
+ "format specifies type %0 but the argument has "
+ "%select{type|underlying type}2 %1">,
+ InGroup<FormatPedantic>;
def warn_format_argument_needs_cast : Warning<
"%select{values of type|enum values with underlying type}2 '%0' should not "
"be used as format arguments; add an explicit cast to %1 instead">,
Modified: cfe/trunk/lib/Analysis/FormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/FormatString.cpp?rev=231211&r1=231210&r2=231211&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/FormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/FormatString.cpp Tue Mar 3 21:12:10 2015
@@ -256,16 +256,17 @@ clang::analyze_format_string::ParseLengt
// Methods on ArgType.
//===----------------------------------------------------------------------===//
-bool ArgType::matchesType(ASTContext &C, QualType argTy) const {
+clang::analyze_format_string::ArgType::MatchKind
+ArgType::matchesType(ASTContext &C, QualType argTy) const {
if (Ptr) {
// It has to be a pointer.
const PointerType *PT = argTy->getAs<PointerType>();
if (!PT)
- return false;
+ return NoMatch;
// We cannot write through a const qualified pointer.
if (PT->getPointeeType().isConstQualified())
- return false;
+ return NoMatch;
argTy = PT->getPointeeType();
}
@@ -275,8 +276,8 @@ bool ArgType::matchesType(ASTContext &C,
llvm_unreachable("ArgType must be valid");
case UnknownTy:
- return true;
-
+ return Match;
+
case AnyCharTy: {
if (const EnumType *ETy = argTy->getAs<EnumType>())
argTy = ETy->getDecl()->getIntegerType();
@@ -289,18 +290,18 @@ bool ArgType::matchesType(ASTContext &C,
case BuiltinType::SChar:
case BuiltinType::UChar:
case BuiltinType::Char_U:
- return true;
+ return Match;
}
- return false;
+ return NoMatch;
}
-
+
case SpecificTy: {
if (const EnumType *ETy = argTy->getAs<EnumType>())
argTy = ETy->getDecl()->getIntegerType();
argTy = C.getCanonicalType(argTy).getUnqualifiedType();
if (T == argTy)
- return true;
+ return Match;
// Check for "compatible types".
if (const BuiltinType *BT = argTy->getAs<BuiltinType>())
switch (BT->getKind()) {
@@ -309,32 +310,33 @@ bool ArgType::matchesType(ASTContext &C,
case BuiltinType::Char_S:
case BuiltinType::SChar:
case BuiltinType::Char_U:
- case BuiltinType::UChar:
- return T == C.UnsignedCharTy || T == C.SignedCharTy;
+ case BuiltinType::UChar:
+ return T == C.UnsignedCharTy || T == C.SignedCharTy ? Match
+ : NoMatch;
case BuiltinType::Short:
- return T == C.UnsignedShortTy;
+ return T == C.UnsignedShortTy ? Match : NoMatch;
case BuiltinType::UShort:
- return T == C.ShortTy;
+ return T == C.ShortTy ? Match : NoMatch;
case BuiltinType::Int:
- return T == C.UnsignedIntTy;
+ return T == C.UnsignedIntTy ? Match : NoMatch;
case BuiltinType::UInt:
- return T == C.IntTy;
+ return T == C.IntTy ? Match : NoMatch;
case BuiltinType::Long:
- return T == C.UnsignedLongTy;
+ return T == C.UnsignedLongTy ? Match : NoMatch;
case BuiltinType::ULong:
- return T == C.LongTy;
+ return T == C.LongTy ? Match : NoMatch;
case BuiltinType::LongLong:
- return T == C.UnsignedLongLongTy;
+ return T == C.UnsignedLongLongTy ? Match : NoMatch;
case BuiltinType::ULongLong:
- return T == C.LongLongTy;
+ return T == C.LongLongTy ? Match : NoMatch;
}
- return false;
+ return NoMatch;
}
case CStrTy: {
const PointerType *PT = argTy->getAs<PointerType>();
if (!PT)
- return false;
+ return NoMatch;
QualType pointeeTy = PT->getPointeeType();
if (const BuiltinType *BT = pointeeTy->getAs<BuiltinType>())
switch (BT->getKind()) {
@@ -343,50 +345,56 @@ bool ArgType::matchesType(ASTContext &C,
case BuiltinType::UChar:
case BuiltinType::Char_S:
case BuiltinType::SChar:
- return true;
+ return Match;
default:
break;
}
- return false;
+ return NoMatch;
}
case WCStrTy: {
const PointerType *PT = argTy->getAs<PointerType>();
if (!PT)
- return false;
+ return NoMatch;
QualType pointeeTy =
C.getCanonicalType(PT->getPointeeType()).getUnqualifiedType();
- return pointeeTy == C.getWideCharType();
+ return pointeeTy == C.getWideCharType() ? Match : NoMatch;
}
-
+
case WIntTy: {
-
+
QualType PromoArg =
argTy->isPromotableIntegerType()
? C.getPromotedIntegerType(argTy) : argTy;
-
+
QualType WInt = C.getCanonicalType(C.getWIntType()).getUnqualifiedType();
PromoArg = C.getCanonicalType(PromoArg).getUnqualifiedType();
-
+
// If the promoted argument is the corresponding signed type of the
// wint_t type, then it should match.
if (PromoArg->hasSignedIntegerRepresentation() &&
C.getCorrespondingUnsignedType(PromoArg) == WInt)
- return true;
+ return Match;
- return WInt == PromoArg;
+ return WInt == PromoArg ? Match : NoMatch;
}
case CPointerTy:
- return argTy->isPointerType() || argTy->isObjCObjectPointerType() ||
- argTy->isBlockPointerType() || argTy->isNullPtrType();
+ if (argTy->isVoidPointerType()) {
+ return Match;
+ } if (argTy->isPointerType() || argTy->isObjCObjectPointerType() ||
+ argTy->isBlockPointerType() || argTy->isNullPtrType()) {
+ return NoMatchPedantic;
+ } else {
+ return NoMatch;
+ }
case ObjCPointerTy: {
if (argTy->getAs<ObjCObjectPointerType>() ||
argTy->getAs<BlockPointerType>())
- return true;
-
+ return Match;
+
// Handle implicit toll-free bridging.
if (const PointerType *PT = argTy->getAs<PointerType>()) {
// Things such as CFTypeRef are really just opaque pointers
@@ -395,9 +403,9 @@ bool ArgType::matchesType(ASTContext &C,
// structs can be toll-free bridged, we just accept them all.
QualType pointee = PT->getPointeeType();
if (pointee->getAsStructureType() || pointee->isVoidType())
- return true;
+ return Match;
}
- return false;
+ return NoMatch;
}
}
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=231211&r1=231210&r2=231211&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Mar 3 21:12:10 2015
@@ -3669,8 +3669,11 @@ CheckPrintfHandler::checkFormatExpr(cons
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(cons
// 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::HandleScanfSpeci
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::HandleScanfSpeci
// 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::HandleScanfSpeci
// 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::HandleScanfSpeci
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));
}
}
Modified: cfe/trunk/test/SemaCXX/format-strings-0x.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/format-strings-0x.cpp?rev=231211&r1=231210&r2=231211&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/format-strings-0x.cpp (original)
+++ cfe/trunk/test/SemaCXX/format-strings-0x.cpp Tue Mar 3 21:12:10 2015
@@ -8,6 +8,9 @@ extern int printf(const char *restrict,
void f(char **sp, float *fp) {
scanf("%as", sp); // expected-warning{{format specifies type 'float *' but the argument has type 'char **'}}
+ printf("%p", sp); // expected-warning{{format specifies type 'void *' but the argument has type 'char **'}}
+ scanf("%p", sp); // expected-warning{{format specifies type 'void **' but the argument has type 'char **'}}
+
printf("%a", 1.0);
scanf("%afoobar", fp);
printf(nullptr);
More information about the cfe-commits
mailing list