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