Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -482,6 +482,11 @@ "%0 will always overflow destination buffer">, InGroup>; +def warn_sprintf_chk_overflow : Warning< + "call will always overflow destination buffer as it requires at least %0 " + "bytes whereas %1 are provided">, + InGroup; + /// main() // static main() is not an error in C, just in C++. def warn_static_main : Warning<"'main' should not be declared static">, Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -8740,6 +8740,7 @@ enum FormatStringType { FST_Scanf, FST_Printf, + FST_Sprintf, // extra check on top of Printf FST_NSString, FST_Strftime, FST_Strfmon, @@ -8767,13 +8768,15 @@ bool IsCXXMember, VariadicCallType CallType, SourceLocation Loc, SourceRange Range, - llvm::SmallBitVector &CheckedVarArgs); + llvm::SmallBitVector &CheckedVarArgs, + bool isSprintf); bool CheckFormatArguments(ArrayRef Args, bool HasVAListArg, unsigned format_idx, unsigned firstDataArg, FormatStringType Type, VariadicCallType CallType, SourceLocation Loc, SourceRange range, - llvm::SmallBitVector &CheckedVarArgs); + llvm::SmallBitVector &CheckedVarArgs, + bool isSprintf); void CheckAbsoluteValueFunction(const CallExpr *Call, const FunctionDecl *FDecl, Index: lib/Sema/SemaChecking.cpp =================================================================== --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -1313,12 +1313,16 @@ // Printf and scanf checking. llvm::SmallBitVector CheckedVarArgs; if (FDecl) { + bool isSprintf = false; + if (auto *CastFDecl = dyn_cast(FDecl)) + if (CastFDecl->getBuiltinID() == Builtin::BI__builtin___sprintf_chk) + isSprintf = true; for (const auto *I : FDecl->specific_attrs()) { // Only create vector if there are format attributes. CheckedVarArgs.resize(Args.size()); CheckFormatArguments(I, Args, IsMemberFunction, CallType, Loc, Range, - CheckedVarArgs); + CheckedVarArgs, isSprintf); } } @@ -3050,12 +3054,14 @@ bool IsCXXMember, VariadicCallType CallType, SourceLocation Loc, SourceRange Range, - llvm::SmallBitVector &CheckedVarArgs) { + llvm::SmallBitVector &CheckedVarArgs, + bool isSprintf) { FormatStringInfo FSI; if (getFormatStringInfo(Format, IsCXXMember, &FSI)) return CheckFormatArguments(Args, FSI.HasVAListArg, FSI.FormatIdx, FSI.FirstDataArg, GetFormatStringType(Format), - CallType, Loc, Range, CheckedVarArgs); + CallType, Loc, Range, CheckedVarArgs, + isSprintf); return false; } @@ -3064,13 +3070,17 @@ unsigned firstDataArg, FormatStringType Type, VariadicCallType CallType, SourceLocation Loc, SourceRange Range, - llvm::SmallBitVector &CheckedVarArgs) { + llvm::SmallBitVector &CheckedVarArgs, + bool isSprintf) { // CHECK: printf/scanf-like function is called with no format string. if (format_idx >= Args.size()) { Diag(Loc, diag::warn_missing_format_string) << Range; return false; } + if (isSprintf) + Type = FST_Sprintf; + const Expr *OrigFormatExpr = Args[format_idx]->IgnoreParenCasts(); // CHECK: format string is not a string literal. @@ -3523,7 +3533,7 @@ } } -//===--- CHECK: Printf format string checking ------------------------------===// +//===--- CHECK: Printf format string checking -----------------------------===// namespace { class CheckPrintfHandler : public CheckFormatHandler { @@ -3536,12 +3546,15 @@ ArrayRef Args, unsigned formatIdx, bool inFunctionCall, Sema::VariadicCallType CallType, - llvm::SmallBitVector &CheckedVarArgs) + llvm::SmallBitVector &CheckedVarArgs, + bool isSprintf) : CheckFormatHandler(s, fexpr, origFormatExpr, firstDataArg, numDataArgs, beg, hasVAListArg, Args, formatIdx, inFunctionCall, CallType, CheckedVarArgs), - ObjCContext(isObjC) - {} + ObjCContext(isObjC), isSprintf(isSprintf) + { + SprintfRequiredStorage = 1; + } bool HandleInvalidPrintfConversionSpecifier( @@ -3557,6 +3570,10 @@ unsigned SpecifierLen, const Expr *E); + bool HandleSprintfSpecifier(const analyze_printf::PrintfSpecifier &FS, + const char *startSpecifier, + unsigned specifierLen); + bool HandleAmount(const analyze_format_string::OptionalAmount &Amt, unsigned k, const char *startSpecifier, unsigned specifierLen); void HandleInvalidAmount(const analyze_printf::PrintfSpecifier &FS, @@ -3572,8 +3589,10 @@ const char *startSpecifier, unsigned specifierLen); bool checkForCStrMembers(const analyze_printf::ArgType &AT, const Expr *E); - -}; +private: + bool isSprintf; + uint64_t SprintfRequiredStorage; +}; } bool CheckPrintfHandler::HandleInvalidPrintfConversionSpecifier( @@ -3916,7 +3935,171 @@ if (!Arg) return true; - return checkFormatExpr(FS, startSpecifier, specifierLen, Arg); + + if (checkFormatExpr(FS, startSpecifier, specifierLen, Arg)) { + if (isSprintf) + return HandleSprintfSpecifier(FS, startSpecifier, specifierLen); + else + return true; + } + return false; +} + +static const ConstantArrayType *getConstArrayType(Sema &S, const Expr *Array) { + return S.Context.getAsConstantArrayType(Array->getType()); +} + +// Analyses the data arguments to sprintf to warn the user of memory overflows. +// Mainly handles integer and string literals with their specifiers and flags +// with little support for floating point values. None specifier text is also +// not considered towards the storage space required for the string. +bool CheckPrintfHandler::HandleSprintfSpecifier( + const analyze_printf::PrintfSpecifier &FS, + const char *startSpecifier, unsigned specifierLen) { + + using namespace analyze_format_string; + using namespace analyze_printf; + + const PrintfConversionSpecifier &CS = FS.getConversionSpecifier(); + ConversionSpecifier::Kind CSKind = CS.getKind(); + OptionalAmount Precision = FS.getPrecision(); + OptionalAmount Width = FS.getFieldWidth(); + // Offset the argument index to the data items to be printed. + unsigned Idx = 4 + FS.getArgIndex(); + const Expr* Data = Args[Idx]->IgnoreImplicit(); + uint64_t StorageBufSize = 0; + uint64_t RequiredStorage = 0; + + if (auto *StorageBufTy = getConstArrayType(S, Args[0]->IgnoreImplicit())) + StorageBufSize = StorageBufTy->getSize().getZExtValue(); + else + return true; + + switch (CSKind) { + default: + // We can't handle eArg, EArg, fArg, FArg, gArg, GArg, nArg + break; + case ConversionSpecifier::aArg: + case ConversionSpecifier::AArg: { + if (isa(Data)) { + llvm::APFloat Value = cast(Data)->getValue(); + char TempBuf[32]; + unsigned HexDigits = 0; + + if (Precision.getHowSpecified() == OptionalAmount::Constant) + HexDigits = Precision.getConstantAmount(); + + // Get the number of digits used to print the value. + unsigned NumDigits = + Value.convertToHexString(TempBuf, HexDigits, false, + llvm::APFloat::rmNearestTiesToEven); + RequiredStorage += NumDigits; + } + break; + } + case ConversionSpecifier::cArg: + case ConversionSpecifier::CArg: + ++RequiredStorage; + break; + case ConversionSpecifier::dArg: + case ConversionSpecifier::DArg: + case ConversionSpecifier::iArg: + case ConversionSpecifier::uArg: + case ConversionSpecifier::UArg: { + if (Data->isEvaluatable(S.getASTContext())) { + llvm::APSInt Result = llvm::APSInt(64); + if (CSKind == ConversionSpecifier::uArg || + CSKind == ConversionSpecifier::UArg) + Result = llvm::APSInt(64, false); + if (Data->EvaluateAsInt(Result, S.getASTContext())) { + std::string DataStr = Result.toString(10, true); + RequiredStorage += DataStr.length(); + } + } + break; + } + case ConversionSpecifier::pArg: { + // When using the %p specifier, all platforms appear to use hex digits, this + // is prepended with '0x' on BSD, Linux and OSX. + const TargetInfo &TI = S.Context.getTargetInfo(); + RequiredStorage += TI.getPointerWidth(0) / 4; + if (TI.getTriple().isOSDarwin() || + TI.getTriple().isOSLinux() || + TI.getTriple().isOSNetBSD() || + TI.getTriple().isOSOpenBSD() || + TI.getTriple().isOSFreeBSD() || + TI.getTriple().isOSDragonFly()) + RequiredStorage += 2; + break; + } + case ConversionSpecifier::sArg: + case ConversionSpecifier::SArg: + case ConversionSpecifier::ZArg: + if (auto SL = dyn_cast(Data)) + RequiredStorage += SL->getByteLength(); + else if (auto DataBuf = getConstArrayType(S, Data)) { + uint64_t BufSize = DataBuf->getSize().getZExtValue(); + RequiredStorage += BufSize; + } + break; + case ConversionSpecifier::oArg: + case ConversionSpecifier::OArg: + case ConversionSpecifier::xArg: + case ConversionSpecifier::XArg: { + unsigned Radix = 16; + if (CSKind == ConversionSpecifier::oArg || + CSKind == ConversionSpecifier::OArg) + Radix = 8; + if (isa(Data)) { + // Create a real string to use its size. + std::string DataStr = + cast(Data)->getValue().toString(Radix, true); + RequiredStorage += DataStr.length(); + // 0x will can be prepended for oct and hex values. + if (FS.hasAlternativeForm()) + RequiredStorage += 2; + } + break; + } + } + + // An extra will be required for +/- + if (FS.hasPlusPrefix() || FS.hasSpacePrefix()) + ++RequiredStorage; + + // The calculated sizes could be smaller than sizes specified by the user + // in their format string. + if (CS.isAnyIntArg()) { + // For integers, the precision modifier specifies how many digits to be + // written so this may overwrite the calculated size. + if (Precision.getHowSpecified() == OptionalAmount::Constant) { + if (RequiredStorage <= Precision.getConstantAmount()) + RequiredStorage = Precision.getConstantAmount(); + } + } + + // Width specifies the minimum number of characters to be printed. + if (Width.getHowSpecified() == OptionalAmount::Constant) { + if (RequiredStorage <= Width.getConstantAmount()) + RequiredStorage = Width.getConstantAmount(); + } + + // Update the total required. + SprintfRequiredStorage += RequiredStorage; + + if (StorageBufSize < SprintfRequiredStorage) { + std::string Limit = llvm::APInt(64, StorageBufSize).toString(10, false); + std::string Required = + llvm::APInt(64, SprintfRequiredStorage).toString(10, false); + S.Diag(Args[0]->getLocStart(), diag::warn_sprintf_chk_overflow) + << getSpecifierRange(startSpecifier, specifierLen) + << Data->getSourceRange() + << Required + << Limit; + return false; + } + + return true; } static bool requiresParensToAddCast(const Expr *E) { @@ -4493,11 +4676,14 @@ } if (Type == FST_Printf || Type == FST_NSString || - Type == FST_FreeBSDKPrintf || Type == FST_OSTrace) { + Type == FST_FreeBSDKPrintf || Type == FST_OSTrace || + Type == FST_Sprintf) { CheckPrintfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg, - numDataArgs, (Type == FST_NSString || Type == FST_OSTrace), + numDataArgs, + (Type == FST_NSString || Type == FST_OSTrace), Str, HasVAListArg, Args, format_idx, - inFunctionCall, CallType, CheckedVarArgs); + inFunctionCall, CallType, CheckedVarArgs, + Type == FST_Sprintf); if (!analyze_format_string::ParsePrintfString(H, Str, Str + StrLen, getLangOpts(), Index: test/Sema/sprintf-chk.cpp =================================================================== --- /dev/null +++ test/Sema/sprintf-chk.cpp @@ -0,0 +1,106 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -fsyntax-only %s +void test() { + + char bufA[4]; + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%d", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%i", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%u", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%x", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%X", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%o", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%s", "foo"); + char addrBuf[19]; + __builtin___sprintf_chk(addrBuf, 0, __builtin_object_size(addrBuf, 0), "%p", + bufA); + + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%d%d", 10, 10); + // expected-warning@-1{{call will always overflow destination buffer as it requires at least 5 bytes whereas 4 are provided}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%i%i", 10, 10); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%u%u", 10, 10); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%x%x", 100, 100); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%X%X", 100, 100); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%o%o", 100, 100); + // expected-warning@-1{{call will always overflow destination buffer}} + + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%d", 1000); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%i", 1000); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%u", 1000); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%x", 10000); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%X", 10000); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%o", 1000); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%a", 10.0); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%A", 10.0); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%s", "foobar"); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%p", bufA); + // expected-warning@-1{{call will always overflow destination buffer}} + + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%+d", 100); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%+d", 10); + + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%3d", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%3i", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%3u", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%3x", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%3X", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%3o", 100); + + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%4d", 100); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%4i", 100); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%4u", 100); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%4x", 100); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%4X", 100); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%4o", 100); + // expected-warning@-1{{call will always overflow destination buffer}} + + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%#x", 15); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%#X", 15); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%#o", 7); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%#x", 100); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%#X", 100); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%#o", 100); + // expected-warning@-1{{call will always overflow destination buffer}} + + + char bufB[7]; + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%a", 0.5); + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%A", 0.5); + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%a", 10.0); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%A", 10.0); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%1.1a", 0.5); + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%1.1A", 0.5); + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%1.2a", 0.5); + // expected-warning@-1{{call will always overflow destination buffer}} + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%1.2A", 0.5); + // expected-warning@-1{{call will always overflow destination buffer}} + + char bufC[5]; + __builtin___sprintf_chk(bufC, 0, __builtin_object_size(bufC, 0), "%s%s", + "foo", "bar"); + // expected-warning@-2{{call will always overflow destination buffer}} + const char bar[] = "foobar"; + __builtin___sprintf_chk(bufC, 0, __builtin_object_size(bufC, 0), "%s", bar); + // expected-warning@-1{{call will always overflow destination buffer}} +}