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

Fri Sep 8 12:04:26 PDT 2017

On Thu, Sep 7, 2017 at 5:44 PM, Mehdi AMINI <joker.eph at gmail.com> wrote:

> 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...).
>

I haven't tried with Xcode 9.

>
> 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:
>

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.

- Johan

> 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/20170908/a2be8068/attachment.html>