Index: clang-tidy/mpi/BufferDerefCheck.h =================================================================== --- /dev/null +++ clang-tidy/mpi/BufferDerefCheck.h @@ -0,0 +1,51 @@ +//===--- BufferDerefCheck.h - clang-tidy-------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MPI_BUFFER_DEREF_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MPI_BUFFER_DEREF_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace mpi { + +/// This check verifies if a buffer passed to an MPI (Message Passing Interface) +/// function is sufficiently dereferenced. Buffers should be passed as a single +/// pointer or array. As MPI function signatures specify void * for their buffer +/// types, insufficiently dereferenced buffers can be passed, like for example +/// as double pointers or multidimensional arrays, without a compiler warning +/// emitted. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/mpi-buffer-deref.html +class BufferDerefCheck : public ClangTidyCheck { +public: + BufferDerefCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + /// Checks for all buffers in an MPI call if they are sufficiently + /// dereferenced. + /// + /// \param BufferTypes buffer types + /// \param BufferExprs buffer arguments as expressions + void checkBuffers(ArrayRef BufferTypes, + ArrayRef BufferExprs); + + enum class IndirectionType : unsigned char { Pointer, Array }; +}; + +} // namespace mpi +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MPI_BUFFER_DEREF_H Index: clang-tidy/mpi/BufferDerefCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/mpi/BufferDerefCheck.cpp @@ -0,0 +1,127 @@ +//===--- BufferDerefCheck.cpp - clang-tidy---------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#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; + +namespace clang { +namespace tidy { +namespace mpi { + +void BufferDerefCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(callExpr().bind("CE"), this); +} + +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; + + const IdentifierInfo *Identifier = CE->getDirectCallee()->getIdentifier(); + if (!Identifier || !FuncClassifier.isMPIType(Identifier)) + return; + + // These containers are used, to capture the type and expression of a buffer. + SmallVector BufferTypes; + SmallVector BufferExprs; + + // Adds the type and expression of a buffer that is used in the MPI call + // expression to the captured containers. + auto addBuffer = [&CE, &Result, &BufferTypes, + &BufferExprs](const size_t BufferIdx) { + // Skip null pointer constants and in place 'operators'. + if (CE->getArg(BufferIdx)->isNullPointerConstant( + *Result.Context, Expr::NPC_ValueDependentIsNull) || + tooling::fixit::getText(*CE->getArg(BufferIdx), *Result.Context) == + "MPI_IN_PLACE") + return; + + const Type *ArgType = + CE->getArg(BufferIdx)->IgnoreImpCasts()->getType().getTypePtr(); + BufferExprs.push_back(CE->getArg(BufferIdx)); + BufferTypes.push_back(ArgType); + }; + + // Collect buffer types and argument expressions for all buffers used in the + // MPI call expression. + if (FuncClassifier.isPointToPointType(Identifier)) { + addBuffer(0); + } else if (FuncClassifier.isCollectiveType(Identifier)) { + if (FuncClassifier.isReduceType(Identifier)) { + addBuffer(0); + addBuffer(1); + } else if (FuncClassifier.isScatterType(Identifier) || + FuncClassifier.isGatherType(Identifier) || + FuncClassifier.isAlltoallType(Identifier)) { + addBuffer(0); + addBuffer(3); + } else if (FuncClassifier.isBcastType(Identifier)) { + addBuffer(0); + } + } + + checkBuffers(BufferTypes, BufferExprs); +} + +void BufferDerefCheck::checkBuffers(ArrayRef BufferTypes, + ArrayRef BufferExprs) { + for (size_t i = 0; i < BufferTypes.size(); ++i) { + unsigned IndirectionCount = 0; + const Type *BufferType = BufferTypes[i]; + llvm::SmallVector Indirections; + + // Capture the depth and types of indirections for the passed buffer. + while (true) { + if (BufferType->isPointerType()) { + BufferType = BufferType->getPointeeType().getTypePtr(); + Indirections.push_back(IndirectionType::Pointer); + } else if (BufferType->isArrayType()) { + BufferType = BufferType->getArrayElementTypeNoTypeQual(); + Indirections.push_back(IndirectionType::Array); + } else { + break; + } + ++IndirectionCount; + } + + if (IndirectionCount > 1) { + // Referencing an array with '&' is valid, as this also points to the + // beginning of the array. + if (IndirectionCount == 2 && + Indirections[0] == IndirectionType::Pointer && + Indirections[1] == IndirectionType::Array) + return; + + // Build the indirection description in reverse order of discovery. + std::string IndirectionDesc; + for (int i = Indirections.size() - 1; i >= 0; --i) { + if (Indirections[i] == IndirectionType::Pointer) { + IndirectionDesc += "pointer"; + } else { + IndirectionDesc += "array"; + } + if (i > 0) { + IndirectionDesc += "->"; + } + } + const auto Loc = BufferExprs[i]->getSourceRange().getBegin(); + diag(Loc, "buffer is insufficiently dereferenced: %0") << IndirectionDesc; + } + } +} + +} // namespace mpi +} // namespace tidy +} // namespace clang Index: clang-tidy/mpi/CMakeLists.txt =================================================================== --- clang-tidy/mpi/CMakeLists.txt +++ clang-tidy/mpi/CMakeLists.txt @@ -1,6 +1,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyMPIModule + BufferDerefCheck.cpp MPITidyModule.cpp TypeMismatchCheck.cpp Index: clang-tidy/mpi/MPITidyModule.cpp =================================================================== --- clang-tidy/mpi/MPITidyModule.cpp +++ clang-tidy/mpi/MPITidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "BufferDerefCheck.h" #include "TypeMismatchCheck.h" namespace clang { @@ -19,6 +20,7 @@ class MPIModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck("mpi-buffer-deref"); CheckFactories.registerCheck("mpi-type-mismatch"); } }; Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -108,6 +108,7 @@ modernize-use-nullptr modernize-use-override modernize-use-using + mpi-buffer-deref mpi-type-mismatch performance-faster-string-find performance-for-range-copy Index: docs/clang-tidy/checks/mpi-buffer-deref.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/mpi-buffer-deref.rst @@ -0,0 +1,24 @@ +.. title:: clang-tidy - mpi-buffer-deref + +mpi-buffer-deref +================ + +This check verifies if a buffer passed to an MPI (Message Passing Interface) +function is sufficiently dereferenced. Buffers should be passed as a single +pointer or array. As MPI function signatures specify ``void *`` for their buffer +types, insufficiently dereferenced buffers can be passed, like for example as +double pointers or multidimensional arrays, without a compiler warning emitted. + +Examples: +.. code:: c++ + // A double pointer is passed to the MPI function. + char *buf; + MPI_Send(&buf, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD); + + // A multidimensional array is passed to the MPI function. + short buf[1][1]; + MPI_Send(buf, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD); + + // A pointer to an array is passed to the MPI function. + short *buf[1]; + MPI_Send(buf, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD); Index: test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h =================================================================== --- test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h +++ test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h @@ -24,6 +24,7 @@ #define MPI_DATATYPE_NULL 0 #define MPI_CHAR 0 #define MPI_BYTE 0 +#define MPI_SHORT 0 #define MPI_INT 0 #define MPI_LONG 0 #define MPI_LONG_DOUBLE 0 @@ -31,6 +32,7 @@ #define MPI_INT8_T 0 #define MPI_UINT8_T 0 #define MPI_UINT16_T 0 +#define MPI_C_FLOAT_COMPLEX 0 #define MPI_C_LONG_DOUBLE_COMPLEX 0 #define MPI_FLOAT 0 #define MPI_DOUBLE 0 Index: test/clang-tidy/mpi-buffer-deref.cpp =================================================================== --- /dev/null +++ test/clang-tidy/mpi-buffer-deref.cpp @@ -0,0 +1,50 @@ +// RUN: %check_clang_tidy %s mpi-buffer-deref %t -- -- -I %S/Inputs/mpi-type-mismatch + +#include "mpimock.h" + +void negativeTests() { + char *buf; + MPI_Send(&buf, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->pointer [mpi-buffer-deref] + + unsigned **buf2; + MPI_Send(buf2, 1, MPI_UNSIGNED, 0, 0, MPI_COMM_WORLD); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->pointer + + short buf3[1][1]; + MPI_Send(buf3, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: array->array + + long double _Complex *buf4[1]; + MPI_Send(buf4, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->array + + std::complex *buf5[1][1]; + MPI_Send(&buf5, 1, MPI_CXX_FLOAT_COMPLEX, 0, 0, MPI_COMM_WORLD); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->array->array->pointer +} + +void positiveTests() { + char buf; + MPI_Send(&buf, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD); + + unsigned *buf2; + MPI_Send(buf2, 1, MPI_UNSIGNED, 0, 0, MPI_COMM_WORLD); + + short buf3[1][1]; + MPI_Send(buf3[0], 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD); + + long double _Complex *buf4[1]; + MPI_Send(*buf4, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD); + + long double _Complex buf5[1]; + MPI_Send(buf5, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD); + + std::complex *buf6[1][1]; + MPI_Send(*buf6[0], 1, MPI_CXX_FLOAT_COMPLEX, 0, 0, MPI_COMM_WORLD); + + // Referencing an array with '&' is valid, as this also points to the + // beginning of the array. + long double _Complex buf7[1]; + MPI_Send(&buf7, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD); +}