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

[Jonathan Roelofs via cfe-commits]

On 1/29/16 11:51 AM, Serge Pavlov via cfe-commits wrote:
> Can somebody have a look at this fix?
> Thank you!

Testcase?

>
> --Serge
>
> 2016-01-11 23:50 GMT+06:00 Serge Pavlov <sepavloff at gmail.com
> <mailto:sepavloff at gmail.com>>:
>
>     Any feedback?
>
>     Thanks,
>     --Serge
>
>     2015-12-11 20:24 GMT+06:00 Serge Pavlov <sepavloff at gmail.com
>     <mailto:sepavloff at gmail.com>>:
>
>         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);
>
>
>
>
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded