[clang] [Clang][Sema] Add fortify warnings for fread, fwrite, and fgets (PR #204337)
Radovan Božić via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 21 06:52:53 PDT 2026
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?= <radovan.bozic at htecgroup.com>
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?= <radovan.bozic at htecgroup.com>
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 at -4{{'fread' will always overflow; destination buffer has size 4096, but size argument is 16384}}
+ // bugpath-warning at -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 at -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 at -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?= <radovan.bozic at htecgroup.com>
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?= <radovan.bozic at htecgroup.com>
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?= <radovan.bozic at htecgroup.com>
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;
}
More information about the cfe-commits
mailing list