llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: Baranov Victor (vbvictor)
<details>
<summary>Changes</summary>
This PR add diagnostics for 3-parameter `std::basic_string(const char* t,
size_type pos, size_type count)` constructor in bugprone-string-constructor
check:
```cpp
std::string r1("test", 1, 0); // constructor creating an empty string
std::string r2("test", 0, -4); // negative value used as length parameter
// more examples in test file
```
Fixes false-positives reported in
https://github.com/llvm/llvm-project/issues/123198.
---
Full diff: https://github.com/llvm/llvm-project/pull/123413.diff
3 Files Affected:
- (modified) clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
(+45-5)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/bugprone/string-constructor.cpp
(+30)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
index 8ae4351ac2830a..d1902b658061b1 100644
--- a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
@@ -82,7 +82,7 @@ void StringConstructorCheck::registerMatchers(MatchFinder
*Finder) {
Finder->addMatcher(
cxxConstructExpr(
hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
- hasArgument(0, hasType(qualType(isInteger()))),
+ argumentCountIs(2), hasArgument(0, hasType(qualType(isInteger()))),
hasArgument(1, hasType(qualType(isInteger()))),
anyOf(
// Detect the expression: string('x', 40);
@@ -102,7 +102,7 @@ void StringConstructorCheck::registerMatchers(MatchFinder
*Finder) {
cxxConstructExpr(
hasDeclaration(cxxConstructorDecl(ofClass(
cxxRecordDecl(hasAnyName(removeNamespaces(StringNames)))))),
- hasArgument(0, hasType(CharPtrType)),
+ argumentCountIs(2), hasArgument(0, hasType(CharPtrType)),
hasArgument(1, hasType(isInteger())),
anyOf(
// Detect the expression: string("...", 0);
@@ -114,7 +114,34 @@ void StringConstructorCheck::registerMatchers(MatchFinder
*Finder) {
// Detect the expression: string("lit", 5)
allOf(hasArgument(0,
ConstStrLiteral.bind("literal-with-length")),
hasArgument(1, ignoringParenImpCasts(
- integerLiteral().bind("int"))))))
+ integerLiteral().bind("length"))))))
+ .bind("constructor"),
+ this);
+
+ // Check the literal string constructor with char pointer, start position and
+ // length parameters. [i.e. string (const char* s, size_t pos, size_t
count);]
+ Finder->addMatcher(
+ cxxConstructExpr(
+ hasDeclaration(cxxConstructorDecl(ofClass(
+ cxxRecordDecl(hasAnyName(removeNamespaces(StringNames)))))),
+ argumentCountIs(3), hasArgument(0, hasType(CharPtrType)),
+ hasArgument(1, hasType(qualType(isInteger()))),
+ hasArgument(2, hasType(qualType(isInteger()))),
+ anyOf(
+ // Detect the expression: string("...", 1, 0);
+ hasArgument(2, ZeroExpr.bind("empty-string")),
+ // Detect the expression: string("...", -4, 1);
+ hasArgument(1, NegativeExpr.bind("negative-pos")),
+ // Detect the expression: string("...", 0, -4);
+ hasArgument(2, NegativeExpr.bind("negative-length")),
+ // Detect the expression: string("lit", 0, 0x1234567);
+ hasArgument(2, LargeLengthExpr.bind("large-length")),
+ // Detect the expression: string("lit", 1, 5)
+ allOf(hasArgument(0,
ConstStrLiteral.bind("literal-with-length")),
+ hasArgument(
+ 1,
ignoringParenImpCasts(integerLiteral().bind("pos"))),
+ hasArgument(2, ignoringParenImpCasts(
+ integerLiteral().bind("length"))))))
.bind("constructor"),
this);
@@ -155,14 +182,27 @@ void StringConstructorCheck::check(const
MatchFinder::MatchResult &Result) {
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>("negative-pos")) {
+ diag(Loc, "negative value used as position of the "
+ "first character 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())) {
+ const auto *Length = Result.Nodes.getNodeAs<IntegerLiteral>("length");
+ if (Length->getValue().ugt(Str->getLength())) {
diag(Loc, "length is bigger than string literal size");
+ return;
+ }
+ if (const auto *Pos = Result.Nodes.getNodeAs<IntegerLiteral>("pos")) {
+ if (Pos->getValue().uge(Str->getLength())) {
+ diag(Loc, "position of the first character parameter is bigger than "
+ "string literal character range");
+ } else if (Length->getValue().ugt(
+ (Str->getLength() - Pos->getValue()).getZExtValue())) {
+ diag(Loc, "length is bigger than remaining string literal size");
+ }
}
} else if (const auto *Ptr = Result.Nodes.getNodeAs<Expr>("from-ptr")) {
Expr::EvalResult ConstPtr;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index fa3a8e577a33ad..0171fe556a611b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -221,6 +221,11 @@ Changes in existing checks
subtracting from a pointer directly or when used to scale a numeric value and
fix false positive when sizeof expression with template types.
+- Improved :doc:`bugprone-string-constructor
+ <clang-tidy/checks/bugprone/string-constructor>` check to find suspicious
+ calls of string constructor with char pointer, start position
+ and length parameters.
+
- Improved :doc:`bugprone-throw-keyword-missing
<clang-tidy/checks/bugprone/throw-keyword-missing>` by fixing a false
positive
when using non-static member initializers and a constructor.
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/string-constructor.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/string-constructor.cpp
index a5b6b240ddc665..2576d199162509 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/string-constructor.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/string-constructor.cpp
@@ -11,6 +11,7 @@ struct basic_string {
basic_string(const C*, unsigned int size);
basic_string(const C *, const A &allocator = A());
basic_string(unsigned int size, C c);
+ basic_string(const C*, unsigned int pos, unsigned int size);
};
typedef basic_string<char> string;
typedef basic_string<wchar_t> wstring;
@@ -61,6 +62,21 @@ void Test() {
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructing string from nullptr
is undefined behaviour
std::string q7 = 0;
// CHECK-MESSAGES: [[@LINE-1]]:20: warning: constructing string from nullptr
is undefined behaviour
+
+ std::string r1("test", 1, 0);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor creating an empty
string
+ std::string r2("test", 0, -4);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: negative value used as length
parameter
+ std::string r3("test", -4, 1);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: negative value used as position
of the first character parameter
+ std::string r4("test", 0, 0x1000000);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: suspicious large length parameter
+ std::string r5("test", 0, 5);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger than string
literal size
+ std::string r6("test", 3, 2);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger than remaining
string literal size
+ std::string r7("test", 4, 1);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: position of the first character
parameter is bigger than string literal character range
}
void TestView() {
@@ -82,6 +98,17 @@ void TestView() {
// CHECK-MESSAGES: [[@LINE-1]]:25: warning: constructing string from nullptr
is undefined behaviour
}
+void TestUnsignedArguments() {
+ std::string s0("test", 0u);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor creating an empty
string
+ std::string s1(0x1000000ull, 'x');
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: suspicious large length parameter
+ std::string s2("test", 3ull, 2u);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger than remaining
string literal size
+ std::string s3("test", 0u, 5ll);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger than string
literal size
+}
+
std::string StringFromZero() {
return 0;
// CHECK-MESSAGES: [[@LINE-1]]:10: warning: constructing string from nullptr
is undefined behaviour
@@ -101,6 +128,9 @@ void Valid() {
std::string s3("test");
std::string s4("test\000", 5);
std::string s6("te" "st", 4);
+ std::string s7("test", 0, 4);
+ std::string s8("test", 3, 1);
+ std::string s9("te" "st", 1, 2);
std::string_view emptyv();
std::string_view sv1("test", 4);
``````````
</details>
https://github.com/llvm/llvm-project/pull/123413
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
