From a3251bf24c8bc0ebe8fb09e0d59dbc70fa706859 Mon Sep 17 00:00:00 2001 From: Alexander Kornienko Date: Thu, 23 Nov 2017 13:49:14 +0000 Subject: [clang-tidy] rename_check.py misc-string-constructor bugprone-string-constructor Summary: Rename misc-string-constructor to bugprone-string-constructor + manually update the lenght of '==='s in the doc file. Reviewers: hokein, xazax.hun Reviewed By: hokein, xazax.hun Subscribers: mgorny, xazax.hun, cfe-commits Differential Revision: https://reviews.llvm.org/D40388 llvm-svn: 318916 --- .../clang-tidy/bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../clang-tidy/bugprone/StringConstructorCheck.cpp | 134 +++++++++++++++++++++ .../clang-tidy/bugprone/StringConstructorCheck.h | 39 ++++++ 4 files changed, 177 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h (limited to 'clang-tools-extra/clang-tidy/bugprone') diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index a5b3d640b20..1719500fb86 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -13,6 +13,7 @@ #include "CopyConstructorInitCheck.h" #include "IntegerDivisionCheck.h" #include "MisplacedOperatorInStrlenInAllocCheck.h" +#include "StringConstructorCheck.h" #include "SuspiciousMemsetUsageCheck.h" #include "UndefinedMemoryManipulationCheck.h" @@ -29,6 +30,8 @@ public: "bugprone-integer-division"); CheckFactories.registerCheck( "bugprone-misplaced-operator-in-strlen-in-alloc"); + CheckFactories.registerCheck( + "bugprone-string-constructor"); CheckFactories.registerCheck( "bugprone-suspicious-memset-usage"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 46101c6037e..db724ea1f0a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -5,6 +5,7 @@ add_clang_library(clangTidyBugproneModule CopyConstructorInitCheck.cpp IntegerDivisionCheck.cpp MisplacedOperatorInStrlenInAllocCheck.cpp + StringConstructorCheck.cpp SuspiciousMemsetUsageCheck.cpp UndefinedMemoryManipulationCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp new file mode 100644 index 00000000000..5607ce6a487 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp @@ -0,0 +1,134 @@ +//===--- StringConstructorCheck.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 "StringConstructorCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Tooling/FixIt.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +AST_MATCHER_P(IntegerLiteral, isBiggerThan, unsigned, N) { + return Node.getValue().getZExtValue() > N; +} + +StringConstructorCheck::StringConstructorCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + WarnOnLargeLength(Options.get("WarnOnLargeLength", 1) != 0), + LargeLengthThreshold(Options.get("LargeLengthThreshold", 0x800000)) {} + +void StringConstructorCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "WarnOnLargeLength", WarnOnLargeLength); + Options.store(Opts, "LargeLengthThreshold", LargeLengthThreshold); +} + +void StringConstructorCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) + return; + + const auto ZeroExpr = expr(ignoringParenImpCasts(integerLiteral(equals(0)))); + const auto CharExpr = expr(ignoringParenImpCasts(characterLiteral())); + const auto NegativeExpr = expr(ignoringParenImpCasts( + unaryOperator(hasOperatorName("-"), + hasUnaryOperand(integerLiteral(unless(equals(0))))))); + const auto LargeLengthExpr = expr(ignoringParenImpCasts( + integerLiteral(isBiggerThan(LargeLengthThreshold)))); + const auto CharPtrType = type(anyOf(pointerType(), arrayType())); + + // Match a string-literal; even through a declaration with initializer. + const auto BoundStringLiteral = stringLiteral().bind("str"); + const auto ConstStrLiteralDecl = varDecl( + isDefinition(), hasType(constantArrayType()), hasType(isConstQualified()), + hasInitializer(ignoringParenImpCasts(BoundStringLiteral))); + const auto ConstPtrStrLiteralDecl = varDecl( + isDefinition(), + hasType(pointerType(pointee(isAnyCharacter(), isConstQualified()))), + hasInitializer(ignoringParenImpCasts(BoundStringLiteral))); + const auto ConstStrLiteral = expr(ignoringParenImpCasts(anyOf( + BoundStringLiteral, declRefExpr(hasDeclaration(anyOf( + ConstPtrStrLiteralDecl, ConstStrLiteralDecl)))))); + + // Check the fill constructor. Fills the string with n consecutive copies of + // character c. [i.e string(size_t n, char c);]. + Finder->addMatcher( + cxxConstructExpr( + hasDeclaration(cxxMethodDecl(hasName("basic_string"))), + hasArgument(0, hasType(qualType(isInteger()))), + hasArgument(1, hasType(qualType(isInteger()))), + anyOf( + // Detect the expression: string('x', 40); + hasArgument(0, CharExpr.bind("swapped-parameter")), + // Detect the expression: string(0, ...); + hasArgument(0, ZeroExpr.bind("empty-string")), + // Detect the expression: string(-4, ...); + hasArgument(0, NegativeExpr.bind("negative-length")), + // Detect the expression: string(0x1234567, ...); + hasArgument(0, LargeLengthExpr.bind("large-length")))) + .bind("constructor"), + this); + + // Check the literal string constructor with char pointer and length + // parameters. [i.e. string (const char* s, size_t n);] + Finder->addMatcher( + cxxConstructExpr( + hasDeclaration(cxxMethodDecl(hasName("basic_string"))), + hasArgument(0, hasType(CharPtrType)), + hasArgument(1, hasType(isInteger())), + anyOf( + // Detect the expression: string("...", 0); + hasArgument(1, ZeroExpr.bind("empty-string")), + // Detect the expression: string("...", -4); + hasArgument(1, NegativeExpr.bind("negative-length")), + // Detect the expression: string("lit", 0x1234567); + hasArgument(1, LargeLengthExpr.bind("large-length")), + // Detect the expression: string("lit", 5) + allOf(hasArgument(0, ConstStrLiteral.bind("literal-with-length")), + hasArgument(1, ignoringParenImpCasts( + integerLiteral().bind("int")))))) + .bind("constructor"), + this); +} + +void StringConstructorCheck::check(const MatchFinder::MatchResult &Result) { + const ASTContext &Ctx = *Result.Context; + const auto *E = Result.Nodes.getNodeAs("constructor"); + assert(E && "missing constructor expression"); + SourceLocation Loc = E->getLocStart(); + + if (Result.Nodes.getNodeAs("swapped-parameter")) { + const Expr *P0 = E->getArg(0); + const Expr *P1 = E->getArg(1); + diag(Loc, "string constructor parameters are probably swapped;" + " expecting string(count, character)") + << tooling::fixit::createReplacement(*P0, *P1, Ctx) + << tooling::fixit::createReplacement(*P1, *P0, Ctx); + } else if (Result.Nodes.getNodeAs("empty-string")) { + diag(Loc, "constructor creating an empty string"); + } else if (Result.Nodes.getNodeAs("negative-length")) { + diag(Loc, "negative value used as length parameter"); + } else if (Result.Nodes.getNodeAs("large-length")) { + if (WarnOnLargeLength) + diag(Loc, "suspicious large length parameter"); + } else if (Result.Nodes.getNodeAs("literal-with-length")) { + const auto *Str = Result.Nodes.getNodeAs("str"); + const auto *Lit = Result.Nodes.getNodeAs("int"); + if (Lit->getValue().ugt(Str->getLength())) { + diag(Loc, "length is bigger then string literal size"); + } + } +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h new file mode 100644 index 00000000000..52e9791fd9a --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h @@ -0,0 +1,39 @@ +//===--- StringConstructorCheck.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_STRING_CONSTRUCTOR_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_STRING_CONSTRUCTOR_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Finds suspicious string constructor and check their parameters. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-string-constructor.html +class StringConstructorCheck : public ClangTidyCheck { +public: + StringConstructorCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool WarnOnLargeLength; + const unsigned int LargeLengthThreshold; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_STRING_CONSTRUCTOR_H -- cgit v1.2.3