ziqingluo-90 created this revision.
ziqingluo-90 added reviewers: jkorous, NoQ, t-rasmud, malavikasamak.
Herald added a project: All.
ziqingluo-90 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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 `span` type,
we generate fix-its that transform `e` to `(DRE = DRE.subspan(1)).data()`.
For reference, the UPC is currently defined as
- 1. an argument of a function call (except the callee has
[[unsafe_buffer_usage]] attribute), or
- 2. the operand of a cast-to-(Integer or Boolean) operation; or
- 3. the operand of a pointer subtraction operation; or
- 4. the operand of a pointer comparison operation;
We may extend the definition of UPC by adding more cases later.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D144304
Files:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp
===================================================================
--- /dev/null
+++ 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()"
+
+ // 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])) {
+ }
+}
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===================================================================
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -24,6 +24,7 @@
#include "llvm/ADT/SmallVector.h"
#include <memory>
#include <optional>
+#include <sstream>
using namespace llvm;
using namespace clang;
@@ -145,6 +146,11 @@
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) {
@@ -628,6 +634,46 @@
return {};
}
};
+
+// 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()).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 {
+ if (auto DRE = dyn_cast<DeclRefExpr>(Node->getSubExpr())) {
+ return {DRE};
+ }
+ return {};
+ }
+};
} // namespace
namespace {
@@ -1199,6 +1245,34 @@
return std::nullopt;
}
+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.
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(ULCArraySubscript) // `DRE[any]` in an Unspecified Lvalue Context
FIXABLE_GADGET(PointerDereference)
FIXABLE_GADGET(UPCAddressofArraySubscript) // '&DRE[any]' in an Unspecified Pointer Context
+FIXABLE_GADGET(UPCPreIncrement) // '++Ptr' in an Unspecified Pointer Context
FIXABLE_GADGET(DerefSimplePtrArithFixable)
FIXABLE_GADGET(PointerCtxAccess)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
