[llvm-dev] RFC: A new llvm-dlltool driver and llvm-lib driver improvements

Martell Malone via llvm-dev llvm-dev at lists.llvm.org
Mon Jul 24 11:05:06 PDT 2017


Hey Peter,

Wanted to follow up here after getting this test case setup on MSVC
link.exe over the weekend.
When I updated LLD to the latest trunk the problem seems to have resolved
itself.
I haven't gotten a decent chance to check through each iteration of the
history to find the commit yet but I will move back on to getting a mingw
driver into LLD (hopefully this coming weekend) to make sure we have not
only a driver but also test cases to ensure this stays working.

For anyone interested, llvm-dlltool landed in rL308379 which was before the
5.0 branch thanks to Rui for his patience on getting this merged.

Best,
Martell

On Sun, May 21, 2017 at 11:23 PM, Peter Collingbourne <peter at pcc.me.uk>
wrote:

> On Sun, May 21, 2017 at 3:11 PM, Martell Malone <martellmalone at gmail.com>
> wrote:
>
>> Hey Peter,
>>
>> How is crt0_c.c being added to the link? If crt0_c.c is supplying a
>>> definition of the main function I would expect it to be in an archive which
>>> would appear after the user supplied inputs on the command line. Archives
>>> and objects appearing earlier in the link have priority over archives
>>> appearing later, so the user's definition of main should then have priority
>>> over the CRT's definition. However, objects added to the link have equal
>>> priority, so if crt0_c.c is being added directly as an object file I would
>>> expect it to cause a duplicate symbol error, as its definition of main
>>> would conflict with the user's definition.
>>
>>
>> crt0_c.c is not being added directly to the linker, this is when linking
>> a simple hello world program with libmingwex.a and the other MS libraries.
>>
> A few months ago I was able to alter mingw-w64 to be able to link with
>> msvc link.exe, I will try pull out those patches again to produce a link
>> repo.
>> It should be much clearer when MSVC links it and lld doesn't but I do
>> feel as though this is definitely a regression.
>>
>
> So the crt0_c.c object file is a member of libmingwex.a? If so, this does
> sound like a regression to me. Please do send the repro once you have
> something that links with the MSVC linker and fails with lld.
>
> Peter
>
>>
>>
>> We should probably implement /FORCE:MULTIPLE for MSVC compatibility, but
>>> I don't think you should be passing it from your driver, as that would just
>>> be working around the bug that was presumably introduced by r289280. If you
>>> just want to get past this issue though, implementing it would probably
>>> just involve downgrading the fatal error in SymbolTable::reportDuplicate to
>>> a warning if /FORCE:MULTIPLE is passed.
>>
>> I worked around it earlier by commenting out the path that result in an
>> error on duplicates earlier.
>> This isn't ideal of course as you mentioned.
>>
>> Best,
>> Martell
>>
>>
>> On Sun, May 21, 2017 at 9:51 PM, Peter Collingbourne <peter at pcc.me.uk>
>> wrote:
>>
>>> Hi Martell,
>>>
>>> r289280 was not intended to be a significant functional change in the
>>> sense that it would cause programs to fail to link, so this may be a bug I
>>> introduced in r289280 (or one of the followup patches, which also changed
>>> link order).
>>>
>>> How is crt0_c.c being added to the link? If crt0_c.c is supplying a
>>> definition of the main function I would expect it to be in an archive which
>>> would appear after the user supplied inputs on the command line. Archives
>>> and objects appearing earlier in the link have priority over archives
>>> appearing later, so the user's definition of main should then have priority
>>> over the CRT's definition. However, objects added to the link have equal
>>> priority, so if crt0_c.c is being added directly as an object file I would
>>> expect it to cause a duplicate symbol error, as its definition of main
>>> would conflict with the user's definition.
>>>
>>> Note that this is all how I would expect things to work both before and
>>> after my change. The priority that existed before was just an
>>> implementation detail; priority is now implicit in the link order (i.e. the
>>> order in which files appear in the linker's command line, roughly speaking).
>>>
>>> We should probably implement /FORCE:MULTIPLE for MSVC compatibility, but
>>> I don't think you should be passing it from your driver, as that would just
>>> be working around the bug that was presumably introduced by r289280. If you
>>> just want to get past this issue though, implementing it would probably
>>> just involve downgrading the fatal error in SymbolTable::reportDuplicate to
>>> a warning if /FORCE:MULTIPLE is passed.
>>>
>>> Peter
>>>
>>> On Sun, May 21, 2017 at 7:05 AM, Martell Malone <martellmalone at gmail.com
>>> > wrote:
>>>
>>>> Hey Peter,
>>>>
>>>> Currently I still use the custom frontend I have for using lld so it
>>>> wouldn't be much use to do that.
>>>> I'm looking to get all the other components merged before I submit an
>>>> in process shim like Rui suggested.
>>>>
>>>> I think the best way to approach this would be from MSVC terms.
>>>>
>>>> Before rL289280 we had some sort of priority for deciding on which
>>>> symbol to link.
>>>>
>>>> MSVC Link.exe defines a flag called /FORCE
>>>> https://msdn.microsoft.com/en-us/library/70abkas3.aspx
>>>> We currently implement /FORCE:UNRESOLVED from what I see in the driver.
>>>> I would need /FORCE:MULTIPLE to have this functionality back.
>>>>
>>>> I'm not sure how we would go about implementing that since that commit
>>>> brings us further away from that.
>>>> Feedback on what we should do for /FORCE:MULTIPLE would be a good place
>>>> to start.
>>>>
>>>> Best,
>>>> Martell
>>>>
>>>>
>>>> On Thu, May 11, 2017 at 6:41 PM, Peter Collingbourne <peter at pcc.me.uk>
>>>> wrote:
>>>>
>>>>> Would it be possible to share a repro.tar file created with /linkrepro
>>>>> so that we can reproduce the problem?
>>>>>
>>>>> Peter
>>>>>
>>>>> On Thu, May 11, 2017 at 3:00 AM, Martell Malone <
>>>>> martellmalone at gmail.com> wrote:
>>>>>
>>>>>> There are a few things running in parallel and thanks for taking the
>>>>>> time to review and help me get this in tree.
>>>>>> I wanted to follow up with a question on the linker side of things.
>>>>>>
>>>>>> Since rL289280 I can no longer link programs with the mingw-crt
>>>>>> because of dup function errors of the main function.
>>>>>>
>>>>>> In mingw-w64-crt/crt/crtexe.c a main function is called
>>>>>> from __tmainCRTStartup
>>>>>> mingw-w64-crt/crt/crt0_c.c defines this main and calls WinMain in the
>>>>>> case where main does not exist.
>>>>>> What used to happen before was the crt0_c.c main would be ignored at
>>>>>> link time because of priority on the users main function but now I just get
>>>>>> a dup symbol error as there is no checking on priority anymore.
>>>>>>
>>>>>>
>>>>>> On Mon, Mar 27, 2017 at 7:00 PM, Martell Malone <
>>>>>> martellmalone at gmail.com> wrote:
>>>>>>
>>>>>>> Just to follow up here I found some time to progress.
>>>>>>> Anyone who wants to follow along can see here.
>>>>>>> https://reviews.llvm.org/D29892
>>>>>>>
>>>>>>> Kind Regards
>>>>>>> Martell
>>>>>>>
>>>>>>> On Tue, Feb 14, 2017 at 2:21 AM, Rui Ueyama <ruiu at google.com> wrote:
>>>>>>>
>>>>>>>> The code in LLD is somewhat tightly coupled with the rest of the
>>>>>>>> linker. Probably it is better to create a new library rather than trying to
>>>>>>>> split LLD into two. I believe this would result in a better API. Of course
>>>>>>>> you can reuse code, but what we need is a clean interface.
>>>>>>>>
>>>>>>>> The library shouldn't be complicated. In fact, it would export only
>>>>>>>> one function.
>>>>>>>>
>>>>>>>>   std::vector<uint8_t> createImportLibrary(std::vecto
>>>>>>>> r<ExportSymbol>)
>>>>>>>>
>>>>>>>> I.e. it takes a list of DLLExported symbols and returns a .lib
>>>>>>>> file. ExportSymbol type contains information about DLLExported symbols.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> --
>>>>> Peter
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> --
>>> Peter
>>>
>>
>>
>
>
> --
> --
> Peter
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170724/3e9a6cbe/attachment.html>


More information about the llvm-dev mailing list