[clang] ffdf216 - Revert "[Clang][UnsafeBufferUsage] Warn about two-arg string_view constructors. (#180471)" (#185692)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 10 15:00:29 PDT 2026
Author: Nico Weber
Date: 2026-03-10T18:00:24-04:00
New Revision: ffdf216cdada5d3caa844854c40290f1c61f4bfd
URL: https://github.com/llvm/llvm-project/commit/ffdf216cdada5d3caa844854c40290f1c61f4bfd
DIFF: https://github.com/llvm/llvm-project/commit/ffdf216cdada5d3caa844854c40290f1c61f4bfd.diff
LOG: Revert "[Clang][UnsafeBufferUsage] Warn about two-arg string_view constructors. (#180471)" (#185692)
This reverts commit 75b2ea57d5f4a5ae0de1b3ca1ca7eec464811b45.
Makes clang assert, see:
https://github.com/llvm/llvm-project/pull/180471#issuecomment-4033081814
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
Removed:
clang/test/SemaCXX/warn-unsafe-buffer-usage-string-view.cpp
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a024d19fcd363..bf3e8646de041 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -282,9 +282,6 @@ Improvements to Clang's diagnostics
- Clang now emits ``-Wsizeof-pointer-memaccess`` when snprintf/vsnprintf use the sizeof
the destination buffer(dynamically allocated) in the len parameter(#GH162366)
-- ``-Wunsafe-buffer-usage`` now warns about unsafe two-parameter constructors of
- ``std::string_view`` (pointer and size), consistent with the existing warning for ``std::span``.
-
- 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
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index bffb45022b8bc..876682ad779d4 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -129,10 +129,6 @@ class UnsafeBufferUsageHandler {
bool IsRelatedToDecl,
ASTContext &Ctx) = 0;
- virtual void handleUnsafeOperationInStringView(const Stmt *Operation,
- bool IsRelatedToDecl,
- ASTContext &Ctx) = 0;
-
/// Invoked when a fix is suggested against a variable. This function groups
/// all variables that must be fixed together (i.e their types must be changed
/// to the same target type to prevent type mismatches) into a single fixit.
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index 7ce3c5f0fc7c5..129ce95c1c0e0 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -42,7 +42,6 @@ WARNING_OPTIONAL_GADGET(ArraySubscript)
WARNING_OPTIONAL_GADGET(UnsafeLibcFunctionCall)
WARNING_OPTIONAL_GADGET(UnsafeFormatAttributedFunctionCall)
WARNING_OPTIONAL_GADGET(SpanTwoParamConstructor) // Uses of `std::span(arg0, arg1)`
-WARNING_OPTIONAL_GADGET(StringViewTwoParamConstructor)
FIXABLE_GADGET(ULCArraySubscript) // `DRE[any]` in an Unspecified Lvalue Context
FIXABLE_GADGET(DerefSimplePtrArithFixable)
FIXABLE_GADGET(PointerDereference)
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 22af8eec76f34..5d39f12d5c00f 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1922,5 +1922,3 @@ def TrivialAutoVarInit : DiagGroup<"trivial-auto-var-init">;
// A warning for options that enable a feature that is not yet complete
def ExperimentalOption : DiagGroup<"experimental-option">;
-
-def UnsafeBufferUsageInStringView : DiagGroup<"unsafe-buffer-usage-in-string-view">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 94d8566636c36..ecdeb67e14a95 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -13703,12 +13703,8 @@ def note_unsafe_buffer_variable_fixit_together : Note<
def note_safe_buffer_usage_suggestions_disabled : Note<
"pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions">;
def warn_unsafe_buffer_usage_in_container : Warning<
- "the two-parameter %0 construction is unsafe as it can introduce mismatch "
- "between buffer size and the bound information">,
+ "the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information">,
InGroup<UnsafeBufferUsageInContainer>, DefaultIgnore;
-def warn_unsafe_buffer_usage_in_string_view : Warning<
- "the two-parameter std::string_view construction is unsafe">,
- InGroup<UnsafeBufferUsageInStringView>, DefaultIgnore;
def warn_unsafe_buffer_usage_unique_ptr_array_access : Warning<"direct access using operator[] on std::unique_ptr<T[]> is unsafe due to lack of bounds checking">,
InGroup<UnsafeBufferUsageInUniquePtrArrayAccess>, DefaultIgnore;
def warn_unsafe_buffer_usage_in_static_sized_array : Warning<"direct access on T[N] is unsafe due to the lack of bounds checking">,
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index eee58ebf571bd..133e39b8fac2b 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -699,71 +699,6 @@ static bool isSafeSpanTwoParamConstruct(const CXXConstructExpr &Node,
return isPtrBufferSafe(Arg0, Arg1, Ctx);
}
-static bool isSafeStringViewTwoParamConstruct(const CXXConstructExpr &Node,
- ASTContext &Ctx) {
- const Expr *Arg0 = Node.getArg(0)->IgnoreParenImpCasts();
- const Expr *Arg1 = Node.getArg(1)->IgnoreParenImpCasts();
-
- // Pattern 1: String Literals
- if (const auto *SL = dyn_cast<StringLiteral>(Arg0)) {
- if (auto ArgSize = Arg1->getIntegerConstantExpr(Ctx)) {
- if (llvm::APSInt::compareValues(
- llvm::APSInt::getUnsigned(SL->getLength()), *ArgSize) >= 0)
- return true;
- return false; // Explicitly unsafe if size > length
- }
- }
-
- // Pattern 2: Constant Arrays
- if (const auto *CAT = Ctx.getAsConstantArrayType(Arg0->getType())) {
- if (auto ArgSize = Arg1->getIntegerConstantExpr(Ctx)) {
- if (llvm::APSInt::compareValues(llvm::APSInt(CAT->getSize(), true),
- *ArgSize) >= 0)
- return true;
- return false; // Explicitly unsafe if size > ArraySize
- }
- }
-
- // Pattern 3: Zero length
- if (auto Val = Arg1->getIntegerConstantExpr(Ctx)) {
- if (Val->isZero())
- return true;
- }
-
- // Pattern 4: string_view(it, it) - Only safe if it's .begin() and .end() of
- // the SAME object
- auto GetContainerObj = [](const Expr *E) -> const Expr * {
- E = E->IgnoreParenImpCasts();
- if (const auto *MCE = dyn_cast<CXXMemberCallExpr>(E)) {
- const auto *MD = MCE->getMethodDecl();
- if (MD && (MD->getName() == "begin" || MD->getName() == "end"))
- return MCE->getImplicitObjectArgument()->IgnoreParenImpCasts();
- }
- return nullptr;
- };
-
- const Expr *Obj0 = GetContainerObj(Arg0);
- const Expr *Obj1 = GetContainerObj(Arg1);
-
- if (Obj0 && Obj1) {
- const auto *DRE0 = dyn_cast<DeclRefExpr>(Obj0);
- const auto *DRE1 = dyn_cast<DeclRefExpr>(Obj1);
-
- // If both are references to variables, they MUST point to the same
- // declaration.
- if (DRE0 && DRE1) {
- if (DRE0->getDecl()->getCanonicalDecl() ==
- DRE1->getDecl()->getCanonicalDecl())
- return true;
- }
-
- // If they aren't both DeclRefExprs or don't match, we DO NOT return true.
- // This ensures v1.begin(), v2.end() triggers a warning.
- }
-
- return false; // Default to unsafe
-}
-
static bool isSafeArraySubscript(const ArraySubscriptExpr &Node,
const ASTContext &Ctx,
const bool IgnoreStaticSizedArrays) {
@@ -1867,70 +1802,6 @@ class SpanTwoParamConstructorGadget : public WarningGadget {
SmallVector<const Expr *, 1> getUnsafePtrs() const override { return {}; }
};
-class StringViewTwoParamConstructorGadget : public WarningGadget {
- static constexpr const char *const StringViewTwoParamConstructorTag =
- "stringViewTwoParamConstructor";
- const CXXConstructExpr *Ctor; // the string_view constructor expression
-
-public:
- StringViewTwoParamConstructorGadget(const MatchResult &Result)
- : WarningGadget(Kind::StringViewTwoParamConstructor),
- Ctor(Result.getNodeAs<CXXConstructExpr>(
- StringViewTwoParamConstructorTag)) {}
-
- static bool classof(const Gadget *G) {
- return G->getKind() == Kind::StringViewTwoParamConstructor;
- }
-
- static bool matches(const Stmt *S, ASTContext &Ctx, MatchResult &Result) {
- const auto *CE = dyn_cast<CXXConstructExpr>(S);
- if (!CE)
- return false;
- const auto *CDecl = CE->getConstructor();
- const auto *CRecordDecl = CDecl->getParent();
-
- // MATCH: std::basic_string_view
- bool IsStringView =
- CRecordDecl->isInStdNamespace() &&
- CDecl->getDeclName().getAsString() == "basic_string_view" &&
- CE->getNumArgs() == 2;
-
- if (!IsStringView || isSafeStringViewTwoParamConstruct(*CE, Ctx))
- return false;
-
- Result.addNode(StringViewTwoParamConstructorTag, DynTypedNode::create(*CE));
- return true;
- }
-
- static bool matches(const Stmt *S, ASTContext &Ctx,
- const UnsafeBufferUsageHandler *Handler,
- MatchResult &Result) {
- if (ignoreUnsafeBufferInContainer(*S, Handler))
- return false;
- return matches(S, Ctx, Result);
- }
-
- void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
- bool IsRelatedToDecl,
- ASTContext &Ctx) const override {
- Handler.handleUnsafeOperationInStringView(Ctor, IsRelatedToDecl, Ctx);
- }
-
- SourceLocation getSourceLoc() const override { return Ctor->getBeginLoc(); }
-
- DeclUseList getClaimedVarUseSites() const override {
- // If the constructor call is of the form `std::string_view{var, n}`, `var`
- // is considered an unsafe variable.
- if (auto *DRE = dyn_cast<DeclRefExpr>(Ctor->getArg(0))) {
- 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;
@@ -3080,8 +2951,6 @@ std::set<const Expr *> clang::findUnsafePointers(const FunctionDecl *FD) {
const Expr *UnsafeArg = nullptr) override {}
void handleUnsafeOperationInContainer(const Stmt *, bool,
ASTContext &) override {}
- void handleUnsafeOperationInStringView(const Stmt *, bool,
- ASTContext &) override {}
void handleUnsafeVariableGroup(const VarDecl *,
const VariableGroupsManager &, FixItList &&,
const Decl *,
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 8b415a04c4e0e..7ed9b43b76d9d 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2555,19 +2555,12 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
SourceRange Range;
unsigned MsgParam = 0;
+ // This function only handles SpanTwoParamConstructorGadget so far, which
+ // always gives a CXXConstructExpr.
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();
- }
-
- // FIX: Pass the container name to fill the %0 parameter
- S.Diag(Loc, diag::warn_unsafe_buffer_usage_in_container) << ContainerName;
+ S.Diag(Loc, diag::warn_unsafe_buffer_usage_in_container);
if (IsRelatedToDecl) {
assert(!SuggestSuggestions &&
"Variables blamed for unsafe buffer usage without suggestions!");
@@ -2575,29 +2568,6 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
}
}
- void handleUnsafeOperationInStringView(const Stmt *Operation,
- bool IsRelatedToDecl,
- ASTContext &Ctx) override {
- // Extracting location: prioritize the specific location of the constructor
- SourceLocation Loc = Operation->getBeginLoc();
- SourceRange Range = Operation->getSourceRange();
-
- if (const auto *CtorExpr = dyn_cast<CXXConstructExpr>(Operation)) {
- Loc = CtorExpr->getLocation();
- }
-
- // 1. Emit the primary warning for string_view
- S.Diag(Loc, diag::warn_unsafe_buffer_usage_in_container)
- << "std::string_view";
-
- // 2. If a specific variable is 'blamed', emit the note
- if (IsRelatedToDecl) {
- // MsgParam 0 is "unsafe operation"
- // Range helps the IDE underline the whole expression
- S.Diag(Loc, diag::note_unsafe_buffer_operation) << 0 << Range;
- }
- }
-
void handleUnsafeVariableGroup(const VarDecl *Variable,
const VariableGroupsManager &VarGrpMgr,
FixItList &&Fixes, const Decl *D,
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-string-view.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-string-view.cpp
deleted file mode 100644
index f63986b3f2554..0000000000000
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-string-view.cpp
+++ /dev/null
@@ -1,44 +0,0 @@
-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage-in-container -verify %s
-
-namespace std {
- typedef __SIZE_TYPE__ size_t;
- template <typename T> class basic_string_view {
- public:
- basic_string_view(const T *, size_t);
- template <typename It> basic_string_view(It, It);
- };
- typedef basic_string_view<char> string_view;
- typedef basic_string_view<wchar_t> wstring_view;
- template <typename T> class vector { public: T* begin(); T* end(); };
-}
-
-typedef std::size_t size_t;
-
-void test_final_coverage() {
- std::vector<char> v1, v2;
-
- // 1. Iterator Pairs
- std::string_view it_ok(v1.begin(), v1.end()); // no-warning
- // expected-warning at +1 {{the two-parameter std::string_view construction is unsafe}}
- std::string_view it_bad(v1.begin(), v2.end());
-
- // 2. Character Types
- std::string_view s1("hi", 2); // no-warning
- // expected-warning at +1 {{the two-parameter std::string_view construction is unsafe}}
- std::string_view s2("hi", 3);
-
- std::wstring_view w1(L"hi", 2); // no-warning
- // expected-warning at +1 {{the two-parameter std::string_view construction is unsafe}}
- std::wstring_view w2(L"hi", 3);
-
- // 3. Arrays
- char arr[5];
- std::string_view a1(arr, 5); // no-warning
- // expected-warning at +1 {{the two-parameter std::string_view construction is unsafe}}
- std::string_view a2(arr, 6);
-
- // 4. Dynamic/Unknown
- extern size_t get_size();
- // expected-warning at +1 {{the two-parameter std::string_view construction is unsafe}}
- std::string_view d1("hi", get_size());
-}
More information about the cfe-commits
mailing list