Mark all weak aliases for copy relocations

Message ID 20200125160210.136389-1-hjl.tools@gmail.com
State New
Headers show
Series
  • Mark all weak aliases for copy relocations
Related show

Commit Message

H.J. Lu Jan. 25, 2020, 4:02 p.m.
With copy relocations, mark all weak aliases when marking the non-weak
definition for reference to one weak alias.

bfd/

	PR ld/25458
	* elflink.c (_bfd_elf_gc_mark_rsec): Mark all weak aliases for
	copy relocations.

ld/

	PR ld/25458
	* testsuite/ld-elf/pr25458.map: New file.
	* testsuite/ld-elf/pr25458.rd: Likewise.
	* testsuite/ld-elf/pr25458a.s: Likewise.
	* testsuite/ld-elf/pr25458b.s: Likewise.
	* testsuite/ld-elf/shared.exp: Run PR ld/25458 test.
---
 bfd/elflink.c                   | 10 +++++++---
 ld/testsuite/ld-elf/pr25458.map |  4 ++++
 ld/testsuite/ld-elf/pr25458.rd  | 10 ++++++++++
 ld/testsuite/ld-elf/pr25458a.s  |  6 ++++++
 ld/testsuite/ld-elf/pr25458b.s  | 11 +++++++++++
 ld/testsuite/ld-elf/shared.exp  | 25 +++++++++++++++++++++++++
 6 files changed, 63 insertions(+), 3 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/pr25458.map
 create mode 100644 ld/testsuite/ld-elf/pr25458.rd
 create mode 100644 ld/testsuite/ld-elf/pr25458a.s
 create mode 100644 ld/testsuite/ld-elf/pr25458b.s

-- 
2.24.1

Comments

Alan Modra Jan. 26, 2020, 4:11 a.m. | #1
On Sat, Jan 25, 2020 at 08:02:10AM -0800, H.J. Lu wrote:
> With copy relocations, mark all weak aliases when marking the non-weak

> definition for reference to one weak alias.

> 

> bfd/

> 

> 	PR ld/25458

> 	* elflink.c (_bfd_elf_gc_mark_rsec): Mark all weak aliases for

> 	copy relocations.

> 

> ld/

> 

> 	PR ld/25458

> 	* testsuite/ld-elf/pr25458.map: New file.

> 	* testsuite/ld-elf/pr25458.rd: Likewise.

> 	* testsuite/ld-elf/pr25458a.s: Likewise.

> 	* testsuite/ld-elf/pr25458b.s: Likewise.

> 	* testsuite/ld-elf/shared.exp: Run PR ld/25458 test.


OK.

-- 
Alan Modra
Australia Development Lab, IBM
Alan Modra Jan. 26, 2020, 4:39 a.m. | #2
On Sun, Jan 26, 2020 at 02:41:56PM +1030, Alan Modra wrote:
> On Sat, Jan 25, 2020 at 08:02:10AM -0800, H.J. Lu wrote:

> > With copy relocations, mark all weak aliases when marking the non-weak

> > definition for reference to one weak alias.

> > 

> > bfd/

> > 

> > 	PR ld/25458

> > 	* elflink.c (_bfd_elf_gc_mark_rsec): Mark all weak aliases for

> > 	copy relocations.

> > 

> > ld/

> > 

> > 	PR ld/25458

> > 	* testsuite/ld-elf/pr25458.map: New file.

> > 	* testsuite/ld-elf/pr25458.rd: Likewise.

> > 	* testsuite/ld-elf/pr25458a.s: Likewise.

> > 	* testsuite/ld-elf/pr25458b.s: Likewise.

> > 	* testsuite/ld-elf/shared.exp: Run PR ld/25458 test.

> 

> OK.


Cancel that OK until you fix the following.

