Re: [Bug ld/20828] GC-ed DSO symbols make corresponding symbols defined by a linker script local

Re: [Bug ld/20828] GC-ed DSO symbols make corresponding symbols defined by a linker script local

[Maciej W. Rozycki]

On Thu, 19 Jan 2017, Nick Clifton wrote:

> Checking Binutils in: bfin-elf ... LD REGRESSION: PR ld/20828 dynamic symbols with 

 As noted before, `_edata' is removed with `--gc-sections' and 
`--version-script' combined, where it is supposed to be exported.  This is 
regardless of the presence of `libpr20828.so' in the link, so it does not 
seem related to PR ld/20828 (same with `bfin-uclinux', so this is a 
genuine backend bug).

> Checking Binutils in: riscv32-elf ... LD REGRESSION: PR ld/20828 dynamic symbols with 

 LD segfaults with the initial `libpr20828.so' link (same with 
`riscv32-linux' BTW), so this is not related to PR ld/20828.  
Consequently all the subsequent dependent tests fail.

 It may be something missing from the linker script used, which is 
required by this port, but then LD should complain rather than crash (ISTR 
`aarch64' and `arm' ports segfaulting too if `.got' and/or `.got.plt' 
sections were discarded from output; this may be something worth looking 
into and handling gracefully as well).

> Checking Binutils in: metag-elf ... LD REGRESSION: PR ld/20828 dynamic symbols with

 Same as with `bfin-elf' (and likewise `metag-linux').

> Checking Binutils in: riscv64-elf ... LD REGRESSION: PR ld/20828 dynamic symbols with

 Same as with `riscv32-elf' (and likewise `riscv64-linux').

> Checking Binutils in: x86_64-solaris2 ... LD REGRESSION: PR ld/20828 dynamic symbols with

 There are two issues here -- one is trivial with `libpr20828.so' having 
extra symbols between `_fdata' and `_edata':

Symbol table '.dynsym' contains 6 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
     1: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT    1 _fdata
     2: 0000000000000000     0 OBJECT  GLOBAL DEFAULT    1 _DYNAMIC
     3: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  ABS _PROCEDURE_LINKAGE_TABLE_
     4: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT    1 _edata
     5: 00000000000001b8     0 OBJECT  GLOBAL DEFAULT    4 _GLOBAL_OFFSET_TABLE_

so this will be a straightforward dump pattern fix.

 The other is tricky -- the sole version script used is as follows:

{ global: _edata; local: *; };

and yet LD complains:

ld: anonymous version tag cannot be combined with other version tags

