<div dir="ltr">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.</div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature">Thanks,<br>--Serge<br></div></div>
<br><div class="gmail_quote">2016-01-30 0:54 GMT+06:00 Jonathan Roelofs <span dir="ltr"><<a href="mailto:jonathan@codesourcery.com" target="_blank">jonathan@codesourcery.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
On 1/29/16 11:51 AM, Serge Pavlov via cfe-commits wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Can somebody have a look at this fix?<br>
Thank you!<br>
</blockquote>
<br></span>
Testcase?<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
<br>
--Serge<br>
<br>
2016-01-11 23:50 GMT+06:00 Serge Pavlov <<a href="mailto:sepavloff@gmail.com" target="_blank">sepavloff@gmail.com</a><br></span>
<mailto:<a href="mailto:sepavloff@gmail.com" target="_blank">sepavloff@gmail.com</a>>>:<span class=""><br>
<br>
    Any feedback?<br>
<br>
    Thanks,<br>
    --Serge<br>
<br>
    2015-12-11 20:24 GMT+06:00 Serge Pavlov <<a href="mailto:sepavloff@gmail.com" target="_blank">sepavloff@gmail.com</a><br></span>
    <mailto:<a href="mailto:sepavloff@gmail.com" target="_blank">sepavloff@gmail.com</a>>>:<div><div class="h5"><br>
<br>
        sepavloff created this revision.<br>
        sepavloff added a subscriber: cfe-commits.<br>
<br>
        Llvm module object is shared between CodeGenerator and<br>
        BackendConsumer,<br>
        in both classes it is stored as std::unique_ptr, which is not a good<br>
        design solution and can cause double deletion error. Usually it does<br>
        not occur because in BackendConsumer::HandleTranslationUnit the<br>
        ownership of CodeGenerator over the module is taken away. If however<br>
        this method is not called, the module is deleted twice and<br>
        compiler crashes.<br>
<br>
        As the module owned by BackendConsumer is always the same as<br>
        CodeGenerator<br>
        has, local copy of llvm module can be removed from BackendGenerator.<br>
<br>
        <a href="http://reviews.llvm.org/D15450" rel="noreferrer" target="_blank">http://reviews.llvm.org/D15450</a><br>
<br>
        Files:<br>
           lib/CodeGen/CodeGenAction.cpp<br>
<br>
        Index: lib/CodeGen/CodeGenAction.cpp<br>
        ===================================================================<br>
        --- lib/CodeGen/CodeGenAction.cpp<br>
        +++ lib/CodeGen/CodeGenAction.cpp<br>
        @@ -73,7 +73,6 @@<br>
<br>
              std::unique_ptr<CodeGenerator> Gen;<br>
<br>
        -    std::unique_ptr<llvm::Module> TheModule;<br>
              SmallVector<std::pair<unsigned,<br>
        std::unique_ptr<llvm::Module>>, 4><br>
                  LinkModules;<br>
<br>
        @@ -97,7 +96,10 @@<br>
                  this->LinkModules.push_back(<br>
                      std::make_pair(I.first,<br>
        std::unique_ptr<llvm::Module>(I.second)));<br>
              }<br>
        -    std::unique_ptr<llvm::Module> takeModule() { return<br>
        std::move(TheModule); }<br>
        +    llvm::Module *getModule() const { return Gen->GetModule(); }<br>
        +    std::unique_ptr<llvm::Module> takeModule() {<br>
        +      return std::unique_ptr<llvm::Module>(Gen->ReleaseModule());<br>
        +    }<br>
              void releaseLinkModules() {<br>
                for (auto &I : LinkModules)<br>
                  I.second.release();<br>
        @@ -117,8 +119,6 @@<br>
<br>
                Gen->Initialize(Ctx);<br>
<br>
        -      TheModule.reset(Gen->GetModule());<br>
        -<br>
                if (llvm::TimePassesIsEnabled)<br>
                  LLVMIRGeneration.stopTimer();<br>
              }<br>
        @@ -165,27 +165,14 @@<br>
                }<br>
<br>
                // Silently ignore if we weren't initialized for some<br>
        reason.<br>
        -      if (!TheModule)<br>
        -        return;<br>
        -<br>
        -      // Make sure IR generation is happy with the module. This<br>
        is released by<br>
        -      // the module provider.<br>
        -      llvm::Module *M = Gen->ReleaseModule();<br>
        -      if (!M) {<br>
        -        // The module has been released by IR gen on failures,<br>
        do not double<br>
        -        // free.<br>
        -        TheModule.release();<br>
        +      if (!getModule())<br>
                  return;<br>
        -      }<br>
        -<br>
        -      assert(TheModule.get() == M &&<br>
        -             "Unexpected module change during IR generation");<br>
<br>
                // Link LinkModule into this module if present,<br>
        preserving its validity.<br>
                for (auto &I : LinkModules) {<br>
                  unsigned LinkFlags = I.first;<br>
                  llvm::Module *LinkModule = I.second.get();<br>
        -        if (Linker::linkModules(*M, *LinkModule,<br>
        +        if (Linker::linkModules(*getModule(), *LinkModule,<br>
                                          [=](const DiagnosticInfo &DI) {<br>
                                            linkerDiagnosticHandler(DI,<br>
        LinkModule, Diags);<br>
                                          },<br>
        @@ -195,7 +182,7 @@<br>
<br>
                // Install an inline asm handler so that diagnostics get<br>
        printed through<br>
                // our diagnostics hooks.<br>
        -      LLVMContext &Ctx = TheModule->getContext();<br>
        +      LLVMContext &Ctx = getModule()->getContext();<br>
                LLVMContext::InlineAsmDiagHandlerTy OldHandler =<br>
                  Ctx.getInlineAsmDiagnosticHandler();<br>
                void *OldContext = Ctx.getInlineAsmDiagnosticContext();<br>
        @@ -208,7 +195,7 @@<br>
<br>
                EmitBackendOutput(Diags, CodeGenOpts, TargetOpts, LangOpts,<br>
                                  C.getTargetInfo().getDataLayoutString(),<br>
        -                        TheModule.get(), Action, AsmOutStream);<br>
        +                        getModule(), Action, AsmOutStream);<br>
<br>
                Ctx.setInlineAsmDiagnosticHandler(OldHandler, OldContext);<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br></div></div>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
<br><span class="HOEnZb"><font color="#888888">
</font></span></blockquote><span class="HOEnZb"><font color="#888888">
<br>
-- <br>
Jon Roelofs<br>
<a href="mailto:jonathan@codesourcery.com" target="_blank">jonathan@codesourcery.com</a><br>
CodeSourcery / Mentor Embedded<br>
</font></span></blockquote></div><br></div>