[PATCH] D72493: Fix ordering of PassExtension registry and LibraryHandles registry construction

[serge via Phabricator via llvm-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: Meinersbur, echristo, fhahn, RKSimon, gkistanova, bollu, marsupial.
Herald added subscribers: llvm-commits, hiraditya, mehdi_amini.
Herald added a project: LLVM.
serge-sans-paille added a comment.

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.


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.

      

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72493

Files:
  llvm/include/llvm/Support/DynamicLibrary.h
  llvm/lib/Support/DynamicLibrary.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/test/Feature/load_extension.ll


Index: llvm/test/Feature/load_extension.ll
===================================================================
--- llvm/test/Feature/load_extension.ll
+++ llvm/test/Feature/load_extension.ll
@@ -1,8 +1,5 @@
-; 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
Index: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
===================================================================
--- llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
+++ llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
@@ -27,6 +27,7 @@
 #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"
@@ -194,12 +195,20 @@
 /// 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)));
 }
 
Index: llvm/lib/Support/DynamicLibrary.cpp
===================================================================
--- llvm/lib/Support/DynamicLibrary.cpp
+++ llvm/lib/Support/DynamicLibrary.cpp
@@ -145,6 +145,8 @@
   (*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
Index: llvm/include/llvm/Support/DynamicLibrary.h
===================================================================
--- llvm/include/llvm/Support/DynamicLibrary.h
+++ llvm/include/llvm/Support/DynamicLibrary.h
@@ -123,6 +123,10 @@
     /// 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;
   };
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72493.237265.patch
Type: text/x-patch
Size: 3121 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200110/1e5ed8f4/attachment-0001.bin>