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,50 @@ } }; +/// 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())) { + return DeclUseList {LDRE}; + } + return {}; + } +}; /// A call of a function or method that performs unchecked buffer operations /// over one of its pointer parameters. @@ -710,6 +754,30 @@ 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; @@ -898,7 +966,7 @@ case Strategy::Kind::Vector: llvm_unreachable("Strategy not implemented yet!"); case Strategy::Kind::Wontfix: - llvm_unreachable("Invalid strategy!"); + return {}; } } @@ -969,34 +1037,34 @@ assert(D && D->getBody()); WarningGadgetSets UnsafeOps; - FixableGadgetSets FixablesForUnsafeVars; + FixableGadgetSets FixablesForAllVars; DeclUseTracker Tracker; { auto [FixableGadgets, WarningGadgets, TrackerRes] = findGadgets(D); UnsafeOps = groupWarningGadgetsByVar(std::move(WarningGadgets)); - FixablesForUnsafeVars = groupFixablesByVar(std::move(FixableGadgets)); + FixablesForAllVars = 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(); - it != FixablesForUnsafeVars.byVar.cend();) { + for (auto it = FixablesForAllVars.byVar.cbegin(); + it != FixablesForAllVars.byVar.cend();) { // FIXME: Support ParmVarDecl as well. - if (!it->first->isLocalVarDecl() || Tracker.hasUnclaimedUses(it->first)) { - it = FixablesForUnsafeVars.byVar.erase(it); + if (!it->first->isLocalVarDecl() /*|| Tracker.hasUnclaimedUses(it->first)*/) { + it = FixablesForAllVars.byVar.erase(it); } else { ++it; } } llvm::SmallVector UnsafeVars; - for (const auto &[VD, ignore] : FixablesForUnsafeVars.byVar) + for (const auto &[VD, ignore] : UnsafeOps.byVar) UnsafeVars.push_back(VD); Strategy NaiveStrategy = getNaiveStrategy(UnsafeVars); std::map FixItsForVariable = - getFixIts(FixablesForUnsafeVars, NaiveStrategy, Tracker, + getFixIts(FixablesForAllVars, NaiveStrategy, Tracker, D->getASTContext(), Handler); // FIXME Detect overlapping FixIts. 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,44 @@ +// 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; +} +