[clang] 7d0d34f - Re-land "[-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher that skips callable declarations"
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 6 10:40:38 PST 2023
Author: ziqingluo-90
Date: 2023-01-06T10:33:21-08:00
New Revision: 7d0d34fbb153ef044b254d41ae91fc57efbdc77d
URL: https://github.com/llvm/llvm-project/commit/7d0d34fbb153ef044b254d41ae91fc57efbdc77d
DIFF: https://github.com/llvm/llvm-project/commit/7d0d34fbb153ef044b254d41ae91fc57efbdc77d.diff
LOG: Re-land "[-Wunsafe-buffer-usage] Add a new `forEachDescendant` matcher that skips callable declarations"
This reverts commit 22df4549a3718dcd8b387ba8246978349e4be50c.
After a quick investigation, realizing that the Sanitizer test
failures caused by this patch is not likely to block other
contributors. I re-land this patch before taking a closer look at
those tests so that it won't block the [-Wunsafe-buffer-usage]
development.
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 a699d27c1544c..80a54c3ad38b7 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -8,12 +8,112 @@
#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
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 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).
@@ -398,7 +498,7 @@ static std::pair<GadgetList, DeclUseTracker> findGadgets(const Decl *D) {
// clang-format off
M.addMatcher(
- stmt(forEachDescendant(
+ stmt(forEveryDescendant(
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 e828414424855..476ec73c4f744 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 -include %s -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fblocks -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]; // TODO: this is not warned
+int gvar = gp[1]; // FIXME: file scope unsafe buffer access is not warned
void testLambdaCaptureAndGlobal(int * p) {
int a[10];
auto Lam = [p, a]() {
- return p[1] // expected-warning2{{unchecked operation on raw buffer in expression}}
+ return p[1] // expected-warning{{unchecked operation on raw buffer in expression}}
+ a[1] + garray[1]
- + gp[1]; // expected-warning2{{unchecked operation on raw buffer in expression}}
+ + gp[1]; // expected-warning{{unchecked operation on raw buffer in expression}}
};
}
@@ -213,4 +213,66 @@ 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