[clang] [-Wunsafe-buffer-usage] Add FixableGadget for AddAssign in UnspecifiedUntypedContext (PR #71862)
Rashmi Mudduluru via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 17 10:12:48 PST 2023
https://github.com/t-rasmud updated https://github.com/llvm/llvm-project/pull/71862
>From 6636745d1c444747a33c91b366a730d81ca5db5a Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_mudduluru at apple.com>
Date: Wed, 1 Nov 2023 13:43:12 -0700
Subject: [PATCH 01/14] [-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_mudduluru at apple.com>
Date: Tue, 7 Nov 2023 16:02:02 -0800
Subject: [PATCH 02/14] [-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_mudduluru at apple.com>
Date: Thu, 9 Nov 2023 10:42:58 -0800
Subject: [PATCH 03/14] [-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)"
}
>From b040dcb613f1fe74080d2900e7c51a554d93aa36 Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_mudduluru at apple.com>
Date: Thu, 9 Nov 2023 13:26:10 -0800
Subject: [PATCH 04/14] [-Wunsafe-buffer-usage] Fixable gadget for AddAssign:
handle DeclRefExpr, add test
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 44 +++++++++++-------------
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 54923620274c0d5..32d3f816b915020 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1033,7 +1033,7 @@ class UPCPreIncrementGadget : public FixableGadget {
class UUCAddAssignGadget : public FixableGadget {
private:
static constexpr const char *const UUCAddAssignTag =
- "PointerAddAssignUnderUUC";
+ "PointerAddAssignUnderUUC";
static constexpr const char *const IntOffsetTag = "IntOffset";
static constexpr const char *const OffsetTag = "Offset";
@@ -1043,10 +1043,10 @@ class UUCAddAssignGadget : public FixableGadget {
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)) {
+ : 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");
}
@@ -1056,13 +1056,11 @@ class UUCAddAssignGadget : public FixableGadget {
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)))));
+ 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;
@@ -1812,7 +1810,8 @@ fixUPCAddressofArraySubscriptWithSpan(const UnaryOperator *Node) {
FixItHint::CreateReplacement(Node->getSourceRange(), SS.str())};
}
-std::optional<FixItList> UUCAddAssignGadget::getFixits(const Strategy &S) const {
+std::optional<FixItList>
+UUCAddAssignGadget::getFixits(const Strategy &S) const {
DeclUseList DREs = getClaimedVarUseSites();
if (DREs.size() != 1)
@@ -1825,34 +1824,33 @@ std::optional<FixItList> UUCAddAssignGadget::getFixits(const Strategy &S) const
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 {
+ } else {
SubSpanOffset = Offset->getDecl()->getName().str();
}
// To transform UUC(p += n) to UUC(p = p.subspan(..)):
- SS << varName.data() << " = " << varName.data()
- << ".subspan(" << SubSpanOffset << ")";
+ SS << varName.data() << " = " << varName.data() << ".subspan("
+ << SubSpanOffset << ")";
- std::optional<SourceLocation> AddAssignLocation =
- getEndCharLoc(AddAssignNode, Ctx.getSourceManager(), Ctx.getLangOpts());
+ 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()));
+ SourceRange(AddAssignNode->getBeginLoc(), *AddAssignLocation),
+ SS.str()));
return Fixes;
}
}
>From 65ae8a823ae6c4826e2322bdf173f7611dd4e5b6 Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_mudduluru at apple.com>
Date: Thu, 9 Nov 2023 15:21:02 -0800
Subject: [PATCH 05/14] [-Wunsafe-buffer-usage] Fixable gadget for AddAssign:
fix formatting
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 32d3f816b915020..7ce114e3dd5a334 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1841,15 +1841,15 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
// To transform UUC(p += n) to UUC(p = p.subspan(..)):
SS << varName.data() << " = " << varName.data() << ".subspan("
- << SubSpanOffset << ")";
-
+ << 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),
+ SourceRange(AddAssignNode->getBeginLoc(), *AddAssignLocation),
SS.str()));
return Fixes;
}
>From 4ccd7e12e1708b7df4d05b029cda26f6219d934c Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_mudduluru at apple.com>
Date: Fri, 10 Nov 2023 12:01:46 -0800
Subject: [PATCH 06/14] [-Wunsafe-buffer-usage] Fixable gadget for AddAssign:
fix formatting
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 7ce114e3dd5a334..32e0868bd6c5ea8 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1036,7 +1036,7 @@ class UUCAddAssignGadget : public FixableGadget {
"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;
@@ -1838,7 +1838,7 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
} else {
SubSpanOffset = Offset->getDecl()->getName().str();
}
-
+
// To transform UUC(p += n) to UUC(p = p.subspan(..)):
SS << varName.data() << " = " << varName.data() << ".subspan("
<< SubSpanOffset << ")";
>From 19083c11752a5151ad556d6bc6858c8d704a1983 Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_mudduluru at apple.com>
Date: Tue, 14 Nov 2023 15:17:04 -0800
Subject: [PATCH 07/14] [-Wunsafe-buffer-usage] Fixable gadget for AddAssign:
Address comments
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 49 ++++++++++---------
...-unsafe-buffer-usage-fixits-add-assign.cpp | 17 ++++++-
2 files changed, 42 insertions(+), 24 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 32e0868bd6c5ea8..311e47e730ee280 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1034,19 +1034,16 @@ 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;
+ const Expr *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)) {
+ Offset(Result.Nodes.getNodeAs<Expr>(OffsetTag)){
assert(Node != nullptr && "Expecting a non-null matching result");
}
@@ -1058,8 +1055,7 @@ class UUCAddAssignGadget : public FixableGadget {
return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts(
binaryOperator(
hasOperatorName("+="), hasLHS(declRefExpr(toSupportedVariable())),
- hasRHS(expr(anyOf(ignoringImpCasts(declRefExpr().bind(OffsetTag)),
- integerLiteral().bind(IntOffsetTag)))))
+ hasRHS(ignoringParens(expr().bind(OffsetTag))))
.bind(UUCAddAssignTag)))));
}
@@ -1356,6 +1352,16 @@ PointerInitGadget::getFixits(const Strategy &S) const {
return std::nullopt;
}
+static bool isNonNegativeIntegerExpr(const Expr *Expr, const VarDecl *VD,
+ const ASTContext &Ctx) {
+ if (auto ConstVal = Expr->getIntegerConstantExpr(Ctx)) {
+ if (ConstVal->isNegative())
+ return false;
+ } else if (!Expr->getType()->isUnsignedIntegerType())
+ return false;
+ return true;
+}
+
std::optional<FixItList>
ULCArraySubscriptGadget::getFixits(const Strategy &S) const {
if (const auto *DRE =
@@ -1363,14 +1369,12 @@ ULCArraySubscriptGadget::getFixits(const Strategy &S) const {
if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
switch (S.lookup(VD)) {
case Strategy::Kind::Span: {
+
// If the index has a negative constant value, we give up as no valid
// fix-it can be generated:
const ASTContext &Ctx = // FIXME: we need ASTContext to be passed in!
VD->getASTContext();
- if (auto ConstVal = Node->getIdx()->getIntegerConstantExpr(Ctx)) {
- if (ConstVal->isNegative())
- return std::nullopt;
- } else if (!Node->getIdx()->getType()->isUnsignedIntegerType())
+ if (!isNonNegativeIntegerExpr(Node->getIdx(), VD, Ctx))
return std::nullopt;
// no-op is a good fix-it, otherwise
return FixItList{};
@@ -1825,19 +1829,18 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
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;
+ if (!isNonNegativeIntegerExpr(Offset, VD, Ctx))
+ return std::nullopt;
- SmallString<256> OffsetStr;
- ConstVal->toString(OffsetStr);
- SubSpanOffset = OffsetStr.c_str();
- SubSpanOffset = OffsetStr.c_str();
- } else {
- SubSpanOffset = Offset->getDecl()->getName().str();
- }
+ std::string SubSpanOffset;
+ const SourceManager &SM = Ctx.getSourceManager();
+ const LangOptions &LangOpts = Ctx.getLangOpts();
+ std::optional<StringRef> ExtentString = getExprText(Offset, SM, LangOpts);;
+
+ if (ExtentString)
+ SubSpanOffset = ExtentString->str();
+ else
+ SubSpanOffset = getUserFillPlaceHolder(); // FIXME: When does this happen?
// To transform UUC(p += n) to UUC(p = p.subspan(..)):
SS << varName.data() << " = " << varName.data() << ".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 30c587b2110d19b..5aad4b9558e5fa4 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, int *a) {
+void add_assign_test(unsigned int n, int *a, int y) {
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}:"{"
@@ -37,4 +37,19 @@ void add_assign_test(int n, int *a) {
a += -9;
// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(-9)"
+
+ a += y;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(y)"
+}
+
+int expr_test(unsigned x, int *q, int y) {
+ char *p = new char[8];
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<char> p"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 8}"
+ p += (x + 1);
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:15}:"p = p.subspan(x + 1)"
+
+ q += (y + 7);
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:15}:"q = q.subspan(y + 1)"
}
>From 432a8d32f72b870f6ca1936adc12f57b0170b34c Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_mudduluru at apple.com>
Date: Tue, 14 Nov 2023 16:07:17 -0800
Subject: [PATCH 08/14] [-Wunsafe-buffer-usage] Fixable gadget for AddAssign:
Fix formatting
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 311e47e730ee280..a041ec6cd35fbdb 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1054,8 +1054,9 @@ class UUCAddAssignGadget : public FixableGadget {
static Matcher matcher() {
return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts(
binaryOperator(
- hasOperatorName("+="), hasLHS(declRefExpr(toSupportedVariable())),
- hasRHS(ignoringParens(expr().bind(OffsetTag))))
+ hasOperatorName("+="),
+ hasLHS(declRefExpr(toSupportedVariable())),
+ hasRHS(ignoringParens(expr().bind(OffsetTag))))
.bind(UUCAddAssignTag)))));
}
@@ -1835,12 +1836,13 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
std::string SubSpanOffset;
const SourceManager &SM = Ctx.getSourceManager();
const LangOptions &LangOpts = Ctx.getLangOpts();
- std::optional<StringRef> ExtentString = getExprText(Offset, SM, LangOpts);;
+ std::optional<StringRef> ExtentString = getExprText(Offset, SM, LangOpts);
if (ExtentString)
SubSpanOffset = ExtentString->str();
else
- SubSpanOffset = getUserFillPlaceHolder(); // FIXME: When does this happen?
+ SubSpanOffset =
+ getUserFillPlaceHolder(); // FIXME: When does this happen?
// To transform UUC(p += n) to UUC(p = p.subspan(..)):
SS << varName.data() << " = " << varName.data() << ".subspan("
>From 6217e3d108affdf3594283439cacd331b651a86c Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_mudduluru at apple.com>
Date: Wed, 15 Nov 2023 09:26:07 -0800
Subject: [PATCH 09/14] [-Wunsafe-buffer-usage] Fixable gadget for AddAssign:
Fix formatting
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a041ec6cd35fbdb..16f6a999cf75ab6 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1043,7 +1043,7 @@ class UUCAddAssignGadget : public FixableGadget {
UUCAddAssignGadget(const MatchFinder::MatchResult &Result)
: FixableGadget(Kind::UUCAddAssign),
Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)),
- Offset(Result.Nodes.getNodeAs<Expr>(OffsetTag)){
+ Offset(Result.Nodes.getNodeAs<Expr>(OffsetTag)) {
assert(Node != nullptr && "Expecting a non-null matching result");
}
@@ -1053,8 +1053,7 @@ class UUCAddAssignGadget : public FixableGadget {
static Matcher matcher() {
return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts(
- binaryOperator(
- hasOperatorName("+="),
+ binaryOperator(hasOperatorName("+="),
hasLHS(declRefExpr(toSupportedVariable())),
hasRHS(ignoringParens(expr().bind(OffsetTag))))
.bind(UUCAddAssignTag)))));
@@ -1841,7 +1840,7 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
if (ExtentString)
SubSpanOffset = ExtentString->str();
else
- SubSpanOffset =
+ SubSpanOffset =
getUserFillPlaceHolder(); // FIXME: When does this happen?
// To transform UUC(p += n) to UUC(p = p.subspan(..)):
>From 7236e8e3324cb733732bb4cd36a9bd85bf15ccaa Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_mudduluru at apple.com>
Date: Wed, 15 Nov 2023 16:18:45 -0800
Subject: [PATCH 10/14] [-Wunsafe-buffer-usage][WIP] Fixable gadget for
AddAssign: Address partial comments
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 16 +++++++++------
...-unsafe-buffer-usage-fixits-add-assign.cpp | 20 +++++++++++--------
2 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 16f6a999cf75ab6..a2224d7f05102f2 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1055,7 +1055,7 @@ class UUCAddAssignGadget : public FixableGadget {
return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts(
binaryOperator(hasOperatorName("+="),
hasLHS(declRefExpr(toSupportedVariable())),
- hasRHS(ignoringParens(expr().bind(OffsetTag))))
+ hasRHS(expr().bind(OffsetTag)))
.bind(UUCAddAssignTag)))));
}
@@ -1453,10 +1453,8 @@ static std::optional<SourceLocation> getPastLoc(const NodeTy *Node,
const LangOptions &LangOpts) {
SourceLocation Loc =
Lexer::getLocForEndOfToken(Node->getEndLoc(), 0, SM, LangOpts);
-
if (Loc.isValid())
return Loc;
-
return std::nullopt;
}
@@ -1844,17 +1842,23 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
getUserFillPlaceHolder(); // FIXME: When does this happen?
// To transform UUC(p += n) to UUC(p = p.subspan(..)):
- SS << varName.data() << " = " << varName.data() << ".subspan("
- << SubSpanOffset << ")";
+ SS << varName.data() << " = " << varName.data() << ".subspan";
+ bool NotParenExpr = (Offset->IgnoreParens()->getBeginLoc() == Offset->getBeginLoc());
+ if (NotParenExpr)
+ SS << "(";
+
std::optional<SourceLocation> AddAssignLocation = getEndCharLoc(
AddAssignNode, Ctx.getSourceManager(), Ctx.getLangOpts());
if (!AddAssignLocation)
return std::nullopt;
Fixes.push_back(FixItHint::CreateReplacement(
- SourceRange(AddAssignNode->getBeginLoc(), *AddAssignLocation),
+ SourceRange(AddAssignNode->getBeginLoc(),
+ Node->getOperatorLoc()),
SS.str()));
+ if (NotParenExpr)
+ Fixes.push_back(FixItHint::CreateInsertion(Offset->getEndLoc().getLocWithOffset(1), ")"));
return Fixes;
}
}
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 5aad4b9558e5fa4..5c03cc10025c684 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
@@ -9,7 +9,8 @@ void add_assign_test(unsigned int n, int *a, int y) {
// 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)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:7}:"p = p.subspan("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:")"
int *r = p;
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
@@ -19,27 +20,30 @@ void add_assign_test(unsigned int n, int *a, int y) {
// 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)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:9}:"r = r.subspan("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:11}:")"
}
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)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:9}:"p = p.subspan("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:11}:")"
}
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)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:9}:"p = p.subspan("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:11}:")"
a += -9;
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(-9)"
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:9}:"p = p.subspan("
a += y;
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(y)"
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:9}:"p = p.subspan("
}
int expr_test(unsigned x, int *q, int y) {
@@ -48,8 +52,8 @@ int expr_test(unsigned x, int *q, int y) {
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 8}"
p += (x + 1);
- // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:15}:"p = p.subspan(x + 1)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:7}:"p = p.subspan"
q += (y + 7);
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:15}:"q = q.subspan(y + 1)"
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:7}:"q = q.subspan"
}
>From 0cbf6f34238cb839de9d64e35f9b2ca4db26f74f Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_mudduluru at apple.com>
Date: Wed, 15 Nov 2023 16:57:51 -0800
Subject: [PATCH 11/14] [-Wunsafe-buffer-usage][WIP] Fixable gadget for
AddAssign: Clean-up
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a2224d7f05102f2..c51e3b9c0414495 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1830,21 +1830,11 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
if (!isNonNegativeIntegerExpr(Offset, VD, Ctx))
return std::nullopt;
- std::string SubSpanOffset;
- const SourceManager &SM = Ctx.getSourceManager();
- const LangOptions &LangOpts = Ctx.getLangOpts();
- std::optional<StringRef> ExtentString = getExprText(Offset, SM, LangOpts);
-
- if (ExtentString)
- SubSpanOffset = ExtentString->str();
- else
- SubSpanOffset =
- getUserFillPlaceHolder(); // FIXME: When does this happen?
-
// To transform UUC(p += n) to UUC(p = p.subspan(..)):
SS << varName.data() << " = " << varName.data() << ".subspan";
- bool NotParenExpr = (Offset->IgnoreParens()->getBeginLoc() == Offset->getBeginLoc());
+ bool NotParenExpr =
+ (Offset->IgnoreParens()->getBeginLoc() == Offset->getBeginLoc());
if (NotParenExpr)
SS << "(";
@@ -1854,11 +1844,11 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
return std::nullopt;
Fixes.push_back(FixItHint::CreateReplacement(
- SourceRange(AddAssignNode->getBeginLoc(),
- Node->getOperatorLoc()),
+ SourceRange(AddAssignNode->getBeginLoc(), Node->getOperatorLoc()),
SS.str()));
if (NotParenExpr)
- Fixes.push_back(FixItHint::CreateInsertion(Offset->getEndLoc().getLocWithOffset(1), ")"));
+ Fixes.push_back(FixItHint::CreateInsertion(
+ Offset->getEndLoc().getLocWithOffset(1), ")"));
return Fixes;
}
}
>From b21e329385eb676957e426f113aa75e140078e9e Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_mudduluru at apple.com>
Date: Thu, 16 Nov 2023 09:57:45 -0800
Subject: [PATCH 12/14] [-Wunsafe-buffer-usage][WIP] Fixable gadget for
AddAssign: Clean-up
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index c51e3b9c0414495..d945a0b9cebdaf8 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1837,7 +1837,7 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
(Offset->IgnoreParens()->getBeginLoc() == Offset->getBeginLoc());
if (NotParenExpr)
SS << "(";
-
+
std::optional<SourceLocation> AddAssignLocation = getEndCharLoc(
AddAssignNode, Ctx.getSourceManager(), Ctx.getLangOpts());
if (!AddAssignLocation)
>From efc57907868ca44700bd9fde3ea81a5632e2b0a1 Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_mudduluru at apple.com>
Date: Thu, 16 Nov 2023 13:51:55 -0800
Subject: [PATCH 13/14] [-Wunsafe-buffer-usage][WIP] Fixable gadget for
AddAssign: Clean-up
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index d945a0b9cebdaf8..6349e21875f9935 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1369,7 +1369,7 @@ ULCArraySubscriptGadget::getFixits(const Strategy &S) const {
if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
switch (S.lookup(VD)) {
case Strategy::Kind::Span: {
-
+
// If the index has a negative constant value, we give up as no valid
// fix-it can be generated:
const ASTContext &Ctx = // FIXME: we need ASTContext to be passed in!
>From db8cbde5bb828f896b6551779d537d37b3ba529e Mon Sep 17 00:00:00 2001
From: Rashmi Mudduluru <r_mudduluru at apple.com>
Date: Fri, 17 Nov 2023 10:12:27 -0800
Subject: [PATCH 14/14] [-Wunsafe-buffer-usage][WIP] Fixable gadget for
AddAssign: Address comments
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 6349e21875f9935..a1efb76be68b7d7 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1822,7 +1822,7 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
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();
@@ -1831,12 +1831,11 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
return std::nullopt;
// To transform UUC(p += n) to UUC(p = p.subspan(..)):
- SS << varName.data() << " = " << varName.data() << ".subspan";
-
bool NotParenExpr =
(Offset->IgnoreParens()->getBeginLoc() == Offset->getBeginLoc());
+ std::string SS = varName.str() + " = " + varName.str() + ".subspan";
if (NotParenExpr)
- SS << "(";
+ SS += "(";
std::optional<SourceLocation> AddAssignLocation = getEndCharLoc(
AddAssignNode, Ctx.getSourceManager(), Ctx.getLangOpts());
@@ -1845,7 +1844,7 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
Fixes.push_back(FixItHint::CreateReplacement(
SourceRange(AddAssignNode->getBeginLoc(), Node->getOperatorLoc()),
- SS.str()));
+ SS));
if (NotParenExpr)
Fixes.push_back(FixItHint::CreateInsertion(
Offset->getEndLoc().getLocWithOffset(1), ")"));
More information about the cfe-commits
mailing list