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