From: Nathan James Date: Thu, 25 Mar 2021 14:38:35 +0000 (+0000) Subject: [clang-tidy] Fix mpi checks when running multiple TUs per clang-tidy process X-Git-Tag: llvmorg-14-init~11264 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=02d7ef3181dd6a043a8ad16d747353dd02cbb5ef;p=platform%2Fupstream%2Fllvm.git [clang-tidy] Fix mpi checks when running multiple TUs per clang-tidy process Both the mpi-type-mismatch and mpi-buffer-deref check make use of a static MPIFunctionClassifier object. This causes issue as the classifier is initialized with the first ASTContext that produces a match. If the check is enabled on multiple translation units in a single clang-tidy process, this classifier won't be reinitialized for each TU. I'm not an expert in the MPIFunctionClassifier but I'd imagine this is a source of UB. It is suspected that this bug may result in the crash caused here: https://bugs.llvm.org/show_bug.cgi?id=48985. However even if not the case, this should still be addressed. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D98275 --- diff --git a/clang-tools-extra/clang-tidy/mpi/BufferDerefCheck.cpp b/clang-tools-extra/clang-tidy/mpi/BufferDerefCheck.cpp index b108f75..ebe9658 100644 --- a/clang-tools-extra/clang-tidy/mpi/BufferDerefCheck.cpp +++ b/clang-tools-extra/clang-tidy/mpi/BufferDerefCheck.cpp @@ -9,7 +9,6 @@ #include "BufferDerefCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/StaticAnalyzer/Checkers/MPIFunctionClassifier.h" #include "clang/Tooling/FixIt.h" using namespace clang::ast_matchers; @@ -23,13 +22,15 @@ void BufferDerefCheck::registerMatchers(MatchFinder *Finder) { } void BufferDerefCheck::check(const MatchFinder::MatchResult &Result) { - static ento::mpi::MPIFunctionClassifier FuncClassifier(*Result.Context); const auto *CE = Result.Nodes.getNodeAs("CE"); if (!CE->getDirectCallee()) return; + if (!FuncClassifier) + FuncClassifier.emplace(*Result.Context); + const IdentifierInfo *Identifier = CE->getDirectCallee()->getIdentifier(); - if (!Identifier || !FuncClassifier.isMPIType(Identifier)) + if (!Identifier || !FuncClassifier->isMPIType(Identifier)) return; // These containers are used, to capture the type and expression of a buffer. @@ -60,18 +61,18 @@ void BufferDerefCheck::check(const MatchFinder::MatchResult &Result) { // Collect buffer types and argument expressions for all buffers used in the // MPI call expression. The number passed to the lambda corresponds to the // argument index of the currently verified MPI function call. - if (FuncClassifier.isPointToPointType(Identifier)) { + if (FuncClassifier->isPointToPointType(Identifier)) { AddBuffer(0); - } else if (FuncClassifier.isCollectiveType(Identifier)) { - if (FuncClassifier.isReduceType(Identifier)) { + } else if (FuncClassifier->isCollectiveType(Identifier)) { + if (FuncClassifier->isReduceType(Identifier)) { AddBuffer(0); AddBuffer(1); - } else if (FuncClassifier.isScatterType(Identifier) || - FuncClassifier.isGatherType(Identifier) || - FuncClassifier.isAlltoallType(Identifier)) { + } else if (FuncClassifier->isScatterType(Identifier) || + FuncClassifier->isGatherType(Identifier) || + FuncClassifier->isAlltoallType(Identifier)) { AddBuffer(0); AddBuffer(3); - } else if (FuncClassifier.isBcastType(Identifier)) { + } else if (FuncClassifier->isBcastType(Identifier)) { AddBuffer(0); } } @@ -126,6 +127,7 @@ void BufferDerefCheck::checkBuffers(ArrayRef BufferTypes, } } +void BufferDerefCheck::onEndOfTranslationUnit() { FuncClassifier.reset(); } } // namespace mpi } // namespace tidy } // namespace clang diff --git a/clang-tools-extra/clang-tidy/mpi/BufferDerefCheck.h b/clang-tools-extra/clang-tidy/mpi/BufferDerefCheck.h index 040e3f7..a3be5a8 100644 --- a/clang-tools-extra/clang-tidy/mpi/BufferDerefCheck.h +++ b/clang-tools-extra/clang-tidy/mpi/BufferDerefCheck.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MPI_BUFFER_DEREF_H #include "../ClangTidyCheck.h" +#include "clang/StaticAnalyzer/Checkers/MPIFunctionClassifier.h" namespace clang { namespace tidy { @@ -30,6 +31,7 @@ public: : ClangTidyCheck(Name, Context) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void onEndOfTranslationUnit() override; private: /// Checks for all buffers in an MPI call if they are sufficiently @@ -41,6 +43,8 @@ private: ArrayRef BufferExprs); enum class IndirectionType : unsigned char { Pointer, Array }; + + Optional FuncClassifier; }; } // namespace mpi diff --git a/clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.cpp b/clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.cpp index cc60ea3..fb96ce7 100644 --- a/clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.cpp +++ b/clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.cpp @@ -8,7 +8,6 @@ #include "TypeMismatchCheck.h" #include "clang/Lex/Lexer.h" -#include "clang/StaticAnalyzer/Checkers/MPIFunctionClassifier.h" #include "clang/Tooling/FixIt.h" #include #include @@ -241,13 +240,15 @@ void TypeMismatchCheck::registerMatchers(MatchFinder *Finder) { } void TypeMismatchCheck::check(const MatchFinder::MatchResult &Result) { - static ento::mpi::MPIFunctionClassifier FuncClassifier(*Result.Context); const auto *const CE = Result.Nodes.getNodeAs("CE"); if (!CE->getDirectCallee()) return; + if (!FuncClassifier) + FuncClassifier.emplace(*Result.Context); + const IdentifierInfo *Identifier = CE->getDirectCallee()->getIdentifier(); - if (!Identifier || !FuncClassifier.isMPIType(Identifier)) + if (!Identifier || !FuncClassifier->isMPIType(Identifier)) return; // These containers are used, to capture buffer, MPI datatype pairs. @@ -281,18 +282,18 @@ void TypeMismatchCheck::check(const MatchFinder::MatchResult &Result) { }; // Collect all buffer, MPI datatype pairs for the inspected call expression. - if (FuncClassifier.isPointToPointType(Identifier)) { + if (FuncClassifier->isPointToPointType(Identifier)) { AddPair(0, 2); - } else if (FuncClassifier.isCollectiveType(Identifier)) { - if (FuncClassifier.isReduceType(Identifier)) { + } else if (FuncClassifier->isCollectiveType(Identifier)) { + if (FuncClassifier->isReduceType(Identifier)) { AddPair(0, 3); AddPair(1, 3); - } else if (FuncClassifier.isScatterType(Identifier) || - FuncClassifier.isGatherType(Identifier) || - FuncClassifier.isAlltoallType(Identifier)) { + } else if (FuncClassifier->isScatterType(Identifier) || + FuncClassifier->isGatherType(Identifier) || + FuncClassifier->isAlltoallType(Identifier)) { AddPair(0, 2); AddPair(3, 5); - } else if (FuncClassifier.isBcastType(Identifier)) { + } else if (FuncClassifier->isBcastType(Identifier)) { AddPair(0, 2); } } @@ -331,6 +332,7 @@ void TypeMismatchCheck::checkArguments(ArrayRef BufferTypes, } } +void TypeMismatchCheck::onEndOfTranslationUnit() { FuncClassifier.reset(); } } // namespace mpi } // namespace tidy } // namespace clang diff --git a/clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.h b/clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.h index a563e29..d09ba27 100644 --- a/clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.h +++ b/clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.h @@ -11,6 +11,7 @@ #include "../ClangTidyCheck.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/StaticAnalyzer/Checkers/MPIFunctionClassifier.h" namespace clang { namespace tidy { @@ -31,6 +32,8 @@ public: void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void onEndOfTranslationUnit() override; + private: /// Check if the buffer type MPI datatype pairs match. /// @@ -41,6 +44,8 @@ private: void checkArguments(ArrayRef BufferTypes, ArrayRef BufferExprs, ArrayRef MPIDatatypes, const LangOptions &LO); + + Optional FuncClassifier; }; } // namespace mpi