<div dir="ltr">Any feedback?</div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature">Thanks,<br>--Serge<br></div></div>
<br><div class="gmail_quote">2015-12-11 20:24 GMT+06:00 Serge Pavlov <span dir="ltr"><<a href="mailto:sepavloff@gmail.com" target="_blank">sepavloff@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">sepavloff created this revision.<br>
sepavloff added a subscriber: cfe-commits.<br>
<br>
Llvm module object is shared between CodeGenerator and BackendConsumer,<br>
in both classes it is stored as std::unique_ptr, which is not a good<br>
design solution and can cause double deletion error. Usually it does<br>
not occur because in BackendConsumer::HandleTranslationUnit the<br>
ownership of CodeGenerator over the module is taken away. If however<br>
this method is not called, the module is deleted twice and compiler crashes.<br>
<br>
As the module owned by BackendConsumer is always the same as CodeGenerator<br>
has, local copy of llvm module can be removed from BackendGenerator.<br>
<br>
<a href="http://reviews.llvm.org/D15450" rel="noreferrer" target="_blank">http://reviews.llvm.org/D15450</a><br>
<br>
Files:<br>
  lib/CodeGen/CodeGenAction.cpp<br>
<br>
Index: lib/CodeGen/CodeGenAction.cpp<br>
===================================================================<br>
--- lib/CodeGen/CodeGenAction.cpp<br>
+++ lib/CodeGen/CodeGenAction.cpp<br>
@@ -73,7 +73,6 @@<br>
<br>
     std::unique_ptr<CodeGenerator> Gen;<br>
<br>
-    std::unique_ptr<llvm::Module> TheModule;<br>
     SmallVector<std::pair<unsigned, std::unique_ptr<llvm::Module>>, 4><br>
         LinkModules;<br>
<br>
@@ -97,7 +96,10 @@<br>
         this->LinkModules.push_back(<br>
             std::make_pair(I.first, std::unique_ptr<llvm::Module>(I.second)));<br>
     }<br>
-    std::unique_ptr<llvm::Module> takeModule() { return std::move(TheModule); }<br>
+    llvm::Module *getModule() const { return Gen->GetModule(); }<br>
+    std::unique_ptr<llvm::Module> takeModule() {<br>
+      return std::unique_ptr<llvm::Module>(Gen->ReleaseModule());<br>
+    }<br>
     void releaseLinkModules() {<br>
       for (auto &I : LinkModules)<br>
         I.second.release();<br>
@@ -117,8 +119,6 @@<br>
<br>
       Gen->Initialize(Ctx);<br>
<br>
-      TheModule.reset(Gen->GetModule());<br>
-<br>
       if (llvm::TimePassesIsEnabled)<br>
         LLVMIRGeneration.stopTimer();<br>
     }<br>
@@ -165,27 +165,14 @@<br>
       }<br>
<br>
       // Silently ignore if we weren't initialized for some reason.<br>
-      if (!TheModule)<br>
-        return;<br>
-<br>
-      // Make sure IR generation is happy with the module. This is released by<br>
-      // the module provider.<br>
-      llvm::Module *M = Gen->ReleaseModule();<br>
-      if (!M) {<br>
-        // The module has been released by IR gen on failures, do not double<br>
-        // free.<br>
-        TheModule.release();<br>
+      if (!getModule())<br>
         return;<br>
-      }<br>
-<br>
-      assert(TheModule.get() == M &&<br>
-             "Unexpected module change during IR generation");<br>
<br>
       // Link LinkModule into this module if present, preserving its validity.<br>
       for (auto &I : LinkModules) {<br>
         unsigned LinkFlags = I.first;<br>
         llvm::Module *LinkModule = I.second.get();<br>
-        if (Linker::linkModules(*M, *LinkModule,<br>
+        if (Linker::linkModules(*getModule(), *LinkModule,<br>
                                 [=](const DiagnosticInfo &DI) {<br>
                                   linkerDiagnosticHandler(DI, LinkModule, Diags);<br>
                                 },<br>
@@ -195,7 +182,7 @@<br>
<br>
       // Install an inline asm handler so that diagnostics get printed through<br>
       // our diagnostics hooks.<br>
-      LLVMContext &Ctx = TheModule->getContext();<br>
+      LLVMContext &Ctx = getModule()->getContext();<br>
       LLVMContext::InlineAsmDiagHandlerTy OldHandler =<br>
         Ctx.getInlineAsmDiagnosticHandler();<br>
       void *OldContext = Ctx.getInlineAsmDiagnosticContext();<br>
@@ -208,7 +195,7 @@<br>
<br>
       EmitBackendOutput(Diags, CodeGenOpts, TargetOpts, LangOpts,<br>
                         C.getTargetInfo().getDataLayoutString(),<br>
-                        TheModule.get(), Action, AsmOutStream);<br>
+                        getModule(), Action, AsmOutStream);<br>
<br>
       Ctx.setInlineAsmDiagnosticHandler(OldHandler, OldContext);<br>
<br>
<br>
<br>
</blockquote></div><br></div>