[Interpreter][ClangRepl] Call LLJIT deinitailize on exit.

[sunho]

Summary

Calls LLJIT deinitailize on exit. This will correctly run dtors of emitted modules so that cleanup can be done properly.

[sunho]

Some notes: this is probably not related to asan errors of clang-repl popping up recently. There is no dtors emitted in related test cases. There's a chance this can fix things, though. Nevertheless, This is the right thing to do.

[sunho]

Clean up inside ~Interpreter

[sunho]

Add testcase.

[Purva-Chaudhari]

Hi @sunho Did you test the changes after stage 2 asan build? The execute.cpp fails in my asan build area after adding this patch

[sunho]

Hi @Purva-Chaudhari I haven't set up asan build setting yet. It's sad to hear it's still mem leaking. We need to look into somewhere else then...

[sunho]

@Purva-Chaudhari Could you give me the full log of asan failure for reference?

[Purva-Chaudhari]

In D127945#3588719, @sunho wrote:

Hi @Purva-Chaudhari I haven't set up asan build setting yet. It's sad to hear it's still mem leaking. We need to look into somewhere else then...

Adding this patch is causing the original execute.cpp to fail in my area (i.e without my additions which were previously causing memory leak). I would suggest to test patch in stage2 build and confirm the behavior once

Here is the log :

[sunho]

Gotcha. Let me look into it.

[sunho]

Fix in windows build requires improving on ORC side. (extending GenericLLVMIRPlatformSupport or write new platform support) I'll reopen this once I have done this groundwork. For asan failiures, I believe https://reviews.llvm.org/D127991 should be enough.

clang/lib/Interpreter/IncrementalExecutor.h

public:
   enum SymbolNameKind { IRName, LinkerName };
 
   IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC, llvm::Error &Err,
                       const llvm::Triple &Triple);
   ~IncrementalExecutor();
 
   llvm::Error addModule(std::unique_ptr<llvm::Module> M);
   llvm::Error runCtors() const;
+  llvm::Error cleanUp();
   llvm::Expected<llvm::JITTargetAddress>
   getSymbolAddress(llvm::StringRef Name, SymbolNameKind NameKind) const;
   llvm::orc::LLJIT *getExecutionEngine() const { return Jit.get(); }
 };
 
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_INTERPRETER_INCREMENTALEXECUTOR_H

clang/lib/Interpreter/IncrementalExecutor.cpp

IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC,
   else {
     Err = PSGOrErr.takeError();
     return;
   }
 }
 
 IncrementalExecutor::~IncrementalExecutor() {}
 
+llvm::Error IncrementalExecutor::cleanUp() {
+  return Jit->deinitialize(Jit->getMainJITDylib());
+}
+
 llvm::Error IncrementalExecutor::addModule(std::unique_ptr<llvm::Module> M) {
   return Jit->addIRModule(llvm::orc::ThreadSafeModule(std::move(M), TSCtx));
 }
 
 llvm::Error IncrementalExecutor::runCtors() const {
   return Jit->initialize(Jit->getMainJITDylib());
 }
 

clang/lib/Interpreter/Interpreter.cpp

Interpreter::Interpreter(std::unique_ptr<CompilerInstance> CI,
                          llvm::Error &Err) {
   llvm::ErrorAsOutParameter EAO(&Err);
   auto LLVMCtx = std::make_unique<llvm::LLVMContext>();
   TSCtx = std::make_unique<llvm::orc::ThreadSafeContext>(std::move(LLVMCtx));
   IncrParser = std::make_unique<IncrementalParser>(std::move(CI),
                                                    *TSCtx->getContext(), Err);
 }
 
-Interpreter::~Interpreter() {}
+Interpreter::~Interpreter() {
+  if (IncrExecutor) {
+    if (llvm::Error Err = IncrExecutor->cleanUp())
+      llvm::report_fatal_error(
+          llvm::Twine("Failed to clean up IncrementalExecutor: ") +
+          toString(std::move(Err)));
+  }
+}
 
 llvm::Expected<std::unique_ptr<Interpreter>>
 Interpreter::create(std::unique_ptr<CompilerInstance> CI) {
   llvm::Error Err = llvm::Error::success();
   auto Interp =
       std::unique_ptr<Interpreter>(new Interpreter(std::move(CI), Err));
   if (Err)
     return std::move(Err);

clang/test/Interpreter/execute.cpp

+// clang-format off
 // RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
 // RUN:            'auto r1 = printf("i = %d\n", i);' | FileCheck --check-prefix=CHECK-DRIVER %s
 // REQUIRES: host-supports-jit
 // UNSUPPORTED: system-aix
 // CHECK-DRIVER: i = 10
 // RUN: cat %s | clang-repl | FileCheck %s
 extern "C" int printf(const char *, ...);
 int i = 42;
 auto r1 = printf("i = %d\n", i);
 // CHECK: i = 42
 
 struct S { float f = 1.0; S *m = nullptr;} s;
 
 auto r2 = printf("S[f=%f, m=0x%llx]\n", s.f, reinterpret_cast<unsigned long long>(s.m));
 // CHECK-NEXT: S[f=1.000000, m=0x0]
+
+struct D { float f = 1.0; D *m = nullptr; D(){} ~D() { printf("D[f=%f, m=0x%llx]\n", f, reinterpret_cast<unsigned long long>(m)); }} d;
+// CHECK: D[f=1.000000, m=0x0]
+
 quit

clang/tools/clang-repl/ClangRepl.cpp

static void LLVMErrorHandler(void *UserData, const char *Message,
   exit(GenCrashDiag ? 70 : 1);
 }
 
 llvm::ExitOnError ExitOnErr;
 int main(int argc, const char **argv) {
   ExitOnErr.setBanner("clang-repl: ");
   llvm::cl::ParseCommandLineOptions(argc, argv);
 
+  llvm::llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
   std::vector<const char *> ClangArgv(ClangArgs.size());
   std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
                  [](const std::string &s) -> const char * { return s.data(); });
   llvm::InitializeNativeTarget();
   llvm::InitializeNativeTargetAsmPrinter();
 
   if (OptHostSupportsJit) {
     auto J = llvm::orc::LLJITBuilder().create();
if (OptInputs.empty()) {
     }
   }
 
   // Our error handler depends on the Diagnostics object, which we're
   // potentially about to delete. Uninstall the handler now so that any
   // later errors use the default handling behavior instead.
   llvm::remove_fatal_error_handler();
 
-  llvm::llvm_shutdown();
-
   return 0;
 }