[Orc] Fix global variable destructor function support when --jit-kind=orc-lazy

[StephenFan]

Summary

The bug was reported here https://bugs.llvm.org/show_bug.cgi?id=52030

This patch follows the idea that @lhames commented in the above webpage.

[StephenFan]

clang-format

[StephenFan]

Rename file.

[StephenFan]

Remove blank line

[lhames]

LGTM. Thanks Stephen!

llvm/lib/ExecutionEngine/Orc/LLJIT.cpp

486

It would be good to use a DeInitFunctionPrefix here (along the same lines as InitFunctionPrefix).

Once the deinit functions are named with a known prefix we can look them up in GenericLLVMIRPlatformSupport::notifyAdding and add them to the DeInitFunctions map automatically. This will make destructors work from cached objects. This could be done in this patch, or in a follow-up.

[awarzynski]

Solves my original problem that lead to https://bugs.llvm.org/show_bug.cgi?id=52030. Thank you for working on this!

[StephenFan]

Add deInitFuctionPrefix

[StephenFan]

Thanks for your review! @lhames
Thanks for confirming! @awarzynski

[StephenFan]

llvm/lib/ExecutionEngine/Orc/LLJIT.cpp

486

Once the deinit functions are named with a known prefix we can look them up in GenericLLVMIRPlatformSupport::notifyAdding and add them to the DeInitFunctions map automatically. This will make destructors work from cached objects. This could be done in this patch, or in a follow-up.

I have a question that what do cached objects mean in ORC ? After looking through the source code, I found that when define the symbol in JITDylib, the notifyAdding will be called, and when emit the symbol (MaterializationUnit), the layer will call the transform function to transform the module, in which the GlobalCtorDtorScraper::operator() will be called, is my understand right ?

[lhames]

llvm/lib/ExecutionEngine/Orc/LLJIT.cpp

486

Hi Stephen,

I have a question that what do cached objects mean in ORC ?

By "cached objects", we mean stored object files that were created by an ObjectCache instance from LLVM IR that was added to the JIT.

In MCJIT we used to discover constructors at the IR layer only by looking for llvm.global_ctors instances. In GenericLLVMIRPlatformSupport we rewrite llvm.global_ctors instances into functions with reserved names that we can look for at the object level. We could do the same thing with llvm.global_dtors.

This approach is a stop-gap until we have generic JITLink support on all platforms. Once we have JITLink everywhere we can remove GenericLLVMIRPlatformSupport entirely.

[StephenFan]

llvm/lib/ExecutionEngine/Orc/LLJIT.cpp

486

Thanks for your explanation! @lhames

llvm/lib/ExecutionEngine/Orc/LLJIT.cpp

private:
   GenericLLVMIRPlatformSupport &S;
 };
 
 /// This transform parses llvm.global_ctors to produce a single initialization
 /// function for the module, records the function, then deletes
 /// llvm.global_ctors.
 class GlobalCtorDtorScraper {
 public:
-
   GlobalCtorDtorScraper(GenericLLVMIRPlatformSupport &PS,
-                        StringRef InitFunctionPrefix)
-    : PS(PS), InitFunctionPrefix(InitFunctionPrefix) {}
+                        StringRef InitFunctionPrefix,
+                        StringRef DeInitFunctionPrefix)
+      : PS(PS), InitFunctionPrefix(InitFunctionPrefix),
+        DeInitFunctionPrefix(DeInitFunctionPrefix) {}
   Expected<ThreadSafeModule> operator()(ThreadSafeModule TSM,
                                         MaterializationResponsibility &R);
 
 private:
   GenericLLVMIRPlatformSupport &PS;
   StringRef InitFunctionPrefix;
+  StringRef DeInitFunctionPrefix;
 };
 
 /// Generic IR Platform Support
 ///
 /// Scrapes llvm.global_ctors and llvm.global_dtors and replaces them with
 /// specially named 'init' and 'deinit'. Injects definitions / interposes for
 /// some runtime API, including __cxa_atexit, dlopen, and dlclose.
 class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
 public:
   GenericLLVMIRPlatformSupport(LLJIT &J)
