Any feedback? Thanks, --Serge
2015-12-11 20:24 GMT+06:00 Serge Pavlov <sepavl...@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@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
