Skip to content

Commit

Permalink
[PassManagerBuilder] Remove global extension when a plugin is unloaded
Browse files Browse the repository at this point in the history
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

(cherry picked from commit ab2300b)
  • Loading branch information
EliaGeretto authored and zmodem committed Jan 29, 2020
1 parent 425198b commit 52c1d20
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 11 deletions.
28 changes: 25 additions & 3 deletions llvm/include/llvm/Transforms/IPO/PassManagerBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ class PassManagerBuilder {
typedef std::function<void(const PassManagerBuilder &Builder,
legacy::PassManagerBase &PM)>
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
Expand Down Expand Up @@ -193,7 +195,17 @@ class PassManagerBuilder {
/// 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:
Expand Down Expand Up @@ -222,10 +234,20 @@ class PassManagerBuilder {
/// 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);
}
};

Expand Down
41 changes: 33 additions & 8 deletions llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -187,8 +188,13 @@ PassManagerBuilder::~PassManagerBuilder() {
}

/// Set of global extensions, automatically added as part of the standard set.
static ManagedStatic<SmallVector<std::pair<PassManagerBuilder::ExtensionPointTy,
PassManagerBuilder::ExtensionFn>, 8> > GlobalExtensions;
static ManagedStatic<
SmallVector<std::tuple<PassManagerBuilder::ExtensionPointTy,
PassManagerBuilder::ExtensionFn,
PassManagerBuilder::GlobalExtensionID>,
8>>
GlobalExtensions;
static PassManagerBuilder::GlobalExtensionID GlobalExtensionsCounter;

/// Check if GlobalExtensions is constructed and not empty.
/// Since GlobalExtensions is a managed static, calling 'empty()' will trigger
Expand All @@ -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) {
Expand All @@ -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)
Expand Down

0 comments on commit 52c1d20

Please sign in to comment.