[PATCH] D15450: Avoid double deletion in Clang driver.
Serge Pavlov via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 29 11:50:21 PST 2016
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 <jonathan at 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 <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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160130/f88a1b88/attachment.html>
More information about the cfe-commits
mailing list