[PATCH][PR ld/10144] MIPS/BFD: Don't make debug section relocs dynamic

["Maciej W. Rozycki" <macro at codesourcery dot com>]

Hi,

 There is a bug in BFD for MIPS ELF targets that affects symbol processing 
by the linker.  The prerequisite is a pair objects being linked together 
-- a shared object and a relocatable object -- for the purpose of creating 
an executable.  They have to multiply-define a symbol, which has to be a 
regular definition in the shared object and a common one in the 
relocatable object.  Additionally there has to be a relocation from an 
unallocatable (e.g. debug) section referring to the symbol in the 
relocatable object.

 If a symbol is multiply-defined like this, then the regular definition 
should override the common one and the latter should be converted to an 
undefined reference.  The relocation, if from a debug section, can be 
discarded and resolve to nil as the symbol debug records describe is no 
longer there; this is what other targets do and also what the MIPS target 
did before non-PIC support was added.  If not from a debug section, then 
other targets complain and fail, because a dynamic relocation cannot work 
from an unallocatable section.

 What the MIPS target does now is as follows:

1. If copy relocations are enabled, then the common definition is retained 
   in the relocatable object and the relocation is statically resolved 
   against it.  This invalid symbol definition may be discarded by the 
   dynamic linker, covering the bug; I have not checked it.

2. If copy relocations are disabled, then the static link fails, 
   complaining about the inability to convert the relocation to a dynamic 
   one.

 Here is a fix that works for me.  Following the practice from other 
targets I decided to explicitly check for the SEC_DEBUGGING flag (rather 
than SEC_ALLOC).  This makes our code work for the two cases mentioned 
above as proved by the test case below.

 The case of a non-debug non-alloc section is not covered; this is a rare 
corner case and I haven't looked into it (not that it shouldn't be fixed).

 Tested with the mips-linux-gnu target; generic test cases also tested 
with i686-linux-gnu (both with -m32 and with -m64) and powerpc-linux-gnu.  
The MIPS-specific test case extends coverage across all the three Linux 
ABIs and the two cases of copy relocations being enabled or disabled.  
The generic test case covers the default configuration only.

2010-09-15  Maciej W. Rozycki  <macro@codesourcery.com>

	PR ld/10144

	bfd/
	* elfxx-mips.c (_bfd_mips_elf_check_relocs)
	[R_MIPS_32, R_MIPS_REL32, R_MIPS_64]: Ignore relocs from
	SEC_DEBUGGING sections.

	ld/testsuite/
	* lib/ld-lib.exp (run_ld_link_tests): Handle sources from other
	directories.
	(run_ld_link_exec_tests): Likewise.
	(run_cc_link_tests): Likewise.
	* ld-elf/comm-data1.sd: New test.
	* ld-elf/comm-data1.s: Source for the new test.
	* ld-elf/comm-data2.sd: New test.
	* ld-elf/comm-data2.rd: Likewise.
	* ld-elf/comm-data2.xd: Likewise.
	* ld-elf/comm-data2.s: Source for the new tests.
	* ld-elf/comm-data.exp: Run the new tests.
	* ld-mips-elf/comm-data.exp: Likewise.

  Maciej

