Crash is observed when HandleTranslation unit is not called, clang always calls this method but a clang based project may don't call it if it is not needed. I saw the crash in such project and used this patch to fix it. I do not know how to make a regression test, I am not even sure such test would be practical. The fact that an object is owned by two unique_ptr's may be enought to change implementation. However any thoughts how to test this behavior are wellcome.
Thanks, --Serge 2016-01-30 0:54 GMT+06:00 Jonathan Roelofs <jonat...@codesourcery.com>: > > > 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 <sepavl...@gmail.com >> <mailto:sepavl...@gmail.com>>: >> >> Any feedback? >> >> Thanks, >> --Serge >> >> 2015-12-11 20:24 GMT+06:00 Serge Pavlov <sepavl...@gmail.com >> <mailto: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 >> >> > -- > Jon Roelofs > jonat...@codesourcery.com > CodeSourcery / Mentor Embedded >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
