llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-analysis Author: None (jkorous-apple) <details> <summary>Changes</summary> --- Patch is 26.48 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82027.diff 1 Files Affected: - (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+132-133) ``````````diff diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 769c6d9ebefaa5..50f5d9fb2ba29d 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -130,42 +130,42 @@ class MatchDescendantVisitor bool TraverseGenericSelectionExpr(GenericSelectionExpr *Node) { // These are unevaluated, except the result expression. - if(ignoreUnevaluatedContext) + if (ignoreUnevaluatedContext) return TraverseStmt(Node->getResultExpr()); return VisitorBase::TraverseGenericSelectionExpr(Node); } bool TraverseUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *Node) { // Unevaluated context. - if(ignoreUnevaluatedContext) + if (ignoreUnevaluatedContext) return true; return VisitorBase::TraverseUnaryExprOrTypeTraitExpr(Node); } bool TraverseTypeOfExprTypeLoc(TypeOfExprTypeLoc Node) { // Unevaluated context. - if(ignoreUnevaluatedContext) + if (ignoreUnevaluatedContext) return true; return VisitorBase::TraverseTypeOfExprTypeLoc(Node); } bool TraverseDecltypeTypeLoc(DecltypeTypeLoc Node) { // Unevaluated context. - if(ignoreUnevaluatedContext) + if (ignoreUnevaluatedContext) return true; return VisitorBase::TraverseDecltypeTypeLoc(Node); } bool TraverseCXXNoexceptExpr(CXXNoexceptExpr *Node) { // Unevaluated context. - if(ignoreUnevaluatedContext) + if (ignoreUnevaluatedContext) return true; return VisitorBase::TraverseCXXNoexceptExpr(Node); } bool TraverseCXXTypeidExpr(CXXTypeidExpr *Node) { // Unevaluated context. - if(ignoreUnevaluatedContext) + if (ignoreUnevaluatedContext) return true; return VisitorBase::TraverseCXXTypeidExpr(Node); } @@ -213,24 +213,26 @@ class MatchDescendantVisitor // Because we're dealing with raw pointers, let's define what we mean by that. static auto hasPointerType() { - return hasType(hasCanonicalType(pointerType())); + return hasType(hasCanonicalType(pointerType())); } -static auto hasArrayType() { - return hasType(hasCanonicalType(arrayType())); -} +static auto hasArrayType() { return hasType(hasCanonicalType(arrayType())); } -AST_MATCHER_P(Stmt, forEachDescendantEvaluatedStmt, internal::Matcher<Stmt>, innerMatcher) { +AST_MATCHER_P(Stmt, forEachDescendantEvaluatedStmt, internal::Matcher<Stmt>, + innerMatcher) { const DynTypedMatcher &DTM = static_cast<DynTypedMatcher>(innerMatcher); - MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All, true); + MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All, + true); return Visitor.findMatch(DynTypedNode::create(Node)); } -AST_MATCHER_P(Stmt, forEachDescendantStmt, internal::Matcher<Stmt>, innerMatcher) { +AST_MATCHER_P(Stmt, forEachDescendantStmt, internal::Matcher<Stmt>, + innerMatcher) { const DynTypedMatcher &DTM = static_cast<DynTypedMatcher>(innerMatcher); - MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All, false); + MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All, + false); return Visitor.findMatch(DynTypedNode::create(Node)); } @@ -268,10 +270,9 @@ static auto isInUnspecifiedLvalueContext(internal::Matcher<Expr> innerMatcher) { hasLHS(innerMatcher) ) )); -// clang-format on + // clang-format on } - // Returns a matcher that matches any expression `e` such that `InnerMatcher` // matches `e` and `e` is in an Unspecified Pointer Context (UPC). static internal::Matcher<Stmt> @@ -315,7 +316,7 @@ isInUnspecifiedPointerContext(internal::Matcher<Stmt> InnerMatcher) { // clang-format on return stmt(anyOf(CallArgMatcher, CastOperandMatcher, CompOperandMatcher, - PtrSubtractionMatcher)); + PtrSubtractionMatcher)); // FIXME: any more cases? (UPC excludes the RHS of an assignment. For now we // don't have to check that.) } @@ -481,7 +482,9 @@ class Gadget { #ifndef NDEBUG StringRef getDebugName() const { switch (K) { -#define GADGET(x) case Kind::x: return #x; +#define GADGET(x) \ + case Kind::x: \ + return #x; #include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def" } llvm_unreachable("Unhandled Gadget::Kind enum"); @@ -502,7 +505,6 @@ class Gadget { Kind K; }; - /// Warning gadgets correspond to unsafe code patterns that warrants /// an immediate warning. class WarningGadget : public Gadget { @@ -513,10 +515,10 @@ class WarningGadget : public Gadget { bool isWarningGadget() const final { return true; } }; -/// Fixable gadgets correspond to code patterns that aren't always unsafe but need to be -/// properly recognized in order to emit fixes. For example, if a raw pointer-type -/// variable is replaced by a safe C++ container, every use of such variable must be -/// carefully considered and possibly updated. +/// Fixable gadgets correspond to code patterns that aren't always unsafe but +/// need to be properly recognized in order to emit fixes. For example, if a raw +/// pointer-type variable is replaced by a safe C++ container, every use of such +/// variable must be carefully considered and possibly updated. class FixableGadget : public Gadget { public: FixableGadget(Kind K) : Gadget(K) {} @@ -531,20 +533,19 @@ class FixableGadget : public Gadget { return std::nullopt; } - /// Returns a list of two elements where the first element is the LHS of a pointer assignment - /// statement and the second element is the RHS. This two-element list represents the fact that - /// the LHS buffer gets its bounds information from the RHS buffer. This information will be used - /// later to group all those variables whose types must be modified together to prevent type - /// mismatches. + /// Returns a list of two elements where the first element is the LHS of a + /// pointer assignment statement and the second element is the RHS. This + /// two-element list represents the fact that the LHS buffer gets its bounds + /// information from the RHS buffer. This information will be used later to + /// group all those variables whose types must be modified together to prevent + /// type mismatches. virtual std::optional<std::pair<const VarDecl *, const VarDecl *>> getStrategyImplications() const { return std::nullopt; } }; -static auto toSupportedVariable() { - return to(varDecl()); -} +static auto toSupportedVariable() { return to(varDecl()); } using FixableGadgetList = std::vector<std::unique_ptr<FixableGadget>>; using WarningGadgetList = std::vector<std::unique_ptr<WarningGadget>>; @@ -565,10 +566,10 @@ class IncrementGadget : public WarningGadget { } static Matcher matcher() { - return stmt(unaryOperator( - hasOperatorName("++"), - hasUnaryOperand(ignoringParenImpCasts(hasPointerType())) - ).bind(OpTag)); + return stmt( + unaryOperator(hasOperatorName("++"), + hasUnaryOperand(ignoringParenImpCasts(hasPointerType()))) + .bind(OpTag)); } const UnaryOperator *getBaseStmt() const override { return Op; } @@ -600,10 +601,10 @@ class DecrementGadget : public WarningGadget { } static Matcher matcher() { - return stmt(unaryOperator( - hasOperatorName("--"), - hasUnaryOperand(ignoringParenImpCasts(hasPointerType())) - ).bind(OpTag)); + return stmt( + unaryOperator(hasOperatorName("--"), + hasUnaryOperand(ignoringParenImpCasts(hasPointerType()))) + .bind(OpTag)); } const UnaryOperator *getBaseStmt() const override { return Op; } @@ -754,26 +755,25 @@ class PointerInitGadget : public FixableGadget { private: static constexpr const char *const PointerInitLHSTag = "ptrInitLHS"; static constexpr const char *const PointerInitRHSTag = "ptrInitRHS"; - const VarDecl * PtrInitLHS; // the LHS pointer expression in `PI` - const DeclRefExpr * PtrInitRHS; // the RHS pointer expression in `PI` + const VarDecl *PtrInitLHS; // the LHS pointer expression in `PI` + const DeclRefExpr *PtrInitRHS; // the RHS pointer expression in `PI` public: PointerInitGadget(const MatchFinder::MatchResult &Result) : FixableGadget(Kind::PointerInit), - PtrInitLHS(Result.Nodes.getNodeAs<VarDecl>(PointerInitLHSTag)), - PtrInitRHS(Result.Nodes.getNodeAs<DeclRefExpr>(PointerInitRHSTag)) {} + PtrInitLHS(Result.Nodes.getNodeAs<VarDecl>(PointerInitLHSTag)), + PtrInitRHS(Result.Nodes.getNodeAs<DeclRefExpr>(PointerInitRHSTag)) {} static bool classof(const Gadget *G) { return G->getKind() == Kind::PointerInit; } static Matcher matcher() { - auto PtrInitStmt = declStmt(hasSingleDecl(varDecl( - hasInitializer(ignoringImpCasts(declRefExpr( - hasPointerType(), - toSupportedVariable()). - bind(PointerInitRHSTag)))). - bind(PointerInitLHSTag))); + auto PtrInitStmt = declStmt(hasSingleDecl( + varDecl(hasInitializer(ignoringImpCasts( + declRefExpr(hasPointerType(), toSupportedVariable()) + .bind(PointerInitRHSTag)))) + .bind(PointerInitLHSTag))); return stmt(PtrInitStmt); } @@ -793,8 +793,7 @@ class PointerInitGadget : public FixableGadget { virtual std::optional<std::pair<const VarDecl *, const VarDecl *>> getStrategyImplications() const override { - return std::make_pair(PtrInitLHS, - cast<VarDecl>(PtrInitRHS->getDecl())); + return std::make_pair(PtrInitLHS, cast<VarDecl>(PtrInitRHS->getDecl())); } }; @@ -807,8 +806,8 @@ class PtrToPtrAssignmentGadget : public FixableGadget { private: static constexpr const char *const PointerAssignLHSTag = "ptrLHS"; static constexpr const char *const PointerAssignRHSTag = "ptrRHS"; - const DeclRefExpr * PtrLHS; // the LHS pointer expression in `PA` - const DeclRefExpr * PtrRHS; // the RHS pointer expression in `PA` + const DeclRefExpr *PtrLHS; // the LHS pointer expression in `PA` + const DeclRefExpr *PtrRHS; // the RHS pointer expression in `PA` public: PtrToPtrAssignmentGadget(const MatchFinder::MatchResult &Result) @@ -821,13 +820,13 @@ class PtrToPtrAssignmentGadget : public FixableGadget { } static Matcher matcher() { - auto PtrAssignExpr = binaryOperator(allOf(hasOperatorName("="), - hasRHS(ignoringParenImpCasts(declRefExpr(hasPointerType(), - toSupportedVariable()). - bind(PointerAssignRHSTag))), - hasLHS(declRefExpr(hasPointerType(), - toSupportedVariable()). - bind(PointerAssignLHSTag)))); + auto PtrAssignExpr = binaryOperator( + allOf(hasOperatorName("="), + hasRHS(ignoringParenImpCasts( + declRefExpr(hasPointerType(), toSupportedVariable()) + .bind(PointerAssignRHSTag))), + hasLHS(declRefExpr(hasPointerType(), toSupportedVariable()) + .bind(PointerAssignLHSTag)))); return stmt(isInUnspecifiedUntypedContext(PtrAssignExpr)); } @@ -981,9 +980,8 @@ class ULCArraySubscriptGadget : public FixableGadget { static Matcher matcher() { auto ArrayOrPtr = anyOf(hasPointerType(), hasArrayType()); - auto BaseIsArrayOrPtrDRE = - hasBase(ignoringParenImpCasts(declRefExpr(ArrayOrPtr, - toSupportedVariable()))); + auto BaseIsArrayOrPtrDRE = hasBase( + ignoringParenImpCasts(declRefExpr(ArrayOrPtr, toSupportedVariable()))); auto Target = arraySubscriptExpr(BaseIsArrayOrPtrDRE).bind(ULCArraySubscriptTag); @@ -1025,9 +1023,9 @@ class UPCStandalonePointerGadget : public FixableGadget { static Matcher matcher() { auto ArrayOrPtr = anyOf(hasPointerType(), hasArrayType()); - auto target = expr( - ignoringParenImpCasts(declRefExpr(allOf(ArrayOrPtr, - toSupportedVariable())).bind(DeclRefExprTag))); + auto target = expr(ignoringParenImpCasts( + declRefExpr(allOf(ArrayOrPtr, toSupportedVariable())) + .bind(DeclRefExprTag))); return stmt(isInUnspecifiedPointerContext(target)); } @@ -1036,9 +1034,7 @@ class UPCStandalonePointerGadget : public FixableGadget { virtual const Stmt *getBaseStmt() const override { return Node; } - virtual DeclUseList getClaimedVarUseSites() const override { - return {Node}; - } + virtual DeclUseList getClaimedVarUseSites() const override { return {Node}; } }; class PointerDereferenceGadget : public FixableGadget { @@ -1103,10 +1099,10 @@ class UPCAddressofArraySubscriptGadget : public FixableGadget { static Matcher matcher() { return expr(isInUnspecifiedPointerContext(expr(ignoringImpCasts( - unaryOperator(hasOperatorName("&"), - hasUnaryOperand(arraySubscriptExpr( - hasBase(ignoringParenImpCasts(declRefExpr( - toSupportedVariable())))))) + unaryOperator( + hasOperatorName("&"), + hasUnaryOperand(arraySubscriptExpr(hasBase( + ignoringParenImpCasts(declRefExpr(toSupportedVariable())))))) .bind(UPCAddressofArraySubscriptTag))))); } @@ -1195,13 +1191,13 @@ class DeclUseTracker { class UPCPreIncrementGadget : public FixableGadget { private: static constexpr const char *const UPCPreIncrementTag = - "PointerPreIncrementUnderUPC"; + "PointerPreIncrementUnderUPC"; const UnaryOperator *Node; // the `++Ptr` node public: UPCPreIncrementGadget(const MatchFinder::MatchResult &Result) - : FixableGadget(Kind::UPCPreIncrement), - Node(Result.Nodes.getNodeAs<UnaryOperator>(UPCPreIncrementTag)) { + : FixableGadget(Kind::UPCPreIncrement), + Node(Result.Nodes.getNodeAs<UnaryOperator>(UPCPreIncrementTag)) { assert(Node != nullptr && "Expecting a non-null matching result"); } @@ -1215,10 +1211,9 @@ class UPCPreIncrementGadget : public FixableGadget { // can have the matcher be general, so long as `getClaimedVarUseSites` does // things right. return stmt(isInUnspecifiedPointerContext(expr(ignoringImpCasts( - unaryOperator(isPreInc(), - hasUnaryOperand(declRefExpr( - toSupportedVariable())) - ).bind(UPCPreIncrementTag))))); + unaryOperator(isPreInc(), + hasUnaryOperand(declRefExpr(toSupportedVariable()))) + .bind(UPCPreIncrementTag))))); } virtual std::optional<FixItList> @@ -1791,9 +1786,9 @@ static SourceRange getSourceRangeToTokenEnd(const Decl *D, const LangOptions &LangOpts) { SourceLocation Begin = D->getBeginLoc(); SourceLocation - End = // `D->getEndLoc` should always return the starting location of the - // last token, so we should get the end of the token - Lexer::getLocForEndOfToken(D->getEndLoc(), 0, SM, LangOpts); + End = // `D->getEndLoc` should always return the starting location of the + // last token, so we should get the end of the token + Lexer::getLocForEndOfToken(D->getEndLoc(), 0, SM, LangOpts); return SourceRange(Begin, End); } @@ -1985,7 +1980,7 @@ PointerDereferenceGadget::getFixits(const FixitStrategy &S) const { if (auto LocPastOperand = getPastLoc(BaseDeclRefExpr, SM, Ctx.getLangOpts())) { return FixItList{{FixItHint::CreateRemoval(derefRange), - FixItHint::CreateInsertion(*LocPastOperand, "[0]")}}; + FixItHint::CreateInsertion(*LocPastOperand, "[0]")}}; } break; } @@ -2066,8 +2061,8 @@ fixUPCAddressofArraySubscriptWithSpan(const UnaryOperator *Node) { if (!IndexString) return std::nullopt; - SS << "&" << (*DreString).str() << ".data()" - << "[" << (*IndexString).str() << "]"; + SS << "&" << (*DreString).str() << ".data()" << "[" << (*IndexString).str() + << "]"; } return FixItList{ FixItHint::CreateReplacement(Node->getSourceRange(), SS.str())}; @@ -2160,7 +2155,7 @@ UPCPreIncrementGadget::getFixits(const FixitStrategy &S) const { // `Ctx` a reference to the ASTContext static FixItList FixVarInitializerWithSpan(const Expr *Init, ASTContext &Ctx, - const StringRef UserFillPlaceHolder) { + const StringRef UserFillPlaceHolder) { const SourceManager &SM = Ctx.getSourceManager(); const LangOptions &LangOpts = Ctx.getLangOpts(); @@ -2168,7 +2163,8 @@ FixVarInitializerWithSpan(const Expr *Init, ASTContext &Ctx, // NULL pointer, we use the default constructor to initialize the span // object, i.e., a `std:span` variable declaration with no initializer. // So the fix-it is just to remove the initializer. - if (Init->isNullPointerConstant(Ctx, + if (Init->isNullPointerConstant( + Ctx, // FIXME: Why does this function not ask for `const ASTContext // &`? It should. Maybe worth an NFC patch later. Expr::NullPointerConstantValueDependence:: @@ -2237,8 +2233,10 @@ FixVarInitializerWithSpan(const Expr *Init, ASTContext &Ctx, } #ifndef NDEBUG -#define DEBUG_NOTE_DECL_FAIL(D, Msg) \ -Handler.addDebugNoteForVar((D), (D)->getBeginLoc(), "failed to produce fixit for declaration '" + (D)->getNameAsString() + "'" + (Msg)) +#define DEBUG_NOTE_DECL_FAIL(D, Msg) \ + Handler.addDebugNoteForVar((D), (D)->getBeginLoc(), \ + "failed to produce fixit for declaration '" + \ + (D)->getNameAsString() + "'" + (Msg)) #else #define DEBUG_NOTE_DECL_FAIL(D, Msg) #endif @@ -2246,8 +2244,8 @@ Handler.addDebugNoteForVar((D), (D)->getBeginLoc(), "failed to produce fixit for // For the given variable declaration with a pointer-to-T type, returns the text // `std::span<T>`. If it is unable to generate the text, returns // `std::nullopt`. -static std::optional<std::string> createSpanTypeForVarDecl(const VarDecl *VD, - const ASTContext &Ctx) { +static std::optional<std::string> +createSpanTypeForVarDecl(const VarDecl *VD, const ASTContext &Ctx) { assert(VD->getType()->isPointerType()); std::optional<Qualifiers> PteTyQualifiers = std::nullopt; @@ -2284,8 +2282,8 @@ static std::optional<std::string> createSpanTypeForVarDecl(const VarDecl *VD, // the non-empty fix-it list, if fix-its are successfuly generated; empty // list otherwise. static FixItList fixLocalVarDeclWithSpan(const VarDecl *D, ASTContext &Ctx, - const StringRef UserFillPlaceHolder, - UnsafeBufferUsageHandler &Handler) { + const StringRef UserFillPlaceHolder, + UnsafeBufferUsageHandler &Handler) { if (hasUnsupportedSpecifiers(D, Ctx.getSourceManager())) return {}; @@ -2445,9 +2443,9 @@ createOverloadsForFixedParams(const FixitStrategy &S, const FunctionDecl *FD, // print parameter name if provided: if (IdentifierInfo *II = Parm->getIdentifier()) SS << ' ' << II->getName().str(); - } else if (auto ParmTypeText = getRangeText( - getSourceRangeToTokenEnd(Parm, SM, LangOpts), - SM, LangOpts)) { + } else if (auto ParmTypeText = + getRangeText(getSourceRangeToTokenEnd(Parm, SM, LangOpts), + SM, LangOpts)) { // print the whole `Parm` without modification: SS << ParmTypeText->str(); } else @@ -2591,7 +2589,8 @@ static FixItList... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/82027 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
