diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 7a432ea5323a4..99081490848e2 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -35,10 +35,9 @@ class MatchDescendantVisitor MatchDescendantVisitor(const internal::DynTypedMatcher *Matcher, internal::ASTMatchFinder *Finder, internal::BoundNodesTreeBuilder *Builder, - internal::ASTMatchFinder::BindKind Bind, - const bool ignoreUnevaluatedContext) + internal::ASTMatchFinder::BindKind Bind) : Matcher(Matcher), Finder(Finder), Builder(Builder), Bind(Bind), - Matches(false), ignoreUnevaluatedContext(ignoreUnevaluatedContext) {} + Matches(false) {} // Returns true if a match is found in a subtree of `DynNode`, which belongs // to the same callable of `DynNode`. @@ -71,48 +70,6 @@ class MatchDescendantVisitor return VisitorBase::TraverseDecl(Node); } - bool TraverseGenericSelectionExpr(GenericSelectionExpr *Node) { - // These are unevaluated, except the result expression. - if(ignoreUnevaluatedContext) - return TraverseStmt(Node->getResultExpr()); - return VisitorBase::TraverseGenericSelectionExpr(Node); - } - - bool TraverseUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *Node) { - // Unevaluated context. - if(ignoreUnevaluatedContext) - return true; - return VisitorBase::TraverseUnaryExprOrTypeTraitExpr(Node); - } - - bool TraverseTypeOfExprTypeLoc(TypeOfExprTypeLoc Node) { - // Unevaluated context. - if(ignoreUnevaluatedContext) - return true; - return VisitorBase::TraverseTypeOfExprTypeLoc(Node); - } - - bool TraverseDecltypeTypeLoc(DecltypeTypeLoc Node) { - // Unevaluated context. - if(ignoreUnevaluatedContext) - return true; - return VisitorBase::TraverseDecltypeTypeLoc(Node); - } - - bool TraverseCXXNoexceptExpr(CXXNoexceptExpr *Node) { - // Unevaluated context. - if(ignoreUnevaluatedContext) - return true; - return VisitorBase::TraverseCXXNoexceptExpr(Node); - } - - bool TraverseCXXTypeidExpr(CXXTypeidExpr *Node) { - // Unevaluated context. - if(ignoreUnevaluatedContext) - return true; - return VisitorBase::TraverseCXXTypeidExpr(Node); - } - bool TraverseStmt(Stmt *Node, DataRecursionQueue *Queue = nullptr) { if (!Node) return true; @@ -154,7 +111,6 @@ class MatchDescendantVisitor internal::BoundNodesTreeBuilder ResultBindings; const internal::ASTMatchFinder::BindKind Bind; bool Matches; - bool ignoreUnevaluatedContext; }; // Because we're dealing with raw pointers, let's define what we mean by that. @@ -165,18 +121,11 @@ static auto hasPointerType() { static auto hasArrayType() { return hasType(hasCanonicalType(arrayType())); } - -AST_MATCHER_P(Stmt, forEachDescendantEvaluatedStmt, internal::Matcher, innerMatcher) { + +AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher, innerMatcher) { const DynTypedMatcher &DTM = static_cast(innerMatcher); - MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All, true); - return Visitor.findMatch(DynTypedNode::create(Node)); -} - -AST_MATCHER_P(Stmt, forEachDescendantStmt, internal::Matcher, innerMatcher) { - const DynTypedMatcher &DTM = static_cast(innerMatcher); - - MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All, false); + MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All); return Visitor.findMatch(DynTypedNode::create(Node)); } @@ -921,31 +870,32 @@ findGadgets(const Decl *D, const UnsafeBufferUsageHandler &Handler) { // clang-format off M.addMatcher( - stmt(eachOf( + stmt(forEveryDescendant( + eachOf( // A `FixableGadget` matcher and a `WarningGadget` matcher should not disable // each other (they could if they were put in the same `anyOf` group). // We also should make sure no two `FixableGadget` (resp. `WarningGadget`) matchers // match for the same node, so that we can group them // in one `anyOf` group (for better performance via short-circuiting). - forEachDescendantStmt(stmt(eachOf( + stmt(eachOf( #define FIXABLE_GADGET(x) \ x ## Gadget::matcher().bind(#x), -#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def" - // In parallel, match all DeclRefExprs so that to find out - // whether there are any uncovered by gadgets. - declRefExpr(anyOf(hasPointerType(), hasArrayType()), to(varDecl())).bind("any_dre") - ))), - forEachDescendantEvaluatedStmt(stmt(anyOf( - // Add Gadget::matcher() for every gadget in the registry. -#define WARNING_GADGET(x) \ - allOf(x ## Gadget::matcher().bind(#x), notInSafeBufferOptOut(&Handler)), #include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def" // Also match DeclStmts because we'll need them when fixing // their underlying VarDecls that otherwise don't have // any backreferences to DeclStmts. declStmt().bind("any_ds") - )) - ))), + )), + stmt(anyOf( + // Add Gadget::matcher() for every gadget in the registry. +#define WARNING_GADGET(x) \ + allOf(x ## Gadget::matcher().bind(#x), notInSafeBufferOptOut(&Handler)), +#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def" + // In parallel, match all DeclRefExprs so that to find out + // whether there are any uncovered by gadgets. + declRefExpr(anyOf(hasPointerType(), hasArrayType()), to(varDecl())).bind("any_dre") + ))) + )), &CB ); // clang-format on diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-unevaluated-context.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-unevaluated-context.cpp deleted file mode 100644 index 10d2e3e7484eb..0000000000000 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-unevaluated-context.cpp +++ /dev/null @@ -1,79 +0,0 @@ -// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fsyntax-only %s 2>&1 | FileCheck %s - -namespace std { - class type_info; - class bad_cast; - class bad_typeid; -} -using size_t = __typeof(sizeof(int)); -void *malloc(size_t); - -void foo(...); -int bar(int *ptr); - -void uneval_context_fix_pointer_dereference() { - auto p = new int[10]; - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p" - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}" - - int tmp = p[5]; - typeid(foo(*p)); - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:14-[[@LINE-1]]:15}:"" - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"[0]" - _Generic(*p, int: 2, float: 3); - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:13}:"" - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:14-[[@LINE-2]]:14}:"[0]" -} - -void uneval_context_fix_pointer_array_access() { - auto p = new int[10]; - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p" - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}" - - int tmp = p[5]; - typeid(foo(p[5])); - _Generic(p[2], int: 2, float: 3); -} - -void uneval_context_fix_pointer_reference() { - auto p = new int[10]; - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p" - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}" - - int tmp = p[5]; - typeid(bar(p)); - // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:15-[[@LINE-1]]:15}:".data()" -} - -// The FixableGagdtes are not working in the following scenarios: -// 1. sizeof(DRE) -// 2. typeid(DRE) -// 3. __typeof(DRE) -// 4. _Generic(expr, type_1: DRE, type_2:) -// 5. decltype(DRE) var = y; -// 6. noexcept(DRE); -// This is becauste the UPC and ULC context matchers do not handle these contexts -// and almost all FixableGagdets currently depend on these matchers. - -// FIXME: Emit fixits for each of the below use. -void uneval_context_fix_pointer_dereference_not_handled() { - auto p = new int[10]; - int tmp = p[5]; - - foo(sizeof(*p), sizeof(decltype(*p))); - __typeof(*p) x; - int *q = (int *)malloc(sizeof(*p)); - int y = sizeof(*p); - __is_pod(__typeof(*p)); - __is_trivially_constructible(__typeof(*p), decltype(*p)); - _Generic(*p, int: 2, float: 3); - _Generic(1, int: *p, float: 3); - _Generic(1, int: 2, float: *p); - decltype(*p) var = y; - noexcept(*p); - typeid(*p); -} - diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-unevaluated-context.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-unevaluated-context.cpp deleted file mode 100644 index b3bcfa8c65d80..0000000000000 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-unevaluated-context.cpp +++ /dev/null @@ -1,50 +0,0 @@ -// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -fblocks -include %s -verify %s - -// RUN: %clang -x c++ -fsyntax-only -fblocks -include %s %s 2>&1 | FileCheck --allow-empty %s -// RUN: %clang_cc1 -std=c++11 -fblocks -include %s %s 2>&1 | FileCheck --allow-empty %s -// RUN: %clang_cc1 -std=c++20 -fblocks -include %s %s 2>&1 | FileCheck --allow-empty %s -// CHECK-NOT: [-Wunsafe-buffer-usage] - -#ifndef INCLUDED -#define INCLUDED -#pragma clang system_header - -// no spanification warnings for system headers -void foo(...); // let arguments of `foo` to hold testing expressions -#else - -namespace std { - class type_info; - class bad_cast; - class bad_typeid; -} -using size_t = __typeof(sizeof(int)); -void *malloc(size_t); - -void foo(int v) { -} - -void foo(int *p){} - -void uneval_context_fix() { - auto p = new int[10]; // expected-warning{{'p' is an unsafe pointer used for buffer access}} - - // Warn on the following DREs - _Generic(1, int: p[2], float: 3); // expected-note{{used in buffer access here}} - - // Do not warn for following DREs - auto q = new int[10]; - foo(sizeof(q[1]), // no-note - sizeof(decltype(q[1]))); // no-note - __typeof(q[5]) x; // no-note - int *r = (int *)malloc(sizeof(q[5])); // no-note - int y = sizeof(q[5]); // no-note - __is_pod(__typeof(q[5])); // no-note - __is_trivially_constructible(__typeof(q[5]), decltype(q[5])); // no-note - _Generic(q[1], int: 2, float: 3); // no-note - _Generic(1, int: 2, float: q[3]); // no-note - decltype(q[2]) var = y; // no-note - noexcept(q[2]); // no-note - typeid(q[3]); // no-note -} -#endif diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp index 62aeb1c24b547..783c8ea865112 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp @@ -102,6 +102,12 @@ void testArraySubscriptsWithAuto(int *p, int **pp) { foo(ap4[1]); // expected-note{{used in buffer access here}} } +//TODO: do not warn for unevaluated context +void testUnevaluatedContext(int * p) {// expected-warning{{'p' is an unsafe pointer used for buffer access}} + foo(sizeof(p[1]), // expected-note{{used in buffer access here}} + sizeof(decltype(p[1]))); // expected-note{{used in buffer access here}} +} + void testQualifiedParameters(const int * p, const int * const q, const int a[10], const int b[10][10]) { // expected-warning@-1{{'p' is an unsafe pointer used for buffer access}} // expected-warning@-2{{'q' is an unsafe pointer used for buffer access}}