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/9] [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 0aec57f201301..26faebe9b4343 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 cfb2ee3368201..16f08ebd2e006 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 5c831e6cdebce..a7a8a82f00d23 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 0a6c44f59af9e..0227d6703cfda 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/9] 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 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/9] 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 a96b0af93ab0b..1b373e715914b 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 16f08ebd2e006..78e6fb5357e22 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 a7a8a82f00d23..34002436c7a1d 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 0227d6703cfda..17dbc4ecde2fa 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/9] 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 78e6fb5357e22..f4e40381f059a 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 34002436c7a1d..aa215b48bafd6 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 17dbc4ecde2fa..44c2c934717d9 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/9] 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 aa215b48bafd6..a964526efe49a 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; } From 4f621329cb9ca7985b4a7d4cd2090dbf9dd6f380 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 6/9] Remove unnecessary newlines and space --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/test/Sema/warn-fortify-source.c | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index f4e40381f059a..113d969274822 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -984,7 +984,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 44c2c934717d9..75b3abdd97702 100644 --- a/clang/test/Sema/warn-fortify-source.c +++ b/clang/test/Sema/warn-fortify-source.c @@ -11,7 +11,6 @@ typedef unsigned long size_t; typedef struct _IO_FILE FILE; - #ifdef __cplusplus extern "C" { #endif @@ -29,7 +28,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 c58e905530b1cf285218ded22c69ef0b92edd6d2 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 7/9] 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 75b3abdd97702..c116edf6af924 100644 --- a/clang/test/Sema/warn-fortify-source.c +++ b/clang/test/Sema/warn-fortify-source.c @@ -132,6 +132,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 b3aa10378b19d4ee06676d07864280765e682111 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 8/9] [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 a964526efe49a..c0d482c5730a8 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1344,8 +1344,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) { @@ -1358,8 +1358,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; } @@ -1367,8 +1367,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; } @@ -1431,12 +1431,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; } @@ -1454,9 +1454,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 || @@ -1470,8 +1470,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; } @@ -1487,9 +1487,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; } @@ -1504,9 +1504,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 && @@ -1520,28 +1520,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()); @@ -1549,7 +1549,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) @@ -1574,11 +1574,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(); @@ -1588,13 +1588,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) @@ -1603,7 +1603,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(); @@ -1611,19 +1611,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 31583c594ae22f657f2011b6e79254cbd5cf7fb9 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 9/9] Hoist argument bounds check into TranslateIndex --- clang/lib/Sema/SemaChecking.cpp | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index c0d482c5730a8..a8e0b5bae68d4 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; @@ -1180,13 +1180,8 @@ 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(NewIndex); + Expr *Arg = TheCall->getArg(*IndexOptional); if (!Arg->EvaluateAsInt(Result, S.getASTContext())) return std::nullopt; @@ -1219,12 +1214,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. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
