[clang] 762af11 - [-Wunsafe-buffer-usage] Add a Fixable for pointer pre-increment
Ziqing Luo via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 12 15:00:23 PDT 2023
Author: Ziqing Luo
Date: 2023-04-12T14:51:46-07:00
New Revision: 762af11d4c5d0bd1e76f23a07087773db09ef17d
URL: https://github.com/llvm/llvm-project/commit/762af11d4c5d0bd1e76f23a07087773db09ef17d
DIFF: https://github.com/llvm/llvm-project/commit/762af11d4c5d0bd1e76f23a07087773db09ef17d.diff
LOG: [-Wunsafe-buffer-usage] Add a Fixable for pointer pre-increment
For a pointer type expression `e` of the form `++DRE`, if `e` is under
an Unspecified Pointer Context (UPC) and `DRE` is suppose to be
transformed to have std:span type, we generate fix-its that transform `e` to
`(DRE = DRE.subspan(1)).data()`.
For reference, `e` is in an UPC if `e` is
- an argument of a function call (except the callee has [[unsafe_buffer_usage]] attribute), or
- the operand of a cast-to-(Integer or Boolean) operation; or
- the operand of a pointer subtraction operation; or
- the operand of a pointer comparison operation;
We may extend the definition of UPC by adding more cases later.
Reviewed by: NoQ (Artem Dergachev)
Differential revision: https://reviews.llvm.org/D144304
Added:
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.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 a52b00bf8d4ce..a112b6d105025 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -35,6 +35,7 @@ FIXABLE_GADGET(DerefSimplePtrArithFixable)
FIXABLE_GADGET(PointerDereference)
FIXABLE_GADGET(UPCAddressofArraySubscript) // '&DRE[any]' in an Unspecified Pointer Context
FIXABLE_GADGET(UPCStandalonePointer)
+FIXABLE_GADGET(UPCPreIncrement) // '++Ptr' in an Unspecified Pointer Context
#undef FIXABLE_GADGET
#undef WARNING_GADGET
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 6a597c8851002..ff818140b2d89 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -139,6 +139,11 @@ AST_MATCHER_P(CastExpr, castSubExpr, internal::Matcher<Expr>, innerMatcher) {
return innerMatcher.matches(*Node.getSubExpr(), Finder, Builder);
}
+// Matches a `UnaryOperator` whose operator is pre-increment:
+AST_MATCHER(UnaryOperator, isPreInc) {
+ return Node.getOpcode() == UnaryOperator::Opcode::UO_PreInc;
+}
+
// Returns a matcher that matches any expression 'e' such that `innerMatcher`
// matches 'e' and 'e' is in an Unspecified Lvalue Context.
static auto isInUnspecifiedLvalueContext(internal::Matcher<Expr> innerMatcher) {
@@ -719,6 +724,46 @@ class Strategy {
};
} // namespace
+
+// Representing a pointer type expression of the form `++Ptr` in an Unspecified
+// Pointer Context (UPC):
+class UPCPreIncrementGadget : public FixableGadget {
+private:
+ static constexpr const char *const UPCPreIncrementTag =
+ "PointerPreIncrementUnderUPC";
+ const UnaryOperator *Node; // the `++Ptr` node
+
+public:
+ UPCPreIncrementGadget(const MatchFinder::MatchResult &Result)
+ : FixableGadget(Kind::UPCPreIncrement),
+ Node(Result.Nodes.getNodeAs<UnaryOperator>(UPCPreIncrementTag)) {
+ assert(Node != nullptr && "Expecting a non-null matching result");
+ }
+
+ static bool classof(const Gadget *G) {
+ return G->getKind() == Kind::UPCPreIncrement;
+ }
+
+ static Matcher matcher() {
+ // Note here we match `++Ptr` for any expression `Ptr` of pointer type.
+ // Although currently we can only provide fix-its when `Ptr` is a DRE, we
+ // can have the matcher be general, so long as `getClaimedVarUseSites` does
+ // things right.
+ return stmt(isInUnspecifiedPointerContext(expr(ignoringImpCasts(
+ unaryOperator(isPreInc(),
+ hasUnaryOperand(declRefExpr())
+ ).bind(UPCPreIncrementTag)))));
+ }
+
+ virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+
+ virtual const Stmt *getBaseStmt() const override { return Node; }
+
+ virtual DeclUseList getClaimedVarUseSites() const override {
+ return {dyn_cast<DeclRefExpr>(Node->getSubExpr())};
+ }
+};
+
// Representing a fixable expression of the form `*(ptr + 123)` or `*(123 +
// ptr)`:
class DerefSimplePtrArithFixableGadget : public FixableGadget {
@@ -1196,6 +1241,36 @@ fixUPCAddressofArraySubscriptWithSpan(const UnaryOperator *Node) {
FixItHint::CreateReplacement(Node->getSourceRange(), SS.str())};
}
+
+std::optional<FixItList> UPCPreIncrementGadget::getFixits(const Strategy &S) const {
+ DeclUseList DREs = getClaimedVarUseSites();
+
+ if (DREs.size() != 1)
+ return std::nullopt; // In cases of `++Ptr` where `Ptr` is not a DRE, we
+ // give up
+ if (const VarDecl *VD = dyn_cast<VarDecl>(DREs.front()->getDecl())) {
+ if (S.lookup(VD) == Strategy::Kind::Span) {
+ FixItList Fixes;
+ std::stringstream SS;
+ const Stmt *PreIncNode = getBaseStmt();
+ StringRef varName = VD->getName();
+ const ASTContext &Ctx = VD->getASTContext();
+
+ // To transform UPC(++p) to UPC((p = p.subspan(1)).data()):
+ SS << "(" << varName.data() << " = " << varName.data()
+ << ".subspan(1)).data()";
+ Fixes.push_back(FixItHint::CreateReplacement(
+ SourceRange(PreIncNode->getBeginLoc(),
+ getEndCharLoc(PreIncNode, Ctx.getSourceManager(),
+ Ctx.getLangOpts())),
+ SS.str()));
+ return Fixes;
+ }
+ }
+ return std::nullopt; // Not in the cases that we can handle for now, give up.
+}
+
+
// For a non-null initializer `Init` of `T *` type, this function returns
// `FixItHint`s producing a list initializer `{Init, S}` as a part of a fix-it
// to output stream.
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp
new file mode 100644
index 0000000000000..9bad7cb558352
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+void foo(int * , int *);
+
+void simple() {
+ int * p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
+ bool b = ++p;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:15}:"(p = p.subspan(1)).data()"
+ unsigned long n = (unsigned long) ++p;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:37-[[@LINE-1]]:40}:"(p = p.subspan(1)).data()"
+ if (++p) {
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:10}:"(p = p.subspan(1)).data()"
+ }
+ if (++p - ++p) {
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:10}:"(p = p.subspan(1)).data()"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:16}:"(p = p.subspan(1)).data()"
+ }
+ foo(++p, p);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:10}:"(p = p.subspan(1)).data()"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:".data()"
+
+ // FIXME: Don't know how to fix the following cases:
+ // CHECK-NOT: fix-it:"{{.*}}":{
+ int * g = new int[10];
+ int * a[10];
+ a[0] = ++g;
+ foo(g++, g);
+ if (++(a[0])) {
+ }
+}
More information about the cfe-commits
mailing list