[clang-tools-extra] 02d7ef3 - [clang-tidy] Fix mpi checks when running multiple TUs per clang-tidy process
Nathan James via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 25 07:38:43 PDT 2021
Author: Nathan James
Date: 2021-03-25T14:38:37Z
New Revision: 02d7ef3181dd6a043a8ad16d747353dd02cbb5ef
URL: https://github.com/llvm/llvm-project/commit/02d7ef3181dd6a043a8ad16d747353dd02cbb5ef
DIFF: https://github.com/llvm/llvm-project/commit/02d7ef3181dd6a043a8ad16d747353dd02cbb5ef.diff
LOG: [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
Added:
Modified:
clang-tools-extra/clang-tidy/mpi/BufferDerefCheck.cpp
clang-tools-extra/clang-tidy/mpi/BufferDerefCheck.h
clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.cpp
clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.h
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/mpi/BufferDerefCheck.cpp b/clang-tools-extra/clang-tidy/mpi/BufferDerefCheck.cpp
index b108f75fbc7a..ebe9658d8b2b 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<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 040e3f790e61..a3be5a8224e0 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 @@ class BufferDerefCheck : public ClangTidyCheck {
: 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 @@ class BufferDerefCheck : public ClangTidyCheck {
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 cc60ea365c2f..fb96ce77a13d 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 <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 a563e295c7b7..d09ba270495b 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 @@ class TypeMismatchCheck : public ClangTidyCheck {
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 @@ class TypeMismatchCheck : public ClangTidyCheck {
void checkArguments(ArrayRef<const Type *> BufferTypes,
ArrayRef<const Expr *> BufferExprs,
ArrayRef<StringRef> MPIDatatypes, const LangOptions &LO);
+
+ Optional<ento::mpi::MPIFunctionClassifier> FuncClassifier;
};
} // namespace mpi
More information about the cfe-commits
mailing list