[PATCH] Always define referenced __start_SECNAME/__stop_SECNAME

[H.J. Lu]

On Tue, Jun 13, 2017 at 12:07 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Jun 13, 2017 at 8:22 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Jun 13, 2017 at 7:28 AM, Alan Modra <amodra@gmail.com> wrote:
>>> On Tue, Jun 13, 2017 at 05:38:45AM -0700, H.J. Lu wrote:
>>>> On Tue, Jun 13, 2017 at 5:26 AM, Alan Modra <amodra@gmail.com> wrote:
>>>> > On Mon, Jun 12, 2017 at 07:10:47PM -0700, H.J. Lu wrote:
>>>> >> On Mon, Jun 12, 2017 at 6:54 PM, Alan Modra <amodra@gmail.com> wrote:
>>>> >> > On Mon, Jun 12, 2017 at 05:18:58PM -0700, H.J. Lu wrote:
>>>> >> >> On Mon, Jun 12, 2017 at 4:32 PM, Alan Modra <amodra@gmail.com> wrote:
>>>> >> >> > On Sat, Jun 10, 2017 at 03:46:49PM -0700, H.J. Lu wrote:
>>>> >> >> >> This patch changes linker to always define referenced __start_SECNAME and
>>>> >> >> >> __stop_SECNAME if the input section name is the same output section name,
>>>> >> >> >> which is always true for orphaned sections, and SECNAME is a C identifier.
>>>> >> >> >
>>>> >> >> > I think this change is reasonable.
>>>> >> >> >
>>>> >> >> >> Also __start_SECNAME and __stop_SECNAME symbols are marked as hidden by
>>>> >> >> >> ELF linker.
>>>> >> >> >
>>>> >> >> > Why is this necessary?  Also, you make another change in behaviour
>>>> >> >>
>>>> >> >> It is to make sure that  __start_SECNAME and __stop_SECNAME
>>>> >> >> symbols     for section SECNAME in different modules are unique.
>>>> >> >
>>>> >> > I understand, and it would have been good if these symbols were
>>>> >> > defined that way in the beginning, but they weren't.  I'm concerned
>>>> >> > that if you make this change as well we will find some code that
>>>> >> > relies on the symbols being dynamic.
>>>> >>
>>>> >> My change fixes a real bug:
>>>> >>
>>>> >> https://sourceware.org/bugzilla/show_bug.cgi?id=20022
>>>> >
>>>> > Really, your change is a workaround not a fix.  You avoid exporting
>>>> > __start or __stop symbols because doing so shows a problem with gc.  A
>>>> > real fix for the PR would make ld do the right thing even in the
>>>> > presense of exported dynamic symbols.
>>>>
>>>> Exporting __start_SECNAME and __stop_SECNAME as dynamic
>>>> symbols may lead to unexpected behavior.  Reference to __start_SECNAME
>>>> and __stop_SECNAME to section SECNAME within a DSO will be
>>>> resolved to __start_SECNAME and __stop_SECNAME in another DSO
>>>> or executable.  In most cases, it isn't the intended behavior and there are
>>>> workaround if it is needed.
>>>
>>> You're correct regarding shared libraries.  I was thinking more about
>>> an executable when I said the patch was a workaround.  OK, so I
>>> suppose we should leave that in, and wait to see if we get complaints.
>>>
>>>> >> We can suggest workarounds if some codes depend on that.
>>>> >>
>>>> >> >>
>>>> >> >> > that you don't mention:  __start and __stop symbols were previously
>>>> >> >> > defined by ld -Ur, not just at final link.  You'll need to modify
>>>> >> >>
>>>> >> >> Is there a testcase for this behavior?
>>>> >> >
>>>> >> > Not that I'm aware of.
>>>> >> >
>>>> >>
>>>> >> >From ld manual:
>>>> >>
>>>> >> '-Ur'
>>>> >>      For anything other than C++ programs, this option is equivalent to
>>>> >>      '-r': it generates relocatable output--i.e., an output file that
>>>> >>      can in turn serve as input to 'ld'.  When linking C++ programs,
>>>> >>      '-Ur' _does_ resolve references to constructors, unlike '-r'.  It
>>>> >>      does not work to use '-Ur' on files that were themselves linked
>>>> >>      with '-Ur'; once the constructor table has been built, it cannot be
>>>> >>      added to.  Use '-Ur' only for the last partial link, and '-r' for
>>>> >>      the others.
>>>> >>
>>>> >> It isn't intended for  __start_SECNAME and __stop_SECNAME.  Check
>>>> >> config.build_constructors for __start_SECNAME and __stop_SECNAME
>>>> >> was introduced by:
>>>> >>
>>>> >> commit 71bfc0aef6964c54b8e29466e97fb246cdeb2049
>>>> >> Author: Alan Modra <amodra@gmail.com>
>>>> >> Date:   Thu Sep 7 07:08:58 2000 +0000
>>>> >>
>>>> >>     Fix list handling for orphan section output statements.
>>>> >
>>>> > That's a long time ago.  Prior to that patch, ld defined __start and
>>>> > __stop symbols for ld -r, which can lead to unexpected results for the
>>>> > unwary.  I can't remember, but I suspect I changed ld to define the
>>>> > symbols for -Ur so that people who wanted the previous ld -r behaviour
>>>> > for __start and __stop could get that by using ld -Ur.
>>>> >
>>>>
>>>> If it is the case, it isn't intended usage for ld -Ur and people shouldn't
>>>> depend on it.
>>>
>>> Please leave that behaviour unchanged.
>>>
>>
>> I am checking this patch to verify it and I will update my patch.
>>
>>
>
> I checked in this patch to add some tests for .startof.SECNAME
> and sizeof.SECNAME.
>

Don't define __start_SECNAME/__stop_SECNAME for -r.


-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ld-Don-t-define-__start_SECNAME-__stop_SECNAME-for-r.patch
Type: text/x-patch
Size: 3421 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20170613/3f9e4cb3/attachment.bin>