[clang] f58b025 - Revert "[-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher that skips callable declarations"
Ziqing Luo via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 4 17:24:00 PST 2023
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
More information about the cfe-commits
mailing list