This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Always define referenced __start_SECNAME/__stop_SECNAME
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.
--
H.J.
From da614360f520b84a9c87506eb0c880f7a056468b Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 13 Jun 2017 12:03:40 -0700
Subject: [PATCH] ld: Add tests for .startof.SECNAME/.sizeof.SECNAME
* testsuite/ld-elf/sizeof.d: Renamed to ...
* testsuite/ld-elf/sizeofa.d: This. Updated.
* testsuite/ld-elf/startof.d: Renamed to ...
* testsuite/ld-elf/startofa.d: This. Updated.
* testsuite/ld-elf/sizeofb.d: New file.
* testsuite/ld-elf/startofb.d: Likewise.
---
ld/ChangeLog | 9 +++++++++
ld/testsuite/ld-elf/{sizeof.d => sizeofa.d} | 1 +
ld/testsuite/ld-elf/sizeofb.d | 13 +++++++++++++
ld/testsuite/ld-elf/{startof.d => startofa.d} | 1 +
ld/testsuite/ld-elf/startofb.d | 13 +++++++++++++
5 files changed, 37 insertions(+)
rename ld/testsuite/ld-elf/{sizeof.d => sizeofa.d} (94%)
create mode 100644 ld/testsuite/ld-elf/sizeofb.d
rename ld/testsuite/ld-elf/{startof.d => startofa.d} (94%)
create mode 100644 ld/testsuite/ld-elf/startofb.d
diff --git a/ld/ChangeLog b/ld/ChangeLog
index dcfc602..59c99ae 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,5 +1,14 @@
2017-06-13 H.J. Lu <hongjiu.lu@intel.com>
+ * testsuite/ld-elf/sizeof.d: Renamed to ...
+ * testsuite/ld-elf/sizeofa.d: This. Updated.
+ * testsuite/ld-elf/startof.d: Renamed to ...
+ * testsuite/ld-elf/startofa.d: This. Updated.
+ * testsuite/ld-elf/sizeofb.d: New file.
+ * testsuite/ld-elf/startofb.d: Likewise.
+
+2017-06-13 H.J. Lu <hongjiu.lu@intel.com>
+
PR ld/20022
PR ld/21557
PR ld/21562
diff --git a/ld/testsuite/ld-elf/sizeof.d b/ld/testsuite/ld-elf/sizeofa.d
similarity index 94%
rename from ld/testsuite/ld-elf/sizeof.d
rename to ld/testsuite/ld-elf/sizeofa.d
index 7cad75a..0aac607 100644
--- a/ld/testsuite/ld-elf/sizeof.d
+++ b/ld/testsuite/ld-elf/sizeofa.d
@@ -1,3 +1,4 @@
+#source: sizeof.s
#ld: -Ur
#readelf: -sW
diff --git a/ld/testsuite/ld-elf/sizeofb.d b/ld/testsuite/ld-elf/sizeofb.d
new file mode 100644
index 0000000..331c386
--- /dev/null
+++ b/ld/testsuite/ld-elf/sizeofb.d
@@ -0,0 +1,13 @@
+#source: sizeof.s
+#ld: -shared
+#readelf: -sW
+#target: *-*-linux* *-*-gnu*
+
+Symbol table '\.dynsym' contains [0-9]+ entries:
+ +Num: +Value +Size Type +Bind +Vis +Ndx Name
+ +0: 0+ +0 +NOTYPE +LOCAL +DEFAULT +UND +
+#...
+ +[0-9]+: +[a-f0-9]+ +0 +NOTYPE +LOCAL +DEFAULT +[0-9]+ +__stop_scnfoo
+#...
+ +[0-9]+: 0+10 + +0 +NOTYPE +GLOBAL +DEFAULT +ABS +.sizeof.scnfoo
+#pass
diff --git a/ld/testsuite/ld-elf/startof.d b/ld/testsuite/ld-elf/startofa.d
similarity index 94%
rename from ld/testsuite/ld-elf/startof.d
rename to ld/testsuite/ld-elf/startofa.d
index cf1f064..00ab27e 100644
--- a/ld/testsuite/ld-elf/startof.d
+++ b/ld/testsuite/ld-elf/startofa.d
@@ -1,3 +1,4 @@
+#source: startof.s
#ld: -Ur
#readelf: -sW
diff --git a/ld/testsuite/ld-elf/startofb.d b/ld/testsuite/ld-elf/startofb.d
new file mode 100644
index 0000000..e03bd23
--- /dev/null
+++ b/ld/testsuite/ld-elf/startofb.d
@@ -0,0 +1,13 @@
+#source: startof.s
+#ld: -shared
+#readelf: -sW
+#target: *-*-linux* *-*-gnu*
+
+Symbol table '\.dynsym' contains [0-9]+ entries:
+ +Num: +Value +Size Type +Bind +Vis +Ndx Name
+ +0: 0+ +0 +NOTYPE +LOCAL +DEFAULT +UND +
+#...
+ +[0-9]+: +[a-f0-9]+ +0 +NOTYPE +GLOBAL +DEFAULT +[0-9]+ +.startof.scnfoo
+#...
+ +[0-9]+: +[a-f0-9]+ +0 +NOTYPE +LOCAL +DEFAULT +[0-9]+ +__start_scnfoo
+#pass
--
2.9.4