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

[Johan Engelen via llvm-dev]

This is a typical error:
http://green.lab.llvm.org/green/job/clang-stage1-configure-RA_check/35542/testReport/junit/LLVM/ThinLTO_X86/funcimport_tbaa_ll/

"

llvm-lto: error loading file
'/Users/buildslave/jenkins/sharedspace/phase1 at 2/clang-build/test/ThinLTO/X86/Output/funcimport-tbaa.ll.tmp.bc_0':
No such file or directory

"

Note the "_0" suffix. I think what happens is that the suffixed module
identifier is used as filename when writing out the thinlto index, and in a
subsequent step that file can (of course) not be found.

Other lit errors have to do with symbols not being found correctly across
modules, leading e.g. to wrong internalization/optimizations after
importing.

List of failures:
http://green.lab.llvm.org/green/job/clang-stage1-configure-RA_check/35542/testReport/junit/LLVM/

regards,
  Johan



On Tue, Sep 19, 2017 at 1:43 AM, Mehdi AMINI <joker.eph at gmail.com> wrote:

> It is expected and not unusual to need to update the lit test in such
> case. I'd need to see exactly which test breaks and how to know though.
>
> Best,
>
> --
> Mehdi
>
> 2017-09-18 13:17 GMT-07:00 Johan Engelen <jbc.engelen at gmail.com>:
>
>> The fix (https://reviews.llvm.org/D37961) does not work.  From what I
>> have learned thusfar, the module identifier is used as filename sometimes
>> (I think when writing an intermediate module index summary), and so a bunch
>> of lit tests fail with the "fix".
>>
>> I'll look further into fixing this, any help is appreciated.
>>
>> ( One thing that may be important is to have a deterministic suffix. The
>> counter solution of https://reviews.llvm.org/D37961 would name the same
>> module differently depending on order, which may happen when doing thinlto
>> in several different steps. Adding the size of the module (or its offset)
>> would be the same in each invocation. Regardless, changing the suffix to a
>> size suffix does not fix the lit test problems. )
>>
>> regards,
>>   Johan
>>
>>
>>
>> On Sun, Sep 17, 2017 at 6:32 PM, Johan Engelen <jbc.engelen at gmail.com>
>> wrote:
>>
>>> I've created a review for your patch Mehdi: https://reviews.llvm.or
>>> g/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/michaelweiser/ld64/blob/master/src/ld/par
>>>>>>> sers/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.
>>>>>>
>>>>>>
>>>>>
>>>>> 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/20170919/c7e12d21/attachment.html>