On 1/29/16 12:50 PM, Serge Pavlov wrote:
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.
This is what the clang/unittests is for (i.e. you can write a testcase that doesn't have to go through the clang driver).
Jon
Thanks, --Serge 2016-01-30 0:54 GMT+06:00 Jonathan Roelofs <jonat...@codesourcery.com <mailto: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> <mailto: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> <mailto: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 <mailto:cfe-commits@lists.llvm.org> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits -- Jon Roelofs jonat...@codesourcery.com <mailto:jonat...@codesourcery.com> CodeSourcery / Mentor Embedded
-- 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
