Author: John Paul Jepko Date: 2026-07-16T17:01:38-04:00 New Revision: aaa4ebd2ad174920ddce67c4641292a44788e879
URL: https://github.com/llvm/llvm-project/commit/aaa4ebd2ad174920ddce67c4641292a44788e879 DIFF: https://github.com/llvm/llvm-project/commit/aaa4ebd2ad174920ddce67c4641292a44788e879.diff LOG: [Clang][Sema] Reland -Wstringop-overread with computeDependence fix (#208012) Reland #183004 and follow-up #205201 (reverted in #207840) which adds `-Wstringop-overread` to warn when memory functions `memcpy`, `memmove`, `memcmp`, and other related builtins read more bytes than the source buffer size. The revert was due to a crash when a memory function is called on `&x` where `x` is a static constexpr local inside a template. The original PR exposed a bug in `computeDependence(UnaryOperator*)` where only the value-dependence bit is set for this case, when the instantiation-dependence bit should also be set. This is the root cause behind the crash. Fix `computeDependence` to properly set both value and instantiation-dependence bits, and add a regression for this crash via `warn-stringop-overread.c`. Added: clang/test/Sema/warn-stringop-overread-fortify.c clang/test/Sema/warn-stringop-overread.c Modified: clang/docs/ReleaseNotes.md clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/AST/ComputeDependence.cpp clang/lib/Sema/SemaChecking.cpp clang/test/AST/ByteCode/builtin-functions.cpp clang/test/Analysis/bstring.c clang/test/Analysis/malloc.c clang/test/Analysis/pr22954.c clang/test/Sema/builtin-memcpy.c clang/test/Sema/builtin-object-size.c clang/test/Sema/warn-fortify-source.c clang/test/SemaCXX/warn-memset-bad-sizeof.cpp compiler-rt/test/asan/TestCases/Windows/issue64990.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 51dc07318e7ce..93e48944239af 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -123,6 +123,172 @@ latest release, please see the [Clang Web Site](https://clang.llvm.org) or the ### Improvements to Clang's diagnostics +- Fixed bug in `-Wdocumentation` so that it correctly handles explicit + function template instantiations (#64087). + +- Fixed concept template parameters not being recognized in `-Wdocumentation` + when mentioned in tparam comments. (#GH64087) + +- `-Wunused-but-set-variable` now diagnoses file-scope variables with + internal linkage (`static` storage class) that are assigned but never used. + This new coverage is added under the subgroup `-Wunused-but-set-global`, + allowing it to be disabled independently with `-Wno-unused-but-set-global`. + (#GH148361) + +- `-Wunused-template` is now part of `-Wunused` (which is enabled by `-Wall`). + It diagnoses unused function and variable templates with internal linkage, + which in a header is a latent ODR hazard. It can be disabled with + `-Wno-unused-template`. (#GH202945) + +- Added `-Wlifetime-safety` to enable lifetime safety analysis, + a CFG-based intra-procedural analysis that detects use-after-free and related + temporal safety bugs. See the + [RFC](https://discourse.llvm.org/t/rfc-intra-procedural-lifetime-analysis-in-clang/86291) + for more details. By design, this warning is enabled in `-Weverything`. To disable + the analysis, use `-Wno-lifetime-safety` or `-fno-lifetime-safety`. + +- Added `-Wlifetime-safety-suggestions` to enable lifetime annotation suggestions. + This provides suggestions for function parameters that + should be marked `[[clang::lifetimebound]]` based on lifetime analysis. For + example, for the following function: + + ```c++ + int* p(int *in) { return in; } + ``` + + Clang will suggest: + + ```c++ + warning: parameter in intra-TU function should be marked [[clang::lifetimebound]] + int* p(int *in) { return in; } + ^~~~~~~ + [[clang::lifetimebound]] + note: param returned here + int* p(int *in) { return in; } + ^~ + ``` + +- Added `-Wlifetime-safety-noescape` to detect misuse of `[[clang::noescape]]` + annotation where the parameter escapes through return. For example: + + ```c++ + int* p(int *in [[clang::noescape]]) { return in; } + ``` + + Clang will warn: + + ```c++ + warning: parameter is marked [[clang::noescape]] but escapes + int* p(int *in [[clang::noescape]]) { return in; } + ^~~~~~~ + note: returned here + int* p(int *in [[clang::noescape]]) { return in; } + ^~ + ``` + +- Added `-Wlifetime-safety-dangling-field` to detect dangling field references + when stack memory escapes to class fields. This is part of `-Wlifetime-safety` + and detects cases where local variables or parameters are stored in fields but + outlive their scope. For example: + + ```c++ + struct DanglingView { + std::string_view view; + DanglingView(std::string s) : view(s) {} // warning: address of stack memory escapes to a field + }; + ``` + +- Improved `-Wassign-enum` performance by caching enum enumerator values. (#GH176454) + +- Fixed a false negative in `-Warray-bounds` where the warning was suppressed + when accessing a member function on a past-the-end array element. + (#GH179128) + +- Added a missing space to the FixIt for the `implicit-int` group of diagnostics and + made sure that only one such diagnostic and FixIt is emitted per declaration group. (#GH179354) + +- Fixed the Fix-It insertion point for `expected ';' after alias declaration` + when parsing alias declarations involving a token-split `>>` sequence + (for example, `using A = X<int>>;`). (#GH184425) + +- Fixed incorrect `implicitly deleted` diagnostic for explicitly deleted + candidate function. (#GH185693) + +- The `-Wloop-analysis` warning has been extended to catch more cases of + variable modification inside lambda expressions (#GH132038). + +- Clang now emits `-Wsizeof-pointer-memaccess` when snprintf/vsnprintf use the sizeof + the destination buffer(dynamically allocated) in the len parameter(#GH162366) + +- Added `-Wmodule-map-path-outside-directory` (off by default) to warn on + header and umbrella directory paths that use `..` to refer outside the module + directory in module maps found via implicit search + (`-fimplicit-module-maps`). This does not affect module maps specified + explicitly via `-fmodule-map-file=`. + +- Honour `[[maybe_unused]]` attribute on private fields. + `-Wunused-private-field` no longer emits a warning for annotated private + fields. + +- Improved `-Wgnu-zero-variadic-macro-arguments` to suggest using + `__VA_OPT__` if the current language version supports it(#GH188624) + +- Clang now emits an error when implicitly casting a complex type to a built-in vector type. (#GH186805) + +- Added `-Wnonportable-include-path-separator` (off by default) to catch + #include directives that use backslashes as a path separator. The warning + includes a FixIt to change all the backslashes to forward slashes, so that the + code can automatically be made portable to other host platforms that don't + support backslashes. + +- Clang now explains why template deduction fails for explicit template arguments. + +- No longer emitting a `-Wpre-c2y-compat` or extension diagnostic about use + of octal literals with a `0o` prefix, and no longer emitting a + `-Wdeprecated-octal-literals` diagnostic for use of octal literals without + a `0o` prefix, when the literal is expanded from a macro defined in a + system header. (#GH192389) + +- Improved error recovery for missing semicolons after class members. Clang now avoids + skipping subsequent valid declarations when their previous decl is missing semicolon. + +- Removed the body of lambdas from some diagnostic messages. + +- Fixed false positive host-device mismatch errors in discarded `if constexpr` branches for CUDA/HIP; + such calls are now correctly skipped. + +- Clang now errors when a function declaration aliases a variable or vice versa. (#GH195550) + +- Added `-Wattribute-alias` to diagnose type mismatches between an alias and its aliased function. (#GH195550) + +- The diagnostics around `__block` now explain why a variable cannot be marked `__block`. (#GH197213) + +- Extended `-Wnonportable-include-path` to warn about trailing whitespace and dots in `#include` paths. (#GH190610) + +- Clang now emits error when attribute is missing closing `]]` followed by `;;`. (#GH187223) + +- 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). + +- Diagnostics for the C++11 range-based for statement now report the correct + iterator type in notes for invalid iterator types. + +- `-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 + bionic libc `diagnose_if` check). + +- Improved how Unicode characters are displayed in diagnostic messages. + +- `-Wtautological-pointer-compare` and `-Wpointer-bool-conversion` now + diagnose a reference to a function (e.g. of type `void (&)()`) compared + against or converted to a null pointer, the same as a bare function name. + (#GH46362) + + ### Improvements to Clang's time-trace ### Improvements to Coverage Mapping diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index f14288dd2967d..371ad4535e95f 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -981,6 +981,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<DiagGroup<"stringop-overread">>; + 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/lib/AST/ComputeDependence.cpp b/clang/lib/AST/ComputeDependence.cpp index 95cfd19c0bcc8..a819bb6dec599 100644 --- a/clang/lib/AST/ComputeDependence.cpp +++ b/clang/lib/AST/ComputeDependence.cpp @@ -64,7 +64,7 @@ ExprDependence clang::computeDependence(UnaryOperator *E, if (VD && VD->isTemplated()) { auto *VarD = dyn_cast<VarDecl>(VD); if (!VarD || !VarD->hasLocalStorage()) - Dep |= ExprDependence::Value; + Dep |= ExprDependence::ValueInstantiation; } } } diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 2a8395e26d0bd..ef07e8c6133ed 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1147,36 +1147,23 @@ 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) { + 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 // function. - if (!UseDABAttr) + if (!DABAttr) return Index; unsigned DABIndices = DABAttr->argIndices_size(); unsigned NewIndex = Index < DABIndices @@ -1185,25 +1172,25 @@ 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); + assert(Integer.isUnsigned() && + "size arg should be unsigned after implicit conversion to size_t"); 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,33 +1228,95 @@ 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; - }; + } + + 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(); + } + + /// 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; + FunctionDecl *FD; + const DiagnoseAsBuiltinAttr *DABAttr; + unsigned SizeTypeWidth; +}; +} // anonymous namespace + +void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, + CallExpr *TheCall) { + if (TheCall->isInstantiationDependent() || isConstantEvaluatedContext()) + return; + + FortifiedBufferChecker Checker(*this, FD, TheCall); + + unsigned BuiltinID = Checker.getBuiltinID(); + if (!BuiltinID) + return; + + unsigned SizeTypeWidth = Checker.getSizeTypeWidth(); 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: @@ -1280,8 +1328,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,9 +1337,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); - IsChkVariant = true; + SourceSize = Checker.ComputeStrLenArgument(1); + DestinationSize = Checker.ComputeExplicitObjectSizeArgument(2); break; } @@ -1317,14 +1364,14 @@ 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); }; auto ShiftedComputeSizeArgument = [&](unsigned Index) { - return ComputeSizeArgument(Index + DataIndex); + return Checker.ComputeSizeArgument(Index + DataIndex); }; ScanfDiagnosticFormatHandler H(ShiftedComputeSizeArgument, Diagnose); const char *FormatBytes = FormatStrRef.data(); @@ -1357,10 +1404,9 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, SourceSize = llvm::APSInt::getUnsigned(H.getSizeLowerBound()) .extOrTrunc(SizeTypeWidth); if (BuiltinID == Builtin::BI__builtin___sprintf_chk) { - DestinationSize = ComputeExplicitObjectSizeArgument(2); - IsChkVariant = true; + DestinationSize = Checker.ComputeExplicitObjectSizeArgument(2); } else { - DestinationSize = ComputeSizeArgument(0); + DestinationSize = Checker.ComputeSizeArgument(0); } break; } @@ -1378,19 +1424,24 @@ 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); - IsChkVariant = true; + Checker.ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); + + if (BuiltinID == Builtin::BI__builtin___memcpy_chk || + BuiltinID == Builtin::BI__builtin___memmove_chk || + BuiltinID == Builtin::BI__builtin___mempcpy_chk) { + Checker.checkSourceOverread(/*SrcArgIdx=*/1, /*SizeArgIdx=*/2); + } break; } case Builtin::BI__builtin___snprintf_chk: case Builtin::BI__builtin___vsnprintf_chk: { DiagID = diag::warn_builtin_chk_overflow; - SourceSize = ComputeExplicitObjectSizeArgument(1); - DestinationSize = ComputeExplicitObjectSizeArgument(3); - IsChkVariant = true; + SourceSize = Checker.ComputeExplicitObjectSizeArgument(1); + DestinationSize = Checker.ComputeExplicitObjectSizeArgument(3); break; } @@ -1406,8 +1457,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,23 +1474,52 @@ 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); + + // Buffer overread doesn't make sense for memset/bzero. + if (BuiltinID != Builtin::BImemset && + BuiltinID != Builtin::BI__builtin_memset && + BuiltinID != Builtin::BIbzero && + BuiltinID != Builtin::BI__builtin_bzero) { + Checker.checkSourceOverread(/*SrcArgIdx=*/1, /*SizeArgIdx=*/2); + } 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); + Checker.checkSourceOverread(/*SrcArgIdx=*/0, /*SizeArgIdx=*/2); break; } + + // memchr(buf, val, size) + case Builtin::BImemchr: + case Builtin::BI__builtin_memchr: { + Checker.checkSourceOverread(/*SrcArgIdx=*/0, /*SizeArgIdx=*/2); + 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: { + Checker.checkSourceOverread(/*SrcArgIdx=*/0, /*SizeArgIdx=*/2); + Checker.checkSourceOverread(/*SrcArgIdx=*/1, /*SizeArgIdx=*/2); + return; + } case Builtin::BIsnprintf: case Builtin::BI__builtin_snprintf: 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; @@ -1462,12 +1543,12 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, FormatSize.toString(FormatSizeStr, /*Radix=*/10); DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, PDiag(TruncationDiagID) - << GetFunctionName() << SpecifiedSizeStr - << FormatSizeStr); + << Checker.getFunctionName() + << SpecifiedSizeStr << FormatSizeStr); } } } - 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(); @@ -1479,7 +1560,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; diff --git a/clang/test/AST/ByteCode/builtin-functions.cpp b/clang/test/AST/ByteCode/builtin-functions.cpp index d6990d1725072..b14ae91c753cb 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 b337c71eb02c7..7c33b09409725 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-disable-checker=unix.cstring.UninitializedRead \ @@ -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-disable-checker=unix.cstring.UninitializedRead \ @@ -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-disable-checker=unix.cstring.UninitializedRead \ @@ -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-disable-checker=unix.cstring.UninitializedRead \ diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c index 4ee09d9022bee..6c3dadfd16021 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 530fa8104bd56..0a6c44f59af9e 100644 --- a/clang/test/Sema/warn-fortify-source.c +++ b/clang/test/Sema/warn-fortify-source.c @@ -100,7 +100,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}} } @@ -271,11 +271,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}} + memcpy(bufferA, bufferB, 10); } 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}} + // 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 diff --git a/clang/test/Sema/warn-stringop-overread-fortify.c b/clang/test/Sema/warn-stringop-overread-fortify.c new file mode 100644 index 0000000000000..6a3d2425e0e16 --- /dev/null +++ b/clang/test/Sema/warn-stringop-overread-fortify.c @@ -0,0 +1,29 @@ +// Check that _FORTIFY_SOURCE can interoperate with -Wstringop-overread without +// interfering with one another. We model it here like glibc does so this test +// is target independent. +// +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -D_FORTIFY_SOURCE=1 %s -verify +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -D_FORTIFY_SOURCE=2 %s -verify +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -D_FORTIFY_SOURCE=3 %s -verify +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -D_FORTIFY_SOURCE=2 -fsanitize=address %s -verify +// +// Confirm we actually see _chk wrappers: +// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -D_FORTIFY_SOURCE=2 %s -o - | FileCheck %s + +typedef __SIZE_TYPE__ size_t; + +#if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 0 +extern __inline __attribute__((__always_inline__, __gnu_inline__)) +void *memcpy(void *dst, const void *src, size_t len) { + return __builtin___memcpy_chk(dst, src, len, __builtin_object_size(dst, 0)); +} +#else +void *memcpy(void *dst, const void *src, size_t c); +#endif + +void test(void) { + char dst[100]; + char src[4]; + // CHECK: call {{.*}}@__memcpy_chk + memcpy(dst, src, 8); // expected-warning {{'memcpy' reading 8 bytes from a region of size 4}} +} diff --git a/clang/test/Sema/warn-stringop-overread.c b/clang/test/Sema/warn-stringop-overread.c new file mode 100644 index 0000000000000..f3b70c000581b --- /dev/null +++ b/clang/test/Sema/warn-stringop-overread.c @@ -0,0 +1,211 @@ +// 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 __SIZE_TYPE__ 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 + +int bcmp(const void *s1, const void *s2, size_t n); +void bcopy(const void *src, void *dst, size_t n); + +#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 +} + +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_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]; + __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}} +} + +#ifdef __cplusplus + +// Mimic how <cstring> brings C library memory functions into the std namespace +// via using-declarations. -Wstringop-overread should fire on them too. Skipped +// under USE_BUILTINS, where these names are function-like macros. +#ifndef USE_BUILTINS +namespace std { +using ::memcpy; +using ::memmove; +} + +void test_std_memcpy_overread() { + char dst[100]; + char src[4]; + std::memcpy(dst, src, 8); // expected-warning {{'memcpy' reading 8 bytes from a region of size 4}} +} + +void test_std_memmove_overread() { + char dst[100]; + char src[4]; + std::memmove(dst, src, 8); // expected-warning {{'memmove' reading 8 bytes from a region of size 4}} +} +#endif + +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}} +} + +// 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}} +} + +// Taking the address of a static constexpr local inside a template is +// value-dependent. This regression test ensures stringop-overread doesn't crash +// when trying to constant-evaluate such an operand. +template <typename T> +void test_addr_of_static_constexpr_local() { + static constexpr bool true_value = true; + memcmp(&true_value, &true_value, 1); +} + +void call_test_addr_of_static_constexpr_local() { + test_addr_of_static_constexpr_local<int>(); +} + +#endif diff --git a/clang/test/SemaCXX/warn-memset-bad-sizeof.cpp b/clang/test/SemaCXX/warn-memset-bad-sizeof.cpp index 6f1cd4dd639ec..405652e3daf5e 100644 --- a/clang/test/SemaCXX/warn-memset-bad-sizeof.cpp +++ b/clang/test/SemaCXX/warn-memset-bad-sizeof.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -Wno-sizeof-array-argument %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-sizeof-array-argument -Wno-stringop-overread %s // extern "C" void *bzero(void *, unsigned); extern "C" void *memset(void *, int, unsigned); 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}} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
