[llvm-dev] [ThinLTO] static library failure with object files with the same name

[Mehdi AMINI via llvm-dev]

Hi Johan,

ld64 only calls functions from llvm/include/llvm-c/lto.h  (defined
in llvm/tools/lto/lto.cpp)

For instance ThinLTOCodeGenerator::addModule is called
through thinlto_codegen_add_module().

Apple hasn't released the code for ld64 in Xcode 9 yet, did you check if it
is fixed in Xcode 9?
(I think I remember fixing it in ld64 but I'm not totally sure...).

>From what I can see with Xcode 8.2, the linker just passes the file name
(prefixed with the archive name):
https://github.com/michaelweiser/ld64/blob/master/src/ld/parsers/lto_file.cpp#L546
(original here:
https://opensource.apple.com/source/ld64/ld64-274.2/src/ld/parsers/lto_file.cpp.auto.html
)

We could workaround this in ThinLTOCodeGenerator by adding a incremental
suffix to every new buffer. Something like this diff:

diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
index ffd78dad9228..d6e5d4d0c213 100644
--- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -535,7 +535,9 @@ static void initTMBuilder(TargetMachineBuilder
&TMBuilder,
 } // end anonymous namespace

 void ThinLTOCodeGenerator::addModule(StringRef Identifier, StringRef Data)
{
-  ThinLTOBuffer Buffer(Data, Identifier);
+  std::string Id =
+      (Twine(Identifier) + "_" + std::to_string(Modules.size())).str();
+  ThinLTOBuffer Buffer(Data, std::move(Id));
   LLVMContext Context;
   StringRef TripleStr;
   ErrorOr<std::string> TripleOrErr = expectedToErrorOrAndEmitErrors(




-- 
Mehdi


2017-09-07 8:18 GMT-07:00 Teresa Johnson <tejohnson at google.com>:

>
>
> On Wed, Sep 6, 2017 at 1:28 PM, Davide Italiano via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> On Wed, Sep 6, 2017 at 1:10 PM, Johan Engelen via llvm-dev
>> <llvm-dev at lists.llvm.org> wrote:
>> > On Tue, Sep 5, 2017 at 11:34 PM, Davide Italiano <dccitaliano at gmail.com
>> >
>> > wrote:
>> >>
>> >> On Tue, Sep 5, 2017 at 2:09 PM, Teresa Johnson <tejohnson at google.com>
>> >> wrote:
>> >> >
>> >> > Hi Johan,
>> >> >
>> >> > Right, per the bug this is fixed in lld (and was already handled in
>> >> > gold-plugin), but I guess not in ld64. Note that lld and gold-plugin
>> use the
>> >> > new LTO API, while ld64 (and probably other linkers) are still using
>> the
>> >> > legacy libLTO (which is what ThinLTOCodeGenerator.cpp is part of).
>> Fixing it
>> >> > in the location you propose could work for all legacy libLTO users.
>> But I
>> >> > don't think that adding just the size will (always) be enough to
>> >> > disambiguate (couldn't the 2 same named members have the same size?)
>> -
>> >> > although lld is doing the same thing so this may be as good as what
>> is done
>> >> > there. For gold-plugin we add the byte offset into the archive where
>> the
>> >> > member starts, which will be unique.
>> >> > +davide for thoughts since he fixed it on the lld side.
>> >> >
>> >>
>> >> Yes, Teresa is right, this is the correct fix.
>> >> I'm not sure what subset of GNU archives Mach-O supports, but the only
>> >> way of being safe is using offset in the archive + archive name.
>> >
>> >
>> > ThinLTOCodeGenerator::addModule is called by the linker, right? (I
>> can't
>> > find any callers)
>> > When it is called, we cannot retrieve the offset of the module inside
>> the
>> > archive, because the linker didn't tell us about it.
>>
>> See here for the fix.
>> https://reviews.llvm.org/D25495
>> You pass the the archive name + offset to `lto::InputFile::create`,
>> assuming your linker uses the new LTO API (and I'm not sure whether
>> ld64 already switched).
>>
>
> AFAIK, no. Mehdi started to look at this awhile back but likely has not
> been able to pick it back up.
>
>
>> The linker knows the archive name from which it's fetching the member,
>> as well as its offset (it asks libArchive about it, for lld).
>> I'm not sure how it works ld64, but of course, to get this mechanism
>> working, you need some linker modifications.
>
>
> Right, the linker can compose the new identifier. The proposed fix
> in ThinLTOCodeGenerator.cpp could be a partial workaround until linkers do
> this.
>
> 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?
>
> Teresa
>
>
>
> CC:ed some Mach-O people, they probably know more about this than I do.
>>
>> Thanks,
>>
>> --
>> Davide
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
>
>
> --
> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
> 408-460-2413 <(408)%20460-2413>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170907/f6d35f03/attachment.html>