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

[Serge Pavlov via cfe-commits]

Can somebody have a look at this fix?
Thank you!

--Serge

2016-01-11 23:50 GMT+06:00 Serge Pavlov <sepavloff at gmail.com>:

> Any feedback?
>
> Thanks,
> --Serge
>
> 2015-12-11 20:24 GMT+06:00 Serge Pavlov <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);
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160130/28565413/attachment-0001.html>