[lld] r189776 - Partially revert r189718 to add entry symbol to dead strip root.

[Shankar Easwaran]

Hi Rui,

Currently the entrySymbol gets added to the undefined atom pool by 
createInternalFiles().

The main problem that I have with the change thats now present is every 
occurence of the entry option is going to cause add an undefined symbol, 
which is not correct.

Thanks

Shankar Easwaran

On 9/2/2013 10:13 PM, Rui Ueyama wrote:
> On Mon, Sep 2, 2013 at 8:08 PM, Shankar Easwaran <shankare at codeaurora.org>wrote:
>
>> First, I dont think setEntrySymbolName should add to the list of undefined
>> symbols as soon as you see it.
>>
> So, if we do that in Driver.cpp (not in code to process command line
> options in each driver file), we do not add entry symbol to the dead strip
> list?
>
>
>> If you still want to do that, make the setEntrySymbolName overridden in
>> PECOFFLinkingContext and add your code there.
>>
>> Default the behavior on how it was originally, that it just sets the
>> internal protected variable, as ELF/MachO has the same usecase.
>>
>> Thanks
>>
>> Shankar Easwaran
>>
>>
>> On 9/2/2013 9:51 PM, Rui Ueyama wrote:
>>
>>> Just rolling back this patch will again regress COFF handling, so I don't
>>> want to simply revert. Instead, I think I can add code to Driver.cpp to
>>> copy entry symbol name to deadStripRoot. In this way the two symbols won't
>>> be added in your example. What do you think?
>>>
>>>
>>> On Mon, Sep 2, 2013 at 6:50 PM, Shankar Easwaran <shankare at codeaurora.org
>>>> **wrote:
>>>   Hi Ruiu,
>>>> This cannot be done, there is a genuine case that if you set the entry
>>>> symbol twice like this
>>>>
>>>> lld -flavor gnu -target x86_64 -e _start -e main
>>>>
>>>> I want to take only the last entry symbol, now this will trigger an
>>>> undefine symbol twice.
>>>>
>>>> Can you please revert this change as it breaks ELF ?
>>>>
>>>> Thanks
>>>>
>>>> Shankar Easwaran
>>>>
>>>>
>>>>
>>>> On 9/2/2013 8:00 PM, Rui Ueyama wrote:
>>>>
>>>>   Author: ruiu
>>>>> Date: Mon Sep  2 20:00:01 2013
>>>>> New Revision: 189776
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-****project?rev=189776&view=rev<http://llvm.org/viewvc/llvm-**project?rev=189776&view=rev>
>>>>> <ht**tp://llvm.org/viewvc/llvm-**project?rev=189776&view=rev<http://llvm.org/viewvc/llvm-project?rev=189776&view=rev>
>>>>> Log:
>>>>> Partially revert r189718 to add entry symbol to dead strip root.
>>>>>
>>>>> Also added a test to verify that entry symbol is not stripped even if
>>>>> dead stripping is enabled.
>>>>>
>>>>> Added:
>>>>>        lld/trunk/test/pecoff/entry.****test
>>>>> Modified:
>>>>>        lld/trunk/include/lld/Core/****LinkingContext.h
>>>>>
>>>>> Modified: lld/trunk/include/lld/Core/****LinkingContext.h
>>>>> URL: http://llvm.org/viewvc/llvm-****project/lld/trunk/include/lld/****<http://llvm.org/viewvc/llvm-**project/lld/trunk/include/lld/**>
>>>>> Core/LinkingContext.h?rev=****189776&r1=189775&r2=189776&****view=diff<
>>>>> http://llvm.org/**viewvc/llvm-project/lld/trunk/**include/lld/Core/**
>>>>> LinkingContext.h?rev=189776&**r1=189775&r2=189776&view=diff<http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/Core/LinkingContext.h?rev=189776&r1=189775&r2=189776&view=diff>
>>>>> ==============================****============================**==**
>>>>> ==================
>>>>> --- lld/trunk/include/lld/Core/****LinkingContext.h (original)
>>>>> +++ lld/trunk/include/lld/Core/****LinkingContext.h Mon Sep  2 20:00:01
>>>>>
>>>>> 2013
>>>>> @@ -184,7 +184,9 @@ public:
>>>>>       // Set the entry symbol name. You may also need to call
>>>>> addDeadStripRoot() for
>>>>>       // the symbol if your platform supports dead-stripping, so that the
>>>>> symbol
>>>>>       // will not be removed from the output.
>>>>> -  virtual void setEntrySymbolName(StringRef name) {
>>>>> +  void setEntrySymbolName(StringRef name) {
>>>>> +    // Entry function have to be resolved as an undefined symbol.
>>>>> +    addInitialUndefinedSymbol(****name);
>>>>>         _entrySymbolName = name;
>>>>>       }
>>>>>
>>>>> Added: lld/trunk/test/pecoff/entry.****test
>>>>> URL: http://llvm.org/viewvc/llvm-****project/lld/trunk/test/pecoff/****<http://llvm.org/viewvc/llvm-**project/lld/trunk/test/pecoff/**>
>>>>> entry.test?rev=189776&view=****auto<http://llvm.org/viewvc/**
>>>>> llvm-project/lld/trunk/test/**pecoff/entry.test?rev=189776&**view=auto<http://llvm.org/viewvc/llvm-project/lld/trunk/test/pecoff/entry.test?rev=189776&view=auto>
>>>>> ==============================****============================**==**
>>>>> ==================
>>>>> --- lld/trunk/test/pecoff/entry.****test (added)
>>>>> +++ lld/trunk/test/pecoff/entry.****test Mon Sep  2 20:00:01 2013
>>>>>
>>>>> @@ -0,0 +1,9 @@
>>>>> +# REQUIRES: debug
>>>>> +# Verify that entry atom will not be dead-stripped.
>>>>> +
>>>>> +# RUN: yaml2obj %p/Inputs/main.obj.yaml > %t.obj
>>>>> +# RUN: lld -flavor link /mllvm -debug-only=WriterPECOFF /out:%t.exe \
>>>>> +# RUN:   /subsystem:console /entry:_main /force -- %t.obj >& %t.log
>>>>> +# RUN: FileCheck %s < %t.log
>>>>> +
>>>>> +CHECK: : _main
>>>>>
>>>>>
>>>>> ______________________________****_________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/****mailman/listinfo/llvm-commits<http://lists.cs.uiuc.edu/**mailman/listinfo/llvm-commits>
>>>>> <**http://lists.cs.uiuc.edu/**mailman/listinfo/llvm-commits<http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>>>>>
>>>>>
>>>>>   --
>>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
>>>> by the Linux Foundation
>>>>
>>>>
>>>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
>> by the Linux Foundation
>>
>>


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation