[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
https://github.com/jkorous-apple closed https://github.com/llvm/llvm-project/pull/81343 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
https://github.com/haoNoQ approved this pull request. LGTM let's land! https://github.com/llvm/llvm-project/pull/81343 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
https://github.com/jkorous-apple updated
https://github.com/llvm/llvm-project/pull/81343
>From 791130c5c5de31084c168db33531a5d856104506 Mon Sep 17 00:00:00 2001
From: Jan Korous
Date: Thu, 8 Feb 2024 14:30:20 -0800
Subject: [PATCH 1/5] [-Wunsafe-buffer-usage][NFC] Factor out .data() fixit to
a function
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a74c113e29f1cf..cc49876779ece2 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1490,6 +1490,9 @@ PointerAssignmentGadget::getFixits(const FixitStrategy
) const {
return std::nullopt;
}
+/// \returns fixit that adds .data() call after \DRE.
+static inline std::optional createDataFixit(const ASTContext& Ctx,
const DeclRefExpr * DRE);
+
std::optional
PointerInitGadget::getFixits(const FixitStrategy ) const {
const auto *LeftVD = PtrInitLHS;
@@ -1907,6 +1910,18 @@ PointerDereferenceGadget::getFixits(const FixitStrategy
) const {
return std::nullopt;
}
+static inline std::optional createDataFixit(const ASTContext& Ctx,
const DeclRefExpr * DRE) {
+ const SourceManager = Ctx.getSourceManager();
+ // Inserts the .data() after the DRE
+ std::optional EndOfOperand =
+ getPastLoc(DRE, SM, Ctx.getLangOpts());
+
+ if (EndOfOperand)
+return FixItList{{FixItHint::CreateInsertion(*EndOfOperand, ".data()")}};
+
+ return std::nullopt;
+}
+
// Generates fix-its replacing an expression of the form UPC(DRE) with
// `DRE.data()`
std::optional
@@ -1915,14 +1930,7 @@ UPCStandalonePointerGadget::getFixits(const
FixitStrategy ) const {
switch (S.lookup(VD)) {
case FixitStrategy::Kind::Array:
case FixitStrategy::Kind::Span: {
-ASTContext = VD->getASTContext();
-SourceManager = Ctx.getSourceManager();
-// Inserts the .data() after the DRE
-std::optional EndOfOperand =
-getPastLoc(Node, SM, Ctx.getLangOpts());
-
-if (EndOfOperand)
- return FixItList{{FixItHint::CreateInsertion(*EndOfOperand, ".data()")}};
+return createDataFixit(VD->getASTContext(), Node);
// FIXME: Points inside a macro expansion.
break;
}
>From 1b12f7413288b01d1806b98fb90c2d44e53b7437 Mon Sep 17 00:00:00 2001
From: Jan Korous
Date: Thu, 8 Feb 2024 14:33:03 -0800
Subject: [PATCH 2/5] [-Wunsafe-buffer-usage][NFC] Rename PointerAssignment
gadget to PtrToPtrAssignment
---
.../Analysis/Analyses/UnsafeBufferUsageGadgets.def| 2 +-
clang/lib/Analysis/UnsafeBufferUsage.cpp | 11 ++-
clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp | 2 +-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index 07f805ebb11013..2babc1df93d515 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -45,7 +45,7 @@ FIXABLE_GADGET(UPCAddressofArraySubscript) // '[any]' in
an Unspecified Poin
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(PtrToPtrAssignment)
FIXABLE_GADGET(PointerInit)
#undef FIXABLE_GADGET
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index cc49876779ece2..927baef2fffa39 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -799,7 +799,8 @@ class PointerInitGadget : public FixableGadget {
/// \code
/// p = q;
/// \endcode
-class PointerAssignmentGadget : public FixableGadget {
+/// where both `p` and `q` are pointers.
+class PtrToPtrAssignmentGadget : public FixableGadget {
private:
static constexpr const char *const PointerAssignLHSTag = "ptrLHS";
static constexpr const char *const PointerAssignRHSTag = "ptrRHS";
@@ -807,13 +808,13 @@ class PointerAssignmentGadget : public FixableGadget {
const DeclRefExpr * PtrRHS; // the RHS pointer expression in `PA`
public:
- PointerAssignmentGadget(const MatchFinder::MatchResult )
- : FixableGadget(Kind::PointerAssignment),
+ PtrToPtrAssignmentGadget(const MatchFinder::MatchResult )
+ : FixableGadget(Kind::PtrToPtrAssignment),
PtrLHS(Result.Nodes.getNodeAs(PointerAssignLHSTag)),
PtrRHS(Result.Nodes.getNodeAs(PointerAssignRHSTag)) {}
static bool classof(const Gadget *G) {
-return G->getKind() == Kind::PointerAssignment;
+return G->getKind() == Kind::PtrToPtrAssignment;
}
static Matcher matcher() {
@@ -1471,7 +1472,7 @@ bool clang::internal::anyConflict(const
SmallVectorImpl ,
}
[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
https://github.com/haoNoQ commented: LGTM! I have a couple minor nitpicks. https://github.com/llvm/llvm-project/pull/81343 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:-fsafe-buffer-usage-suggestions \
+// RUN:-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+void safe_array_assigned_to_safe_ptr(unsigned idx) {
+ int buffer[10];
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+ int* ptr;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+ ptr = buffer;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+}
+
+void safe_array_assigned_to_unsafe_ptr(unsigned idx) {
+ int buffer[10];
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+ int* ptr;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span ptr"
+ ptr = buffer;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+ ptr[idx] = 0;
+}
+
+void unsafe_array_assigned_to_safe_ptr(unsigned idx) {
+ int buffer[10];
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:17}:"std::array buffer"
+ int* ptr;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+ ptr = buffer;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:15-[[@LINE-1]]:15}:".data()"
+ buffer[idx] = 0;
+}
+
+void unsafe_array_assigned_to_unsafe_ptr(unsigned idx) {
haoNoQ wrote:
Similarly, let's mark this test as a FIXME test, and add a comment explaining
why this is hard and the naive fix would be incorrect.
https://github.com/llvm/llvm-project/pull/81343
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
@@ -1490,6 +1548,26 @@ PointerAssignmentGadget::getFixits(const FixitStrategy
) const {
return std::nullopt;
}
+/// \returns fixit that adds .data() call after \DRE.
+static inline std::optional createDataFixit(const ASTContext ,
+ const DeclRefExpr *DRE);
+
+std::optional
+CArrayToPtrAssignmentGadget::getFixits(const FixitStrategy ) const {
haoNoQ wrote:
I think there should be a comment explaining why the "both sides are fixed"
case is tricky. Otherwise somebody may accidentally implement it
https://github.com/llvm/llvm-project/pull/81343
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
@@ -848,6 +852,60 @@ class PointerAssignmentGadget : public FixableGadget {
}
};
+/// An assignment expression of the form:
+/// \code
+/// ptr = array;
+/// \endcode
+/// where `p` is a pointer and `array` is a constant size array.
+class CArrayToPtrAssignmentGadget : public FixableGadget {
+private:
+ static constexpr const char *const PointerAssignLHSTag = "ptrLHS";
+ static constexpr const char *const PointerAssignRHSTag = "ptrRHS";
+ const DeclRefExpr *PtrLHS; // the LHS pointer expression in `PA`
+ const DeclRefExpr *PtrRHS; // the RHS pointer expression in `PA`
+
+public:
+ CArrayToPtrAssignmentGadget(const MatchFinder::MatchResult )
+ : FixableGadget(Kind::CArrayToPtrAssignment),
+PtrLHS(Result.Nodes.getNodeAs(PointerAssignLHSTag)),
+PtrRHS(Result.Nodes.getNodeAs(PointerAssignRHSTag)) {}
+
+ static bool classof(const Gadget *G) {
+return G->getKind() == Kind::CArrayToPtrAssignment;
+ }
+
+ static Matcher matcher() {
+auto PtrAssignExpr = binaryOperator(
+allOf(hasOperatorName("="),
+ hasRHS(ignoringParenImpCasts(
+ declRefExpr(hasType(hasCanonicalType(constantArrayType())),
+ toSupportedVariable())
+ .bind(PointerAssignRHSTag))),
+ hasLHS(declRefExpr(hasPointerType(), toSupportedVariable())
+ .bind(PointerAssignLHSTag;
+
+return stmt(isInUnspecifiedUntypedContext(PtrAssignExpr));
+ }
+
+ virtual std::optional
+ getFixits(const FixitStrategy ) const override;
+
+ virtual const Stmt *getBaseStmt() const override {
+// FIXME: This should be the binary operator, assuming that this method
+// makes sense at all on a FixableGadget.
+return PtrLHS;
+ }
+
+ virtual DeclUseList getClaimedVarUseSites() const override {
+return DeclUseList{PtrLHS, PtrRHS};
+ }
+
+ virtual std::optional>
+ getStrategyImplications() const override {
+return {};
+ }
haoNoQ wrote:
This is probably the default behavior 樂
https://github.com/llvm/llvm-project/pull/81343
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: None (jkorous-apple)
Changes
depends on
https://github.com/llvm/llvm-project/pull/80347
---
Full diff: https://github.com/llvm/llvm-project/pull/81343.diff
4 Files Affected:
- (modified) clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
(+2-1)
- (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+99-15)
- (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp (+1-1)
- (added)
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-array-assign-to-ptr.cpp
(+43)
``diff
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index 07f805ebb11013..3273c642eed517 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -45,7 +45,8 @@ FIXABLE_GADGET(UPCAddressofArraySubscript) // '[any]' in
an Unspecified Poin
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(PtrToPtrAssignment)
+FIXABLE_GADGET(CArrayToPtrAssignment)
FIXABLE_GADGET(PointerInit)
#undef FIXABLE_GADGET
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a74c113e29f1cf..8e810876950c81 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -7,11 +7,14 @@
//===--===//
#include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
+#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Lex/Lexer.h"
@@ -799,7 +802,8 @@ class PointerInitGadget : public FixableGadget {
/// \code
/// p = q;
/// \endcode
-class PointerAssignmentGadget : public FixableGadget {
+/// where both `p` and `q` are pointers.
+class PtrToPtrAssignmentGadget : public FixableGadget {
private:
static constexpr const char *const PointerAssignLHSTag = "ptrLHS";
static constexpr const char *const PointerAssignRHSTag = "ptrRHS";
@@ -807,13 +811,13 @@ class PointerAssignmentGadget : public FixableGadget {
const DeclRefExpr * PtrRHS; // the RHS pointer expression in `PA`
public:
- PointerAssignmentGadget(const MatchFinder::MatchResult )
- : FixableGadget(Kind::PointerAssignment),
-PtrLHS(Result.Nodes.getNodeAs(PointerAssignLHSTag)),
-PtrRHS(Result.Nodes.getNodeAs(PointerAssignRHSTag)) {}
+ PtrToPtrAssignmentGadget(const MatchFinder::MatchResult )
+ : FixableGadget(Kind::PtrToPtrAssignment),
+PtrLHS(Result.Nodes.getNodeAs(PointerAssignLHSTag)),
+PtrRHS(Result.Nodes.getNodeAs(PointerAssignRHSTag)) {}
static bool classof(const Gadget *G) {
-return G->getKind() == Kind::PointerAssignment;
+return G->getKind() == Kind::PtrToPtrAssignment;
}
static Matcher matcher() {
@@ -848,6 +852,60 @@ class PointerAssignmentGadget : public FixableGadget {
}
};
+/// An assignment expression of the form:
+/// \code
+/// ptr = array;
+/// \endcode
+/// where `p` is a pointer and `array` is a constant size array.
+class CArrayToPtrAssignmentGadget : public FixableGadget {
+private:
+ static constexpr const char *const PointerAssignLHSTag = "ptrLHS";
+ static constexpr const char *const PointerAssignRHSTag = "ptrRHS";
+ const DeclRefExpr *PtrLHS; // the LHS pointer expression in `PA`
+ const DeclRefExpr *PtrRHS; // the RHS pointer expression in `PA`
+
+public:
+ CArrayToPtrAssignmentGadget(const MatchFinder::MatchResult )
+ : FixableGadget(Kind::CArrayToPtrAssignment),
+PtrLHS(Result.Nodes.getNodeAs(PointerAssignLHSTag)),
+PtrRHS(Result.Nodes.getNodeAs(PointerAssignRHSTag)) {}
+
+ static bool classof(const Gadget *G) {
+return G->getKind() == Kind::CArrayToPtrAssignment;
+ }
+
+ static Matcher matcher() {
+auto PtrAssignExpr = binaryOperator(
+allOf(hasOperatorName("="),
+ hasRHS(ignoringParenImpCasts(
+ declRefExpr(hasType(hasCanonicalType(constantArrayType())),
+ toSupportedVariable())
+ .bind(PointerAssignRHSTag))),
+ hasLHS(declRefExpr(hasPointerType(), toSupportedVariable())
+ .bind(PointerAssignLHSTag;
+
+return stmt(isInUnspecifiedUntypedContext(PtrAssignExpr));
+ }
+
+ virtual std::optional
+ getFixits(const FixitStrategy ) const override;
+
+
[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
https://github.com/jkorous-apple updated
https://github.com/llvm/llvm-project/pull/81343
>From 791130c5c5de31084c168db33531a5d856104506 Mon Sep 17 00:00:00 2001
From: Jan Korous
Date: Thu, 8 Feb 2024 14:30:20 -0800
Subject: [PATCH 1/4] [-Wunsafe-buffer-usage][NFC] Factor out .data() fixit to
a function
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a74c113e29f1cf..cc49876779ece2 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1490,6 +1490,9 @@ PointerAssignmentGadget::getFixits(const FixitStrategy
) const {
return std::nullopt;
}
+/// \returns fixit that adds .data() call after \DRE.
+static inline std::optional createDataFixit(const ASTContext& Ctx,
const DeclRefExpr * DRE);
+
std::optional
PointerInitGadget::getFixits(const FixitStrategy ) const {
const auto *LeftVD = PtrInitLHS;
@@ -1907,6 +1910,18 @@ PointerDereferenceGadget::getFixits(const FixitStrategy
) const {
return std::nullopt;
}
+static inline std::optional createDataFixit(const ASTContext& Ctx,
const DeclRefExpr * DRE) {
+ const SourceManager = Ctx.getSourceManager();
+ // Inserts the .data() after the DRE
+ std::optional EndOfOperand =
+ getPastLoc(DRE, SM, Ctx.getLangOpts());
+
+ if (EndOfOperand)
+return FixItList{{FixItHint::CreateInsertion(*EndOfOperand, ".data()")}};
+
+ return std::nullopt;
+}
+
// Generates fix-its replacing an expression of the form UPC(DRE) with
// `DRE.data()`
std::optional
@@ -1915,14 +1930,7 @@ UPCStandalonePointerGadget::getFixits(const
FixitStrategy ) const {
switch (S.lookup(VD)) {
case FixitStrategy::Kind::Array:
case FixitStrategy::Kind::Span: {
-ASTContext = VD->getASTContext();
-SourceManager = Ctx.getSourceManager();
-// Inserts the .data() after the DRE
-std::optional EndOfOperand =
-getPastLoc(Node, SM, Ctx.getLangOpts());
-
-if (EndOfOperand)
- return FixItList{{FixItHint::CreateInsertion(*EndOfOperand, ".data()")}};
+return createDataFixit(VD->getASTContext(), Node);
// FIXME: Points inside a macro expansion.
break;
}
>From 1b12f7413288b01d1806b98fb90c2d44e53b7437 Mon Sep 17 00:00:00 2001
From: Jan Korous
Date: Thu, 8 Feb 2024 14:33:03 -0800
Subject: [PATCH 2/4] [-Wunsafe-buffer-usage][NFC] Rename PointerAssignment
gadget to PtrToPtrAssignment
---
.../Analysis/Analyses/UnsafeBufferUsageGadgets.def| 2 +-
clang/lib/Analysis/UnsafeBufferUsage.cpp | 11 ++-
clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp | 2 +-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index 07f805ebb11013..2babc1df93d515 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -45,7 +45,7 @@ FIXABLE_GADGET(UPCAddressofArraySubscript) // '[any]' in
an Unspecified Poin
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(PtrToPtrAssignment)
FIXABLE_GADGET(PointerInit)
#undef FIXABLE_GADGET
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index cc49876779ece2..927baef2fffa39 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -799,7 +799,8 @@ class PointerInitGadget : public FixableGadget {
/// \code
/// p = q;
/// \endcode
-class PointerAssignmentGadget : public FixableGadget {
+/// where both `p` and `q` are pointers.
+class PtrToPtrAssignmentGadget : public FixableGadget {
private:
static constexpr const char *const PointerAssignLHSTag = "ptrLHS";
static constexpr const char *const PointerAssignRHSTag = "ptrRHS";
@@ -807,13 +808,13 @@ class PointerAssignmentGadget : public FixableGadget {
const DeclRefExpr * PtrRHS; // the RHS pointer expression in `PA`
public:
- PointerAssignmentGadget(const MatchFinder::MatchResult )
- : FixableGadget(Kind::PointerAssignment),
+ PtrToPtrAssignmentGadget(const MatchFinder::MatchResult )
+ : FixableGadget(Kind::PtrToPtrAssignment),
PtrLHS(Result.Nodes.getNodeAs(PointerAssignLHSTag)),
PtrRHS(Result.Nodes.getNodeAs(PointerAssignRHSTag)) {}
static bool classof(const Gadget *G) {
-return G->getKind() == Kind::PointerAssignment;
+return G->getKind() == Kind::PtrToPtrAssignment;
}
static Matcher matcher() {
@@ -1471,7 +1472,7 @@ bool clang::internal::anyConflict(const
SmallVectorImpl ,
}
[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
https://github.com/jkorous-apple updated
https://github.com/llvm/llvm-project/pull/81343
>From dfc9d95c185f5228f5c9680a19aa396d20e33d19 Mon Sep 17 00:00:00 2001
From: Jan Korous
Date: Thu, 25 Jan 2024 13:52:12 -0800
Subject: [PATCH 01/11] [-Wunsafe-buffer-usage] Emit fixits for arguments of
function pointers calls
Currently we ignore calls on function pointers (unlike direct calls of
functions and class methods). This patch adds support for function pointers as
well.
The change is to simply replace use of forEachArgumentWithParam matcher in UPC
gadget with forEachArgumentWithParamType.
from the documentation of forEachArgumentWithParamType:
/// Matches all arguments and their respective types for a \c CallExpr or
/// \c CXXConstructExpr. It is very similar to \c forEachArgumentWithParam but
/// it works on calls through function pointers as well.
Currently the matcher also uses hasPointerType() which checks that the
canonical type of an argument is pointer and won't match on arrays decayed to
pointer. Replacing hasPointerType() with isAnyPointerType() which allows
implicit casts allows for the arrays to be matched as well and this way we get
fixits for array arguments to function pointer calls too.
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 4 ++--
...fer-usage-fixits-pointer-arg-to-func-ptr-call.cpp | 12
2 files changed, 14 insertions(+), 2 deletions(-)
create mode 100644
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index d00c598c4b9de3..c5a87f14bc8880 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -282,8 +282,8 @@ isInUnspecifiedPointerContext(internal::Matcher
InnerMatcher) {
//(i.e., computing the distance between two pointers); or ...
auto CallArgMatcher =
- callExpr(forEachArgumentWithParam(InnerMatcher,
- hasPointerType() /* array also decays to pointer type*/),
+ callExpr(forEachArgumentWithParamType(InnerMatcher,
+ isAnyPointer() /* array also decays to pointer type*/),
unless(callee(functionDecl(hasAttr(attr::UnsafeBufferUsage);
auto CastOperandMatcher =
diff --git
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
new file mode 100644
index 00..ae761e46a98191
--- /dev/null
+++
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:-fsafe-buffer-usage-suggestions \
+// RUN:-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+void unsafe_array_func_ptr_call(void (*fn_ptr)(int *param)) {
+ int p[32];
+ // CHECK-DAG:
fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::array p"
+
+ int tmp = p[5];
+ fn_ptr(p);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:11}:".data()"
+}
>From 44dab965c459f2cd6084ea332f4a6756f57254e0 Mon Sep 17 00:00:00 2001
From: Jan Korous
Date: Thu, 1 Feb 2024 14:44:01 -0800
Subject: [PATCH 02/11] [-Wunsafe-buffer-usage][NFC] Add tests for function
pointer call fixits
---
...ge-fixits-pointer-arg-to-func-ptr-call.cpp | 38 ++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
index ae761e46a98191..0459d6549fd86f 100644
---
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
+++
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
@@ -6,7 +6,43 @@ void unsafe_array_func_ptr_call(void (*fn_ptr)(int *param)) {
int p[32];
// CHECK-DAG:
fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::array p"
- int tmp = p[5];
+ p[5] = 10;
fn_ptr(p);
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:11}:".data()"
}
+
+void unsafe_ptr_func_ptr_call(void (*fn_ptr)(int *param)) {
+ int *p;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"std::span
p"
+
+ p[5] = 10;
+ fn_ptr(p);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:11}:".data()"
+}
+
+void addr_of_unsafe_ptr_func_ptr_call(void (*fn_ptr)(int *param)) {
+ int *p;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"std::span
p"
+
+ p[5] = 10;
+ fn_ptr([0]);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:15}:"p.data()"
+}
+
+void addr_of_unsafe_ptr_w_offset_func_ptr_call(void (*fn_ptr)(int *param)) {
+ int *p;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"std::span
p"
+
+ p[5] = 10;
+ fn_ptr([3]);
+ // CHECK-DAG:
[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
https://github.com/jkorous-apple updated
https://github.com/llvm/llvm-project/pull/81343
>From dfc9d95c185f5228f5c9680a19aa396d20e33d19 Mon Sep 17 00:00:00 2001
From: Jan Korous
Date: Thu, 25 Jan 2024 13:52:12 -0800
Subject: [PATCH 01/10] [-Wunsafe-buffer-usage] Emit fixits for arguments of
function pointers calls
Currently we ignore calls on function pointers (unlike direct calls of
functions and class methods). This patch adds support for function pointers as
well.
The change is to simply replace use of forEachArgumentWithParam matcher in UPC
gadget with forEachArgumentWithParamType.
from the documentation of forEachArgumentWithParamType:
/// Matches all arguments and their respective types for a \c CallExpr or
/// \c CXXConstructExpr. It is very similar to \c forEachArgumentWithParam but
/// it works on calls through function pointers as well.
Currently the matcher also uses hasPointerType() which checks that the
canonical type of an argument is pointer and won't match on arrays decayed to
pointer. Replacing hasPointerType() with isAnyPointerType() which allows
implicit casts allows for the arrays to be matched as well and this way we get
fixits for array arguments to function pointer calls too.
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 4 ++--
...fer-usage-fixits-pointer-arg-to-func-ptr-call.cpp | 12
2 files changed, 14 insertions(+), 2 deletions(-)
create mode 100644
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index d00c598c4b9de3..c5a87f14bc8880 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -282,8 +282,8 @@ isInUnspecifiedPointerContext(internal::Matcher
InnerMatcher) {
//(i.e., computing the distance between two pointers); or ...
auto CallArgMatcher =
- callExpr(forEachArgumentWithParam(InnerMatcher,
- hasPointerType() /* array also decays to pointer type*/),
+ callExpr(forEachArgumentWithParamType(InnerMatcher,
+ isAnyPointer() /* array also decays to pointer type*/),
unless(callee(functionDecl(hasAttr(attr::UnsafeBufferUsage);
auto CastOperandMatcher =
diff --git
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
new file mode 100644
index 00..ae761e46a98191
--- /dev/null
+++
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:-fsafe-buffer-usage-suggestions \
+// RUN:-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+void unsafe_array_func_ptr_call(void (*fn_ptr)(int *param)) {
+ int p[32];
+ // CHECK-DAG:
fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::array p"
+
+ int tmp = p[5];
+ fn_ptr(p);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:11}:".data()"
+}
>From 44dab965c459f2cd6084ea332f4a6756f57254e0 Mon Sep 17 00:00:00 2001
From: Jan Korous
Date: Thu, 1 Feb 2024 14:44:01 -0800
Subject: [PATCH 02/10] [-Wunsafe-buffer-usage][NFC] Add tests for function
pointer call fixits
---
...ge-fixits-pointer-arg-to-func-ptr-call.cpp | 38 ++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
index ae761e46a98191..0459d6549fd86f 100644
---
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
+++
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp
@@ -6,7 +6,43 @@ void unsafe_array_func_ptr_call(void (*fn_ptr)(int *param)) {
int p[32];
// CHECK-DAG:
fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::array p"
- int tmp = p[5];
+ p[5] = 10;
fn_ptr(p);
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:11}:".data()"
}
+
+void unsafe_ptr_func_ptr_call(void (*fn_ptr)(int *param)) {
+ int *p;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"std::span
p"
+
+ p[5] = 10;
+ fn_ptr(p);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:11}:".data()"
+}
+
+void addr_of_unsafe_ptr_func_ptr_call(void (*fn_ptr)(int *param)) {
+ int *p;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"std::span
p"
+
+ p[5] = 10;
+ fn_ptr([0]);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:15}:"p.data()"
+}
+
+void addr_of_unsafe_ptr_w_offset_func_ptr_call(void (*fn_ptr)(int *param)) {
+ int *p;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"std::span
p"
+
+ p[5] = 10;
+ fn_ptr([3]);
+ // CHECK-DAG:
[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
https://github.com/jkorous-apple ready_for_review https://github.com/llvm/llvm-project/pull/81343 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
https://github.com/jkorous-apple converted_to_draft https://github.com/llvm/llvm-project/pull/81343 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Add fixits for array to pointer assignment (PR #81343)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 86cd2fbdfe67d70a7fe061ed5d3a644f50f070f5
47c18be02329b853513d0955b8443e3bd4f04778 --
clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-array-assign-to-ptr.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 7b1ff89628..7b07acdb18 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -776,8 +776,8 @@ private:
public:
PtrToPtrAssignmentGadget(const MatchFinder::MatchResult )
: FixableGadget(Kind::PtrToPtrAssignment),
-PtrLHS(Result.Nodes.getNodeAs(PointerAssignLHSTag)),
-PtrRHS(Result.Nodes.getNodeAs(PointerAssignRHSTag)) {}
+PtrLHS(Result.Nodes.getNodeAs(PointerAssignLHSTag)),
+PtrRHS(Result.Nodes.getNodeAs(PointerAssignRHSTag)) {}
static bool classof(const Gadget *G) {
return G->getKind() == Kind::PtrToPtrAssignment;
@@ -824,27 +824,28 @@ class CArrayToPtrAssignmentGadget : public FixableGadget {
private:
static constexpr const char *const PointerAssignLHSTag = "ptrLHS";
static constexpr const char *const PointerAssignRHSTag = "ptrRHS";
- const DeclRefExpr * PtrLHS; // the LHS pointer expression in `PA`
- const DeclRefExpr * PtrRHS; // the RHS pointer expression in `PA`
+ const DeclRefExpr *PtrLHS; // the LHS pointer expression in `PA`
+ const DeclRefExpr *PtrRHS; // the RHS pointer expression in `PA`
public:
CArrayToPtrAssignmentGadget(const MatchFinder::MatchResult )
: FixableGadget(Kind::CArrayToPtrAssignment),
-PtrLHS(Result.Nodes.getNodeAs(PointerAssignLHSTag)),
-PtrRHS(Result.Nodes.getNodeAs(PointerAssignRHSTag)) {}
+PtrLHS(Result.Nodes.getNodeAs(PointerAssignLHSTag)),
+PtrRHS(Result.Nodes.getNodeAs(PointerAssignRHSTag)) {}
static bool classof(const Gadget *G) {
return G->getKind() == Kind::CArrayToPtrAssignment;
}
static Matcher matcher() {
-auto PtrAssignExpr = binaryOperator(allOf(hasOperatorName("="),
-
hasRHS(ignoringParenImpCasts(declRefExpr(hasType(hasCanonicalType(constantArrayType())),
- toSupportedVariable()).
- bind(PointerAssignRHSTag))),
- hasLHS(declRefExpr(hasPointerType(),
- toSupportedVariable()).
- bind(PointerAssignLHSTag;
+auto PtrAssignExpr = binaryOperator(
+allOf(hasOperatorName("="),
+ hasRHS(ignoringParenImpCasts(
+ declRefExpr(hasType(hasCanonicalType(constantArrayType())),
+ toSupportedVariable())
+ .bind(PointerAssignRHSTag))),
+ hasLHS(declRefExpr(hasPointerType(), toSupportedVariable())
+ .bind(PointerAssignLHSTag;
return stmt(isInUnspecifiedUntypedContext(PtrAssignExpr));
}
@@ -1511,7 +1512,8 @@ PtrToPtrAssignmentGadget::getFixits(const FixitStrategy
) const {
}
/// \returns fixit that adds .data() call after \DRE.
-static inline std::optional createDataFixit(const ASTContext& Ctx,
const DeclRefExpr * DRE);
+static inline std::optional createDataFixit(const ASTContext ,
+ const DeclRefExpr *DRE);
std::optional
CArrayToPtrAssignmentGadget::getFixits(const FixitStrategy ) const {
@@ -1520,27 +1522,30 @@ CArrayToPtrAssignmentGadget::getFixits(const
FixitStrategy ) const {
switch (S.lookup(LeftVD)) {
case FixitStrategy::Kind::Span: {
switch (S.lookup(RightVD)) {
- case FixitStrategy::Kind::Array:
- case FixitStrategy::Kind::Vector:
- case FixitStrategy::Kind::Wontfix:
-return FixItList{};
- default: break;
+case FixitStrategy::Kind::Array:
+case FixitStrategy::Kind::Vector:
+case FixitStrategy::Kind::Wontfix:
+ return FixItList{};
+default:
+ break;
}
break;
}
case FixitStrategy::Kind::Wontfix: {
switch (S.lookup(RightVD)) {
- case FixitStrategy::Kind::Wontfix:
-return FixItList{}; // tautological
- case FixitStrategy::Kind::Array:
- case FixitStrategy::Kind::Span:
-return