aarch64-elf  +FAIL: Build pr25458
arc-elf  +FAIL: Build pr25458
arm-elf  +FAIL: Build pr25458
arm-netbsdelf  +FAIL: Build pr25458
arm-nto  +FAIL: Build pr25458
arm-symbianelf  +FAIL: Build pr25458
frv-linux  +FAIL: Build pr25458
i386-lynxos  +FAIL: Build pr25458
i686-nto  +FAIL: Build pr25458
i686-pc-elf  +FAIL: Build pr25458
lm32-linux  +FAIL: Build pr25458
m68k-elf  +FAIL: Build pr25458
mn10300-elf  +FAIL: Build pr25458
powerpc64-freebsd  +FAIL: Build pr25458
powerpc-eabisim  +FAIL: Build pr25458
powerpc-eabivle  +FAIL: Build pr25458
powerpc-freebsd  +FAIL: Build pr25458
powerpcle-elf  +FAIL: Build pr25458
powerpc-nto  +FAIL: Build pr25458
score-elf  +FAIL: Build pr25458
shle-unknown-netbsdelf  +FAIL: Build pr25458
sh-nto  +FAIL: Build pr25458
sh-rtems  +FAIL: Build pr25458
sparc-elf  +FAIL: Build pr25458
sparc-sun-solaris2  +FAIL: Build pr25458
tic6x-elf  +FAIL: Build pr25458
vax-netbsdelf  +FAIL: Build pr25458
x86_64-cloudabi  +FAIL: Build pr25458
xtensa-elf  +FAIL: Build pr25458

-- 
Alan Modra
Australia Development Lab, IBM
Alan Modra Jan. 26, 2020, 12:23 p.m. | #3
On Sun, Jan 26, 2020 at 03:09:42PM +1030, Alan Modra wrote:
> Cancel that OK until you fix the following.


Never mind, I'll commit the following tomorrow provided my overnight
test run looks reasonable.  I decided the comment needed fixing too.

