[clang] 7bf5f46 - Revert "[-Wunsafe-buffer-usage] Handle unevaluated contexts that contain unsafe buffer usages"

via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 19 16:09:31 PDT 2023


Author: MalavikaSamak
Date: 2023-04-19T16:09:21-07:00
New Revision: 7bf5f4692ad6f9ba2d5c155f6b630049bb59876f

URL: https://github.com/llvm/llvm-project/commit/7bf5f4692ad6f9ba2d5c155f6b630049bb59876f
DIFF: https://github.com/llvm/llvm-project/commit/7bf5f4692ad6f9ba2d5c155f6b630049bb59876f.diff

LOG: Revert "[-Wunsafe-buffer-usage] Handle unevaluated contexts that contain unsafe buffer usages"

This reverts commit 777eb4bcfc3265359edb7c979d3e5ac699ad4641.

Added: 
    

Modified: 
    clang/lib/Analysis/UnsafeBufferUsage.cpp
    clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp

Removed: 
    clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-unevaluated-context.cpp
    clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-unevaluated-context.cpp


################################################################################
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<Stmt>, innerMatcher) {
+  
+AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher<Stmt>, innerMatcher) {
   const DynTypedMatcher &DTM = static_cast<DynTypedMatcher>(innerMatcher);
 
-  MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All, true);
-  return Visitor.findMatch(DynTypedNode::create(Node));
-}
-
-AST_MATCHER_P(Stmt, forEachDescendantStmt, internal::Matcher<Stmt>, innerMatcher) {
-  const DynTypedMatcher &DTM = static_cast<DynTypedMatcher>(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<int> 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<int> 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<int> 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 at -1{{'p' is an unsafe pointer used for buffer access}}
   // expected-warning at -2{{'q' is an unsafe pointer used for buffer access}}


        


More information about the cfe-commits mailing list