llvm dev - May 2017 - RFC: A new llvm-dlltool driver and llvm-lib driver improvements

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::vector<ExportSymbol>)
>>>
>>> I.e. it takes a list of DLLExported symbols and returns a .lib
file.
>>> ExportSymbol type contains information about DLLExported symbols.
>>>
>>
>>
>

-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170511/379ee814/attachment.html>

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::vector<ExportSymbol>)
>>>>
>>>> I.e. it takes a list of DLLExported symbols and returns a .lib
file.
>>>> ExportSymbol type contains information about DLLExported
symbols.
>>>>
>>>
>>>
>>
>
>
> --
> --
> Peter
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170521/667c7d1e/attachment.html>

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::vector<ExportSymbol>)
>>>>>
>>>>> I.e. it takes a list of DLLExported symbols and returns a
.lib file.
>>>>> ExportSymbol type contains information about DLLExported
symbols.
>>>>>
>>>>
>>>>
>>>
>>
>>
>> --
>> --
>> Peter
>>
>
>

-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170521/45915e86/attachment.html>