From ab2300bc154f7bed43f85f74fd3fe31be71d90e0 Mon Sep 17 00:00:00 2001 From: Elia Geretto Date: Wed, 29 Jan 2020 05:45:27 +0000 Subject: [PATCH] [PassManagerBuilder] Remove global extension when a plugin is unloaded This commit fixes PR39321. GlobalExtensions is not guaranteed to be destroyed when optimizer plugins are unloaded. If it is indeed destroyed after a plugin is dlclose-d, the destructor of the corresponding ExtensionFn is not mapped anymore, causing a call to unmapped memory during destruction. This commit guarantees that extensions coming from external plugins are removed from GlobalExtensions when the plugin is unloaded if GlobalExtensions has not been destroyed yet. Differential Revision: https://reviews.llvm.org/D71959 --- .../llvm/Transforms/IPO/PassManagerBuilder.h | 28 +++++++++++++-- llvm/lib/Transforms/IPO/PassManagerBuilder.cpp | 41 +++++++++++++++++----- 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/llvm/include/llvm/Transforms/IPO/PassManagerBuilder.h b/llvm/include/llvm/Transforms/IPO/PassManagerBuilder.h index 63ff00a..ababa1d 100644 --- a/llvm/include/llvm/Transforms/IPO/PassManagerBuilder.h +++ b/llvm/include/llvm/Transforms/IPO/PassManagerBuilder.h @@ -62,6 +62,8 @@ public: typedef std::function ExtensionFn; + typedef int GlobalExtensionID; + enum ExtensionPointTy { /// EP_EarlyAsPossible - This extension point allows adding passes before /// any other transformations, allowing them to see the code as it is coming @@ -193,7 +195,17 @@ public: /// Adds an extension that will be used by all PassManagerBuilder instances. /// This is intended to be used by plugins, to register a set of /// optimisations to run automatically. - static void addGlobalExtension(ExtensionPointTy Ty, ExtensionFn Fn); + /// + /// \returns A global extension identifier that can be used to remove the + /// extension. + static GlobalExtensionID addGlobalExtension(ExtensionPointTy Ty, + ExtensionFn Fn); + /// Removes an extension that was previously added using addGlobalExtension. + /// This is also intended to be used by plugins, to remove any extension that + /// was previously registered before being unloaded. + /// + /// \param ExtensionID Identifier of the extension to be removed. + static void removeGlobalExtension(GlobalExtensionID ExtensionID); void addExtension(ExtensionPointTy Ty, ExtensionFn Fn); private: @@ -222,10 +234,20 @@ public: /// used by optimizer plugins to allow all front ends to transparently use /// them. Create a static instance of this class in your plugin, providing a /// private function that the PassManagerBuilder can use to add your passes. -struct RegisterStandardPasses { +class RegisterStandardPasses { + PassManagerBuilder::GlobalExtensionID ExtensionID; + +public: RegisterStandardPasses(PassManagerBuilder::ExtensionPointTy Ty, PassManagerBuilder::ExtensionFn Fn) { - PassManagerBuilder::addGlobalExtension(Ty, std::move(Fn)); + ExtensionID = PassManagerBuilder::addGlobalExtension(Ty, std::move(Fn)); + } + + ~RegisterStandardPasses() { + // If the collection holding the global extensions is destroyed after the + // plugin is unloaded, the extension has to be removed here. Indeed, the + // destructor of the ExtensionFn may reference code in the plugin. + PassManagerBuilder::removeGlobalExtension(ExtensionID); } }; diff --git a/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp b/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp index 9c99283..7cfc29f 100644 --- a/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp +++ b/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp @@ -13,6 +13,7 @@ #include "llvm/Transforms/IPO/PassManagerBuilder.h" #include "llvm-c/Transforms/PassManagerBuilder.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Analysis/BasicAliasAnalysis.h" #include "llvm/Analysis/CFLAndersAliasAnalysis.h" @@ -187,8 +188,13 @@ PassManagerBuilder::~PassManagerBuilder() { } /// Set of global extensions, automatically added as part of the standard set. -static ManagedStatic, 8> > GlobalExtensions; +static ManagedStatic< + SmallVector, + 8>> + GlobalExtensions; +static PassManagerBuilder::GlobalExtensionID GlobalExtensionsCounter; /// Check if GlobalExtensions is constructed and not empty. /// Since GlobalExtensions is a managed static, calling 'empty()' will trigger @@ -197,10 +203,29 @@ static bool GlobalExtensionsNotEmpty() { return GlobalExtensions.isConstructed() && !GlobalExtensions->empty(); } -void PassManagerBuilder::addGlobalExtension( - PassManagerBuilder::ExtensionPointTy Ty, - PassManagerBuilder::ExtensionFn Fn) { - GlobalExtensions->push_back(std::make_pair(Ty, std::move(Fn))); +PassManagerBuilder::GlobalExtensionID +PassManagerBuilder::addGlobalExtension(PassManagerBuilder::ExtensionPointTy Ty, + PassManagerBuilder::ExtensionFn Fn) { + auto ExtensionID = GlobalExtensionsCounter++; + GlobalExtensions->push_back(std::make_tuple(Ty, std::move(Fn), ExtensionID)); + return ExtensionID; +} + +void PassManagerBuilder::removeGlobalExtension( + PassManagerBuilder::GlobalExtensionID ExtensionID) { + // RegisterStandardPasses may try to call this function after GlobalExtensions + // has already been destroyed; doing so should not generate an error. + if (!GlobalExtensions.isConstructed()) + return; + + auto GlobalExtension = + llvm::find_if(*GlobalExtensions, [ExtensionID](const auto &elem) { + return std::get<2>(elem) == ExtensionID; + }); + assert(GlobalExtension != GlobalExtensions->end() && + "The extension ID to be removed should always be valid."); + + GlobalExtensions->erase(GlobalExtension); } void PassManagerBuilder::addExtension(ExtensionPointTy Ty, ExtensionFn Fn) { @@ -211,8 +236,8 @@ void PassManagerBuilder::addExtensionsToPM(ExtensionPointTy ETy, legacy::PassManagerBase &PM) const { if (GlobalExtensionsNotEmpty()) { for (auto &Ext : *GlobalExtensions) { - if (Ext.first == ETy) - Ext.second(*this, PM); + if (std::get<0>(Ext) == ETy) + std::get<1>(Ext)(*this, PM); } } for (unsigned i = 0, e = Extensions.size(); i != e; ++i) -- 2.7.4