diff options
Diffstat (limited to 'clang-tools-extra/clang-tidy')
11 files changed, 223 insertions, 139 deletions
diff --git a/clang-tools-extra/clang-tidy/CMakeLists.txt b/clang-tools-extra/clang-tidy/CMakeLists.txt index 57ec851085e..9d6fc7f035a 100644 --- a/clang-tools-extra/clang-tidy/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/CMakeLists.txt @@ -28,6 +28,7 @@ add_clang_library(clangTidy add_subdirectory(android) add_subdirectory(boost) +add_subdirectory(bugprone) add_subdirectory(cert) add_subdirectory(cppcoreguidelines) add_subdirectory(google) diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp new file mode 100644 index 00000000000..65353a23c7a --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -0,0 +1,38 @@ +//===--- BugproneTidyModule.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 "../ClangTidy.h" +#include "../ClangTidyModule.h" +#include "../ClangTidyModuleRegistry.h" +#include "SuspiciousMemsetUsageCheck.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +class BugproneModule : public ClangTidyModule { +public: + void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>( + "bugprone-suspicious-memset-usage"); + } +}; + +} // namespace bugprone + +// Register the BugproneTidyModule using this statically initialized variable. +static ClangTidyModuleRegistry::Add<bugprone::BugproneModule> + X("bugprone-module", "Adds checks for bugprone code constructs."); + +// This anchor is used to force the linker to link in the generated object file +// and thus register the BugproneModule. +volatile int BugproneModuleAnchorSource = 0; + +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt new file mode 100644 index 00000000000..1f0ec6e0427 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -0,0 +1,16 @@ +set(LLVM_LINK_COMPONENTS support) + +add_clang_library(clangTidyBugproneModule + BugproneTidyModule.cpp + SuspiciousMemsetUsageCheck.cpp + + LINK_LIBS + clangAnalysis + clangAST + clangASTMatchers + clangBasic + clangLex + clangTidy + clangTidyUtils + clangTooling + ) diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp new file mode 100644 index 00000000000..8e11a435174 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp @@ -0,0 +1,127 @@ +//===--- SuspiciousMemsetUsageCheck.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 "SuspiciousMemsetUsageCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Lex/Lexer.h" +#include "clang/Tooling/FixIt.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +void SuspiciousMemsetUsageCheck::registerMatchers(MatchFinder *Finder) { + // Note: void *memset(void *buffer, int fill_char, size_t byte_count); + // Look for memset(x, '0', z). Probably memset(x, 0, z) was intended. + Finder->addMatcher( + callExpr( + callee(functionDecl(hasName("::memset"))), + hasArgument(1, characterLiteral(equals(static_cast<unsigned>('0'))) + .bind("char-zero-fill")), + unless( + eachOf(hasArgument(0, anyOf(hasType(pointsTo(isAnyCharacter())), + hasType(arrayType(hasElementType( + isAnyCharacter()))))), + isInTemplateInstantiation()))), + this); + + // Look for memset with an integer literal in its fill_char argument. + // Will check if it gets truncated. + Finder->addMatcher(callExpr(callee(functionDecl(hasName("::memset"))), + hasArgument(1, integerLiteral().bind("num-fill")), + unless(isInTemplateInstantiation())), + this); + + // Look for memset(x, y, 0) as that is most likely an argument swap. + Finder->addMatcher( + callExpr(callee(functionDecl(hasName("::memset"))), + unless(hasArgument(1, anyOf(characterLiteral(equals( + static_cast<unsigned>('0'))), + integerLiteral()))), + unless(isInTemplateInstantiation())) + .bind("call"), + this); +} + +void SuspiciousMemsetUsageCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *CharZeroFill = + Result.Nodes.getNodeAs<CharacterLiteral>("char-zero-fill")) { + // Case 1: fill_char of memset() is a character '0'. Probably an + // integer zero was intended. + + SourceRange CharRange = CharZeroFill->getSourceRange(); + auto Diag = + diag(CharZeroFill->getLocStart(), "memset fill value is char '0', " + "potentially mistaken for int 0"); + + // Only suggest a fix if no macros are involved. + if (CharRange.getBegin().isMacroID()) + return; + Diag << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(CharRange), "0"); + } + + else if (const auto *NumFill = + Result.Nodes.getNodeAs<IntegerLiteral>("num-fill")) { + // Case 2: fill_char of memset() is larger in size than an unsigned char + // so it gets truncated during conversion. + + llvm::APSInt NumValue; + const auto UCharMax = (1 << Result.Context->getCharWidth()) - 1; + if (!NumFill->EvaluateAsInt(NumValue, *Result.Context) || + (NumValue >= 0 && NumValue <= UCharMax)) + return; + + diag(NumFill->getLocStart(), "memset fill value is out of unsigned " + "character range, gets truncated"); + } + + else if (const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call")) { + // Case 3: byte_count of memset() is zero. This is most likely an + // argument swap. + + const Expr *FillChar = Call->getArg(1); + const Expr *ByteCount = Call->getArg(2); + + // Return if `byte_count` is not zero at compile time. + llvm::APSInt Value1, Value2; + if (ByteCount->isValueDependent() || + !ByteCount->EvaluateAsInt(Value2, *Result.Context) || Value2 != 0) + return; + + // Return if `fill_char` is known to be zero or negative at compile + // time. In these cases, swapping the args would be a nop, or + // introduce a definite bug. The code is likely correct. + if (!FillChar->isValueDependent() && + FillChar->EvaluateAsInt(Value1, *Result.Context) && + (Value1 == 0 || Value1.isNegative())) + return; + + // `byte_count` is known to be zero at compile time, and `fill_char` is + // either not known or known to be a positive integer. Emit a warning + // and fix-its to swap the arguments. + auto D = diag(Call->getLocStart(), + "memset of size zero, potentially swapped arguments"); + StringRef RHSString = tooling::fixit::getText(*ByteCount, *Result.Context); + StringRef LHSString = tooling::fixit::getText(*FillChar, *Result.Context); + if (LHSString.empty() || RHSString.empty()) + return; + + D << tooling::fixit::createReplacement(*FillChar, RHSString) + << tooling::fixit::createReplacement(*ByteCount, LHSString); + } +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.h b/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.h new file mode 100644 index 00000000000..1c0d1bcf3e9 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.h @@ -0,0 +1,35 @@ +//===--- SuspiciousMemsetUsageCheck.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_BUGPRONE_SUSPICIOUS_MEMSET_USAGE_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUS_MEMSET_USAGE_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Finds memset calls with potential mistakes in their arguments. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-memset-usage.html +class SuspiciousMemsetUsageCheck : public ClangTidyCheck { +public: + SuspiciousMemsetUsageCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUS_MEMSET_USAGE_H diff --git a/clang-tools-extra/clang-tidy/google/CMakeLists.txt b/clang-tools-extra/clang-tidy/google/CMakeLists.txt index efe3b3cde9c..ff48b63d03d 100644 --- a/clang-tools-extra/clang-tidy/google/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/google/CMakeLists.txt @@ -8,7 +8,6 @@ add_clang_library(clangTidyGoogleModule GlobalNamesInHeadersCheck.cpp GoogleTidyModule.cpp IntegerTypesCheck.cpp - MemsetZeroLengthCheck.cpp NonConstReferences.cpp OverloadedUnaryAndCheck.cpp StringReferenceMemberCheck.cpp diff --git a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp index e9b4bedf011..1328463e356 100644 --- a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp @@ -20,7 +20,6 @@ #include "ExplicitMakePairCheck.h" #include "GlobalNamesInHeadersCheck.h" #include "IntegerTypesCheck.h" -#include "MemsetZeroLengthCheck.h" #include "NonConstReferences.h" #include "OverloadedUnaryAndCheck.h" #include "StringReferenceMemberCheck.h" @@ -55,8 +54,6 @@ public: "google-runtime-references"); CheckFactories.registerCheck<runtime::StringReferenceMemberCheck>( "google-runtime-member-string-references"); - CheckFactories.registerCheck<runtime::MemsetZeroLengthCheck>( - "google-runtime-memset"); CheckFactories.registerCheck<readability::AvoidCStyleCastsCheck>( "google-readability-casting"); CheckFactories.registerCheck<readability::TodoCommentCheck>( diff --git a/clang-tools-extra/clang-tidy/google/MemsetZeroLengthCheck.cpp b/clang-tools-extra/clang-tidy/google/MemsetZeroLengthCheck.cpp deleted file mode 100644 index fa87fab204d..00000000000 --- a/clang-tools-extra/clang-tidy/google/MemsetZeroLengthCheck.cpp +++ /dev/null @@ -1,96 +0,0 @@ -//===--- MemsetZeroLengthCheck.cpp - clang-tidy -------------------*- C++ -*-===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -#include "MemsetZeroLengthCheck.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/ASTMatchers/ASTMatchers.h" -#include "clang/Lex/Lexer.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace google { -namespace runtime { - -void MemsetZeroLengthCheck::registerMatchers( - ast_matchers::MatchFinder *Finder) { - // Look for memset(x, y, 0) as those is most likely an argument swap. - // TODO: Also handle other standard functions that suffer from the same - // problem, e.g. memchr. - Finder->addMatcher(callExpr(callee(functionDecl(hasName("::memset"))), - argumentCountIs(3), - unless(isInTemplateInstantiation())) - .bind("decl"), - this); -} - -/// \brief Get a StringRef representing a SourceRange. -static StringRef getAsString(const MatchFinder::MatchResult &Result, - SourceRange R) { - const SourceManager &SM = *Result.SourceManager; - // Don't even try to resolve macro or include contraptions. Not worth emitting - // a fixit for. - if (R.getBegin().isMacroID() || - !SM.isWrittenInSameFile(R.getBegin(), R.getEnd())) - return StringRef(); - - const char *Begin = SM.getCharacterData(R.getBegin()); - const char *End = SM.getCharacterData(Lexer::getLocForEndOfToken( - R.getEnd(), 0, SM, Result.Context->getLangOpts())); - - return StringRef(Begin, End - Begin); -} - -void MemsetZeroLengthCheck::check(const MatchFinder::MatchResult &Result) { - const auto *Call = Result.Nodes.getNodeAs<CallExpr>("decl"); - - // Note, this is: - // void *memset(void *buffer, int fill_char, size_t byte_count); - // Arg1 is fill_char, Arg2 is byte_count. - const Expr *Arg1 = Call->getArg(1); - const Expr *Arg2 = Call->getArg(2); - - // Return if `byte_count` is not zero at compile time. - llvm::APSInt Value1, Value2; - if (Arg2->isValueDependent() || - !Arg2->EvaluateAsInt(Value2, *Result.Context) || Value2 != 0) - return; - - // Return if `fill_char` is known to be zero or negative at compile - // time. In these cases, swapping the args would be a nop, or - // introduce a definite bug. The code is likely correct. - if (!Arg1->isValueDependent() && - Arg1->EvaluateAsInt(Value1, *Result.Context) && - (Value1 == 0 || Value1.isNegative())) - return; - - // `byte_count` is known to be zero at compile time, and `fill_char` is - // either not known or known to be a positive integer. Emit a warning - // and fix-its to swap the arguments. - auto D = diag(Call->getLocStart(), - "memset of size zero, potentially swapped arguments"); - SourceRange LHSRange = Arg1->getSourceRange(); - SourceRange RHSRange = Arg2->getSourceRange(); - StringRef RHSString = getAsString(Result, RHSRange); - StringRef LHSString = getAsString(Result, LHSRange); - if (LHSString.empty() || RHSString.empty()) - return; - - D << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(LHSRange), - RHSString) - << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(RHSRange), - LHSString); -} - -} // namespace runtime -} // namespace google -} // namespace tidy -} // namespace clang diff --git a/clang-tools-extra/clang-tidy/google/MemsetZeroLengthCheck.h b/clang-tools-extra/clang-tidy/google/MemsetZeroLengthCheck.h deleted file mode 100644 index 57c7aec746b..00000000000 --- a/clang-tools-extra/clang-tidy/google/MemsetZeroLengthCheck.h +++ /dev/null @@ -1,39 +0,0 @@ -//===--- MemsetZeroLengthCheck.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_GOOGLE_MEMSETZEROLENGTHCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_MEMSETZEROLENGTHCHECK_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace google { -namespace runtime { - -/// Finds calls to memset with a literal zero in the length argument. -/// -/// This is most likely unintended and the length and value arguments are -/// swapped. -/// -/// Corresponding cpplint.py check name: 'runtime/memset'. -class MemsetZeroLengthCheck : public ClangTidyCheck { -public: - MemsetZeroLengthCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; -}; - -} // namespace runtime -} // namespace google -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_MEMSETZEROLENGTHCHECK_H diff --git a/clang-tools-extra/clang-tidy/tool/CMakeLists.txt b/clang-tools-extra/clang-tidy/tool/CMakeLists.txt index 5b952c4f225..b8c34f36750 100644 --- a/clang-tools-extra/clang-tidy/tool/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/tool/CMakeLists.txt @@ -15,6 +15,7 @@ target_link_libraries(clang-tidy clangTidy clangTidyAndroidModule clangTidyBoostModule + clangTidyBugproneModule clangTidyCERTModule clangTidyCppCoreGuidelinesModule clangTidyGoogleModule diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp index bfc995ac68f..803dca17122 100644 --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -462,6 +462,11 @@ extern volatile int BoostModuleAnchorSource; static int LLVM_ATTRIBUTE_UNUSED BoostModuleAnchorDestination = BoostModuleAnchorSource; +// This anchor is used to force the linker to link the BugproneModule. +extern volatile int BugproneModuleAnchorSource; +static int LLVM_ATTRIBUTE_UNUSED BugproneModuleAnchorDestination = + BugproneModuleAnchorSource; + // This anchor is used to force the linker to link the LLVMModule. extern volatile int LLVMModuleAnchorSource; static int LLVM_ATTRIBUTE_UNUSED LLVMModuleAnchorDestination = |
