https://github.com/Harald-R updated https://github.com/llvm/llvm-project/pull/210727
>From 9f2095b10fc723993728b00ec6a597173c4b1f59 Mon Sep 17 00:00:00 2001 From: Harald-R <[email protected]> Date: Mon, 13 Jul 2026 14:17:39 +0300 Subject: [PATCH] Add llvm-mlir-use-after-erase check --- .../clang-tidy/llvm/CMakeLists.txt | 1 + .../clang-tidy/llvm/LLVMTidyModule.cpp | 3 + .../llvm/MlirUseAfterEraseCheck.cpp | 365 ++++++++++++++ .../clang-tidy/llvm/MlirUseAfterEraseCheck.h | 37 ++ clang-tools-extra/docs/ReleaseNotes.rst | 9 + .../docs/clang-tidy/checks/list.rst | 1 + .../checks/llvm/mlir-use-after-erase.rst | 39 ++ .../checkers/llvm/mlir-use-after-erase.cpp | 476 ++++++++++++++++++ 8 files changed, 931 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/llvm/MlirUseAfterEraseCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/llvm/MlirUseAfterEraseCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/llvm/mlir-use-after-erase.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/llvm/mlir-use-after-erase.cpp diff --git a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt index bec3ba50c81c5..847695c853cf5 100644 --- a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt @@ -8,6 +8,7 @@ add_clang_library(clangTidyLLVMModule STATIC HeaderGuardCheck.cpp IncludeOrderCheck.cpp LLVMTidyModule.cpp + MlirUseAfterEraseCheck.cpp PreferIsaOrDynCastInConditionalsCheck.cpp PreferRegisterOverUnsignedCheck.cpp PreferStaticOverAnonymousNamespaceCheck.cpp diff --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp index 918af88c979e0..1f42cce26a0b8 100644 --- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp @@ -14,6 +14,7 @@ #include "FormatvStringCheck.h" #include "HeaderGuardCheck.h" #include "IncludeOrderCheck.h" +#include "MlirUseAfterEraseCheck.h" #include "PreferIsaOrDynCastInConditionalsCheck.h" #include "PreferRegisterOverUnsignedCheck.h" #include "PreferStaticOverAnonymousNamespaceCheck.h" @@ -36,6 +37,8 @@ class LLVMModule : public ClangTidyModule { CheckFactories.registerCheck<FormatvStringCheck>("llvm-formatv-string"); CheckFactories.registerCheck<LLVMHeaderGuardCheck>("llvm-header-guard"); CheckFactories.registerCheck<IncludeOrderCheck>("llvm-include-order"); + CheckFactories.registerCheck<MlirUseAfterEraseCheck>( + "llvm-mlir-use-after-erase"); CheckFactories.registerCheck<readability::NamespaceCommentCheck>( "llvm-namespace-comment"); CheckFactories.registerCheck<PreferIsaOrDynCastInConditionalsCheck>( diff --git a/clang-tools-extra/clang-tidy/llvm/MlirUseAfterEraseCheck.cpp b/clang-tools-extra/clang-tidy/llvm/MlirUseAfterEraseCheck.cpp new file mode 100644 index 0000000000000..5b805cef34b40 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/MlirUseAfterEraseCheck.cpp @@ -0,0 +1,365 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "MlirUseAfterEraseCheck.h" +#include "../utils/ExprSequence.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h" +#include "clang/Analysis/CFG.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallPtrSet.h" +#include <optional> + +using namespace clang::ast_matchers; +using namespace clang::tidy::utils; + +namespace clang::tidy::llvm_check { + +namespace { + +/// Contains information about a use-after-erase. +struct UseAfterErase { + // The DeclRefExpr that constituted the use of the operation. + const DeclRefExpr *DeclRef; + + // Is the order in which the erase and the use are evaluated undefined? + bool EvaluationOrderUndefined = false; + + // Does the use happen in a later loop iteration than the erase? + // + // We default to false and change it to true if required in find(). + bool UseHappensInLaterLoopIteration = false; +}; + +/// Finds uses of an `mlir::Operation` variable that are reachable after the +/// operation was erased (and maintains state required by the various internal +/// helper functions). +class UseAfterEraseFinder { +public: + UseAfterEraseFinder(ASTContext *TheContext); + + // Within the given code block, finds the first use of 'ErasedVariable' that + // occurs after 'EraseCall' (the expression that erases the operation). If a + // use-after-erase is found, returns information about it. + std::optional<UseAfterErase> find(Stmt *CodeBlock, const Expr *EraseCall, + const DeclRefExpr *ErasedVariable); + +private: + std::optional<UseAfterErase> findInternal(const CFGBlock *Block, + const Expr *EraseCall, + const ValueDecl *ErasedVariable); + void getUsesAndReinits(const CFGBlock *Block, const ValueDecl *ErasedVariable, + SmallVectorImpl<const DeclRefExpr *> *Uses, + llvm::SmallPtrSetImpl<const Stmt *> *Reinits); + void getDeclRefs(const CFGBlock *Block, const Decl *ErasedVariable, + llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs); + void getReinits(const CFGBlock *Block, const Decl *ErasedVariable, + llvm::SmallPtrSetImpl<const Stmt *> *Stmts, + llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs); + + ASTContext *Context; + std::unique_ptr<ExprSequence> Sequence; + std::unique_ptr<StmtToBlockMap> BlockMap; + llvm::SmallPtrSet<const CFGBlock *, 8> Visited; +}; + +} // namespace + +UseAfterEraseFinder::UseAfterEraseFinder(ASTContext *TheContext) + : Context(TheContext) {} + +std::optional<UseAfterErase> +UseAfterEraseFinder::find(Stmt *CodeBlock, const Expr *EraseCall, + const DeclRefExpr *ErasedVariable) { + // We include implicit and temporary destructors in the CFG so that + // destructors marked [[noreturn]] are handled correctly in the control flow + // analysis. (These are used in some styles of assertion macros.) + CFG::BuildOptions Options; + Options.AddImplicitDtors = true; + Options.AddTemporaryDtors = true; + const auto TheCFG = CFG::buildCFG(nullptr, CodeBlock, Context, Options); + if (!TheCFG) + return std::nullopt; + + Sequence = std::make_unique<ExprSequence>(TheCFG.get(), CodeBlock, Context); + BlockMap = std::make_unique<StmtToBlockMap>(TheCFG.get(), Context); + Visited.clear(); + + const CFGBlock *EraseBlock = BlockMap->blockContainingStmt(EraseCall); + if (!EraseBlock) + EraseBlock = &TheCFG->getEntry(); + + auto TheUseAfterErase = + findInternal(EraseBlock, EraseCall, ErasedVariable->getDecl()); + + if (TheUseAfterErase) { + if (const CFGBlock *UseBlock = + BlockMap->blockContainingStmt(TheUseAfterErase->DeclRef)) { + // Does the use happen in a later loop iteration than the erase? + // - If they are in the same CFG block, we know the use happened in a + // later iteration if we visited that block a second time. + // - Otherwise, we know the use happened in a later iteration if the + // erase is reachable from the use. + CFGReverseBlockReachabilityAnalysis CFA(*TheCFG); + TheUseAfterErase->UseHappensInLaterLoopIteration = + UseBlock == EraseBlock ? Visited.contains(UseBlock) + : CFA.isReachable(UseBlock, EraseBlock); + } + } + return TheUseAfterErase; +} + +std::optional<UseAfterErase> +UseAfterEraseFinder::findInternal(const CFGBlock *Block, const Expr *EraseCall, + const ValueDecl *ErasedVariable) { + if (Visited.contains(Block)) + return std::nullopt; + + // Mark the block as visited (except if this is the block containing the erase + // call and it's being visited the first time). + if (!EraseCall) + Visited.insert(Block); + + // Get all uses and reinits in the block. + SmallVector<const DeclRefExpr *, 1> Uses; + llvm::SmallPtrSet<const Stmt *, 1> Reinits; + getUsesAndReinits(Block, ErasedVariable, &Uses, &Reinits); + + // Ignore all reinitializations where the erase potentially comes after the + // reinit. + SmallVector<const Stmt *, 1> ReinitsToDelete; + for (const auto *Reinit : Reinits) + if (EraseCall && Sequence->potentiallyAfter(EraseCall, Reinit)) + ReinitsToDelete.push_back(Reinit); + for (const auto *Reinit : ReinitsToDelete) + Reinits.erase(Reinit); + + // Find all uses that potentially come after the erase. + for (const auto *Use : Uses) { + if (!EraseCall || Sequence->potentiallyAfter(Use, EraseCall)) { + // Does the use have a saving reinit? A reinit is saving if it definitely + // comes before the use, i.e. if there's no potential that the reinit is + // after the use. + bool HaveSavingReinit = false; + for (const auto *Reinit : Reinits) + if (!Sequence->potentiallyAfter(Reinit, Use)) + HaveSavingReinit = true; + + if (!HaveSavingReinit) { + UseAfterErase TheUseAfterErase; + TheUseAfterErase.DeclRef = Use; + + // Is this a use-after-erase that depends on order of evaluation? + // This is the case if the erase potentially comes after the use (and we + // already know that the use potentially comes after the erase, which + // taken together tells us that the ordering is unclear). + TheUseAfterErase.EvaluationOrderUndefined = + EraseCall != nullptr && Sequence->potentiallyAfter(EraseCall, Use); + + return TheUseAfterErase; + } + } + } + + // If the operation wasn't reinitialized, call ourselves recursively on all + // successors. + if (Reinits.empty()) { + for (const auto &Succ : Block->succs()) + if (Succ) + if (auto Found = findInternal(Succ, nullptr, ErasedVariable)) + return Found; + } + + return std::nullopt; +} + +void UseAfterEraseFinder::getUsesAndReinits( + const CFGBlock *Block, const ValueDecl *ErasedVariable, + SmallVectorImpl<const DeclRefExpr *> *Uses, + llvm::SmallPtrSetImpl<const Stmt *> *Reinits) { + llvm::SmallPtrSet<const DeclRefExpr *, 1> DeclRefs; + llvm::SmallPtrSet<const DeclRefExpr *, 1> ReinitDeclRefs; + + getDeclRefs(Block, ErasedVariable, &DeclRefs); + getReinits(Block, ErasedVariable, Reinits, &ReinitDeclRefs); + + // All references to the variable that aren't reinitializations are uses. + Uses->clear(); + for (const DeclRefExpr *DeclRef : DeclRefs) + if (!ReinitDeclRefs.contains(DeclRef)) + Uses->push_back(DeclRef); + + // Sort the uses by their occurrence in the source code. + llvm::sort(*Uses, [](const DeclRefExpr *D1, const DeclRefExpr *D2) { + return D1->getExprLoc() < D2->getExprLoc(); + }); +} + +void UseAfterEraseFinder::getDeclRefs( + const CFGBlock *Block, const Decl *ErasedVariable, + llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs) { + DeclRefs->clear(); + const auto DeclRefMatcher = + declRefExpr(to(equalsNode(ErasedVariable))).bind("declref"); + + for (const auto &Elem : *Block) { + const auto S = Elem.getAs<CFGStmt>(); + if (!S) + continue; + + for (const auto &Match : + match(findAll(DeclRefMatcher), *S->getStmt(), *Context)) { + const auto *DeclRef = Match.getNodeAs<DeclRefExpr>("declref"); + if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block) + DeclRefs->insert(DeclRef); + } + } +} + +void UseAfterEraseFinder::getReinits( + const CFGBlock *Block, const Decl *ErasedVariable, + llvm::SmallPtrSetImpl<const Stmt *> *Stmts, + llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs) { + const auto DeclRefMatcher = + declRefExpr(to(equalsNode(ErasedVariable))).bind("declref"); + + // A reinitialization gives the variable a new, valid operation, which makes + // subsequent uses safe again. We treat plain assignments and + // (re-)declarations as reinitializations. For "derived" ops (e.g. + // `mlir::func::FuncOp`), the assignment is an overloaded `operator=`, which + // is represented as a `CXXOperatorCallExpr` rather than a `BinaryOperator`. + const auto ReinitMatcher = + stmt(anyOf(binaryOperator(hasOperatorName("="), + hasLHS(ignoringParenImpCasts(DeclRefMatcher))), + cxxOperatorCallExpr( + hasOverloadedOperatorName("="), + hasArgument(0, ignoringParenImpCasts(DeclRefMatcher))), + declStmt(hasDescendant(equalsNode(ErasedVariable))))) + .bind("reinit"); + + Stmts->clear(); + DeclRefs->clear(); + for (const CFGElement &Elem : *Block) { + const auto S = Elem.getAs<CFGStmt>(); + if (!S) + continue; + + for (const auto &Match : + match(findAll(ReinitMatcher), *S->getStmt(), *Context)) { + const auto *TheStmt = Match.getNodeAs<Stmt>("reinit"); + const auto *DeclRef = Match.getNodeAs<DeclRefExpr>("declref"); + if (TheStmt && BlockMap->blockContainingStmt(TheStmt) == Block) { + Stmts->insert(TheStmt); + + // We count DeclStmts as reinitializations, but they don't have a + // DeclRefExpr associated with them -- so we need to check 'DeclRef' + // before adding it to the set. + if (DeclRef) + DeclRefs->insert(DeclRef); + } + } + } +} + +static void emitDiagnostic(const Expr *EraseCall, const DeclRefExpr *EraseArg, + const UseAfterErase &Use, ClangTidyCheck *Check) { + const auto UseLoc = Use.DeclRef->getExprLoc(); + const auto EraseLoc = EraseCall->getExprLoc(); + + Check->diag(UseLoc, "operation %0 is used after it was erased") + << EraseArg->getDecl(); + Check->diag(EraseLoc, "operation erased here", DiagnosticIDs::Note); + if (Use.EvaluationOrderUndefined) { + Check->diag(UseLoc, + "the use and erase are unsequenced, i.e. there is no guarantee " + "about the order in which they are evaluated", + DiagnosticIDs::Note); + } else if (Use.UseHappensInLaterLoopIteration) { + Check->diag(UseLoc, + "the use happens in a later loop iteration than the erase", + DiagnosticIDs::Note); + } +} + +void MlirUseAfterEraseCheck::registerMatchers(MatchFinder *Finder) { + // The reference to the operation that is being erased. + const auto Arg = declRefExpr(to(varDecl())).bind("arg"); + + // A "derived" op is a value of a subclass of `mlir::OpState` (e.g. + // `mlir::func::FuncOp`) that wraps an `mlir::Operation *`. The underlying + // operation can be obtained via the overloaded `operator->` or via + // `getOperation()`, both of which are declared in `mlir::OpState`. + const auto OfOpState = ofClass(isSameOrDerivedFrom("::mlir::OpState")); + const auto UnwrapDerivedOp = + expr(anyOf(cxxMemberCallExpr( + on(ignoringParenImpCasts(Arg)), + callee(cxxMethodDecl(hasName("getOperation"), OfOpState))), + cxxOperatorCallExpr(hasOverloadedOperatorName("->"), + hasArgument(0, ignoringParenImpCasts(Arg)), + callee(cxxMethodDecl(OfOpState))))); + + // A reference to the operation, either directly (a variable of type + // `mlir::Operation *`) or through a derived op that is unwrapped to the + // underlying `mlir::Operation *`. + const auto OpRef = expr(ignoringParenImpCasts(anyOf(Arg, UnwrapDerivedOp))); + + // Erasure through `mlir::Operation::erase` and `mlir::Operation::destroy` + // member functions. + const auto MemberErase = cxxMemberCallExpr( + on(OpRef), callee(cxxMethodDecl(hasAnyName("erase", "destroy"), + ofClass(hasName("::mlir::Operation"))))); + + // Erasure through `mlir::RewriterBase`. The operation that gets invalidated + // is always the first argument of these methods. + const auto RewriterErase = cxxMemberCallExpr( + callee( + cxxMethodDecl(hasAnyName("eraseOp", "eraseOpResults", "replaceOp", + "replaceOpWithNewOp"), + ofClass(isSameOrDerivedFrom("::mlir::RewriterBase")))), + hasArgument(0, OpRef)); + + Finder->addMatcher( + traverse(TK_AsIs, + expr(anyOf(MemberErase, RewriterErase), + anyOf(hasAncestor(compoundStmt(hasParent( + lambdaExpr().bind("containing-lambda")))), + hasAncestor(functionDecl().bind("containing-func")))) + .bind("erase-call")), + this); +} + +void MlirUseAfterEraseCheck::check(const MatchFinder::MatchResult &Result) { + const auto *EraseCall = Result.Nodes.getNodeAs<Expr>("erase-call"); + const auto *Arg = Result.Nodes.getNodeAs<DeclRefExpr>("arg"); + const auto *ContainingLambda = + Result.Nodes.getNodeAs<LambdaExpr>("containing-lambda"); + const auto *ContainingFunc = + Result.Nodes.getNodeAs<FunctionDecl>("containing-func"); + + // Only track operations that are stored in a local variable. + if (!Arg->getDecl()->getDeclContext()->isFunctionOrMethod()) + return; + + // Collect the code block that could use the operation after it was erased. + SmallVector<Stmt *> CodeBlocks; + if (ContainingLambda) + CodeBlocks.push_back(ContainingLambda->getBody()); + else if (ContainingFunc) + CodeBlocks.push_back(ContainingFunc->getBody()); + + for (Stmt *CodeBlock : CodeBlocks) { + UseAfterEraseFinder Finder(Result.Context); + if (auto Use = Finder.find(CodeBlock, EraseCall, Arg)) + emitDiagnostic(EraseCall, Arg, *Use, this); + } +} + +} // namespace clang::tidy::llvm_check diff --git a/clang-tools-extra/clang-tidy/llvm/MlirUseAfterEraseCheck.h b/clang-tools-extra/clang-tidy/llvm/MlirUseAfterEraseCheck.h new file mode 100644 index 0000000000000..52574ac76fdf9 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/MlirUseAfterEraseCheck.h @@ -0,0 +1,37 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_MLIRUSEAFTERERASECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_MLIRUSEAFTERERASECHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::llvm_check { + +/// Detects uses of an ``mlir::Operation`` after it has been erased, either +/// directly through ``Operation::erase``/``Operation::destroy``, through +/// ``mlir::RewriterBase`` helpers such as ``eraseOp`` and ``replaceOp``, +/// or through derived ops of ``mlir::OpState`` that provide are wrappers +/// around an ``mlir::Operation``. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/llvm/mlir-use-after-erase.html +class MlirUseAfterEraseCheck : public ClangTidyCheck { +public: + MlirUseAfterEraseCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } +}; + +} // namespace clang::tidy::llvm_check + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_MLIRUSEAFTERERASECHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 69c3bcf67b8db..efd5444633cd8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -97,6 +97,15 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`llvm-mlir-use-after-erase + <clang-tidy/checks/llvm/mlir-use-after-erase>` check. + + Detects uses of an ``mlir::Operation`` after it has been erased, either + directly through ``Operation::erase``/``Operation::destroy``, through + ``mlir::RewriterBase`` helpers such as ``eraseOp`` and ``replaceOp``, + or through derived ops of ``mlir::OpState`` that are wrappers around + an ``mlir::Operation``. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 2a44dc78fbc89..7013c5f5e7510 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -246,6 +246,7 @@ Clang-Tidy Checks :doc:`llvm-formatv-string <llvm/formatv-string>`, :doc:`llvm-header-guard <llvm/header-guard>`, :doc:`llvm-include-order <llvm/include-order>`, "Yes" + :doc:`llvm-mlir-use-after-erase <llvm/mlir-use-after-erase>`, :doc:`llvm-namespace-comment <llvm/namespace-comment>`, :doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm/prefer-isa-or-dyn-cast-in-conditionals>`, "Yes" :doc:`llvm-prefer-register-over-unsigned <llvm/prefer-register-over-unsigned>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/mlir-use-after-erase.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/mlir-use-after-erase.rst new file mode 100644 index 0000000000000..b0a05453600ea --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/mlir-use-after-erase.rst @@ -0,0 +1,39 @@ +.. title:: clang-tidy - llvm-mlir-use-after-erase + +llvm-mlir-use-after-erase +========================= + +Detects uses of an ``mlir::Operation`` after it has been erased. + +Once an operation has been erased its memory is no longer valid, so any +subsequent access to it is a use-after-free. This check flags such uses when +the operation is invalidated through: + +- ``mlir::Operation`` member functions ``erase()`` and ``destroy()``, or +- ``mlir::RewriterBase`` helpers ``eraseOp()``, ``eraseOpResults()``, + ``replaceOp()`` and ``replaceOpWithNewOp()``. +- ``mlir::OpState`` wrappers that provide ``operator->`` or ``getOperation()``. + This includes derived ops from ``mlir::Op``, which inherits ``mlir::OpState``. + +The analysis is control-flow aware: only uses that can actually be reached +after the erasing call are reported, and reassigning the variable to a new +operation clears the diagnostic. + +.. code-block:: c++ + + void example(mlir::RewriterBase &rewriter, mlir::Operation *op) { + rewriter.eraseOp(op); + op->dump(); // warning: operation 'op' is used after it was erased + } + +Uses that happen *before* the operation is erased, or after it has been reassigned, are not reported: + +.. code-block:: c++ + + void ok(mlir::Operation *op) { + op->dump(); // fine + op->erase(); + + op = ...; + op->dump(); // fine, op is reassigned to a new operation + } diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/mlir-use-after-erase.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/mlir-use-after-erase.cpp new file mode 100644 index 0000000000000..f4cfdb5873fe7 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/mlir-use-after-erase.cpp @@ -0,0 +1,476 @@ +// RUN: %check_clang_tidy %s llvm-mlir-use-after-erase %t + +namespace mlir { + +class BitVector {}; +class ValueRange {}; +class Location {}; + +class Operation { +public: + void destroy(); + void erase(); + + void dump(); + Location getLoc() { + return Location{}; + } +}; + +class OpState { +public: + Operation *operator->() const { return state; } + Operation *getOperation() { return state; } + +private: + Operation *state; +}; +class Op : public OpState {}; + +class Builder {}; +class OpBuilder : public Builder {}; +class RewriterBase : public OpBuilder { +public: + virtual ~RewriterBase() = default; + + virtual void eraseOp(Operation *op); + Operation *eraseOpResults(Operation *op, const BitVector &eraseIndices); + + virtual void replaceOp(Operation *op, ValueRange newValues); + virtual void replaceOp(Operation *op, Operation *newOp); + template <typename OpTy, typename... Args> + OpTy replaceOpWithNewOp(Operation *op, Args &&...args); +}; +class PatternRewriter : public RewriterBase {}; + +} // namespace mlir + +namespace test { +class MyOp : public mlir::Op { +}; + + +} // namespace test + +namespace { + +void consume([[maybe_unused]] mlir::Operation *op) { +} + +struct NoReturnDestructor { + [[noreturn]] ~NoReturnDestructor(); +}; +#define FATAL() NoReturnDestructor() + +// A type that happens to have erase() / destroy() methods but is unrelated to mlir::Operation +class NotAnOperation { +public: + void erase() {} + void destroy() {} + void dump() {} +}; + +// A type that exposes operator-> / getOperation() returning mlir::Operation* +// but is not derived from mlir::OpState +class NotAnOp { +public: + mlir::Operation* operator->() const { return op; } + mlir::Operation* getOperation() const { return op; } +private: + mlir::Operation* op; +}; + +// A type that has RewriterBase-like method names but is not derived from mlir::RewriterBase +class NotARewriter { +public: + void eraseOp(mlir::Operation *op) {} + void replaceOp(mlir::Operation *op, mlir::Operation *newOp) {} +}; + +} // namespace + +//////////////////////////////////////////////////////////////////////////////// +// General tests + +void useAfterDestroy() { + mlir::Operation* op{}; + op->destroy(); + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:9: note: operation erased here +} + +void useAfterErase() { + mlir::Operation* op{}; + op->erase(); + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:9: note: operation erased here +} + +void useAfterRewriterErase() { + mlir::RewriterBase rewriter{}; + mlir::Operation* op{}; + rewriter.eraseOp(op); + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:14: note: operation erased here +} + +void useAfterRewriterEraseOpResults() { + mlir::RewriterBase rewriter{}; + mlir::Operation* op{}; + rewriter.eraseOpResults(op, {}); + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:14: note: operation erased here +} + +void useAfterRewriterReplaceOp() { + mlir::RewriterBase rewriter{}; + { + mlir::Operation* op{}; + rewriter.replaceOp(op, {}); + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:18: note: operation erased here + } + { + mlir::Operation* op{}; + mlir::Operation* op2{}; + rewriter.replaceOp(op, op2); + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:18: note: operation erased here + } +} + +void useAfterRewriterReplaceOpWithNewOp() { + mlir::RewriterBase rewriter{}; + mlir::Operation* op{}; + rewriter.replaceOpWithNewOp<mlir::Operation>(op); + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:14: note: operation erased here +} + +void useAfterPatternRewriterErase() { + mlir::PatternRewriter rewriter{}; + mlir::Operation* op{}; + rewriter.eraseOp(op); + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:14: note: operation erased here +} + +void useAfterDerivedArrowErase() { + test::MyOp op{}; + op->erase(); + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:9: note: operation erased here +} + +void useAfterDerivedGetOperationErase() { + test::MyOp op{}; + op.getOperation()->erase(); + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:24: note: operation erased here +} + +void useAfterRewriterEraseDerived() { + mlir::RewriterBase rewriter{}; + test::MyOp op{}; + rewriter.eraseOp(op.getOperation()); + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:14: note: operation erased here +} + +void onlyFlagOneUseAfterErase() { + mlir::Operation* op{}; + op->erase(); + op->dump(); // A warning should only be emitted for one use-after-erase + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:9: note: operation erased here + op->dump(); +} + +void eraseAfterErase() { + mlir::Operation* op{}; + op->erase(); + op->erase(); // Erase-after-erase also counts as a use + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:9: note: operation erased here +} + +void useInCall() { + mlir::Operation* op{}; + op->erase(); + consume(op); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:9: note: operation erased here +} + +void useParameterAfterErase(mlir::Operation* op) { + op->erase(); + op->dump(); // The erased operation may be a function parameter + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:9: note: operation erased here +} + +struct Container { + void useAfterEraseInMemberFunction() { + mlir::Operation* op{}; + op->erase(); + op->dump(); // The check also works in member functions + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:13: note: operation erased here + } +}; + +void useAfterEraseInLambda() { + [] { + mlir::Operation* op{}; + op->erase(); + op->dump(); // The check also works in lambdas + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-3]]:13: note: operation erased here + }(); +} + +// Using an operation before it is erased is fine +void useBeforeErase() { + { + mlir::Operation* op{}; + op->dump(); + op->destroy(); + } + { + mlir::PatternRewriter rewriter{}; + mlir::Operation* op{}; + op->dump(); + rewriter.eraseOp(op); + } +} + +void derivedUseBeforeErase() { + test::MyOp op{}; + op->dump(); // Use before erase is fine + op->erase(); +} + +//////////////////////////////////////////////////////////////////////////////// +// Tests involving control flow + +void useAndEraseInLoop(int size) { + mlir::Operation* op{}; + for (int i = 0; i < size; ++i) { + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE+2]]:13: note: operation erased here + // CHECK-MESSAGES: :[[@LINE-3]]:9: note: the use happens in a later loop iteration than the erase + op->erase(); + } +} + +void derivedUseAndEraseInLoop(int size) { + test::MyOp op{}; + for (int i = 0; i < size; ++i) { + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE+2]]:13: note: operation erased here + // CHECK-MESSAGES: :[[@LINE-3]]:9: note: the use happens in a later loop iteration than the erase + op->erase(); + } +} + +void declaredInLoop() { + for (int i = 0; i < 10; ++i) { + mlir::Operation* op{}; + op->dump(); + op->erase(); // No warning if 'op' is declared inside the loop, because it is a fresh operation on every iteration + } +} + +void returnAfterErase(int i) { + mlir::Operation* op{}; + for (int j = 0; j < 10; ++j) { + op->dump(); + if (i > 0) { + op->erase(); // Don't warn if we return after the erase + return; + } + } +} + +void differentBranches(int i) { + mlir::Operation* op{}; + if (i > 0) { + op->erase(); + } else { + op->dump(); // Don't warn if the use is in a different branch from the erase + } +} + +void differentBranchesRewriter(int i) { + mlir::PatternRewriter rewriter{}; + mlir::Operation* op{}; + if (i > 0) { + rewriter.eraseOp(op); + } else { + op->dump(); // Don't warn if the use is in a different branch from the erase + } +} + +void switchFallthrough(int i) { + mlir::Operation* op{}; + switch (i) { + case 1: + op->erase(); + case 2: // A fallthrough in a switch statement causes a warning + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-4]]:13: note: operation erased here + break; + } +} + +void noReturnDestructorAfterErase(bool cond, mlir::Operation* other) { + mlir::Operation* op{}; + op->erase(); + if (cond) { + FATAL(); // [[noreturn]] destructor + } else { + op = other; + } + op->dump(); // only reachable through the branch that reassigns 'op', so no warning should be emitted +} + +//////////////////////////////////////////////////////////////////////////////// +// Tests for reinitializations + +void reassignAfterErase(mlir::Operation* other) { + mlir::Operation* op{}; + op->erase(); + op = other; // Reassignment to a new operation makes the later use safe + op->dump(); +} + +void derivedReassignAfterErase(test::MyOp other) { + test::MyOp op{}; + op->erase(); + op = other; // Reassignment to a new operation makes the later use safe + op->dump(); +} + +void conditionalReassign(int i, mlir::Operation* other) { + mlir::Operation* op{}; + op->erase(); + if (i > 0) { + op = other; // Reassignment is not guaranteed to happen before the use, so we still warn + } + op->dump(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-6]]:9: note: operation erased here +} + +//////////////////////////////////////////////////////////////////////////////// +// Tests related to order of evaluation within expressions + +void sequencingOfEraseAndUse() { + const auto fn = [](mlir::Location, mlir::Operation) { + }; + mlir::RewriterBase rewriter{}; + mlir::Operation* op{}; + fn(op->getLoc(), rewriter.replaceOpWithNewOp<mlir::Operation>(op)); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: operation 'op' is used after it was erased [llvm-mlir-use-after-erase] + // CHECK-MESSAGES: :[[@LINE-2]]:31: note: operation erased here + // CHECK-MESSAGES: :[[@LINE-3]]:8: note: the use and erase are unsequenced, i.e. there is no guarantee about the order in which they are evaluated +} + +//////////////////////////////////////////////////////////////////////////////// +// Tests for types and operations that are not tracked + +void eraseOnNonOperation() { + NotAnOperation* obj{}; + obj->erase(); // erase() on an unrelated type are not flagged + obj->dump(); +} + +void destroyOnNonOperation() { + NotAnOperation* obj{}; + obj->destroy(); // destroy() on an unrelated type is not flagged + obj->dump(); +} + +void eraseOpOnNonRewriter() { + NotARewriter rewriter{}; + mlir::Operation* op{}; + rewriter.eraseOp(op); // eraseOp() on a type not derived from RewriterBase is not flagged + op->dump(); +} + +void replaceOpOnNonRewriter() { + NotARewriter rewriter{}; + mlir::Operation* op{}; + mlir::Operation* newOp{}; + rewriter.replaceOp(op, newOp); // replaceOp() on a type not derived from RewriterBase is not flagged + op->dump(); +} + +void eraseOnNonOpState() { + NotAnOp op{}; + op->erase(); // operator-> on a type not derived from mlir::OpState is not flagged + op->dump(); +} + +void eraseGetOperationOnNonOpState() { + NotAnOp op{}; + op.getOperation()->erase(); // getOperation() on a type not derived from mlir::OpState is not flagged + op->dump(); +} + +mlir::Operation* globalOp{}; // Operations that are not local variables are not tracked +void eraseGlobalOperation() { + globalOp->erase(); + globalOp->dump(); +} + +struct OperationHolder { + mlir::Operation* op; // Operations stored in member fields are not tracked + void eraseAndUse() { + op->erase(); + op->dump(); + } +}; + +void differentOperations() { + mlir::Operation* op1{}; + mlir::Operation* op2{}; + op1->erase(); + op2->dump(); // Erasing one operation doesn't flag the use of a different operation +} + +void replacementOperationNotErased() { + mlir::RewriterBase rewriter{}; + mlir::Operation* op{}; + mlir::Operation* newOp{}; + rewriter.replaceOp(op, newOp); // The replacement operation passed to replaceOp is not the erased one + newOp->dump(); +} + +void decltypeIsNotUse() { + mlir::Operation* op{}; + op->erase(); + using OpType = decltype(op); // Using decltype on an erased operation is not flagged + OpType op2{}; + op2->dump(); +} + +void unevaluatedIsNotUse() { + mlir::Operation* op{}; + op->erase(); + static_assert(sizeof(op) == sizeof(void*)); // Unevaluated, so not a real use +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
