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.2...@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.2...@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.2...@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.2...@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits