There are some cases that weren't correctly put into the new FormatPedantic group, but instead reported through the normal Format group. Fixed some in r231242. Could you double-check that there aren't more incorrect classifications?
On Wed, Mar 4, 2015 at 4:12 AM, Seth Cantrell <[email protected]> wrote: > 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); > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
