Skip to content

Commit f5f0ba1

Browse files
vbvictorvbvictor
committed
[clang-tidy] Added support for 3-argument string ctor
1 parent c5492e3 commit f5f0ba1

File tree

3 files changed

+80
-5
lines changed

3 files changed

+80
-5
lines changed

‎clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
8282
Finder->addMatcher(
8383
cxxConstructExpr(
8484
hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
85-
hasArgument(0, hasType(qualType(isInteger()))),
85+
argumentCountIs(2), hasArgument(0, hasType(qualType(isInteger()))),
8686
hasArgument(1, hasType(qualType(isInteger()))),
8787
anyOf(
8888
// Detect the expression: string('x', 40);
@@ -102,7 +102,7 @@ void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
102102
cxxConstructExpr(
103103
hasDeclaration(cxxConstructorDecl(ofClass(
104104
cxxRecordDecl(hasAnyName(removeNamespaces(StringNames)))))),
105-
hasArgument(0, hasType(CharPtrType)),
105+
argumentCountIs(2), hasArgument(0, hasType(CharPtrType)),
106106
hasArgument(1, hasType(isInteger())),
107107
anyOf(
108108
// Detect the expression: string("...", 0);
@@ -114,7 +114,34 @@ void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
114114
// Detect the expression: string("lit", 5)
115115
allOf(hasArgument(0, ConstStrLiteral.bind("literal-with-length")),
116116
hasArgument(1, ignoringParenImpCasts(
117-
integerLiteral().bind("int"))))))
117+
integerLiteral().bind("length"))))))
118+
.bind("constructor"),
119+
this);
120+
121+
// Check the literal string constructor with char pointer, start position and
122+
// length parameters. [i.e. string (const char* s, size_t pos, size_t count);]
123+
Finder->addMatcher(
124+
cxxConstructExpr(
125+
hasDeclaration(cxxConstructorDecl(ofClass(
126+
cxxRecordDecl(hasAnyName(removeNamespaces(StringNames)))))),
127+
argumentCountIs(3), hasArgument(0, hasType(CharPtrType)),
128+
hasArgument(1, hasType(qualType(isInteger()))),
129+
hasArgument(2, hasType(qualType(isInteger()))),
130+
anyOf(
131+
// Detect the expression: string("...", 1, 0);
132+
hasArgument(2, ZeroExpr.bind("empty-string")),
133+
// Detect the expression: string("...", -4, 1);
134+
hasArgument(1, NegativeExpr.bind("negative-pos")),
135+
// Detect the expression: string("...", 0, -4);
136+
hasArgument(2, NegativeExpr.bind("negative-length")),
137+
// Detect the expression: string("lit", 0, 0x1234567);
138+
hasArgument(2, LargeLengthExpr.bind("large-length")),
139+
// Detect the expression: string("lit", 1, 5)
140+
allOf(hasArgument(0, ConstStrLiteral.bind("literal-with-length")),
141+
hasArgument(
142+
1, ignoringParenImpCasts(integerLiteral().bind("pos"))),
143+
hasArgument(2, ignoringParenImpCasts(
144+
integerLiteral().bind("length"))))))
118145
.bind("constructor"),
119146
this);
120147

