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

[Johan Engelen via llvm-dev]

I've created a review for your patch Mehdi: https://reviews.llvm.org/D37961

First time using `arc`, so hope things went well.

- Johan


On Tue, Sep 12, 2017 at 5:25 AM, Mehdi AMINI <joker.eph at gmail.com> wrote:

> Hi Johan,
>
> 2017-09-11 14:21 GMT-07:00 Johan Engelen <jbc.engelen at gmail.com>:
>
>> On Fri, Sep 8, 2017 at 9:04 PM, Johan Engelen <jbc.engelen at gmail.com>
>> wrote:
>>
>>>
>>> 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/michaelweis
>>>> er/ld64/blob/master/src/ld/parsers/lto_file.cpp#L546
>>>> (original here: https://opensource.apple.com/source/ld64/ld64-274.2/sr
>>>> c/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.
>>>
>>>
>>
>> Hi Mehdi,
>>   Can you advise me?
>> Is it OK to not error upon the exact same module being added twice? (and
>> thus your patch would be good)
>>
>>
> It is a matter of API contract, so I'm not sure I perceive a clear
> right/wrong here. If we want to enforce an API contract on the caller's
> responsibility to provide a unique ID, then the assert may be OK (however
> we disable assertions in production and it means random crashes which isn't
> super user-friendly).
> On the other hand my patch here was trying to be resilient to API misuse
> for "little cost".
>
> Note that the plan was that this API (and the ThinLTOCodegenerator.cpp)
> should just die, but unfortunately I haven't had time to work on the
> replacement (a C binding for the new LTO API).
>
> --
> Mehdi
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170917/64cfb9d6/attachment.html>