https://github.com/bozicrHT updated https://github.com/llvm/llvm-project/pull/204337
From dd12a31fc6ce5d035a12258c29072faa4b13fdca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <[email protected]> Date: Wed, 17 Jun 2026 13:17:28 +0200 Subject: [PATCH 1/5] [Clang][Sema] Add fortify warnings for fread, fwrite, and fgets --- clang/include/clang/Basic/Builtins.td | 5 +++ .../clang/Basic/DiagnosticSemaKinds.td | 5 +++ clang/lib/Sema/SemaChecking.cpp | 38 ++++++++++++++++++- clang/test/Sema/warn-fortify-source.c | 14 +++++++ 4 files changed, 61 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td index 0aec57f2013013..26faebe9b43430 100644 --- a/clang/include/clang/Basic/Builtins.td +++ b/clang/include/clang/Basic/Builtins.td @@ -3570,6 +3570,11 @@ def Fwrite : LibBuiltin<"stdio.h"> { let Prototype = "size_t(void const*, size_t, size_t, FILE*)"; } +def Fgets : LibBuiltin<"stdio.h"> { + let Spellings = ["fgets"]; + let Prototype = "char*(char*, int, FILE*)"; +} + // C99 ctype.h def IsAlNum : LibBuiltin<"ctype.h"> { diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index cfb2ee33682013..16f08ebd2e0064 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -1026,6 +1026,11 @@ def warn_fortify_scanf_overflow : Warning< "%2, but the corresponding specifier may require size %3">, InGroup<FortifySource>; +def warn_fortify_source_overread : Warning< + "'%0' will always read past the source buffer; source buffer has " + "size %1, but size argument is %2">, + InGroup<FortifySource>; + def err_function_start_invalid_type: Error< "argument must be a function">; diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 5c831e6cdebce0..a7a8a82f00d238 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1219,6 +1219,25 @@ class FortifiedBufferChecker { return std::nullopt; } + std::optional<llvm::APSInt> + ComputeExplicitObjectSizeArgumentProduct(unsigned LIndex, unsigned RIndex) { + auto L = ComputeExplicitObjectSizeArgument(LIndex); + auto R = ComputeExplicitObjectSizeArgument(RIndex); + if (!L || !R) + return std::nullopt; + + unsigned W = + 2 * std::max({L->getBitWidth(), R->getBitWidth(), SizeTypeWidth}); + + llvm::APSInt LE = L->extOrTrunc(W); + llvm::APSInt RE = R->extOrTrunc(W); + + LE.setIsUnsigned(true); + RE.setIsUnsigned(true); + + return LE * RE; + }; + std::optional<llvm::APSInt> ComputeStrLenArgument(unsigned Index) { std::optional<unsigned> IndexOptional = TranslateIndex(Index); if (!IndexOptional) @@ -1496,7 +1515,24 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, Checker.checkSourceOverread(/*SrcArgIdx=*/0, /*SizeArgIdx=*/2); break; } - + case Builtin::BIfread: { + DiagID = diag::warn_fortify_source_overflow; + SourceSize = Checker.ComputeExplicitObjectSizeArgumentProduct(1, 2); + DestinationSize = Checker.ComputeSizeArgument(0); + break; + } + case Builtin::BIfwrite: { + DiagID = diag::warn_fortify_source_overread; + SourceSize = Checker.ComputeExplicitObjectSizeArgumentProduct(1, 2); + DestinationSize = Checker.ComputeSizeArgument(0); + break; + } + case Builtin::BIfgets: { + DiagID = diag::warn_fortify_source_size_mismatch; + SourceSize = Checker.ComputeExplicitObjectSizeArgument(1); + DestinationSize = Checker.ComputeSizeArgument(0); + break; + } // memchr(buf, val, size) case Builtin::BImemchr: case Builtin::BI__builtin_memchr: { diff --git a/clang/test/Sema/warn-fortify-source.c b/clang/test/Sema/warn-fortify-source.c index 0a6c44f59af9ec..0227d6703cfdad 100644 --- a/clang/test/Sema/warn-fortify-source.c +++ b/clang/test/Sema/warn-fortify-source.c @@ -9,6 +9,8 @@ // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS -fexperimental-new-constant-interpreter typedef unsigned long size_t; +typedef struct _IO_FILE FILE; + #ifdef __cplusplus extern "C" { @@ -23,6 +25,10 @@ void *memcpy(void *dst, const void *src, size_t c); #endif void bcopy(const void *src, void *dst, size_t n); void bzero(void *dst, size_t n); +size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream); +size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream); +char *fgets(char *s, int size, FILE *stream); + #ifdef __cplusplus } @@ -120,6 +126,14 @@ void call_bcopy_bzero(void) { __builtin_bzero(dst, 11); // expected-warning {{'bzero' will always overflow; destination buffer has size 10, but size argument is 11}} } +void call_fread_fwrite_fgets(FILE *fp) { + char src[4]; + fread(src, 2, 3, fp); // expected-warning {{'fread' will always overflow; destination buffer has size 4, but size argument is 6}} + fwrite(src, 2, 3, fp); // expected-warning {{'fwrite' will always read past the source buffer; source buffer has size 4, but size argument is 6}} + fgets(src, 5, fp); // expected-warning {{'fgets' size argument is too large; destination buffer has size 4, but size argument is 5}} + +} + void call_snprintf(double d, int n) { char buf[10]; __builtin_snprintf(buf, 10, "merp"); From 56ae89c1768cb853f938507daee113e1eb2eeacf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <[email protected]> Date: Wed, 17 Jun 2026 20:04:55 +0200 Subject: [PATCH 2/5] Fix failing tests --- clang/test/Analysis/std-c-library-functions-arg-constraints.c | 2 ++ clang/test/Analysis/stream-noopen.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.c b/clang/test/Analysis/std-c-library-functions-arg-constraints.c index 2cefa80341fc46..646bfff7d9e4b8 100644 --- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c +++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c @@ -248,6 +248,8 @@ void ARR38_C_F(FILE *file) { // report-warning{{The 1st argument to 'fread' is a buffer with size 4096 but should be a buffer with size equal to or greater than the value of the 2nd argument (which is 4) times the 3rd argument (which is 4096)}} \ // bugpath-warning{{The 1st argument to 'fread' is a buffer with size 4096 but should be a buffer with size equal to or greater than the value of the 2nd argument (which is 4) times the 3rd argument (which is 4096)}} \ // bugpath-note{{The 1st argument to 'fread' is a buffer with size 4096 but should be a buffer with size equal to or greater than the value of the 2nd argument (which is 4) times the 3rd argument (which is 4096)}} + // report-warning@-4{{'fread' will always overflow; destination buffer has size 4096, but size argument is 16384}} + // bugpath-warning@-5{{'fread' will always overflow; destination buffer has size 4096, but size argument is 16384}} } int __two_constrained_args(int, int); diff --git a/clang/test/Analysis/stream-noopen.c b/clang/test/Analysis/stream-noopen.c index 87761b3afb76b4..3f291b61645967 100644 --- a/clang/test/Analysis/stream-noopen.c +++ b/clang/test/Analysis/stream-noopen.c @@ -100,11 +100,13 @@ void test_fgets(char *Buf, int N, FILE *F) { char Buf1[10]; Ret = fgets(Buf1, 11, F); // expected-warning {{The 1st argument to 'fgets' is a buffer with size 10}} + // expected-warning@-1 {{'fgets' size argument is too large; destination buffer has size 10, but size argument is 11}} } void test_fgets_bufsize(FILE *F) { char Buf[10]; fgets(Buf, 11, F); // expected-warning {{The 1st argument to 'fgets' is a buffer with size 10}} + // expected-warning@-1 {{'fgets' size argument is too large; destination buffer has size 10, but size argument is 11}} } void test_fputs(char *Buf, FILE *F) { From 577356d9ceb8018d6dcffcf5a46369efa0dd4486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <[email protected]> Date: Mon, 31 Aug 2026 15:58:57 +0200 Subject: [PATCH 3/5] Fix signed integer evaluation for fgets fortify checks Factor integer argument evaluation into `EvaluateIntegerArgument` so that `fgets` signed int size parameter can be handled without violating the `size_t` invariant of `ComputeExplicitObjectSizeArgument`. --- clang/docs/ReleaseNotes.md | 3 ++ .../clang/Basic/DiagnosticSemaKinds.td | 4 +-- clang/lib/Sema/SemaChecking.cpp | 29 ++++++++++++------- clang/test/Sema/warn-fortify-source.c | 2 +- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index a96b0af93ab0b2..1b373e715914bd 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -391,6 +391,9 @@ features cannot lower the translation-unit ABI level; - Diagnostics for the C++11 range-based for statement now report the correct iterator type in notes for invalid iterator types. +- `-Wfortify-source` now diagnoses calls to `fread`, `fwrite`, and `fgets` + when the requested size exceeds the corresponding buffer. (#GH204337) + - `-Wfortify-source` now warns when the constant-evaluated argument to `umask` has bits set outside `0777`. Those bits are silently discarded by the kernel, so setting them is almost always a typo (matching the diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 16f08ebd2e0064..78e6fb5357e223 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -1027,8 +1027,8 @@ def warn_fortify_scanf_overflow : Warning< InGroup<FortifySource>; def warn_fortify_source_overread : Warning< - "'%0' will always read past the source buffer; source buffer has " - "size %1, but size argument is %2">, + "'%0' will always read past the end of the source buffer; source buffer has " + "size %1, but the size is %2">, InGroup<FortifySource>; def err_function_start_invalid_type: Error< diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index a7a8a82f00d238..34002436c7a1d2 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1174,18 +1174,28 @@ class FortifiedBufferChecker { return NewIndex; } - std::optional<llvm::APSInt> - ComputeExplicitObjectSizeArgument(unsigned Index) { + /// Evaluate the argument at Index as an integer constant while preserving + /// its signedness, or return std::nullopt if it cannot be evaluated. + std::optional<llvm::APSInt> EvaluateIntegerArgument(unsigned Index) { std::optional<unsigned> IndexOptional = TranslateIndex(Index); if (!IndexOptional) return std::nullopt; - unsigned NewIndex = *IndexOptional; + Expr::EvalResult Result; - Expr *SizeArg = TheCall->getArg(NewIndex); - if (!SizeArg->EvaluateAsInt(Result, S.getASTContext())) + Expr *Arg = TheCall->getArg(*IndexOptional); + if (!Arg->EvaluateAsInt(Result, S.getASTContext())) return std::nullopt; - llvm::APSInt Integer = Result.Val.getInt(); - assert(Integer.isUnsigned() && + + return Result.Val.getInt(); + } + + std::optional<llvm::APSInt> + ComputeExplicitObjectSizeArgument(unsigned Index) { + std::optional<llvm::APSInt> Integer = EvaluateIntegerArgument(Index); + if (!Integer) + return std::nullopt; + + assert(Integer->isUnsigned() && "size arg should be unsigned after implicit conversion to size_t"); return Integer; } @@ -1232,9 +1242,6 @@ class FortifiedBufferChecker { llvm::APSInt LE = L->extOrTrunc(W); llvm::APSInt RE = R->extOrTrunc(W); - LE.setIsUnsigned(true); - RE.setIsUnsigned(true); - return LE * RE; }; @@ -1529,7 +1536,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, } case Builtin::BIfgets: { DiagID = diag::warn_fortify_source_size_mismatch; - SourceSize = Checker.ComputeExplicitObjectSizeArgument(1); + SourceSize = Checker.EvaluateIntegerArgument(1); DestinationSize = Checker.ComputeSizeArgument(0); break; } diff --git a/clang/test/Sema/warn-fortify-source.c b/clang/test/Sema/warn-fortify-source.c index 0227d6703cfdad..17dbc4ecde2fad 100644 --- a/clang/test/Sema/warn-fortify-source.c +++ b/clang/test/Sema/warn-fortify-source.c @@ -129,7 +129,7 @@ void call_bcopy_bzero(void) { void call_fread_fwrite_fgets(FILE *fp) { char src[4]; fread(src, 2, 3, fp); // expected-warning {{'fread' will always overflow; destination buffer has size 4, but size argument is 6}} - fwrite(src, 2, 3, fp); // expected-warning {{'fwrite' will always read past the source buffer; source buffer has size 4, but size argument is 6}} + fwrite(src, 2, 3, fp); // expected-warning {{'fwrite' will always read past the end of the source buffer; source buffer has size 4, but the size is 6}} fgets(src, 5, fp); // expected-warning {{'fgets' size argument is too large; destination buffer has size 4, but size argument is 5}} } From 3cdf23f0d37284d855b48b6b6c9266109015257d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <[email protected]> Date: Mon, 21 Sep 2026 15:42:18 +0200 Subject: [PATCH 4/5] Address some of the comments * Added handling for negative size argument for fgets, * Updated some test cases, * Added bound check. --- .../clang/Basic/DiagnosticSemaKinds.td | 4 ++++ clang/lib/Sema/SemaChecking.cpp | 19 ++++++++++++++++--- clang/test/Sema/warn-fortify-source.c | 2 ++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 78e6fb5357e223..f4e40381f059a4 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -983,6 +983,10 @@ def warn_fortify_source_size_mismatch : Warning< "'%0' size argument is too large; destination buffer has size %1," " but size argument is %2">, InGroup<FortifySource>; +def warn_fortify_source_negative_size : Warning< + " '%0' size argument is negative">, + InGroup<FortifySource>; + def warn_stringop_overread : Warning<"'%0' reading %1 byte%s1 from a region of size %2">, InGroup<DiagGroup<"stringop-overread">>; diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 34002436c7a1d2..aa215b48bafd62 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1180,9 +1180,13 @@ class FortifiedBufferChecker { std::optional<unsigned> IndexOptional = TranslateIndex(Index); if (!IndexOptional) return std::nullopt; + unsigned NewIndex = *IndexOptional; + + if (NewIndex >= TheCall->getNumArgs()) + return std::nullopt; Expr::EvalResult Result; - Expr *Arg = TheCall->getArg(*IndexOptional); + Expr *Arg = TheCall->getArg(NewIndex); if (!Arg->EvaluateAsInt(Result, S.getASTContext())) return std::nullopt; @@ -1243,7 +1247,7 @@ class FortifiedBufferChecker { llvm::APSInt RE = R->extOrTrunc(W); return LE * RE; - }; + } std::optional<llvm::APSInt> ComputeStrLenArgument(unsigned Index) { std::optional<unsigned> IndexOptional = TranslateIndex(Index); @@ -1535,8 +1539,17 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, break; } case Builtin::BIfgets: { - DiagID = diag::warn_fortify_source_size_mismatch; SourceSize = Checker.EvaluateIntegerArgument(1); + + if (SourceSize && SourceSize->isNegative()) { + DiagRuntimeBehavior( + TheCall->getBeginLoc(), TheCall, + PDiag(diag::warn_fortify_source_negative_size) + << Checker.getFunctionName()); + return; + } + + DiagID = diag::warn_fortify_source_size_mismatch; DestinationSize = Checker.ComputeSizeArgument(0); break; } diff --git a/clang/test/Sema/warn-fortify-source.c b/clang/test/Sema/warn-fortify-source.c index 17dbc4ecde2fad..44c2c934717d93 100644 --- a/clang/test/Sema/warn-fortify-source.c +++ b/clang/test/Sema/warn-fortify-source.c @@ -129,8 +129,10 @@ void call_bcopy_bzero(void) { void call_fread_fwrite_fgets(FILE *fp) { char src[4]; fread(src, 2, 3, fp); // expected-warning {{'fread' will always overflow; destination buffer has size 4, but size argument is 6}} + fread(src, 1ULL << 32, 1ULL << 32, fp); // expected-warning {{'fread' will always overflow; destination buffer has size 4, but size argument is 18446744073709551616}} fwrite(src, 2, 3, fp); // expected-warning {{'fwrite' will always read past the end of the source buffer; source buffer has size 4, but the size is 6}} fgets(src, 5, fp); // expected-warning {{'fgets' size argument is too large; destination buffer has size 4, but size argument is 5}} + fgets(src, -1, fp); // expected-warning {{'fgets' size argument is negative}} } From 50a0d7c62e5b7a3a4c183754477a4124336cacb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <[email protected]> Date: Mon, 21 Sep 2026 15:52:29 +0200 Subject: [PATCH 5/5] Fix formatting --- clang/lib/Sema/SemaChecking.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index aa215b48bafd62..a964526efe49af 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1542,10 +1542,9 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, SourceSize = Checker.EvaluateIntegerArgument(1); if (SourceSize && SourceSize->isNegative()) { - DiagRuntimeBehavior( - TheCall->getBeginLoc(), TheCall, - PDiag(diag::warn_fortify_source_negative_size) - << Checker.getFunctionName()); + DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, + PDiag(diag::warn_fortify_source_negative_size) + << Checker.getFunctionName()); return; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
