barcisz updated this revision to Diff 459870. barcisz added a comment. Added some explanation comments
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133804/new/ https://reviews.llvm.org/D133804 Files: clang-tools-extra/clang-tidy/CMakeLists.txt clang-tools-extra/clang-tidy/ClangTidyForceLinker.h clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp clang-tools-extra/clang-tidy/cuda/CMakeLists.txt clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h clang-tools-extra/clang-tidy/utils/Matchers.h clang-tools-extra/test/clang-tidy/check_clang_tidy.py clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu clang-tools-extra/test/lit.cfg.py
Index: clang-tools-extra/test/lit.cfg.py =================================================================== --- clang-tools-extra/test/lit.cfg.py +++ clang-tools-extra/test/lit.cfg.py @@ -16,7 +16,7 @@ config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell) # suffixes: A list of file extensions to treat as test files. -config.suffixes = ['.c', '.cpp', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s', +config.suffixes = ['.c', '.cpp', '.cu', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s', '.modularize', '.module-map-checker', '.test'] # Test-time dependencies located in directories called 'Inputs' are excluded Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu @@ -0,0 +1,104 @@ +//===--- SlicingCheck.cpp - clang-tidy-------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: [{key: cuda-unsafe-api-call.HandlerName, \ +// RUN: value: 'CUDA_HANDLER'}] \ +// RUN: }" \ +// RUN: -- -isystem %clang_tidy_headers -nocudalib -nocudainc -std=c++14 +#include <cuda/cuda_runtime.h> + +class DummyContainer { + public: + int* begin(); + int* end(); +}; + +#define DUMMY_CUDA_HANDLER(stmt) stmt +#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0) +#define API_CALL() do {cudaDeviceReset();} while(0) + +void errorCheck(); +void errorCheck(cudaError_t error); + +void bad() { + API_CALL(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call. + // There isn't supposed to be a fix here since it's a macro call + + cudaDeviceReset(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call. + // CHECK-FIXES: {{^}} CUDA_HANDLER(cudaDeviceReset());{{$}} + errorCheck(); + + if (true) + cudaDeviceReset(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call. + // CHECK-FIXES: {{^}} CUDA_HANDLER(cudaDeviceReset());{{$}} + + while (true) + cudaDeviceReset(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call. + // CHECK-FIXES: {{^}} CUDA_HANDLER(cudaDeviceReset());{{$}} + + do + cudaDeviceReset(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call. + // CHECK-FIXES: {{^}} CUDA_HANDLER(cudaDeviceReset());{{$}} + while(false); + + switch (0) { + case 0: + cudaDeviceReset(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call. + // CHECK-FIXES: {{^}} CUDA_HANDLER(cudaDeviceReset());{{$}} + } + + for( + cudaDeviceReset(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call. + // CHECK-FIXES: {{^}} CUDA_HANDLER(cudaDeviceReset());{{$}} + ; + cudaDeviceReset() + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call. + // CHECK-FIXES: {{^}} CUDA_HANDLER(cudaDeviceReset()){{$}} + ) cudaDeviceReset(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call. + // CHECK-FIXES: {{^}} ) CUDA_HANDLER(cudaDeviceReset());{{$}} + + for(int i : DummyContainer()) + cudaDeviceReset(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call. + // CHECK-FIXES: {{^}} CUDA_HANDLER(cudaDeviceReset());{{$}} + + auto x = ({ + cudaDeviceReset(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call. + // CHECK-FIXES: {{^}} CUDA_HANDLER(cudaDeviceReset());{{$}} + true; + }); +} + +int good() { + DUMMY_CUDA_HANDLER(cudaDeviceReset()); + + if (cudaDeviceReset()) { + return 0; + } + + switch (cudaDeviceReset()) { + case cudaErrorInvalidValue: return 1; + case cudaErrorMemoryAllocation: return 2; + default: return 3; + } + + auto err = ({cudaDeviceReset();}); + // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does. + errorCheck(cudaDeviceReset()); +} Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu @@ -0,0 +1,73 @@ +// (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary. + +// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: [{key: cuda-unsafe-api-call.HandlerName, \ +// RUN: value: 'cudaHandler'}, \ +// RUN: {key: cuda-unsafe-api-call.AcceptedHandlers, \ +// RUN: value: 'CUDA_HANDLER, DUMMY_CUDA_HANDLER, \ +// RUN: alternative::cudaAlternativeHandler, \ +// RUN: cudaOtherAlternativeHandler, bad::cudaBadHandler'}] \ +// RUN: }" \ +// RUN: -- -isystem %clang_tidy_headers -nocudalib -nocudainc -std=c++14 +#include <cuda/cuda_runtime.h> + +#define DUMMY_CUDA_HANDLER(stmt) stmt +#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0) +#define API_CALL() do {cudaDeviceReset();} while(0) +#define HANDLED_API_CALL() do {int err2 = cudaDeviceReset();} while(0) + +void cudaHandler(); +void cudaHandler(cudaError_t error); +void badCudaHandler(cudaError_t error); + +namespace alternative { + +void cudaAlternativeHandler(cudaError_t error); + +void cudaOtherAlternativeHandler(cudaError_t error); + +} // namespace alternative + +void bad() { + API_CALL(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: CUDA API call not checked properly. + // There isn't supposed to be a fix here since it's a macro call + + HANDLED_API_CALL(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: CUDA API call not checked properly. + // There isn't supposed to be a fix here since it's a macro call + + cudaDeviceReset(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: CUDA API call not checked properly. + // CHECK-FIXES: {{^}} cudaHandler(cudaDeviceReset());{{$}} + cudaHandler(); + + if (true) + cudaDeviceReset(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: CUDA API call not checked properly. + // CHECK-FIXES: {{^}} cudaHandler(cudaDeviceReset());{{$}} + + badCudaHandler(cudaDeviceReset()); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: CUDA API call not checked properly. + // There isn't supposed to be a fix here since the result value is not unused + + int err = cudaDeviceReset(); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: CUDA API call not checked properly. + // There isn't supposed to be a fix here since the result value is not unused + + if (cudaDeviceReset()) { + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: CUDA API call not checked properly. + // There isn't supposed to be a fix here since the result value is not unused + return; + } + +} + +void good() { + cudaHandler(cudaDeviceReset()); + alternative::cudaAlternativeHandler(cudaDeviceReset()); + alternative::cudaOtherAlternativeHandler(cudaDeviceReset()); + CUDA_HANDLER(cudaDeviceReset() + 1); + DUMMY_CUDA_HANDLER(cudaDeviceReset()); +} Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h @@ -0,0 +1,3 @@ +#include "cuda.h" + +cudaError_t cudaDeviceReset(); Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h @@ -0,0 +1,31 @@ +/* Minimal declarations for CUDA support. Testing purposes only. */ + +#include <stddef.h> + +#define __constant__ __attribute__((constant)) +#define __device__ __attribute__((device)) +#define __global__ __attribute__((global)) +#define __host__ __attribute__((host)) +#define __shared__ __attribute__((shared)) + +struct dim3 { + unsigned x, y, z; + __host__ __device__ dim3(unsigned x, unsigned y = 1, unsigned z = 1) : x(x), y(y), z(z) {} +}; + +typedef struct cudaStream *cudaStream_t; +typedef enum cudaError { + cudaErrorInvalidValue, + cudaErrorMemoryAllocation +} cudaError_t; +extern "C" int cudaConfigureCall(dim3 gridSize, dim3 blockSize, + size_t sharedSize = 0, + cudaStream_t stream = 0); +extern "C" int __cudaPushCallConfiguration(dim3 gridSize, dim3 blockSize, + size_t sharedSize = 0, + cudaStream_t stream = 0); +extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim, + dim3 blockDim, void **args, + size_t sharedMem, cudaStream_t stream); + +extern "C" __device__ int printf(const char*, ...); Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h @@ -0,0 +1,145 @@ +// CUDA struct types with interesting initialization properties. +// Keep in sync with clang/test/SemaCUDA/Inputs/cuda-initializers.h. + +// Base classes with different initializer variants. + +// trivial constructor -- allowed +struct T { + int t; +}; + +// empty constructor +struct EC { + int ec; + __device__ EC() {} // -- allowed + __device__ EC(int) {} // -- not allowed +}; + +// empty destructor +struct ED { + __device__ ~ED() {} // -- allowed +}; + +struct ECD { + __device__ ECD() {} // -- allowed + __device__ ~ECD() {} // -- allowed +}; + +// empty templated constructor -- allowed with no arguments +struct ETC { + template <typename... T> __device__ ETC(T...) {} +}; + +// undefined constructor -- not allowed +struct UC { + int uc; + __device__ UC(); +}; + +// undefined destructor -- not allowed +struct UD { + int ud; + __device__ ~UD(); +}; + +// empty constructor w/ initializer list -- not allowed +struct ECI { + int eci; + __device__ ECI() : eci(1) {} +}; + +// non-empty constructor -- not allowed +struct NEC { + int nec; + __device__ NEC() { nec = 1; } +}; + +// non-empty destructor -- not allowed +struct NED { + int ned; + __device__ ~NED() { ned = 1; } +}; + +// no-constructor, virtual method -- not allowed +struct NCV { + int ncv; + __device__ virtual void vm() {} +}; + +// virtual destructor -- not allowed. +struct VD { + __device__ virtual ~VD() {} +}; + +// dynamic in-class field initializer -- not allowed +__device__ int f(); +struct NCF { + int ncf = f(); +}; + +// static in-class field initializer. NVCC does not allow it, but +// clang generates static initializer for this, so we'll accept it. +// We still can't use it on __shared__ vars as they don't allow *any* +// initializers. +struct NCFS { + int ncfs = 3; +}; + +// undefined templated constructor -- not allowed +struct UTC { + template <typename... T> __device__ UTC(T...); +}; + +// non-empty templated constructor -- not allowed +struct NETC { + int netc; + template <typename... T> __device__ NETC(T...) { netc = 1; } +}; + +// Regular base class -- allowed +struct T_B_T : T {}; + +// Incapsulated object of allowed class -- allowed +struct T_F_T { + T t; +}; + +// array of allowed objects -- allowed +struct T_FA_T { + T t[2]; +}; + + +// Calling empty base class initializer is OK +struct EC_I_EC : EC { + __device__ EC_I_EC() : EC() {} +}; + +// .. though passing arguments is not allowed. +struct EC_I_EC1 : EC { + __device__ EC_I_EC1() : EC(1) {} +}; + +// Virtual base class -- not allowed +struct T_V_T : virtual T {}; + +// Inherited from or incapsulated class with non-empty constructor -- +// not allowed +struct T_B_NEC : NEC {}; +struct T_F_NEC { + NEC nec; +}; +struct T_FA_NEC { + NEC nec[2]; +}; + + +// Inherited from or incapsulated class with non-empty desstructor -- +// not allowed +struct T_B_NED : NED {}; +struct T_F_NED { + NED ned; +}; +struct T_FA_NED { + NED ned[2]; +}; Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py =================================================================== --- clang-tools-extra/test/clang-tidy/check_clang_tidy.py +++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py @@ -93,7 +93,7 @@ file_name_with_extension = self.assume_file_name or self.input_file_name _, extension = os.path.splitext(file_name_with_extension) - if extension not in ['.c', '.hpp', '.m', '.mm']: + if extension not in ['.c', '.cu', '.hpp', '.m', '.mm']: extension = '.cpp' self.temp_file_name = self.temp_file_name + extension @@ -115,9 +115,15 @@ self.clang_extra_args = ['-fobjc-abi-version=2', '-fobjc-arc', '-fblocks'] + \ self.clang_extra_args - if extension in ['.cpp', '.hpp', '.mm']: + if extension in ['.cpp', '.cu', '.hpp', '.mm']: self.clang_extra_args.append('-std=' + self.std) + # Tests should not rely on a certain cuda device being available on the machine, + # or a certain version of it + if extension == '.cu': + self.clang_extra_args.extend(["--no-cuda-version-check", "-nocudalib", "-nocudainc"]) + + # Tests should not rely on STL being available, and instead provide mock # implementations of relevant APIs. self.clang_extra_args.append('-nostdinc++') Index: clang-tools-extra/clang-tidy/utils/Matchers.h =================================================================== --- clang-tools-extra/clang-tidy/utils/Matchers.h +++ clang-tools-extra/clang-tidy/utils/Matchers.h @@ -49,6 +49,51 @@ return pointerType(pointee(qualType(isConstQualified()))); } +// Matches the statements in a GNU statement-expression that are not returned +// from it. +AST_MATCHER_P(StmtExpr, hasUnreturning, + clang::ast_matchers::internal::Matcher<Stmt>, matcher) { + const auto compoundStmt = Node.getSubStmt(); + assert(compoundStmt); + + clang::ast_matchers::internal::BoundNodesTreeBuilder result; + bool matched = false; + for (auto stmt = compoundStmt->body_begin(); + stmt + 1 < compoundStmt->body_end(); ++stmt) { + clang::ast_matchers::internal::BoundNodesTreeBuilder builderInner(*Builder); + assert(stmt && *stmt); + if (matcher.matches(**stmt, Finder, &builderInner)) { + result.addMatch(builderInner); + matched = true; + } + } + *Builder = result; + return matched; +} + +// Matches all of the nodes (simmilar to forEach) that match the matcher +// and have return values not used in any statement. +AST_MATCHER_FUNCTION_P(ast_matchers::StatementMatcher, isValueUnused, + ast_matchers::StatementMatcher, Matcher) { + using namespace ast_matchers; + const auto UnusedInCompoundStmt = + compoundStmt(forEach(Matcher), unless(hasParent(stmtExpr()))); + const auto UnusedInGnuExprStmt = stmtExpr(hasUnreturning(Matcher)); + const auto UnusedInIfStmt = + ifStmt(eachOf(hasThen(Matcher), hasElse(Matcher))); + const auto UnusedInWhileStmt = whileStmt(hasBody(Matcher)); + const auto UnusedInDoStmt = doStmt(hasBody(Matcher)); + const auto UnusedInForStmt = forStmt( + eachOf(hasLoopInit(Matcher), hasIncrement(Matcher), hasBody(Matcher))); + const auto UnusedInRangeForStmt = cxxForRangeStmt(hasBody(Matcher)); + const auto UnusedInCaseStmt = switchCase(forEach(Matcher)); + const auto Unused = + stmt(anyOf(UnusedInCompoundStmt, UnusedInGnuExprStmt, UnusedInIfStmt, + UnusedInWhileStmt, UnusedInDoStmt, UnusedInForStmt, + UnusedInRangeForStmt, UnusedInCaseStmt)); + return stmt(eachOf(Unused, forEachDescendant(Unused))); +} + // A matcher implementation that matches a list of type name regular expressions // against a NamedDecl. If a regular expression contains the substring "::" // matching will occur against the qualified name, otherwise only the typename. Index: clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h @@ -0,0 +1,107 @@ +//===--- SlicingCheck.cpp - clang-tidy-------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +#pragma once + +#include "../ClangTidyCheck.h" +#include "llvm/ADT/StringSet.h" +#include <memory> +#include <unordered_set> + +namespace clang { +namespace tidy { +namespace cuda { + +/// Checks for whether the possible errors with the CUDA API invocations have +/// been handled. +/// +/// Calls to CUDA API can sometimes fail to perform the action. This may happen +/// due to a driver malfunction, lack of permissions, lack of a GPU, or a +/// multitude of other reasons. Such errors are returned by those API calls and +/// should be handled in some way. +/// The check provides the following options: +/// - "HandlerName" (optional): +/// specifies the name of the function or the macro to which the return +/// value of the API call should be passed. This effectively automates the +/// process of adding the error checks in question for projects that have +/// such a mechanism implemented in them. +/// - "AcceptedHandlers" (optional): +/// a comma-separated list specifying the only accepted handling +/// functions/macros into which the error from the api call can be passed. +/// If not specified all ways to handle the error that do not just ignore +/// the output value are accepted. The handlers may have scope specifiers +/// included in them, but if so then the full qualified name (with all +/// namespaces explicitly stated) has to be provided (for the performance +/// sake). If the handler set in the "HandlerName" is not in the list of +/// accepted handlers then it gets added to it automatially. +/// +/// Since the behavior of the check is significantly different when the +/// "AcceptedHandlers" option is set, the implementation is essentially split +/// into 2 paths, as highlighted by the comments near declarations. +class UnsafeApiCallCheck : public ClangTidyCheck { + class PPCallbacks; + + // For gathering api calls with an unused value - only those nodes + // can have a FixItHint when we limit the accepted handlers. + // + // Only used when "AcceptedHandlers" is set + class UnusedValueCallback + : public clang::ast_matchers::MatchFinder::MatchCallback { + public: + UnusedValueCallback(UnsafeApiCallCheck *check) : Check(check) {} + void run(const clang::ast_matchers::MatchFinder::MatchResult &Result); + void onStartOfTranslationUnit(); + + private: + UnsafeApiCallCheck *Check; + }; + +public: + UnsafeApiCallCheck(llvm::StringRef Name, + clang::tidy::ClangTidyContext *Context); + + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void registerMatchers(clang::ast_matchers::MatchFinder *Finder) override; + void + check(const clang::ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + const std::string HandlerName; + + // Only used when "AcceptedHandlers" is set + void + checkUnusedValue(const clang::ast_matchers::MatchFinder::MatchResult &Result); + // Only used when "AcceptedHandlers" is not set + void + checkBadHandler(const clang::ast_matchers::MatchFinder::MatchResult &Result); + + // Only used when "AcceptedHandlers" is not set + void registerUnusedValueMatchers(clang::ast_matchers::MatchFinder *Finder); + // Only used when "AcceptedHandlers" is set + void registerBadlyHandledMatchers(clang::ast_matchers::MatchFinder *Finder); + + const std::string AcceptedHandlersList; + const llvm::StringSet<llvm::MallocAllocator> AcceptedHandlersSet; + static llvm::StringSet<llvm::MallocAllocator> + splitAcceptedHandlers(const llvm::StringRef &AcceptedHandlers, + const llvm::StringRef &HandlerName); + bool limitAcceptedHandlers(); + + // Only used when "AcceptedHandlers" is set + std::unordered_set<SourceLocation, + std::function<unsigned(const SourceLocation &)>> + AcceptedHandlerMacroLocations; + std::unordered_set<const Stmt *> UnusedValueNodes; + std::unique_ptr<UnusedValueCallback> UnusedValueCallbackInstance; +}; + +} // namespace cuda +} // namespace tidy +} // namespace clang Index: clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp @@ -0,0 +1,293 @@ +//===--- SlicingCheck.cpp - clang-tidy-------------------------------------===// +// +// 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 "UnsafeApiCallCheck.h" +#include "../utils/Matchers.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Preprocessor.h" +#include "clang/Tooling/FixIt.h" + +#include <functional> +#include <sstream> + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cuda { + +namespace { + +constexpr auto HandlerNameOptionName = "HandlerName"; +constexpr auto AcceptedHandlersOptionName = "AcceptedHandlers"; + +} // namespace + +UnsafeApiCallCheck::UnsafeApiCallCheck(llvm::StringRef Name, + clang::tidy::ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + HandlerName(Options.get(HandlerNameOptionName, "")), + AcceptedHandlersList(Options.get(AcceptedHandlersOptionName, "")), + AcceptedHandlersSet( + splitAcceptedHandlers(AcceptedHandlersList, HandlerName)), + AcceptedHandlerMacroLocations( + 8, [](const SourceLocation &sLoc) { return sLoc.getHashValue(); }) { + if (AcceptedHandlersSet.find("") != AcceptedHandlersSet.end()) { + configurationDiag( + "Empty handler name found in the list of accepted handlers", + DiagnosticIDs::Error); + } +} + +llvm::StringSet<llvm::MallocAllocator> +UnsafeApiCallCheck::splitAcceptedHandlers( + const llvm::StringRef &AcceptedHandlers, + const llvm::StringRef &HandlerName) { + if (AcceptedHandlers.trim().empty()) { + return llvm::StringSet(); + } + llvm::SmallVector<llvm::StringRef> AcceptedHandlersVector; + AcceptedHandlers.split(AcceptedHandlersVector, ','); + + llvm::StringSet AcceptedHandlersSet; + for (auto AcceptedHandler : AcceptedHandlersVector) { + AcceptedHandlersSet.insert(AcceptedHandler.trim()); + } + if (!AcceptedHandlersSet.empty() && !HandlerName.empty()) { + AcceptedHandlersSet.insert(HandlerName); + } + + return AcceptedHandlersSet; +} + +void UnsafeApiCallCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, HandlerNameOptionName, HandlerName); + Options.store(Opts, AcceptedHandlersOptionName, AcceptedHandlersList); +} + +inline bool UnsafeApiCallCheck::limitAcceptedHandlers() { + return !AcceptedHandlersSet.empty(); +} + +// For finding the occurences of accepted handler macros. +class UnsafeApiCallCheck::PPCallbacks : public clang::PPCallbacks { +public: + PPCallbacks(UnsafeApiCallCheck *Check, const SourceManager &SM) + : Check(Check), SM(SM) {} + + void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD, + SourceRange Range, const MacroArgs *Args) { + if (Check->AcceptedHandlersSet.find( + MacroNameTok.getIdentifierInfo()->getName()) != + Check->AcceptedHandlersSet.end()) { + Check->AcceptedHandlerMacroLocations.insert(MacroNameTok.getLocation()); + } + } + +private: + UnsafeApiCallCheck *Check; + const SourceManager &SM; +}; + +void UnsafeApiCallCheck::registerPPCallbacks(const SourceManager &SM, + Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + if (limitAcceptedHandlers()) { + ModuleExpanderPP->addPPCallbacks(std::make_unique<PPCallbacks>(this, SM)); + } +} + +namespace { + +AST_MATCHER_P(Decl, isInSourceFile, std::function<bool(const StringRef &)>, + SourceFileNameCond) { + auto Loc = Node.getLocation(); + const auto &SM = Finder->getASTContext().getSourceManager(); + while (Loc.isValid()) { + if (SourceFileNameCond(SM.getFilename(Loc))) { + return true; + } + Loc = SM.getIncludeLoc(SM.getFileID(Loc)); + } + return false; +} + +AST_MATCHER_P(NamedDecl, hasName, std::function<bool(const StringRef &)>, + DeclNameCond) { + return DeclNameCond(Node.getName()); +} + +AST_MATCHER_P(NamedDecl, hasQualName, std::function<bool(const StringRef &)>, + DeclNameCond) { + return DeclNameCond(Node.getQualifiedNameAsString()); +} + +constexpr auto UnusedValueBinding = "UnusedValueCall"; +constexpr auto badlyHandledBinding = "badlyHandledCall"; + +// Common matchers for both unlimited and limited accepted handlers. +const auto HostFunction = functionDecl(unless(anyOf( + hasAttr(attr::CUDADevice), + hasAttr(attr::CUDAGlobal)))); // Cuda API cannot be called from device code +const auto ApiCallExpression = + callExpr(callee(functionDecl(isInSourceFile([](StringRef FileName) { + return FileName.endswith("cuda_runtime.h"); + }), + returns(asString("cudaError_t"))))); + +} // namespace + +void UnsafeApiCallCheck::UnusedValueCallback::run( + const MatchFinder::MatchResult &Result) { + auto Node = Result.Nodes.getNodeAs<Stmt>(UnusedValueBinding); + assert(Node); + Check->UnusedValueNodes.insert(Node); +} + +void UnsafeApiCallCheck::UnusedValueCallback::onStartOfTranslationUnit() { + Check->UnusedValueNodes.clear(); +} + +void UnsafeApiCallCheck::registerMatchers(MatchFinder *Finder) { + if (limitAcceptedHandlers()) { + registerBadlyHandledMatchers(Finder); + } else { + registerUnusedValueMatchers(Finder); + } +} + +void UnsafeApiCallCheck::registerUnusedValueMatchers(MatchFinder *Finder) { + const auto UnusedValue = + matchers::isValueUnused(stmt(ApiCallExpression.bind(UnusedValueBinding))); + Finder->addMatcher(functionDecl(HostFunction, hasBody(UnusedValue)), this); +} + +void UnsafeApiCallCheck::registerBadlyHandledMatchers(MatchFinder *Finder) { + const auto UnusedValue = + matchers::isValueUnused(stmt(ApiCallExpression.bind(UnusedValueBinding))); + UnusedValueCallbackInstance = std::make_unique<UnusedValueCallback>(this); + Finder->addMatcher(functionDecl(HostFunction, hasBody(UnusedValue)), + UnusedValueCallbackInstance.get()); + + const auto AcceptedHandlerPred = [this](const StringRef &Name) { + return AcceptedHandlersSet.contains(Name); + }; + + const auto AcceptedHandlerDecl = functionDecl( + anyOf(hasName(AcceptedHandlerPred), hasQualName(AcceptedHandlerPred))); + const auto AcceptedHandlerParent = callExpr(callee(AcceptedHandlerDecl)); + + Finder->addMatcher( + functionDecl( + HostFunction, + forEachDescendant(stmt(ApiCallExpression.bind(badlyHandledBinding), + unless(hasParent(AcceptedHandlerParent))))), + this); +} + +namespace { + +constexpr auto HandlerMsg = + "Consider wrapping it with a call to an error handler:"; +constexpr auto NoHandlerMsg = + "Consider adding logic to check if an error has been returned " + "or specify the error handler for this project."; +constexpr auto MacroMsg = + "Consider adding logic to check if an error has been returned."; + +inline bool isStmtInMacro(const Stmt *const Stmt) { + return Stmt->getBeginLoc().isInvalid() || Stmt->getBeginLoc().isMacroID() || + Stmt->getEndLoc().isInvalid() || Stmt->getEndLoc().isMacroID(); +} + +} // namespace + +void UnsafeApiCallCheck::check(const MatchFinder::MatchResult &Result) { + if (limitAcceptedHandlers()) { + checkBadHandler(Result); + } else { + checkUnusedValue(Result); + } +} + +void UnsafeApiCallCheck::checkUnusedValue( + const MatchFinder::MatchResult &Result) { + const auto ApiCallNode = Result.Nodes.getNodeAs<Stmt>(UnusedValueBinding); + assert(ApiCallNode); + + // This disables the check for arguments inside macros, since we assume that + // such a macro is intended as a handler (even if it just passes the argument + // right through) + if (Result.SourceManager->isMacroArgExpansion(ApiCallNode->getBeginLoc())) { + return; + } + + const auto DiagnosticBuilder = + diag(ApiCallNode->getBeginLoc(), "Unchecked CUDA API call. "); + if (HandlerName.empty()) { + DiagnosticBuilder << NoHandlerMsg; + } else if (isStmtInMacro(ApiCallNode)) { + DiagnosticBuilder << MacroMsg; + } else { + DiagnosticBuilder << HandlerMsg + << FixItHint::CreateReplacement( + ApiCallNode->getSourceRange(), + (HandlerName + "(" + + tooling::fixit::getText( + ApiCallNode->getSourceRange(), + *Result.Context) + + ")") + .str()); + } +} + +void UnsafeApiCallCheck::checkBadHandler( + const MatchFinder::MatchResult &Result) { + const auto ApiCallNode = Result.Nodes.getNodeAs<Stmt>(badlyHandledBinding); + assert(ApiCallNode); + + // The 0 offset is to strip the spelling info + const auto ApiCallNodeMacroLocation = Result.SourceManager->getExpansionLoc( + Result.SourceManager->getMacroArgExpandedLocation( + ApiCallNode->getBeginLoc())); + + // This disables the check for arguments inside macros, since we assume that + // such a macro is intended as a handler (even if it just passes the argument + // right through) + if (Result.SourceManager->isMacroArgExpansion(ApiCallNode->getBeginLoc()) && + AcceptedHandlerMacroLocations.find(ApiCallNodeMacroLocation) != + AcceptedHandlerMacroLocations.end()) { + return; + } + + const auto DiagnosticBuilder = + diag(ApiCallNode->getBeginLoc(), "CUDA API call not checked properly. "); + + if (HandlerName.empty()) { + DiagnosticBuilder << NoHandlerMsg; + } else if (isStmtInMacro(ApiCallNode) || + UnusedValueNodes.find(ApiCallNode) == UnusedValueNodes.end()) { + DiagnosticBuilder << "Consider wrapping it with a call to `" << HandlerName + << '`'; + } else { + DiagnosticBuilder << HandlerMsg + << FixItHint::CreateReplacement( + ApiCallNode->getSourceRange(), + (HandlerName + "(" + + tooling::fixit::getText( + ApiCallNode->getSourceRange(), + *Result.Context) + + ")") + .str()); + } +} + +} // namespace cuda +} // namespace tidy +} // namespace clang Index: clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp @@ -0,0 +1,38 @@ +//===--- GoogleTidyModule.cpp - clang-tidy --------------------------------===// +// +// 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 "../ClangTidy.h" +#include "../ClangTidyModule.h" +#include "../ClangTidyModuleRegistry.h" +#include "UnsafeApiCallCheck.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cuda { + +class CudaModule : public ClangTidyModule { + public: + void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck<UnsafeApiCallCheck>("cuda-unsafe-api-call"); + } +}; + +// Register the GoogleTidyModule using this statically initialized variable. +static ClangTidyModuleRegistry::Add<CudaModule> X("cuda-module", + "Adds Cuda-related lint checks."); + +} // namespace google + +// This anchor is used to force the linker to link in the generated object file +// and thus register the GoogleModule. +volatile int CudaModuleAnchorSource = 0; + +} // namespace tidy +} // namespace clang Index: clang-tools-extra/clang-tidy/cuda/CMakeLists.txt =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/cuda/CMakeLists.txt @@ -0,0 +1,16 @@ +add_clang_library(clangTidyCudaModule + CudaTidyModule.cpp + UnsafeApiCallCheck.cpp + LINK_LIBS + clangTidy + clangTidyUtils + ) + +clang_target_link_libraries(clangTidyAlteraModule + PRIVATE + clangAnalysis + clangAST + clangASTMatchers + clangBasic + clangLex + ) Index: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp +++ clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "UnusedReturnValueCheck.h" +#include "../utils/Matchers.h" #include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" @@ -159,10 +160,7 @@ auto UnusedInCaseStmt = switchCase(forEach(MatchedCallExpr)); Finder->addMatcher( - stmt(anyOf(UnusedInCompoundStmt, UnusedInIfStmt, UnusedInWhileStmt, - UnusedInDoStmt, UnusedInForStmt, UnusedInRangeForStmt, - UnusedInCaseStmt)), - this); + functionDecl(hasBody(matchers::isValueUnused(MatchedCallExpr))), this); } void UnusedReturnValueCheck::check(const MatchFinder::MatchResult &Result) { Index: clang-tools-extra/clang-tidy/ClangTidyForceLinker.h =================================================================== --- clang-tools-extra/clang-tidy/ClangTidyForceLinker.h +++ clang-tools-extra/clang-tidy/ClangTidyForceLinker.h @@ -55,6 +55,11 @@ static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination = CppCoreGuidelinesModuleAnchorSource; +// This anchor is used to force the linker to link the CudaModule. +extern volatile int CudaModuleAnchorSource; +static int LLVM_ATTRIBUTE_UNUSED CudaModuleAnchorDestination = + CudaModuleAnchorSource; + // This anchor is used to force the linker to link the DarwinModule. extern volatile int DarwinModuleAnchorSource; static int LLVM_ATTRIBUTE_UNUSED DarwinModuleAnchorDestination = Index: clang-tools-extra/clang-tidy/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/CMakeLists.txt +++ clang-tools-extra/clang-tidy/CMakeLists.txt @@ -58,6 +58,7 @@ add_subdirectory(cert) add_subdirectory(concurrency) add_subdirectory(cppcoreguidelines) +add_subdirectory(cuda) add_subdirectory(darwin) add_subdirectory(fuchsia) add_subdirectory(google) @@ -85,6 +86,7 @@ clangTidyCERTModule clangTidyConcurrencyModule clangTidyCppCoreGuidelinesModule + clangTidyCudaModule clangTidyDarwinModule clangTidyFuchsiaModule clangTidyGoogleModule
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
