This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa046d1877201: [-Wunsafe-buffer-usage] FixableGadget for
handling stand alone pointers under… (authored by malavikasamak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D143676?vs=507918&id=511810#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143676/new/
https://reviews.llvm.org/D143676
Files:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp
===================================================================
--- /dev/null
+++ 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);
+}
+
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===================================================================
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -165,17 +165,22 @@
// 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 @@
}
};
+// 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 @@
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>
Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
===================================================================
--- clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -34,6 +34,7 @@
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
