[clang] 27c1033 - [WIP][-Wunsafe-buffer-usage] Handle lambda expressions within a method.
Rashmi Mudduluru via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 20 10:01:16 PDT 2023
Author: Rashmi Mudduluru
Date: 2023-07-20T10:00:16-07:00
New Revision: 27c10337831c94ef59f8790f6ca1c3d1b66b4494
URL: https://github.com/llvm/llvm-project/commit/27c10337831c94ef59f8790f6ca1c3d1b66b4494
DIFF: https://github.com/llvm/llvm-project/commit/27c10337831c94ef59f8790f6ca1c3d1b66b4494.diff
LOG: [WIP][-Wunsafe-buffer-usage] Handle lambda expressions within a method.
Differential Revision: https://reviews.llvm.org/D150386
Added:
Modified:
clang/docs/LibASTMatchersReference.html
clang/docs/ReleaseNotes.rst
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/lib/ASTMatchers/ASTMatchersInternal.cpp
clang/lib/ASTMatchers/Dynamic/Registry.cpp
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
Removed:
################################################################################
diff --git a/clang/docs/LibASTMatchersReference.html b/clang/docs/LibASTMatchersReference.html
index bde8ac7de52a73..b4f282fbf43816 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -1257,6 +1257,43 @@ <h2 id="decl-matchers">Node Matchers</h2>
</pre></td></tr>
+<tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>></td><td class="name" onclick="toggle('arrayInitIndexExpr0')"><a name="arrayInitIndexExpr0Anchor">arrayInitIndexExpr</a></td><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1ArrayInitIndexExpr.html">ArrayInitIndexExpr</a>>...</td></tr>
+<tr><td colspan="4" class="doc" id="arrayInitIndexExpr0"><pre>The arrayInitIndexExpr consists of two subexpressions: a common expression
+(the source array) that is evaluated once up-front, and a per-element initializer
+that runs once for each array element. Within the per-element initializer,
+the current index may be obtained via an ArrayInitIndexExpr.
+
+Given
+ void testStructBinding() {
+ int a[2] = {1, 2};
+ auto [x, y] = a;
+ }
+arrayInitIndexExpr() matches the array index that implicitly iterates
+over the array `a` to copy each element to the anonymous array
+that backs the structured binding `[x, y]` elements of which are
+referred to by their aliases `x` and `y`.
+</pre></td></tr>
+
+
+<tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>></td><td class="name" onclick="toggle('arrayInitLoopExpr0')"><a name="arrayInitLoopExpr0Anchor">arrayInitLoopExpr</a></td><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1ArrayInitLoopExpr.html">ArrayInitLoopExpr</a>>...</td></tr>
+<tr><td colspan="4" class="doc" id="arrayInitLoopExpr0"><pre>Matches a loop initializing the elements of an array in a number of contexts:
+ * in the implicit copy/move constructor for a class with an array member
+ * when a lambda-expression captures an array by value
+ * when a decomposition declaration decomposes an array
+
+Given
+ void testLambdaCapture() {
+ int a[10];
+ auto Lam1 = [a]() {
+ return;
+ };
+ }
+arrayInitLoopExpr() matches the implicit loop that initializes each element of
+the implicit array field inside the lambda object, that represents the array `a`
+captured by value.
+</pre></td></tr>
+
+
<tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>></td><td class="name" onclick="toggle('arraySubscriptExpr0')"><a name="arraySubscriptExpr0Anchor">arraySubscriptExpr</a></td><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1ArraySubscriptExpr.html">ArraySubscriptExpr</a>>...</td></tr>
<tr><td colspan="4" class="doc" id="arraySubscriptExpr0"><pre>Matches array subscript expressions.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ef1cc898c21f6d..f1d098ef02f41d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -943,6 +943,8 @@ AST Matchers
- The ``hasBody`` matcher now matches coroutine body nodes in
``CoroutineBodyStmts``.
+
+- Add ``arrayInitIndexExpr`` and ``arrayInitLoopExpr`` matchers.
clang-format
------------
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index b698365f949bfa..a9313139226ca4 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -1971,6 +1971,45 @@ extern const internal::VariadicDynCastAllOfMatcher<Stmt, CXXDeleteExpr>
extern const internal::VariadicDynCastAllOfMatcher<Stmt, CXXNoexceptExpr>
cxxNoexceptExpr;
+/// Matches a loop initializing the elements of an array in a number of contexts:
+/// * in the implicit copy/move constructor for a class with an array member
+/// * when a lambda-expression captures an array by value
+/// * when a decomposition declaration decomposes an array
+///
+/// Given
+/// \code
+/// void testLambdaCapture() {
+/// int a[10];
+/// auto Lam1 = [a]() {
+/// return;
+/// };
+/// }
+/// \endcode
+/// arrayInitLoopExpr() matches the implicit loop that initializes each element of
+/// the implicit array field inside the lambda object, that represents the array `a`
+/// captured by value.
+extern const internal::VariadicDynCastAllOfMatcher<Stmt, ArrayInitLoopExpr>
+ arrayInitLoopExpr;
+
+/// The arrayInitIndexExpr consists of two subexpressions: a common expression
+/// (the source array) that is evaluated once up-front, and a per-element initializer
+/// that runs once for each array element. Within the per-element initializer,
+/// the current index may be obtained via an ArrayInitIndexExpr.
+///
+/// Given
+/// \code
+/// void testStructBinding() {
+/// int a[2] = {1, 2};
+/// auto [x, y] = a;
+/// }
+/// \endcode
+/// arrayInitIndexExpr() matches the array index that implicitly iterates
+/// over the array `a` to copy each element to the anonymous array
+/// that backs the structured binding `[x, y]` elements of which are
+/// referred to by their aliases `x` and `y`.
+extern const internal::VariadicDynCastAllOfMatcher<Stmt, ArrayInitIndexExpr>
+ arrayInitIndexExpr;
+
/// Matches array subscript expressions.
///
/// Given
diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
index 26e40c4ed36a55..ad6b20f4fd2ff8 100644
--- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -882,6 +882,10 @@ const internal::VariadicDynCastAllOfMatcher<Stmt, CXXNoexceptExpr>
cxxNoexceptExpr;
const internal::VariadicDynCastAllOfMatcher<Stmt, ArraySubscriptExpr>
arraySubscriptExpr;
+const internal::VariadicDynCastAllOfMatcher<Stmt, ArrayInitIndexExpr>
+ arrayInitIndexExpr;
+const internal::VariadicDynCastAllOfMatcher<Stmt, ArrayInitLoopExpr>
+ arrayInitLoopExpr;
const internal::VariadicDynCastAllOfMatcher<Stmt, CXXDefaultArgExpr>
cxxDefaultArgExpr;
const internal::VariadicDynCastAllOfMatcher<Stmt, CXXOperatorCallExpr>
diff --git a/clang/lib/ASTMatchers/Dynamic/Registry.cpp b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
index 995c082d533655..d3594455b55a46 100644
--- a/clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -134,6 +134,8 @@ RegistryMaps::RegistryMaps() {
REGISTER_MATCHER(allOf);
REGISTER_MATCHER(anyOf);
REGISTER_MATCHER(anything);
+ REGISTER_MATCHER(arrayInitIndexExpr);
+ REGISTER_MATCHER(arrayInitLoopExpr);
REGISTER_MATCHER(argumentCountIs);
REGISTER_MATCHER(argumentCountAtLeast);
REGISTER_MATCHER(arraySubscriptExpr);
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5295d9d92b2977..54569b420ca357 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -119,9 +119,6 @@ class MatchDescendantVisitor
return true;
if (!match(*Node))
return false;
- // To skip callables:
- if (isa<LambdaExpr>(Node))
- return true;
return VisitorBase::TraverseStmt(Node);
}
@@ -467,7 +464,9 @@ class ArraySubscriptGadget : public WarningGadget {
return stmt(arraySubscriptExpr(
hasBase(ignoringParenImpCasts(
anyOf(hasPointerType(), hasArrayType()))),
- unless(hasIndex(integerLiteral(equals(0)))))
+ unless(hasIndex(
+ anyOf(integerLiteral(equals(0)), arrayInitIndexExpr())
+ )))
.bind(ArraySubscrTag));
// clang-format on
}
@@ -2209,6 +2208,13 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
UnsafeBufferUsageHandler &Handler,
bool EmitSuggestions) {
assert(D && D->getBody());
+
+ // We do not want to visit a Lambda expression defined inside a method independently.
+ // Instead, it should be visited along with the outer method.
+ if (const auto *fd = dyn_cast<CXXMethodDecl>(D)) {
+ if (fd->getParent()->isLambda() && fd->getParent()->isLocalClass())
+ return;
+ }
// Do not emit fixit suggestions for functions declared in an
// extern "C" block.
@@ -2254,7 +2260,7 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
it != FixablesForAllVars.byVar.cend();) {
// FIXME: need to deal with global variables later
if ((!it->first->isLocalVarDecl() && !isa<ParmVarDecl>(it->first)) ||
- Tracker.hasUnclaimedUses(it->first)) {
+ Tracker.hasUnclaimedUses(it->first) || it->first->isInitCapture()) {
it = FixablesForAllVars.byVar.erase(it);
} else {
++it;
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp
index 0235dce828f0cf..f1b92ae8d82bcf 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp
@@ -110,3 +110,70 @@ void unsafe_method_invocation_double_param() {
m1(q, q, 8);
}
+void unsafe_access_in_lamda() {
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ auto my_lambda = [&](){
+ p[5] = 10;
+ };
+
+ foo(p);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:8-[[@LINE-1]]:8}:".data()"
+}
+
+void fixits_in_lamda() {
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ auto my_lambda = [&](){
+ foo(p);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:10}:".data()"
+ };
+
+ p[5] = 10;
+}
+
+// FIXME: Emit fixits for lambda captured variables
+void fixits_in_lambda_capture() {
+ auto p = new int[10];
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ auto my_lambda = [p](){ // No fixits emitted here.
+ foo(p);
+ };
+
+ p[5] = 10;
+}
+
+void fixits_in_lambda_capture_reference() {
+ auto p = new int[10];
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ auto my_lambda = [&p](){ // No fixits emitted here.
+ foo(p);
+ };
+
+ p[5] = 10;
+}
+
+void fixits_in_lambda_capture_rename() {
+ auto p = new int[10];
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ auto my_lambda = [x = p](){ // No fixits emitted here.
+ foo(x);
+ };
+
+ p[5] = 10;
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
index b6f16756bda682..fd3d6cbdf5d93d 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -166,7 +166,6 @@ int garray[10]; // expected-warning{{'garray' is an unsafe buffer that does
int * gp = garray; // expected-warning{{'gp' is an unsafe pointer used for buffer access}}
int gvar = gp[1]; // FIXME: file scope unsafe buffer access is not warned
-// FIXME: Add test for lambda capture with initializer. E. g. auto Lam = [new_p = p]() {...
void testLambdaCaptureAndGlobal(int * p) {
// expected-warning at -1{{'p' is an unsafe pointer used for buffer access}}
int a[10]; // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}}
@@ -175,6 +174,44 @@ void testLambdaCaptureAndGlobal(int * p) {
return p[1] // expected-note{{used in buffer access here}}
+ a[1] + garray[1] // expected-note2{{used in buffer access here}}
+ gp[1]; // expected-note{{used in buffer access here}}
+
+ };
+}
+
+auto file_scope_lambda = [](int *ptr) {
+ // expected-warning at -1{{'ptr' is an unsafe pointer used for buffer access}}
+
+ ptr[5] = 10; // expected-note{{used in buffer access here}}
+};
+
+void testLambdaCapture() {
+ int a[10]; // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}}
+ int b[10]; // expected-warning{{'b' is an unsafe buffer that does not perform bounds checks}}
+ int c[10];
+
+ auto Lam1 = [a]() {
+ return a[1]; // expected-note{{used in buffer access here}}
+ };
+
+ auto Lam2 = [x = b[3]]() { // expected-note{{used in buffer access here}}
+ return x;
+ };
+
+ auto Lam = [x = c]() { // expected-warning{{'x' is an unsafe pointer used for buffer access}}
+ return x[3]; // expected-note{{used in buffer access here}}
+ };
+}
+
+void testLambdaImplicitCapture() {
+ int a[10]; // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}}
+ int b[10]; // expected-warning{{'b' is an unsafe buffer that does not perform bounds checks}}
+
+ auto Lam1 = [=]() {
+ return a[1]; // expected-note{{used in buffer access here}}
+ };
+
+ auto Lam2 = [&]() {
+ return b[1]; // expected-note{{used in buffer access here}}
};
}
More information about the cfe-commits
mailing list