[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

479

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

479

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

479

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

479

Thanks for your explanation! @lhames

llvm/lib/ExecutionEngine/Orc/LLJIT.cpp

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;

 }
 
 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) << 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"
+@ResultHeaderStrIR = 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]* @ResultHeaderStrIR, i64 0, i64 0))
+  ret void
+}
+
 declare i32 @puts(i8* nocapture readonly)
+