https://github.com/t-rasmud created
https://github.com/llvm/llvm-project/pull/71862
None
>From 6636745d1c444747a33c91b366a730d81ca5db5a Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_muddul...@apple.com>
Date: Wed, 1 Nov 2023 13:43:12 -0700
Subject: [PATCH 1/3] [-Wunsafe-buffer-usage][WIP] Fixable gadget for AddAssign
---
.../Analyses/UnsafeBufferUsageGadgets.def | 1 +
clang/lib/Analysis/UnsafeBufferUsage.cpp | 65 +++++++++++++++++++
...-unsafe-buffer-usage-fixits-add-assign.cpp | 38 +++++++++++
3 files changed, 104 insertions(+)
create mode 100644
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp
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..7b79f5360c79d6e 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1028,6 +1028,42 @@ 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";
+ const BinaryOperator *Node; // the `Ptr += n` node
+
+public:
+ UUCAddAssignGadget(const MatchFinder::MatchResult &Result)
+ : FixableGadget(Kind::UUCAddAssign),
+ Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)) {
+ 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()))
+ ).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 +1802,35 @@ 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();
+
+ // To transform UUC(p += n) to UUC((p = p.subspan(1)).data()):
+ SS << varName.data() << " = " << varName.data()
+ << ".subspan(" << getUserFillPlaceHolder() << ")";
+ 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..e2b9a43dee9b3c3
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp
@@ -0,0 +1,38 @@
+// 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 *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(<#
placeholder #>)"
+
+ 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(<#
placeholder #>)"
+ }
+
+ if (*p == 0) {
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]"
+ p += 2;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(<#
placeholder #>)"
+ }
+
+ 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(<#
placeholder #>)"
+}
+
>From d5f00a5978b8a1b3574d7a3d0a18bce956138bb3 Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_muddul...@apple.com>
Date: Tue, 7 Nov 2023 16:02:02 -0800
Subject: [PATCH 2/3] [-Wunsafe-buffer-usage][WIP] Fixable gadget for
AddAssign: handle integer literal case
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 34 ++++++++++++++++---
...-unsafe-buffer-usage-fixits-add-assign.cpp | 11 +++---
2 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 7b79f5360c79d6e..dd8ccbdd1533aa4 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1034,12 +1034,16 @@ class UUCAddAssignGadget : public FixableGadget {
private:
static constexpr const char *const UUCAddAssignTag =
"PointerAddAssignUnderUUC";
+ static constexpr const char *const IntOffsetTag = "IntOffset";
+
const BinaryOperator *Node; // the `Ptr += n` node
+ const IntegerLiteral *IntOffset = nullptr;
public:
UUCAddAssignGadget(const MatchFinder::MatchResult &Result)
: FixableGadget(Kind::UUCAddAssign),
- Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)) {
+ Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)),
+ IntOffset(Result.Nodes.getNodeAs<IntegerLiteral>(IntOffsetTag)) {
assert(Node != nullptr && "Expecting a non-null matching result");
}
@@ -1051,7 +1055,10 @@ class UUCAddAssignGadget : public FixableGadget {
return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts(
binaryOperator(hasOperatorName("+="),
hasLHS(declRefExpr(
- toSupportedVariable()))
+ toSupportedVariable())),
+ hasRHS(expr(anyOf(
+ ignoringImpCasts(declRefExpr()),
+ integerLiteral().bind(IntOffsetTag))))
).bind(UUCAddAssignTag)))));
}
@@ -1815,10 +1822,27 @@ std::optional<FixItList>
UUCAddAssignGadget::getFixits(const Strategy &S) const
const Stmt *AddAssignNode = getBaseStmt();
StringRef varName = VD->getName();
const ASTContext &Ctx = VD->getASTContext();
-
- // To transform UUC(p += n) to UUC((p = p.subspan(1)).data()):
+
+ 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 = getUserFillPlaceHolder();
+ }
+
+ // To transform UUC(p += n) to UUC(p = p.subspan(..)):
SS << varName.data() << " = " << varName.data()
- << ".subspan(" << getUserFillPlaceHolder() << ")";
+ << ".subspan(" << SubSpanOffset << ")";
+
std::optional<SourceLocation> AddAssignLocation =
getEndCharLoc(AddAssignNode, Ctx.getSourceManager(),
Ctx.getLangOpts());
if (!AddAssignLocation)
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
index e2b9a43dee9b3c3..786335b5cfcef97 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp
@@ -3,13 +3,13 @@
// RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
void foo(int * , int *);
-void add_assign_test() {
+void add_assign_test(int n) {
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(<#
placeholder #>)"
+ // 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"
@@ -19,13 +19,13 @@ void add_assign_test() {
// 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(<#
placeholder #>)"
+ // 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 += 2;
+ p += n;
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(<#
placeholder #>)"
}
@@ -33,6 +33,5 @@ void add_assign_test() {
// 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(<#
placeholder #>)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(3)"
}
-
>From ec71495b80e209e7c2dffaaadb1f05814cfafaed Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_muddul...@apple.com>
Date: Thu, 9 Nov 2023 10:42:58 -0800
Subject: [PATCH 3/3] [-Wunsafe-buffer-usage][WIP] Fixable gadget for
AddAssign: handle DeclRefExpr, add test
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 9 ++++++---
.../warn-unsafe-buffer-usage-fixits-add-assign.cpp | 7 +++++--
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index dd8ccbdd1533aa4..54923620274c0d5 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1035,15 +1035,18 @@ class UUCAddAssignGadget : public FixableGadget {
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)) {
+ IntOffset(Result.Nodes.getNodeAs<IntegerLiteral>(IntOffsetTag)),
+ Offset(Result.Nodes.getNodeAs<DeclRefExpr>(OffsetTag)) {
assert(Node != nullptr && "Expecting a non-null matching result");
}
@@ -1057,7 +1060,7 @@ class UUCAddAssignGadget : public FixableGadget {
hasLHS(declRefExpr(
toSupportedVariable())),
hasRHS(expr(anyOf(
- ignoringImpCasts(declRefExpr()),
+
ignoringImpCasts(declRefExpr().bind(OffsetTag)),
integerLiteral().bind(IntOffsetTag))))
).bind(UUCAddAssignTag)))));
}
@@ -1836,7 +1839,7 @@ std::optional<FixItList>
UUCAddAssignGadget::getFixits(const Strategy &S) const
SubSpanOffset = OffsetStr.c_str();
}
else {
- SubSpanOffset = getUserFillPlaceHolder();
+ SubSpanOffset = Offset->getDecl()->getName().str();
}
// To transform UUC(p += n) to UUC(p = p.subspan(..)):
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
index 786335b5cfcef97..30c587b2110d19b 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp
@@ -3,7 +3,7 @@
// RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
void foo(int * , int *);
-void add_assign_test(int n) {
+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}:"{"
@@ -26,7 +26,7 @@ void add_assign_test(int n) {
// 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(<#
placeholder #>)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(n)"
}
if (*p == 1)
@@ -34,4 +34,7 @@ void add_assign_test(int n) {
// 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)"
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