(so where's the non-anonymous version tag?).  I'll look into it.

> Checking Binutils in: mn10300-elf ... LD REGRESSION: PR ld/20828 dynamic symbols with

 Same as with `bfin-elf'.  I have checked `mn10300-linux' too, but all 
tests fail with:

Assembler messages:
Fatal error: selected target format 'elf32-am33lin' unknown

> Checking Binutils in: score-elf ... LD REGRESSION: PR ld/20828 dynamic symbols with

 As noted before, `_fdata' is exported with `--gc-sections' and 
`--version-script' combined, where it is supposed to be removed.  This 
only happens in the presence of `libpr20828.so' in the link, so it may be 
related to PR ld/20828.  I'll have a look into it.
 
> Checking Binutils in: sh-elf ... LD REGRESSION: PR ld/20828 dynamic symbols with

 Same as with `bfin-elf' (`sh-linux' is fine).

 I'll see if I can figure out easily what is going on with `bfin-elf' 
etc., but I am not going to dive into generic port breakage and I do 
encourage port maintainers to verify their port instead.

  Maciej

[PATCH 0/5][Bug ld/20828] GC-ed DSO symbols make corresponding symbols defined by a linker script local

[Maciej W. Rozycki]

On Fri, 20 Jan 2017, Maciej W. Rozycki wrote:

> On Thu, 19 Jan 2017, Nick Clifton wrote:
> 
> > Checking Binutils in: bfin-elf ... LD REGRESSION: PR ld/20828 dynamic symbols with 
> 
>  As noted before, `_edata' is removed with `--gc-sections' and 
> `--version-script' combined, where it is supposed to be exported.  This is 
> regardless of the presence of `libpr20828.so' in the link, so it does not 
> seem related to PR ld/20828 (same with `bfin-uclinux', so this is a 
> genuine backend bug).

 This has turned out to be a version script problem with targets that set 
their `bfd_get_symbol_leading_char' to `_'.  Test cases fixed with 2/5 in 
this patch series, covering all the targets affected; see the patch 
description for details.

> > Checking Binutils in: riscv32-elf ... LD REGRESSION: PR ld/20828 dynamic symbols with 
> 
>  LD segfaults with the initial `libpr20828.so' link (same with 
> `riscv32-linux' BTW), so this is not related to PR ld/20828.  
> Consequently all the subsequent dependent tests fail.
> 
>  It may be something missing from the linker script used, which is 
> required by this port, but then LD should complain rather than crash (ISTR 
> `aarch64' and `arm' ports segfaulting too if `.got' and/or `.got.plt' 
> sections were discarded from output; this may be something worth looking 
> into and handling gracefully as well).

 Indeed it is, as `riscv' targets crash when there's no `.plt' in output.  
This can probably be trivially fixed, but I'll leave it up to the port 
maintainers.  Test case failures worked around with 3/5 in this patch 
series, covering all the targets affected.

> > Checking Binutils in: x86_64-solaris2 ... LD REGRESSION: PR ld/20828 dynamic symbols with
> 
>  There are two issues here -- one is trivial with `libpr20828.so' having 
> extra symbols between `_fdata' and `_edata':
> 
> Symbol table '.dynsym' contains 6 entries:
>    Num:    Value          Size Type    Bind   Vis      Ndx Name
>      0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
>      1: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT    1 _fdata
>      2: 0000000000000000     0 OBJECT  GLOBAL DEFAULT    1 _DYNAMIC
>      3: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  ABS _PROCEDURE_LINKAGE_TABLE_
>      4: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT    1 _edata
>      5: 00000000000001b8     0 OBJECT  GLOBAL DEFAULT    4 _GLOBAL_OFFSET_TABLE_
> 
> so this will be a straightforward dump pattern fix.

 Fixed with 1/5 in this patch series, which also serves as a preparation 
for 2/5; see the comment with the patch itself.

>  The other is tricky -- the sole version script used is as follows:
> 
> { global: _edata; local: *; };
> 
> and yet LD complains:
> 
> ld: anonymous version tag cannot be combined with other version tags
> 
> (so where's the non-anonymous version tag?).  I'll look into it.

 This looks like a genuine port bug to me, which is however easy to fix I 
believe.  Fix proposed with 4/5.

> > Checking Binutils in: score-elf ... LD REGRESSION: PR ld/20828 dynamic symbols with
> 
>  As noted before, `_fdata' is exported with `--gc-sections' and 
> `--version-script' combined, where it is supposed to be removed.  This 
> only happens in the presence of `libpr20828.so' in the link, so it may be 
> related to PR ld/20828.  I'll have a look into it.

 Fix proposed with 5/5 now too, although it required a redesign of the 
original change, making it more complex.  See the patch description for 
details.

 Apologies for the breakage.

  Maciej

[committed 2/5] PR ld/20828: Remove leading `_' from symbols used in tests

[Maciej W. Rozycki]

Complement commit 81ff47b3a546 ("PR ld/20828: Fix linker script symbols
wrongly forced local with section GC") and remove the leading underscore 
from `_fdata' and `_edata' symbols used in tests, fixing a:

FAIL: PR ld/20828 dynamic symbols with section GC (version script)

failure with targets such as: `bfin-elf', `bfin-uclinux', `metag-elf', 
`metag-linux' `mn10300-elf', `sh-elf', `sh64-elf', and possibly other 
ones, that have `_' set (with `elf_symbol_leading_char') as the leading 
character for symbols.  As from commit 93252b1cf41a ("bfd/ld: handle ABI 
prefixes in version scripts") these targets strip the leading underscore 
before applying version script rules, because the (default) syntax for 
symbol names is that of the C language rather than their low-level 
symbol table encoding.

	ld/
	PR ld/20828
	* testsuite/ld-elf/pr20828.ld: Rename `_fdata' and `_edata' to 
	`fdata' and `edata' respectively.
	* testsuite/ld-elf/pr20828.ver: Adjust accordingly.
	* testsuite/ld-elf/pr20828-a.sd: Likewise.
	* testsuite/ld-elf/pr20828-b.sd: Likewise.
	* testsuite/ld-elf/pr20828-c.sd: Likewise.
---
 NB this change makes `edata' appear before `fdata' in symbol tables, 
which is unlike `_fdata' vs `_edata', however 1/5 has already made tests 
not depend on entry ordering.  No regressions with other targets.  
Committed as obvious, and backported to 2.28.

 As a side note, perhaps we might want to support a `raw' or suchlike 
version script entry encoding one day so that we can handle prefixed and 
unprefixed symbols separately, e.g. if say `_edata' and `edata' were 
present both at a time and required to have a different scope or version 
tag each.

  Maciej

binutils-bfd-elf-link-assignment-forced-local-test-symbol-leading-char.diff
Index: binutils/ld/testsuite/ld-elf/pr20828-a.sd
===================================================================
--- binutils.orig/ld/testsuite/ld-elf/pr20828-a.sd	2017-01-20 20:43:42.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/pr20828-a.sd	2017-01-20 20:39:54.878920000 +0000
@@ -1,9 +1,9 @@
-# Make sure `_fdata' is global rather than local in the dynamic symbol table,
+# Make sure `fdata' is global rather than local in the dynamic symbol table,
 # e.g.:
 #    Num:    Value  Size Type    Bind   Vis      Ndx Name
-#      1: 00000000     0 NOTYPE  GLOBAL DEFAULT    1 _fdata
+#      1: 00000000     0 NOTYPE  GLOBAL DEFAULT    1 fdata
 # vs:
-#      1: 00000000     0 NOTYPE  LOCAL  DEFAULT    1 _fdata
+#      1: 00000000     0 NOTYPE  LOCAL  DEFAULT    1 fdata
 #...
- *[0-9]+: +[0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +[0-9]+ +_fdata
+ *[0-9]+: +[0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +[0-9]+ +fdata
 #pass
Index: binutils/ld/testsuite/ld-elf/pr20828-b.sd
===================================================================
--- binutils.orig/ld/testsuite/ld-elf/pr20828-b.sd	2017-01-20 20:43:42.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/pr20828-b.sd	2017-01-20 20:39:54.896117000 +0000
@@ -1,9 +1,9 @@
-# Make sure `_edata' is global rather than local in the dynamic symbol table,
+# Make sure `edata' is global rather than local in the dynamic symbol table,
 # e.g.:
 #    Num:    Value  Size Type    Bind   Vis      Ndx Name
-#      1: 00000000     0 NOTYPE  GLOBAL DEFAULT    1 _edata
+#      1: 00000000     0 NOTYPE  GLOBAL DEFAULT    1 edata
 # vs:
-#      1: 00000000     0 NOTYPE  LOCAL  DEFAULT    1 _edata
+#      1: 00000000     0 NOTYPE  LOCAL  DEFAULT    1 edata
 #...
- *[0-9]+: +[0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +[0-9]+ +_edata
+ *[0-9]+: +[0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +[0-9]+ +edata
 #pass
Index: binutils/ld/testsuite/ld-elf/pr20828-c.sd
===================================================================
--- binutils.orig/ld/testsuite/ld-elf/pr20828-c.sd	2017-01-20 20:43:42.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/pr20828-c.sd	2017-01-20 20:39:54.908304000 +0000
@@ -1,7 +1,7 @@
-# Make sure no `_fdata' is present in the dynamic symbol table, e.g.:
+# Make sure no `fdata' is present in the dynamic symbol table, e.g.:
 #    Num:    Value  Size Type    Bind   Vis      Ndx Name
-#      1: 00000000     0 NOTYPE  LOCAL  DEFAULT    1 _fdata
+#      1: 00000000     0 NOTYPE  LOCAL  DEFAULT    1 fdata
 #failif
 #...
-.+ +_fdata
+.+ +fdata
 #pass
Index: binutils/ld/testsuite/ld-elf/pr20828.ld
===================================================================
--- binutils.orig/ld/testsuite/ld-elf/pr20828.ld	2017-01-20 20:43:42.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/pr20828.ld	2017-01-20 20:45:37.648329567 +0000
@@ -2,9 +2,9 @@ SECTIONS
 {
   .data :
   {
-    _fdata = .;
+    fdata = .;
     *(.data)
-    _edata = .;
+    edata = .;
   }
   .dynamic : { *(.dynamic) }
   .hash : { *(.hash) }
Index: binutils/ld/testsuite/ld-elf/pr20828.ver
===================================================================
--- binutils.orig/ld/testsuite/ld-elf/pr20828.ver	2017-01-20 20:43:42.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/pr20828.ver	2017-01-20 20:45:37.659399218 +0000
@@ -1 +1 @@
-{ global: _edata; local: *; };
+{ global: edata; local: *; };

[committed 1/5] PR ld/20828: Relax symbol ordering in tests

[Maciej W. Rozycki]

Complement commit 81ff47b3a546 ("PR ld/20828: Fix linker script symbols 
wrongly forced local with section GC") and make tests check for the 
presence of global `_fdata' and `_edata' symbols separately, removing 
any dependency on symbol table ordering for tests to succeed and 
removing:

FAIL: PR ld/20828 dynamic symbols with section GC (auxiliary shared library)
FAIL: PR ld/20828 dynamic symbols with section GC (plain)

failures with the `x86_64-solaris2' target, which has additional 
intervening entries:

Symbol table '.dynsym' contains 6 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
     1: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT    1 _fdata
     2: 0000000000000000     0 OBJECT  GLOBAL DEFAULT    1 _DYNAMIC
     3: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  ABS _PROCEDURE_LINKAGE_TABLE_
     4: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT    1 _edata
     5: 00000000000001b8     0 OBJECT  GLOBAL DEFAULT    4 _GLOBAL_OFFSET_TABLE_

Rename dump pattern files accordingly for consistency.

	ld/
	PR ld/20828
	* testsuite/ld-elf/pr20828-1.sd: Remove test.
	* testsuite/ld-elf/pr20828-a.sd: New test.
	* testsuite/ld-elf/pr20828-2a.sd: Rename test to...
	* testsuite/ld-elf/pr20828-b.sd: ... this.
	* testsuite/ld-elf/pr20828-2b.sd: Rename test to...
	* testsuite/ld-elf/pr20828-c.sd: ... this.
	* testsuite/ld-elf/shared.exp: Adjust accordingly.
---
 This also prepares for 2/5, which will reverse the ordering in the symbol 
table of the entries being checked for, for some reason I didn't bother 
investigating as irrelevant to PR ld/20828.

 Committed as obvious, and backported to 2.28.

  Maciej

binutils-bfd-elf-link-assignment-forced-local-test-symbol-order.diff
Index: binutils/ld/testsuite/ld-elf/pr20828-1.sd
===================================================================
--- binutils.orig/ld/testsuite/ld-elf/pr20828-1.sd	2017-01-20 20:41:09.442398616 +0000
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,12 +0,0 @@
-# Make sure symbols are global rather than local in the dynamic symbol table,
-# e.g.:
-#    Num:    Value  Size Type    Bind   Vis      Ndx Name
-#      1: 00000000     0 NOTYPE  GLOBAL DEFAULT    1 _fdata
-#      2: 00000000     0 NOTYPE  GLOBAL DEFAULT    1 _edata
-# vs:
-#      1: 00000000     0 NOTYPE  LOCAL  DEFAULT    1 _fdata
-#      2: 00000000     0 NOTYPE  LOCAL  DEFAULT    1 _edata
-#...
- *[0-9]+: +[0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +[0-9]+ +_fdata
- *[0-9]+: +[0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +[0-9]+ +_edata
-#pass
Index: binutils/ld/testsuite/ld-elf/pr20828-2a.sd
===================================================================
--- binutils.orig/ld/testsuite/ld-elf/pr20828-2a.sd	2017-01-20 20:41:09.465784341 +0000
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,9 +0,0 @@
-# Make sure `_edata' is global rather than local in the dynamic symbol table,
-# e.g.:
-#    Num:    Value  Size Type    Bind   Vis      Ndx Name
-#      1: 00000000     0 NOTYPE  GLOBAL DEFAULT    1 _edata
-# vs:
-#      1: 00000000     0 NOTYPE  LOCAL  DEFAULT    1 _edata
-#...
- *[0-9]+: +[0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +[0-9]+ +_edata
-#pass
Index: binutils/ld/testsuite/ld-elf/pr20828-2b.sd
===================================================================
--- binutils.orig/ld/testsuite/ld-elf/pr20828-2b.sd	2017-01-20 20:41:09.480896080 +0000
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,7 +0,0 @@
-# Make sure no `_fdata' is present in the dynamic symbol table, e.g.:
-#    Num:    Value  Size Type    Bind   Vis      Ndx Name
-#      1: 00000000     0 NOTYPE  LOCAL  DEFAULT    1 _fdata
-#failif
-#...
-.+ +_fdata
-#pass
Index: binutils/ld/testsuite/ld-elf/pr20828-a.sd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/pr20828-a.sd	2017-01-20 20:43:42.680046124 +0000
@@ -0,0 +1,9 @@
+# Make sure `_fdata' is global rather than local in the dynamic symbol table,
+# e.g.:
+#    Num:    Value  Size Type    Bind   Vis      Ndx Name
+#      1: 00000000     0 NOTYPE  GLOBAL DEFAULT    1 _fdata
+# vs:
+#      1: 00000000     0 NOTYPE  LOCAL  DEFAULT    1 _fdata
+#...
+ *[0-9]+: +[0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +[0-9]+ +_fdata
+#pass
Index: binutils/ld/testsuite/ld-elf/pr20828-b.sd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/pr20828-b.sd	2017-01-20 20:43:42.685227876 +0000
@@ -0,0 +1,9 @@
+# Make sure `_edata' is global rather than local in the dynamic symbol table,
+# e.g.:
+#    Num:    Value  Size Type    Bind   Vis      Ndx Name
+#      1: 00000000     0 NOTYPE  GLOBAL DEFAULT    1 _edata
+# vs:
+#      1: 00000000     0 NOTYPE  LOCAL  DEFAULT    1 _edata
+#...
+ *[0-9]+: +[0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +[0-9]+ +_edata
+#pass
Index: binutils/ld/testsuite/ld-elf/pr20828-c.sd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/pr20828-c.sd	2017-01-20 20:43:42.706662600 +0000
@@ -0,0 +1,7 @@
+# Make sure no `_fdata' is present in the dynamic symbol table, e.g.:
+#    Num:    Value  Size Type    Bind   Vis      Ndx Name
+#      1: 00000000     0 NOTYPE  LOCAL  DEFAULT    1 _fdata
+#failif
+#...
+.+ +_fdata
+#pass
Index: binutils/ld/testsuite/ld-elf/shared.exp
===================================================================
--- binutils.orig/ld/testsuite/ld-elf/shared.exp	2017-01-20 20:41:09.524308704 +0000
+++ binutils/ld/testsuite/ld-elf/shared.exp	2017-01-20 20:43:42.740061346 +0000
@@ -57,14 +57,16 @@ if { [check_gc_sections_available] } {
 	      (auxiliary shared library)" \
 	     "$LFLAGS -shared --gc-sections -T pr20828.ld" "" "$AFLAGS_PIC" \
 	     {pr20828.s} \
-	     {{readelf --dyn-syms pr20828-1.sd}} \
+	     {{readelf --dyn-syms pr20828-a.sd} \
+	      {readelf --dyn-syms pr20828-b.sd}} \
 	     "libpr20828.so"] \
 	[list \
 	    "PR ld/20828 dynamic symbols with section GC (plain)" \
 	     "$LFLAGS -shared --gc-sections -T pr20828.ld" \
 	     "tmpdir/libpr20828.so" "$AFLAGS_PIC" \
 	     {pr20828.s} \
-	     {{readelf --dyn-syms pr20828-1.sd}} \
+	     {{readelf --dyn-syms pr20828-a.sd} \
+	      {readelf --dyn-syms pr20828-b.sd}} \
 	     "pr20828-1.so"] \
 	[list \
 	    "PR ld/20828 dynamic symbols with section GC (version script)" \
@@ -73,8 +75,8 @@ if { [check_gc_sections_available] } {
 	     "tmpdir/libpr20828.so" \
 	     "$AFLAGS_PIC" \
 	     {pr20828.s} \
-	     {{readelf --dyn-syms pr20828-2a.sd} \
-	      {readelf --dyn-syms pr20828-2b.sd}} \
+	     {{readelf --dyn-syms pr20828-b.sd} \
+	      {readelf --dyn-syms pr20828-c.sd}} \
 	     "pr20828-2.so"]]
 }
 

[committed 3/5] PR ld/20828: Work around RISC-V failures

[Maciej W. Rozycki]

Complement commit 81ff47b3a546 ("PR ld/20828: Fix linker script symbols
wrongly forced local with section GC") and add `.plt' to the list of 
output sections created, fixing:

FAIL: PR ld/20828 dynamic symbols with section GC (auxiliary shared library)
FAIL: PR ld/20828 dynamic symbols with section GC (plain)
FAIL: PR ld/20828 dynamic symbols with section GC (version script)

failures with `riscv32-elf', `riscv32-linux', `riscv64-elf' and 
`riscv64-linux' targets caused by LD crashing in the absence of such a 
section.

	ld/
	PR ld/20828
	* testsuite/ld-elf/pr20828.ld: Add `.plt'.
---
 Committed as obvious and backported to 2.28.

  Maciej

binutils-bfd-elf-link-assignment-forced-local-test-riscvxx-elf.diff
Index: binutils/ld/testsuite/ld-elf/pr20828.ld
===================================================================
--- binutils.orig/ld/testsuite/ld-elf/pr20828.ld	2017-01-18 23:39:05.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/pr20828.ld	2017-01-20 13:52:21.319633176 +0000
@@ -13,6 +13,7 @@ SECTIONS
   .shstrtab : { *(.shstrtab) }
   .symtab : { *(.symtab) }
   .strtab : { *(.strtab) }
+  .plt : { *(.plt) }
   .got.plt : { *(.got.plt) }
   .got : { *(.got) }
   /DISCARD/ : { *(*) }

[PATCH 4/5] Solaris2/LD: Fix anonymous version script acceptance bug

[Maciej W. Rozycki]

Correct a bug in Solaris 2 linker emulation code triggered by a test 
introduced with commit 81ff47b3a546 ("PR ld/20828: Fix linker script 
symbols wrongly forced local with section GC") and only create implicit 
version nodes if versioning is actually introduced with a version script
(or VERSION command) rather than only global vs local symbol visibility 
selected, fixing an:

ld: anonymous version tag cannot be combined with other version tags

linker error produced whenever a version script (or VERSION command) is 
used that does not assign symbol versions, such as:

{ global: foo; bar; local: *; };

and consequently removing a:

FAIL: PR ld/20828 dynamic symbols with section GC (version script)

test suite failure with the `x86_64-solaris2' target.

	ld/
	* emultempl/solaris2.em (elf_solaris2_before_allocation): Do not
	add implicit version nodes if any existing version information 
	has no name.
---
 No regressions in `x86_64-solaris2' testing, OK to apply and backport to 
2.28?

  Maciej

binutils-solaris2-ld-register-vers-node.diff
Index: binutils/ld/emultempl/solaris2.em
===================================================================
--- binutils.orig/ld/emultempl/solaris2.em	2017-01-16 19:12:50.000000000 +0000
+++ binutils/ld/emultempl/solaris2.em	2017-01-20 14:02:15.617924483 +0000
@@ -76,7 +76,8 @@ elf_solaris2_before_allocation (void)
 
   /* Only do this if emitting a shared object and versioning is in place. */
   if (bfd_link_dll (&link_info)
-      && (link_info.version_info != NULL
+      && ((link_info.version_info != NULL
+	   && link_info.version_info->name[0] != '\0')
 	  || link_info.create_default_symver))
     {
       struct bfd_elf_version_expr *globals = NULL, *locals = NULL;

[PATCH 5/5] PR ld/20828: Reorder the symbol sweep stage of section GC

[Maciej W. Rozycki]

Complement commit 81ff47b3a546 ("PR ld/20828: Fix linker script symbols 
wrongly forced local with section GC") and move the symbol sweep stage 
of section GC from `elf_gc_sweep' to `bfd_elf_size_dynamic_sections', 
avoiding the need to clear the `forced_local' marker, problematic for 
targets that have special processing in their `elf_backend_hide_symbol' 
handler.  Set `mark' instead in `bfd_elf_record_link_assignment' and, 
matching changes from commit 3bd43ebcb602 ("ld --gc-sections fail with 
__tls_get_addr_opt"), also in PowerPC `__tls_get_addr_opt' handling 
code, removing a:

FAIL: PR ld/20828 dynamic symbols with section GC (version script)

test suite failure with the `score-elf' target.

The rationale is it is enough if symbols are swept at the beginning of 
`bfd_elf_size_dynamic_sections' as it is only in this function that the 
size of the GOT, the dynamic symbol table and other dynamic sections is 
determined, which will depend on the number of symbols making it to the 
dynamic symbol table.  It is also appropriate to do the sweep at this 
point as it is already after any changes have been made to symbols with 
`bfd_elf_record_link_assignment', and not possible any earlier as calls 
to that function are only made just beforehand -- barring audit entry 
processing -- via `gld${EMULATION_NAME}_find_statement_assignment' 
invoked from `gld${EMULATION_NAME}_before_allocation' which is the ELF 
handler for `ldemul_before_allocation'.

	bfd/
	PR ld/20828
	* elflink.c (bfd_elf_record_link_assignment): Revert last 
	change and don't ever clear `forced_local'.  Set `mark' 
	unconditionally.
	(elf_gc_sweep_symbol_info, elf_gc_sweep_symbol): Reorder within
	file.
	(elf_gc_sweep): Move the call to `elf_gc_sweep_symbol'...
	(bfd_elf_size_dynamic_sections): ... here.
	* elf32-ppc.c (ppc_elf_tls_setup): Don't clear `forced_local'
	and set `mark' instead in `__tls_get_addr_opt' processing.
	* elf64-ppc.c (ppc64_elf_tls_setup): Likewise.
---
 The `score-elf' target turned out problematic because it has non-trivial 
code in `s3_bfd_score_elf_hide_symbol' and `s7_bfd_score_elf_hide_symbol' 
(which interestingly enough are both identical down to individual 
characters, even though these handlers are dispatched from common 
`_bfd_score_elf_hide_symbol' code) for GOT accounting, all of which would 
have to be undone if the hiding of a symbol were to be undone.  There are 
some other targets which do extra stuff in their `elf_backend_hide_symbol' 
handlers as well, all of which has led me to conclude that clearing the 
`forced_local' marker is not a good idea after all and we need to redesign 
code such that we don't hide a symbol until we are absolutely sure it is 
going to be its final state.

 I've scheduled this change after the Solaris 2 anonymous version script 
support bug fix so that there are no failures across the PR ld/20828 tests
after this change.

 No regressions across my usual targets.  OK to apply and backport to 
2.28?

  Maciej

binutils-bfd-elf-gc-sweep-symbols-in-size-dynamic-sections.diff
Index: binutils/bfd/elf32-ppc.c
===================================================================
--- binutils.orig/bfd/elf32-ppc.c	2017-01-16 19:07:42.000000000 +0000
+++ binutils/bfd/elf32-ppc.c	2017-01-21 04:07:37.891052348 +0000
@@ -5287,7 +5287,7 @@ ppc_elf_tls_setup (bfd *obfd, struct bfd
 		  tga->root.type = bfd_link_hash_indirect;
 		  tga->root.u.i.link = &opt->root;
 		  ppc_elf_copy_indirect_symbol (info, opt, tga);
-		  opt->forced_local = 0;
+		  opt->mark = 1;
 		  if (opt->dynindx != -1)
 		    {
 		      /* Use __tls_get_addr_opt in dynamic relocations.  */
Index: binutils/bfd/elf64-ppc.c
===================================================================
--- binutils.orig/bfd/elf64-ppc.c	2017-01-16 19:07:43.000000000 +0000
+++ binutils/bfd/elf64-ppc.c	2017-01-21 04:07:30.777545315 +0000
@@ -8324,7 +8324,7 @@ ppc64_elf_tls_setup (struct bfd_link_inf
 		  tga_fd->root.type = bfd_link_hash_indirect;
 		  tga_fd->root.u.i.link = &opt_fd->root;
 		  ppc64_elf_copy_indirect_symbol (info, opt_fd, tga_fd);
-		  opt_fd->forced_local = 0;
+		  opt_fd->mark = 1;
 		  if (opt_fd->dynindx != -1)
 		    {
 		      /* Use __tls_get_addr_opt in dynamic relocations.  */
@@ -8341,7 +8341,7 @@ ppc64_elf_tls_setup (struct bfd_link_inf
 		      tga->root.type = bfd_link_hash_indirect;
 		      tga->root.u.i.link = &opt->root;
 		      ppc64_elf_copy_indirect_symbol (info, opt, tga);
-		      opt->forced_local = 0;
+		      opt->mark = 1;
 		      _bfd_elf_link_hash_hide_symbol (info, opt,
 						      tga->forced_local);
 		      htab->tls_get_addr = (struct ppc_link_hash_entry *) opt;
Index: binutils/bfd/elflink.c
===================================================================
--- binutils.orig/bfd/elflink.c	2017-01-20 23:54:23.624477307 +0000
+++ binutils/bfd/elflink.c	2017-01-21 03:47:47.528807062 +0000
@@ -671,17 +671,15 @@ bfd_elf_record_link_assignment (bfd *out
 
   /* If this symbol is not being provided by the linker script, and it is
      currently defined by a dynamic object, but not by a regular object,
-     then undo any forced local marking that may have been set by input
-     section garbage collection and clear out any version information
-     because the symbol will not be associated with the dynamic object
-     any more.  */
+     then clear out any version information because the symbol will not be
+     associated with the dynamic object any more.  */
   if (!provide
       && h->def_dynamic
       && !h->def_regular)
-    {
-      h->forced_local = 0;
-      h->verinfo.verdef = NULL;
-    }
+    h->verinfo.verdef = NULL;
+
+  /* Make sure this symbol is not garbage collected.  */
+  h->mark = 1;
 
   h->def_regular = 1;
 
@@ -5876,6 +5874,38 @@ bfd_elf_stack_segment_size (bfd *output_
   return TRUE;
 }
 
+/* Sweep symbols in swept sections.  Called via elf_link_hash_traverse.  */
+
+struct elf_gc_sweep_symbol_info
+{
+  struct bfd_link_info *info;
+  void (*hide_symbol) (struct bfd_link_info *, struct elf_link_hash_entry *,
+		       bfd_boolean);
+};
+
+static bfd_boolean
+elf_gc_sweep_symbol (struct elf_link_hash_entry *h, void *data)
+{
+  if (!h->mark
+      && (((h->root.type == bfd_link_hash_defined
+	    || h->root.type == bfd_link_hash_defweak)
+	   && !((h->def_regular || ELF_COMMON_DEF_P (h))
+		&& h->root.u.def.section->gc_mark))
+	  || h->root.type == bfd_link_hash_undefined
+	  || h->root.type == bfd_link_hash_undefweak))
+    {
+      struct elf_gc_sweep_symbol_info *inf;
+
+      inf = (struct elf_gc_sweep_symbol_info *) data;
+      (*inf->hide_symbol) (inf->info, h, TRUE);
+      h->def_regular = 0;
+      h->ref_regular = 0;
+      h->ref_regular_nonweak = 0;
+    }
+
+  return TRUE;
+}
+
 /* Set up the sizes and contents of the ELF dynamic sections.  This is
    called by the ELF linker emulation before_allocation routine.  We
    must set the sizes of the sections before the linker sets the
@@ -5906,6 +5936,22 @@ bfd_elf_size_dynamic_sections (bfd *outp
 
   bed = get_elf_backend_data (output_bfd);
 
+  if (info->gc_sections && bed->can_gc_sections)
+    {
+      struct elf_gc_sweep_symbol_info sweep_info;
+      unsigned long section_sym_count;
+
+      /* Remove the symbols that were in the swept sections from the
+	 dynamic symbol table.  GCFIXME: Anyone know how to get them
+	 out of the static symbol table as well?  */
+      sweep_info.info = info;
+      sweep_info.hide_symbol = bed->elf_backend_hide_symbol;
+      elf_link_hash_traverse (elf_hash_table (info), elf_gc_sweep_symbol,
+			      &sweep_info);
+
+      _bfd_elf_link_renumber_dynsyms (output_bfd, info, &section_sym_count);
+    }
+
   /* Any syms created from now on start with -1 in
      got.refcount/offset and plt.refcount/offset.  */
   elf_hash_table (info)->init_got_refcount
@@ -12853,38 +12899,6 @@ _bfd_elf_gc_mark_extra_sections (struct 
   return TRUE;
 }
 
-/* Sweep symbols in swept sections.  Called via elf_link_hash_traverse.  */
-
-struct elf_gc_sweep_symbol_info
-{
-  struct bfd_link_info *info;
-  void (*hide_symbol) (struct bfd_link_info *, struct elf_link_hash_entry *,
-		       bfd_boolean);
-};
-
-static bfd_boolean
-elf_gc_sweep_symbol (struct elf_link_hash_entry *h, void *data)
-{
-  if (!h->mark
-      && (((h->root.type == bfd_link_hash_defined
-	    || h->root.type == bfd_link_hash_defweak)
-	   && !((h->def_regular || ELF_COMMON_DEF_P (h))
-		&& h->root.u.def.section->gc_mark))
-	  || h->root.type == bfd_link_hash_undefined
-	  || h->root.type == bfd_link_hash_undefweak))
-    {
-      struct elf_gc_sweep_symbol_info *inf;
-
-      inf = (struct elf_gc_sweep_symbol_info *) data;
-      (*inf->hide_symbol) (inf->info, h, TRUE);
-      h->def_regular = 0;
-      h->ref_regular = 0;
-      h->ref_regular_nonweak = 0;
-    }
-
-  return TRUE;
-}
-
 /* The sweep phase of garbage collection.  Remove all garbage sections.  */
 
 typedef bfd_boolean (*gc_sweep_hook_fn)
@@ -12896,8 +12910,6 @@ elf_gc_sweep (bfd *abfd, struct bfd_link
   bfd *sub;
   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
   gc_sweep_hook_fn gc_sweep_hook = bed->gc_sweep_hook;
-  unsigned long section_sym_count;
-  struct elf_gc_sweep_symbol_info sweep_info;
 
   for (sub = info->input_bfds; sub != NULL; sub = sub->link.next)
     {
@@ -12963,15 +12975,6 @@ elf_gc_sweep (bfd *abfd, struct bfd_link
 	}
     }
 
-  /* Remove the symbols that were in the swept sections from the dynamic
-     symbol table.  GCFIXME: Anyone know how to get them out of the
-     static symbol table as well?  */
-  sweep_info.info = info;
-  sweep_info.hide_symbol = bed->elf_backend_hide_symbol;
-  elf_link_hash_traverse (elf_hash_table (info), elf_gc_sweep_symbol,
-			  &sweep_info);
-
-  _bfd_elf_link_renumber_dynsyms (abfd, info, &section_sym_count);
   return TRUE;
 }
 

Re: [committed 3/5] PR ld/20828: Work around RISC-V failures

[Andrew Waterman]

Thanks for bringing this to our attention, Maciej; I'll work on an actual fix.

On Mon, Jan 23, 2017 at 3:36 AM, Maciej W. Rozycki <macro@imgtec.com> wrote:
> Complement commit 81ff47b3a546 ("PR ld/20828: Fix linker script symbols
> wrongly forced local with section GC") and add `.plt' to the list of
> output sections created, fixing:
>
> FAIL: PR ld/20828 dynamic symbols with section GC (auxiliary shared library)
> FAIL: PR ld/20828 dynamic symbols with section GC (plain)
> FAIL: PR ld/20828 dynamic symbols with section GC (version script)
>
> failures with `riscv32-elf', `riscv32-linux', `riscv64-elf' and
> `riscv64-linux' targets caused by LD crashing in the absence of such a
> section.
>
>         ld/
>         PR ld/20828
>         * testsuite/ld-elf/pr20828.ld: Add `.plt'.
> ---
>  Committed as obvious and backported to 2.28.
>
>   Maciej
>
> binutils-bfd-elf-link-assignment-forced-local-test-riscvxx-elf.diff
> Index: binutils/ld/testsuite/ld-elf/pr20828.ld
> ===================================================================
> --- binutils.orig/ld/testsuite/ld-elf/pr20828.ld        2017-01-18 23:39:05.000000000 +0000
> +++ binutils/ld/testsuite/ld-elf/pr20828.ld     2017-01-20 13:52:21.319633176 +0000
> @@ -13,6 +13,7 @@ SECTIONS
>    .shstrtab : { *(.shstrtab) }
>    .symtab : { *(.symtab) }
>    .strtab : { *(.strtab) }
> +  .plt : { *(.plt) }
>    .got.plt : { *(.got.plt) }
>    .got : { *(.got) }
>    /DISCARD/ : { *(*) }

Re: [PATCH 4/5] Solaris2/LD: Fix anonymous version script acceptance bug

[Alan Modra]

On Mon, Jan 23, 2017 at 11:37:19AM +0000, Maciej W. Rozycki wrote:
> 	* emultempl/solaris2.em (elf_solaris2_before_allocation): Do not
> 	add implicit version nodes if any existing version information 
> 	has no name.

OK, but suggest you change the above to "..nodes if anonymous version
tag is being used".  "any existing version information has no name"
implies a search over all tags to me (which of course isn't needed due
to the restriction in lang_register_vers_node).

> ---
>  No regressions in `x86_64-solaris2' testing, OK to apply and backport to 
> 2.28?
> 
>   Maciej
> 
> binutils-solaris2-ld-register-vers-node.diff
> Index: binutils/ld/emultempl/solaris2.em
> ===================================================================
> --- binutils.orig/ld/emultempl/solaris2.em	2017-01-16 19:12:50.000000000 +0000
> +++ binutils/ld/emultempl/solaris2.em	2017-01-20 14:02:15.617924483 +0000
> @@ -76,7 +76,8 @@ elf_solaris2_before_allocation (void)
>  
>    /* Only do this if emitting a shared object and versioning is in place. */
>    if (bfd_link_dll (&link_info)
> -      && (link_info.version_info != NULL
> +      && ((link_info.version_info != NULL
> +	   && link_info.version_info->name[0] != '\0')
>  	  || link_info.create_default_symver))
>      {
>        struct bfd_elf_version_expr *globals = NULL, *locals = NULL;

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 5/5] PR ld/20828: Reorder the symbol sweep stage of section GC

[Alan Modra]

On Mon, Jan 23, 2017 at 11:38:20AM +0000, Maciej W. Rozycki wrote:
> 	PR ld/20828
> 	* elflink.c (bfd_elf_record_link_assignment): Revert last 
> 	change and don't ever clear `forced_local'.  Set `mark' 
> 	unconditionally.
> 	(elf_gc_sweep_symbol_info, elf_gc_sweep_symbol): Reorder within
> 	file.
> 	(elf_gc_sweep): Move the call to `elf_gc_sweep_symbol'...
> 	(bfd_elf_size_dynamic_sections): ... here.
> 	* elf32-ppc.c (ppc_elf_tls_setup): Don't clear `forced_local'
> 	and set `mark' instead in `__tls_get_addr_opt' processing.
> 	* elf64-ppc.c (ppc64_elf_tls_setup): Likewise.

I think this is OK for master, but please delay putting this patch on
the branch for a while.  There is a possibility that the change might
tickle bugs in some target check_relocs.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 4/5] Solaris2/LD: Fix anonymous version script acceptance bug

[Maciej W. Rozycki]

On Tue, 24 Jan 2017, Alan Modra wrote:

> > 	* emultempl/solaris2.em (elf_solaris2_before_allocation): Do not
> > 	add implicit version nodes if any existing version information 
> > 	has no name.
> 
> OK, but suggest you change the above to "..nodes if anonymous version
> tag is being used".  "any existing version information has no name"
> implies a search over all tags to me (which of course isn't needed due
> to the restriction in lang_register_vers_node).

 Right, I used "any" in the sense of "if there is", however I agree your 
proposed wording is clearer.  Thanks, applied now, with the entry updated.

  Maciej

Re: [PATCH 5/5] PR ld/20828: Reorder the symbol sweep stage of section GC

[Maciej W. Rozycki]

On Tue, 24 Jan 2017, Alan Modra wrote:

> > 	PR ld/20828
> > 	* elflink.c (bfd_elf_record_link_assignment): Revert last 
> > 	change and don't ever clear `forced_local'.  Set `mark' 
> > 	unconditionally.
> > 	(elf_gc_sweep_symbol_info, elf_gc_sweep_symbol): Reorder within
> > 	file.
> > 	(elf_gc_sweep): Move the call to `elf_gc_sweep_symbol'...
> > 	(bfd_elf_size_dynamic_sections): ... here.
> > 	* elf32-ppc.c (ppc_elf_tls_setup): Don't clear `forced_local'
> > 	and set `mark' instead in `__tls_get_addr_opt' processing.
> > 	* elf64-ppc.c (ppc64_elf_tls_setup): Likewise.
> 
> I think this is OK for master, but please delay putting this patch on
> the branch for a while.  There is a possibility that the change might
> tickle bugs in some target check_relocs.

 OK.  I think it should be fine as version scripts do their symbol forcing 
to the local scope at around this time too (I haven't dug through the 
sources to find the exact spot though; in any case they do it after 
`bfd_elf_record_link_assignment' calls have been made or the 
`pr20828-2.so' test aka "PR ld/20828 dynamic symbols with section GC 
(version script)" wouldn't pass with my original solution for this bug) 
and their action is effectively the same, with any and all the 
consequences to relocation handling.  This is one reason why I concluded 
doing it at this stage was fine; which I actually meant to state along the 
submission, but in the end it has escaped me somehow.

 Patch committed to master only then, thanks for your review.

 BTW, in the process of making this change I have discovered that the 
linker script PROVIDE keyword causes the symbol requested to be emitted 
even in the absence of a local reference if there is an identically named 
symbol exported from a DSO present in the link.  This can be reproduced 
with the `pr20828-1.so' test and its `pr20828.ld' script modified to use 
PROVIDE with its symbols, regardless of the presence of `--gc-sections', 
as long as `libpr20828.so' is included in the link.  If the DSO is absent, 
then the symbols are not emitted.

 I believe this is due to this piece:

  /* If this symbol is being provided by the linker script, and it is
     currently defined by a dynamic object, but not by a regular
     object, then mark it as undefined so that the generic linker will
     force the correct value.  */
  if (provide
      && h->def_dynamic
      && !h->def_regular)
    h->root.type = bfd_link_hash_undefined;

in `bfd_elf_record_link_assignment', the presence of which however 
predates our repo history and therefore any justification is hard to track 
down.  Our manual however only states that:

"In some cases, it is desirable for a linker script to define a symbol
only if it is referenced and is not defined by any object included in
the link.  For example, traditional linkers defined the symbol `etext'.
However, ANSI C requires that the user be able to use `etext' as a
function name without encountering an error.  The `PROVIDE' keyword may
be used to define a symbol, such as `etext', only if it is referenced
but not defined.  The syntax is `PROVIDE(SYMBOL = EXPRESSION)'."

which given our semantics is I believe at least ambiguous as it does not 
tell static and dynamic objects apart -- it merely states "any object".

 Do you or does anyone know what the purpose of this special case is?  It 
might be to preempt the DSO symbol so that the DSO uses ours and not its 
own, however preemption is not going to happen in a `-shared' link anyway 
(and it seems against the spirit of PROVIDE).

 Shall we keep it (presumably yes, owing to its long history) and make it 
clear in documentation?

  Maciej

Re: [PATCH 5/5] PR ld/20828: Reorder the symbol sweep stage of section GC

[Alan Modra]

On Tue, Jan 24, 2017 at 02:16:06PM +0000, Maciej W. Rozycki wrote:
> On Tue, 24 Jan 2017, Alan Modra wrote:
> 
> > > 	PR ld/20828
> > > 	* elflink.c (bfd_elf_record_link_assignment): Revert last 
> > > 	change and don't ever clear `forced_local'.  Set `mark' 
> > > 	unconditionally.
> > > 	(elf_gc_sweep_symbol_info, elf_gc_sweep_symbol): Reorder within
> > > 	file.
> > > 	(elf_gc_sweep): Move the call to `elf_gc_sweep_symbol'...
> > > 	(bfd_elf_size_dynamic_sections): ... here.
> > > 	* elf32-ppc.c (ppc_elf_tls_setup): Don't clear `forced_local'
> > > 	and set `mark' instead in `__tls_get_addr_opt' processing.
> > > 	* elf64-ppc.c (ppc64_elf_tls_setup): Likewise.
> > 
> > I think this is OK for master, but please delay putting this patch on
> > the branch for a while.  There is a possibility that the change might
> > tickle bugs in some target check_relocs.
> 
>  OK.  I think it should be fine as version scripts do their symbol forcing 

Yes, I think it is OK too, but I'd like to give it a few days on
master to see whether there might be unforseen interactions.

>  BTW, in the process of making this change I have discovered that the 
> linker script PROVIDE keyword causes the symbol requested to be emitted 
> even in the absence of a local reference if there is an identically named 
> symbol exported from a DSO present in the link.  This can be reproduced 
> with the `pr20828-1.so' test and its `pr20828.ld' script modified to use 
> PROVIDE with its symbols, regardless of the presence of `--gc-sections', 
> as long as `libpr20828.so' is included in the link.  If the DSO is absent, 
> then the symbols are not emitted.
> 
>  I believe this is due to this piece:
> 
>   /* If this symbol is being provided by the linker script, and it is
>      currently defined by a dynamic object, but not by a regular
>      object, then mark it as undefined so that the generic linker will
>      force the correct value.  */
>   if (provide
>       && h->def_dynamic
>       && !h->def_regular)
>     h->root.type = bfd_link_hash_undefined;
> 
> in `bfd_elf_record_link_assignment', the presence of which however 
> predates our repo history and therefore any justification is hard to track 
> down.

This one goes back to
commit b5279eb6a9672dba08ce9bbef0490f4bf26243f3
Author: Ian Lance Taylor <ian@airs.com>
Date:   Tue Jul 4 17:43:05 1995 +0000

The old history is in our git repo but it's a little difficult to go
beyond 1999-05-03.  You need to pick one of the commits before the
multiple "Initial creation of sourceware repository" commits..  In
this case

git blame 1730ec6b -- bfd/elflink.h

gets you the old history.

> Our manual however only states that:
> 
> "In some cases, it is desirable for a linker script to define a symbol
> only if it is referenced and is not defined by any object included in
> the link.  For example, traditional linkers defined the symbol `etext'.
> However, ANSI C requires that the user be able to use `etext' as a
> function name without encountering an error.  The `PROVIDE' keyword may
> be used to define a symbol, such as `etext', only if it is referenced
> but not defined.  The syntax is `PROVIDE(SYMBOL = EXPRESSION)'."
> 
> which given our semantics is I believe at least ambiguous as it does not 
> tell static and dynamic objects apart -- it merely states "any object".
> 
>  Do you or does anyone know what the purpose of this special case is?  It 
> might be to preempt the DSO symbol so that the DSO uses ours and not its 
> own, however preemption is not going to happen in a `-shared' link anyway 
> (and it seems against the spirit of PROVIDE).
> 
>  Shall we keep it (presumably yes, owing to its long history) and make it 
> clear in documentation?

I think we have to keep the current behaviour.  PROVIDE is likely the
way it is for symbols like "etext" that might be exported from shared
libraries but the executable needs its own value as given by the script.

A doc patch is preapproved.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 5/5] PR ld/20828: Reorder the symbol sweep stage of section GC

[Maciej W. Rozycki]

On Wed, 25 Jan 2017, Alan Modra wrote:

> >  BTW, in the process of making this change I have discovered that the 
> > linker script PROVIDE keyword causes the symbol requested to be emitted 
> > even in the absence of a local reference if there is an identically named 
> > symbol exported from a DSO present in the link.  This can be reproduced 
> > with the `pr20828-1.so' test and its `pr20828.ld' script modified to use 
> > PROVIDE with its symbols, regardless of the presence of `--gc-sections', 
> > as long as `libpr20828.so' is included in the link.  If the DSO is absent, 
> > then the symbols are not emitted.
> > 
> >  I believe this is due to this piece:
> > 
> >   /* If this symbol is being provided by the linker script, and it is
> >      currently defined by a dynamic object, but not by a regular
> >      object, then mark it as undefined so that the generic linker will
> >      force the correct value.  */
> >   if (provide
> >       && h->def_dynamic
> >       && !h->def_regular)
> >     h->root.type = bfd_link_hash_undefined;
> > 
> > in `bfd_elf_record_link_assignment', the presence of which however 
> > predates our repo history and therefore any justification is hard to track 
> > down.
> 
> This one goes back to
> commit b5279eb6a9672dba08ce9bbef0490f4bf26243f3
> Author: Ian Lance Taylor <ian@airs.com>
> Date:   Tue Jul 4 17:43:05 1995 +0000

 Thanks, though unsurprisingly this doesn't provide any input beyond what 
we already know.  I take it PR 7164 refers to our old GNATS bug tracker or 
maybe even Cygnus's internal records -- has this data been migrated 
anywhere or has it been lost (or perhaps sits catching dust on tapes 
somewhere)?

> The old history is in our git repo but it's a little difficult to go
> beyond 1999-05-03.  You need to pick one of the commits before the
> multiple "Initial creation of sourceware repository" commits..  In
> this case
> 
> git blame 1730ec6b -- bfd/elflink.h
> 
> gets you the old history.

 Thanks, that's really useful indeed!  I didn't realise we had this data.  
I'm not a GIT expert, but for the record:

$ git log --full-history

seems to go past commit 252b5132c753 ("19990502 sourceware import") 
automagically.

> > Our manual however only states that:
> > 
> > "In some cases, it is desirable for a linker script to define a symbol
> > only if it is referenced and is not defined by any object included in
> > the link.  For example, traditional linkers defined the symbol `etext'.
> > However, ANSI C requires that the user be able to use `etext' as a
> > function name without encountering an error.  The `PROVIDE' keyword may
> > be used to define a symbol, such as `etext', only if it is referenced
> > but not defined.  The syntax is `PROVIDE(SYMBOL = EXPRESSION)'."
> > 
> > which given our semantics is I believe at least ambiguous as it does not 
> > tell static and dynamic objects apart -- it merely states "any object".
> > 
> >  Do you or does anyone know what the purpose of this special case is?  It 
> > might be to preempt the DSO symbol so that the DSO uses ours and not its 
> > own, however preemption is not going to happen in a `-shared' link anyway 
> > (and it seems against the spirit of PROVIDE).
> > 
> >  Shall we keep it (presumably yes, owing to its long history) and make it 
> > clear in documentation?
> 
> I think we have to keep the current behaviour.  PROVIDE is likely the
> way it is for symbols like "etext" that might be exported from shared
> libraries but the executable needs its own value as given by the script.

 Right, but why is the symbol PROVIDEd defined even if there's no regular 
reference?  I'm thinking in terms of a change like below, which makes the 
keyword match its description -- in the absence of a regular reference I'd 
expect no regular definition regardless of whether there's a dynamic 
definition present or not.  I wonder if this was a deliberate choice or an 
omission; and either way if we decide to keep the current semantics, then 
I think we need to properly document it to avoid people's confusion.

> A doc patch is preapproved.

 I'm tempted to just s/any object/any static object/ in the paragraph 
quoted above, but let's sort out my concern first.

  Maciej

binutils-bfd-record-link-assignment-provide-unref.diff
Index: binutils/bfd/elflink.c
===================================================================
--- binutils.orig/bfd/elflink.c	2017-01-25 22:35:51.360905740 +0000
+++ binutils/bfd/elflink.c	2017-01-25 23:05:51.315467007 +0000
@@ -667,7 +667,12 @@ bfd_elf_record_link_assignment (bfd *out
   if (provide
       && h->def_dynamic
       && !h->def_regular)
-    h->root.type = bfd_link_hash_undefined;
+    {
+      if (h->ref_regular)
+	h->root.type = bfd_link_hash_undefined;
+      else
+	return TRUE;
+    }
 
   /* If this symbol is not being provided by the linker script, and it is
      currently defined by a dynamic object, but not by a regular object,

Re: [PATCH 5/5] PR ld/20828: Reorder the symbol sweep stage of section GC

[Alan Modra]

On Wed, Jan 25, 2017 at 11:51:34PM +0000, Maciej W. Rozycki wrote:
> --- binutils.orig/bfd/elflink.c	2017-01-25 22:35:51.360905740 +0000
> +++ binutils/bfd/elflink.c	2017-01-25 23:05:51.315467007 +0000
> @@ -667,7 +667,12 @@ bfd_elf_record_link_assignment (bfd *out
>    if (provide
>        && h->def_dynamic
>        && !h->def_regular)
> -    h->root.type = bfd_link_hash_undefined;
> +    {
> +      if (h->ref_regular)
> +	h->root.type = bfd_link_hash_undefined;
> +      else
> +	return TRUE;
> +    }
>  
>    /* If this symbol is not being provided by the linker script, and it is
>       currently defined by a dynamic object, but not by a regular object,

After thinking about this some more, I'm inclined to say the intent of
PROVIDE was to define the symbol if referenced from either static or
dynamic objects.  So how do you tell if a symbol is referenced from a
dynamic object?  You can't exactly, short of decompiling the code, but
if the dynamic object contains a dynamic undefined symbol then I'd say
that should count as a reference.  What about a dynamic defined
symbol?  Well, if it is default visibility then a dynamic defined
symbol will be overridden by a symbol defined in the executable.  I
think that means we need to count dynamic definitions as potential
references too.

If a defined dynamic symbol is non-default visibility then clearly the
dynamic object can't reference another symbol of the same name in the
executable (ignoring HJ's horrible protected copies).  So we do have a
bug, but not the one you're "fixing" here.

It might be reasonable to change the meaning of PROVIDE to define the
symbol only if referenced from static objects, but that needs a
different patch.  Also, be prepared for howls of outrage.  :)

-- 
Alan Modra
Australia Development Lab, IBM