[clang] a046d18 - [-Wunsafe-buffer-usage] FixableGadget for handling stand alone pointers under UPC
via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 7 15:32:28 PDT 2023
Author: MalavikaSamak
Date: 2023-04-07T15:32:19-07:00
New Revision: a046d187720137d944cece4aa4561f4bceb54e3c
URL: https://github.com/llvm/llvm-project/commit/a046d187720137d944cece4aa4561f4bceb54e3c
DIFF: https://github.com/llvm/llvm-project/commit/a046d187720137d944cece4aa4561f4bceb54e3c.diff
LOG: [-Wunsafe-buffer-usage] FixableGadget for handling stand alone pointers under UPC
This patch introduces UPCStandalonePointerGadget, a FixableGadget that emits fixits to
handle cases where a pointer identified as unsafe is simply referenced. An example of
such a case is when the pointer is input as an argument to a method call, where we can
not change the type of the argument. For cases where the strategy for the unsafe pointer is
to use std::span, the idea is to extract the underlying pointer by invoking the "data()"
method on the span instance.
For example, the gadget emits a fixit for S3, where S1, S2 are handled by other gadgets:
S1: int *ptr = new int[10];
S2: int val1 = ptr[k]; // Unsafe operation on ptr
S3: foo(ptr); // Some method that accepts raw pointer => FIXIT: foo(ptr.data());
Reviewed by: NoQ, ziqingluo-90, jkorous
Differential revision: https://reviews.llvm.org/D143676
Added:
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp
Modified:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/lib/Analysis/UnsafeBufferUsage.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index f0c99a767071f..a52b00bf8d4ce 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -34,6 +34,7 @@ FIXABLE_GADGET(ULCArraySubscript) // `DRE[any]` in an Unspecified Lvalu
FIXABLE_GADGET(DerefSimplePtrArithFixable)
FIXABLE_GADGET(PointerDereference)
FIXABLE_GADGET(UPCAddressofArraySubscript) // '&DRE[any]' in an Unspecified Pointer Context
+FIXABLE_GADGET(UPCStandalonePointer)
#undef FIXABLE_GADGET
#undef WARNING_GADGET
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 1ef276ab90444..9fb928f6c4c68 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -165,17 +165,22 @@ isInUnspecifiedPointerContext(internal::Matcher<Stmt> InnerMatcher) {
// 1. an argument of a function call (except the callee has [[unsafe_...]]
// attribute), or
// 2. the operand of a cast operation; or
- // ...
+ // 3. the operand of a comparator operation; or
auto CallArgMatcher =
- callExpr(hasAnyArgument(allOf(
- hasPointerType() /* array also decays to pointer type*/,
- InnerMatcher)),
- unless(callee(functionDecl(hasAttr(attr::UnsafeBufferUsage)))));
+ callExpr(forEachArgumentWithParam(InnerMatcher,
+ hasPointerType() /* array also decays to pointer type*/),
+ unless(callee(functionDecl(hasAttr(attr::UnsafeBufferUsage)))));
+
auto CastOperandMatcher =
explicitCastExpr(hasCastKind(CastKind::CK_PointerToIntegral),
castSubExpr(allOf(hasPointerType(), InnerMatcher)));
- return stmt(anyOf(CallArgMatcher, CastOperandMatcher));
+ auto CompOperandMatcher =
+ binaryOperator(hasAnyOperatorName("!=", "==", "<", "<=", ">", ">="),
+ eachOf(hasLHS(allOf(hasPointerType(), InnerMatcher)),
+ hasRHS(allOf(hasPointerType(), InnerMatcher))));
+
+ return stmt(anyOf(CallArgMatcher, CastOperandMatcher, CompOperandMatcher));
// FIXME: any more cases? (UPC excludes the RHS of an assignment. For now we
// don't have to check that.)
}
@@ -489,6 +494,41 @@ class ULCArraySubscriptGadget : public FixableGadget {
}
};
+// Fixable gadget to handle stand alone pointers of the form `UPC(DRE)` in the
+// unspecified pointer context (isInUnspecifiedPointerContext). The gadget emits
+// fixit of the form `UPC(DRE.data())`.
+class UPCStandalonePointerGadget : public FixableGadget {
+private:
+ static constexpr const char *const DeclRefExprTag = "StandalonePointer";
+ const DeclRefExpr *Node;
+
+public:
+ UPCStandalonePointerGadget(const MatchFinder::MatchResult &Result)
+ : FixableGadget(Kind::UPCStandalonePointer),
+ Node(Result.Nodes.getNodeAs<DeclRefExpr>(DeclRefExprTag)) {
+ assert(Node != nullptr && "Expecting a non-null matching result");
+ }
+
+ static bool classof(const Gadget *G) {
+ return G->getKind() == Kind::UPCStandalonePointer;
+ }
+
+ static Matcher matcher() {
+ auto ArrayOrPtr = anyOf(hasPointerType(), hasArrayType());
+ auto target = expr(
+ ignoringParenImpCasts(declRefExpr(allOf(ArrayOrPtr, to(varDecl()))).bind(DeclRefExprTag)));
+ return stmt(isInUnspecifiedPointerContext(target));
+ }
+
+ virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+
+ virtual const Stmt *getBaseStmt() const override { return Node; }
+
+ virtual DeclUseList getClaimedVarUseSites() const override {
+ return {Node};
+ }
+};
+
class PointerDereferenceGadget : public FixableGadget {
static constexpr const char *const BaseDeclRefExprTag = "BaseDRE";
static constexpr const char *const OperatorTag = "op";
@@ -1070,6 +1110,31 @@ PointerDereferenceGadget::getFixits(const Strategy &S) const {
return std::nullopt;
}
+// Generates fix-its replacing an expression of the form UPC(DRE) with
+// `DRE.data()`
+std::optional<FixItList> UPCStandalonePointerGadget::getFixits(const Strategy &S)
+ const {
+ const auto VD = cast<VarDecl>(Node->getDecl());
+ switch (S.lookup(VD)) {
+ case Strategy::Kind::Span: {
+ ASTContext &Ctx = VD->getASTContext();
+ SourceManager &SM = Ctx.getSourceManager();
+ // Inserts the .data() after the DRE
+ SourceLocation endOfOperand = getEndCharLoc(Node, SM, Ctx.getLangOpts());
+
+ return FixItList{{FixItHint::CreateInsertion(
+ endOfOperand.getLocWithOffset(1), ".data()")}};
+ }
+ case Strategy::Kind::Wontfix:
+ case Strategy::Kind::Iterator:
+ case Strategy::Kind::Array:
+ case Strategy::Kind::Vector:
+ llvm_unreachable("unsupported strategies for FixableGadgets");
+ }
+
+ return std::nullopt;
+}
+
// Generates fix-its replacing an expression of the form `&DRE[e]` with
// `&DRE.data()[e]`:
static std::optional<FixItList>
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp
new file mode 100644
index 0000000000000..109f8b19db5aa
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp
@@ -0,0 +1,110 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+void foo(int* v) {
+}
+
+void m1(int* v1, int* v2, int) {
+
+}
+
+void condition_check_nullptr() {
+ 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];
+ if(p != nullptr) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+}
+
+void condition_check() {
+ 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}"
+
+ auto q = new int[10];
+
+ int tmp = p[5];
+ if(p == q) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+
+ if(q != p){}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:12}:".data()"
+
+ if(p < q) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+
+ if(p <= q) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+
+ if(p > q) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+
+ if(p >= q) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+
+ if( p == q && p != nullptr) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:8-[[@LINE-1]]:8}:".data()"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:18-[[@LINE-2]]:18}:".data()"
+}
+
+void condition_check_two_usafe_buffers() {
+ 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}"
+
+ auto q = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> q"
+ // 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];
+ tmp = q[5];
+
+ if(p == q) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:".data()"
+}
+
+void unsafe_method_invocation_single_param() {
+ 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];
+ foo(p);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:8-[[@LINE-1]]:8}:".data()"
+
+}
+
+void safe_method_invocation_single_param() {
+ auto p = new int[10];
+ foo(p);
+}
+
+void unsafe_method_invocation_double_param() {
+ 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];
+ m1(p, p, 10);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:10}:".data()"
+
+ auto q = new int[10];
+
+ m1(p, q, 4);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+
+ m1(q, p, 9);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:10}:".data()"
+
+ m1(q, q, 8);
+}
+
More information about the cfe-commits
mailing list