https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/183004
>From d21feba35b70341935df51faab7e5735d4a61cea Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Mon, 23 Feb 2026 21:31:37 +0100 Subject: [PATCH 01/19] Initial refactor commit We need to use the lambdas in checkFortifiedBuiltinMemoryFunction. This commit refactors the lambdas into a helper class so they can be used by other functions. --- clang/lib/Sema/SemaChecking.cpp | 128 +++++++++++++++++++------------- 1 file changed, 75 insertions(+), 53 deletions(-) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 2cf8221d933fd..d50fa6d238c71 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1147,31 +1147,19 @@ static bool ProcessFormatStringLiteral(const Expr *FormatExpr, return false; } -void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, - CallExpr *TheCall) { - if (TheCall->isValueDependent() || TheCall->isTypeDependent() || - isConstantEvaluatedContext()) - return; - - bool UseDABAttr = false; - const FunctionDecl *UseDecl = FD; - - const auto *DABAttr = FD->getAttr<DiagnoseAsBuiltinAttr>(); - if (DABAttr) { - UseDecl = DABAttr->getFunction(); - assert(UseDecl && "Missing FunctionDecl in DiagnoseAsBuiltin attribute!"); - UseDABAttr = true; +namespace { +/// Helper class for buffer overflow/overread checking in fortified functions. +class FortifiedBufferChecker { +public: + FortifiedBufferChecker(Sema &S, FunctionDecl *FD, CallExpr *TheCall) + : S(S), TheCall(TheCall), FD(FD), + DABAttr(FD ? FD->getAttr<DiagnoseAsBuiltinAttr>() : nullptr), + UseDABAttr(DABAttr != nullptr) { + const TargetInfo &TI = S.getASTContext().getTargetInfo(); + SizeTypeWidth = TI.getTypeWidth(TI.getSizeType()); } - unsigned BuiltinID = UseDecl->getBuiltinID(/*ConsiderWrappers=*/true); - - if (!BuiltinID) - return; - - const TargetInfo &TI = getASTContext().getTargetInfo(); - unsigned SizeTypeWidth = TI.getTypeWidth(TI.getSizeType()); - - auto TranslateIndex = [&](unsigned Index) -> std::optional<unsigned> { + std::optional<unsigned> TranslateIndex(unsigned Index) { // If we refer to a diagnose_as_builtin attribute, we need to change the // 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 @@ -1185,25 +1173,24 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, if (NewIndex >= TheCall->getNumArgs()) return std::nullopt; return NewIndex; - }; + } - auto ComputeExplicitObjectSizeArgument = - [&](unsigned Index) -> std::optional<llvm::APSInt> { + 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, getASTContext())) + if (!SizeArg->EvaluateAsInt(Result, S.getASTContext())) return std::nullopt; llvm::APSInt Integer = Result.Val.getInt(); Integer.setIsUnsigned(true); return Integer; - }; + } - auto ComputeSizeArgument = - [&](unsigned Index) -> std::optional<llvm::APSInt> { + std::optional<llvm::APSInt> ComputeSizeArgument(unsigned Index) { // If the parameter has a pass_object_size attribute, then we should use its // (potentially) more strict checking mode. Otherwise, conservatively assume // type 0. @@ -1225,15 +1212,14 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, const Expr *ObjArg = TheCall->getArg(NewIndex); if (std::optional<uint64_t> ObjSize = - ObjArg->tryEvaluateObjectSize(getASTContext(), BOSType)) { + ObjArg->tryEvaluateObjectSize(S.getASTContext(), BOSType)) { // Get the object size in the target's size_t width. return llvm::APSInt::getUnsigned(*ObjSize).extOrTrunc(SizeTypeWidth); } return std::nullopt; - }; + } - auto ComputeStrLenArgument = - [&](unsigned Index) -> std::optional<llvm::APSInt> { + std::optional<llvm::APSInt> ComputeStrLenArgument(unsigned Index) { std::optional<unsigned> IndexOptional = TranslateIndex(Index); if (!IndexOptional) return std::nullopt; @@ -1242,12 +1228,45 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, const Expr *ObjArg = TheCall->getArg(NewIndex); if (std::optional<uint64_t> Result = - ObjArg->tryEvaluateStrLen(getASTContext())) { + ObjArg->tryEvaluateStrLen(S.getASTContext())) { // Add 1 for null byte. return llvm::APSInt::getUnsigned(*Result + 1).extOrTrunc(SizeTypeWidth); } return std::nullopt; - }; + } + + const DiagnoseAsBuiltinAttr *getDABAttr() const { return DABAttr; } + unsigned getSizeTypeWidth() const { return SizeTypeWidth; } + +private: + Sema &S; + CallExpr *TheCall; + FunctionDecl *FD; + const DiagnoseAsBuiltinAttr *DABAttr; + bool UseDABAttr; + unsigned SizeTypeWidth; +}; +} // anonymous namespace + +void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, + CallExpr *TheCall) { + if (TheCall->isValueDependent() || TheCall->isTypeDependent() || + isConstantEvaluatedContext()) + return; + + FortifiedBufferChecker Checker(*this, FD, TheCall); + + const FunctionDecl *UseDecl = FD; + if (const auto *DABAttr = Checker.getDABAttr()) { + UseDecl = DABAttr->getFunction(); + assert(UseDecl && "Missing FunctionDecl in DiagnoseAsBuiltin attribute!"); + } + + unsigned BuiltinID = UseDecl->getBuiltinID(/*ConsiderWrappers=*/true); + if (!BuiltinID) + return; + + unsigned SizeTypeWidth = Checker.getSizeTypeWidth(); std::optional<llvm::APSInt> SourceSize; std::optional<llvm::APSInt> DestinationSize; @@ -1280,8 +1299,8 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, case Builtin::BI__builtin_strcpy: case Builtin::BIstrcpy: { DiagID = diag::warn_fortify_strlen_overflow; - SourceSize = ComputeStrLenArgument(1); - DestinationSize = ComputeSizeArgument(0); + SourceSize = Checker.ComputeStrLenArgument(1); + DestinationSize = Checker.ComputeSizeArgument(0); break; } @@ -1289,8 +1308,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 = ComputeStrLenArgument(1); - DestinationSize = ComputeExplicitObjectSizeArgument(2); + SourceSize = Checker.ComputeStrLenArgument(1); + DestinationSize = Checker.ComputeExplicitObjectSizeArgument(2); IsChkVariant = true; break; } @@ -1324,7 +1343,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, }; auto ShiftedComputeSizeArgument = [&](unsigned Index) { - return ComputeSizeArgument(Index + DataIndex); + return Checker.ComputeSizeArgument(Index + DataIndex); }; ScanfDiagnosticFormatHandler H(ShiftedComputeSizeArgument, Diagnose); const char *FormatBytes = FormatStrRef.data(); @@ -1357,10 +1376,10 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, SourceSize = llvm::APSInt::getUnsigned(H.getSizeLowerBound()) .extOrTrunc(SizeTypeWidth); if (BuiltinID == Builtin::BI__builtin___sprintf_chk) { - DestinationSize = ComputeExplicitObjectSizeArgument(2); + DestinationSize = Checker.ComputeExplicitObjectSizeArgument(2); IsChkVariant = true; } else { - DestinationSize = ComputeSizeArgument(0); + DestinationSize = Checker.ComputeSizeArgument(0); } break; } @@ -1378,9 +1397,10 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, case Builtin::BI__builtin___memccpy_chk: case Builtin::BI__builtin___mempcpy_chk: { DiagID = diag::warn_builtin_chk_overflow; - SourceSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 2); + SourceSize = + Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 2); DestinationSize = - ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); + Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); IsChkVariant = true; break; } @@ -1388,8 +1408,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 = ComputeExplicitObjectSizeArgument(1); - DestinationSize = ComputeExplicitObjectSizeArgument(3); + SourceSize = Checker.ComputeExplicitObjectSizeArgument(1); + DestinationSize = Checker.ComputeExplicitObjectSizeArgument(3); IsChkVariant = true; break; } @@ -1406,8 +1426,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 = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); - DestinationSize = ComputeSizeArgument(0); + SourceSize = + Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); + DestinationSize = Checker.ComputeSizeArgument(0); break; } @@ -1422,8 +1443,9 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, case Builtin::BImempcpy: case Builtin::BI__builtin_mempcpy: { DiagID = diag::warn_fortify_source_overflow; - SourceSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); - DestinationSize = ComputeSizeArgument(0); + SourceSize = + Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); + DestinationSize = Checker.ComputeSizeArgument(0); break; } case Builtin::BIbcopy: @@ -1438,7 +1460,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, case Builtin::BIvsnprintf: case Builtin::BI__builtin_vsnprintf: { DiagID = diag::warn_fortify_source_size_mismatch; - SourceSize = ComputeExplicitObjectSizeArgument(1); + SourceSize = Checker.ComputeExplicitObjectSizeArgument(1); const auto *FormatExpr = TheCall->getArg(2)->IgnoreParenImpCasts(); StringRef FormatStrRef; size_t StrLen; @@ -1467,7 +1489,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, } } } - DestinationSize = ComputeSizeArgument(0); + DestinationSize = Checker.ComputeSizeArgument(0); const Expr *LenArg = TheCall->getArg(1)->IgnoreCasts(); const Expr *Dest = TheCall->getArg(0)->IgnoreCasts(); IdentifierInfo *FnInfo = FD->getIdentifier(); >From 3245429627f7da065f2e229a00375aefbc119f53 Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Mon, 23 Feb 2026 22:38:16 +0100 Subject: [PATCH 02/19] Add overread checks for memcpy family of functions Checks builtins and their _chk variants. --- clang/include/clang/Basic/DiagnosticGroups.td | 1 + .../clang/Basic/DiagnosticSemaKinds.td | 4 + clang/include/clang/Sema/Sema.h | 5 + clang/lib/Sema/SemaChecking.cpp | 78 +++++++++- clang/test/Sema/warn-stringop-overread.c | 138 ++++++++++++++++++ 5 files changed, 224 insertions(+), 2 deletions(-) create mode 100644 clang/test/Sema/warn-stringop-overread.c diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 8031f99419bdc..103d1ccd0306b 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1839,6 +1839,7 @@ def CrossTURemarks : DiagGroup<"ctu-remarks">; def CTADMaybeUnsupported : DiagGroup<"ctad-maybe-unsupported">; def FortifySource : DiagGroup<"fortify-source", [FormatOverflow, FormatTruncation]>; +def StringopOverread : DiagGroup<"stringop-overread">; def OverflowBehaviorAttributeIgnored : DiagGroup<"overflow-behavior-attribute-ignored">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index fab50fe6768d8..147eef2e120ff 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -966,6 +966,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_stringop_overread + : Warning<"'%0' reading %1 byte%s1 from a region of size %2">, + InGroup<StringopOverread>; + def warn_fortify_strlen_overflow: Warning< "'%0' will always overflow; destination buffer has size %1," " but the source string has length %2 (including NUL byte)">, diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 10d489dc96a34..7a2346d8a9784 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2936,6 +2936,11 @@ class Sema final : public SemaBase { CallExpr *TheCall, EltwiseBuiltinArgTyRestriction ArgTyRestr = EltwiseBuiltinArgTyRestriction::None); + /// Check for source buffer overread in memory functions. + void checkSourceBufferOverread(FunctionDecl *FD, CallExpr *TheCall, + unsigned SrcArgIdx, unsigned SizeArgIdx, + StringRef FunctionName = ""); + private: void CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr, const ArraySubscriptExpr *ASE = nullptr, diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index d50fa6d238c71..7e7fbe4378157 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1248,6 +1248,43 @@ class FortifiedBufferChecker { }; } // anonymous namespace +void Sema::checkSourceBufferOverread(FunctionDecl *FD, CallExpr *TheCall, + unsigned SrcArgIdx, unsigned SizeArgIdx, + StringRef FunctionName) { + if (TheCall->isValueDependent() || TheCall->isTypeDependent() || + isConstantEvaluatedContext()) + return; + + FortifiedBufferChecker Checker(*this, FD, TheCall); + + std::optional<llvm::APSInt> CopyLen = + Checker.ComputeExplicitObjectSizeArgument(SizeArgIdx); + std::optional<llvm::APSInt> SrcBufSize = + Checker.ComputeSizeArgument(SrcArgIdx); + + if (!CopyLen || !SrcBufSize) + return; + + // Warn only if copy length exceeds source buffer size. + if (llvm::APSInt::compareValues(*CopyLen, *SrcBufSize) <= 0) + return; + + std::string FuncName; + if (FunctionName.empty()) { + if (const FunctionDecl *CalleeDecl = TheCall->getDirectCallee()) + FuncName = CalleeDecl->getName().str(); + else + FuncName = "memory function"; + } else { + FuncName = FunctionName.str(); + } + + DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, + PDiag(diag::warn_stringop_overread) + << FuncName << CopyLen->getZExtValue() + << SrcBufSize->getZExtValue()); +} + void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, CallExpr *TheCall) { if (TheCall->isValueDependent() || TheCall->isTypeDependent() || @@ -1402,6 +1439,13 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, DestinationSize = Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); IsChkVariant = true; + + if (BuiltinID == Builtin::BI__builtin___memcpy_chk || + BuiltinID == Builtin::BI__builtin___memmove_chk || + BuiltinID == Builtin::BI__builtin___mempcpy_chk) { + checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/1, /*SizeArgIdx=*/2, + GetFunctionName()); + } break; } @@ -1446,15 +1490,45 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, SourceSize = Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); DestinationSize = Checker.ComputeSizeArgument(0); + + // Buffer overread doesn't make sense for memset. + if (BuiltinID != Builtin::BImemset && + BuiltinID != Builtin::BI__builtin_memset) { + checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/1, /*SizeArgIdx=*/2, + GetFunctionName()); + } break; } case Builtin::BIbcopy: case Builtin::BI__builtin_bcopy: { DiagID = diag::warn_fortify_source_overflow; - SourceSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); - DestinationSize = ComputeSizeArgument(1); + SourceSize = + Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); + DestinationSize = Checker.ComputeSizeArgument(1); break; } + + // memchr(buf, val, size) + case Builtin::BImemchr: + case Builtin::BI__builtin_memchr: { + checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/0, /*SizeArgIdx=*/2, + GetFunctionName()); + return; + } + + // memcmp/bcmp(buf0, buf1, size) + // Two checks since each buffer is read + case Builtin::BImemcmp: + case Builtin::BI__builtin_memcmp: + case Builtin::BIbcmp: + case Builtin::BI__builtin_bcmp: { + std::string Name = GetFunctionName(); + checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/0, /*SizeArgIdx=*/2, + Name); + checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/1, /*SizeArgIdx=*/2, + Name); + return; + } case Builtin::BIsnprintf: case Builtin::BI__builtin_snprintf: case Builtin::BIvsnprintf: diff --git a/clang/test/Sema/warn-stringop-overread.c b/clang/test/Sema/warn-stringop-overread.c new file mode 100644 index 0000000000000..746684272729b --- /dev/null +++ b/clang/test/Sema/warn-stringop-overread.c @@ -0,0 +1,138 @@ +// RUN: %clang_cc1 %s -verify +// RUN: %clang_cc1 %s -verify -DUSE_BUILTINS + +typedef unsigned long size_t; + +#ifdef __cplusplus +extern "C" { +#endif + +#if defined(USE_BUILTINS) +#define memcpy(x,y,z) __builtin_memcpy(x,y,z) +#define memmove(x,y,z) __builtin_memmove(x,y,z) +#define memchr(x,y,z) __builtin_memchr(x,y,z) +#define memcmp(x,y,z) __builtin_memcmp(x,y,z) +#else +void *memcpy(void *dst, const void *src, size_t c); +void *memmove(void *dst, const void *src, size_t c); +void *memchr(const void *s, int c, size_t n); +int memcmp(const void *s1, const void *s2, size_t n); +#endif + +#ifdef __cplusplus +} +#endif + +void test_memcpy_overread(void) { + char dst[100]; + int src = 0; + memcpy(dst, &src, sizeof(src) + 1); // expected-warning {{'memcpy' reading 5 bytes from a region of size 4}} +} + +void test_memcpy_array_overread(void) { + int dest[10]; + int src[5] = {1, 2, 3, 4, 5}; + memcpy(dest, src, 10 * sizeof(int)); // expected-warning {{'memcpy' reading 40 bytes from a region of size 20}} +} + +void test_memcpy_struct_overread(void) { + struct S { + int x; + int y; + }; + char dst[100]; + struct S src = {1, 2}; + memcpy(dst, &src, sizeof(struct S) + 1); // expected-warning {{'memcpy' reading 9 bytes from a region of size 8}} +} + +void test_memmove_overread(void) { + char dst[100]; + char src[10]; + memmove(dst, src, 20); // expected-warning {{'memmove' reading 20 bytes from a region of size 10}} +} + +void test_memcpy_no_warning_exact_size(void) { + char dst[100]; + int src = 0; + memcpy(dst, &src, sizeof(src)); // no warning +} + +void test_memcpy_no_warning_smaller_size(void) { + char dst[100]; + int src[10]; + memcpy(dst, src, 5 * sizeof(int)); // no warning +} + +void test_memcpy_both_overflow(void) { + char dst[5]; + int src = 0; + memcpy(dst, &src, 10); // expected-warning {{'memcpy' reading 10 bytes from a region of size 4}} + // expected-warning@-1 {{'memcpy' will always overflow; destination buffer has size 5, but size argument is 10}} +} + +void test_memchr_overread(void) { + char buf[4]; + memchr(buf, 'a', 8); // expected-warning {{'memchr' reading 8 bytes from a region of size 4}} +} + +void test_memchr_no_warning(void) { + char buf[10]; + memchr(buf, 'a', 10); // no warning +} + +void test_memcmp_overread_first(void) { + char a[4], b[100]; + memcmp(a, b, 8); // expected-warning {{'memcmp' reading 8 bytes from a region of size 4}} +} + +void test_memcmp_overread_second(void) { + char a[100], b[4]; + memcmp(a, b, 8); // expected-warning {{'memcmp' reading 8 bytes from a region of size 4}} +} + +void test_memcmp_overread_both(void) { + char a[4], b[2]; + memcmp(a, b, 8); // expected-warning {{'memcmp' reading 8 bytes from a region of size 4}} \ + // expected-warning {{'memcmp' reading 8 bytes from a region of size 2}} +} + +void test_memcmp_no_warning(void) { + char a[10], b[10]; + memcmp(a, b, 10); // no warning +} + +void test_memcpy_src_offset_overread(void) { + char src[] = {1, 2, 3, 4}; + char dst[10]; + memcpy(dst, src + 2, 3); // expected-warning {{'memcpy' reading 3 bytes from a region of size 2}} +} + +void test_memcpy_src_offset_no_warning(void) { + char src[] = {1, 2, 3, 4}; + char dst[10]; + memcpy(dst, src + 2, 2); // no warning +} + +int bcmp(const void *s1, const void *s2, size_t n); + +void test_bcmp_overread(void) { + char a[4], b[100]; + bcmp(a, b, 8); // expected-warning {{'bcmp' reading 8 bytes from a region of size 4}} +} + +void test_bcmp_no_warning(void) { + char a[10], b[10]; + bcmp(a, b, 10); // no warning +} + +void test_memcpy_chk_overread(void) { + char dst[100]; + char src[4]; + __builtin___memcpy_chk(dst, src, 8, sizeof(dst)); // expected-warning {{'memcpy' reading 8 bytes from a region of size 4}} +} + +void test_memmove_chk_overread(void) { + char dst[100]; + char src[4]; + __builtin___memmove_chk(dst, src, 8, sizeof(dst)); // expected-warning {{'memmove' reading 8 bytes from a region of size 4}} +} >From 3b9725d97e0669216811c8dc5a8197865a4be6e1 Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Tue, 24 Feb 2026 05:34:14 +0100 Subject: [PATCH 03/19] Fix findings in test suite --- clang/test/AST/ByteCode/builtin-functions.cpp | 20 +++++++++---------- clang/test/Analysis/bstring.c | 4 ++++ clang/test/Analysis/malloc.c | 1 + clang/test/Analysis/pr22954.c | 2 +- clang/test/Sema/builtin-memcpy.c | 3 ++- clang/test/Sema/builtin-object-size.c | 2 +- clang/test/Sema/warn-fortify-source.c | 7 ++++--- 7 files changed, 23 insertions(+), 16 deletions(-) diff --git a/clang/test/AST/ByteCode/builtin-functions.cpp b/clang/test/AST/ByteCode/builtin-functions.cpp index 214cb06a27d99..add919dacbf94 100644 --- a/clang/test/AST/ByteCode/builtin-functions.cpp +++ b/clang/test/AST/ByteCode/builtin-functions.cpp @@ -1,17 +1,17 @@ -// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple x86_64 %s -verify=expected,both -// RUN: %clang_cc1 -Wno-string-plus-int -triple x86_64 %s -verify=ref,both +// RUN: %clang_cc1 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter -triple x86_64 %s -verify=expected,both +// RUN: %clang_cc1 -Wno-string-plus-int -Wno-stringop-overread -triple x86_64 %s -verify=ref,both // -// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify=expected,both -// RUN: %clang_cc1 -Wno-string-plus-int -triple i686 %s -verify=ref,both +// RUN: %clang_cc1 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter -triple i686 %s -verify=expected,both +// RUN: %clang_cc1 -Wno-string-plus-int -Wno-stringop-overread -triple i686 %s -verify=ref,both // -// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple x86_64 %s -verify=expected,both -// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -triple x86_64 %s -verify=ref,both +// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter -triple x86_64 %s -verify=expected,both +// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -triple x86_64 %s -verify=ref,both // -// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify=expected,both -// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -triple i686 %s -verify=ref,both +// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter -triple i686 %s -verify=expected,both +// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -triple i686 %s -verify=ref,both // -// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify=expected,both -// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -verify=ref,both %s +// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter %s -verify=expected,both +// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -verify=ref,both %s #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ #define LITTLE_END 1 diff --git a/clang/test/Analysis/bstring.c b/clang/test/Analysis/bstring.c index 810241accffa2..bc56135a50a5d 100644 --- a/clang/test/Analysis/bstring.c +++ b/clang/test/Analysis/bstring.c @@ -1,4 +1,5 @@ // RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix.cstring \ // RUN: -analyzer-checker=alpha.unix.cstring \ @@ -7,6 +8,7 @@ // RUN: -analyzer-config eagerly-assume=false // // RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS \ +// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix.cstring \ // RUN: -analyzer-checker=alpha.unix.cstring \ @@ -15,6 +17,7 @@ // RUN: -analyzer-config eagerly-assume=false // // RUN: %clang_analyze_cc1 -verify %s -DVARIANT \ +// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix.cstring \ // RUN: -analyzer-checker=alpha.unix.cstring \ @@ -23,6 +26,7 @@ // RUN: -analyzer-config eagerly-assume=false // // RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS -DVARIANT \ +// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix.cstring \ // RUN: -analyzer-checker=alpha.unix.cstring \ diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c index 849ab3a3a0f37..efb2d52d3a321 100644 --- a/clang/test/Analysis/malloc.c +++ b/clang/test/Analysis/malloc.c @@ -1,5 +1,6 @@ // RUN: %clang_analyze_cc1 -Wno-strict-prototypes -Wno-error=implicit-int -verify %s \ // RUN: -Wno-alloc-size \ +// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=alpha.deadcode.UnreachableCode \ // RUN: -analyzer-checker=unix \ diff --git a/clang/test/Analysis/pr22954.c b/clang/test/Analysis/pr22954.c index 3d1cac1972066..b3910da6c70ab 100644 --- a/clang/test/Analysis/pr22954.c +++ b/clang/test/Analysis/pr22954.c @@ -3,7 +3,7 @@ // At the moment the whole of the destination array content is invalidated. // If a.s1 region has a symbolic offset, the whole region of 'a' is invalidated. // Specific triple set to test structures of size 0. -// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,unix.Malloc,debug.ExprInspection -Wno-error=int-conversion -verify -analyzer-config eagerly-assume=false %s +// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,unix.Malloc,debug.ExprInspection -Wno-error=int-conversion -Wno-stringop-overread -verify -analyzer-config eagerly-assume=false %s typedef __typeof(sizeof(int)) size_t; diff --git a/clang/test/Sema/builtin-memcpy.c b/clang/test/Sema/builtin-memcpy.c index 2a55e78034a02..94f71e4c42a58 100644 --- a/clang/test/Sema/builtin-memcpy.c +++ b/clang/test/Sema/builtin-memcpy.c @@ -7,7 +7,8 @@ /// Zero-sized structs should not crash. int b() { struct { } a[10]; - __builtin_memcpy(&a[2], a, 2); // c-warning {{buffer has size 0, but size argument is 2}} + __builtin_memcpy(&a[2], a, 2); // c-warning {{buffer has size 0, but size argument is 2}} \ + // c-warning {{'memcpy' reading 2 bytes from a region of size 0}} return 0; } diff --git a/clang/test/Sema/builtin-object-size.c b/clang/test/Sema/builtin-object-size.c index a763c24fd6620..8d48d3f569d91 100644 --- a/clang/test/Sema/builtin-object-size.c +++ b/clang/test/Sema/builtin-object-size.c @@ -50,7 +50,7 @@ void f5(void) { char buf[10]; memset((void *)0x100000000ULL, 0, 0x1000); - memcpy((char *)NULL + 0x10000, buf, 0x10); + memcpy((char *)NULL + 0x10000, buf, 0x10); // expected-warning {{'memcpy' reading 16 bytes from a region of size 10}} memcpy1((char *)NULL + 0x10000, buf, 0x10); // expected-error {{argument value 4 is outside the valid range [0, 3]}} } diff --git a/clang/test/Sema/warn-fortify-source.c b/clang/test/Sema/warn-fortify-source.c index d0b519a516545..8a0c9f3d0e31c 100644 --- a/clang/test/Sema/warn-fortify-source.c +++ b/clang/test/Sema/warn-fortify-source.c @@ -96,7 +96,7 @@ void call_stpcpy(void) { void call_memmove(void) { char s1[10], s2[20]; - __builtin_memmove(s2, s1, 20); + __builtin_memmove(s2, s1, 20); // expected-warning {{'memmove' reading 20 bytes from a region of size 10}} __builtin_memmove(s1, s2, 20); // expected-warning {{'memmove' will always overflow; destination buffer has size 10, but size argument is 20}} } @@ -255,11 +255,12 @@ template <int A, int B> void call_memcpy_dep() { char bufferA[A]; char bufferB[B]; - memcpy(bufferA, bufferB, 10); // expected-warning{{'memcpy' will always overflow; destination buffer has size 9, but size argument is 10}} + memcpy(bufferA, bufferB, 10); // expected-warning{{'memcpy' will always overflow; destination buffer has size 9, but size argument is 10}} \ + // expected-warning{{'memcpy' reading 10 bytes from a region of size 9}} } void call_call_memcpy() { - call_memcpy_dep<10, 9>(); + call_memcpy_dep<10, 9>(); // expected-note {{in instantiation of function template specialization 'call_memcpy_dep<10, 9>' requested here}} call_memcpy_dep<9, 10>(); // expected-note {{in instantiation of function template specialization 'call_memcpy_dep<9, 10>' requested here}} } #endif >From 0cb2b2779645192a07ea7d48d15ed75d6de7185a Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Wed, 25 Feb 2026 21:43:55 +0100 Subject: [PATCH 04/19] Derive FuncName, remove UseDABAttr, clarify tests --- clang/include/clang/Sema/Sema.h | 3 +- clang/lib/Sema/SemaChecking.cpp | 54 ++++++++++++++------------- clang/test/Sema/warn-fortify-source.c | 5 ++- 3 files changed, 33 insertions(+), 29 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 7a2346d8a9784..8082a26692f81 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2938,8 +2938,7 @@ class Sema final : public SemaBase { /// Check for source buffer overread in memory functions. void checkSourceBufferOverread(FunctionDecl *FD, CallExpr *TheCall, - unsigned SrcArgIdx, unsigned SizeArgIdx, - StringRef FunctionName = ""); + unsigned SrcArgIdx, unsigned SizeArgIdx); private: void CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr, diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 7e7fbe4378157..256ddc8de25d6 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1153,8 +1153,7 @@ class FortifiedBufferChecker { public: FortifiedBufferChecker(Sema &S, FunctionDecl *FD, CallExpr *TheCall) : S(S), TheCall(TheCall), FD(FD), - DABAttr(FD ? FD->getAttr<DiagnoseAsBuiltinAttr>() : nullptr), - UseDABAttr(DABAttr != nullptr) { + DABAttr(FD ? FD->getAttr<DiagnoseAsBuiltinAttr>() : nullptr) { const TargetInfo &TI = S.getASTContext().getTargetInfo(); SizeTypeWidth = TI.getTypeWidth(TI.getSizeType()); } @@ -1164,7 +1163,7 @@ 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 (!UseDABAttr) + if (DABAttr == nullptr) // Not using DABAttr. return Index; unsigned DABIndices = DABAttr->argIndices_size(); unsigned NewIndex = Index < DABIndices @@ -1238,19 +1237,30 @@ class FortifiedBufferChecker { const DiagnoseAsBuiltinAttr *getDABAttr() const { return DABAttr; } unsigned getSizeTypeWidth() const { return SizeTypeWidth; } + /// Return function name after stripping __builtin_ and _chk affixes. + std::string GetFunctionName(unsigned BuiltinID, bool IsChkVariant) const { + std::string Name = S.getASTContext().BuiltinInfo.getName(BuiltinID); + llvm::StringRef Ref = Name; + if (IsChkVariant) { + Ref = Ref.drop_front(std::strlen("__builtin___")); + Ref = Ref.drop_back(std::strlen("_chk")); + } else { + Ref.consume_front("__builtin_"); + } + return Ref.str(); + } + private: Sema &S; CallExpr *TheCall; FunctionDecl *FD; const DiagnoseAsBuiltinAttr *DABAttr; - bool UseDABAttr; unsigned SizeTypeWidth; }; } // anonymous namespace void Sema::checkSourceBufferOverread(FunctionDecl *FD, CallExpr *TheCall, - unsigned SrcArgIdx, unsigned SizeArgIdx, - StringRef FunctionName) { + unsigned SrcArgIdx, unsigned SizeArgIdx) { if (TheCall->isValueDependent() || TheCall->isTypeDependent() || isConstantEvaluatedContext()) return; @@ -1269,14 +1279,14 @@ void Sema::checkSourceBufferOverread(FunctionDecl *FD, CallExpr *TheCall, if (llvm::APSInt::compareValues(*CopyLen, *SrcBufSize) <= 0) return; - std::string FuncName; - if (FunctionName.empty()) { - if (const FunctionDecl *CalleeDecl = TheCall->getDirectCallee()) - FuncName = CalleeDecl->getName().str(); - else - FuncName = "memory function"; - } else { - FuncName = FunctionName.str(); + llvm::StringRef FuncName = "memory function"; + if (const FunctionDecl *CalleeDecl = TheCall->getDirectCallee()) { + FuncName = CalleeDecl->getName(); + // __builtin___memcpy_chk -> memcpy, __builtin_memcpy -> memcpy. + // The _chk variants have a different prefix so try that one first. + if (!(FuncName.consume_front("__builtin___") && + FuncName.consume_back("_chk"))) + FuncName.consume_front("__builtin_"); } DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, @@ -1443,8 +1453,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, if (BuiltinID == Builtin::BI__builtin___memcpy_chk || BuiltinID == Builtin::BI__builtin___memmove_chk || BuiltinID == Builtin::BI__builtin___mempcpy_chk) { - checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/1, /*SizeArgIdx=*/2, - GetFunctionName()); + checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/1, /*SizeArgIdx=*/2); } break; } @@ -1494,8 +1503,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, // Buffer overread doesn't make sense for memset. if (BuiltinID != Builtin::BImemset && BuiltinID != Builtin::BI__builtin_memset) { - checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/1, /*SizeArgIdx=*/2, - GetFunctionName()); + checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/1, /*SizeArgIdx=*/2); } break; } @@ -1511,8 +1519,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, // memchr(buf, val, size) case Builtin::BImemchr: case Builtin::BI__builtin_memchr: { - checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/0, /*SizeArgIdx=*/2, - GetFunctionName()); + checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/0, /*SizeArgIdx=*/2); return; } @@ -1522,11 +1529,8 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, case Builtin::BI__builtin_memcmp: case Builtin::BIbcmp: case Builtin::BI__builtin_bcmp: { - std::string Name = GetFunctionName(); - checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/0, /*SizeArgIdx=*/2, - Name); - checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/1, /*SizeArgIdx=*/2, - Name); + checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/0, /*SizeArgIdx=*/2); + checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/1, /*SizeArgIdx=*/2); return; } case Builtin::BIsnprintf: diff --git a/clang/test/Sema/warn-fortify-source.c b/clang/test/Sema/warn-fortify-source.c index 8a0c9f3d0e31c..55a54b46a1175 100644 --- a/clang/test/Sema/warn-fortify-source.c +++ b/clang/test/Sema/warn-fortify-source.c @@ -255,12 +255,13 @@ template <int A, int B> void call_memcpy_dep() { char bufferA[A]; char bufferB[B]; - memcpy(bufferA, bufferB, 10); // expected-warning{{'memcpy' will always overflow; destination buffer has size 9, but size argument is 10}} \ - // expected-warning{{'memcpy' reading 10 bytes from a region of size 9}} + memcpy(bufferA, bufferB, 10); } void call_call_memcpy() { call_memcpy_dep<10, 9>(); // expected-note {{in instantiation of function template specialization 'call_memcpy_dep<10, 9>' requested here}} + // expected-warning@-5 {{'memcpy' reading 10 bytes from a region of size 9}} call_memcpy_dep<9, 10>(); // expected-note {{in instantiation of function template specialization 'call_memcpy_dep<9, 10>' requested here}} + // expected-warning@-7 {{'memcpy' will always overflow; destination buffer has size 9, but size argument is 10}} } #endif >From 081b269516680264284f39c7f0b0e950def561bf Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Wed, 25 Feb 2026 23:38:34 +0100 Subject: [PATCH 05/19] Narrow dependency check in checkSourceBufferOverread --- clang/lib/Sema/SemaChecking.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 256ddc8de25d6..4fe1fd795945c 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1261,8 +1261,13 @@ class FortifiedBufferChecker { void Sema::checkSourceBufferOverread(FunctionDecl *FD, CallExpr *TheCall, unsigned SrcArgIdx, unsigned SizeArgIdx) { - if (TheCall->isValueDependent() || TheCall->isTypeDependent() || - isConstantEvaluatedContext()) + if (isConstantEvaluatedContext()) + return; + + const Expr *SrcArg = TheCall->getArg(SrcArgIdx); + const Expr *SizeArg = TheCall->getArg(SizeArgIdx); + if (SrcArg->isValueDependent() || SrcArg->isTypeDependent() || + SizeArg->isValueDependent() || SizeArg->isTypeDependent()) return; FortifiedBufferChecker Checker(*this, FD, TheCall); >From fc8331d4f6fbbfe649869520570d043d4c51a37c Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Wed, 25 Feb 2026 23:49:07 +0100 Subject: [PATCH 06/19] Add cpp and dependency tests --- clang/test/Sema/warn-stringop-overread.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/clang/test/Sema/warn-stringop-overread.c b/clang/test/Sema/warn-stringop-overread.c index 746684272729b..27298d663dc5e 100644 --- a/clang/test/Sema/warn-stringop-overread.c +++ b/clang/test/Sema/warn-stringop-overread.c @@ -1,5 +1,7 @@ // RUN: %clang_cc1 %s -verify // RUN: %clang_cc1 %s -verify -DUSE_BUILTINS +// RUN: %clang_cc1 -xc++ %s -verify +// RUN: %clang_cc1 -xc++ %s -verify -DUSE_BUILTINS typedef unsigned long size_t; @@ -19,6 +21,8 @@ void *memchr(const void *s, int c, size_t n); int memcmp(const void *s1, const void *s2, size_t n); #endif +int bcmp(const void *s1, const void *s2, size_t n); + #ifdef __cplusplus } #endif @@ -113,8 +117,6 @@ void test_memcpy_src_offset_no_warning(void) { memcpy(dst, src + 2, 2); // no warning } -int bcmp(const void *s1, const void *s2, size_t n); - void test_bcmp_overread(void) { char a[4], b[100]; bcmp(a, b, 8); // expected-warning {{'bcmp' reading 8 bytes from a region of size 4}} @@ -136,3 +138,16 @@ void test_memmove_chk_overread(void) { char src[4]; __builtin___memmove_chk(dst, src, 8, sizeof(dst)); // expected-warning {{'memmove' reading 8 bytes from a region of size 4}} } + +#ifdef __cplusplus +template <int N> +void test_memcpy_dependent_dest() { + char dst[N]; + int src = 0; + memcpy(dst, &src, sizeof(src) + 1); // expected-warning {{'memcpy' reading 5 bytes from a region of size 4}} +} + +void call_test_memcpy_dependent_dest() { + test_memcpy_dependent_dest<100>(); // expected-note {{in instantiation}} +} +#endif >From 77cd9e3804e6889d6acb31e5fa994dd434a3fce1 Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Wed, 25 Feb 2026 23:51:13 +0100 Subject: [PATCH 07/19] Enable warning and catch findings in tests The macro `#if !defined(VARIANT) || defined(USE_BUILTINS)` in bstring.c is needed because the call must resolve to a builtin for the diagnostic to be emitted (which matches -Wfortify-source). --- clang/test/AST/ByteCode/builtin-functions.cpp | 21 +++++++------- clang/test/Analysis/bstring.c | 29 ++++++++++++++++--- clang/test/Analysis/malloc.c | 3 +- clang/test/Analysis/pr22954.c | 3 +- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/clang/test/AST/ByteCode/builtin-functions.cpp b/clang/test/AST/ByteCode/builtin-functions.cpp index add919dacbf94..db8b5d984f531 100644 --- a/clang/test/AST/ByteCode/builtin-functions.cpp +++ b/clang/test/AST/ByteCode/builtin-functions.cpp @@ -1,17 +1,17 @@ -// RUN: %clang_cc1 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter -triple x86_64 %s -verify=expected,both -// RUN: %clang_cc1 -Wno-string-plus-int -Wno-stringop-overread -triple x86_64 %s -verify=ref,both +// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple x86_64 %s -verify=expected,both +// RUN: %clang_cc1 -Wno-string-plus-int -triple x86_64 %s -verify=ref,both // -// RUN: %clang_cc1 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter -triple i686 %s -verify=expected,both -// RUN: %clang_cc1 -Wno-string-plus-int -Wno-stringop-overread -triple i686 %s -verify=ref,both +// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify=expected,both +// RUN: %clang_cc1 -Wno-string-plus-int -triple i686 %s -verify=ref,both // -// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter -triple x86_64 %s -verify=expected,both -// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -triple x86_64 %s -verify=ref,both +// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple x86_64 %s -verify=expected,both +// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -triple x86_64 %s -verify=ref,both // -// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter -triple i686 %s -verify=expected,both -// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -triple i686 %s -verify=ref,both +// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify=expected,both +// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -triple i686 %s -verify=ref,both // -// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter %s -verify=expected,both -// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -verify=ref,both %s +// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify=expected,both +// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -verify=ref,both %s #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ #define LITTLE_END 1 @@ -1641,6 +1641,7 @@ namespace Memcmp { constexpr int onepasttheend(char a) { __builtin_memcmp(&a, &a + 1, 1); // both-note {{read of dereferenced one-past-the-end pointer}} + // ref-warning@-1 {{'memcmp' reading 1 byte from a region of size 0}} return 1; } static_assert(onepasttheend(10)); // both-error {{not an integral constant expression}} \ diff --git a/clang/test/Analysis/bstring.c b/clang/test/Analysis/bstring.c index bc56135a50a5d..b883481530fef 100644 --- a/clang/test/Analysis/bstring.c +++ b/clang/test/Analysis/bstring.c @@ -1,5 +1,4 @@ // RUN: %clang_analyze_cc1 -verify %s \ -// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix.cstring \ // RUN: -analyzer-checker=alpha.unix.cstring \ @@ -8,7 +7,6 @@ // RUN: -analyzer-config eagerly-assume=false // // RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS \ -// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix.cstring \ // RUN: -analyzer-checker=alpha.unix.cstring \ @@ -17,7 +15,6 @@ // RUN: -analyzer-config eagerly-assume=false // // RUN: %clang_analyze_cc1 -verify %s -DVARIANT \ -// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix.cstring \ // RUN: -analyzer-checker=alpha.unix.cstring \ @@ -26,7 +23,6 @@ // RUN: -analyzer-config eagerly-assume=false // // RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS -DVARIANT \ -// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix.cstring \ // RUN: -analyzer-checker=alpha.unix.cstring \ @@ -97,6 +93,9 @@ void memcpy1 (void) { char dst[10]; memcpy(dst, src, 5); // expected-warning{{Memory copy function accesses out-of-bound array element}} +#if !defined(VARIANT) || defined(USE_BUILTINS) + // expected-warning@-2{{'memcpy' reading 5 bytes from a region of size 4}} +#endif } void memcpy2 (void) { @@ -121,6 +120,9 @@ void memcpy4 (void) { char dst[10]; memcpy(dst+2, src+2, 3); // expected-warning{{Memory copy function accesses out-of-bound array element}} +#if !defined(VARIANT) || defined(USE_BUILTINS) + // expected-warning@-2{{'memcpy' reading 3 bytes from a region of size 2}} +#endif } void memcpy5(void) { @@ -223,6 +225,9 @@ void mempcpy1 (void) { char dst[10]; mempcpy(dst, src, 5); // expected-warning{{Memory copy function accesses out-of-bound array element}} +#if !defined(VARIANT) || defined(USE_BUILTINS) + // expected-warning@-2{{'mempcpy' reading 5 bytes from a region of size 4}} +#endif } void mempcpy2 (void) { @@ -247,6 +252,9 @@ void mempcpy4 (void) { char dst[10]; mempcpy(dst+2, src+2, 3); // expected-warning{{Memory copy function accesses out-of-bound array element}} +#if !defined(VARIANT) || defined(USE_BUILTINS) + // expected-warning@-2{{'mempcpy' reading 3 bytes from a region of size 2}} +#endif } void mempcpy5(void) { @@ -388,6 +396,9 @@ void memmove1 (void) { char dst[10]; memmove(dst, src, 5); // expected-warning{{out-of-bound}} +#if !defined(VARIANT) || defined(USE_BUILTINS) + // expected-warning@-2{{'memmove' reading 5 bytes from a region of size 4}} +#endif } void memmove2 (void) { @@ -430,6 +441,11 @@ void memcmp1 (void) { char b[10] = { 0 }; memcmp(a, b, 5); // expected-warning{{out-of-bound}} +#ifdef VARIANT + // expected-warning@-2{{'bcmp' reading 5 bytes from a region of size 4}} +#else + // expected-warning@-4{{'memcmp' reading 5 bytes from a region of size 4}} +#endif } void memcmp2 (void) { @@ -437,6 +453,11 @@ void memcmp2 (void) { char b[1] = { 0 }; memcmp(a, b, 4); // expected-warning{{out-of-bound}} +#ifdef VARIANT + // expected-warning@-2{{'bcmp' reading 4 bytes from a region of size 1}} +#else + // expected-warning@-4{{'memcmp' reading 4 bytes from a region of size 1}} +#endif } void memcmp3 (void) { diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c index efb2d52d3a321..591120af9ca48 100644 --- a/clang/test/Analysis/malloc.c +++ b/clang/test/Analysis/malloc.c @@ -1,6 +1,5 @@ // RUN: %clang_analyze_cc1 -Wno-strict-prototypes -Wno-error=implicit-int -verify %s \ // RUN: -Wno-alloc-size \ -// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=alpha.deadcode.UnreachableCode \ // RUN: -analyzer-checker=unix \ @@ -891,7 +890,7 @@ void overlappingMemcpyDoesNotSinkPath(char *s) { // Treat source buffer contents as escaped. void escapeSourceContents(char *s) { char *p = malloc(12); - memcpy(s, &p, 12); // no warning + memcpy(s, &p, 12); // expected-warning {{'memcpy' reading 12 bytes from a region of size 8}} void *p1 = malloc(7); char *a; diff --git a/clang/test/Analysis/pr22954.c b/clang/test/Analysis/pr22954.c index b3910da6c70ab..7e925c26317e1 100644 --- a/clang/test/Analysis/pr22954.c +++ b/clang/test/Analysis/pr22954.c @@ -3,7 +3,7 @@ // At the moment the whole of the destination array content is invalidated. // If a.s1 region has a symbolic offset, the whole region of 'a' is invalidated. // Specific triple set to test structures of size 0. -// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,unix.Malloc,debug.ExprInspection -Wno-error=int-conversion -Wno-stringop-overread -verify -analyzer-config eagerly-assume=false %s +// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,unix.Malloc,debug.ExprInspection -Wno-error=int-conversion -verify -analyzer-config eagerly-assume=false %s typedef __typeof(sizeof(int)) size_t; @@ -537,6 +537,7 @@ int f262(void) { a262.s2 = strdup("hello"); char input[] = {'a', 'b', 'c', 'd'}; memcpy(a262.s1, input, -1); // expected-warning{{'memcpy' will always overflow; destination buffer has size 16, but size argument is 18446744073709551615}} + // expected-warning@-1{{'memcpy' reading 18446744073709551615 bytes from a region of size 4}} clang_analyzer_eval(a262.s1[0] == 1); // expected-warning{{UNKNOWN}}\ expected-warning{{Potential leak of memory pointed to by 'a262.s2'}} clang_analyzer_eval(a262.s1[1] == 1); // expected-warning{{UNKNOWN}} >From eff22384e6e793e1d9522094269323df722f7b41 Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Thu, 26 Feb 2026 05:40:35 +0100 Subject: [PATCH 08/19] Revert enabling of warning --- clang/test/AST/ByteCode/builtin-functions.cpp | 21 +++++++------- clang/test/Analysis/bstring.c | 29 +++---------------- clang/test/Analysis/malloc.c | 3 +- clang/test/Analysis/pr22954.c | 3 +- 4 files changed, 17 insertions(+), 39 deletions(-) diff --git a/clang/test/AST/ByteCode/builtin-functions.cpp b/clang/test/AST/ByteCode/builtin-functions.cpp index db8b5d984f531..add919dacbf94 100644 --- a/clang/test/AST/ByteCode/builtin-functions.cpp +++ b/clang/test/AST/ByteCode/builtin-functions.cpp @@ -1,17 +1,17 @@ -// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple x86_64 %s -verify=expected,both -// RUN: %clang_cc1 -Wno-string-plus-int -triple x86_64 %s -verify=ref,both +// RUN: %clang_cc1 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter -triple x86_64 %s -verify=expected,both +// RUN: %clang_cc1 -Wno-string-plus-int -Wno-stringop-overread -triple x86_64 %s -verify=ref,both // -// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify=expected,both -// RUN: %clang_cc1 -Wno-string-plus-int -triple i686 %s -verify=ref,both +// RUN: %clang_cc1 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter -triple i686 %s -verify=expected,both +// RUN: %clang_cc1 -Wno-string-plus-int -Wno-stringop-overread -triple i686 %s -verify=ref,both // -// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple x86_64 %s -verify=expected,both -// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -triple x86_64 %s -verify=ref,both +// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter -triple x86_64 %s -verify=expected,both +// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -triple x86_64 %s -verify=ref,both // -// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify=expected,both -// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -triple i686 %s -verify=ref,both +// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter -triple i686 %s -verify=expected,both +// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -triple i686 %s -verify=ref,both // -// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify=expected,both -// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -verify=ref,both %s +// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -fexperimental-new-constant-interpreter %s -verify=expected,both +// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -Wno-stringop-overread -verify=ref,both %s #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ #define LITTLE_END 1 @@ -1641,7 +1641,6 @@ namespace Memcmp { constexpr int onepasttheend(char a) { __builtin_memcmp(&a, &a + 1, 1); // both-note {{read of dereferenced one-past-the-end pointer}} - // ref-warning@-1 {{'memcmp' reading 1 byte from a region of size 0}} return 1; } static_assert(onepasttheend(10)); // both-error {{not an integral constant expression}} \ diff --git a/clang/test/Analysis/bstring.c b/clang/test/Analysis/bstring.c index b883481530fef..bc56135a50a5d 100644 --- a/clang/test/Analysis/bstring.c +++ b/clang/test/Analysis/bstring.c @@ -1,4 +1,5 @@ // RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix.cstring \ // RUN: -analyzer-checker=alpha.unix.cstring \ @@ -7,6 +8,7 @@ // RUN: -analyzer-config eagerly-assume=false // // RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS \ +// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix.cstring \ // RUN: -analyzer-checker=alpha.unix.cstring \ @@ -15,6 +17,7 @@ // RUN: -analyzer-config eagerly-assume=false // // RUN: %clang_analyze_cc1 -verify %s -DVARIANT \ +// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix.cstring \ // RUN: -analyzer-checker=alpha.unix.cstring \ @@ -23,6 +26,7 @@ // RUN: -analyzer-config eagerly-assume=false // // RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS -DVARIANT \ +// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix.cstring \ // RUN: -analyzer-checker=alpha.unix.cstring \ @@ -93,9 +97,6 @@ void memcpy1 (void) { char dst[10]; memcpy(dst, src, 5); // expected-warning{{Memory copy function accesses out-of-bound array element}} -#if !defined(VARIANT) || defined(USE_BUILTINS) - // expected-warning@-2{{'memcpy' reading 5 bytes from a region of size 4}} -#endif } void memcpy2 (void) { @@ -120,9 +121,6 @@ void memcpy4 (void) { char dst[10]; memcpy(dst+2, src+2, 3); // expected-warning{{Memory copy function accesses out-of-bound array element}} -#if !defined(VARIANT) || defined(USE_BUILTINS) - // expected-warning@-2{{'memcpy' reading 3 bytes from a region of size 2}} -#endif } void memcpy5(void) { @@ -225,9 +223,6 @@ void mempcpy1 (void) { char dst[10]; mempcpy(dst, src, 5); // expected-warning{{Memory copy function accesses out-of-bound array element}} -#if !defined(VARIANT) || defined(USE_BUILTINS) - // expected-warning@-2{{'mempcpy' reading 5 bytes from a region of size 4}} -#endif } void mempcpy2 (void) { @@ -252,9 +247,6 @@ void mempcpy4 (void) { char dst[10]; mempcpy(dst+2, src+2, 3); // expected-warning{{Memory copy function accesses out-of-bound array element}} -#if !defined(VARIANT) || defined(USE_BUILTINS) - // expected-warning@-2{{'mempcpy' reading 3 bytes from a region of size 2}} -#endif } void mempcpy5(void) { @@ -396,9 +388,6 @@ void memmove1 (void) { char dst[10]; memmove(dst, src, 5); // expected-warning{{out-of-bound}} -#if !defined(VARIANT) || defined(USE_BUILTINS) - // expected-warning@-2{{'memmove' reading 5 bytes from a region of size 4}} -#endif } void memmove2 (void) { @@ -441,11 +430,6 @@ void memcmp1 (void) { char b[10] = { 0 }; memcmp(a, b, 5); // expected-warning{{out-of-bound}} -#ifdef VARIANT - // expected-warning@-2{{'bcmp' reading 5 bytes from a region of size 4}} -#else - // expected-warning@-4{{'memcmp' reading 5 bytes from a region of size 4}} -#endif } void memcmp2 (void) { @@ -453,11 +437,6 @@ void memcmp2 (void) { char b[1] = { 0 }; memcmp(a, b, 4); // expected-warning{{out-of-bound}} -#ifdef VARIANT - // expected-warning@-2{{'bcmp' reading 4 bytes from a region of size 1}} -#else - // expected-warning@-4{{'memcmp' reading 4 bytes from a region of size 1}} -#endif } void memcmp3 (void) { diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c index 591120af9ca48..efb2d52d3a321 100644 --- a/clang/test/Analysis/malloc.c +++ b/clang/test/Analysis/malloc.c @@ -1,5 +1,6 @@ // RUN: %clang_analyze_cc1 -Wno-strict-prototypes -Wno-error=implicit-int -verify %s \ // RUN: -Wno-alloc-size \ +// RUN: -Wno-stringop-overread \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=alpha.deadcode.UnreachableCode \ // RUN: -analyzer-checker=unix \ @@ -890,7 +891,7 @@ void overlappingMemcpyDoesNotSinkPath(char *s) { // Treat source buffer contents as escaped. void escapeSourceContents(char *s) { char *p = malloc(12); - memcpy(s, &p, 12); // expected-warning {{'memcpy' reading 12 bytes from a region of size 8}} + memcpy(s, &p, 12); // no warning void *p1 = malloc(7); char *a; diff --git a/clang/test/Analysis/pr22954.c b/clang/test/Analysis/pr22954.c index 7e925c26317e1..b3910da6c70ab 100644 --- a/clang/test/Analysis/pr22954.c +++ b/clang/test/Analysis/pr22954.c @@ -3,7 +3,7 @@ // At the moment the whole of the destination array content is invalidated. // If a.s1 region has a symbolic offset, the whole region of 'a' is invalidated. // Specific triple set to test structures of size 0. -// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,unix.Malloc,debug.ExprInspection -Wno-error=int-conversion -verify -analyzer-config eagerly-assume=false %s +// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,unix.Malloc,debug.ExprInspection -Wno-error=int-conversion -Wno-stringop-overread -verify -analyzer-config eagerly-assume=false %s typedef __typeof(sizeof(int)) size_t; @@ -537,7 +537,6 @@ int f262(void) { a262.s2 = strdup("hello"); char input[] = {'a', 'b', 'c', 'd'}; memcpy(a262.s1, input, -1); // expected-warning{{'memcpy' will always overflow; destination buffer has size 16, but size argument is 18446744073709551615}} - // expected-warning@-1{{'memcpy' reading 18446744073709551615 bytes from a region of size 4}} clang_analyzer_eval(a262.s1[0] == 1); // expected-warning{{UNKNOWN}}\ expected-warning{{Potential leak of memory pointed to by 'a262.s2'}} clang_analyzer_eval(a262.s1[1] == 1); // expected-warning{{UNKNOWN}} >From dc6fb2f15103022a3dd596f6000bf5dfbad2e529 Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Thu, 26 Feb 2026 06:36:18 +0100 Subject: [PATCH 09/19] Add uninstantiated template false negative --- clang/test/Sema/warn-stringop-overread.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/clang/test/Sema/warn-stringop-overread.c b/clang/test/Sema/warn-stringop-overread.c index 27298d663dc5e..a59573320c618 100644 --- a/clang/test/Sema/warn-stringop-overread.c +++ b/clang/test/Sema/warn-stringop-overread.c @@ -150,4 +150,15 @@ void test_memcpy_dependent_dest() { void call_test_memcpy_dependent_dest() { test_memcpy_dependent_dest<100>(); // expected-note {{in instantiation}} } + +// FIXME: We should warn here at the template definition since src and size are +// not dependent, but checkFortifiedBuiltinMemoryFunction exits when any part of +// the call is dependent (and thus uninstantiated). +template <int N> +void test_memcpy_dependent_dest_uninstantiated() { + char dst[N]; + int src = 0; + memcpy(dst, &src, sizeof(src) + 1); // missing-warning {{'memcpy' reading 5 bytes from a region of size 4}} +} + #endif >From cd5b1e5967824478257defbbcd179997491e9d70 Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Thu, 26 Feb 2026 20:19:37 +0100 Subject: [PATCH 10/19] Assert size arg is unsigned --- clang/lib/Sema/SemaChecking.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 4fe1fd795945c..339073ba00edc 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1185,7 +1185,8 @@ class FortifiedBufferChecker { if (!SizeArg->EvaluateAsInt(Result, S.getASTContext())) return std::nullopt; llvm::APSInt Integer = Result.Val.getInt(); - Integer.setIsUnsigned(true); + assert(Integer.isUnsigned() && + "size arg should be unsigned after implicit conversion to size_t"); return Integer; } >From c8e0c2944543fdd6ff759e2a26e0434332bd3a0c Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Mon, 2 Mar 2026 16:27:39 +0100 Subject: [PATCH 11/19] Add release note --- clang/docs/ReleaseNotes.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 11cce36a0906c..6dbf2ec3c9491 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -595,6 +595,9 @@ Improvements to Clang's diagnostics - Clang now rejects inline asm constraints and clobbers that contain an embedded null character, instead of silently truncating them. (#GH173900) +- Added ``-Wstringop-overread`` to warn when ``memcpy``, ``memmove``, ``memcmp``, + and related builtins read more bytes than the source buffer size (#GH83728). + Improvements to Clang's time-trace ---------------------------------- >From 4a47d25111d22f2d479b26e47a4f423c08ac6ac3 Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Tue, 17 Mar 2026 23:37:00 +0100 Subject: [PATCH 12/19] Use SIZE_TYPE for size_t in test Since we use the default triple for testing, we have to set size_t to the right value for Windows. --- clang/test/Sema/warn-stringop-overread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/Sema/warn-stringop-overread.c b/clang/test/Sema/warn-stringop-overread.c index a59573320c618..1c7876d4d33b9 100644 --- a/clang/test/Sema/warn-stringop-overread.c +++ b/clang/test/Sema/warn-stringop-overread.c @@ -3,7 +3,7 @@ // RUN: %clang_cc1 -xc++ %s -verify // RUN: %clang_cc1 -xc++ %s -verify -DUSE_BUILTINS -typedef unsigned long size_t; +typedef __SIZE_TYPE__ size_t; #ifdef __cplusplus extern "C" { >From bb45029d48b5a13dcad759e418da94e79ee1cd45 Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Wed, 18 Mar 2026 03:36:29 +0100 Subject: [PATCH 13/19] suppress warning on asan test --- compiler-rt/test/asan/TestCases/Windows/issue64990.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler-rt/test/asan/TestCases/Windows/issue64990.cpp b/compiler-rt/test/asan/TestCases/Windows/issue64990.cpp index 5222ec6e08191..7fccff0313c62 100644 --- a/compiler-rt/test/asan/TestCases/Windows/issue64990.cpp +++ b/compiler-rt/test/asan/TestCases/Windows/issue64990.cpp @@ -1,5 +1,5 @@ // Repro for the issue #64990: Asan with Windows EH generates __asan_xxx runtime calls without required funclet tokens -// RUN: %clang_cl_asan %Od %if MSVC %{ /Oi %} %s -EHsc %Fe%t +// RUN: %clang_cl_asan %Od %if MSVC %{ /Oi %} %s -EHsc -Wno-stringop-overread %Fe%t // RUN: not %run %t 2>&1 | FileCheck %s // UNSUPPORTED: target={{.*-windows-gnu}} >From 010d226c074405ee45091b90d72d164362181183 Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Wed, 1 Apr 2026 00:30:01 +0200 Subject: [PATCH 14/19] NFC changes --- clang/include/clang/Basic/DiagnosticGroups.td | 4 ++-- clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/lib/Sema/SemaChecking.cpp | 10 ++++------ 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 103d1ccd0306b..61db2364a25c5 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1838,8 +1838,8 @@ def CrossTURemarks : DiagGroup<"ctu-remarks">; def CTADMaybeUnsupported : DiagGroup<"ctad-maybe-unsupported">; -def FortifySource : DiagGroup<"fortify-source", [FormatOverflow, FormatTruncation]>; -def StringopOverread : DiagGroup<"stringop-overread">; +def FortifySource + : DiagGroup<"fortify-source", [FormatOverflow, FormatTruncation]>; def OverflowBehaviorAttributeIgnored : DiagGroup<"overflow-behavior-attribute-ignored">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 147eef2e120ff..ef5b70459c552 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -968,7 +968,7 @@ def warn_fortify_source_size_mismatch : Warning< def warn_stringop_overread : Warning<"'%0' reading %1 byte%s1 from a region of size %2">, - InGroup<StringopOverread>; + InGroup<DiagGroup<"stringop-overread">>; def warn_fortify_strlen_overflow: Warning< "'%0' will always overflow; destination buffer has size %1," diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 339073ba00edc..e39854e61ee19 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1163,7 +1163,7 @@ 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 == nullptr) // Not using DABAttr. + if (!DABAttr) return Index; unsigned DABIndices = DABAttr->argIndices_size(); unsigned NewIndex = Index < DABIndices @@ -1239,7 +1239,7 @@ class FortifiedBufferChecker { unsigned getSizeTypeWidth() const { return SizeTypeWidth; } /// Return function name after stripping __builtin_ and _chk affixes. - std::string GetFunctionName(unsigned BuiltinID, bool IsChkVariant) const { + std::string getFunctionName(unsigned BuiltinID, bool IsChkVariant) const { std::string Name = S.getASTContext().BuiltinInfo.getName(BuiltinID); llvm::StringRef Ref = Name; if (IsChkVariant) { @@ -1267,8 +1267,7 @@ void Sema::checkSourceBufferOverread(FunctionDecl *FD, CallExpr *TheCall, const Expr *SrcArg = TheCall->getArg(SrcArgIdx); const Expr *SizeArg = TheCall->getArg(SizeArgIdx); - if (SrcArg->isValueDependent() || SrcArg->isTypeDependent() || - SizeArg->isValueDependent() || SizeArg->isTypeDependent()) + if (SrcArg->isInstantiationDependent() || SizeArg->isInstantiationDependent()) return; FortifiedBufferChecker Checker(*this, FD, TheCall); @@ -1303,8 +1302,7 @@ void Sema::checkSourceBufferOverread(FunctionDecl *FD, CallExpr *TheCall, void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, CallExpr *TheCall) { - if (TheCall->isValueDependent() || TheCall->isTypeDependent() || - isConstantEvaluatedContext()) + if (TheCall->isInstantiationDependent() || isConstantEvaluatedContext()) return; FortifiedBufferChecker Checker(*this, FD, TheCall); >From 0404aae6a0aa78af801b52e1708884c1e5432b8e Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Wed, 1 Apr 2026 19:55:21 +0200 Subject: [PATCH 15/19] Add asserts to funcName logic --- clang/lib/Sema/SemaChecking.cpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index e39854e61ee19..10a063bc7065e 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1284,15 +1284,17 @@ void Sema::checkSourceBufferOverread(FunctionDecl *FD, CallExpr *TheCall, if (llvm::APSInt::compareValues(*CopyLen, *SrcBufSize) <= 0) return; - llvm::StringRef FuncName = "memory function"; - if (const FunctionDecl *CalleeDecl = TheCall->getDirectCallee()) { - FuncName = CalleeDecl->getName(); - // __builtin___memcpy_chk -> memcpy, __builtin_memcpy -> memcpy. - // The _chk variants have a different prefix so try that one first. - if (!(FuncName.consume_front("__builtin___") && - FuncName.consume_back("_chk"))) - FuncName.consume_front("__builtin_"); - } + const FunctionDecl *CalleeDecl = TheCall->getDirectCallee(); + assert(CalleeDecl && "expected builtin callee"); + StringRef FuncName = CalleeDecl->getName(); + + // Need to strip affixes from function name, see memcpy for example: + // __builtin___memcpy_chk, __builtin_memcpy + // The _chk variants have a different prefix so try that one first. + if (!(FuncName.consume_front("__builtin___") && + FuncName.consume_back("_chk"))) + FuncName.consume_front("__builtin_"); + assert(!FuncName.empty() && "expected non-empty function name"); DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, PDiag(diag::warn_stringop_overread) >From 6565b68baf6f48ba97df49c89171b0f60cc75cd2 Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Wed, 1 Apr 2026 20:01:13 +0200 Subject: [PATCH 16/19] Remove dead getFunctionName method --- clang/lib/Sema/SemaChecking.cpp | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 10a063bc7065e..8bb3514b2180e 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1238,19 +1238,6 @@ class FortifiedBufferChecker { const DiagnoseAsBuiltinAttr *getDABAttr() const { return DABAttr; } unsigned getSizeTypeWidth() const { return SizeTypeWidth; } - /// Return function name after stripping __builtin_ and _chk affixes. - std::string getFunctionName(unsigned BuiltinID, bool IsChkVariant) const { - std::string Name = S.getASTContext().BuiltinInfo.getName(BuiltinID); - llvm::StringRef Ref = Name; - if (IsChkVariant) { - Ref = Ref.drop_front(std::strlen("__builtin___")); - Ref = Ref.drop_back(std::strlen("_chk")); - } else { - Ref.consume_front("__builtin_"); - } - return Ref.str(); - } - private: Sema &S; CallExpr *TheCall; >From 684db8845f6c1ab7ae2ed6bec6b9bd3630ba28c6 Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Wed, 1 Apr 2026 20:49:37 +0200 Subject: [PATCH 17/19] Refactor (merge getFunctionName and GetFunctionName) --- clang/lib/Sema/SemaChecking.cpp | 77 ++++++++++++++------------------- 1 file changed, 33 insertions(+), 44 deletions(-) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 8bb3514b2180e..2cae87aca6af7 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1235,9 +1235,35 @@ class FortifiedBufferChecker { return std::nullopt; } - const DiagnoseAsBuiltinAttr *getDABAttr() const { return DABAttr; } unsigned getSizeTypeWidth() const { return SizeTypeWidth; } + unsigned getBuiltinID() const { + const FunctionDecl *UseDecl = FD; + if (DABAttr) { + UseDecl = DABAttr->getFunction(); + assert(UseDecl && "Missing FunctionDecl in DiagnoseAsBuiltin attribute!"); + } + return UseDecl->getBuiltinID(/*ConsiderWrappers=*/true); + } + + /// Return function name after stripping __builtin_ and _chk affixes. + std::string getFunctionName() const { + unsigned ID = getBuiltinID(); + if (!ID) { + // Use callee name directly if not a builtin. + const FunctionDecl *Callee = TheCall->getDirectCallee(); + assert(Callee && "expected callee"); + return Callee->getName().str(); + } + std::string Name = S.getASTContext().BuiltinInfo.getName(ID); + StringRef Ref = Name; + // Strip __builtin___*_chk or __builtin_ prefix. + if (!(Ref.consume_front("__builtin___") && Ref.consume_back("_chk"))) + Ref.consume_front("__builtin_"); + assert(!Ref.empty() && "expected non-empty function name"); + return Ref.str(); + } + private: Sema &S; CallExpr *TheCall; @@ -1271,17 +1297,7 @@ void Sema::checkSourceBufferOverread(FunctionDecl *FD, CallExpr *TheCall, if (llvm::APSInt::compareValues(*CopyLen, *SrcBufSize) <= 0) return; - const FunctionDecl *CalleeDecl = TheCall->getDirectCallee(); - assert(CalleeDecl && "expected builtin callee"); - StringRef FuncName = CalleeDecl->getName(); - - // Need to strip affixes from function name, see memcpy for example: - // __builtin___memcpy_chk, __builtin_memcpy - // The _chk variants have a different prefix so try that one first. - if (!(FuncName.consume_front("__builtin___") && - FuncName.consume_back("_chk"))) - FuncName.consume_front("__builtin_"); - assert(!FuncName.empty() && "expected non-empty function name"); + std::string FuncName = Checker.getFunctionName(); DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, PDiag(diag::warn_stringop_overread) @@ -1296,13 +1312,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, FortifiedBufferChecker Checker(*this, FD, TheCall); - const FunctionDecl *UseDecl = FD; - if (const auto *DABAttr = Checker.getDABAttr()) { - UseDecl = DABAttr->getFunction(); - assert(UseDecl && "Missing FunctionDecl in DiagnoseAsBuiltin attribute!"); - } - - unsigned BuiltinID = UseDecl->getBuiltinID(/*ConsiderWrappers=*/true); + unsigned BuiltinID = Checker.getBuiltinID(); if (!BuiltinID) return; @@ -1311,23 +1321,6 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, std::optional<llvm::APSInt> SourceSize; std::optional<llvm::APSInt> DestinationSize; unsigned DiagID = 0; - bool IsChkVariant = false; - - auto GetFunctionName = [&]() { - std::string FunctionNameStr = - getASTContext().BuiltinInfo.getName(BuiltinID); - llvm::StringRef FunctionName = FunctionNameStr; - // Skim off the details of whichever builtin was called to produce a better - // diagnostic, as it's unlikely that the user wrote the __builtin - // explicitly. - if (IsChkVariant) { - FunctionName = FunctionName.drop_front(std::strlen("__builtin___")); - FunctionName = FunctionName.drop_back(std::strlen("_chk")); - } else { - FunctionName.consume_front("__builtin_"); - } - return FunctionName.str(); - }; switch (BuiltinID) { default: @@ -1350,7 +1343,6 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, DiagID = diag::warn_fortify_strlen_overflow; SourceSize = Checker.ComputeStrLenArgument(1); DestinationSize = Checker.ComputeExplicitObjectSizeArgument(2); - IsChkVariant = true; break; } @@ -1376,7 +1368,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, unsigned SourceSize) { DiagID = diag::warn_fortify_scanf_overflow; unsigned Index = ArgIndex + DataIndex; - std::string FunctionName = GetFunctionName(); + std::string FunctionName = Checker.getFunctionName(); DiagRuntimeBehavior(TheCall->getArg(Index)->getBeginLoc(), TheCall, PDiag(DiagID) << FunctionName << (Index + 1) << DestSize << SourceSize); @@ -1417,7 +1409,6 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, .extOrTrunc(SizeTypeWidth); if (BuiltinID == Builtin::BI__builtin___sprintf_chk) { DestinationSize = Checker.ComputeExplicitObjectSizeArgument(2); - IsChkVariant = true; } else { DestinationSize = Checker.ComputeSizeArgument(0); } @@ -1441,7 +1432,6 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 2); DestinationSize = Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); - IsChkVariant = true; if (BuiltinID == Builtin::BI__builtin___memcpy_chk || BuiltinID == Builtin::BI__builtin___memmove_chk || @@ -1456,7 +1446,6 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, DiagID = diag::warn_builtin_chk_overflow; SourceSize = Checker.ComputeExplicitObjectSizeArgument(1); DestinationSize = Checker.ComputeExplicitObjectSizeArgument(3); - IsChkVariant = true; break; } @@ -1555,8 +1544,8 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, FormatSize.toString(FormatSizeStr, /*Radix=*/10); DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, PDiag(TruncationDiagID) - << GetFunctionName() << SpecifiedSizeStr - << FormatSizeStr); + << Checker.getFunctionName() + << SpecifiedSizeStr << FormatSizeStr); } } } @@ -1572,7 +1561,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, llvm::APSInt::compareValues(*SourceSize, *DestinationSize) <= 0) return; - std::string FunctionName = GetFunctionName(); + std::string FunctionName = Checker.getFunctionName(); SmallString<16> DestinationStr; SmallString<16> SourceStr; >From 4bdcb25860140be0e5d663e3b9fef9dab5620fca Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Tue, 19 May 2026 01:07:25 +0200 Subject: [PATCH 18/19] Fix crash on bzero/bcopy tests and add bcopy overread check --- clang/lib/Sema/SemaChecking.cpp | 7 +++++-- clang/test/Sema/warn-stringop-overread.c | 11 +++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 2cae87aca6af7..619f78f5d5f1a 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1482,9 +1482,11 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); DestinationSize = Checker.ComputeSizeArgument(0); - // Buffer overread doesn't make sense for memset. + // Buffer overread doesn't make sense for memset/bzero. if (BuiltinID != Builtin::BImemset && - BuiltinID != Builtin::BI__builtin_memset) { + BuiltinID != Builtin::BI__builtin_memset && + BuiltinID != Builtin::BIbzero && + BuiltinID != Builtin::BI__builtin_bzero) { checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/1, /*SizeArgIdx=*/2); } break; @@ -1495,6 +1497,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, SourceSize = Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); DestinationSize = Checker.ComputeSizeArgument(1); + checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/0, /*SizeArgIdx=*/2); break; } diff --git a/clang/test/Sema/warn-stringop-overread.c b/clang/test/Sema/warn-stringop-overread.c index 1c7876d4d33b9..e4a6289a1df2a 100644 --- a/clang/test/Sema/warn-stringop-overread.c +++ b/clang/test/Sema/warn-stringop-overread.c @@ -22,6 +22,7 @@ int memcmp(const void *s1, const void *s2, size_t n); #endif int bcmp(const void *s1, const void *s2, size_t n); +void bcopy(const void *src, void *dst, size_t n); #ifdef __cplusplus } @@ -127,6 +128,16 @@ void test_bcmp_no_warning(void) { bcmp(a, b, 10); // no warning } +void test_bcopy_overread(void) { + char src[4], dst[100]; + bcopy(src, dst, 8); // expected-warning {{'bcopy' reading 8 bytes from a region of size 4}} +} + +void test_bcopy_no_warning(void) { + char src[10], dst[10]; + bcopy(src, dst, 10); // no warning +} + void test_memcpy_chk_overread(void) { char dst[100]; char src[4]; >From 7974d252ff92ba0885ffa3086fc293330d274f65 Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Tue, 19 May 2026 01:43:37 +0200 Subject: [PATCH 19/19] Refactor check into FortifiedBufferChecker --- clang/include/clang/Basic/DiagnosticGroups.td | 3 +- clang/include/clang/Sema/Sema.h | 4 -- clang/lib/Sema/SemaChecking.cpp | 72 +++++++++---------- 3 files changed, 35 insertions(+), 44 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 61db2364a25c5..8031f99419bdc 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1838,8 +1838,7 @@ def CrossTURemarks : DiagGroup<"ctu-remarks">; def CTADMaybeUnsupported : DiagGroup<"ctad-maybe-unsupported">; -def FortifySource - : DiagGroup<"fortify-source", [FormatOverflow, FormatTruncation]>; +def FortifySource : DiagGroup<"fortify-source", [FormatOverflow, FormatTruncation]>; def OverflowBehaviorAttributeIgnored : DiagGroup<"overflow-behavior-attribute-ignored">; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 8082a26692f81..10d489dc96a34 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2936,10 +2936,6 @@ class Sema final : public SemaBase { CallExpr *TheCall, EltwiseBuiltinArgTyRestriction ArgTyRestr = EltwiseBuiltinArgTyRestriction::None); - /// Check for source buffer overread in memory functions. - void checkSourceBufferOverread(FunctionDecl *FD, CallExpr *TheCall, - unsigned SrcArgIdx, unsigned SizeArgIdx); - private: void CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr, const ArraySubscriptExpr *ASE = nullptr, diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 619f78f5d5f1a..44fc307508efb 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1264,6 +1264,34 @@ class FortifiedBufferChecker { return Ref.str(); } + /// Check for source buffer overread in memory functions. + void checkSourceOverread(unsigned SrcArgIdx, unsigned SizeArgIdx) { + if (S.isConstantEvaluatedContext()) + return; + + const Expr *SrcArg = TheCall->getArg(SrcArgIdx); + const Expr *SizeArg = TheCall->getArg(SizeArgIdx); + if (SrcArg->isInstantiationDependent() || + SizeArg->isInstantiationDependent()) + return; + + std::optional<llvm::APSInt> CopyLen = + ComputeExplicitObjectSizeArgument(SizeArgIdx); + std::optional<llvm::APSInt> SrcBufSize = ComputeSizeArgument(SrcArgIdx); + + if (!CopyLen || !SrcBufSize) + return; + + // Warn only if copy length exceeds source buffer size. + if (llvm::APSInt::compareValues(*CopyLen, *SrcBufSize) <= 0) + return; + + S.DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, + S.PDiag(diag::warn_stringop_overread) + << getFunctionName() << CopyLen->getZExtValue() + << SrcBufSize->getZExtValue()); + } + private: Sema &S; CallExpr *TheCall; @@ -1273,38 +1301,6 @@ class FortifiedBufferChecker { }; } // anonymous namespace -void Sema::checkSourceBufferOverread(FunctionDecl *FD, CallExpr *TheCall, - unsigned SrcArgIdx, unsigned SizeArgIdx) { - if (isConstantEvaluatedContext()) - return; - - const Expr *SrcArg = TheCall->getArg(SrcArgIdx); - const Expr *SizeArg = TheCall->getArg(SizeArgIdx); - if (SrcArg->isInstantiationDependent() || SizeArg->isInstantiationDependent()) - return; - - FortifiedBufferChecker Checker(*this, FD, TheCall); - - std::optional<llvm::APSInt> CopyLen = - Checker.ComputeExplicitObjectSizeArgument(SizeArgIdx); - std::optional<llvm::APSInt> SrcBufSize = - Checker.ComputeSizeArgument(SrcArgIdx); - - if (!CopyLen || !SrcBufSize) - return; - - // Warn only if copy length exceeds source buffer size. - if (llvm::APSInt::compareValues(*CopyLen, *SrcBufSize) <= 0) - return; - - std::string FuncName = Checker.getFunctionName(); - - DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, - PDiag(diag::warn_stringop_overread) - << FuncName << CopyLen->getZExtValue() - << SrcBufSize->getZExtValue()); -} - void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, CallExpr *TheCall) { if (TheCall->isInstantiationDependent() || isConstantEvaluatedContext()) @@ -1436,7 +1432,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, if (BuiltinID == Builtin::BI__builtin___memcpy_chk || BuiltinID == Builtin::BI__builtin___memmove_chk || BuiltinID == Builtin::BI__builtin___mempcpy_chk) { - checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/1, /*SizeArgIdx=*/2); + Checker.checkSourceOverread(/*SrcArgIdx=*/1, /*SizeArgIdx=*/2); } break; } @@ -1487,7 +1483,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, BuiltinID != Builtin::BI__builtin_memset && BuiltinID != Builtin::BIbzero && BuiltinID != Builtin::BI__builtin_bzero) { - checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/1, /*SizeArgIdx=*/2); + Checker.checkSourceOverread(/*SrcArgIdx=*/1, /*SizeArgIdx=*/2); } break; } @@ -1497,14 +1493,14 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, SourceSize = Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); DestinationSize = Checker.ComputeSizeArgument(1); - checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/0, /*SizeArgIdx=*/2); + Checker.checkSourceOverread(/*SrcArgIdx=*/0, /*SizeArgIdx=*/2); break; } // memchr(buf, val, size) case Builtin::BImemchr: case Builtin::BI__builtin_memchr: { - checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/0, /*SizeArgIdx=*/2); + Checker.checkSourceOverread(/*SrcArgIdx=*/0, /*SizeArgIdx=*/2); return; } @@ -1514,8 +1510,8 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, case Builtin::BI__builtin_memcmp: case Builtin::BIbcmp: case Builtin::BI__builtin_bcmp: { - checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/0, /*SizeArgIdx=*/2); - checkSourceBufferOverread(FD, TheCall, /*SrcArgIdx=*/1, /*SizeArgIdx=*/2); + Checker.checkSourceOverread(/*SrcArgIdx=*/0, /*SizeArgIdx=*/2); + Checker.checkSourceOverread(/*SrcArgIdx=*/1, /*SizeArgIdx=*/2); return; } case Builtin::BIsnprintf: _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
