<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Sep 8, 2017 at 9:04 PM, Johan Engelen <span dir="ltr"><<a href="mailto:jbc.engelen@gmail.com" target="_blank">jbc.engelen@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Thu, Sep 7, 2017 at 5:44 PM, Mehdi AMINI <span dir="ltr"><<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi Johan,<div><br></div><div>ld64 only calls functions from llvm/include/llvm-c/lto.h  (defined in llvm/tools/lto/lto.cpp)</div><div><br></div><div>For instance ThinLTOCodeGenerator::addModul<wbr>e is called through thinlto_codegen_add_mo<wbr>dule().<br></div><div><br></div><div>Apple hasn't released the code for ld64 in Xcode 9 yet, did you check if it is fixed in Xcode 9?</div><div>(I think I remember fixing it in ld64 but I'm not totally sure...).</div></div></blockquote><div><br></div></span><div>I haven't tried with Xcode 9.</div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>From what I can see with Xcode 8.2, the linker just passes the file name (prefixed with the archive name): <a href="https://github.com/michaelweiser/ld64/blob/master/src/ld/parsers/lto_file.cpp#L546" target="_blank">https://github.com/michaelweis<wbr>er/ld64/blob/master/src/ld/<wbr>parsers/lto_file.cpp#L546</a></div><div>(original here: <a href="https://opensource.apple.com/source/ld64/ld64-274.2/src/ld/parsers/lto_file.cpp.auto.html" target="_blank">https://opensource.apple<wbr>.com/source/ld64/ld64-274.2/<wbr>src/ld/parsers/lto_file.cpp.<wbr>auto.html</a> )</div><div><br></div><div>We could workaround this in ThinLTOCodeGenerator by adding a incremental suffix to every new buffer. Something like this diff:</div></div></blockquote><div><br></div></span><div><span style="font-size:12.8px">I was assuming that we do want to assert/error on calling addModule with the exact same module twice? Otherwise your diff is nice, thanks.</span><span class="HOEnZb"><font color="#888888"><br></font></span></div><span class="HOEnZb"><font color="#888888"><div> </div></font></span></div></div></div></blockquote><div><br></div><div>Hi Mehdi,</div><div>  Can you advise me?</div><div>Is it OK to not error upon the exact same module being added twice? (and thus your patch would be good)</div><div><br></div><div>Thanks,</div><div>  Johan</div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div><div>diff --git a/llvm/lib/LTO/ThinLTOCodeGene<wbr>rator.cpp b/llvm/lib/LTO/ThinLTOCodeGene<wbr>rator.cpp</div><div>index ffd78dad9228..d6e5d4d0c213 100644</div><div>--- a/llvm/lib/LTO/ThinLTOCodeGene<wbr>rator.cpp</div><div>+++ b/llvm/lib/LTO/ThinLTOCodeGene<wbr>rator.cpp</div><div>@@ -535,7 +535,9 @@ static void initTMBuilder(TargetMachineBui<wbr>lder &TMBuilder,</div><div> } // end anonymous namespace</div><span class="m_-1829764174203390664gmail-"><div> </div><div> void ThinLTOCodeGenerator::addModul<wbr>e(StringRef Identifier, StringRef Data) {</div></span><div>-  ThinLTOBuffer Buffer(Data, Identifier);</div><div>+  std::string Id =</div><div>+      (Twine(Identifier) + "_" + std::to_string(Modules.size())<wbr>).str();</div><div>+  ThinLTOBuffer Buffer(Data, std::move(Id));</div><div>   LLVMContext Context;</div><div>   StringRef TripleStr;</div><div>   ErrorOr<std::string> TripleOrErr = expectedToErrorOrAndEmitErrors<wbr>(</div></div><span class="m_-1829764174203390664gmail-HOEnZb"><font color="#888888"><div><br></div><div><br></div><div><br></div><div><br></div><div>-- </div><div>Mehdi</div></font></span><div><div class="m_-1829764174203390664gmail-h5"><div><br></div><div class="gmail_extra"><br><div class="gmail_quote">2017-09-07 8:18 GMT-07:00 Teresa Johnson <span dir="ltr"><<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="m_-1829764174203390664gmail-m_-2444105776949506006gmail-h5">On Wed, Sep 6, 2017 at 1:28 PM, Davide Italiano via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="m_-1829764174203390664gmail-m_-2444105776949506006gmail-m_-7701003061177321895gmail-m_-9138310503008520837HOEnZb"><div class="m_-1829764174203390664gmail-m_-2444105776949506006gmail-m_-7701003061177321895gmail-m_-9138310503008520837h5">On Wed, Sep 6, 2017 at 1:10 PM, Johan Engelen via llvm-dev<br>
<<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
> On Tue, Sep 5, 2017 at 11:34 PM, Davide Italiano <<a href="mailto:dccitaliano@gmail.com" target="_blank">dccitaliano@gmail.com</a>><br>
> wrote:<br>
>><br>
>> On Tue, Sep 5, 2017 at 2:09 PM, Teresa Johnson <<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>><br>
>> wrote:<br>
>> ><br>
>> > Hi Johan,<br>
>> ><br>
>> > Right, per the bug this is fixed in lld (and was already handled in<br>
>> > gold-plugin), but I guess not in ld64. Note that lld and gold-plugin use the<br>
>> > new LTO API, while ld64 (and probably other linkers) are still using the<br>
>> > legacy libLTO (which is what ThinLTOCodeGenerator.cpp is part of). Fixing it<br>
>> > in the location you propose could work for all legacy libLTO users. But I<br>
>> > don't think that adding just the size will (always) be enough to<br>
>> > disambiguate (couldn't the 2 same named members have the same size?) -<br>
>> > although lld is doing the same thing so this may be as good as what is done<br>
>> > there. For gold-plugin we add the byte offset into the archive where the<br>
>> > member starts, which will be unique.<br>
>> > +davide for thoughts since he fixed it on the lld side.<br>
>> ><br>
>><br>
>> Yes, Teresa is right, this is the correct fix.<br>
>> I'm not sure what subset of GNU archives Mach-O supports, but the only<br>
>> way of being safe is using offset in the archive + archive name.<br>
><br>
><br>
> ThinLTOCodeGenerator::addModul<wbr>e is called by the linker, right? (I can't<br>
> find any callers)<br>
> When it is called, we cannot retrieve the offset of the module inside the<br>
> archive, because the linker didn't tell us about it.<br>
<br>
</div></div>See here for the fix.<br>
<a href="https://reviews.llvm.org/D25495" rel="noreferrer" target="_blank">https://reviews.llvm.org/D2549<wbr>5</a><br>
You pass the the archive name + offset to `lto::InputFile::create`,<br>
assuming your linker uses the new LTO API (and I'm not sure whether<br>
ld64 already switched).<br></blockquote><div><br></div></div></div><div>AFAIK, no. Mehdi started to look at this awhile back but likely has not been able to pick it back up.</div><span class="m_-1829764174203390664gmail-m_-2444105776949506006gmail-"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
The linker knows the archive name from which it's fetching the member,<br>
as well as its offset (it asks libArchive about it, for lld).<br>
I'm not sure how it works ld64, but of course, to get this mechanism<br>
working, you need some linker modifications.</blockquote><div><br></div></span><div>Right, the linker can compose the new identifier. The proposed fix in ThinLTOCodeGenerator.cpp could be a partial workaround until linkers do this.</div><div><br></div><div>Agree that it should be documented, at the very least in the header file interfaces to the new/old LTO APIs. Not sure there is a better place to document?</div><span class="m_-1829764174203390664gmail-m_-2444105776949506006gmail-HOEnZb"><font color="#888888"><div><br></div><div>Teresa</div></font></span><span class="m_-1829764174203390664gmail-m_-2444105776949506006gmail-"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
CC:ed some Mach-O people, they probably know more about this than I do.<br>
<br>
Thanks,<br>
<br>
--<br>
Davide<br>
<div class="m_-1829764174203390664gmail-m_-2444105776949506006gmail-m_-7701003061177321895gmail-m_-9138310503008520837HOEnZb"><div class="m_-1829764174203390664gmail-m_-2444105776949506006gmail-m_-7701003061177321895gmail-m_-9138310503008520837h5">______________________________<wbr>_________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br>
</div></div></blockquote></span></div><br><br clear="all"><span class="m_-1829764174203390664gmail-m_-2444105776949506006gmail-"><div><br></div>-- <br><div class="m_-1829764174203390664gmail-m_-2444105776949506006gmail-m_-7701003061177321895gmail-m_-9138310503008520837gmail_signature"><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top:2px solid rgb(213,15,37)">Teresa Johnson |</td><td nowrap style="border-top:2px solid rgb(51,105,232)"> Software Engineer |</td><td nowrap style="border-top:2px solid rgb(0,153,57)"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top:2px solid rgb(238,178,17)"> <a href="tel:(408)%20460-2413" value="+14084602413" target="_blank">408-460-2413</a></td></tr></tbody></table></span></div>
</span></div></div>
</blockquote></div><br></div></div></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>