[clang] 9516419 - Revert "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:54:47 PDT 2023
Author: MalavikaSamak
Date: 2023-04-19T16:53:34-07:00
New Revision: 9516419c50a9bb318fdde626716823e14a5fee71
URL: https://github.com/llvm/llvm-project/commit/9516419c50a9bb318fdde626716823e14a5fee71
DIFF: https://github.com/llvm/llvm-project/commit/9516419c50a9bb318fdde626716823e14a5fee71.diff
LOG: Revert "Revert "[-Wunsafe-buffer-usage] Handle unevaluated contexts that contain unsafe buffer usages""
This reverts commit 7bf5f4692ad6f9ba2d5c155f6b630049bb59876f and adding -frtti flag to support PS4/PS5 builds.
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 99081490848e..7a432ea5323a 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 000000000000..db0f1d293c61
--- /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 000000000000..1447f001ae9d
--- /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++ -frtti -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 783c8ea86511..62aeb1c24b54 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