@@ -155,14 +182,27 @@ void StringConstructorCheck::check(const MatchFinder::MatchResult &Result) {
155182
diag(Loc, "constructor creating an empty string");
156183
} else if (Result.Nodes.getNodeAs<Expr>("negative-length")) {
157184
diag(Loc, "negative value used as length parameter");
185+
} else if (Result.Nodes.getNodeAs<Expr>("negative-pos")) {
186+
diag(Loc, "negative value used as position of the "
187+
"first character parameter");
158188
} else if (Result.Nodes.getNodeAs<Expr>("large-length")) {
159189
if (WarnOnLargeLength)
160190
diag(Loc, "suspicious large length parameter");
161191
} else if (Result.Nodes.getNodeAs<Expr>("literal-with-length")) {
162192
const auto *Str = Result.Nodes.getNodeAs<StringLiteral>("str");
163-
const auto *Lit = Result.Nodes.getNodeAs<IntegerLiteral>("int");
164-
if (Lit->getValue().ugt(Str->getLength())) {
193+
const auto *Length = Result.Nodes.getNodeAs<IntegerLiteral>("length");
194+
if (Length->getValue().ugt(Str->getLength())) {
165195
diag(Loc, "length is bigger than string literal size");
196+
return;
197+
}
198+
if (const auto *Pos = Result.Nodes.getNodeAs<IntegerLiteral>("pos")) {
199+
if (Pos->getValue().uge(Str->getLength())) {
200+
diag(Loc, "position of the first character parameter is bigger than "
201+
"string literal character range");
202+
} else if (Length->getValue().ugt(
203+
(Str->getLength() - Pos->getValue()).getZExtValue())) {
204+
diag(Loc, "length is bigger than remaining string literal size");
205+
}
166206
}
167207
} else if (const auto *Ptr = Result.Nodes.getNodeAs<Expr>("from-ptr")) {
168208
Expr::EvalResult ConstPtr;

‎clang-tools-extra/docs/ReleaseNotes.rst‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,11 @@ Changes in existing checks
221221
subtracting from a pointer directly or when used to scale a numeric value and
222222
fix false positive when sizeof expression with template types.
223223

224+
- Improved :doc:`bugprone-string-constructor
225+
<clang-tidy/checks/bugprone/string-constructor>` check to find suspicious
226+
calls of string constructor with char pointer, start position
227+
and length parameters.
228+
224229
- Improved :doc:`bugprone-throw-keyword-missing
225230
<clang-tidy/checks/bugprone/throw-keyword-missing>` by fixing a false positive
226231
when using non-static member initializers and a constructor.

‎clang-tools-extra/test/clang-tidy/checkers/bugprone/string-constructor.cpp‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ struct basic_string {
1111
basic_string(const C*, unsigned int size);
1212
basic_string(const C *, const A &allocator = A());
1313
basic_string(unsigned int size, C c);
14+
basic_string(const C*, unsigned int pos, unsigned int size);
1415
};
1516
typedef basic_string<char> string;
1617
typedef basic_string<wchar_t> wstring;
@@ -61,6 +62,21 @@ void Test() {
6162
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructing string from nullptr is undefined behaviour
6263
std::string q7 = 0;
6364
// CHECK-MESSAGES: [[@LINE-1]]:20: warning: constructing string from nullptr is undefined behaviour
65+
66+
std::string r1("test", 1, 0);
67+
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor creating an empty string
68+
std::string r2("test", 0, -4);
69+
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: negative value used as length parameter
70+
std::string r3("test", -4, 1);
71+
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: negative value used as position of the first character parameter
72+
std::string r4("test", 0, 0x1000000);
73+
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: suspicious large length parameter
74+
std::string r5("test", 0, 5);
75+
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger than string literal size
76+
std::string r6("test", 3, 2);
77+
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger than remaining string literal size
78+
std::string r7("test", 4, 1);
79+
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: position of the first character parameter is bigger than string literal character range
6480
}
6581

6682
void TestView() {
@@ -82,6 +98,17 @@ void TestView() {
8298
// CHECK-MESSAGES: [[@LINE-1]]:25: warning: constructing string from nullptr is undefined behaviour
8399
}
84100

101+
void TestUnsignedArguments() {
102+
std::string s0("test", 0u);
103+
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor creating an empty string
104+
std::string s1(0x1000000ull, 'x');
105+
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: suspicious large length parameter
106+
std::string s2("test", 3ull, 2u);
107+
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger than remaining string literal size
108+
std::string s3("test", 0u, 5ll);
109+
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger than string literal size
110+
}
111+
85112
std::string StringFromZero() {
86113
return 0;
87114
// CHECK-MESSAGES: [[@LINE-1]]:10: warning: constructing string from nullptr is undefined behaviour
@@ -101,6 +128,9 @@ void Valid() {
101128
std::string s3("test");
102129
std::string s4("test\000", 5);
103130
std::string s6("te" "st", 4);
131+
std::string s7("test", 0, 4);
132+
std::string s8("test", 3, 1);
133+
std::string s9("te" "st", 1, 2);
104134

105135
std::string_view emptyv();
106136
std::string_view sv1("test", 4);

0 commit comments

Comments
 (0)