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