[clang] e1655a9 - [-Wunsafe-buffer-usage] Add FixableGadget for AddAssign in UnspecifiedUntypedContext (#71862)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 11 15:00:12 PST 2023
Author: Rashmi Mudduluru
Date: 2023-12-11T15:00:08-08:00
New Revision: e1655a98cb9c098fa941ed199664927ba8a4b031
URL: https://github.com/llvm/llvm-project/commit/e1655a98cb9c098fa941ed199664927ba8a4b031
DIFF: https://github.com/llvm/llvm-project/commit/e1655a98cb9c098fa941ed199664927ba8a4b031.diff
LOG: [-Wunsafe-buffer-usage] Add FixableGadget for AddAssign in UnspecifiedUntypedContext (#71862)
Added:
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp
Modified:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/lib/Analysis/UnsafeBufferUsage.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index ff687a0d178bd..757ee452ced74 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 e332a3609290a..a1efb76be68b7 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1028,6 +1028,46 @@ 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 OffsetTag = "Offset";
+
+ const BinaryOperator *Node; // the `Ptr += n` node
+ const Expr *Offset = nullptr;
+
+public:
+ UUCAddAssignGadget(const MatchFinder::MatchResult &Result)
+ : FixableGadget(Kind::UUCAddAssign),
+ Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)),
+ Offset(Result.Nodes.getNodeAs<Expr>(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().bind(OffsetTag)))
+ .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 {
@@ -1312,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 =
@@ -1319,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{};
@@ -1405,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;
}
@@ -1766,6 +1812,47 @@ 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;
+
+ const Stmt *AddAssignNode = getBaseStmt();
+ StringRef varName = VD->getName();
+ const ASTContext &Ctx = VD->getASTContext();
+
+ if (!isNonNegativeIntegerExpr(Offset, VD, Ctx))
+ return std::nullopt;
+
+ // To transform UUC(p += n) to UUC(p = p.subspan(..)):
+ bool NotParenExpr =
+ (Offset->IgnoreParens()->getBeginLoc() == Offset->getBeginLoc());
+ std::string SS = varName.str() + " = " + varName.str() + ".subspan";
+ 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(), Node->getOperatorLoc()),
+ SS));
+ if (NotParenExpr)
+ Fixes.push_back(FixItHint::CreateInsertion(
+ Offset->getEndLoc().getLocWithOffset(1), ")"));
+ 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 0000000000000..5c03cc10025c6
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp
@@ -0,0 +1,59 @@
+// 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(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}:"{"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+ p += 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"
+ // 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]]: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]]: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]]:9}:"p = p.subspan("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:11}:")"
+
+ a += -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]]:9}:"p = p.subspan("
+}
+
+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]]:7}:"p = p.subspan"
+
+ q += (y + 7);
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:7}:"q = q.subspan"
+}
More information about the cfe-commits
mailing list