[PATCH] D15450: Avoid double deletion in Clang driver.

Serge Pavlov via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 11 06:24:53 PST 2015


sepavloff created this revision.
sepavloff added a subscriber: cfe-commits.

Llvm module object is shared between CodeGenerator and BackendConsumer,
in both classes it is stored as std::unique_ptr, which is not a good
design solution and can cause double deletion error. Usually it does
not occur because in BackendConsumer::HandleTranslationUnit the
ownership of CodeGenerator over the module is taken away. If however
this method is not called, the module is deleted twice and compiler crashes.

As the module owned by BackendConsumer is always the same as CodeGenerator
has, local copy of llvm module can be removed from BackendGenerator.

http://reviews.llvm.org/D15450

Files:
  lib/CodeGen/CodeGenAction.cpp

Index: lib/CodeGen/CodeGenAction.cpp
===================================================================
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -73,7 +73,6 @@
 
     std::unique_ptr<CodeGenerator> Gen;
 
-    std::unique_ptr<llvm::Module> TheModule;
     SmallVector<std::pair<unsigned, std::unique_ptr<llvm::Module>>, 4>
         LinkModules;
 
@@ -97,7 +96,10 @@
         this->LinkModules.push_back(
             std::make_pair(I.first, std::unique_ptr<llvm::Module>(I.second)));
     }
-    std::unique_ptr<llvm::Module> takeModule() { return std::move(TheModule); }
+    llvm::Module *getModule() const { return Gen->GetModule(); }
+    std::unique_ptr<llvm::Module> takeModule() {
+      return std::unique_ptr<llvm::Module>(Gen->ReleaseModule());
+    }
     void releaseLinkModules() {
       for (auto &I : LinkModules)
         I.second.release();
@@ -117,8 +119,6 @@
 
       Gen->Initialize(Ctx);
 
-      TheModule.reset(Gen->GetModule());
-
       if (llvm::TimePassesIsEnabled)
         LLVMIRGeneration.stopTimer();
     }
@@ -165,27 +165,14 @@
       }
 
       // Silently ignore if we weren't initialized for some reason.
-      if (!TheModule)
-        return;
-
-      // Make sure IR generation is happy with the module. This is released by
-      // the module provider.
-      llvm::Module *M = Gen->ReleaseModule();
-      if (!M) {
-        // The module has been released by IR gen on failures, do not double
-        // free.
-        TheModule.release();
+      if (!getModule())
         return;
-      }
-
-      assert(TheModule.get() == M &&
-             "Unexpected module change during IR generation");
 
       // Link LinkModule into this module if present, preserving its validity.
       for (auto &I : LinkModules) {
         unsigned LinkFlags = I.first;
         llvm::Module *LinkModule = I.second.get();
-        if (Linker::linkModules(*M, *LinkModule,
+        if (Linker::linkModules(*getModule(), *LinkModule,
                                 [=](const DiagnosticInfo &DI) {
                                   linkerDiagnosticHandler(DI, LinkModule, Diags);
                                 },
@@ -195,7 +182,7 @@
 
       // Install an inline asm handler so that diagnostics get printed through
       // our diagnostics hooks.
-      LLVMContext &Ctx = TheModule->getContext();
+      LLVMContext &Ctx = getModule()->getContext();
       LLVMContext::InlineAsmDiagHandlerTy OldHandler =
         Ctx.getInlineAsmDiagnosticHandler();
       void *OldContext = Ctx.getInlineAsmDiagnosticContext();
@@ -208,7 +195,7 @@
 
       EmitBackendOutput(Diags, CodeGenOpts, TargetOpts, LangOpts,
                         C.getTargetInfo().getDataLayoutString(),
-                        TheModule.get(), Action, AsmOutStream);
+                        getModule(), Action, AsmOutStream);
 
       Ctx.setInlineAsmDiagnosticHandler(OldHandler, OldContext);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D15450.42519.patch
Type: text/x-patch
Size: 2930 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151211/3a947ce2/attachment.bin>


More information about the cfe-commits mailing list