[clang-tidy] New checker to detect suspicious string constructor.
Summary: Checker to validate string constructor parameters. A common mistake is to swap parameter for the fill-constructor. ``` std::string str('x', 4); std::string str('4', x); ``` Reviewers: alexfh Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D19146 llvm-svn: 267011
This commit is contained in:
parent
a7a55c4d27
commit
1dbd582387
|
@ -25,6 +25,7 @@ add_clang_library(clangTidyMiscModule
|
||||||
SizeofContainerCheck.cpp
|
SizeofContainerCheck.cpp
|
||||||
SizeofExpressionCheck.cpp
|
SizeofExpressionCheck.cpp
|
||||||
StaticAssertCheck.cpp
|
StaticAssertCheck.cpp
|
||||||
|
StringConstructorCheck.cpp
|
||||||
StringIntegerAssignmentCheck.cpp
|
StringIntegerAssignmentCheck.cpp
|
||||||
StringLiteralWithEmbeddedNulCheck.cpp
|
StringLiteralWithEmbeddedNulCheck.cpp
|
||||||
SuspiciousMissingCommaCheck.cpp
|
SuspiciousMissingCommaCheck.cpp
|
||||||
|
|
|
@ -33,6 +33,7 @@
|
||||||
#include "SizeofContainerCheck.h"
|
#include "SizeofContainerCheck.h"
|
||||||
#include "SizeofExpressionCheck.h"
|
#include "SizeofExpressionCheck.h"
|
||||||
#include "StaticAssertCheck.h"
|
#include "StaticAssertCheck.h"
|
||||||
|
#include "StringConstructorCheck.h"
|
||||||
#include "StringIntegerAssignmentCheck.h"
|
#include "StringIntegerAssignmentCheck.h"
|
||||||
#include "StringLiteralWithEmbeddedNulCheck.h"
|
#include "StringLiteralWithEmbeddedNulCheck.h"
|
||||||
#include "SuspiciousMissingCommaCheck.h"
|
#include "SuspiciousMissingCommaCheck.h"
|
||||||
|
@ -99,6 +100,8 @@ public:
|
||||||
"misc-sizeof-expression");
|
"misc-sizeof-expression");
|
||||||
CheckFactories.registerCheck<StaticAssertCheck>(
|
CheckFactories.registerCheck<StaticAssertCheck>(
|
||||||
"misc-static-assert");
|
"misc-static-assert");
|
||||||
|
CheckFactories.registerCheck<StringConstructorCheck>(
|
||||||
|
"misc-string-constructor");
|
||||||
CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
|
CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
|
||||||
"misc-string-integer-assignment");
|
"misc-string-integer-assignment");
|
||||||
CheckFactories.registerCheck<StringLiteralWithEmbeddedNulCheck>(
|
CheckFactories.registerCheck<StringLiteralWithEmbeddedNulCheck>(
|
||||||
|
|
|
@ -0,0 +1,126 @@
|
||||||
|
//===--- 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"
|
||||||
|
|
||||||
|
using namespace clang::ast_matchers;
|
||||||
|
|
||||||
|
namespace clang {
|
||||||
|
namespace tidy {
|
||||||
|
namespace misc {
|
||||||
|
|
||||||
|
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)));
|
||||||
|
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 auto *E = Result.Nodes.getNodeAs<Expr>("constructor");
|
||||||
|
SourceLocation Loc = E->getLocStart();
|
||||||
|
|
||||||
|
if (Result.Nodes.getNodeAs<Expr>("swapped-parameter")) {
|
||||||
|
diag(Loc, "constructor parameters are probably swapped");
|
||||||
|
} else if (Result.Nodes.getNodeAs<Expr>("empty-string")) {
|
||||||
|
diag(Loc, "constructor creating an empty string");
|
||||||
|
} else if (Result.Nodes.getNodeAs<Expr>("negative-length")) {
|
||||||
|
diag(Loc, "negative value used as length parameter");
|
||||||
|
} else if (Result.Nodes.getNodeAs<Expr>("large-length")) {
|
||||||
|
if (WarnOnLargeLength)
|
||||||
|
diag(Loc, "suspicious large length parameter");
|
||||||
|
} else if (Result.Nodes.getNodeAs<Expr>("literal-with-length")) {
|
||||||
|
const auto *Str = Result.Nodes.getNodeAs<StringLiteral>("str");
|
||||||
|
const auto *Lit = Result.Nodes.getNodeAs<IntegerLiteral>("int");
|
||||||
|
if (Lit->getValue().ugt(Str->getLength())) {
|
||||||
|
diag(Loc, "length is bigger then string literal size");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
} // namespace misc
|
||||||
|
} // namespace tidy
|
||||||
|
} // namespace clang
|
|
@ -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_MISC_STRING_CONSTRUCTOR_H
|
||||||
|
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_CONSTRUCTOR_H
|
||||||
|
|
||||||
|
#include "../ClangTidy.h"
|
||||||
|
|
||||||
|
namespace clang {
|
||||||
|
namespace tidy {
|
||||||
|
namespace misc {
|
||||||
|
|
||||||
|
/// Finds suspicious string constructor and check their parameters.
|
||||||
|
///
|
||||||
|
/// For the user-facing documentation see:
|
||||||
|
/// http://clang.llvm.org/extra/clang-tidy/checks/misc-string-constructor.html
|
||||||
|
class StringConstructorCheck : public ClangTidyCheck {
|
||||||
|
public:
|
||||||
|
StringConstructorCheck(StringRef Name, ClangTidyContext *Context);
|
||||||
|
void storeOptions(ClangTidyOptions::OptionMap &Opts);
|
||||||
|
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 misc
|
||||||
|
} // namespace tidy
|
||||||
|
} // namespace clang
|
||||||
|
|
||||||
|
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_CONSTRUCTOR_H
|
|
@ -69,6 +69,7 @@ Clang-Tidy Checks
|
||||||
misc-sizeof-container
|
misc-sizeof-container
|
||||||
misc-sizeof-expression
|
misc-sizeof-expression
|
||||||
misc-static-assert
|
misc-static-assert
|
||||||
|
misc-string-constructor
|
||||||
misc-string-integer-assignment
|
misc-string-integer-assignment
|
||||||
misc-string-literal-with-embedded-nul
|
misc-string-literal-with-embedded-nul
|
||||||
misc-suspicious-missing-comma
|
misc-suspicious-missing-comma
|
||||||
|
|
|
@ -0,0 +1,36 @@
|
||||||
|
.. title:: clang-tidy - misc-string-constructor
|
||||||
|
|
||||||
|
misc-string-constructor
|
||||||
|
=======================
|
||||||
|
|
||||||
|
Finds string constructors that are suspicious and probably errors.
|
||||||
|
|
||||||
|
|
||||||
|
A common mistake is to swap parameters to the 'fill' string-constructor.
|
||||||
|
|
||||||
|
Examples:
|
||||||
|
|
||||||
|
.. code:: c++
|
||||||
|
|
||||||
|
std::string('x', 50) str; // should be std::string(50, 'x')
|
||||||
|
|
||||||
|
|
||||||
|
Calling the string-literal constructor with a length bigger than the literal is
|
||||||
|
suspicious and adds extra random characters to the string.
|
||||||
|
|
||||||
|
Examples:
|
||||||
|
|
||||||
|
.. code:: c++
|
||||||
|
|
||||||
|
std::string("test", 200); // Will include random characters after "test".
|
||||||
|
|
||||||
|
|
||||||
|
Creating an empty string from constructors with parameters is considered
|
||||||
|
suspicious. The programmer should use the empty constructor instead.
|
||||||
|
|
||||||
|
Examples:
|
||||||
|
|
||||||
|
.. code:: c++
|
||||||
|
|
||||||
|
std::string("test", 0); // Creation of an empty string.
|
||||||
|
|
|
@ -0,0 +1,54 @@
|
||||||
|
// RUN: %check_clang_tidy %s misc-string-constructor %t
|
||||||
|
|
||||||
|
namespace std {
|
||||||
|
template <typename T>
|
||||||
|
class allocator {};
|
||||||
|
template <typename T>
|
||||||
|
class char_traits {};
|
||||||
|
template <typename C, typename T = std::char_traits<C>, typename A = std::allocator<C> >
|
||||||
|
struct basic_string {
|
||||||
|
basic_string();
|
||||||
|
basic_string(const C*, unsigned int size);
|
||||||
|
basic_string(unsigned int size, C c);
|
||||||
|
};
|
||||||
|
typedef basic_string<char> string;
|
||||||
|
typedef basic_string<wchar_t> wstring;
|
||||||
|
}
|
||||||
|
|
||||||
|
const char* kText = "";
|
||||||
|
const char kText2[] = "";
|
||||||
|
extern const char kText3[];
|
||||||
|
|
||||||
|
void Test() {
|
||||||
|
std::string str('x', 4);
|
||||||
|
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor parameters are probably swapped [misc-string-constructor]
|
||||||
|
std::wstring wstr(L'x', 4);
|
||||||
|
// CHECK-MESSAGES: [[@LINE-1]]:16: warning: constructor parameters are probably swapped
|
||||||
|
std::string s0(0, 'x');
|
||||||
|
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor creating an empty string
|
||||||
|
std::string s1(-4, 'x');
|
||||||
|
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: negative value used as length parameter
|
||||||
|
std::string s2(0x1000000, 'x');
|
||||||
|
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: suspicious large length parameter
|
||||||
|
|
||||||
|
std::string q0("test", 0);
|
||||||
|
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor creating an empty string
|
||||||
|
std::string q1(kText, -4);
|
||||||
|
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: negative value used as length parameter
|
||||||
|
std::string q2("test", 200);
|
||||||
|
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger then string literal size
|
||||||
|
std::string q3(kText, 200);
|
||||||
|
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger then string literal size
|
||||||
|
std::string q4(kText2, 200);
|
||||||
|
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger then string literal size
|
||||||
|
std::string q5(kText3, 0x1000000);
|
||||||
|
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: suspicious large length parameter
|
||||||
|
}
|
||||||
|
|
||||||
|
void Valid() {
|
||||||
|
std::string empty();
|
||||||
|
std::string str(4, 'x');
|
||||||
|
std::wstring wstr(4, L'x');
|
||||||
|
std::string s1("test", 4);
|
||||||
|
std::string s2("test", 3);
|
||||||
|
}
|
Loading…
Reference in New Issue