llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Balázs Benics (steakhal) <details> <summary>Changes</summary> The code assumed that taking the lexical parent decl context of a node and traversing it will eventually visit the node itself. While this is certeanly true for most AST constructs, template specializations (aka. instantiations) might inject their AST to surprising lexical parents, depending on when they get instantiated. This means that just taking the lexical parent of a template specialization might land us on some AST node that won't contain (thus visit) the definition, and consequently, miss the Suppress attribute... To fix this, we must take special care for template specializations. For a regular instantiation select the primary template (that has the definition). For an instantiation coming from a partial specialization, pretend if it was the partial specialization instead. Once we canonicalize to the primary template/partial specialization definition, the usual "walk the lexical parents" logic covers the rest as usual. Assisted-by: claude rdar://168941095 --- Full diff: https://github.com/llvm/llvm-project/pull/178441.diff 3 Files Affected: - (modified) clang/lib/StaticAnalyzer/Core/BugSuppression.cpp (+60) - (added) clang/test/Analysis/suppress-namespace-redecl-core.cpp (+33) - (added) clang/test/Analysis/suppress-namespace-redecl-webkit.cpp (+85) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Core/BugSuppression.cpp b/clang/lib/StaticAnalyzer/Core/BugSuppression.cpp index a7300b7183590..c2c05421f0b68 100644 --- a/clang/lib/StaticAnalyzer/Core/BugSuppression.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugSuppression.cpp @@ -165,6 +165,57 @@ bool BugSuppression::isSuppressed(const BugReport &R) { isSuppressed(UniqueingLocation, DeclWithIssue, {}); } +// For template specializations, returns the primary template definition or +// partial specialization that was used to instantiate the specialization. +// This ensures suppression attributes on templates apply to their +// specializations. +// +// For example, given: +// template <typename T> class [[clang::suppress]] Wrapper { ... }; +// Wrapper<int> w; // instantiates ClassTemplateSpecializationDecl +// +// When analyzing code in Wrapper<int>, this function maps the specialization +// back to the primary template definition, allowing us to find the suppression +// attribute. +// +// The function handles two cases: +// 1. Instantiation from a class template - searches redeclarations to find +// the definition (not just a forward declaration). +// 2. Instantiation from a partial specialization - returns it directly. +// +// For non-template-specialization decls, returns the input unchanged. +static const Decl * +preferTemplateDefinitionForTemplateSpecializations(const Decl *D) { + const auto *SpecializationDecl = dyn_cast<ClassTemplateSpecializationDecl>(D); + if (!SpecializationDecl) + return D; + + auto InstantiatedFrom = SpecializationDecl->getInstantiatedFrom(); + if (!InstantiatedFrom) + return D; + + // This might be a class template. + if (const auto *Tmpl = InstantiatedFrom.dyn_cast<ClassTemplateDecl *>()) { + // Interestingly, the source template might be a forward declaration, so we + // need to find the definition redeclaration. + for (const auto *Redecl : Tmpl->redecls()) { + if (cast<ClassTemplateDecl>(Redecl)->isThisDeclarationADefinition()) { + return Redecl; + } + } + assert(false && "A redecl must be a definition"); + return D; + } + + // It might be a partial specialization. + const auto *PartialSpecialization = + InstantiatedFrom.dyn_cast<ClassTemplatePartialSpecializationDecl *>(); + + // The partial specialization should be a definition. + assert(PartialSpecialization->isThisDeclarationADefinition()); + return PartialSpecialization; +} + bool BugSuppression::isSuppressed(const PathDiagnosticLocation &Location, const Decl *DeclWithIssue, DiagnosticIdentifierList Hashtags) { @@ -182,6 +233,15 @@ bool BugSuppression::isSuppressed(const PathDiagnosticLocation &Location, // declaration that isn't TranslationUnitDecl, because we should respect // attributes on the entire declaration chain. while (true) { + + // Template specializations (e.g., Wrapper<int>) should inherit + // suppression attributes from their primary template or partial + // specialization. Transform specializations to their template definitions + // before checking for suppressions or walking up the lexical parent + // chain. + DeclWithIssue = + preferTemplateDefinitionForTemplateSpecializations(DeclWithIssue); + // Use the "lexical" parent. Eg., if the attribute is on a class, suppress // warnings in inline methods but not in out-of-line methods. const Decl *Parent = diff --git a/clang/test/Analysis/suppress-namespace-redecl-core.cpp b/clang/test/Analysis/suppress-namespace-redecl-core.cpp new file mode 100644 index 0000000000000..500accdc79d72 --- /dev/null +++ b/clang/test/Analysis/suppress-namespace-redecl-core.cpp @@ -0,0 +1,33 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s +// expected-no-diagnostics + +void clang_analyzer_warnIfReached(); + +// Forward declaration +namespace N { // 1st +template <class T> struct Wrapper; +} // namespace N + +// This becomes the lexical parent for implicit specializations +namespace N { // 2nd +template <class T> struct Wrapper; // DeclWithIssue changes to this 1st. +template <class T> void trigger() { + Wrapper<T>::get(); +} +} // namespace N + +// [[clang::suppress]] is here, at the primary template +namespace N { // 3rd +template <class T> struct Wrapper { + static void get() { + // This [[clang::suppress]] should suppress the warning. + // Bug: Without the fix, it doesn't work because the implicit + // specialization's lexical parent points to second namespace block. + [[clang::suppress]] clang_analyzer_warnIfReached(); // no-warning + } +}; +} // namespace N + +void rdar_168941095() { + N::trigger<int>(); +} diff --git a/clang/test/Analysis/suppress-namespace-redecl-webkit.cpp b/clang/test/Analysis/suppress-namespace-redecl-webkit.cpp new file mode 100644 index 0000000000000..290b8b5f02af5 --- /dev/null +++ b/clang/test/Analysis/suppress-namespace-redecl-webkit.cpp @@ -0,0 +1,85 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncheckedCallArgsChecker -std=c++20 -verify %s +// expected-no-diagnostics + +// Test: [[clang::suppress]] fails for implicit template specializations when +// namespace has multiple redeclarations and suppression is in a different +// redeclaration than the lexical parent of the implicit specialization. +// +// The bug occured when: +// 1. A template is forward-declared in one namespace block +// 2. A partial specialization with [[clang::suppress]] is in a different block +// 3. An implicit instantiation's getLexicalDeclContext() points to the wrong block + +typedef __SIZE_TYPE__ size_t; + +namespace std { +template<class T> struct remove_cvref { using type = T; }; +template<class T> struct remove_cvref<T&> { using type = T; }; +template<class T> using remove_cvref_t = typename remove_cvref<T>::type; + +template<class... Ts> struct tuple {}; +template<size_t I, class T> struct tuple_element; +template<class T, class... Rs> struct tuple_element<0, tuple<T, Rs...>> { using type = T; }; +template<size_t I, class T, class... Rs> struct tuple_element<I, tuple<T, Rs...>> : tuple_element<I-1, tuple<Rs...>> {}; +template<size_t I, class T> using tuple_element_t = typename tuple_element<I, T>::type; + +template<class T> struct optional { T v; T&& operator*() && { return static_cast<T&&>(v); } operator bool() const { return true; } }; +template<class T, class... As> optional<T> make_optional(As...); +} // namespace std + +namespace WTF { template<class T> T&& move(T&); } + +void WTFCrash(); +template<class T> struct CanMakeCheckedPtrBase { + void incrementCheckedPtrCount() const { ++m; } + void decrementCheckedPtrCount() const { if (!m) WTFCrash(); --m; } + mutable T m{}; +}; +struct Checked : CanMakeCheckedPtrBase<unsigned> {}; +Checked* get(); + +// First namespace block - forward declaration +# 1 "Header1.h" 1 +namespace N { +template<class> struct C; +template<class T> struct C<T*> { + template<class D> static std::optional<T*> decode(D&) { return {get()}; } +}; +} // namespace N +# 50 "test.cpp" 2 + +// Second namespace block - this becomes the lexical parent for implicit specializations +# 1 "Header2.h" 1 +namespace N { +template<class> struct C; +struct Dec { + template<class T> std::optional<T> decode() { return {C<std::remove_cvref_t<T>>::decode(*this)}; } +}; +} // namespace N +# 100 "test.cpp" 2 + +// Third namespace block - [[clang::suppress]] is here +# 1 "Header3.h" 1 +namespace N { +template<class... Es> struct C<std::tuple<Es...>> { + template<class D, class... Os> + static std::optional<std::tuple<Es...>> decode(D& d, std::optional<Os>&&... os) { + if constexpr (sizeof...(Os) < sizeof...(Es)) { + auto o = d.template decode<std::tuple_element_t<sizeof...(Os), std::tuple<Es...>>>(); + if (!o) return {}; + return decode(d, WTF::move(os)..., WTF::move(o)); + } else { + // This [[clang::suppress]] should suppress the warning. + // Bug: Without the fix, it doesn't work because the implicit + // specialization's lexical parent points to Header2.h's namespace block. + [[clang::suppress]] return std::make_optional<std::tuple<Es...>>(*WTF::move(os)...); + } + } +}; +} // namespace N +# 150 "test.cpp" 2 + +void rdar_168941095() { + N::Dec d; + (void)d.decode<std::tuple<Checked*, Checked*>>(); +} `````````` </details> https://github.com/llvm/llvm-project/pull/178441 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
