Author: Nico Weber Date: 2021-10-28T10:41:18-04:00 New Revision: bf87294cd4fa5781aa2fc0954e117712befedf87
URL: https://github.com/llvm/llvm-project/commit/bf87294cd4fa5781aa2fc0954e117712befedf87 DIFF: https://github.com/llvm/llvm-project/commit/bf87294cd4fa5781aa2fc0954e117712befedf87.diff LOG: Revert "[clang] Fortify warning for scanf calls with field width too big." This reverts commit 15e3d39110fa4449be4f56196af3bc81b623f3ab. See https://reviews.llvm.org/D111833#3093629 Added: Modified: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaChecking.cpp Removed: clang/test/Sema/warn-fortify-scanf.c ################################################################################ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index f2dd69bbdbbd..7153b9c66563 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -833,10 +833,6 @@ def warn_fortify_source_format_overflow : Warning< " but format string expands to at least %2">, InGroup<FortifySource>; -def warn_fortify_scanf_overflow : Warning< - "'%0' may overflow; destination buffer in argument %1 has size " - "%2, but the corresponding field width plus NUL byte is %3">, - InGroup<FortifySource>; /// main() // static main() is not an error in C, just in C++. diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index e11966020ae9..147f50aeed97 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -408,50 +408,6 @@ static bool SemaBuiltinCallWithStaticChain(Sema &S, CallExpr *BuiltinCall) { namespace { -class ScanfDiagnosticFormatHandler - : public analyze_format_string::FormatStringHandler { - // Accepts the argument index (relative to the first destination index) of the - // argument whose size we want. - using ComputeSizeFunction = - llvm::function_ref<Optional<llvm::APSInt>(unsigned)>; - - // Accepts the argument index (relative to the first destination index), the - // destination size, and the source size). - using DiagnoseFunction = - llvm::function_ref<void(unsigned, unsigned, unsigned)>; - - ComputeSizeFunction ComputeSizeArgument; - DiagnoseFunction Diagnose; - -public: - ScanfDiagnosticFormatHandler(ComputeSizeFunction ComputeSizeArgument, - DiagnoseFunction Diagnose) - : ComputeSizeArgument(ComputeSizeArgument), Diagnose(Diagnose) {} - - bool HandleScanfSpecifier(const analyze_scanf::ScanfSpecifier &FS, - const char *StartSpecifier, - unsigned specifierLen) override { - auto OptionalFW = FS.getFieldWidth(); - if (OptionalFW.getHowSpecified() != - analyze_format_string::OptionalAmount::HowSpecified::Constant) - return true; - - // We have to write the data plus a NUL byte. - unsigned SourceSize = OptionalFW.getConstantAmount() + 1; - - auto DestSizeAPS = ComputeSizeArgument(FS.getArgIndex()); - if (!DestSizeAPS) - return true; - - unsigned DestSize = DestSizeAPS->getZExtValue(); - - if (DestSize < SourceSize) - Diagnose(FS.getArgIndex(), DestSize, SourceSize); - - return true; - } -}; - class EstimateSizeFormatHandler : public analyze_format_string::FormatStringHandler { size_t Size; @@ -659,12 +615,9 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, // (potentially) more strict checking mode. Otherwise, conservatively assume // type 0. int BOSType = 0; - // This check can fail for variadic functions. - if (Index < FD->getNumParams()) { - if (const auto *POS = - FD->getParamDecl(Index)->getAttr<PassObjectSizeAttr>()) - BOSType = POS->getType(); - } + if (const auto *POS = + FD->getParamDecl(Index)->getAttr<PassObjectSizeAttr>()) + BOSType = POS->getType(); const Expr *ObjArg = TheCall->getArg(Index); uint64_t Result; @@ -689,20 +642,6 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, unsigned DiagID = 0; bool IsChkVariant = false; - auto GetFunctionName = [&]() { - StringRef FunctionName = getASTContext().BuiltinInfo.getName(BuiltinID); - // Skim off the details of whichever builtin was called to produce a better - // diagnostic, as it's unlikely that the user wrote the __builtin - // explicitly. - if (IsChkVariant) { - FunctionName = FunctionName.drop_front(std::strlen("__builtin___")); - FunctionName = FunctionName.drop_back(std::strlen("_chk")); - } else if (FunctionName.startswith("__builtin_")) { - FunctionName = FunctionName.drop_front(std::strlen("__builtin_")); - } - return FunctionName; - }; - switch (BuiltinID) { default: return; @@ -722,61 +661,6 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, break; } - case Builtin::BIscanf: - case Builtin::BIfscanf: - case Builtin::BIsscanf: { - unsigned FormatIndex = 1; - unsigned DataIndex = 2; - if (BuiltinID == Builtin::BIscanf) { - FormatIndex = 0; - DataIndex = 1; - } - - const auto *FormatExpr = - TheCall->getArg(FormatIndex)->IgnoreParenImpCasts(); - - const auto *Format = dyn_cast<StringLiteral>(FormatExpr); - if (!Format) - return; - - if (!Format->isAscii() && !Format->isUTF8()) - return; - - auto Diagnose = [&](unsigned ArgIndex, unsigned DestSize, - unsigned SourceSize) { - DiagID = diag::warn_fortify_scanf_overflow; - StringRef FunctionName = GetFunctionName(); - DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, - PDiag(DiagID) - << FunctionName << (ArgIndex + DataIndex + 1) - << DestSize << SourceSize); - }; - - StringRef FormatStrRef = Format->getString(); - auto ShiftedComputeSizeArgument = [&](unsigned Index) { - return ComputeSizeArgument(Index + DataIndex); - }; - ScanfDiagnosticFormatHandler H(ShiftedComputeSizeArgument, Diagnose); - const char *FormatBytes = FormatStrRef.data(); - const ConstantArrayType *T = - Context.getAsConstantArrayType(Format->getType()); - assert(T && "String literal not of constant array type!"); - size_t TypeSize = T->getSize().getZExtValue(); - - // In case there's a null byte somewhere. - size_t StrLen = - std::min(std::max(TypeSize, size_t(1)) - 1, FormatStrRef.find(0)); - - analyze_format_string::ParseScanfString(H, FormatBytes, - FormatBytes + StrLen, getLangOpts(), - Context.getTargetInfo()); - - // Unlike the other cases, in this one we have already issued the diagnostic - // here, so no need to continue (because unlike the other cases, here the - // diagnostic refers to the argument number). - return; - } - case Builtin::BIsprintf: case Builtin::BI__builtin___sprintf_chk: { size_t FormatIndex = BuiltinID == Builtin::BIsprintf ? 1 : 3; @@ -887,7 +771,15 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, SourceSize.getValue().ule(DestinationSize.getValue())) return; - StringRef FunctionName = GetFunctionName(); + StringRef FunctionName = getASTContext().BuiltinInfo.getName(BuiltinID); + // Skim off the details of whichever builtin was called to produce a better + // diagnostic, as it's unlikely that the user wrote the __builtin explicitly. + if (IsChkVariant) { + FunctionName = FunctionName.drop_front(std::strlen("__builtin___")); + FunctionName = FunctionName.drop_back(std::strlen("_chk")); + } else if (FunctionName.startswith("__builtin_")) { + FunctionName = FunctionName.drop_front(std::strlen("__builtin_")); + } SmallString<16> DestinationStr; SmallString<16> SourceStr; diff --git a/clang/test/Sema/warn-fortify-scanf.c b/clang/test/Sema/warn-fortify-scanf.c deleted file mode 100644 index 99a3273bec3f..000000000000 --- a/clang/test/Sema/warn-fortify-scanf.c +++ /dev/null @@ -1,45 +0,0 @@ -// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify - -typedef struct _FILE FILE; -extern int scanf(const char *format, ...); -extern int fscanf(FILE *f, const char *format, ...); -extern int sscanf(const char *input, const char *format, ...); - -void call_scanf() { - char buf10[10]; - char buf20[20]; - char buf30[30]; - scanf("%4s %5s %10s", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 4 has size 10, but the corresponding field width plus NUL byte is 11}} - scanf("%4s %5s %11s", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 4 has size 10, but the corresponding field width plus NUL byte is 12}} - scanf("%4s %5s %9s", buf20, buf30, buf10); - scanf("%20s %5s %9s", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 2 has size 20, but the corresponding field width plus NUL byte is 21}} - scanf("%21s %5s %9s", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 2 has size 20, but the corresponding field width plus NUL byte is 22}} - scanf("%19s %5s %9s", buf20, buf30, buf10); - scanf("%19s %29s %9s", buf20, buf30, buf10); -} - -void call_sscanf() { - char buf10[10]; - char buf20[20]; - char buf30[30]; - sscanf("a b c", "%4s %5s %10s", buf20, buf30, buf10); // expected-warning {{'sscanf' may overflow; destination buffer in argument 5 has size 10, but the corresponding field width plus NUL byte is 11}} - sscanf("a b c", "%4s %5s %11s", buf20, buf30, buf10); // expected-warning {{'sscanf' may overflow; destination buffer in argument 5 has size 10, but the corresponding field width plus NUL byte is 12}} - sscanf("a b c", "%4s %5s %9s", buf20, buf30, buf10); - sscanf("a b c", "%20s %5s %9s", buf20, buf30, buf10); // expected-warning {{'sscanf' may overflow; destination buffer in argument 3 has size 20, but the corresponding field width plus NUL byte is 21}} - sscanf("a b c", "%21s %5s %9s", buf20, buf30, buf10); // expected-warning {{'sscanf' may overflow; destination buffer in argument 3 has size 20, but the corresponding field width plus NUL byte is 22}} - sscanf("a b c", "%19s %5s %9s", buf20, buf30, buf10); - sscanf("a b c", "%19s %29s %9s", buf20, buf30, buf10); -} - -void call_fscanf() { - char buf10[10]; - char buf20[20]; - char buf30[30]; - fscanf(0, "%4s %5s %10s", buf20, buf30, buf10); // expected-warning {{'fscanf' may overflow; destination buffer in argument 5 has size 10, but the corresponding field width plus NUL byte is 11}} - fscanf(0, "%4s %5s %11s", buf20, buf30, buf10); // expected-warning {{'fscanf' may overflow; destination buffer in argument 5 has size 10, but the corresponding field width plus NUL byte is 12}} - fscanf(0, "%4s %5s %9s", buf20, buf30, buf10); - fscanf(0, "%20s %5s %9s", buf20, buf30, buf10); // expected-warning {{'fscanf' may overflow; destination buffer in argument 3 has size 20, but the corresponding field width plus NUL byte is 21}} - fscanf(0, "%21s %5s %9s", buf20, buf30, buf10); // expected-warning {{'fscanf' may overflow; destination buffer in argument 3 has size 20, but the corresponding field width plus NUL byte is 22}} - fscanf(0, "%19s %5s %9s", buf20, buf30, buf10); - fscanf(0, "%19s %29s %9s", buf20, buf30, buf10); -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
