From a663e78a6eb6bbd20c0c74b3e6a78e24ee423544 Mon Sep 17 00:00:00 2001 From: Piotr Zegar Date: Thu, 13 Feb 2025 17:51:28 +0100 Subject: [PATCH] [clang-tidy] Add recursion protection in ExceptionSpecAnalyzer (#66810) Normally endless recursion should not happen in ExceptionSpecAnalyzer, but if AST would be malformed (missing include), this could cause crash. I run into this issue when due to missing include constructor argument were parsed as FieldDecl. As checking for recursion cost nothing, why not to do this in check just in case. Fixes #111436 --- .../clang-tidy/utils/ExceptionSpecAnalyzer.cpp | 13 +++++++------ .../performance/noexcept-move-constructor.cpp | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp index 4a9426ee7e8b..0637d0eff688 100644 --- a/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp +++ b/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp @@ -14,13 +14,14 @@ namespace clang::tidy::utils { ExceptionSpecAnalyzer::State ExceptionSpecAnalyzer::analyze(const FunctionDecl *FuncDecl) { - // Check if the function has already been analyzed and reuse that result. - const auto CacheEntry = FunctionCache.find(FuncDecl); - if (CacheEntry == FunctionCache.end()) { + // Check if function exist in cache or add temporary value to cache to protect + // against endless recursion. + const auto [CacheEntry, NotFound] = + FunctionCache.try_emplace(FuncDecl, State::NotThrowing); + if (NotFound) { ExceptionSpecAnalyzer::State State = analyzeImpl(FuncDecl); - - // Cache the result of the analysis. - FunctionCache.try_emplace(FuncDecl, State); + // Update result with calculated value + FunctionCache[FuncDecl] = State; return State; } diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp index 347a1e322006..4df0927b62af 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp @@ -1,4 +1,6 @@ // RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t -- -- -fexceptions +// RUN: %check_clang_tidy -std=c++17 -check-suffixes=,ERR %s performance-noexcept-move-constructor %t \ +// RUN: -- --fix-errors -- -fexceptions -DENABLE_ERROR namespace std { @@ -397,3 +399,18 @@ namespace gh68101 Container(Container&&) noexcept(std::is_nothrow_move_constructible::value); }; } // namespace gh68101 + +namespace gh111436 +{ + +template class set { + set(set &&) = default; + +#ifdef ENABLE_ERROR + set(initializer_list __l) {}; + // CHECK-MESSAGES-ERR: :[[@LINE-1]]:7: error: member 'initializer_list' cannot have template arguments [clang-diagnostic-error] + // CHECK-MESSAGES-ERR: :[[@LINE-2]]:36: error: expected ')' [clang-diagnostic-error] +#endif +}; + +} // namespace gh111436