https://github.com/kikairoya updated https://github.com/llvm/llvm-project/pull/185141
>From 81f4f9d0d0d18e039b52b8fec481e13cc19ef298 Mon Sep 17 00:00:00 2001 From: kikairoya <[email protected]> Date: Sat, 14 Mar 2026 07:22:32 +0900 Subject: [PATCH] [LLVM][Support] Enforce proper instantiations of `Registry<T>` Previously, `Registry<T>` and `LLVM_INSTANTIATE_REGISTRY` had some problems: - Asymmetry of declaration and definition. A bare explicit instantiation declaration is required in a header file, while the corresponding explicit instantiation definition is hidden behind the macro `LLVM_INSTANTIATE_REGISTRY`. - Hard to notice missing or incorrect instantiations. Since all of related definitions are visible, the compiler implicitly instantiates `Registry`s in each module(exe/dll)s. This silently breaks dylib builds on Windows (including MinGW and Cygwin). - No support macro for `Registry<T, P...>`, that has extra type parameters. `LLVM_INSTANTIATE_REGISTRY` cannot work with an extra type parameter. I could not find a way to extend it with an interface consistent with the original variant, which has no extra type parameters. This patch defines a new pair of macros, `LLVM_DECLARE_REGISTRY` and `LLVM_DEFINE_REGISTRY`. The problems are solved as follows: - A symmetrical interface between declarations and definitions. In a header file, just put this: ``` LLVM_DECLARE_REGISTRY(SomeExtensionRegistry) ``` In a source file, the syntax is very similar: ``` LLVM_DEFINE_REGISTRY(SomeExtensionRegistry) ``` - A mechanism to detect incorrect or missing explicit instantiations in the link phase. The pointers, `Head` and `Tail`, which reference registered objects via a linked list, are split out into static local variables in a free function template. Since the declaration and definition of each function template specialization are wrapped in `LLVM_DECLARE_REGISTRY` and `LLVM_DEFINE_REGISTRY` respectively, the pointers' storage is not instantiated if the instantiation declaration is missing. - Full support for extra type parameters. The free function template mentioned above is parameterized over `Registry<T, P...>`, so it does not matter how many types are passed to the `Registry` class template. The old method, the combination of a bare instantiation declaration and `LLVM_INSTANTIATE_REGISTRY`, is still works without requiring any changes in downstream. --- .../Core/TUSummary/ExtractorRegistry.h | 5 +- .../Core/TUSummary/ExtractorRegistry.cpp | 5 +- llvm/include/llvm/Support/Registry.h | 138 ++++++++++++++---- 3 files changed, 115 insertions(+), 33 deletions(-) diff --git a/clang/include/clang/ScalableStaticAnalysisFramework/Core/TUSummary/ExtractorRegistry.h b/clang/include/clang/ScalableStaticAnalysisFramework/Core/TUSummary/ExtractorRegistry.h index a49d0e5faeeb1..731cf5f1bf99b 100644 --- a/clang/include/clang/ScalableStaticAnalysisFramework/Core/TUSummary/ExtractorRegistry.h +++ b/clang/include/clang/ScalableStaticAnalysisFramework/Core/TUSummary/ExtractorRegistry.h @@ -40,9 +40,6 @@ using TUSummaryExtractorRegistry = } // namespace clang::ssaf -namespace llvm { -extern template class CLANG_TEMPLATE_ABI - Registry<clang::ssaf::TUSummaryExtractor, clang::ssaf::TUSummaryBuilder &>; -} // namespace llvm +LLVM_DECLARE_REGISTRY(clang::ssaf::TUSummaryExtractorRegistry) #endif // LLVM_CLANG_SCALABLESTATICANALYSISFRAMEWORK_CORE_TUSUMMARY_EXTRACTORREGISTRY_H diff --git a/clang/lib/ScalableStaticAnalysisFramework/Core/TUSummary/ExtractorRegistry.cpp b/clang/lib/ScalableStaticAnalysisFramework/Core/TUSummary/ExtractorRegistry.cpp index 3dcfef7fc7b88..05851b8555b50 100644 --- a/clang/lib/ScalableStaticAnalysisFramework/Core/TUSummary/ExtractorRegistry.cpp +++ b/clang/lib/ScalableStaticAnalysisFramework/Core/TUSummary/ExtractorRegistry.cpp @@ -12,10 +12,7 @@ using namespace clang; using namespace ssaf; -// FIXME: LLVM_INSTANTIATE_REGISTRY can't be used here because it drops extra -// type parameters. -template class CLANG_EXPORT_TEMPLATE - llvm::Registry<TUSummaryExtractor, TUSummaryBuilder &>; +LLVM_DEFINE_REGISTRY(clang::ssaf::TUSummaryExtractorRegistry) bool ssaf::isTUSummaryExtractorRegistered(llvm::StringRef SummaryName) { for (const auto &Entry : TUSummaryExtractorRegistry::entries()) diff --git a/llvm/include/llvm/Support/Registry.h b/llvm/include/llvm/Support/Registry.h index 2c7b479eb91b9..e9b3c24977e3c 100644 --- a/llvm/include/llvm/Support/Registry.h +++ b/llvm/include/llvm/Support/Registry.h @@ -45,11 +45,74 @@ namespace detail { template <typename R> struct IsRegistryType : std::false_type {}; template <typename T, typename... CtorParamTypes> struct IsRegistryType<Registry<T, CtorParamTypes...>> : std::true_type {}; + +/// The endpoint to the instance to hold registered components by a linked list. +/// +/// This is split out from `Registry<T>` to guard against absence of or error in +/// an explicit instantiation, which causes an implicit instantiation. +/// Such instantiation breaks dylib buils on Windows because a reference to a +/// non dllimport-ed implicitly instantiated global variable can't be shared +/// across a DLL boundary. +template <typename R> struct RegistryLinkListStorage { + typename R::node *Head; + typename R::node *Tail; +}; + +/// The accessor to the endpoint. +/// +/// ATTENTION: Be careful when attempting to "refactor" or "simplify" this. +/// +/// - This shall not be a member of `RegistryLinkList`, to control instantiation +/// timing separately. +/// +/// - The body also shall not be inlined here; otherwise, the guard from a +/// missed explicit instantiation definition won't work. +/// +/// - Use an accessor function that returns a reference to a function-local +/// static variable, rather than using a variable template or a static member +/// variable in a class template directly since functions can be imported +/// without the `dllimport` annotation. +template <typename R> RegistryLinkListStorage<R> &getRegistryLinkListInstance(); + +/// Utility to guard against missing declarations or mismatched use of +/// `LLVM_DECLARE_REGISTRY` and `LLVM_INSTANTIATE_REGISTRY`. +template <typename R> +struct RegistryLinkListDeclarationMarker : std::false_type {}; } // namespace detail /// A global registry used in conjunction with static constructors to make /// pluggable components (like targets or garbage collectors) "just work" when /// linked with an executable. +/// +/// To provide a Registry interface, follow these steps: +/// +/// 1. Define your plugin base interface. The interface must have a virtual +/// destructor and the appropriate dllexport/dllimport/visibility annotation. +/// +/// namespace your_ns { +/// class YOURLIB_ABI SomethingPluginBase { +/// virtual ~SomethingPluginBase() = default; +/// virtual void TheInterface() = 0; +/// }; +/// } +/// +/// 2. Declare an alias to your `Registry` for convenience. +/// +/// namespace your_ns { +/// using YourRegistry = llvm::Registry<SomethingPluginBase>; +/// } +/// +/// 3. Declare the specialization of the `Registry` with `LLVM_DECLARE_REGISTRY` +/// in the global namespace. The declaration must be placed before any use of +/// the `YourRegistry`. +/// +/// LLVM_DECLARE_REGISTRY(your_ns::YourRegistry) +/// +/// 4. In a .cpp file, define the specialization with `LLVM_DEFINE_REGISTRY` +/// in the global namespace. +/// +/// LLVM_DEFINE_REGISTRY(your_ns::YourRegistry) +/// template <typename T, typename... CtorParamTypes> class Registry { static_assert( !detail::IsRegistryType<T>::value, @@ -67,13 +130,6 @@ template <typename T, typename... CtorParamTypes> class Registry { Registry() = delete; friend class node; - // These must be must two separate declarations to workaround a 20 year - // old MSVC bug with dllexport and multiple static fields in the same - // declaration causing error C2487 "member of dll interface class may not - // be declared with dll interface". - // https://developercommunity.visualstudio.com/t/c2487-in-dllexport-class-with-static-members/69878 - static inline node *Head = nullptr; - static inline node *Tail = nullptr; public: /// Node in linked list of entries. @@ -92,11 +148,8 @@ template <typename T, typename... CtorParamTypes> class Registry { /// Add a node to the Registry: this is the interface between the plugin and /// the executable. /// - /// This function is exported by the executable and called by the plugin to - /// add a node to the executable's registry. Therefore it's not defined here - /// to avoid it being instantiated in the plugin and is instead defined in - /// the executable (see LLVM_INSTANTIATE_REGISTRY below). static void add_node(node *N) { + auto &[Head, Tail] = detail::getRegistryLinkListInstance<Registry>(); if (Tail) Tail->Next = N; else @@ -122,9 +175,9 @@ template <typename T, typename... CtorParamTypes> class Registry { const entry &operator*() const { return Cur->Val; } }; - // begin is not defined here in order to avoid usage of an undefined static - // data member, instead it's instantiated by LLVM_INSTANTIATE_REGISTRY. - static iterator begin() { return iterator(Head); } + static iterator begin() { + return iterator(detail::getRegistryLinkListInstance<Registry>().Head); + } static iterator end() { return iterator(nullptr); } static iterator_range<iterator> entries() { @@ -155,23 +208,58 @@ template <typename T, typename... CtorParamTypes> class Registry { } // end namespace llvm -#ifdef _WIN32 -/// Instantiate a registry class. -#define LLVM_INSTANTIATE_REGISTRY(REGISTRY_CLASS) \ +/// Helper macro to declare registry class. +/// +#define LLVM_DECLARE_REGISTRY(REGISTRY_CLASS) \ + namespace llvm::detail { \ + template <> \ + struct RegistryLinkListDeclarationMarker<REGISTRY_CLASS> : std::true_type { \ + }; \ + template <> \ + RegistryLinkListStorage<REGISTRY_CLASS> & \ + getRegistryLinkListInstance<REGISTRY_CLASS>(); \ + } + +/// Helper macro to define registry class. +/// +/// Technically, these macros don't instantiate `Registry` despite the name. +/// They handle underlying storage that used by `Registry` indirectly, enforcing +/// proper declarations/definitions by compilers or linkers. If you forget to +/// place `LLVM_DEFINE_REGISTRY`, your linker will complain about a missing +/// `getRegistryLinkListInstance` definiton. +#define LLVM_DEFINE_REGISTRY(REGISTRY_CLASS) \ + namespace llvm::detail { \ + static_assert(RegistryLinkListDeclarationMarker<REGISTRY_CLASS>::value, \ + "Missing matching registry delcaration of " #REGISTRY_CLASS \ + ". Place `LLVM_DECLARE_REGISTRY(" #REGISTRY_CLASS \ + ")` in a header."); \ + template <> \ + LLVM_ABI_EXPORT RegistryLinkListStorage<REGISTRY_CLASS> & \ + getRegistryLinkListInstance<REGISTRY_CLASS>() { \ + static RegistryLinkListStorage<REGISTRY_CLASS> Instance; \ + return Instance; \ + } \ + } + +#define LLVM_DETAIL_INSTANTIATE_REGISTRY_1(ABITAG, REGISTRY_CLASS) \ + LLVM_DECLARE_REGISTRY(REGISTRY_CLASS) \ + LLVM_DEFINE_REGISTRY(REGISTRY_CLASS) \ namespace llvm { \ - template class LLVM_ABI_EXPORT Registry<REGISTRY_CLASS::type>; \ + template class ABITAG Registry<REGISTRY_CLASS::type>; \ static_assert(!REGISTRY_CLASS::HasCtorParamTypes, \ "LLVM_INSTANTIATE_REGISTRY can't be used with extra " \ - "constructor parameter types"); \ + "constructor parameter types. Use " \ + "LLVM_DECLARE/DEFINE_REGISTRY istead."); \ } + +/// Old style helper macro to instantiate registry class. +/// +#ifdef _WIN32 +#define LLVM_INSTANTIATE_REGISTRY(REGISTRY_CLASS) \ + LLVM_DETAIL_INSTANTIATE_REGISTRY_1(LLVM_ABI_EXPORT, REGISTRY_CLASS) #else #define LLVM_INSTANTIATE_REGISTRY(REGISTRY_CLASS) \ - namespace llvm { \ - template class Registry<REGISTRY_CLASS::type>; \ - static_assert(!REGISTRY_CLASS::HasCtorParamTypes, \ - "LLVM_INSTANTIATE_REGISTRY can't be used with extra " \ - "constructor parameter types"); \ - } + LLVM_DETAIL_INSTANTIATE_REGISTRY_1(, REGISTRY_CLASS) #endif #endif // LLVM_SUPPORT_REGISTRY_H _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
