[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
Wed Sep 30 04:00:30 PDT 2026
https://github.com/bozicrHT updated https://github.com/llvm/llvm-project/pull/204337
>From 4a3f201660a3bfb2ab32ef0a6400c85bc199287b 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 01/10] [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 | 13 +++++++
4 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index 5480aed3d5439..c6286df50f97c 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 d293a9798da6a..f351270827ab9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1050,6 +1050,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 c0cfc51f5b68f..66252903ead0b 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1218,6 +1218,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)
@@ -1554,7 +1573,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 ee353c81c9e0e..f06429b3868c0 100644
--- a/clang/test/Sema/warn-fortify-source.c
+++ b/clang/test/Sema/warn-fortify-source.c
@@ -20,6 +20,7 @@ struct pollfd {
struct timespec;
typedef unsigned long sigset_t;
typedef unsigned long sigset64_t;
+typedef struct _IO_FILE FILE;
#ifdef __cplusplus
extern "C" {
@@ -45,6 +46,10 @@ int ppoll64(struct pollfd *, nfds_t, const struct timespec *,
const sigset64_t *);
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
}
@@ -154,6 +159,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 e59770b6cb8296e270029ec1c86b6657f39ba243 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 02/10] 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 2cefa80341fc4..646bfff7d9e4b 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 87761b3afb76b..3f291b6164596 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 4ad682b05b6b0468fc95f83006e29b9b6ea7175f 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 03/10] 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 c778703e8cc6f..1165768a058d9 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -466,6 +466,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 f351270827ab9..eca9129b56240 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1051,8 +1051,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 66252903ead0b..463a5a2cf9da7 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;
+
+ return Result.Val.getInt();
+ }
+
+ std::optional<llvm::APSInt>
+ ComputeExplicitObjectSizeArgument(unsigned Index) {
+ std::optional<llvm::APSInt> Integer = EvaluateIntegerArgument(Index);
+ if (!Integer)
return std::nullopt;
- llvm::APSInt Integer = Result.Val.getInt().extOrTrunc(SizeTypeWidth);
- Integer.setIsUnsigned(true);
+
+ assert(Integer->isUnsigned() &&
+ "size arg should be unsigned after implicit conversion to size_t");
return Integer;
}
@@ -1231,9 +1241,6 @@ class FortifiedBufferChecker {
llvm::APSInt LE = L->extOrTrunc(W);
llvm::APSInt RE = R->extOrTrunc(W);
- LE.setIsUnsigned(true);
- RE.setIsUnsigned(true);
-
return LE * RE;
};
@@ -1587,7 +1594,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 f06429b3868c0..58d2bdf069013 100644
--- a/clang/test/Sema/warn-fortify-source.c
+++ b/clang/test/Sema/warn-fortify-source.c
@@ -162,7 +162,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 6f07752494fbcd47ac6f1717a5e1439461300118 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 04/10] Address some of the comments
* Added handling for negative size argument for fgets,
* Updated some test cases,
* Added bound check.
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 ++++
clang/lib/Sema/SemaChecking.cpp | 13 +++++++++++--
clang/test/Sema/warn-fortify-source.c | 2 ++
3 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eca9129b56240..3f96503d24398 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1007,6 +1007,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 463a5a2cf9da7..08a166206418b 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1242,7 +1242,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);
@@ -1593,8 +1593,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 58d2bdf069013..3515b02c496d8 100644
--- a/clang/test/Sema/warn-fortify-source.c
+++ b/clang/test/Sema/warn-fortify-source.c
@@ -162,8 +162,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 3c9923248cde65b4decc84f7fe300ca4dde8c8f7 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 05/10] 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 08a166206418b..1c90c312606ce 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1596,10 +1596,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;
}
>From 067e53a07870a5c7e21f83551fa8376db4802d64 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <radovan.bozic at htecgroup.com>
Date: Thu, 24 Sep 2026 10:44:26 +0200
Subject: [PATCH 06/10] Remove unnecessary newlines and space
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +-
clang/test/Sema/warn-fortify-source.c | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3f96503d24398..9208aba1445d7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1008,7 +1008,7 @@ def warn_fortify_source_size_mismatch : Warning<
" but size argument is %2">, InGroup<FortifySource>;
def warn_fortify_source_negative_size : Warning<
- " '%0' size argument is negative">,
+ "'%0' size argument is negative">,
InGroup<FortifySource>;
def warn_stringop_overread
diff --git a/clang/test/Sema/warn-fortify-source.c b/clang/test/Sema/warn-fortify-source.c
index 3515b02c496d8..1f862c07db9c0 100644
--- a/clang/test/Sema/warn-fortify-source.c
+++ b/clang/test/Sema/warn-fortify-source.c
@@ -50,7 +50,6 @@ 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
}
#endif
>From 96690476d3651a5f8d94e6f4a2ed97e90a949e39 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <radovan.bozic at htecgroup.com>
Date: Thu, 24 Sep 2026 10:47:21 +0200
Subject: [PATCH 07/10] Add valid test cases that will not trigger the warning
---
clang/test/Sema/warn-fortify-source.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/clang/test/Sema/warn-fortify-source.c b/clang/test/Sema/warn-fortify-source.c
index 1f862c07db9c0..73a10070008f4 100644
--- a/clang/test/Sema/warn-fortify-source.c
+++ b/clang/test/Sema/warn-fortify-source.c
@@ -166,6 +166,11 @@ void call_fread_fwrite_fgets(FILE *fp) {
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}}
+ fread(src, 2, 2, fp);
+ fread(src, 0, 10, fp);
+ fwrite(src, 2, 2, fp);
+ fgets(src, 4, fp);
+ fgets(src, 0, fp);
}
void call_snprintf(double d, int n) {
>From 034f9cfcb3d642bb0d2a83eae16bd8de9d082179 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <radovan.bozic at htecgroup.com>
Date: Thu, 24 Sep 2026 11:07:47 +0200
Subject: [PATCH 08/10] [NFC] Rename fortify size variables
---
clang/lib/Sema/SemaChecking.cpp | 76 ++++++++++++++++-----------------
1 file changed, 38 insertions(+), 38 deletions(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 1c90c312606ce..57332a9d7f2e3 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1339,8 +1339,8 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
unsigned SizeTypeWidth = Checker.getSizeTypeWidth();
- std::optional<llvm::APSInt> SourceSize;
- std::optional<llvm::APSInt> DestinationSize;
+ std::optional<llvm::APSInt> AccessSize;
+ std::optional<llvm::APSInt> BufferSize;
unsigned DiagID = 0;
switch (BuiltinID) {
@@ -1353,8 +1353,8 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
case Builtin::BI__builtin_strcpy:
case Builtin::BIstrcpy: {
DiagID = diag::warn_fortify_strlen_overflow;
- SourceSize = Checker.ComputeStrLenArgument(1);
- DestinationSize = Checker.ComputeSizeArgument(0);
+ AccessSize = Checker.ComputeStrLenArgument(1);
+ BufferSize = Checker.ComputeSizeArgument(0);
break;
}
@@ -1362,8 +1362,8 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
case Builtin::BI__builtin___stpcpy_chk:
case Builtin::BI__builtin___strcpy_chk: {
DiagID = diag::warn_fortify_strlen_overflow;
- SourceSize = Checker.ComputeStrLenArgument(1);
- DestinationSize = Checker.ComputeExplicitObjectSizeArgument(2);
+ AccessSize = Checker.ComputeStrLenArgument(1);
+ BufferSize = Checker.ComputeExplicitObjectSizeArgument(2);
break;
}
@@ -1426,12 +1426,12 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
DiagID = H.isKernelCompatible()
? diag::warn_format_overflow
: diag::warn_format_overflow_non_kprintf;
- SourceSize = llvm::APSInt::getUnsigned(H.getSizeLowerBound())
+ AccessSize = llvm::APSInt::getUnsigned(H.getSizeLowerBound())
.extOrTrunc(SizeTypeWidth);
if (BuiltinID == Builtin::BI__builtin___sprintf_chk) {
- DestinationSize = Checker.ComputeExplicitObjectSizeArgument(2);
+ BufferSize = Checker.ComputeExplicitObjectSizeArgument(2);
} else {
- DestinationSize = Checker.ComputeSizeArgument(0);
+ BufferSize = Checker.ComputeSizeArgument(0);
}
break;
}
@@ -1449,9 +1449,9 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
case Builtin::BI__builtin___memccpy_chk:
case Builtin::BI__builtin___mempcpy_chk: {
DiagID = diag::warn_builtin_chk_overflow;
- SourceSize =
+ AccessSize =
Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 2);
- DestinationSize =
+ BufferSize =
Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1);
if (BuiltinID == Builtin::BI__builtin___memcpy_chk ||
@@ -1465,8 +1465,8 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
case Builtin::BI__builtin___snprintf_chk:
case Builtin::BI__builtin___vsnprintf_chk: {
DiagID = diag::warn_builtin_chk_overflow;
- SourceSize = Checker.ComputeExplicitObjectSizeArgument(1);
- DestinationSize = Checker.ComputeExplicitObjectSizeArgument(3);
+ AccessSize = Checker.ComputeExplicitObjectSizeArgument(1);
+ BufferSize = Checker.ComputeExplicitObjectSizeArgument(3);
break;
}
@@ -1486,9 +1486,9 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
// size larger than the destination buffer though; this is a runtime abort
// in _FORTIFY_SOURCE mode, and is quite suspicious otherwise.
DiagID = diag::warn_fortify_source_size_mismatch;
- SourceSize =
+ AccessSize =
Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1);
- DestinationSize = Checker.ComputeSizeArgument(0);
+ BufferSize = Checker.ComputeSizeArgument(0);
break;
}
@@ -1558,9 +1558,9 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
case Builtin::BImempcpy:
case Builtin::BI__builtin_mempcpy: {
DiagID = diag::warn_fortify_source_overflow;
- SourceSize =
+ AccessSize =
Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1);
- DestinationSize = Checker.ComputeSizeArgument(0);
+ BufferSize = Checker.ComputeSizeArgument(0);
// Buffer overread doesn't make sense for memset/bzero.
if (BuiltinID != Builtin::BImemset &&
@@ -1574,28 +1574,28 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
case Builtin::BIbcopy:
case Builtin::BI__builtin_bcopy: {
DiagID = diag::warn_fortify_source_overflow;
- SourceSize =
+ AccessSize =
Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1);
- DestinationSize = Checker.ComputeSizeArgument(1);
+ BufferSize = Checker.ComputeSizeArgument(1);
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);
+ AccessSize = Checker.ComputeExplicitObjectSizeArgumentProduct(1, 2);
+ BufferSize = Checker.ComputeSizeArgument(0);
break;
}
case Builtin::BIfwrite: {
DiagID = diag::warn_fortify_source_overread;
- SourceSize = Checker.ComputeExplicitObjectSizeArgumentProduct(1, 2);
- DestinationSize = Checker.ComputeSizeArgument(0);
+ AccessSize = Checker.ComputeExplicitObjectSizeArgumentProduct(1, 2);
+ BufferSize = Checker.ComputeSizeArgument(0);
break;
}
case Builtin::BIfgets: {
- SourceSize = Checker.EvaluateIntegerArgument(1);
+ AccessSize = Checker.EvaluateIntegerArgument(1);
- if (SourceSize && SourceSize->isNegative()) {
+ if (AccessSize && AccessSize->isNegative()) {
DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall,
PDiag(diag::warn_fortify_source_negative_size)
<< Checker.getFunctionName());
@@ -1603,7 +1603,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
}
DiagID = diag::warn_fortify_source_size_mismatch;
- DestinationSize = Checker.ComputeSizeArgument(0);
+ BufferSize = Checker.ComputeSizeArgument(0);
break;
}
// memchr(buf, val, size)
@@ -1628,11 +1628,11 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
case Builtin::BIvsnprintf:
case Builtin::BI__builtin_vsnprintf: {
DiagID = diag::warn_fortify_source_size_mismatch;
- SourceSize = Checker.ComputeExplicitObjectSizeArgument(1);
+ AccessSize = Checker.ComputeExplicitObjectSizeArgument(1);
const auto *FormatExpr = TheCall->getArg(2)->IgnoreParenImpCasts();
StringRef FormatStrRef;
size_t StrLen;
- if (SourceSize &&
+ if (AccessSize &&
ProcessFormatStringLiteral(FormatExpr, FormatStrRef, StrLen, Context)) {
EstimateSizeFormatHandler H(FormatStrRef);
const char *FormatBytes = FormatStrRef.data();
@@ -1642,13 +1642,13 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
llvm::APSInt FormatSize =
llvm::APSInt::getUnsigned(H.getSizeLowerBound())
.extOrTrunc(SizeTypeWidth);
- if (FormatSize > *SourceSize && *SourceSize != 0) {
+ if (FormatSize > *AccessSize && *AccessSize != 0) {
unsigned TruncationDiagID =
H.isKernelCompatible() ? diag::warn_format_truncation
: diag::warn_format_truncation_non_kprintf;
SmallString<16> SpecifiedSizeStr;
SmallString<16> FormatSizeStr;
- SourceSize->toString(SpecifiedSizeStr, /*Radix=*/10);
+ AccessSize->toString(SpecifiedSizeStr, /*Radix=*/10);
FormatSize.toString(FormatSizeStr, /*Radix=*/10);
DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall,
PDiag(TruncationDiagID)
@@ -1657,7 +1657,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
}
}
}
- DestinationSize = Checker.ComputeSizeArgument(0);
+ BufferSize = Checker.ComputeSizeArgument(0);
const Expr *LenArg = TheCall->getArg(1)->IgnoreCasts();
const Expr *Dest = TheCall->getArg(0)->IgnoreCasts();
IdentifierInfo *FnInfo = FD->getIdentifier();
@@ -1665,19 +1665,19 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
}
}
- if (!SourceSize || !DestinationSize ||
- llvm::APSInt::compareValues(*SourceSize, *DestinationSize) <= 0)
+ if (!AccessSize || !BufferSize ||
+ llvm::APSInt::compareValues(*AccessSize, *BufferSize) <= 0)
return;
std::string FunctionName = Checker.getFunctionName();
- SmallString<16> DestinationStr;
- SmallString<16> SourceStr;
- DestinationSize->toString(DestinationStr, /*Radix=*/10);
- SourceSize->toString(SourceStr, /*Radix=*/10);
+ SmallString<16> BufferSizeStr;
+ SmallString<16> AccessSizeStr;
+ BufferSize->toString(BufferSizeStr, /*Radix=*/10);
+ AccessSize->toString(AccessSizeStr, /*Radix=*/10);
DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall,
PDiag(DiagID)
- << FunctionName << DestinationStr << SourceStr);
+ << FunctionName << BufferSizeStr << AccessSizeStr);
}
void Sema::checkFortifiedLibcArgument(FunctionDecl *FD, CallExpr *TheCall) {
>From 931d31bc41445e2a8397ff4f187e6c64cc244029 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <radovan.bozic at htecgroup.com>
Date: Thu, 24 Sep 2026 11:17:25 +0200
Subject: [PATCH 09/10] Hoist argument bounds check into TranslateIndex
---
clang/lib/Sema/SemaChecking.cpp | 36 ++++++++++++++++-----------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 57332a9d7f2e3..54d9d5ca3d78b 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1163,12 +1163,12 @@ class FortifiedBufferChecker {
// argument index to refer to the arguments of the called function. Unless
// the index is out of bounds, which presumably means it's a variadic
// function.
- if (!DABAttr)
- return Index;
- unsigned DABIndices = DABAttr->argIndices_size();
- unsigned NewIndex = Index < DABIndices
- ? DABAttr->argIndices_begin()[Index]
- : Index - DABIndices + FD->getNumParams();
+ unsigned NewIndex = Index;
+ if (DABAttr) {
+ unsigned DABIndices = DABAttr->argIndices_size();
+ NewIndex = Index < DABIndices ? DABAttr->argIndices_begin()[Index]
+ : Index - DABIndices + FD->getNumParams();
+ }
if (NewIndex >= TheCall->getNumArgs())
return std::nullopt;
return NewIndex;
@@ -1190,12 +1190,16 @@ class FortifiedBufferChecker {
std::optional<llvm::APSInt>
ComputeExplicitObjectSizeArgument(unsigned Index) {
- std::optional<llvm::APSInt> Integer = EvaluateIntegerArgument(Index);
- if (!Integer)
+ std::optional<unsigned> IndexOptional = TranslateIndex(Index);
+ if (!IndexOptional)
return std::nullopt;
-
- assert(Integer->isUnsigned() &&
- "size arg should be unsigned after implicit conversion to size_t");
+ unsigned NewIndex = *IndexOptional;
+ Expr::EvalResult Result;
+ Expr *SizeArg = TheCall->getArg(NewIndex);
+ if (!SizeArg->EvaluateAsInt(Result, S.getASTContext()))
+ return std::nullopt;
+ llvm::APSInt Integer = Result.Val.getInt().extOrTrunc(SizeTypeWidth);
+ Integer.setIsUnsigned(true);
return Integer;
}
@@ -1214,12 +1218,8 @@ class FortifiedBufferChecker {
std::optional<unsigned> IndexOptional = TranslateIndex(Index);
if (!IndexOptional)
return std::nullopt;
- unsigned NewIndex = *IndexOptional;
- if (NewIndex >= TheCall->getNumArgs())
- return std::nullopt;
-
- const Expr *ObjArg = TheCall->getArg(NewIndex);
+ const Expr *ObjArg = TheCall->getArg(*IndexOptional);
if (std::optional<uint64_t> ObjSize =
ObjArg->tryEvaluateObjectSize(S.getASTContext(), BOSType)) {
// Get the object size in the target's size_t width.
@@ -1500,8 +1500,8 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
!TheCall->getArg(2)->getType()->isIntegerType())
return;
DiagID = diag::warn_fortify_source_size_mismatch;
- SourceSize = Checker.ComputeExplicitObjectSizeArgument(2);
- DestinationSize = Checker.ComputeSizeArgument(1);
+ AccessSize = Checker.ComputeExplicitObjectSizeArgument(2);
+ BufferSize = Checker.ComputeSizeArgument(1);
break;
}
>From 539d33af072b817c5193fce665cbfbdbd35e85d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <radovan.bozic at htecgroup.com>
Date: Wed, 30 Sep 2026 12:54:30 +0200
Subject: [PATCH 10/10] Reuse `EvaluateIntegerArgument` for object size checks
---
clang/lib/Sema/SemaChecking.cpp | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 54d9d5ca3d78b..c04d16dcf9323 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1190,16 +1190,11 @@ class FortifiedBufferChecker {
std::optional<llvm::APSInt>
ComputeExplicitObjectSizeArgument(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()))
+ std::optional<llvm::APSInt> Integer = EvaluateIntegerArgument(Index);
+ if (!Integer)
return std::nullopt;
- llvm::APSInt Integer = Result.Val.getInt().extOrTrunc(SizeTypeWidth);
- Integer.setIsUnsigned(true);
+ *Integer = Integer->extOrTrunc(SizeTypeWidth);
+ Integer->setIsUnsigned(true);
return Integer;
}
More information about the cfe-commits
mailing list