[clang-tools-extra] [clang-tidy] Added support for 3-argument std::string ctor in bugprone-string-constructor check (PR #123413)
Baranov Victor via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 29 09:12:15 PST 2025
https://github.com/vbvictor updated https://github.com/llvm/llvm-project/pull/123413
>From bc85c10a7d316630843266779cb1465b02d3dbf6 Mon Sep 17 00:00:00 2001
From: Victor Baranov <bar.victor.2002 at gmail.com>
Date: Sat, 18 Jan 2025 00:49:29 +0300
Subject: [PATCH 1/4] [clang-tidy] Added support for 3-argument string ctor
---
.../bugprone/StringConstructorCheck.cpp | 50 ++++-
clang-tools-extra/docs/ReleaseNotes.rst | 182 ++++++++++++++++++
.../checkers/bugprone/string-constructor.cpp | 30 +++
3 files changed, 257 insertions(+), 5 deletions(-)
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 727c7622426c84..feeca4033c188f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -97,6 +97,188 @@ New check aliases
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Improved :doc:`altera-id-dependent-backward-branch
+ <clang-tidy/checks/altera/id-dependent-backward-branch>` check by fixing
+ crashes from invalid code.
+
+- Improved :doc:`bugprone-branch-clone
+ <clang-tidy/checks/bugprone/branch-clone>` check to improve detection of
+ branch clones by now detecting duplicate inner and outer if statements.
+
+- Improved :doc:`bugprone-casting-through-void
+ <clang-tidy/checks/bugprone/casting-through-void>` check to suggest replacing
+ the offending code with ``reinterpret_cast``, to more clearly express intent.
+
+- Improved :doc:`bugprone-dangling-handle
+ <clang-tidy/checks/bugprone/dangling-handle>` check to treat `std::span` as a
+ handle class.
+
+- Improved :doc:`bugprone-exception-escape
+ <clang-tidy/checks/bugprone/exception-escape>` by fixing false positives
+ when a consteval function with throw statements.
+
+- Improved :doc:`bugprone-forwarding-reference-overload
+ <clang-tidy/checks/bugprone/forwarding-reference-overload>` check by fixing
+ a crash when determining if an ``enable_if[_t]`` was found.
+
+- Improved :doc:`bugprone-optional-value-conversion
+ <clang-tidy/checks/bugprone/optional-value-conversion>` to support detecting
+ conversion directly by ``std::make_unique`` and ``std::make_shared``.
+
+- Improved :doc:`bugprone-posix-return
+ <clang-tidy/checks/bugprone/posix-return>` check to support integer literals
+ as LHS and posix call as RHS of comparison.
+
+- Improved :doc:`bugprone-return-const-ref-from-parameter
+ <clang-tidy/checks/bugprone/return-const-ref-from-parameter>` check to
+ diagnose potential dangling references when returning a ``const &`` parameter
+ by using the conditional operator ``cond ? var1 : var2`` and fixing false
+ positives for functions which contain lambda and ignore parameters
+ with ``[[clang::lifetimebound]]`` attribute.
+
+- Improved :doc:`bugprone-sizeof-expression
+ <clang-tidy/checks/bugprone/sizeof-expression>` check to find suspicious
+ usages of ``sizeof()``, ``alignof()``, and ``offsetof()`` when adding or
+ 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.
+
+- Improved :doc:`bugprone-unchecked-optional-access
+ <clang-tidy/checks/bugprone/unchecked-optional-access>` to support
+ `bsl::optional` and `bdlb::NullableValue` from
+ <https://github.com/bloomberg/bde>_.
+
+- Improved :doc:`bugprone-unsafe-functions
+ <clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying
+ additional functions to match.
+
+- Improved :doc:`bugprone-use-after-move
+ <clang-tidy/checks/bugprone/use-after-move>` to avoid triggering on
+ ``reset()`` calls on moved-from ``std::optional`` and ``std::any`` objects,
+ similarly to smart pointers.
+
+- Improved :doc:`cert-flp30-c <clang-tidy/checks/cert/flp30-c>` check to
+ fix false positive that floating point variable is only used in increment
+ expression.
+
+- Improved :doc:`cppcoreguidelines-avoid-const-or-ref-data-members
+ <clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members>` check to
+ avoid false positives when detecting a templated class with inheritance.
+
+- Improved :doc:`cppcoreguidelines-init-variables
+ <clang-tidy/checks/cppcoreguidelines/init-variables>` check by fixing the
+ insertion location for function pointers.
+
+- Improved :doc:`cppcoreguidelines-prefer-member-initializer
+ <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>` check to
+ avoid false positive when member initialization depends on a structured
+ binding variable.
+
+- Fixed :doc:`cppcoreguidelines-pro-type-union-access
+ <clang-tidy/checks/cppcoreguidelines/pro-type-union-access>` check to
+ report a location even when the member location is not valid.
+
+- Improved :doc:`misc-definitions-in-headers
+ <clang-tidy/checks/misc/definitions-in-headers>` check by rewording the
+ diagnostic note that suggests adding ``inline``.
+
+- Improved :doc:`misc-redundant-expression
+ <clang-tidy/checks/misc/redundant-expression>` check by extending the
+ checker to detect floating point and integer literals in redundant
+ expressions.
+
+- Improved :doc:`misc-unconventional-assign-operator
+ <clang-tidy/checks/misc/unconventional-assign-operator>` check to avoid
+ false positive for C++23 deducing this.
+
+- Improved :doc:`misc-use-internal-linkage
+ <clang-tidy/checks/misc/use-internal-linkage>` check to insert ``static``
+ keyword before type qualifiers such as ``const`` and ``volatile`` and fix
+ false positives for function declaration without body and fix false positives
+ for C++20 export declarations and fix false positives for global scoped
+ overloaded ``operator new`` and ``operator delete``.
+
+- Improved :doc:`modernize-avoid-c-arrays
+ <clang-tidy/checks/modernize/avoid-c-arrays>` check to suggest using
+ ``std::span`` as a replacement for parameters of incomplete C array type in
+ C++20 and ``std::array`` or ``std::vector`` before C++20.
+
+- Improved :doc:`modernize-loop-convert
+ <clang-tidy/checks/modernize/loop-convert>` check to fix false positive when
+ using loop variable in initializer of lambda capture.
+
+- Improved :doc:`modernize-min-max-use-initializer-list
+ <clang-tidy/checks/modernize/min-max-use-initializer-list>` check by fixing
+ a false positive when only an implicit conversion happened inside an
+ initializer list.
+
+- Improved :doc:`modernize-use-designated-initializers
+ <clang-tidy/checks/modernize/use-designated-initializers>` check to fix a
+ crash when a class is declared but not defined.
+
+- Improved :doc:`modernize-use-nullptr
+ <clang-tidy/checks/modernize/use-nullptr>` check to also recognize
+ ``NULL``/``__null`` (but not ``0``) when used with a templated type.
+
+- Improved :doc:`modernize-use-starts-ends-with
+ <clang-tidy/checks/modernize/use-starts-ends-with>` check to handle two new
+ cases from ``rfind`` and ``compare`` to ``ends_with``, and one new case from
+ ``substr`` to ``starts_with``, and a small adjustment to the diagnostic message.
+
+- Improved :doc:`modernize-use-std-format
+ <clang-tidy/checks/modernize/use-std-format>` check to support replacing
+ member function calls too and to only expand macros starting with ``PRI``
+ and ``__PRI`` from ``<inttypes.h>`` in the format string.
+
+- Improved :doc:`modernize-use-std-print
+ <clang-tidy/checks/modernize/use-std-print>` check to support replacing
+ member function calls too and to only expand macros starting with ``PRI``
+ and ``__PRI`` from ``<inttypes.h>`` in the format string.
+
+- Improved :doc:`modernize-use-using
+ <clang-tidy/checks/modernize/use-using>` check by not expanding macros.
+
+- Improved :doc:`performance-avoid-endl
+ <clang-tidy/checks/performance/avoid-endl>` check to use ``std::endl`` as
+ placeholder when lexer cannot get source text.
+
+- Improved :doc:`performance-move-const-arg
+ <clang-tidy/checks/performance/move-const-arg>` check to fix a crash when
+ an argument type is declared but not defined.
+
+- Improved :doc:`readability-container-contains
+ <clang-tidy/checks/readability/container-contains>` check to let it work on
+ any class that has a ``contains`` method. Fix some false negatives in the
+ ``find()`` case.
+
+- Improved :doc:`readability-enum-initial-value
+ <clang-tidy/checks/readability/enum-initial-value>` check by only issuing
+ diagnostics for the definition of an ``enum``, by not emitting a redundant
+ file path for anonymous enums in the diagnostic, and by fixing a typo in the
+ diagnostic.
+
+- Improved :doc:`readability-implicit-bool-conversion
+ <clang-tidy/checks/readability/implicit-bool-conversion>` check
+ by adding the option `UseUpperCaseLiteralSuffix` to select the
+ case of the literal suffix in fixes and fixing false positive for implicit
+ conversion of comparison result in C23.
+
+- Improved :doc:`readability-redundant-smartptr-get
+ <clang-tidy/checks/readability/redundant-smartptr-get>` check to
+ remove `->`, when redundant `get()` is removed.
+
+- Improved :doc:`readability-identifier-naming
+ <clang-tidy/checks/readability/identifier-naming>` check to
+ validate ``namespace`` aliases.
+
Removed checks
^^^^^^^^^^^^^^
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);
>From a9c97de1e80b8641fda0868726deb17363bb7db8 Mon Sep 17 00:00:00 2001
From: Baranov Victor <bar.victor.2002 at gmail.com>
Date: Sun, 19 Jan 2025 16:22:48 +0300
Subject: [PATCH 2/4] [clang-tidy] Add docs for new string-constructor cases
---
.../checks/bugprone/string-constructor.rst | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/string-constructor.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/string-constructor.rst
index b45c2ef4a93129..a0bd1d7c5bc157 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/string-constructor.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/string-constructor.rst
@@ -21,6 +21,7 @@ Examples:
.. code-block:: c++
std::string("test", 200); // Will include random characters after "test".
+ std::string("test", 2, 5); // Will include random characters after "st".
std::string_view("test", 200);
Creating an empty string from constructors with parameters is considered
@@ -31,8 +32,19 @@ Examples:
.. code-block:: c++
std::string("test", 0); // Creation of an empty string.
+ std::string("test", 1, 0);
std::string_view("test", 0);
+Passing an invalid first character position parameter to constructor will
+cause ``std::out_of_range`` exception at runtime.
+
+Examples:
+
+.. code-block:: c++
+
+ std::string("test", -1, 10); // Negative first character position.
+ std::string("test", 10, 10); // First character position is bigger than string literal character range".
+
Options
-------
>From 50f3eeb183f95463616618ce686052444da937a7 Mon Sep 17 00:00:00 2001
From: Victor Baranov <bar.victor.2002 at gmail.com>
Date: Sat, 25 Jan 2025 20:14:24 +0300
Subject: [PATCH 3/4] [clang-tidy] improved docs to fill lines up to
80-character limit
---
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index feeca4033c188f..2323b1f929db8f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -144,8 +144,8 @@ Changes in existing checks
- 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.
+ 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
>From 8d5abfbc7c25e049a94de28802e0d3d9ebe32bf2 Mon Sep 17 00:00:00 2001
From: baranov-V-V <bar.victor.2002 at gmail.com>
Date: Wed, 29 Jan 2025 20:11:50 +0300
Subject: [PATCH 4/4] [clang-tidy] Improved release notes
---
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 2323b1f929db8f..a8ec8eddcd6984 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -144,8 +144,8 @@ Changes in existing checks
- 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.
+ calls of ``std::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
More information about the cfe-commits
mailing list