[lld] r205391 - [ELF] Add -z muldefs option.

[Rui Ueyama]

On Tue, Apr 1, 2014 at 10:06 PM, Shankar Easwaran
<shankare at codeaurora.org>wrote:

> On 4/1/2014 11:54 PM, Rui Ueyama wrote:
>
>> On Tue, Apr 1, 2014 at 9:40 PM, Shankar Easwaran <shankare at codeaurora.org
>> >wrote:
>>
>>  On 4/1/2014 11:30 PM, Rui Ueyama wrote:
>>>
>>>  On Tue, Apr 1, 2014 at 9:17 PM, Shankar Easwaran <
>>>> shankare at codeaurora.org
>>>>
>>>>> wrote:
>>>>>
>>>>   On 4/1/2014 11:12 PM, Rui Ueyama wrote:
>>>>
>>>>>   On Tue, Apr 1, 2014 at 8:57 PM, Shankar Easwaran <
>>>>> shankarke at gmail.com
>>>>>
>>>>>> wrote:
>>>>>>>
>>>>>>>     Author: shankare
>>>>>>
>>>>>>  Date: Tue Apr  1 22:57:37 2014
>>>>>>> New Revision: 205391
>>>>>>>
>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=205391&view=rev
>>>>>>> Log:
>>>>>>> [ELF] Add -z muldefs option.
>>>>>>>
>>>>>>> This adds -z muldefs option which is widely used over
>>>>>>> --allow-multiple-definition.
>>>>>>>
>>>>>>> This option is supported by the GNU linker.
>>>>>>>
>>>>>>> Modified:
>>>>>>>        lld/trunk/lib/Driver/GnuLdDriver.cpp
>>>>>>>        lld/trunk/test/elf/allowduplicates.objtxt
>>>>>>>
>>>>>>> Modified: lld/trunk/lib/Driver/GnuLdDriver.cpp
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/
>>>>>>> GnuLdDriver.cpp?rev=205391&r1=205390&r2=205391&view=diff
>>>>>>>
>>>>>>> ============================================================
>>>>>>> ==================
>>>>>>> --- lld/trunk/lib/Driver/GnuLdDriver.cpp (original)
>>>>>>> +++ lld/trunk/lib/Driver/GnuLdDriver.cpp Tue Apr  1 22:57:37 2014
>>>>>>> @@ -440,6 +440,16 @@ bool GnuLdDriver::parse(int argc, const
>>>>>>>           groupStack.pop();
>>>>>>>           break;
>>>>>>>
>>>>>>> +    case OPT_z: {
>>>>>>> +      StringRef extOpt = inputArg->getValue();
>>>>>>> +      if (extOpt == "muldefs")
>>>>>>> +        ctx->setAllowDuplicates(true);
>>>>>>> +      else
>>>>>>> +        diagnostics << "warning: ignoring unknown argument for -z: "
>>>>>>> <<
>>>>>>> extOpt
>>>>>>> +                    << "\n";
>>>>>>> +      break;
>>>>>>> +    }
>>>>>>> +
>>>>>>>         case OPT_INPUT:
>>>>>>>         case OPT_l: {
>>>>>>>           bool isDashlPrefix = (inputArg->getOption().getID() ==
>>>>>>> OPT_l);
>>>>>>>
>>>>>>> Modified: lld/trunk/test/elf/allowduplicates.objtxt
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf/
>>>>>>> allowduplicates.objtxt?rev=205391&r1=205390&r2=205391&view=diff
>>>>>>>
>>>>>>> ============================================================
>>>>>>> ==================
>>>>>>> --- lld/trunk/test/elf/allowduplicates.objtxt (original)
>>>>>>> +++ lld/trunk/test/elf/allowduplicates.objtxt Tue Apr  1 22:57:37
>>>>>>> 2014
>>>>>>> @@ -1,8 +1,11 @@
>>>>>>> -# RUN: lld -flavor gnu -target x86_64 --allow-multiple-definition -r
>>>>>>> %s
>>>>>>> \
>>>>>>> -# RUN:     --output-filetype=yaml | FileCheck %s
>>>>>>> +# RUN: lld -flavor gnu -target x86_64 --allow-multiple-definition
>>>>>>> %s \
>>>>>>> +# RUN:     --output-filetype=yaml --noinhibit-exec | FileCheck %s
>>>>>>>     #
>>>>>>> -# RUN: not lld -flavor gnu -target x86_64 -r %s
>>>>>>> --output-filetype=yaml
>>>>>>> 2>&1 \
>>>>>>> -# RUN:   | FileCheck -check-prefix=ERROR %s
>>>>>>> +# RUN: not lld -flavor gnu -target x86_64 %s --output-filetype=yaml
>>>>>>> \
>>>>>>> +# RUN: --noinhibit-exec 2>&1 | FileCheck -check-prefix=ERROR %s
>>>>>>> +#
>>>>>>> +# RUN: lld -flavor gnu -target x86_64 -z muldefs %s \
>>>>>>> +# RUN: --noinhibit-exec --output-filetype=yaml | FileCheck %s
>>>>>>>
>>>>>>>    Why do you now need --noinhibit-exec?
>>>>>>>
>>>>>>>  The testcase previously used -r, which is not supported (or) not
>>>>>>
>>>>> implemented at this time.
>>>>>
>>>>> --noinhibit-exec tries to keep the executable even if there is an
>>>>> undefined symbol, which in this case corresponds to _start undefined.
>>>>>
>>>>>  -r was intentional -- although ELF writer does not support writing it,
>>>> YAML
>>>> writer does. It was slightly better than --noinhibit-exec because it did
>>>> not warn on unresolved symbols unlike --noinhibit-exec.
>>>>
>>>>
>>>>  You might already know, The -r option is not handled by YAML writer.
>>>
>>> The GnuLdDriver handles this and sets appropriate flags when -r option is
>>> seen in the LinkingContext, and nothing else is done with it, since you
>>> bypass the writer to the output yaml writer.
>>>
>>> With gnu, we consistently use --noinhibit-exec for all tests, and I would
>>> prefer to stay consistent with all tests.
>>>
>>
>> Well, that's not true because I learned the use of -r from other test
>> files
>> having .objtxt extension in the same directory.
>>
> Ah!, Thanks for pointing that. The vast majority use noinhibit-exec though.
>
> Below are the stats from the test/elf directory.
>
> $find . -type f | xargs grep  -w 'noinhibit-exec' | wc -l
> 90
> $find . -type f | xargs grep  -w '\-r' | grep lld  | wc -l
> 6
>
> Not sure if it was a shortcut chosen to make sure unresolved symbols are
> accepted and not displayed to the user.
>
> If you want to use -r back, let me know, I will not have any issues to go
> back and revert to use -r.


--noinhibit-exec is okay as long as it's working. It might be a good thing
to have some mix of -r and --noinbhit-exec because by doing that we have
broader test coverage.


>
>
> Thanks
>
> Shankar Easwaran
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
> by the Linux Foundation
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140401/8ada2232/attachment.html>