Most of the fails were due to different dynamic symbol order.

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 300be3f743..5217528a79 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -13123,7 +13123,7 @@ _bfd_elf_gc_mark_rsec (struct bfd_link_info *info, asection *sec,
 		       bfd_boolean *start_stop)
 {
   unsigned long r_symndx;
-  struct elf_link_hash_entry *h;
+  struct elf_link_hash_entry *h, *hw;
 
   r_symndx = cookie->rel->r_info >> cookie->r_sym_shift;
   if (r_symndx == STN_UNDEF)
@@ -13143,12 +13143,16 @@ _bfd_elf_gc_mark_rsec (struct bfd_link_info *info, asection *sec,
 	     || h->root.type == bfd_link_hash_warning)
 	h = (struct elf_link_hash_entry *) h->root.u.i.link;
       h->mark = 1;
-      /* If this symbol is weak and there is a non-weak definition, we
-	 keep the non-weak definition because many backends put
-	 dynamic reloc info on the non-weak definition for code
-	 handling copy relocs.  */
-      if (h->is_weakalias)
-	weakdef (h)->mark = 1;
+      /* Keep all aliases of the symbol too.  If an object symbol
+	 needs to be copied into .dynbss then all of its aliases
+	 should be present as dynamic symbols, not just the one used
+	 on the copy relocation.  */
+      hw = h;
+      while (hw->is_weakalias)
+	{
+	  hw = hw->u.alias;
+	  hw->mark = 1;
+	}
 
       if (start_stop != NULL)
 	{
diff --git a/ld/testsuite/ld-elf/pr25458.map b/ld/testsuite/ld-elf/pr25458.map
new file mode 100644
index 0000000000..5578d1fae7
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr25458.map
@@ -0,0 +1,4 @@
+FOO {
+global:
+  __environ; _environ; environ;
+};
diff --git a/ld/testsuite/ld-elf/pr25458.rd b/ld/testsuite/ld-elf/pr25458.rd
new file mode 100644
index 0000000000..d0fc6b9bed
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr25458.rd
@@ -0,0 +1,10 @@
+#...
+Symbol table '\.dynsym' contains [0-9]+ entries:
+ +Num: +Value +Size Type +Bind +Vis +Ndx Name
+#...
+ +[0-9]+: [0-9a-f]+ +(4|8)+ OBJECT +(WEAK|GLOBAL) +DEFAULT +[0-9]+ _*environ@FOO \(2\)
+#...
+ +[0-9]+: [0-9a-f]+ +(4|8)+ OBJECT +(WEAK|GLOBAL) +DEFAULT +[0-9]+ _*environ@FOO \(2\)
+#...
+ +[0-9]+: [0-9a-f]+ +(4|8)+ OBJECT +(WEAK|GLOBAL) +DEFAULT +[0-9]+ _*environ@FOO \(2\)
+#pass
diff --git a/ld/testsuite/ld-elf/pr25458a.s b/ld/testsuite/ld-elf/pr25458a.s
new file mode 100644
index 0000000000..59e6af2c19
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr25458a.s
@@ -0,0 +1,6 @@
+	.text
+	.globl	_start
+	.type	_start, %function
+_start:
+	.dc.a	environ
+	.size	_start, .-_start
diff --git a/ld/testsuite/ld-elf/pr25458b.s b/ld/testsuite/ld-elf/pr25458b.s
new file mode 100644
index 0000000000..ff64cc7584
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr25458b.s
@@ -0,0 +1,11 @@
+	.data
+	.globl __environ
+	.type __environ,%object
+__environ:
+	.dc.a	0
+	.size	__environ, .-__environ
+	.weak _environ
+	.globl _environ
+	.set _environ, __environ
+	.weak environ
+	.set environ, __environ
diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp
index e03906a142..c56df1243c 100644
--- a/ld/testsuite/ld-elf/shared.exp
+++ b/ld/testsuite/ld-elf/shared.exp
@@ -337,6 +337,38 @@ if { [check_gc_sections_available] } {
 	    "pr22150" \
 	] \
     ]
+
+    switch $target_triplet {
+	# exclude targets that don't support copy relocs
+	{ bfin-*-* } { }
+	{ frv-*-* } { }
+	{ lm32-*-* } { }
+	{ mips*-*-* } { }
+	{ tic6x-*-* } { }
+	{ xtensa-*-* } { }
+	default {
+	    run_ld_link_tests [list \
+		[list \
+		     "Build pr25458.so" \
+		     "$LFLAGS -shared --version-script=pr25458.map" \
+		     "" \
+		     "$AFLAGS_PIC" \
+		     {pr25458b.s} \
+		     {} \
+		     "pr25458.so" \
+		] \
+		[list \
+		     "Build pr25458" \
+		     "$LFLAGS -e _start --gc-sections" \
+		     "tmpdir/pr25458.so" \
+		     "$AFLAGS_PIC" \
+		     {pr25458a.s} \
+		     {{readelf {--dyn-sym --wide} pr25458.rd}} \
+		     "pr25458" \
+		] \
+	    ]
+	}
+    }
 }
 
 set ASFLAGS $old_ASFLAGS