-      : J(J), InitFunctionPrefix(J.mangle("__orc_init_func.")) {
+      : J(J), InitFunctionPrefix(J.mangle("__orc_init_func.")),
+        DeInitFunctionPrefix(J.mangle("__orc_deinit_func.")) {
 
     getExecutionSession().setPlatform(
         std::make_unique<GenericLLVMIRPlatform>(*this));
 
-    setInitTransform(J, GlobalCtorDtorScraper(*this, InitFunctionPrefix));
+    setInitTransform(J, GlobalCtorDtorScraper(*this, InitFunctionPrefix,
+                                              DeInitFunctionPrefix));
 
     SymbolMap StdInterposes;
 
     StdInterposes[J.mangleAndIntern("__lljit.platform_support_instance")] =
         JITEvaluatedSymbol(pointerToJITTargetAddress(this),
                            JITSymbolFlags::Exported);
     StdInterposes[J.mangleAndIntern("__lljit.cxa_atexit_helper")] =
         JITEvaluatedSymbol(pointerToJITTargetAddress(registerAtExitHelper),
else {
       // an init function. Add the symbol to both the InitSymbols map (which
       // will trigger a lookup to materialize the module) and the InitFunctions
       // map (which holds the names of the symbols to execute).
       for (auto &KV : MU.getSymbols())
         if ((*KV.first).startswith(InitFunctionPrefix)) {
           InitSymbols[&JD].add(KV.first,
                                SymbolLookupFlags::WeaklyReferencedSymbol);
           InitFunctions[&JD].add(KV.first);
+        } else if ((*KV.first).startswith(DeInitFunctionPrefix)) {
+          DeInitFunctions[&JD].add(KV.first);
         }
     }
     return Error::success();
   }
 
   Error initialize(JITDylib &JD) override {
     LLVM_DEBUG({
       dbgs() << "GenericLLVMIRPlatformSupport getting initializers to run\n";
public:
   }
 
   void registerInitFunc(JITDylib &JD, SymbolStringPtr InitName) {
     getExecutionSession().runSessionLocked([&]() {
         InitFunctions[&JD].add(InitName);
       });
   }
 
+  void registerDeInitFunc(JITDylib &JD, SymbolStringPtr DeInitName) {
+    getExecutionSession().runSessionLocked(
+        [&]() { DeInitFunctions[&JD].add(DeInitName); });
+  }
+
 private:
 
   Expected<std::vector<JITTargetAddress>> getInitializers(JITDylib &JD) {
     if (auto Err = issueInitLookups(JD))
       return std::move(Err);
 
     DenseMap<JITDylib *, SymbolLookupSet> LookupSymbols;
     std::vector<JITDylibSP> DFSLinkOrder;
addHelperAndWrapper(
         GlobalValue::DefaultVisibility, "__lljit.cxa_atexit_helper",
         {PlatformInstanceDecl});
 
     return ThreadSafeModule(std::move(M), std::move(Ctx));
   }
 
   LLJIT &J;
   std::string InitFunctionPrefix;
+  std::string DeInitFunctionPrefix;
   DenseMap<JITDylib *, SymbolLookupSet> InitSymbols;
   DenseMap<JITDylib *, SymbolLookupSet> InitFunctions;
   DenseMap<JITDylib *, SymbolLookupSet> DeInitFunctions;
   ItaniumCXAAtExitSupport AtExitMgr;
 };
 
 Error GenericLLVMIRPlatform::setupJITDylib(JITDylib &JD) {
   return S.setupJITDylib(JD);
 }
 
 Error GenericLLVMIRPlatform::notifyAdding(ResourceTracker &RT,
                                           const MaterializationUnit &MU) {
   return S.notifyAdding(RT, MU);
 }
 
 Expected<ThreadSafeModule>
 GlobalCtorDtorScraper::operator()(ThreadSafeModule TSM,
                                   MaterializationResponsibility &R) {
   auto Err = TSM.withModuleDo([&](Module &M) -> Error {
     auto &Ctx = M.getContext();
     auto *GlobalCtors = M.getNamedGlobal("llvm.global_ctors");
+    auto *GlobalDtors = M.getNamedGlobal("llvm.global_dtors");
 
-    // If there's no llvm.global_ctors or it's just a decl then skip.
-    if (!GlobalCtors || GlobalCtors->isDeclaration())
-      return Error::success();
-
-    std::string InitFunctionName;
-    raw_string_ostream(InitFunctionName)
+    auto RegisterCOrDtors = [&](GlobalVariable *GlobalCOrDtors,
+                                bool isCtor) -> Error {
+      // If there's no llvm.global_c/dtor or it's just a decl then skip.
+      if (!GlobalCOrDtors || GlobalCOrDtors->isDeclaration())
+        return Error::success();
+      std::string InitOrDeInitFunctionName;
+      if (isCtor)
+        raw_string_ostream(InitOrDeInitFunctionName)
             << InitFunctionPrefix << M.getModuleIdentifier();
+      else
+        raw_string_ostream(InitOrDeInitFunctionName)
+            << DeInitFunctionPrefix << M.getModuleIdentifier();
 
       MangleAndInterner Mangle(PS.getExecutionSession(), M.getDataLayout());
-    auto InternedName = Mangle(InitFunctionName);
-    if (auto Err =
-            R.defineMaterializing({{InternedName, JITSymbolFlags::Callable}}))
+      auto InternedInitOrDeInitName = Mangle(InitOrDeInitFunctionName);
+      if (auto Err = R.defineMaterializing(
+              {{InternedInitOrDeInitName, JITSymbolFlags::Callable}}))
         return Err;
 
-    auto *InitFunc =
-        Function::Create(FunctionType::get(Type::getVoidTy(Ctx), {}, false),
-                         GlobalValue::ExternalLinkage, InitFunctionName, &M);
-    InitFunc->setVisibility(GlobalValue::HiddenVisibility);
-    std::vector<std::pair<Function *, unsigned>> Inits;
-    for (auto E : getConstructors(M))
-      Inits.push_back(std::make_pair(E.Func, E.Priority));
-    llvm::sort(Inits, [](const std::pair<Function *, unsigned> &LHS,
-                         const std::pair<Function *, unsigned> &RHS) {
+      auto *InitOrDeInitFunc = Function::Create(
+          FunctionType::get(Type::getVoidTy(Ctx), {}, false),
+          GlobalValue::ExternalLinkage, InitOrDeInitFunctionName, &M);
+      InitOrDeInitFunc->setVisibility(GlobalValue::HiddenVisibility);
+      std::vector<std::pair<Function *, unsigned>> InitsOrDeInits;
+      auto COrDtors = isCtor ? getConstructors(M) : getDestructors(M);
+
+      for (auto E : COrDtors)
+        InitsOrDeInits.push_back(std::make_pair(E.Func, E.Priority));
+      llvm::sort(InitsOrDeInits,
+                 [](const std::pair<Function *, unsigned> &LHS,
                    return LHS.first < RHS.first;
                  });
-    auto *EntryBlock = BasicBlock::Create(Ctx, "entry", InitFunc);
-    IRBuilder<> IB(EntryBlock);
-    for (auto &KV : Inits)
+
+      auto *InitOrDeInitFuncEntryBlock =
+          BasicBlock::Create(Ctx, "entry", InitOrDeInitFunc);
+      IRBuilder<> IB(InitOrDeInitFuncEntryBlock);
+      for (auto &KV : InitsOrDeInits)
         IB.CreateCall(KV.first);
       IB.CreateRetVoid();
 
-    PS.registerInitFunc(R.getTargetJITDylib(), InternedName);
-    GlobalCtors->eraseFromParent();
+      if (isCtor)
+        PS.registerInitFunc(R.getTargetJITDylib(), InternedInitOrDeInitName);
+      else
+        PS.registerDeInitFunc(R.getTargetJITDylib(), InternedInitOrDeInitName);
+
+      GlobalCOrDtors->eraseFromParent();
+      return Error::success();
+    };
+
+    if (auto Err = RegisterCOrDtors(GlobalCtors, true))
+      return Err;
+    if (auto Err = RegisterCOrDtors(GlobalDtors, false))
+      return Err;
+
     return Error::success();
   });
 
   if (Err)
     return std::move(Err);
 
   return std::move(TSM);
 }

llvm/test/ExecutionEngine/OrcLazy/global-ctors-and-dtors.ll

 ; Test that global constructors and destructors are run:
 ;
 ; RUN: lli -jit-kind=orc-lazy -orc-lazy-debug=funcs-to-stdout -extra-module %s \
 ; RUN:   %S/Inputs/noop-main.ll | FileCheck %s
 ;
 ; Test that this is true for global constructors and destructors in other
 ; JITDylibs.
 ; RUN: lli -jit-kind=orc-lazy -orc-lazy-debug=funcs-to-stdout \
 ; RUN:   -jd extra -extra-module %s -jd main %S/Inputs/noop-main.ll | FileCheck %s
 ;
 ; CHECK: Hello
 ; CHECK: [ {{.*}}main{{.*}} ]
 ; CHECK: Goodbye
+; CHECK: Goodbye again
 
 %class.Foo = type { i8 }
 
 @f = global %class.Foo zeroinitializer, align 1
 @__dso_handle = external global i8
[e-mail]_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I_hello.cpp, i8* null }]
[e-mail]_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I_hello.cpp, i8* null }] 
[e-mail]_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @printf_wrapper, i8* null }]
 @str = private unnamed_addr constant [6 x i8] c"Hello\00"
 @str2 = private unnamed_addr constant [8 x i8] c"Goodbye\00"
+@str3 = global [14 x i8] c"Goodbye again\00"
 
 define linkonce_odr void @_ZN3FooD1Ev(%class.Foo* nocapture readnone %this) unnamed_addr align 2 {
 entry:
   %puts.i = tail call i32 @puts(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @str2, i64 0, i64 0))
   ret void
 }
 
 declare i32 @__cxa_atexit(void (i8*)*, i8*, i8*)
 
 define internal void @_GLOBAL__sub_I_hello.cpp() {
 entry:
   %puts.i.i.i = tail call i32 @puts(i8* getelementptr inbounds ([6 x i8], [6 x i8]* @str, i64 0, i64 0))
   %0 = tail call i32 @__cxa_atexit(void (i8*)* bitcast (void (%class.Foo*)* @_ZN3FooD1Ev to void (i8*)*), i8* getelementptr inbounds (%class.Foo, %class.Foo* @f, i64 0, i32 0), i8* @__dso_handle)
   ret void
 }
 
+define void @printf_wrapper() {
+entry:
+  %0 = tail call i32 @puts(i8* getelementptr inbounds ([14 x i8], [14 x i8]* @str3, i64 0, i64 0))
+  ret void
+}
+
 declare i32 @puts(i8* nocapture readonly)