https://github.com/pl98 updated https://github.com/llvm/llvm-project/pull/227108
>From c0b4c0860bc40f2e2706c4488227bf735e8477af Mon Sep 17 00:00:00 2001 From: Phoebe Liang <[email protected]> Date: Mon, 28 Sep 2026 16:16:23 -0400 Subject: [PATCH 1/4] [clang][-Wunsafe-buffer-usage] Warn on annotated unsafe container construction -Wunsafe-buffer-usage-in-container flags constructing a container or view from a decoupled pointer and bound, but only recognizes std::span and std::string_view, which are currently hardcoded. Equivalent APIs elsewhere (e.g., custom span types and factory functions) get no coverage, and safe argument pairs from non-standard containers are reported as false positives. - Libraries can opt in by annotating constructors and factory functions with [[clang::unsafe_buffer_usage_in_container]] (equivalently, [[clang::unsafe_buffer_usage(\"container\")]]). - A new UnsafeBufferUsageContainerAttrGadget matches annotated constructor and factory calls, reusing the existing bounds checks to suppress provably safe arguments. - Those checks now use duck typing rather than a hardcoded type list: x.data(), x.size() and x.begin(), x.end() are safe when called on the same object. This trades rare false negatives for far fewer false positives on user-defined containers. Unannotated code and existing std::span diagnostics are unaffected. --- clang/docs/ReleaseNotes.md | 13 ++ .../Analyses/UnsafeBufferUsageGadgets.def | 1 + clang/include/clang/Basic/Attr.td | 4 +- clang/include/clang/Basic/AttrDocs.td | 38 ++++ clang/lib/Analysis/UnsafeBufferUsage.cpp | 181 ++++++++++++++---- clang/lib/Sema/AnalysisBasedWarnings.cpp | 26 ++- clang/lib/Sema/SemaAPINotes.cpp | 2 +- clang/lib/Sema/SemaDeclAttr.cpp | 19 +- ...fe-buffer-usage-in-container-annotated.cpp | 163 ++++++++++++++++ ...ffer-usage-in-container-span-construct.cpp | 5 +- 10 files changed, 394 insertions(+), 58 deletions(-) create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-annotated.cpp diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index b71758e4b9647e..70d9afb67f8cf8 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -282,6 +282,11 @@ features cannot lower the translation-unit ABI level; - Clang now recognizes the `[[gnu::flag_enum]]` attribute and treats it equivalent to `[[clang::flag_enum]]` +- Added `[[clang::unsafe_buffer_usage_in_container]]` (and equivalent spelling + `[[clang::unsafe_buffer_usage("container")]]`) to allow two-parameter container + and view constructors and factory functions to opt in to + `-Wunsafe-buffer-usage-in-container` diagnostics. + ### Improvements to Clang's diagnostics - `-Wfortify-source` now diagnoses when `strlcat`, `__builtin_strlcat`, `strlcpy`, or @@ -485,6 +490,14 @@ features cannot lower the translation-unit ABI level; for pointer arithmetic on statically-sized arrays when the offset is a non-negative constant within the array bounds. +- `-Wunsafe-buffer-usage-in-container` now warns on unsafe calls to + two-parameter constructors and factory functions annotated with + `[[clang::unsafe_buffer_usage_in_container]]` or + `[[clang::unsafe_buffer_usage("container")]]`. In addition, the safe + `(.data(), .size())` and `(.begin(), .end())` argument checks now use duck + typing rather than a hardcoded type list, suppressing false positives when + both methods are called on the same user-defined container object. + - `-Wc++98-compat` now diagnoses explicit conversion functions in C++20 and later, matching the behavior in C++11 through C++17. (#GH161689) diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def index 38fd9d316b34c0..7e4d5772ee8685 100644 --- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def +++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def @@ -43,6 +43,7 @@ WARNING_OPTIONAL_GADGET(UnsafeLibcFunctionCall) WARNING_OPTIONAL_GADGET(UnsafeFormatAttributedFunctionCall) WARNING_OPTIONAL_GADGET(SpanTwoParamConstructor) // Uses of `std::span(arg0, arg1)` WARNING_OPTIONAL_GADGET(StringViewTwoParamConstructor) +WARNING_OPTIONAL_GADGET(UnsafeBufferUsageContainerAttr) FIXABLE_GADGET(ULCArraySubscript) // `DRE[any]` in an Unspecified Lvalue Context FIXABLE_GADGET(DerefSimplePtrArithFixable) FIXABLE_GADGET(PointerDereference) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index b9eb41654a81b0..b688d5f36583e5 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -5044,7 +5044,9 @@ def ReleaseHandle : InheritableParamAttr { } def UnsafeBufferUsage : InheritableAttr { - let Spellings = [Clang<"unsafe_buffer_usage">]; + let Spellings = [Clang<"unsafe_buffer_usage">, + Clang<"unsafe_buffer_usage_in_container">]; + let Args = [StringArgument<"Category", 1>]; let Subjects = SubjectList<[Function, Field]>; let Documentation = [UnsafeBufferUsageDocs]; } diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 0a8977d3c6b14f..4e2a396ec99c52 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -8433,6 +8433,44 @@ alternatives, though the attribute can be used even when the fix can't be automa Here, every read/write to the fields ptr1, ptr2, buf and sz will trigger a warning that the field has been explcitly marked as unsafe due to unsafe-buffer operations. +- Attribute attached to container constructors and factory functions: The + spellings `[[clang::unsafe_buffer_usage_in_container]]` and + `[[clang::unsafe_buffer_usage("container")]]` are equivalent and can be + placed on two-parameter constructors and factory functions of container or + view types (such as custom span types taking a `(pointer, size)` or + `(begin, end)` pair) to opt them in to `-Wunsafe-buffer-usage-in-container`. + + Unlike the general `[[clang::unsafe_buffer_usage]]` attribute, which warns on + every call, this form suppresses the warning when the argument pair is + provably safe—for example, when constructing from `c.data(), c.size()` or + `c.begin(), c.end()` on the same container object `c`, a constant-sized array + with a matching bound, `&var, 1`, or a `0` size: + + ```c++ + template <typename T> + class CustomSpan { + public: + [[clang::unsafe_buffer_usage_in_container]] + CustomSpan(T *ptr, size_t size); + + template <typename It> + [[clang::unsafe_buffer_usage("container")]] + CustomSpan(It first, It last); + }; + + template <typename T> + [[clang::unsafe_buffer_usage("container")]] + CustomSpan<T> MakeCustomSpan(T *ptr, size_t size); + + void example(int *p, size_t n, MyVector<int> &v) { + CustomSpan<int> s1(p, n); // warning: decoupled pointer and size + auto s2 = MakeCustomSpan(p, n); // warning: decoupled pointer and size + CustomSpan<int> s3(v.data(), v.size()); // no warning + CustomSpan<int> s4(v.begin(), v.end()); // no warning + auto s5 = MakeCustomSpan(v.data(), v.size()); // no warning + } + ``` + }]; } diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 9a4269acb1cdb8..4f081f57eeae9a 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -503,8 +503,8 @@ static const Expr *getSubExprInSizeOfExpr(const Expr &E) { // Providing that `Ptr` is a pointer and `Size` is an unsigned-integral // expression, returns true iff they follow one of the following safe // patterns: -// 1. Ptr is `DRE.data()` and Size is `DRE.size()`, where DRE is a hardened -// container or view; +// 1. Ptr is `DRE.data()` and Size is `DRE.size()` (or `DRE.size_bytes()` for +// char pointers), called on the same container or view object `DRE`; // // 2. Ptr is `a` and Size is `n`, where `a` is of an array-of-T with constant // size `n`; @@ -530,30 +530,24 @@ static bool isPtrBufferSafe(const Expr *Ptr, const Expr *Size, // 'b.size()' otherwise we do not know they match: if (DREOfPtr->getDecl() != DREOfSize->getDecl()) return false; - if (MCEPtr->getMethodDecl()->getName() != "data") + const auto *MDData = MCEPtr->getMethodDecl(); + const auto *MDSize = MCESize->getMethodDecl(); + if (!MDData || !MDSize) return false; - // `MCEPtr->getRecordDecl()` must be non-null as `DREOfPtr` is non-null: - if (!MCEPtr->getRecordDecl()->isInStdNamespace()) - return false; - - auto *ObjII = MCEPtr->getRecordDecl()->getIdentifier(); - - if (!ObjII) + if (MDData->getName() != "data") return false; bool AcceptSizeBytes = Ptr->getType()->getPointeeType()->isCharType(); - if (!((AcceptSizeBytes && - MCESize->getMethodDecl()->getName() == "size_bytes") || + if (!((AcceptSizeBytes && MDSize->getName() == "size_bytes") || // Note here the pointer must be a pointer-to-char type unless there // is explicit casting. If there is explicit casting, this branch // is unreachable. Thus, at this branch "size" and "size_bytes" are // equivalent as the pointer is a char pointer: - MCESize->getMethodDecl()->getName() == "size")) + MDSize->getName() == "size")) return false; - return llvm::is_contained({SIZED_CONTAINER_OR_VIEW_LIST}, - ObjII->getName()); + return true; } Expr::EvalResult ER; @@ -583,24 +577,25 @@ static bool isPtrBufferSafe(const Expr *Ptr, const Expr *Size, return false; } -// Given a two-param std::span construct call, matches iff the call has the -// following forms: -// 1. `std::span<T>{new T[n], n}`, where `n` is a literal or a DRE -// 2. `std::span<T>{new T, 1}` -// 3. `std::span<T>{ (char *)f(args), args[N] * arg*[M]}`, where +// Given the two arguments `(Arg0, Arg1)` of a container/view constructor or +// factory function call, returns true iff the arguments match one of the +// following safe forms: +// 1. `(new T[n], n)`, where `n` is a literal or a DRE +// 2. `(new T, 1)` +// 3. `((char *)f(args), args[N] * args[M])`, where // `f` is a function with attribute `alloc_size(N, M)`; // `args` represents the list of arguments; // `N, M` are parameter indexes to the allocating element number and size. // Sometimes, there is only one parameter index representing the total // size. -// 4. `std::span<T>{x.begin(), x.end()}` where `x` is an object in the -// SIZED_CONTAINER_OR_VIEW_LIST. -// 5. `isPtrBufferSafe` returns true for the two arguments of the span -// constructor -static bool isSafeSpanTwoParamConstruct(const CXXConstructExpr &Node, - ASTContext &Ctx) { +// 4. `(x.begin(), x.end())` where `begin()` and `end()` are called on the +// same container/view object `x`. +// 5. `isPtrBufferSafe` returns true for the two arguments. +template <typename CallOrConstructExpr> +static bool isSafeTwoParamContainerConstruct(const CallOrConstructExpr &Node, + ASTContext &Ctx) { assert(Node.getNumArgs() == 2 && - "expecting a two-parameter std::span constructor"); + "expecting a two-parameter container constructor or factory call"); const Expr *Arg0 = Node.getArg(0)->IgnoreParenImpCasts(); const Expr *Arg1 = Node.getArg(1)->IgnoreParenImpCasts(); auto HaveEqualConstantValues = [&Ctx](const Expr *E0, const Expr *E1) { @@ -674,14 +669,8 @@ static bool isSafeSpanTwoParamConstruct(const CXXConstructExpr &Node, auto IsMethodCallToSizedObject = [](const Stmt *Node, StringRef MethodName) { if (const auto *MC = dyn_cast<CXXMemberCallExpr>(Node)) { const auto *MD = MC->getMethodDecl(); - const auto *RD = MC->getRecordDecl(); - - if (RD && MD) - if (auto *II = RD->getDeclName().getAsIdentifierInfo(); - II && RD->isInStdNamespace()) - return llvm::is_contained({SIZED_CONTAINER_OR_VIEW_LIST}, - II->getName()) && - MD->getName() == MethodName; + if (MD && MD->getName() == MethodName) + return true; } return false; }; @@ -1883,7 +1872,7 @@ class SpanTwoParamConstructorGadget : public WarningGadget { auto HasTwoParamSpanCtorDecl = CRecordDecl->isInStdNamespace() && CDecl->getDeclName().getAsString() == "span" && CE->getNumArgs() == 2; - if (!HasTwoParamSpanCtorDecl || isSafeSpanTwoParamConstruct(*CE, Ctx)) + if (!HasTwoParamSpanCtorDecl || isSafeTwoParamContainerConstruct(*CE, Ctx)) return false; Result.addNode(SpanTwoParamConstructorTag, DynTypedNode::create(*CE)); return true; @@ -1985,6 +1974,99 @@ class StringViewTwoParamConstructorGadget : public WarningGadget { SmallVector<const Expr *, 1> getUnsafePtrs() const override { return {}; } }; +/// A call of a constructor or factory function annotated with +/// `[[clang::unsafe_buffer_usage("container")]]` (or +/// `[[clang::unsafe_buffer_usage_in_container]]`). Evaluates whether the +/// arguments are safe via `isSafeTwoParamContainerConstruct` and emits a +/// diagnostic under `-Wunsafe-buffer-usage-in-container` when unsafe. +class UnsafeBufferUsageContainerAttrGadget : public WarningGadget { + constexpr static const char *const OpTag = "container_attr_expr"; + const Expr *Op; + +public: + UnsafeBufferUsageContainerAttrGadget(const MatchResult &Result) + : WarningGadget(Kind::UnsafeBufferUsageContainerAttr), + Op(Result.getNodeAs<Expr>(OpTag)) {} + + static bool classof(const Gadget *G) { + return G->getKind() == Kind::UnsafeBufferUsageContainerAttr; + } + + // Returns true iff `Callee` is annotated with + // `[[clang::unsafe_buffer_usage("container")]]` and the arguments of `Node` + // are not provably safe. + template <typename CallOrConstructExpr> + static bool isUnsafeContainerConstruction(const Decl *Callee, + const CallOrConstructExpr &Node, + ASTContext &Ctx) { + if (!Callee) + return false; + const auto *Attr = Callee->getAttr<UnsafeBufferUsageAttr>(); + if (!Attr || Attr->getCategory() != "container") + return false; + return Node.getNumArgs() != 2 || + !isSafeTwoParamContainerConstruct(Node, Ctx); + } + + static bool matches(const Stmt *S, ASTContext &Ctx, + const UnsafeBufferUsageHandler *Handler, + MatchResult &Result) { + if (ignoreUnsafeBufferInContainer(*S, Handler)) + return false; + + // S is a constructor call. + if (const auto *CE = dyn_cast<CXXConstructExpr>(S)) { + // std::span(ptr, size) ctor is handled by SpanTwoParamConstructorGadget. + MatchResult Tmp; + if (SpanTwoParamConstructorGadget::matches(CE, Ctx, Tmp)) + return false; + + if (!isUnsafeContainerConstruction(CE->getConstructor(), *CE, Ctx)) + return false; + + Result.addNode(OpTag, DynTypedNode::create(*CE)); + return true; + } + // S is a factory function call. + if (const auto *Call = dyn_cast<CallExpr>(S)) { + if (!isUnsafeContainerConstruction(Call->getDirectCallee(), *Call, Ctx)) + return false; + + Result.addNode(OpTag, DynTypedNode::create(*Call)); + return true; + } + return false; + } + + void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler, + bool IsRelatedToDecl, + ASTContext &Ctx) const override { + Handler.handleUnsafeOperationInContainer(Op, IsRelatedToDecl, Ctx); + } + + SourceLocation getSourceLoc() const override { return Op->getBeginLoc(); } + + DeclUseList getClaimedVarUseSites() const override { + const Expr *Arg0 = nullptr; + + if (const auto *CE = dyn_cast<CXXConstructExpr>(Op); + CE && CE->getNumArgs() > 0) + Arg0 = CE->getArg(0); + else if (const auto *Call = dyn_cast<CallExpr>(Op); + Call && Call->getNumArgs() > 0) + Arg0 = Call->getArg(0); + + if (Arg0) + if (const auto *DRE = dyn_cast<DeclRefExpr>(Arg0->IgnoreParenImpCasts())) + if (isa<VarDecl>(DRE->getDecl())) + return {DRE}; + + return {}; + } + + SmallVector<const Expr *, 1> getUnsafePtrs() const override { return {}; } +}; + /// A pointer initialization expression of the form: /// \code /// int *p = q; @@ -2190,16 +2272,25 @@ class UnsafeBufferUsageAttrGadget : public WarningGadget { static bool matches(const Stmt *S, const ASTContext &Ctx, MatchResult &Result) { if (auto *CE = dyn_cast<CallExpr>(S)) { - if (CE->getDirectCallee() && - CE->getDirectCallee()->hasAttr<UnsafeBufferUsageAttr>()) { - Result.addNode(OpTag, DynTypedNode::create(*CE)); - return true; + if (const auto *Callee = CE->getDirectCallee()) { + if (const auto *Attr = Callee->getAttr<UnsafeBufferUsageAttr>()) { + // Skip if this is annotated with + // `[[clang::unsafe_buffer_usage("container")]]` as that case is + // handled by UnsafeBufferUsageContainerAttrGadget. + if (Attr->getCategory() == "container") + return false; + Result.addNode(OpTag, DynTypedNode::create(*CE)); + return true; + } } } if (auto *ME = dyn_cast<MemberExpr>(S)) { if (!isa<FieldDecl>(ME->getMemberDecl())) return false; - if (ME->getMemberDecl()->hasAttr<UnsafeBufferUsageAttr>()) { + if (const auto *Attr = + ME->getMemberDecl()->getAttr<UnsafeBufferUsageAttr>()) { + if (Attr->getCategory() == "container") + return false; Result.addNode(OpTag, DynTypedNode::create(*ME)); return true; } @@ -2237,7 +2328,13 @@ class UnsafeBufferUsageCtorAttrGadget : public WarningGadget { static bool matches(const Stmt *S, ASTContext &Ctx, MatchResult &Result) { const auto *CE = dyn_cast<CXXConstructExpr>(S); - if (!CE || !CE->getConstructor()->hasAttr<UnsafeBufferUsageAttr>()) + if (!CE) + return false; + const auto *Attr = CE->getConstructor()->getAttr<UnsafeBufferUsageAttr>(); + // Skip if this is annotated with + // `[[clang::unsafe_buffer_usage("container")]]` as that case is + // handled by UnsafeBufferUsageContainerAttrGadget. + if (!Attr || Attr->getCategory() == "container") return false; // std::span(ptr, size) ctor is handled by SpanTwoParamConstructorGadget. MatchResult Tmp; diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index d0500a6defd64a..1b1263b22be53f 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2590,18 +2590,26 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler { SourceLocation Loc; SourceRange Range; unsigned MsgParam = 0; + std::string ContainerName = "container"; - const auto *CtorExpr = cast<CXXConstructExpr>(Operation); - Loc = CtorExpr->getLocation(); - Range = CtorExpr->getSourceRange(); - - std::string ContainerName = "std::span"; - if (auto *TD = CtorExpr->getConstructor()->getParent()) { - // This will provide "std::span" if it's in the std namespace - ContainerName = TD->getQualifiedNameAsString(); + if (const auto *CtorExpr = dyn_cast<CXXConstructExpr>(Operation)) { + Loc = CtorExpr->getLocation(); + Range = CtorExpr->getSourceRange(); + if (auto *TD = CtorExpr->getConstructor()->getParent()) { + ContainerName = TD->getQualifiedNameAsString(); + } + } else if (const auto *Call = dyn_cast<CallExpr>(Operation)) { + Loc = Call->getExprLoc(); + Range = Call->getSourceRange(); + if (const auto *FD = Call->getDirectCallee()) { + ContainerName = FD->getQualifiedNameAsString(); + } + } else { + Loc = Operation->getBeginLoc(); + Range = Operation->getSourceRange(); } - // FIX: Pass the container name to fill the %0 parameter + // Pass the container name to fill the %0 parameter S.Diag(Loc, diag::warn_unsafe_buffer_usage_in_container) << ContainerName; if (IsRelatedToDecl) { diff --git a/clang/lib/Sema/SemaAPINotes.cpp b/clang/lib/Sema/SemaAPINotes.cpp index 4c7e5ea16cfd7d..c6e35cfdc6d4fd 100644 --- a/clang/lib/Sema/SemaAPINotes.cpp +++ b/clang/lib/Sema/SemaAPINotes.cpp @@ -589,7 +589,7 @@ static void ProcessAPINotes(Sema &S, FunctionOrMethod AnyFunc, // Add [[clang::unsafe_buffer_usage]] if (Info.UnsafeBufferUsage && !D->getAttr<UnsafeBufferUsageAttr>()) { handleAPINotedAttribute<UnsafeBufferUsageAttr>(S, D, true, Metadata, [&]() { - return UnsafeBufferUsageAttr::Create(S.getASTContext(), + return UnsafeBufferUsageAttr::Create(S.getASTContext(), "", getPlaceholderAttrInfo()); }); } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index eb4a8c2ab9ae01..bdeaffba33bc59 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -7226,9 +7226,22 @@ static void handleHandleAttr(Sema &S, Decl *D, const ParsedAttr &AL) { D->addAttr(Attr::Create(S.Context, Argument, AL)); } -template<typename Attr> static void handleUnsafeBufferUsage(Sema &S, Decl *D, const ParsedAttr &AL) { - D->addAttr(Attr::Create(S.Context, AL)); + StringRef Category; + if (AL.getAttrName()->getName() == "unsafe_buffer_usage_in_container") { + if (!AL.checkExactlyNumArgs(S, 0)) + return; + Category = "container"; + } else if (AL.getNumArgs() != 0) { + SourceLocation Loc; + if (!S.checkStringLiteralArgumentAttr(AL, 0, Category, &Loc)) + return; + if (Category != "container") { + S.Diag(Loc, diag::warn_attribute_type_not_supported) << AL << Category; + return; + } + } + D->addAttr(UnsafeBufferUsageAttr::Create(S.Context, Category, AL)); } static void handleCFGuardAttr(Sema &S, Decl *D, const ParsedAttr &AL) { @@ -8454,7 +8467,7 @@ ProcessDeclAttribute(Sema &S, Decl *D, const ParsedAttr &AL, break; case ParsedAttr::AT_UnsafeBufferUsage: - handleUnsafeBufferUsage<UnsafeBufferUsageAttr>(S, D, AL); + handleUnsafeBufferUsage(S, D, AL); break; case ParsedAttr::AT_UseHandle: diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-annotated.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-annotated.cpp new file mode 100644 index 00000000000000..79635d97d8a2bd --- /dev/null +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-annotated.cpp @@ -0,0 +1,163 @@ +// clang-format off +// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-container -verify %s +// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -Wno-unsafe-buffer-usage-in-container -verify=nowarn %s + +typedef unsigned long size_t; + +namespace custom { +template <typename T> +class MyVector { +public: + T* data() noexcept; + size_t size() const noexcept; + T* begin() noexcept; + T* end() noexcept; +}; +} // namespace custom + +template <typename T> +class CustomSpan { +public: + [[clang::unsafe_buffer_usage("container")]] + CustomSpan(T* ptr, size_t size); + + template <typename It> + [[clang::unsafe_buffer_usage("container")]] + CustomSpan(It first, It last); +}; + +template <typename T> +[[clang::unsafe_buffer_usage("container")]] +CustomSpan<T> MakeCustomSpan(T* ptr, size_t size) { + return CustomSpan<T>(ptr, size); +} + +template <typename T> +[[clang::unsafe_buffer_usage("container")]] +CustomSpan<T> MakeCustomSpan(T* first, T* last) { + return CustomSpan<T>(first, last); +} + +void test_constructor(int* p, size_t n, custom::MyVector<int>& vec, + custom::MyVector<int>& vec2) { + // Unsafe: decoupled pointer and size + CustomSpan<int> s1(p, n); // expected-warning{{the two-parameter CustomSpan construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + CustomSpan<int> s2(p, 10); // expected-warning{{the two-parameter CustomSpan construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + + // Unsafe: duck typing approach with .data() and .size() called on different custom container objects + CustomSpan<int> s_bad_data_size(vec.data(), vec2.size()); // expected-warning{{the two-parameter CustomSpan construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + + // Unsafe: mismatched or reversed .begin() and .end() + CustomSpan<int> s_bad_iter1(vec.begin(), vec2.end()); // expected-warning{{the two-parameter CustomSpan construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + CustomSpan<int> s_bad_iter2(vec.end(), vec.begin()); // expected-warning{{the two-parameter CustomSpan construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + + // Safe: duck typing approach with .data() and .size() called on the same custom container object + CustomSpan<int> s3(vec.data(), vec.size()); // no-warning + + // Safe: .begin() and .end() called on the same custom container object + CustomSpan<int> s4(vec.begin(), vec.end()); // no-warning + + // Safe: single element address + int x; + CustomSpan<int> s5(&x, 1); // no-warning + + // Safe: zero size + CustomSpan<int> s6(p, 0); // no-warning + + // Safe: constant array + int arr[10]; + CustomSpan<int> s7(arr, 10); // no-warning + + // Safe: new expression + CustomSpan<int> s8(new int[10], 10); // no-warning + CustomSpan<int> s9(new int[n], n); // no-warning +} + +void test_factory(int* p, size_t n, custom::MyVector<int>& vec, + custom::MyVector<int>& vec2) { + // Unsafe: decoupled pointer and size + auto s1 = MakeCustomSpan(p, n); // expected-warning{{the two-parameter MakeCustomSpan construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + auto s2 = MakeCustomSpan(p, 10); // expected-warning{{the two-parameter MakeCustomSpan construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + + // Unsafe: duck typing approach with .data() and .size() called on different custom container objects + auto s_bad_data_size = MakeCustomSpan(vec.data(), vec2.size()); // expected-warning{{the two-parameter MakeCustomSpan construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + + // Unsafe: mismatched or reversed .begin() and .end() + auto s_bad_iter1 = MakeCustomSpan(vec.begin(), vec2.end()); // expected-warning{{the two-parameter MakeCustomSpan construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + auto s_bad_iter2 = MakeCustomSpan(vec.end(), vec.begin()); // expected-warning{{the two-parameter MakeCustomSpan construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + + // Safe: duck typing approach with .data() and .size() called on the same custom container object + auto s3 = MakeCustomSpan(vec.data(), vec.size()); // no-warning + + // Safe: .begin() and .end() called on the same custom container object + auto s4 = MakeCustomSpan(vec.begin(), vec.end()); // no-warning + + // Safe: single element address + int x; + auto s5 = MakeCustomSpan(&x, 1); // no-warning + + // Safe: zero size + auto s6 = MakeCustomSpan(p, 0); // no-warning + + // Safe: constant array + int arr[10]; + auto s7 = MakeCustomSpan(arr, 10); // no-warning +} + +void test_pragma_suppression(int* p, size_t n) { +#pragma clang unsafe_buffer_usage begin + CustomSpan<int> s1(p, n); // no-warning + auto s2 = MakeCustomSpan(p, n); // no-warning +#pragma clang unsafe_buffer_usage end +} + +template <typename T> +class CustomSpanAlt { +public: + [[clang::unsafe_buffer_usage_in_container]] + CustomSpanAlt(T* ptr, size_t size); + + template <typename It> + [[clang::unsafe_buffer_usage_in_container]] + CustomSpanAlt(It first, It last); +}; + +template <typename T> +[[clang::unsafe_buffer_usage_in_container]] +CustomSpanAlt<T> MakeCustomSpanAlt(T* ptr, size_t size) { + return CustomSpanAlt<T>(ptr, size); +} + +template <typename T> +[[clang::unsafe_buffer_usage_in_container]] +CustomSpanAlt<T> MakeCustomSpanAlt(T* first, T* last) { + return CustomSpanAlt<T>(first, last); +} + +void test_in_container_spelling(int* p, size_t n, custom::MyVector<int>& vec, + custom::MyVector<int>& vec2) { + // Unsafe: decoupled pointer and size + CustomSpanAlt<int> s1(p, n); // expected-warning{{the two-parameter CustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + auto s2 = MakeCustomSpanAlt(p, n); // expected-warning{{the two-parameter MakeCustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + + // Unsafe: duck typing approach with .data() and .size() called on different custom container objects + CustomSpanAlt<int> s_bad_data_size1(vec.data(), vec2.size()); // expected-warning{{the two-parameter CustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + auto s_bad_data_size2 = MakeCustomSpanAlt(vec.data(), vec2.size()); // expected-warning{{the two-parameter MakeCustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + + // Unsafe: mismatched or reversed .begin() and .end() + CustomSpanAlt<int> s_bad_iter1(vec.begin(), vec2.end()); // expected-warning{{the two-parameter CustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + CustomSpanAlt<int> s_bad_iter2(vec.end(), vec.begin()); // expected-warning{{the two-parameter CustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + auto s_bad_iter3 = MakeCustomSpanAlt(vec.begin(), vec2.end()); // expected-warning{{the two-parameter MakeCustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + auto s_bad_iter4 = MakeCustomSpanAlt(vec.end(), vec.begin()); // expected-warning{{the two-parameter MakeCustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} + + // Safe: duck typing approach with .data() and .size() called on the same custom container object + CustomSpanAlt<int> s3(vec.data(), vec.size()); // no-warning + auto s4 = MakeCustomSpanAlt(vec.data(), vec.size()); // no-warning + + // Safe: .begin() and .end() called on the same custom container object + CustomSpanAlt<int> s5(vec.begin(), vec.end()); // no-warning + auto s6 = MakeCustomSpanAlt(vec.begin(), vec.end()); // no-warning +} + +[[clang::unsafe_buffer_usage("invalid")]] // expected-warning{{'clang::unsafe_buffer_usage' attribute argument not supported: invalid}} nowarn-warning{{'clang::unsafe_buffer_usage' attribute argument not supported: invalid}} +void test_unsupported_category(int* p, size_t n); diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp index 1e7855517207e5..03f23d0b75d363 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp @@ -274,16 +274,17 @@ namespace test_begin_end { int * begin(); int * end(); }; - void safe_cases(std::span<int> Sp, std::array<int, 10> Arr, std::string Str, std::initializer_list<Object> Il) { + void safe_cases(std::span<int> Sp, std::array<int, 10> Arr, std::string Str, + std::initializer_list<Object> Il, Object Obj) { std::span<int>{Sp.begin(), Sp.end()}; std::span<int>{Arr.begin(), Arr.end()}; std::span<char>{Str.begin(), Str.end()}; std::span<Object>{Il.begin(), Il.end()}; + std::span<int>{Obj.begin(), Obj.end()}; } void unsafe_cases(std::span<int> Sp, std::array<int, 10> Arr, std::string Str, std::initializer_list<Object> Il, Object Obj) { - std::span<int>{Obj.begin(), Obj.end()}; // expected-warning {{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}} std::span<int>{Sp.end(), Sp.begin()}; // expected-warning {{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}} std::span<int>{Sp.begin(), Arr.end()}; // expected-warning {{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}} } >From 4a3e4e0e105cc470850220f63c9d10da3ef553fb Mon Sep 17 00:00:00 2001 From: Phoebe Liang <[email protected]> Date: Mon, 28 Sep 2026 16:40:29 -0400 Subject: [PATCH 2/4] Specify Heading for UnsafeBufferUsageDocs in AttrDocs.td --- clang/include/clang/Basic/AttrDocs.td | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 4e2a396ec99c52..c16370eefc6565 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -8332,6 +8332,7 @@ zx_status_t zx_handle_close(zx_handle_t handle [[clang::release_handle("tag")]]) def UnsafeBufferUsageDocs : Documentation { let Category = DocCatFunction; + let Heading = "unsafe_buffer_usage"; let Content = [{ The attribute `[[clang::unsafe_buffer_usage]]` should be placed on functions that need to be avoided as they are prone to buffer overflows or unsafe buffer >From 8565bb0deb9a9cd57b65402251bd901211cdd5e2 Mon Sep 17 00:00:00 2001 From: Phoebe Liang <[email protected]> Date: Thu, 1 Oct 2026 17:27:00 -0400 Subject: [PATCH 3/4] Address review comments. --- clang/include/clang/Basic/AttrDocs.td | 2 +- clang/lib/Analysis/UnsafeBufferUsage.cpp | 5 ++-- ...fe-buffer-usage-in-container-annotated.cpp | 24 ++----------------- 3 files changed, 5 insertions(+), 26 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index c16370eefc6565..1450dba4032b68 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -8443,7 +8443,7 @@ alternatives, though the attribute can be used even when the fix can't be automa Unlike the general `[[clang::unsafe_buffer_usage]]` attribute, which warns on every call, this form suppresses the warning when the argument pair is - provably safe—for example, when constructing from `c.data(), c.size()` or + provably safe -- for example, when constructing from `c.data(), c.size()` or `c.begin(), c.end()` on the same container object `c`, a constant-sized array with a matching bound, `&var, 1`, or a `0` size: diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 4f081f57eeae9a..4c2ef0cace573a 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -669,8 +669,7 @@ static bool isSafeTwoParamContainerConstruct(const CallOrConstructExpr &Node, auto IsMethodCallToSizedObject = [](const Stmt *Node, StringRef MethodName) { if (const auto *MC = dyn_cast<CXXMemberCallExpr>(Node)) { const auto *MD = MC->getMethodDecl(); - if (MD && MD->getName() == MethodName) - return true; + return MD && MD->getName() == MethodName; } return false; }; @@ -2004,7 +2003,7 @@ class UnsafeBufferUsageContainerAttrGadget : public WarningGadget { const auto *Attr = Callee->getAttr<UnsafeBufferUsageAttr>(); if (!Attr || Attr->getCategory() != "container") return false; - return Node.getNumArgs() != 2 || + return Node.getNumArgs() == 2 && !isSafeTwoParamContainerConstruct(Node, Ctx); } diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-annotated.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-annotated.cpp index 79635d97d8a2bd..66d7ba0e121742 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-annotated.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-annotated.cpp @@ -111,6 +111,7 @@ void test_pragma_suppression(int* p, size_t n) { #pragma clang unsafe_buffer_usage end } +// Test the [[clang::unsafe_buffer_usage_in_container]] spelling. template <typename T> class CustomSpanAlt { public: @@ -128,35 +129,14 @@ CustomSpanAlt<T> MakeCustomSpanAlt(T* ptr, size_t size) { return CustomSpanAlt<T>(ptr, size); } -template <typename T> -[[clang::unsafe_buffer_usage_in_container]] -CustomSpanAlt<T> MakeCustomSpanAlt(T* first, T* last) { - return CustomSpanAlt<T>(first, last); -} - -void test_in_container_spelling(int* p, size_t n, custom::MyVector<int>& vec, - custom::MyVector<int>& vec2) { +void test_in_container_spelling(int* p, size_t n, custom::MyVector<int>& vec) { // Unsafe: decoupled pointer and size CustomSpanAlt<int> s1(p, n); // expected-warning{{the two-parameter CustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} auto s2 = MakeCustomSpanAlt(p, n); // expected-warning{{the two-parameter MakeCustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} - // Unsafe: duck typing approach with .data() and .size() called on different custom container objects - CustomSpanAlt<int> s_bad_data_size1(vec.data(), vec2.size()); // expected-warning{{the two-parameter CustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} - auto s_bad_data_size2 = MakeCustomSpanAlt(vec.data(), vec2.size()); // expected-warning{{the two-parameter MakeCustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} - - // Unsafe: mismatched or reversed .begin() and .end() - CustomSpanAlt<int> s_bad_iter1(vec.begin(), vec2.end()); // expected-warning{{the two-parameter CustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} - CustomSpanAlt<int> s_bad_iter2(vec.end(), vec.begin()); // expected-warning{{the two-parameter CustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} - auto s_bad_iter3 = MakeCustomSpanAlt(vec.begin(), vec2.end()); // expected-warning{{the two-parameter MakeCustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} - auto s_bad_iter4 = MakeCustomSpanAlt(vec.end(), vec.begin()); // expected-warning{{the two-parameter MakeCustomSpanAlt construction is unsafe as it can introduce mismatch between buffer size and the bound information}} - // Safe: duck typing approach with .data() and .size() called on the same custom container object CustomSpanAlt<int> s3(vec.data(), vec.size()); // no-warning auto s4 = MakeCustomSpanAlt(vec.data(), vec.size()); // no-warning - - // Safe: .begin() and .end() called on the same custom container object - CustomSpanAlt<int> s5(vec.begin(), vec.end()); // no-warning - auto s6 = MakeCustomSpanAlt(vec.begin(), vec.end()); // no-warning } [[clang::unsafe_buffer_usage("invalid")]] // expected-warning{{'clang::unsafe_buffer_usage' attribute argument not supported: invalid}} nowarn-warning{{'clang::unsafe_buffer_usage' attribute argument not supported: invalid}} >From 3b702398eaf38adb8b71e44a9053cd1f9eac127f Mon Sep 17 00:00:00 2001 From: Phoebe Liang <[email protected]> Date: Tue, 6 Oct 2026 17:54:14 -0400 Subject: [PATCH 4/4] Check Attr for no argument instead --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 4c2ef0cace573a..6fbb6f828167f3 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -2273,10 +2273,10 @@ class UnsafeBufferUsageAttrGadget : public WarningGadget { if (auto *CE = dyn_cast<CallExpr>(S)) { if (const auto *Callee = CE->getDirectCallee()) { if (const auto *Attr = Callee->getAttr<UnsafeBufferUsageAttr>()) { - // Skip if this is annotated with - // `[[clang::unsafe_buffer_usage("container")]]` as that case is - // handled by UnsafeBufferUsageContainerAttrGadget. - if (Attr->getCategory() == "container") + // Skip if this is annotated with a category (e.g., + // `[[clang::unsafe_buffer_usage("container")]]`) as that case is + // handled by its category-specific gadget. + if (!Attr->getCategory().empty()) return false; Result.addNode(OpTag, DynTypedNode::create(*CE)); return true; @@ -2288,7 +2288,7 @@ class UnsafeBufferUsageAttrGadget : public WarningGadget { return false; if (const auto *Attr = ME->getMemberDecl()->getAttr<UnsafeBufferUsageAttr>()) { - if (Attr->getCategory() == "container") + if (!Attr->getCategory().empty()) return false; Result.addNode(OpTag, DynTypedNode::create(*ME)); return true; @@ -2330,10 +2330,10 @@ class UnsafeBufferUsageCtorAttrGadget : public WarningGadget { if (!CE) return false; const auto *Attr = CE->getConstructor()->getAttr<UnsafeBufferUsageAttr>(); - // Skip if this is annotated with - // `[[clang::unsafe_buffer_usage("container")]]` as that case is - // handled by UnsafeBufferUsageContainerAttrGadget. - if (!Attr || Attr->getCategory() == "container") + // Skip if this is annotated with a category (e.g., + // `[[clang::unsafe_buffer_usage("container")]]`) as that case is + // handled by its category-specific gadget. + if (!Attr || !Attr->getCategory().empty()) return false; // std::span(ptr, size) ctor is handled by SpanTwoParamConstructorGadget. MatchResult Tmp; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
