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?= <[email protected]> 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?= <[email protected]> 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@-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 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@-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 4ad682b05b6b0468fc95f83006e29b9b6ea7175f 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 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?= <[email protected]> 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?= <[email protected]> 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?= <[email protected]> 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?= <[email protected]> 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?= <[email protected]> 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?= <[email protected]> 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?= <[email protected]> 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; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
