llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: F.Tibor (furtib) <details> <summary>Changes</summary> This patch adds a new clang-tidy check, `llvm-regex`, which detects malformed regular expression patterns passed to `llvm::Regex` when the pattern can be resolved from a string literal at compile time. The check validates regex patterns using `llvm::Regex::isValid()` and emits a diagnostic when an invalid pattern is detected. It currently handles patterns provided directly as string literals, as well as string literals stored in immutable, or expected to be used as such, string-like values such as: - `const std::string` - `const char *` - `const llvm::StringRef` - `std::string_view` It also supports detecting invalid patterns stored in class members of the same types with in-class initializers. The check also takes into consideration `llvm::Regex::RegexFlags` modifier flag literals. --- Full diff: https://github.com/llvm/llvm-project/pull/207407.diff 8 Files Affected: - (modified) clang-tools-extra/clang-tidy/llvm/CMakeLists.txt (+1) - (modified) clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp (+2) - (added) clang-tools-extra/clang-tidy/llvm/RegexCheck.cpp (+88) - (added) clang-tools-extra/clang-tidy/llvm/RegexCheck.h (+33) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) - (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) - (added) clang-tools-extra/docs/clang-tidy/checks/llvm/regex.rst (+34) - (added) clang-tools-extra/test/clang-tidy/checkers/llvm/regex.cpp (+140) ``````````diff diff --git a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt index bec3ba50c81c5..2f5d136108191 100644 --- a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt @@ -12,6 +12,7 @@ add_clang_library(clangTidyLLVMModule STATIC PreferRegisterOverUnsignedCheck.cpp PreferStaticOverAnonymousNamespaceCheck.cpp RedundantCastingCheck.cpp + RegexCheck.cpp TwineLocalCheck.cpp TypeSwitchCaseTypesCheck.cpp UseNewMLIROpBuilderCheck.cpp diff --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp index 918af88c979e0..16409770f0467 100644 --- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp @@ -18,6 +18,7 @@ #include "PreferRegisterOverUnsignedCheck.h" #include "PreferStaticOverAnonymousNamespaceCheck.h" #include "RedundantCastingCheck.h" +#include "RegexCheck.h" #include "TwineLocalCheck.h" #include "TypeSwitchCaseTypesCheck.h" #include "UseNewMLIROpBuilderCheck.h" @@ -48,6 +49,7 @@ class LLVMModule : public ClangTidyModule { "llvm-qualified-auto"); CheckFactories.registerCheck<RedundantCastingCheck>( "llvm-redundant-casting"); + CheckFactories.registerCheck<RegexCheck>("llvm-regex"); CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local"); CheckFactories.registerCheck<TypeSwitchCaseTypesCheck>( "llvm-type-switch-case-types"); diff --git a/clang-tools-extra/clang-tidy/llvm/RegexCheck.cpp b/clang-tools-extra/clang-tidy/llvm/RegexCheck.cpp new file mode 100644 index 0000000000000..f9f2b58e424f7 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/RegexCheck.cpp @@ -0,0 +1,88 @@ +//===--- RegexCheck.cpp - clang-tidy --------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "RegexCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/Support/Regex.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::llvm_check { + +void RegexCheck::registerMatchers(MatchFinder *Finder) { + // main matcher + auto IsConstllvmStringRef = qualType( + isConstQualified(), hasUnqualifiedDesugaredType(recordType(hasDeclaration( + cxxRecordDecl(hasName("::llvm::StringRef")))))); + auto IsConstStdString = qualType( + isConstQualified(), hasUnqualifiedDesugaredType(recordType(hasDeclaration( + cxxRecordDecl(hasName("::std::basic_string")))))); + auto GetStringLit = ignoringImplicit(stringLiteral().bind("stringLiteral")); + auto GetStringLiteralFromObject = + ignoringImplicit(cxxConstructExpr(hasAnyArgument(GetStringLit))); + auto IsConstCharPtr = pointerType(pointee(builtinType(), isConstQualified())); + auto IsStdStringView = qualType(hasUnqualifiedDesugaredType(recordType( + hasDeclaration(cxxRecordDecl(hasName("::std::basic_string_view")))))); + auto AnyCastedToStringRef = ignoringImplicit(anyOf( + stringLiteral().bind("stringLiteral"), + declRefExpr(to(varDecl(hasType(IsConstStdString), + hasInitializer(GetStringLiteralFromObject)))), + declRefExpr(to(varDecl(hasType(IsConstllvmStringRef), + hasInitializer(GetStringLiteralFromObject)))), + declRefExpr( + to(varDecl(hasType(IsConstCharPtr), hasInitializer(GetStringLit)))), + declRefExpr(to(varDecl(hasType(IsStdStringView), + hasInitializer(GetStringLiteralFromObject)))), + memberExpr( + member(fieldDecl(hasType(IsConstStdString), + hasInClassInitializer(GetStringLiteralFromObject)))), + memberExpr(member(fieldDecl(hasType(IsConstCharPtr), + hasInClassInitializer(GetStringLit)))), + memberExpr( + member(fieldDecl(hasType(IsConstllvmStringRef), + hasInClassInitializer(GetStringLiteralFromObject)))), + memberExpr(member( + fieldDecl(hasType(IsStdStringView), + hasInClassInitializer(GetStringLiteralFromObject)))))); + + auto IsRegexFlagsType = ignoringParenImpCasts( + anyOf(integerLiteral().bind("regexFlagsInt"), + declRefExpr(to(enumConstantDecl().bind("regexFlagEnum"))))); + Finder->addMatcher( + cxxConstructExpr( + hasDeclaration(cxxConstructorDecl(ofClass(hasName("llvm::Regex")))), + hasArgument(0, ignoringImplicit(cxxConstructExpr( + hasDeclaration(cxxConstructorDecl( + ofClass(hasName("::llvm::StringRef")))), + hasArgument(0, AnyCastedToStringRef)))), + optionally(hasArgument(1, IsRegexFlagsType))), + this); +} + +void RegexCheck::check(const MatchFinder::MatchResult &Result) { + const auto *DetectedPattern = + Result.Nodes.getNodeAs<StringLiteral>("stringLiteral"); + if (DetectedPattern) { + const auto *FlagInt = + Result.Nodes.getNodeAs<IntegerLiteral>("regexFlagsInt"); + const auto *FlagEnum = + Result.Nodes.getNodeAs<EnumConstantDecl>("regexFlagEnum"); + uint Flag = llvm::Regex::RegexFlags::NoFlags; + if (FlagInt) + Flag = FlagInt->getValue().getZExtValue(); + if (FlagEnum) + Flag = FlagEnum->getInitVal().getZExtValue(); + const llvm::Regex TestRegex(DetectedPattern->getString(), Flag); + std::string RegexError; + if (!TestRegex.isValid(RegexError)) + diag(DetectedPattern->getBeginLoc(), "invalid regex pattern: %0") + << RegexError << DetectedPattern->getSourceRange(); + } +} + +} // namespace clang::tidy::llvm_check diff --git a/clang-tools-extra/clang-tidy/llvm/RegexCheck.h b/clang-tools-extra/clang-tidy/llvm/RegexCheck.h new file mode 100644 index 0000000000000..17891713e62ee --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/RegexCheck.h @@ -0,0 +1,33 @@ +//===--- RegexCheck.h - clang-tidy ------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_REGEXCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_REGEXCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::llvm_check { + +/// Detects malformed regex patterns used in ``llvm::Regex``. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/llvm/regex.html +class RegexCheck : public ClangTidyCheck { +public: + RegexCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } +}; + +} // namespace clang::tidy::llvm_check + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_REGEXCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 42e4020f5f85c..221acb5f98081 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -318,6 +318,11 @@ New checks Checks for presence or absence of trailing commas in enum definitions and initializer lists. +- New :doc:`llvm-regex + <clang-tidy/checks/llvm/regex>` check. + + Detects malformed regex patterns used in ``llvm::Regex``. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 2a44dc78fbc89..4b3bacfa8a42e 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -251,6 +251,7 @@ Clang-Tidy Checks :doc:`llvm-prefer-register-over-unsigned <llvm/prefer-register-over-unsigned>`, "Yes" :doc:`llvm-prefer-static-over-anonymous-namespace <llvm/prefer-static-over-anonymous-namespace>`, :doc:`llvm-redundant-casting <llvm/redundant-casting>`, "Yes" + :doc:`llvm-regex <llvm/regex>`, "No" :doc:`llvm-twine-local <llvm/twine-local>`, "Yes" :doc:`llvm-type-switch-case-types <llvm/type-switch-case-types>`, "Yes" :doc:`llvm-use-new-mlir-op-builder <llvm/use-new-mlir-op-builder>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/regex.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/regex.rst new file mode 100644 index 0000000000000..837181a8ea4a6 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/regex.rst @@ -0,0 +1,34 @@ +.. title:: clang-tidy - llvm-regex + +llvm-regex +========== + +The checker detects malformed regex patterns defined in a single string literal +to catch mistakes at compile time. +It detects these string literals if they are defined in the regex constructor +with a string literal, or stored in one of these non mutable container: + +- ``const std::string`` +- ``const char*`` +- ``const llvm::StringRef`` +- ``std::string_view`` + +In the event that the patterns are stored as a class member, the check checks +the initialization value, if defined, even if its overwritten by the constructor. + +Example of detection: + +.. code-block:: c++ + const std::string my_regex_pattern("[0-8"); // invalid regex pattern + llvm::Regex my_regex(my_regex_pattern); + +Example of member data being overwritten: + +.. code-block:: c++ + class foo{ + public: + foo(llvm::StringRef regex_ptrn) : regex_prtn(regex_prtn){}; + const llvm_StringRef regex_prtn = "("; // invalid regex pattern + }; + foo bar("[0-9]"); + llvm::Regex my_regex(bar.regex_prtn); diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/regex.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/regex.cpp new file mode 100644 index 0000000000000..229b35f2eb204 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/regex.cpp @@ -0,0 +1,140 @@ +// RUN: %check_clang_tidy -std=c++17 %s llvm-regex %t + +namespace std { + template <typename T> + class basic_string { + public: + basic_string(const char*); + }; + using string = basic_string<char>; + + template <typename T> + class basic_string_view { + public: + basic_string_view(const char*); + const char* begin() const; + }; + using string_view = basic_string_view<char>; +} // namespace std + +namespace llvm { + class StringRef { + public: + StringRef(const char*); + StringRef(const std::string&); + StringRef(const std::string_view&); + }; + + class Regex { + public: + Regex(StringRef, unsigned int i = 0); + enum RegexFlags : unsigned { + NoFlags = 0, + IgnoreCase = 1, + Newline = 2, + BasicRegex = 4, + }; + }; +} // namespace llvm + +void test_detected_faulty_patterns(){ + llvm::Regex re1("("); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: invalid regex pattern: parentheses not balanced + + const std::string badStdString("("); + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: invalid regex pattern: parentheses not balanced + llvm::Regex re2(badStdString); + + const char* badCharPtr = "[]"; + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: invalid regex pattern: brackets ([ ]) not balanced + llvm::Regex re3(badCharPtr); + + std::string_view badStrView("+"); + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: invalid regex pattern: repetition-operator operand invalid + llvm::Regex re4(badStrView); + + const llvm::StringRef badStrRef = "a*?"; + // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: invalid regex pattern: repetition-operator operand invalid + llvm::Regex re5(badStrRef); + + struct RegexPatterns { + const char* badMemberChar = ""; + // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: invalid regex pattern: empty (sub)expression + + const std::string badMemberStr = "("; + // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: invalid regex pattern: parentheses not balanced + + std::string_view badMemberStrView = "+"; + // CHECK-MESSAGES: :[[@LINE-1]]:41: warning: invalid regex pattern: repetition-operator operand invalid + + const llvm::StringRef badMemberStrRef = "a*?"; + // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: invalid regex pattern: repetition-operator operand invalid + }; + + RegexPatterns Pats; + + llvm::Regex re6(Pats.badMemberChar); + llvm::Regex re7(Pats.badMemberStr); + llvm::Regex re8(Pats.badMemberStrView); + llvm::Regex re9(Pats.badMemberStrRef); +} + +void test_no_detection_on_mutable(){ + std::string badMutStdString("("); + llvm::Regex re10(badMutStdString); + + char* badMutCharPtr = "["; + llvm::Regex re11(badMutCharPtr); + + llvm::StringRef badMutStrRef = "a*?"; + llvm::Regex re12(badMutStrRef); + + struct RegexMutPatterns { + char* badMutMemberChar = ""; + std::string badMutMemberStr = "("; + llvm::StringRef badMutMemberStrRef = "a*?"; + }; + + RegexMutPatterns mutPats; + + llvm::Regex re13(mutPats.badMutMemberChar); + llvm::Regex re14(mutPats.badMutMemberStr); + llvm::Regex re15(mutPats.badMutMemberStrRef); +} + +void test_no_report_on_correct_patterns(){ + llvm::Regex re16("[0-9]"); + + const std::string goodStdString("test"); + llvm::Regex re17(goodStdString); + + const char* goodCharPtr = "\\[test\\]"; + llvm::Regex re18(goodCharPtr); + + std::string_view goodStrView("testi+ng"); + llvm::Regex re19(goodStrView); + + const llvm::StringRef goodStrRef = "a*b?"; + llvm::Regex re20(goodStrRef); + + struct GoodRegexPatterns { + const char* goodMemberChar = "[0-9]"; + const std::string goodMemberStr = "test"; + std::string_view goodMemberStrView = "\[test\]"; + const llvm::StringRef goodMemberStrRef = "a*b"; + }; + + GoodRegexPatterns goodPats; + + llvm::Regex re21(goodPats.goodMemberChar); + llvm::Regex re22(goodPats.goodMemberStr); + llvm::Regex re23(goodPats.goodMemberStrView); + llvm::Regex re24(goodPats.goodMemberStrRef); +} + +void test_grammar_flags(){ + llvm::Regex re1_noflag("(", 0); + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid regex pattern: parentheses not balanced + llvm::Regex re1_basic("(", 4U); + llvm::Regex re2_basic("(", llvm::Regex::RegexFlags::BasicRegex); +} `````````` </details> https://github.com/llvm/llvm-project/pull/207407 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
