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(PtrAssignment) #undef FIXABLE_GADGET #undef WARNING_GADGET Index: clang/lib/Analysis/UnsafeBufferUsage.cpp =================================================================== --- clang/lib/Analysis/UnsafeBufferUsage.cpp +++ clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -216,7 +216,7 @@ /// Returns a fixit that would fix the current gadget according to /// the current strategy. Returns None if the fix cannot be produced; /// returns an empty list if no fixes are necessary. - virtual std::optional getFixits(const Strategy &) const { + virtual std::optional getFixits(Strategy &) const { return std::nullopt; } }; @@ -410,7 +410,7 @@ return expr(isInUnspecifiedLvalueContext(Target)); } - virtual std::optional getFixits(const Strategy &S) const override; + virtual std::optional getFixits(Strategy &S) const override; virtual const Stmt *getBaseStmt() const override { return Node; } @@ -422,6 +422,50 @@ } }; +/// A pointer assignment expression of one of the forms: +/// \code +/// p = q; +/// \endcode +class PtrAssignmentGadget : 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: + PtrAssignmentGadget(const MatchFinder::MatchResult &Result) + : FixableGadget(Kind::PtrAssignment), + 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::PtrAssignment; + } + + 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(Strategy &S) const override; + + virtual const Stmt *getBaseStmt() const override { return PA; } + + virtual DeclUseList getClaimedVarUseSites() const override { + if (const auto *DRE = dyn_cast(PtrLHS->IgnoreParenImpCasts())) { + return {DRE}; + } + return {}; + } +}; /// A call of a function or method that performs unchecked buffer operations /// over one of its pointer parameters. @@ -694,7 +738,7 @@ } std::optional -ULCArraySubscriptGadget::getFixits(const Strategy &S) const { +ULCArraySubscriptGadget::getFixits(Strategy &S) const { if (const auto *DRE = dyn_cast(Node->getBase()->IgnoreImpCasts())) if (const auto *VD = dyn_cast(DRE->getDecl())) { switch (S.lookup(VD)) { @@ -710,6 +754,28 @@ return std::nullopt; } +std::optionalPtrAssignmentGadget::getFixits(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(RightVD)) { + case Strategy::Kind::Span: + S.set(LeftVD, Strategy::Kind::Span); + return FixItList{}; + 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; @@ -917,7 +983,7 @@ } static std::map -getFixIts(FixableGadgetSets &FixablesForUnsafeVars, const Strategy &S, +getFixIts(FixableGadgetSets &FixablesForUnsafeVars, Strategy &S, const DeclUseTracker &Tracker, const ASTContext &Ctx, UnsafeBufferUsageHandler &Handler) { std::map FixItsForVariable; @@ -983,7 +1049,7 @@ for (auto it = FixablesForUnsafeVars.byVar.cbegin(); it != FixablesForUnsafeVars.byVar.cend();) { // FIXME: Support ParmVarDecl as well. - if (!it->first->isLocalVarDecl() || Tracker.hasUnclaimedUses(it->first)) { + if (!it->first->isLocalVarDecl() /*|| Tracker.hasUnclaimedUses(it->first)*/) { it = FixablesForUnsafeVars.byVar.erase(it); } else { ++it; @@ -1014,4 +1080,14 @@ Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/true); } } + + for (const auto &[VD, FixableGadgets] : FixablesForUnsafeVars.byVar) { + auto FixItsIt = FixItsForVariable.find(VD); + Handler.handleFixableVariable(VD, FixItsIt != FixItsForVariable.end() + ? std::move(FixItsIt->second) + : FixItList{}); + for (const auto &G : FixableGadgets) { + Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/true); + } + } } Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-fixes.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-fixes.cpp @@ -0,0 +1,17 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +void multi_decl_local_assign1() { + 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[5]; + + 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}" + q = p; +} +