[clang] bf87294 - Revert "[clang] Fortify warning for scanf calls with field width too big."
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 28 07:42:07 PDT 2021
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);
-}
More information about the cfe-commits
mailing list