binutils-2.20.51-20100914-mips-debug-reloc-3.patch
Index: ld/testsuite/ld-elf/comm-data1.s
===================================================================
--- ld/testsuite/ld-elf/comm-data1.s	(revision 0)
+++ ld/testsuite/ld-elf/comm-data1.s	(revision 0)
@@ -0,0 +1,6 @@
+	.section .rodata,"a",%progbits
+	.balign	8
+	.globl	foo
+	.type	foo,%object
+foo:
+	.skip	4, 0
Index: ld/testsuite/ld-elf/comm-data2.xd
===================================================================
--- ld/testsuite/ld-elf/comm-data2.xd	(revision 0)
+++ ld/testsuite/ld-elf/comm-data2.xd	(revision 0)
@@ -0,0 +1,2 @@
+Hex dump of section '\.debug_foo':
+ +0x0+ +00000000 00000000 00000000 00000000 +\.\.\.\.\.\.\.\.\.\.\.\.\.\.\.\.
Index: ld/testsuite/ld-elf/comm-data2.s
===================================================================
--- ld/testsuite/ld-elf/comm-data2.s	(revision 0)
+++ ld/testsuite/ld-elf/comm-data2.s	(revision 0)
@@ -0,0 +1,14 @@
+	.text
+	.globl	_start
+	.globl	__start
+_start:
+__start:
+	.comm	foo, 4, 4
+	.section .debug_foo,"",%progbits
+	.balign	16
+	.ifdef	ELF64
+	.8byte	foo
+	.else
+	.4byte	foo
+	.endif
+	.balign	16
Index: ld/testsuite/ld-elf/comm-data.exp
===================================================================
--- ld/testsuite/ld-elf/comm-data.exp	(revision 0)
+++ ld/testsuite/ld-elf/comm-data.exp	(revision 0)
@@ -0,0 +1,76 @@
+# Expect script for common symbol override.
+#
+#   Copyright 2010  Free Software Foundation, Inc.
+#
+# This file is part of the GNU Binutils.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+#
+
+#
+# Written by Maciej W. Rozycki <macro@codesourcery.com>
+#
+
+# Exclude non-ELF targets.
+if ![is_elf_format] {
+    return
+}
+
+# Exclude non-Linux targets; feel free to include your favourite one
+# if you like.
+if ![istarget *-*-linux*] {
+    return
+}
+
+set testname "Common symbol override test"
+
+# Define a global symbol.
+run_ld_link_tests [list \
+    [list \
+	"$testname (auxiliary shared object build)" \
+	"-shared" \
+	"" \
+	{ comm-data1.s } \
+	{ \
+	    { readelf -s comm-data1.sd } \
+	} \
+	"libcomm-data.so" \
+    ] \
+]
+
+# Set the pointer size according to the ELF flavour.
+set AFLAGS ""
+if [is_elf64 "tmpdir/libcomm-data.so"] {
+    append AFLAGS " --defsym ELF64=1"
+}
+
+# Verify that a common symbol has been converted to an undefined
+# reference to the global symbol of the same name defined above
+# and that the debug reference has been dropped.
+run_ld_link_tests [list \
+    [list \
+	"$testname" \
+	"-Ltmpdir -lcomm-data" \
+	"$AFLAGS" \
+	{ comm-data2.s } \
+	{ \
+	    { readelf -s comm-data2.sd } \
+	    { readelf -r comm-data2.rd } \
+	    { readelf "-x .debug_foo" comm-data2.xd } \
+	} \
+	"comm-data" \
+    ] \
+]
Index: ld/testsuite/ld-elf/comm-data1.sd
===================================================================
--- ld/testsuite/ld-elf/comm-data1.sd	(revision 0)
+++ ld/testsuite/ld-elf/comm-data1.sd	(revision 0)
@@ -0,0 +1,10 @@
+Symbol table '\.dynsym' contains [0-9]+ entries:
+ +Num: +Value +Size +Type +Bind +Vis +Ndx +Name
+#...
+ +[0-9]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
+#...
+Symbol table '\.symtab' contains [0-9]+ entries:
+ +Num: +Value +Size +Type +Bind +Vis +Ndx +Name
+#...
+ +[0-9]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
+#pass
Index: ld/testsuite/ld-elf/comm-data2.rd
===================================================================
--- ld/testsuite/ld-elf/comm-data2.rd	(revision 0)
+++ ld/testsuite/ld-elf/comm-data2.rd	(revision 0)
@@ -0,0 +1 @@
+There are no relocations in this file\.
Index: ld/testsuite/ld-elf/comm-data2.sd
===================================================================
--- ld/testsuite/ld-elf/comm-data2.sd	(revision 0)
+++ ld/testsuite/ld-elf/comm-data2.sd	(revision 0)
@@ -0,0 +1,10 @@
+Symbol table '\.dynsym' contains [0-9]+ entries:
+ +Num: +Value +Size +Type +Bind +Vis +Ndx +Name
+#...
+ +[0-9]+: +0+ +0 +OBJECT +GLOBAL +DEFAULT +UND +foo
+#...
+Symbol table '\.symtab' contains [0-9]+ entries:
+ +Num: +Value +Size +Type +Bind +Vis +Ndx +Name
+#...
+ +[0-9]+: +0+ +0 +OBJECT +GLOBAL +DEFAULT +UND +foo
+#pass
Index: ld/testsuite/lib/ld-lib.exp
===================================================================
--- ld/testsuite/lib/ld-lib.exp	(revision 297468)
+++ ld/testsuite/lib/ld-lib.exp	(working copy)
@@ -1226,11 +1226,12 @@ proc run_ld_link_tests { ldtests } {
 
 	# Assemble each file in the test.
 	foreach src_file $src_files {
-	    set objfile "tmpdir/[file rootname $src_file].o"
+	    set fileroot "[file rootname [file tail $src_file]]"
+	    set objfile "tmpdir/$fileroot.o"
 	    lappend objfiles $objfile
 
 	    if { [file extension $src_file] == ".c" } {
-		set as_file "tmpdir/[file rootname $src_file].s"
+		set as_file "tmpdir/$fileroot.s"
 		if ![ld_compile "$CC -S $CFLAGS $cflags" $srcdir/$subdir/$src_file $as_file] {
 		    set is_unresolved 1
 		    break
@@ -1420,7 +1421,8 @@ proc run_ld_link_exec_tests { targets_to
 
 	# Assemble each file in the test.
 	foreach src_file $src_files {
-	    set objfile "tmpdir/[file rootname $src_file].o"
+	    set fileroot "[file rootname [file tail $src_file]]"
+	    set objfile "tmpdir/$fileroot.o"
 	    lappend objfiles $objfile
 
 	    # We ignore warnings since some compilers may generate
@@ -1539,7 +1541,8 @@ proc run_cc_link_tests { ldtests } {
 
 	# Compile each file in the test.
 	foreach src_file $src_files {
-	    set objfile "tmpdir/[file rootname $src_file].o"
+	    set fileroot "[file rootname [file tail $src_file]]"
+	    set objfile "tmpdir/$fileroot.o"
 	    lappend objfiles $objfile
 
 	    # We ignore warnings since some compilers may generate
Index: ld/testsuite/ld-mips-elf/comm-data.exp
===================================================================
--- ld/testsuite/ld-mips-elf/comm-data.exp	(revision 0)
+++ ld/testsuite/ld-mips-elf/comm-data.exp	(revision 0)
@@ -0,0 +1,88 @@
+# Expect script for common symbol override, MIPS variation.
+#
+#   Copyright 2010  Free Software Foundation, Inc.
+#
+# This file is part of the GNU Binutils.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+#
+
+#
+# Written by Maciej W. Rozycki <macro@codesourcery.com>
+#
+
+# Exclude non-ELF targets.
+if ![is_elf_format] {
+    return
+}
+
+# Exclude non-Linux targets; feel free to include your favourite one
+# if you like.
+if ![istarget mips*-*-linux*] {
+    return
+}
+
+proc mips_comm_data_test { abi flag emul reloc } {
+
+    set testname "MIPS $abi/$reloc common symbol override test"
+    set AFLAGS "$flag -EB"
+    set LDFLAGS "-m$emul"
+
+    # Define a global symbol.
+    run_ld_link_tests [list \
+	[list \
+	    "$testname (auxiliary shared object build)" \
+	    "$LDFLAGS -shared" \
+	    "$AFLAGS -call_shared" \
+	    { ../ld-elf/comm-data1.s } \
+	    { \
+		{ readelf -s ../ld-elf/comm-data1.sd } \
+	    } \
+	  "libmips-$abi-$reloc-comm-data.so" \
+	] \
+    ]
+
+    # Set the pointer size according to the ABI.
+    if { $abi == "n64" } {
+	append AFLAGS " --defsym ELF64=1"
+    }
+
+    # Verify that a common symbol has been converted to an undefined
+    # reference to the global symbol of the same name defined above
+    # and that the debug reference has been dropped.
+    run_ld_link_tests [list \
+	[list \
+	    "$testname" \
+	    "$LDFLAGS -z $reloc -Ltmpdir -lmips-$abi-$reloc-comm-data" \
+	    "$AFLAGS -call_nonpic" \
+	    { ../ld-elf/comm-data2.s } \
+	    { \
+		{ readelf -s ../ld-elf/comm-data2.sd } \
+		{ readelf -r ../ld-elf/comm-data2.rd } \
+		{ readelf "-x .debug_foo" ../ld-elf/comm-data2.xd } \
+	    } \
+	    "mips-$abi-$reloc-comm-data" \
+	] \
+    ]
+}
+
+set abis { o32 -32 elf32btsmip n32 -n32 elf32btsmipn32 n64 -64 elf64btsmip }
+set relocs { copyreloc nocopyreloc }
+foreach { abi flag emul } $abis {
+    foreach reloc $relocs {
+	mips_comm_data_test $abi $flag $emul $reloc
+    }
+}
Index: bfd/elfxx-mips.c
===================================================================
--- bfd/elfxx-mips.c	(revision 297469)
+++ bfd/elfxx-mips.c	(working copy)
@@ -7834,6 +7834,19 @@ _bfd_mips_elf_check_relocs (bfd *abfd, s
 		elf_hash_table (info)->dynobj = dynobj = abfd;
 	      break;
 	    }
+	  /* Ignore relocs from SEC_DEBUGGING sections because such
+	     sections are not SEC_ALLOC and thus ld.so will not process
+	     them.  Don't set has_static_relocs for the corresponding
+	     symbol.
+
+	     This is needed in cases such as a global symbol definition
+	     in a shared library causing a common symbol from an object
+	     file to be converted to an undefined reference.  If that
+	     happens, then all the relocations against this symbol from
+	     SEC_DEBUGGING sections in the object file will resolve to
+	     nil.  */
+	  if ((sec->flags & SEC_DEBUGGING) != 0)
+	    break;
 	  /* Fall through.  */
 
 	default: