Fix ordering of PassExtension registry and LibraryHandles registry construction

[serge-sans-paille]

Summary

Fix ordering of PassExtension registry and LibraryHandles registry construction

Because PassExtension registry (may) hold reference to function obtained through
dlsym, it must be constructed *after* the libraryHandles. To enforce that order, the ManagedStatic holding the library handles is dereferenced (hence constructed) before each call ro the PassExtension registry.

This effectively fixes llvm/test/Feature/load_extension.ll that exhibit the
problem when Polly is linked statically within LLVM.

[serge-sans-paille]

With this patch applied, I've validated on OSX and Linux that load_extension.ll passes the validation with Polly linked both statically and dynamically.

[lebedev.ri]

Not really the code i know.
Can you spell out in the description *why* void DynamicLibrary::ensureConstructed() { (void)*OpenedHandles; } solves the issue?
Adding some more potential reviewers.

[serge-sans-paille]

@lebedev.ri description updated.

[Meinersbur]

I suspect D71959 tries to solve the same problem

[serge-sans-paille]

Obsoleted by https://reviews.llvm.org/D71959

llvm/include/llvm/Support/DynamicLibrary.h

public:
     }
 
     /// This functions permanently adds the symbol \p symbolName with the
     /// value \p symbolValue.  These symbols are searched before any
     /// libraries.
     /// Add searchable symbol/value pair.
     static void AddSymbol(StringRef symbolName, void *symbolValue);
 
+    /// Ensure the underlying library registry is properly constructed.
+    /// This is only useful to force ManagedStatic registration ordering.
+    static void ensureConstructed();
+
     class HandleSet;
   };
 
 } // End sys namespace
 } // End llvm namespace
 
 #endif

llvm/lib/Support/DynamicLibrary.cpp

 }
 }
 
 void DynamicLibrary::AddSymbol(StringRef SymbolName, void *SymbolValue) {
   SmartScopedLock<true> Lock(*SymbolsMutex);
   (*ExplicitSymbols)[SymbolName] = SymbolValue;
 }
 
+void DynamicLibrary::ensureConstructed() { (void)*OpenedHandles; }
+
 DynamicLibrary DynamicLibrary::getPermanentLibrary(const char *FileName,
                                                    std::string *Err) {
   // Force OpenedHandles to be added into the ManagedStatic list before any
   // ManagedStatic can be added from static constructors in HandleSet::DLOpen.
   HandleSet& HS = *OpenedHandles;
 
   void *Handle = HandleSet::DLOpen(FileName, Err);
   if (Handle != &Invalid) {

llvm/lib/Transforms/IPO/PassManagerBuilder.cpp

 #include "llvm/Analysis/Passes.h"
 #include "llvm/Analysis/ScopedNoAliasAA.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TypeBasedAliasAnalysis.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Transforms/AggressiveInstCombine/AggressiveInstCombine.h"
 #include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/IPO/Attributor.h"
 #include "llvm/Transforms/IPO/ForceFunctionAttrs.h"
 #include "llvm/Transforms/IPO/FunctionAttrs.h"
 #include "llvm/Transforms/IPO/InferFunctionAttrs.h"
 #include "llvm/Transforms/InstCombine/InstCombine.h"

 /// Set of global extensions, automatically added as part of the standard set.
 static ManagedStatic<SmallVector<std::pair<PassManagerBuilder::ExtensionPointTy,
    PassManagerBuilder::ExtensionFn>, 8> > GlobalExtensions;
 
 /// Check if GlobalExtensions is constructed and not empty.
 /// Since GlobalExtensions is a managed static, calling 'empty()' will trigger
 /// the construction of the object.
 static bool GlobalExtensionsNotEmpty() {
+  // Dynamic library static registry **must** be initialized before global
+  // extension registry, because global extension may reference functions from
+  // dynamically opened libraries. Failing to ensure that order (randomly)
+  // triggers a segfault upon destruction, if the dynamically loaded library is
+  // closed before the global extensions are destroyed.
+  sys::DynamicLibrary::ensureConstructed();
   return GlobalExtensions.isConstructed() && !GlobalExtensions->empty();
 }
 
 void PassManagerBuilder::addGlobalExtension(
     PassManagerBuilder::ExtensionPointTy Ty,
     PassManagerBuilder::ExtensionFn Fn) {
+  // See comment in GlobalExtensionsNotEmpty
+  sys::DynamicLibrary::ensureConstructed();
   GlobalExtensions->push_back(std::make_pair(Ty, std::move(Fn)));
 }
 
 void PassManagerBuilder::addExtension(ExtensionPointTy Ty, ExtensionFn Fn) {
   Extensions.push_back(std::make_pair(Ty, std::move(Fn)));
 }
 
 void PassManagerBuilder::addExtensionsToPM(ExtensionPointTy ETy,

llvm/test/Feature/load_extension.ll

-; This is currently failing on multiple platforms. Disable while investigation occurs.
-; XFAIL: *
-
 ; RUN: opt %s %loadbye -goodbye -wave-goodbye -disable-output 2>&1 | FileCheck %s
-; REQUIRES: plugins, examples
+; REQUIRES: plugins
 ; CHECK: Bye
 
 @junk = global i32 0
 
 define i32* @somefunk() {
   ret i32* @junk
 }