Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def =================================================================== --- clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def +++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def @@ -31,6 +31,7 @@ WARNING_GADGET(PointerArithmetic) WARNING_GADGET(UnsafeBufferUsageAttr) FIXABLE_GADGET(ULCArraySubscript) +FIXABLE_GADGET(PointerAssignment) #undef FIXABLE_GADGET #undef WARNING_GADGET Index: clang/lib/Analysis/UnsafeBufferUsage.cpp =================================================================== --- clang/lib/Analysis/UnsafeBufferUsage.cpp +++ clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -422,6 +422,52 @@ } }; +/// A pointer assignment expression of one of the forms: +/// \code +/// p = q; +/// \endcode +class PointerAssignmentGadget : public FixableGadget { +private: + static constexpr const char *const PointerAssignemntTag = "ptrAssign"; + static constexpr const char *const PointerAssignLHSTag = "ptrLHS"; + static constexpr const char *const PointerAssignRHSTag = "ptrRHS"; + const BinaryOperator *PA; // pointer arithmetic expression + const Expr * PtrLHS; // the LHS pointer expression in `PA` + const Expr * PtrRHS; // the RHS pointer expression in `PA` + +public: + PointerAssignmentGadget(const MatchFinder::MatchResult &Result) + : FixableGadget(Kind::PointerAssignment), + PA(Result.Nodes.getNodeAs(PointerAssignemntTag)), + PtrLHS(Result.Nodes.getNodeAs(PointerAssignLHSTag)), + PtrRHS(Result.Nodes.getNodeAs(PointerAssignRHSTag)) { + assert(PA != nullptr && "Expecting a non-null matching result"); + } + + static bool classof(const Gadget *G) { + return G->getKind() == Kind::PointerAssignment; + } + + static Matcher matcher() { + auto PtrAtRight = allOf(hasOperatorName("="), hasRHS(expr(hasPointerType()).bind(PointerAssignRHSTag))); + auto PtrAtLeft = allOf(hasOperatorName("="), hasLHS(expr(hasPointerType()).bind(PointerAssignLHSTag))); + + return stmt(binaryOperator(allOf(PtrAtLeft, PtrAtRight)).bind(PointerAssignemntTag)); + } + + virtual std::optional getFixits(const Strategy &S) const override; + + virtual const Stmt *getBaseStmt() const override { return PA; } + + virtual DeclUseList getClaimedVarUseSites() const override { + if (const auto *LDRE = dyn_cast(PtrLHS->IgnoreParenImpCasts())) { + if (const auto *RDRE = dyn_cast(PtrRHS->IgnoreParenImpCasts())) { + return DeclUseList{LDRE, RDRE}; + } + } + return {}; + } +}; /// A call of a function or method that performs unchecked buffer operations /// over one of its pointer parameters. @@ -645,13 +691,13 @@ } struct WarningGadgetSets { - std::map>> byVar; + std::map> byVar; // These Gadgets are not related to pointer variables (e. g. temporaries). - llvm::SmallVector, 16> noVar; + llvm::SmallVector noVar; }; static WarningGadgetSets -groupWarningGadgetsByVar(WarningGadgetList &&AllUnsafeOperations) { +groupWarningGadgetsByVar(const WarningGadgetList &AllUnsafeOperations) { WarningGadgetSets result; // If some gadgets cover more than one // variable, they'll appear more than once in the map. @@ -661,13 +707,13 @@ bool AssociatedWithVarDecl = false; for (const DeclRefExpr *DRE : ClaimedVarUseSites) { if (const auto *VD = dyn_cast(DRE->getDecl())) { - result.byVar[VD].emplace(std::move(G)); + result.byVar[VD].insert(G.get()); AssociatedWithVarDecl = true; } } if (!AssociatedWithVarDecl) { - result.noVar.emplace_back(std::move(G)); + result.noVar.push_back(G.get()); continue; } } @@ -675,7 +721,7 @@ } struct FixableGadgetSets { - std::map>> byVar; + std::map> byVar; }; static FixableGadgetSets @@ -686,7 +732,7 @@ for (const DeclRefExpr *DRE : DREs) { if (const auto *VD = dyn_cast(DRE->getDecl())) { - FixablesForUnsafeVars.byVar[VD].emplace(std::move(F)); + FixablesForUnsafeVars.byVar[VD].insert(F.get()); } } } @@ -710,6 +756,27 @@ return std::nullopt; } +std::optional +PointerAssignmentGadget::getFixits(const Strategy &S) const { + if (const auto *LeftDRE = dyn_cast(PA->getLHS()->IgnoreImpCasts())) + if (const VarDecl *LeftVD = dyn_cast(LeftDRE->getDecl())) + if (const auto *RightDRE = dyn_cast(PA->getRHS()->IgnoreImpCasts())) + if (const VarDecl *RightVD = dyn_cast(RightDRE->getDecl())) { + switch (S.lookup(LeftVD)) { + case Strategy::Kind::Span: + if (S.lookup(RightVD) == Strategy::Kind::Span) + return FixItList{}; + return std::nullopt; + case Strategy::Kind::Wontfix: + case Strategy::Kind::Iterator: + case Strategy::Kind::Array: + case Strategy::Kind::Vector: + llvm_unreachable("unsupported strategies for FixableGadgets"); + } + } + return std::nullopt; +} + // Return the text representation of the given `APInt Val`: static std::string getAPIntText(APInt Val) { SmallVector Txt; @@ -972,12 +1039,10 @@ FixableGadgetSets FixablesForUnsafeVars; DeclUseTracker Tracker; - { - auto [FixableGadgets, WarningGadgets, TrackerRes] = findGadgets(D); - UnsafeOps = groupWarningGadgetsByVar(std::move(WarningGadgets)); - FixablesForUnsafeVars = groupFixablesByVar(std::move(FixableGadgets)); - Tracker = std::move(TrackerRes); - } + auto [FixableGadgets, WarningGadgets, TrackerRes] = findGadgets(D); + UnsafeOps = groupWarningGadgetsByVar(std::move(WarningGadgets)); + FixablesForUnsafeVars = groupFixablesByVar(std::move(FixableGadgets)); + Tracker = std::move(TrackerRes); // Filter out non-local vars and vars with unclaimed DeclRefExpr-s. for (auto it = FixablesForUnsafeVars.byVar.cbegin(); Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-fixits.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-fixits.cpp @@ -0,0 +1,43 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + + +void local_assign_both_span() { + int tmp; + int* p = new int[10]; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}" + tmp = p[4]; + + int* q = new int[10]; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span q" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}" + tmp = q[4]; + + q = p; +} + +void local_assign_rhs_span() { + int tmp; + int* p = new int[10]; + int* q = new int[10]; + tmp = q[4]; + p = q; +} + +void local_assign_no_span() { + int tmp; + int* p = new int[10]; + int* q = new int[10]; + p = q; +} + +void local_assign_lhs_span() { + int tmp; + int* p = new int[10]; + tmp = p[4]; + int* q = new int[10]; + + p = q; +}