Revert "[Fix]"[-Wunsafe-buffer-usage] Add a new forEachDescendant m…

…atcher that skips callable declarations""

This reverts commit ef47a0a.

Revert "[-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher that skips callable declarations"

This reverts commit b2ac5fd.

This patch is causing failure in some Sanitizer tests
(https://lab.llvm.org/buildbot/#/builders/5/builds/30522/steps/13/logs/stdio).  Reverting the patch and its' fix.

clang/lib/Analysis/UnsafeBufferUsage.cpp

@@ -8,112 +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 {
-// A `RecursiveASTVisitor` that traverses all descendants of a given node "n"
-// except for those belonging to a different 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 different callable of "n".
-  MatchDescendantVisitor(const internal::DynTypedMatcher *Matcher,
-                         internal::ASTMatchFinder *Finder,
-                         internal::BoundNodesTreeBuilder *Builder,
-                         internal::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) {
-    internal::BoundNodesTreeBuilder RecursiveBuilder(*Builder);
-
-    if (Matcher->matches(DynTypedNode::create(Node), Finder,
-                         &RecursiveBuilder)) {
-      ResultBindings.addMatch(RecursiveBuilder);
-      Matches = true;
-      if (Bind != internal::ASTMatchFinder::BK_All)
-        return false; // Abort as soon as a match is found.
-    }
-    return true;
-  }
-
-  const internal::DynTypedMatcher *const Matcher;
-  internal::ASTMatchFinder *const Finder;
-  internal::BoundNodesTreeBuilder *const Builder;
-  internal::BoundNodesTreeBuilder ResultBindings;
-  const internal::ASTMatchFinder::BindKind Bind;
-  bool Matches;
-};
-
-AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher<Stmt>, innerMatcher) {
-  MatchDescendantVisitor Visitor(new DynTypedMatcher(innerMatcher), Finder,
-                                 Builder, ASTMatchFinder::BK_All);
-  return Visitor.findMatch(DynTypedNode::create(Node));
-}
-} // namespace clang::ast_matchers
-
 namespace {
 // Because the analysis revolves around variables and their types, we'll need to
 // track uses of variables (aka DeclRefExprs).
@@ -498,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)                                                              \

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