[clang] 777eb4b - [-Wunsafe-buffer-usage] Handle unevaluated contexts that contain unsafe buffer usages
via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 19 15:53:31 PDT 2023
Author: MalavikaSamak
Date: 2023-04-19T15:53:21-07:00
New Revision: 777eb4bcfc3265359edb7c979d3e5ac699ad4641
URL: https://github.com/llvm/llvm-project/commit/777eb4bcfc3265359edb7c979d3e5ac699ad4641
DIFF: https://github.com/llvm/llvm-project/commit/777eb4bcfc3265359edb7c979d3e5ac699ad4641.diff
LOG: [-Wunsafe-buffer-usage] Handle unevaluated contexts that contain unsafe buffer usages
This patch handles unevaluated contexts to ensure no warnings are produced by the machinery
for buffer access made within an unevaluated contexts. However, such accesses must be
considered by a FixableGadget and produce the necessary fixits.
Reviewed by: NoQ, ziqingluo-90, jkorous
Differential revision: https://reviews.llvm.org/D144905
Added:
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-unevaluated-context.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-unevaluated-context.cpp
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 99081490848e2..7a432ea5323a4 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -35,9 +35,10 @@ class MatchDescendantVisitor
MatchDescendantVisitor(const internal::DynTypedMatcher *Matcher,
internal::ASTMatchFinder *Finder,
internal::BoundNodesTreeBuilder *Builder,
- internal::ASTMatchFinder::BindKind Bind)
+ internal::ASTMatchFinder::BindKind Bind,
+ const bool ignoreUnevaluatedContext)
: Matcher(Matcher), Finder(Finder), Builder(Builder), Bind(Bind),
- Matches(false) {}
+ Matches(false), ignoreUnevaluatedContext(ignoreUnevaluatedContext) {}
// Returns true if a match is found in a subtree of `DynNode`, which belongs
// to the same callable of `DynNode`.
@@ -70,6 +71,48 @@ 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;
@@ -111,6 +154,7 @@ 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.
@@ -121,11 +165,18 @@ static auto hasPointerType() {
static auto hasArrayType() {
return hasType(hasCanonicalType(arrayType()));
}
-
-AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher<Stmt>, innerMatcher) {
+
+AST_MATCHER_P(Stmt, forEachDescendantEvaluatedStmt, internal::Matcher<Stmt>, innerMatcher) {
const DynTypedMatcher &DTM = static_cast<DynTypedMatcher>(innerMatcher);
- MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All);
+ 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);
return Visitor.findMatch(DynTypedNode::create(Node));
}
@@ -870,32 +921,31 @@ findGadgets(const Decl *D, const UnsafeBufferUsageHandler &Handler) {
// clang-format off
M.addMatcher(
- stmt(forEveryDescendant(
- eachOf(
+ stmt(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).
- stmt(eachOf(
+ forEachDescendantStmt(stmt(eachOf(
#define FIXABLE_GADGET(x) \
x ## Gadget::matcher().bind(#x),
#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(
+ // 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"
- // 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")
- )))
- )),
+ // 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")
+ ))
+ ))),
&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
new file mode 100644
index 0000000000000..10d2e3e7484eb
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-unevaluated-context.cpp
@@ -0,0 +1,79 @@
+// 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
new file mode 100644
index 0000000000000..b3bcfa8c65d80
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-unevaluated-context.cpp
@@ -0,0 +1,50 @@
+// 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 783c8ea865112..62aeb1c24b547 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -102,12 +102,6 @@ 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