[-Wunsafe-buffer-usage] Support span creation from std::initializer_list and begin/end pairs #145311
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ist and begin/end pairs
Support safe construction of `std::span` from `begin` and `end` calls
on hardened containers or views or `std::initializer_list`s.
For example, the following code is safe:
```
void create(std::initializer_list<int> il) {
std::span<int> input{ il.begin(), il.end() }; // no warn
}
```
rdar://152637380
ziqingluo-90
requested review from
haoNoQ,
malavikasamak,
ivanaivanovska,
t-rasmud and
jkorous-apple
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Ziqing Luo (ziqingluo-90) ChangesSupport safe construction of For example, the following code is safe: rdar://152637380 Full diff: https://github.com/llvm/llvm-project/pull/145311.diff 2 Files Affected:
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 631a546b45ff4..259db3f092c92 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -115,6 +115,10 @@ class MatchResult {
};
} // namespace
+#define SIZED_CONTAINER_OR_VIEW_LIST \
+ "span", "array", "vector", "basic_string_view", "basic_string", \
+ "initializer_list",
+
// A `RecursiveASTVisitor` that traverses all descendants of a given node "n"
// except for those belonging to a different callable of "n".
class MatchDescendantVisitor : public DynamicRecursiveASTVisitor {
@@ -463,6 +467,8 @@ static bool areEqualIntegers(const Expr *E1, const Expr *E2, ASTContext &Ctx) {
// `N, M` are parameter indexes to the allocating element number and size.
// Sometimes, there is only one parameter index representing the total
// size.
+// 7. `std::span<T>{x.begin(), x.end()}` where `x` is an object in the
+// SIZED_CONTAINER_OR_VIEW_LIST.
static bool isSafeSpanTwoParamConstruct(const CXXConstructExpr &Node,
ASTContext &Ctx) {
assert(Node.getNumArgs() == 2 &&
@@ -560,6 +566,31 @@ static bool isSafeSpanTwoParamConstruct(const CXXConstructExpr &Node,
}
}
}
+ // Check form 7:
+ 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 (auto *II = RD->getDeclName().getAsIdentifierInfo();
+ II && RD->isInStdNamespace())
+ return llvm::is_contained({SIZED_CONTAINER_OR_VIEW_LIST},
+ II->getName()) &&
+ MD->getName() == MethodName;
+ }
+ return false;
+ };
+
+ if (IsMethodCallToSizedObject(Arg0, "begin") &&
+ IsMethodCallToSizedObject(Arg1, "end"))
+ return AreSameDRE(
+ // We know Arg0 and Arg1 are `CXXMemberCallExpr`s:
+ cast<CXXMemberCallExpr>(Arg0)
+ ->getImplicitObjectArgument()
+ ->IgnoreParenImpCasts(),
+ cast<CXXMemberCallExpr>(Arg1)
+ ->getImplicitObjectArgument()
+ ->IgnoreParenImpCasts());
return false;
}
@@ -1058,8 +1089,7 @@ static bool hasUnsafeSnprintfBuffer(const CallExpr &Node,
return false; // not an snprintf call
// Pattern 1:
- static StringRef SizedObjs[] = {"span", "array", "vector",
- "basic_string_view", "basic_string"};
+ static StringRef SizedObjs[] = {SIZED_CONTAINER_OR_VIEW_LIST};
Buf = Buf->IgnoreParenImpCasts();
Size = Size->IgnoreParenImpCasts();
if (auto *MCEPtr = dyn_cast<CXXMemberCallExpr>(Buf))
@@ -1826,9 +1856,8 @@ class DataInvocationGadget : public WarningGadget {
auto *method = cast<CXXMethodDecl>(callee);
if (method->getNameAsString() == "data" &&
method->getParent()->isInStdNamespace() &&
- (method->getParent()->getName() == "span" ||
- method->getParent()->getName() == "array" ||
- method->getParent()->getName() == "vector"))
+ llvm::is_contained({SIZED_CONTAINER_OR_VIEW_LIST},
+ method->getParent()->getName()))
return true;
return false;
}
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 e287f00a5453b..1e7855517207e 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
@@ -1,5 +1,7 @@
// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-container -verify %s
+typedef unsigned int size_t;
+
namespace std {
template <class T> class span {
public:
@@ -16,6 +18,9 @@ namespace std {
template<class R>
constexpr span(R && range){};
+
+ T* begin() noexcept;
+ T* end() noexcept;
};
@@ -27,6 +32,37 @@ namespace std {
return &__x;
}
+ template <typename T, size_t N>
+ struct array {
+ T* begin() noexcept;
+ const T* begin() const noexcept;
+ T* end() noexcept;
+ const T* end() const noexcept;
+ size_t size() const noexcept;
+ T * data() const noexcept;
+ T& operator[](size_t n);
+ };
+
+ template<class T>
+ class initializer_list {
+ public:
+ size_t size() const noexcept;
+ const T* begin() const noexcept;
+ const T* end() const noexcept;
+ T * data() const noexcept;
+ };
+
+ template<typename T>
+ struct basic_string {
+ T *c_str() const noexcept;
+ T *data() const noexcept;
+ unsigned size();
+ const T* begin() const noexcept;
+ const T* end() const noexcept;
+ };
+
+ typedef basic_string<char> string;
+ typedef basic_string<wchar_t> wstring;
}
namespace irrelevant_constructors {
@@ -232,3 +268,27 @@ struct HoldsStdSpanAndNotInitializedInCtor {
: Ptr(P), Size(S)
{}
};
+
+namespace test_begin_end {
+ struct Object {
+ int * begin();
+ int * end();
+ };
+ void safe_cases(std::span<int> Sp, std::array<int, 10> Arr, std::string Str, std::initializer_list<Object> Il) {
+ 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()};
+ }
+
+ 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}}
+ }
+
+ void unsupport_cases(std::array<Object, 10> Arr) {
+ std::span<int>{Arr[0].begin(), Arr[0].end()}; // expected-warning {{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
+ }
+}
|
|
@llvm/pr-subscribers-clang-analysis Author: Ziqing Luo (ziqingluo-90) ChangesSupport safe construction of For example, the following code is safe: rdar://152637380 Full diff: https://github.com/llvm/llvm-project/pull/145311.diff 2 Files Affected:
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 631a546b45ff4..259db3f092c92 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -115,6 +115,10 @@ class MatchResult {
};
} // namespace
+#define SIZED_CONTAINER_OR_VIEW_LIST \
+ "span", "array", "vector", "basic_string_view", "basic_string", \
+ "initializer_list",
+
// A `RecursiveASTVisitor` that traverses all descendants of a given node "n"
// except for those belonging to a different callable of "n".
class MatchDescendantVisitor : public DynamicRecursiveASTVisitor {
@@ -463,6 +467,8 @@ static bool areEqualIntegers(const Expr *E1, const Expr *E2, ASTContext &Ctx) {
// `N, M` are parameter indexes to the allocating element number and size.
// Sometimes, there is only one parameter index representing the total
// size.
+// 7. `std::span<T>{x.begin(), x.end()}` where `x` is an object in the
+// SIZED_CONTAINER_OR_VIEW_LIST.
static bool isSafeSpanTwoParamConstruct(const CXXConstructExpr &Node,
ASTContext &Ctx) {
assert(Node.getNumArgs() == 2 &&
@@ -560,6 +566,31 @@ static bool isSafeSpanTwoParamConstruct(const CXXConstructExpr &Node,
}
}
}
+ // Check form 7:
+ 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 (auto *II = RD->getDeclName().getAsIdentifierInfo();
+ II && RD->isInStdNamespace())
+ return llvm::is_contained({SIZED_CONTAINER_OR_VIEW_LIST},
+ II->getName()) &&
+ MD->getName() == MethodName;
+ }
+ return false;
+ };
+
+ if (IsMethodCallToSizedObject(Arg0, "begin") &&
+ IsMethodCallToSizedObject(Arg1, "end"))
+ return AreSameDRE(
+ // We know Arg0 and Arg1 are `CXXMemberCallExpr`s:
+ cast<CXXMemberCallExpr>(Arg0)
+ ->getImplicitObjectArgument()
+ ->IgnoreParenImpCasts(),
+ cast<CXXMemberCallExpr>(Arg1)
+ ->getImplicitObjectArgument()
+ ->IgnoreParenImpCasts());
return false;
}
@@ -1058,8 +1089,7 @@ static bool hasUnsafeSnprintfBuffer(const CallExpr &Node,
return false; // not an snprintf call
// Pattern 1:
- static StringRef SizedObjs[] = {"span", "array", "vector",
- "basic_string_view", "basic_string"};
+ static StringRef SizedObjs[] = {SIZED_CONTAINER_OR_VIEW_LIST};
Buf = Buf->IgnoreParenImpCasts();
Size = Size->IgnoreParenImpCasts();
if (auto *MCEPtr = dyn_cast<CXXMemberCallExpr>(Buf))
@@ -1826,9 +1856,8 @@ class DataInvocationGadget : public WarningGadget {
auto *method = cast<CXXMethodDecl>(callee);
if (method->getNameAsString() == "data" &&
method->getParent()->isInStdNamespace() &&
- (method->getParent()->getName() == "span" ||
- method->getParent()->getName() == "array" ||
- method->getParent()->getName() == "vector"))
+ llvm::is_contained({SIZED_CONTAINER_OR_VIEW_LIST},
+ method->getParent()->getName()))
return true;
return false;
}
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 e287f00a5453b..1e7855517207e 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
@@ -1,5 +1,7 @@
// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-container -verify %s
+typedef unsigned int size_t;
+
namespace std {
template <class T> class span {
public:
@@ -16,6 +18,9 @@ namespace std {
template<class R>
constexpr span(R && range){};
+
+ T* begin() noexcept;
+ T* end() noexcept;
};
@@ -27,6 +32,37 @@ namespace std {
return &__x;
}
+ template <typename T, size_t N>
+ struct array {
+ T* begin() noexcept;
+ const T* begin() const noexcept;
+ T* end() noexcept;
+ const T* end() const noexcept;
+ size_t size() const noexcept;
+ T * data() const noexcept;
+ T& operator[](size_t n);
+ };
+
+ template<class T>
+ class initializer_list {
+ public:
+ size_t size() const noexcept;
+ const T* begin() const noexcept;
+ const T* end() const noexcept;
+ T * data() const noexcept;
+ };
+
+ template<typename T>
+ struct basic_string {
+ T *c_str() const noexcept;
+ T *data() const noexcept;
+ unsigned size();
+ const T* begin() const noexcept;
+ const T* end() const noexcept;
+ };
+
+ typedef basic_string<char> string;
+ typedef basic_string<wchar_t> wstring;
}
namespace irrelevant_constructors {
@@ -232,3 +268,27 @@ struct HoldsStdSpanAndNotInitializedInCtor {
: Ptr(P), Size(S)
{}
};
+
+namespace test_begin_end {
+ struct Object {
+ int * begin();
+ int * end();
+ };
+ void safe_cases(std::span<int> Sp, std::array<int, 10> Arr, std::string Str, std::initializer_list<Object> Il) {
+ 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()};
+ }
+
+ 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}}
+ }
+
+ void unsupport_cases(std::array<Object, 10> Arr) {
+ std::span<int>{Arr[0].begin(), Arr[0].end()}; // expected-warning {{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
+ }
+}
|
jkorous-apple
approved these changes
Jun 23, 2025
| 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}} |
There was a problem hiding this comment.
It is out of scope for this change but I wonder if we should change the warning message for a two-pointer constructor case.
There was a problem hiding this comment.
My understanding is that it warns about the mismatch between the bound information of the constructed span and the actual buffer size. So I think the message is accurate regardless of how the span is constructed.
anthonyhatran
pushed a commit
to anthonyhatran/llvm-project
that referenced
this pull request
Jun 26, 2025
…ist and begin/end pairs (llvm#145311) Support safe construction of `std::span` from `begin` and `end` calls on hardened containers or views or `std::initializer_list`s. For example, the following code is safe: ``` void create(std::initializer_list<int> il) { std::span<int> input{ il.begin(), il.end() }; // no warn } ``` rdar://152637380
rlavaee
pushed a commit
to rlavaee/llvm-project
that referenced
this pull request
Jul 1, 2025
…ist and begin/end pairs (llvm#145311) Support safe construction of `std::span` from `begin` and `end` calls on hardened containers or views or `std::initializer_list`s. For example, the following code is safe: ``` void create(std::initializer_list<int> il) { std::span<int> input{ il.begin(), il.end() }; // no warn } ``` rdar://152637380
|
As a side effect (by changing DataInvocationGadget), this seems to have made -Wunsafe-buffer-usafe warn on unsafe uses of Was that intentional, and if so is there some test that should be updated to "lock it in"? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support safe construction of
std::spanfrombeginandendcalls on hardened containers or views orstd::initializer_lists.For example, the following code is safe:
rdar://152637380