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

Rui Ueyama ruiu at google.com
Mon Sep 2 20:34:08 PDT 2013


On Mon, Sep 2, 2013 at 8:26 PM, Shankar Easwaran <shankare at codeaurora.org>wrote:

> 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.
>

That's right. I thought that the entry symbol was not handled but looks
like I tested in a wrong way. Sorry about the confusion and thank you for
pointing this out! I'll revert the change to LinkingContext.h. I'll leave
the test as is, for it's a good test.

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>
>>>>>> <**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>
>>>>>> <http://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/****>
>>>>>> <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/**<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/****>
>>>>>> <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/****<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>
>>>>>> >
>>>>>> <**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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130902/c181d989/attachment.html>


More information about the llvm-commits mailing list