-- 
Alan Modra
Australia Development Lab, IBM
Alan Modra Jan. 27, 2020, 12:34 a.m. | #4
On Sun, Jan 26, 2020 at 10:53:58PM +1030, Alan Modra wrote:
> +    switch $target_triplet {

> +	# exclude targets that don't support copy relocs

> +	{ bfin-*-* } { }

> +	{ frv-*-* } { }

> +	{ lm32-*-* } { }

> +	{ mips*-*-* } { }

> +	{ tic6x-*-* } { }

> +	{ xtensa-*-* } { }


Committed with the above last minute change corrected.  I'd originally
used a deprecated tcl case statement, and didn't change enough.  It
now uses "switch -glob" and removes the braces around the patterns.
(A tcl case statement has a list pattern, switch just a text pattern.)

-- 
Alan Modra
Australia Development Lab, IBM

Patch

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 300be3f7437..99587d2ebd6 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -13123,7 +13123,7 @@  _bfd_elf_gc_mark_rsec (struct bfd_link_info *info, asection *sec,
 		       bfd_boolean *start_stop)
 {
   unsigned long r_symndx;
-  struct elf_link_hash_entry *h;
+  struct elf_link_hash_entry *h, *hw;
 
   r_symndx = cookie->rel->r_info >> cookie->r_sym_shift;
   if (r_symndx == STN_UNDEF)
@@ -13147,8 +13147,12 @@  _bfd_elf_gc_mark_rsec (struct bfd_link_info *info, asection *sec,
 	 keep the non-weak definition because many backends put
 	 dynamic reloc info on the non-weak definition for code
 	 handling copy relocs.  */
-      if (h->is_weakalias)
-	weakdef (h)->mark = 1;
+      hw = h;
+      while (hw->is_weakalias)
+	{
+	  hw = hw->u.alias;
+	  hw->mark = 1;
+	}
 
       if (start_stop != NULL)
 	{
diff --git a/ld/testsuite/ld-elf/pr25458.map b/ld/testsuite/ld-elf/pr25458.map
new file mode 100644
index 00000000000..5578d1fae72
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr25458.map
@@ -0,0 +1,4 @@ 
+FOO {
+global:
+  __environ; _environ; environ;
+};
diff --git a/ld/testsuite/ld-elf/pr25458.rd b/ld/testsuite/ld-elf/pr25458.rd
new file mode 100644
index 00000000000..6d78f69aee4
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr25458.rd
@@ -0,0 +1,10 @@ 
+#...
+Symbol table '\.dynsym' contains [0-9]+ entries:
+ +Num: +Value +Size Type +Bind +Vis +Ndx Name
+#...
+ +[0-9]+: [0-9a-f]+ +(4|8)+ OBJECT +WEAK +DEFAULT +[0-9]+ environ@FOO \(2\)
+#...
+ +[0-9]+: [0-9a-f]+ +(4|8)+ OBJECT +WEAK +DEFAULT +[0-9]+ _environ@FOO \(2\)
+#...
+ +[0-9]+: [0-9a-f]+ +(4|8)+ OBJECT +GLOBAL +DEFAULT +[0-9]+ __environ@FOO \(2\)
+#pass
diff --git a/ld/testsuite/ld-elf/pr25458a.s b/ld/testsuite/ld-elf/pr25458a.s
new file mode 100644
index 00000000000..59e6af2c192
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr25458a.s
@@ -0,0 +1,6 @@ 
+	.text
+	.globl	_start
+	.type	_start, %function
+_start:
+	.dc.a	environ
+	.size	_start, .-_start
diff --git a/ld/testsuite/ld-elf/pr25458b.s b/ld/testsuite/ld-elf/pr25458b.s
new file mode 100644
index 00000000000..ff64cc7584b
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr25458b.s
@@ -0,0 +1,11 @@ 
+	.data
+	.globl __environ
+	.type __environ,%object
+__environ:
+	.dc.a	0
+	.size	__environ, .-__environ
+	.weak _environ
+	.globl _environ
+	.set _environ, __environ
+	.weak environ
+	.set environ, __environ
diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp
index e03906a142d..5430f933677 100644
--- a/ld/testsuite/ld-elf/shared.exp
+++ b/ld/testsuite/ld-elf/shared.exp
@@ -337,6 +337,31 @@  if { [check_gc_sections_available] } {
 	    "pr22150" \
 	] \
     ]
+
+    # bfin and mips do not support copy relocation.
+    if { ![istarget bfin-*-*] && \
+	 ![istarget "mips*-*-*"] } {
+	run_ld_link_tests [list \
+	    [list \
+		"Build pr25458.so" \
+		"$LFLAGS -shared --version-script=pr25458.map" \
+		"" \
+		"$AFLAGS_PIC" \
+		{pr25458b.s} \
+		{} \
+		"pr25458.so" \
+	    ] \
+	    [list \
+		"Build pr25458" \
+		"$LFLAGS -e _start --gc-sections" \
+		"tmpdir/pr25458.so" \
+		"$AFLAGS_PIC" \
+		{pr25458a.s} \
+		{{readelf {--dyn-sym --wide} pr25458.rd}} \
+		"pr25458" \
+	    ] \
+	]
+    }
 }
 
 set ASFLAGS $old_ASFLAGS