[clang] Add FixableGadget for AddAssign in UnspecifiedUntypedContext (PR #71862)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 9 12:34:13 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Rashmi Mudduluru (t-rasmud)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/71862.diff
3 Files Affected:
- (modified) clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def (+1)
- (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+92)
- (added) clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp (+40)
``````````diff
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index ff687a0d178bdea..757ee452ced7488 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -36,6 +36,7 @@ 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
+FIXABLE_GADGET(UUCAddAssign) // 'Ptr += n' in an Unspecified Untyped Context
FIXABLE_GADGET(PointerAssignment)
FIXABLE_GADGET(PointerInit)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index e332a3609290aac..54923620274c0d5 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1028,6 +1028,52 @@ class UPCPreIncrementGadget : public FixableGadget {
}
};
+// Representing a pointer type expression of the form `Ptr += n` in an
+// Unspecified Untyped Context (UUC):
+class UUCAddAssignGadget : public FixableGadget {
+private:
+ static constexpr const char *const UUCAddAssignTag =
+ "PointerAddAssignUnderUUC";
+ static constexpr const char *const IntOffsetTag = "IntOffset";
+ static constexpr const char *const OffsetTag = "Offset";
+
+ const BinaryOperator *Node; // the `Ptr += n` node
+ const IntegerLiteral *IntOffset = nullptr;
+ const DeclRefExpr *Offset = nullptr;
+
+public:
+ UUCAddAssignGadget(const MatchFinder::MatchResult &Result)
+ : FixableGadget(Kind::UUCAddAssign),
+ Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)),
+ IntOffset(Result.Nodes.getNodeAs<IntegerLiteral>(IntOffsetTag)),
+ Offset(Result.Nodes.getNodeAs<DeclRefExpr>(OffsetTag)) {
+ assert(Node != nullptr && "Expecting a non-null matching result");
+ }
+
+ static bool classof(const Gadget *G) {
+ return G->getKind() == Kind::UUCAddAssign;
+ }
+
+ static Matcher matcher() {
+ return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts(
+ binaryOperator(hasOperatorName("+="),
+ hasLHS(declRefExpr(
+ toSupportedVariable())),
+ hasRHS(expr(anyOf(
+ ignoringImpCasts(declRefExpr().bind(OffsetTag)),
+ integerLiteral().bind(IntOffsetTag))))
+ ).bind(UUCAddAssignTag)))));
+ }
+
+ 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->getLHS())};
+ }
+};
+
// Representing a fixable expression of the form `*(ptr + 123)` or `*(123 +
// ptr)`:
class DerefSimplePtrArithFixableGadget : public FixableGadget {
@@ -1766,6 +1812,52 @@ fixUPCAddressofArraySubscriptWithSpan(const UnaryOperator *Node) {
FixItHint::CreateReplacement(Node->getSourceRange(), SS.str())};
}
+std::optional<FixItList> UUCAddAssignGadget::getFixits(const Strategy &S) const {
+ DeclUseList DREs = getClaimedVarUseSites();
+
+ if (DREs.size() != 1)
+ return std::nullopt; // In cases of `Ptr += n` 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 *AddAssignNode = getBaseStmt();
+ StringRef varName = VD->getName();
+ const ASTContext &Ctx = VD->getASTContext();
+
+ std::string SubSpanOffset;
+ if (IntOffset) {
+ auto ConstVal = IntOffset->getIntegerConstantExpr(Ctx);
+ if (ConstVal->isNegative())
+ return std::nullopt;
+
+ SmallString<256> OffsetStr;
+ ConstVal->toString(OffsetStr);
+ SubSpanOffset = OffsetStr.c_str();
+ // To transform UUC(p += IntegerLiteral) to UUC(p = p.subspan(IntegerLiteral)):
+ SubSpanOffset = OffsetStr.c_str();
+ }
+ else {
+ SubSpanOffset = Offset->getDecl()->getName().str();
+ }
+
+ // To transform UUC(p += n) to UUC(p = p.subspan(..)):
+ SS << varName.data() << " = " << varName.data()
+ << ".subspan(" << SubSpanOffset << ")";
+
+ std::optional<SourceLocation> AddAssignLocation =
+ getEndCharLoc(AddAssignNode, Ctx.getSourceManager(), Ctx.getLangOpts());
+ if (!AddAssignLocation)
+ return std::nullopt;
+
+ Fixes.push_back(FixItHint::CreateReplacement(
+ SourceRange(AddAssignNode->getBeginLoc(), *AddAssignLocation), SS.str()));
+ return Fixes;
+ }
+ }
+ return std::nullopt; // Not in the cases that we can handle for now, give up.
+}
std::optional<FixItList> UPCPreIncrementGadget::getFixits(const Strategy &S) const {
DeclUseList DREs = getClaimedVarUseSites();
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp
new file mode 100644
index 000000000000000..30c587b2110d19b
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN: -fsafe-buffer-usage-suggestions \
+// RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+void foo(int * , int *);
+
+void add_assign_test(int n, int *a) {
+ int *p = new int[10];
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+ p += 2;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"p = p.subspan(2)"
+
+ int *r = p;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:13}:", <# placeholder #>}"
+ while (*r != 0) {
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:11}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"[0]"
+ r += 2;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"r = r.subspan(2)"
+ }
+
+ if (*p == 0) {
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]"
+ p += n;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(n)"
+ }
+
+ if (*p == 1)
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]"
+ p += 3;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(3)"
+
+ a += -9;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(-9)"
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/71862
More information about the cfe-commits
mailing list