[clang] f58b025 - Revert "[-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher that skips callable declarations"
Roman Lebedev via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 4 17:26:09 PST 2023
Reminder to please always mention the reason for the revert/recommit
in the commit message.
On Thu, Jan 5, 2023 at 4:24 AM Ziqing Luo via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
>
> Author: Ziqing Luo
> Date: 2023-01-04T17:16:21-08:00
> New Revision: f58b025354ee2d3bcd7ab2399a11429ec940c1e0
>
> URL: https://github.com/llvm/llvm-project/commit/f58b025354ee2d3bcd7ab2399a11429ec940c1e0
> DIFF: https://github.com/llvm/llvm-project/commit/f58b025354ee2d3bcd7ab2399a11429ec940c1e0.diff
>
> LOG: Revert "[-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher that skips callable declarations"
>
> This reverts commit b2ac5fd724c44cf662caed84bd8f84af574b981d.
>
> Added:
>
>
> Modified:
> clang/lib/Analysis/UnsafeBufferUsage.cpp
> clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
> index 29c8dbb45fe9..a699d27c1544 100644
> --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
> +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
> @@ -8,111 +8,12 @@
>
> #include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
> #include "clang/ASTMatchers/ASTMatchFinder.h"
> -#include "clang/AST/RecursiveASTVisitor.h"
> #include "llvm/ADT/SmallVector.h"
>
> using namespace llvm;
> using namespace clang;
> using namespace ast_matchers;
>
> -namespace clang::ast_matchers::internal {
> -// A `RecursiveASTVisitor` that traverses all descendants of a given node "n"
> -// except for those belonging to a
> diff erent callable of "n".
> -class MatchDescendantVisitor
> - : public RecursiveASTVisitor<MatchDescendantVisitor> {
> -public:
> - typedef RecursiveASTVisitor<MatchDescendantVisitor> VisitorBase;
> -
> - // Creates an AST visitor that matches `Matcher` on all
> - // descendants of a given node "n" except for the ones
> - // belonging to a
> diff erent callable of "n".
> - MatchDescendantVisitor(const DynTypedMatcher *Matcher, ASTMatchFinder *Finder,
> - BoundNodesTreeBuilder *Builder,
> - ASTMatchFinder::BindKind Bind)
> - : Matcher(Matcher), Finder(Finder), Builder(Builder), Bind(Bind),
> - Matches(false) {}
> -
> - // Returns true if a match is found in a subtree of `DynNode`, which belongs
> - // to the same callable of `DynNode`.
> - bool findMatch(const DynTypedNode &DynNode) {
> - Matches = false;
> - if (const Stmt *StmtNode = DynNode.get<Stmt>()) {
> - TraverseStmt(const_cast<Stmt *>(StmtNode));
> - *Builder = ResultBindings;
> - return Matches;
> - }
> - return false;
> - }
> -
> - // The following are overriding methods from the base visitor class.
> - // They are public only to allow CRTP to work. They are *not *part
> - // of the public API of this class.
> -
> - // For the matchers so far used in safe buffers, we only need to match
> - // `Stmt`s. To override more as needed.
> -
> - bool TraverseDecl(Decl *Node) {
> - if (!Node)
> - return true;
> - if (!match(*Node))
> - return false;
> - // To skip callables:
> - if (isa<FunctionDecl, BlockDecl, ObjCMethodDecl>(Node))
> - return true;
> - // Traverse descendants
> - return VisitorBase::TraverseDecl(Node);
> - }
> -
> - bool TraverseStmt(Stmt *Node, DataRecursionQueue *Queue = nullptr) {
> - if (!Node)
> - return true;
> - if (!match(*Node))
> - return false;
> - // To skip callables:
> - if (isa<LambdaExpr>(Node))
> - return true;
> - return VisitorBase::TraverseStmt(Node);
> - }
> -
> - bool shouldVisitTemplateInstantiations() const { return true; }
> - bool shouldVisitImplicitCode() const {
> - // TODO: let's ignore implicit code for now
> - return false;
> - }
> -
> -private:
> - // Sets 'Matched' to true if 'Matcher' matches 'Node'
> - //
> - // Returns 'true' if traversal should continue after this function
> - // returns, i.e. if no match is found or 'Bind' is 'BK_All'.
> - template <typename T> bool match(const T &Node) {
> - BoundNodesTreeBuilder RecursiveBuilder(*Builder);
> -
> - if (Matcher->matches(DynTypedNode::create(Node), Finder,
> - &RecursiveBuilder)) {
> - ResultBindings.addMatch(RecursiveBuilder);
> - Matches = true;
> - if (Bind != ASTMatchFinder::BK_All)
> - return false; // Abort as soon as a match is found.
> - }
> - return true;
> - }
> -
> - const DynTypedMatcher *const Matcher;
> - ASTMatchFinder *const Finder;
> - BoundNodesTreeBuilder *const Builder;
> - BoundNodesTreeBuilder ResultBindings;
> - const ASTMatchFinder::BindKind Bind;
> - bool Matches;
> -};
> -
> -AST_MATCHER_P(Stmt, forEveryDescendant, Matcher<Stmt>, innerMatcher) {
> - MatchDescendantVisitor Visitor(new DynTypedMatcher(innerMatcher), Finder,
> - Builder, ASTMatchFinder::BK_All);
> - return Visitor.findMatch(DynTypedNode::create(Node));
> -}
> -} // namespace clang::ast_matchers::internal
> -
> namespace {
> // Because the analysis revolves around variables and their types, we'll need to
> // track uses of variables (aka DeclRefExprs).
> @@ -497,7 +398,7 @@ static std::pair<GadgetList, DeclUseTracker> findGadgets(const Decl *D) {
>
> // clang-format off
> M.addMatcher(
> - stmt(forEveryDescendant(
> + stmt(forEachDescendant(
> stmt(anyOf(
> // Add Gadget::matcher() for every gadget in the registry.
> #define GADGET(x) \
>
> diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
> index 476ec73c4f74..e82841442485 100644
> --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
> +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fblocks -include %s -verify %s
> +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -include %s -verify %s
> #ifndef INCLUDED
> #define INCLUDED
> #pragma clang system_header
> @@ -141,15 +141,15 @@ void testStructMembers(struct T * sp, struct T s, T_t * sp2, T_t s2) {
>
> int garray[10];
> int * gp = garray;
> -int gvar = gp[1]; // FIXME: file scope unsafe buffer access is not warned
> +int gvar = gp[1]; // TODO: this is not warned
>
> void testLambdaCaptureAndGlobal(int * p) {
> int a[10];
>
> auto Lam = [p, a]() {
> - return p[1] // expected-warning{{unchecked operation on raw buffer in expression}}
> + return p[1] // expected-warning2{{unchecked operation on raw buffer in expression}}
> + a[1] + garray[1]
> - + gp[1]; // expected-warning{{unchecked operation on raw buffer in expression}}
> + + gp[1]; // expected-warning2{{unchecked operation on raw buffer in expression}}
> };
> }
>
> @@ -213,66 +213,4 @@ void testPointerToMember() {
> foo(S.*p,
> (S.*q)[1]); // expected-warning{{unchecked operation on raw buffer in expression}}
> }
> -
> -// test that nested callable definitions are scanned only once
> -void testNestedCallableDefinition(int * p) {
> - class A {
> - void inner(int * p) {
> - p++; // expected-warning{{unchecked operation on raw buffer in expression}}
> - }
> -
> - static void innerStatic(int * p) {
> - p++; // expected-warning{{unchecked operation on raw buffer in expression}}
> - }
> -
> - void innerInner(int * p) {
> - auto Lam = [p]() {
> - int * q = p;
> - q++; // expected-warning{{unchecked operation on raw buffer in expression}}
> - return *q;
> - };
> - }
> - };
> -
> - auto Lam = [p]() {
> - int * q = p;
> - q++; // expected-warning{{unchecked operation on raw buffer in expression}}
> - return *q;
> - };
> -
> - auto LamLam = [p]() {
> - auto Lam = [p]() {
> - int * q = p;
> - q++; // expected-warning{{unchecked operation on raw buffer in expression}}
> - return *q;
> - };
> - };
> -
> - void (^Blk)(int*) = ^(int *p) {
> - p++; // expected-warning{{unchecked operation on raw buffer in expression}}
> - };
> -
> - void (^BlkBlk)(int*) = ^(int *p) {
> - void (^Blk)(int*) = ^(int *p) {
> - p++; // expected-warning{{unchecked operation on raw buffer in expression}}
> - };
> - Blk(p);
> - };
> -
> - // lambda and block as call arguments...
> - foo( [p]() { int * q = p;
> - q++; // expected-warning{{unchecked operation on raw buffer in expression}}
> - return *q;
> - },
> - ^(int *p) { p++; // expected-warning{{unchecked operation on raw buffer in expression}}
> - }
> - );
> -}
> -
> -void testVariableDecls(int * p) {
> - int * q = p++; // expected-warning{{unchecked operation on raw buffer in expression}}
> - int a[p[1]]; // expected-warning{{unchecked operation on raw buffer in expression}}
> - int b = p[1]; // expected-warning{{unchecked operation on raw buffer in expression}}
> -}
> -
> #endif
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list