[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 (file)
--- 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<CallExpr>("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<const Type *> 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 (file)
--- 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<const Expr *> BufferExprs);
 
   enum class IndirectionType : unsigned char { Pointer, Array };
+
+  Optional<ento::mpi::MPIFunctionClassifier> 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 (file)
--- 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 <map>
 #include <unordered_set>
@@ -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<CallExpr>("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<const Type *> 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 (file)
--- 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<const Type *> BufferTypes,
                       ArrayRef<const Expr *> BufferExprs,
                       ArrayRef<StringRef> MPIDatatypes, const LangOptions &LO);
+
+  Optional<ento::mpi::MPIFunctionClassifier> FuncClassifier;
 };
 
 } // namespace mpi