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@-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@-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}} }; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits