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

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

[Maciej W. Rozycki]

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:

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

[Richard Sandiford]

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
>  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).

This is what worries me.  The first part of the comment, which I realise
you've taken from elsewhere:

> +	  /* 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.  */

makes it sound like we're doing something that would be acceptable for
all !SEC_ALLOC sections, which then raises the question why we're only
testing SEC_DEBUGGING.  If I've understood correctly, we're really
saying something like "We know what debugging sections are for.  It's
OK to silently set external addresses to 0: at worst it will impair
the debugging information."  I.e. it's _not_ something we can safely
apply to all SEC_ALLOC sections.

There also seems to be the implicit judgement call: "If the only
reference to a symbol is via debugging information, it's not worth
creating a copy reloc for it."  That has nothing to do with ld.so;
it's a decision we can make entirely in the static linker.  And it's
a perfectly sensible call of course, and maybe the thinking was that
it was so obvious it didn't need to be stated, but still, the comment
seems to gloss over the rationale.

(E.g. with the simple case you're describing:

--------------------------------------------------------------
cat <<EOF >foo.c
int x = 1;
EOF
cat <<EOF >bar.c
int x;
int main (void)
{
  return 0;
}
EOF
cat <<EOF >commands
break main
run
print x
EOF
gcc foo.c -o foo.so -shared -g
gcc bar.c foo.so -o bar -g
LD_LIBRARY_PATH=. gdb --batch --command=commands bar
--------------------------------------------------------------

I get:

--------------------------------------------------------------
Breakpoint 1 at 0x4005c8: file bar.c, line 4.

Breakpoint 1, main () at bar.c:4
4         return 0;
/tmp/commands:3: Error in sourced command file:
Cannot access memory at address 0x0
--------------------------------------------------------------

on my x86_64 box.  With a copy reloc I'd be able to see "1" instead.)

All of which is fine with me.  And even if it wasn't, I wouldn't
suggest MIPS should be different from other targets.  It's just that
the comments we're using to justify the code seem a little lacking.

So with all that, the patch is OK, thanks.  However, I think you
could change:

	.ifdef	ELF64
	.8byte	foo
	.else
	.4byte	foo
	.endif

to:

	dc.a	foo

and avoid the ELF64 symbol.  If that fails for any target, I think we
could legitimately say it's a bug in the target and Not Your Fault.
The test certainly looks nicely generic.

Also, in the MIPS test:

+# 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
+}

I think we can safely dispense with the first check.  I realise you're
doing it for consistency with the target-independent version,
but I wouldn't expect to have to test both of these elsewhere
in ld-mips-elf.

Richard

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

[Maciej W. Rozycki]

Hi Richard,

 Back to this change as I think it would be best fixed before 2.21 is out.

> >  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).
> 
> This is what worries me.  The first part of the comment, which I realise
> you've taken from elsewhere:
> 
> > +	  /* 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.  */
> 
> makes it sound like we're doing something that would be acceptable for
> all !SEC_ALLOC sections, which then raises the question why we're only
> testing SEC_DEBUGGING.  If I've understood correctly, we're really
> saying something like "We know what debugging sections are for.  It's
> OK to silently set external addresses to 0: at worst it will impair
> the debugging information."  I.e. it's _not_ something we can safely
> apply to all SEC_ALLOC sections.

 Yes, that's what I inferred applies elsewhere.  The SEC_ALLOC attribute 
is referred to, because it's the lack of it that is causing problems, but 
if the SEC_DEBUGGING attribute is set at the same time, then we can avoid 
them at a negligible cost.

> There also seems to be the implicit judgement call: "If the only
> reference to a symbol is via debugging information, it's not worth
> creating a copy reloc for it."  That has nothing to do with ld.so;
> it's a decision we can make entirely in the static linker.  And it's
> a perfectly sensible call of course, and maybe the thinking was that
> it was so obvious it didn't need to be stated, but still, the comment
> seems to gloss over the rationale.

 IMHO creating a copy reloc for a read-only data object is the wrong thing 
to do too in the first place, as far as I understand copy reloc's 
semantics -- the original segment's write protection attribute will not be 
carried over to the copy of the symbol (and propagated to the MMU via 
mprotect(2) as applicable) as this is not the intended use of the copy 
reloc, right?

> (E.g. with the simple case you're describing:
> 
> --------------------------------------------------------------
> cat <<EOF >foo.c
> int x = 1;
> EOF
> cat <<EOF >bar.c
> int x;
> int main (void)
> {
>   return 0;
> }
> EOF
> cat <<EOF >commands
> break main
> run
> print x
> EOF
> gcc foo.c -o foo.so -shared -g
> gcc bar.c foo.so -o bar -g
> LD_LIBRARY_PATH=. gdb --batch --command=commands bar
> --------------------------------------------------------------
> 
> I get:
> 
> --------------------------------------------------------------
> Breakpoint 1 at 0x4005c8: file bar.c, line 4.
> 
> Breakpoint 1, main () at bar.c:4
> 4         return 0;
> /tmp/commands:3: Error in sourced command file:
> Cannot access memory at address 0x0
> --------------------------------------------------------------
> 
> on my x86_64 box.  With a copy reloc I'd be able to see "1" instead.)
> 
> All of which is fine with me.  And even if it wasn't, I wouldn't
> suggest MIPS should be different from other targets.  It's just that
> the comments we're using to justify the code seem a little lacking.

 Frankly what I would be comfortable with the most is an arrangement where 
we would simply discard the corresponding DWARF-2 records altogether.  My 
understanding is LD is not immediately capable of doing such section 
surgery.

> So with all that, the patch is OK, thanks.  However, I think you
> could change:
> 
> 	.ifdef	ELF64
> 	.8byte	foo
> 	.else
> 	.4byte	foo
> 	.endif
> 
> to:
> 
> 	dc.a	foo
> 
> and avoid the ELF64 symbol.  If that fails for any target, I think we
> could legitimately say it's a bug in the target and Not Your Fault.

 Sneaky!  I didn't know about this pseudo-op; I wouldn't mind if such bits 
were actually documented, sigh...  Is this meant to emit a data entity of 
the address size?  If so, then it's broken for MIPS/n32 -- it produces a 
64-bit relocatable field.  Given this obvious failure, how confident do 
you feel this directive is reliable across other targets?  It looks like a 
relatively recent addition to me.

 The MIPS/n32 failure may be trivial to fix, but I may not have time 
immediately available to look into it.  I'll see what I can do though.

> Also, in the MIPS test:
> 
> +# 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
> +}
> 
> I think we can safely dispense with the first check.  I realise you're
> doing it for consistency with the target-independent version,
> but I wouldn't expect to have to test both of these elsewhere
> in ld-mips-elf.

 My understanding is the base framework just runs */*.exp from within 
ld/testsuite/ and it is the responsibility of the scripts themselves to 
filter the targets of concern.  Certainly the two preexisting scripts in 
ld-mips-elf/ do check for ![is_elf_format] explicitly.

 I have updated the comment as well taking your concerns into account.  
Please let me know if the new comment -- a bit lengthy, but there you go 
-- sounds reasonable to you.  I have regression-tested the new change for 
the mips-sde-elf target with no problems.

2010-11-04  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-mips-debug-reloc.patch
Index: binutils-fsf-trunk-quilt/bfd/elfxx-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/bfd/elfxx-mips.c	2010-11-04 14:26:19.000000000 +0000
+++ binutils-fsf-trunk-quilt/bfd/elfxx-mips.c	2010-11-04 14:58:41.000000000 +0000
@@ -7583,6 +7583,25 @@ _bfd_mips_elf_check_relocs (bfd *abfd, s
 		elf_hash_table (info)->dynobj = dynobj = abfd;
 	      break;
 	    }
+	  /* For sections that are not SEC_ALLOC a copy reloc would be
+	     output if possible (implying questionable semantics for
+	     read-only data objects) or otherwise the final link would
+	     fail as ld.so will not process them and could not therefore
+	     handle any outstanding dynamic relocations.
+
+	     For such sections that are also SEC_DEBUGGING, we can avoid
+	     these problems by simply ignoring any relocs as these
+	     sections have a predefined use and we know it is safe to do
+	     so.
+
+	     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:
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data.exp	2010-11-04 14:29:17.000000000 +0000
@@ -0,0 +1,70 @@
+# 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" \
+    ] \
+]
+
+# 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" \
+	"" \
+	{ comm-data2.s } \
+	{ \
+	    { readelf -s comm-data2.sd } \
+	    { readelf -r comm-data2.rd } \
+	    { readelf "-x .debug_foo" comm-data2.xd } \
+	} \
+	"comm-data" \
+    ] \
+]
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data1.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data1.s	2010-11-04 14:26:44.000000000 +0000
@@ -0,0 +1,6 @@
+	.section .rodata,"a",%progbits
+	.balign	8
+	.globl	foo
+	.type	foo,%object
+foo:
+	.skip	4, 0
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data1.sd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data1.sd	2010-11-04 14:26:44.000000000 +0000
@@ -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: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.rd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.rd	2010-11-04 14:26:44.000000000 +0000
@@ -0,0 +1 @@
+There are no relocations in this file\.
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.s	2010-11-04 14:29:59.000000000 +0000
@@ -0,0 +1,10 @@
+	.text
+	.globl	_start
+	.globl	__start
+_start:
+__start:
+	.comm	foo, 4, 4
+	.section .debug_foo,"",%progbits
+	.balign	16
+	.dc.a	foo
+	.balign	16
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.sd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.sd	2010-11-04 14:26:44.000000000 +0000
@@ -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: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.xd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.xd	2010-11-04 14:26:44.000000000 +0000
@@ -0,0 +1,2 @@
+Hex dump of section '\.debug_foo':
+ +0x0+ +00000000 00000000 00000000 00000000 +\.\.\.\.\.\.\.\.\.\.\.\.\.\.\.\.
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/comm-data.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/comm-data.exp	2010-11-04 14:30:59.000000000 +0000
@@ -0,0 +1,83 @@
+# 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" \
+	] \
+    ]
+
+    # 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: binutils-fsf-trunk-quilt/ld/testsuite/lib/ld-lib.exp
===================================================================
--- binutils-fsf-trunk-quilt.orig/ld/testsuite/lib/ld-lib.exp	2010-11-04 14:26:19.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/lib/ld-lib.exp	2010-11-04 14:26:44.000000000 +0000
@@ -1266,11 +1266,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
@@ -1476,7 +1477,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
@@ -1595,7 +1597,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

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

[Richard Sandiford]

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
>> So with all that, the patch is OK, thanks.  However, I think you
>> could change:
>> 
>> 	.ifdef	ELF64
>> 	.8byte	foo
>> 	.else
>> 	.4byte	foo
>> 	.endif
>> 
>> to:
>> 
>> 	dc.a	foo
>> 
>> and avoid the ELF64 symbol.  If that fails for any target, I think we
>> could legitimately say it's a bug in the target and Not Your Fault.
>
>  Sneaky!  I didn't know about this pseudo-op; I wouldn't mind if such bits 
> were actually documented, sigh...  Is this meant to emit a data entity of 
> the address size?  If so, then it's broken for MIPS/n32 -- it produces a 
> 64-bit relocatable field.  Given this obvious failure, how confident do 
> you feel this directive is reliable across other targets?  It looks like a 
> relatively recent addition to me.

OK, will try to fix that this weekend.  I still think dc.a is the
right thing for the test, but see below.

>> Also, in the MIPS test:
>> 
>> +# 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
>> +}
>> 
>> I think we can safely dispense with the first check.  I realise you're
>> doing it for consistency with the target-independent version,
>> but I wouldn't expect to have to test both of these elsewhere
>> in ld-mips-elf.
>
>  My understanding is the base framework just runs */*.exp from within 
> ld/testsuite/ and it is the responsibility of the scripts themselves to 
> filter the targets of concern.

Right.  My point was that [istarget mips*-*-linux*] => [is_elf_format];
we don't support any non-ELF form of GNU/Linux on MIPS.  So I wouldn't
expect to have to check is_elf_format if we're checking the second
condition as well.  I'd still like to see that condition removed.

Oh, yeah, much as it pains me to say it: s/favourite/favorite/.

Anyway, the new comment looks good, thanks.  Please install the
elfxx-mips.c change now.  We can apply the testsuite patch once
the dc.a thing is sorted out.

Richard

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

[Maciej W. Rozycki]

On Thu, 4 Nov 2010, Richard Sandiford wrote:

> >  My understanding is the base framework just runs */*.exp from within 
> > ld/testsuite/ and it is the responsibility of the scripts themselves to 
> > filter the targets of concern.
> 
> Right.  My point was that [istarget mips*-*-linux*] => [is_elf_format];
> we don't support any non-ELF form of GNU/Linux on MIPS.  So I wouldn't
> expect to have to check is_elf_format if we're checking the second
> condition as well.  I'd still like to see that condition removed.

 Good point.  This can be added if a need arises from any other targets 
added.

> Oh, yeah, much as it pains me to say it: s/favourite/favorite/.

 Indeed -- I recall seeing a language convention reference posted to the 
list yesterday.  As a non-British it may be less of a pain to me, but 
still I try to stay consistent with the language I use (I wonder how the 
rule is going to play with the style of expression rather than just 
spelling, especially when American spelling is mixed with British style).  
Oh well...  There may be more of such subtleties in some of my outstanding 
patches -- I'll try to catch them as I go.

> Anyway, the new comment looks good, thanks.  Please install the
> elfxx-mips.c change now.  We can apply the testsuite patch once
> the dc.a thing is sorted out.

 I am assuming you're trying to minimise the risk of a cross-target 
avalanche of failures from the generic test case with the imminent release 
in mind; certainly the lone MIPS/n32 misbehaviour (which doesn't trigger a 
failure here) cannot justify such an extreme caution.

 Thanks for your review; I'm proceeding with the elfxx-mips.c change 
straight away.

  Maciej

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

[Richard Sandiford]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> "Maciej W. Rozycki" <macro@codesourcery.com> writes:
>>> So with all that, the patch is OK, thanks.  However, I think you
>>> could change:
>>> 
>>> 	.ifdef	ELF64
>>> 	.8byte	foo
>>> 	.else
>>> 	.4byte	foo
>>> 	.endif
>>> 
>>> to:
>>> 
>>> 	dc.a	foo
>>> 
>>> and avoid the ELF64 symbol.  If that fails for any target, I think we
>>> could legitimately say it's a bug in the target and Not Your Fault.
>>
>>  Sneaky!  I didn't know about this pseudo-op; I wouldn't mind if such bits 
>> were actually documented, sigh...  Is this meant to emit a data entity of 
>> the address size?  If so, then it's broken for MIPS/n32 -- it produces a 
>> 64-bit relocatable field.  Given this obvious failure, how confident do 
>> you feel this directive is reliable across other targets?  It looks like a 
>> relatively recent addition to me.
>
> OK, will try to fix that this weekend.  I still think dc.a is the
> right thing for the test, but see below.

So, I looked at it this weekend, and I longer think dc.a is the right
thing for the test.  It seemed so well suited, but as you point out,
it's based on the ISA address size rather than the ABI or file
format address size, which probably makes it of limited use.  Oh well.

Please go ahead with your original is_elf64 patch, and sorry for the noise.

Richard

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

[Maciej W. Rozycki]

On Wed, 10 Nov 2010, Richard Sandiford wrote:

> > OK, will try to fix that this weekend.  I still think dc.a is the
> > right thing for the test, but see below.
> 
> So, I looked at it this weekend, and I longer think dc.a is the right
> thing for the test.  It seemed so well suited, but as you point out,
> it's based on the ISA address size rather than the ABI or file
> format address size, which probably makes it of limited use.  Oh well.

 I agreed with you .dc.a is the right approach and have only been 
concerned about the correctness of the implementation for MIPS targets or 
elsewhere.

 Given your consideration I have just now looked into it as well and it 
looks to me our MIPS target should simply override TC_ADDRESS_BYTES with 
its own function that will return 8 for the n64 ABI and the EABI and 4 
with all the other ABIs (I reckon o64 assumes 32-bit addresses too, 
correct?), or the currently selected ISA-implied address size if no ABI 
has been selected, such as with the ECOFF targets -- we have that logic 
implemented elsewhere already, so the return value of function should 
reduce to an expression along the lines of (4 << HAVE_64BIT_ADDRESSES).  
The existence of the macro itself suggests this is something that was 
intended for some platforms to do with the original implementation even 
though no target makes such an override at the moment.

 Or do you have a documentation reference for .dc.a that claims it should 
behave otherwise?  Where have these .dc.* pseudo-ops come from anyway?  
We seem to have collected plenty of various directives providing 
overlapping functionality with confusingly subtle semantics differences, 
sigh...

 And while at it I have noticed the #ifdef TC_ADDRESS_BYTES clauses 
throughout gas/read.c are rather pointless as the file defines the macro 
if not already present.  Was there a specific reason to have them?  If 
not, then I'll make a patch to remove them.  There's also related dead 
code in gas/config/tc-cr16.c -- not sure what to do about it.

> Please go ahead with your original is_elf64 patch, and sorry for the noise.

 I'd rather fixed .dc.a, sorry.  As this is not something critical for 
2.21, let me defer it until I'm done with the next (I dare not say "last") 
iteration of the microMIPS change.

  Maciej

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

[Matthias Klose]

On 10.11.2010 18:57, Maciej W. Rozycki wrote:
> On Wed, 10 Nov 2010, Richard Sandiford wrote:
>   I'd rather fixed .dc.a, sorry.  As this is not something critical for
> 2.21, let me defer it until I'm done with the next (I dare not say "last")
> iteration of the microMIPS change.

hmm, according to the debian mips maintainers this patch was needed to fix some 
build failures (in 2.20). I'd like to see this in 2.21, if possible.

   Matthias

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

[Maciej W. Rozycki]

On Thu, 11 Nov 2010, Matthias Klose wrote:

> >   I'd rather fixed .dc.a, sorry.  As this is not something critical for
> > 2.21, let me defer it until I'm done with the next (I dare not say "last")
> > iteration of the microMIPS change.
> 
> hmm, according to the debian mips maintainers this patch was needed to fix
> some build failures (in 2.20). I'd like to see this in 2.21, if possible.

 The bug fix itself went in as recorded in the PR; we're discussing issues 
related to the complementing testsuite additions.  These are of course 
good to have (additions, not issues, that is ;) ), but their absence does 
not impact the fix itself.  And now that the 2.21 branch has been created 
switching its contents into the maintenance mode, no future changes 
applied should cause the fix to regress, so the lack of a test case on the 
branch is probably negligible.

 As the new generic test case included with the changes discussed may 
possibly trigger failures for other targets, I think it'll be best applied 
to trunk only and any fallout sorted out there, with any fixes possibly 
backported to 2.21 if deemed appropriate.

 I hope this clears the situation a bit -- let me know otherwise.

  Maciej

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

[Richard Sandiford]

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> On Wed, 10 Nov 2010, Richard Sandiford wrote:
>
>> > OK, will try to fix that this weekend.  I still think dc.a is the
>> > right thing for the test, but see below.
>> 
>> So, I looked at it this weekend, and I longer think dc.a is the right
>> thing for the test.  It seemed so well suited, but as you point out,
>> it's based on the ISA address size rather than the ABI or file
>> format address size, which probably makes it of limited use.  Oh well.
>
>  I agreed with you .dc.a is the right approach and have only been 
> concerned about the correctness of the implementation for MIPS targets or 
> elsewhere.
>
>  Given your consideration I have just now looked into it as well and it 
> looks to me our MIPS target should simply override TC_ADDRESS_BYTES with 
> its own function that will return 8 for the n64 ABI and the EABI and 4 
> with all the other ABIs (I reckon o64 assumes 32-bit addresses too, 
> correct?), or the currently selected ISA-implied address size if no ABI 
> has been selected, such as with the ECOFF targets -- we have that logic 
> implemented elsewhere already, so the return value of function should 
> reduce to an expression along the lines of (4 << HAVE_64BIT_ADDRESSES).  
> The existence of the macro itself suggests this is something that was 
> intended for some platforms to do with the original implementation even 
> though no target makes such an override at the moment.
>
>  Or do you have a documentation reference for .dc.a that claims it should 
> behave otherwise?

Well, the point is: this kind of situation is by no means exclusive
to MIPS.  Plenty of other targets allow 64-bit architecturees to use a
32-bit ABI and a 32-bit file format.  If .dc.a was supposed to select
the ABI address size, it could/should/would have used other hooks to
get that size.  E.g. the code in bfd.c:is32bit would probably have
been a better starting point.

As it stands, .dc.a means the architecture address size for all platforms.
We shouldn't change the meaning on MIPS alone and introduce an inconsistency.
And IMO we shouldn't change the meaning at all, given that the directive
has already been released into the wild.  We could of course add a new
directive, but I personally don't think it's worth it.  You'd have to
convince another maintainer if you wanted to do that. :-)

Please just go ahead with your original is_elf64 test.

Richard

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

[Maciej W. Rozycki]

On Thu, 11 Nov 2010, Richard Sandiford wrote:

> >  I agreed with you .dc.a is the right approach and have only been 
> > concerned about the correctness of the implementation for MIPS targets or 
> > elsewhere.
> >
> >  Given your consideration I have just now looked into it as well and it 
> > looks to me our MIPS target should simply override TC_ADDRESS_BYTES with 
> > its own function that will return 8 for the n64 ABI and the EABI and 4 
> > with all the other ABIs (I reckon o64 assumes 32-bit addresses too, 
> > correct?), or the currently selected ISA-implied address size if no ABI 
> > has been selected, such as with the ECOFF targets -- we have that logic 
> > implemented elsewhere already, so the return value of function should 
> > reduce to an expression along the lines of (4 << HAVE_64BIT_ADDRESSES).  
> > The existence of the macro itself suggests this is something that was 
> > intended for some platforms to do with the original implementation even 
> > though no target makes such an override at the moment.
> >
> >  Or do you have a documentation reference for .dc.a that claims it should 
> > behave otherwise?
> 
> Well, the point is: this kind of situation is by no means exclusive
> to MIPS.  Plenty of other targets allow 64-bit architecturees to use a
> 32-bit ABI and a 32-bit file format.  If .dc.a was supposed to select
> the ABI address size, it could/should/would have used other hooks to
> get that size.  E.g. the code in bfd.c:is32bit would probably have
> been a better starting point.

 Maybe, maybe not.  This piece doesn't seem to be used particularly 
extensively and we have many 64-bit targets that support a 32-bit ABI, so 
internally they must have come up with their own ways of determining the 
size of the address, just as we did with the MIPS target.

> As it stands, .dc.a means the architecture address size for all platforms.
> We shouldn't change the meaning on MIPS alone and introduce an inconsistency.
> And IMO we shouldn't change the meaning at all, given that the directive
> has already been released into the wild.  We could of course add a new
> directive, but I personally don't think it's worth it.  You'd have to
> convince another maintainer if you wanted to do that. :-)

 OK, cc-ed Alan Modra, the original offender. ;)  Perhaps he'll be able to 
provide input about the directive and its origins.

 I stand by what I stated.  The pseudo-op has been out, but it's broken 
and useless for an ILP32 ABI running on a 64-bit target.  There's no 
notion of a 64-bit address for such targets, and they need not even 
provide a 64-bit reloc that the pseudo-op requires.  We happen to have 
such a reloc in our n32 backend, but frankly I have no idea what it was 
added for -- perhaps anticipating this directive. ;)

 Anyone relying on the current implementation of .dc.a on such a target 
should have filed a bug in the first place and I don't think we are 
obligated to provide bug compatibility for such obvious problems.  I 
suspect the knowledge of this pseudo-op is limited and hardly anybody uses 
it, mainly because it's not documented in the GAS manual (I doubt anyone 
seriously considers studying sources for feature discovery), so you're 
almost certainly worrying about a scenario the probability of which is 
epsilon.

 Actually the problem with n32 (and the unexpected R_MIPS_64 reloc) is no 
psABI has ever been written up; at least I have never heard of such a 
document.  The closest I know of are SGI's: "MIPSpro N32 ABI Handbook" and 
"MIPSpro Assembly Language Programmer's Guide," but they are more 
programmer-oriented manuals than proper ELF-level specifications.  In the 
end my understanding is our implementation has been crafted based on a 
combination of the two said manuals, n64's "64-bit ELF Object File 
Specification" and o32's "MIPS RISC Processor Supplement" to the SVR4 ABI, 
as well as observation of what IRIX tools do.

> Please just go ahead with your original is_elf64 test.

 Hmm, I guess we can fix it up if we ever come up with a constructive 
conclusion wrt the .dc.a mess.

  Maciej

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

[Alan Modra]

On Fri, Nov 12, 2010 at 05:39:04PM +0000, Maciej W. Rozycki wrote:
>  OK, cc-ed Alan Modra, the original offender. ;)  Perhaps he'll be able to 
> provide input about the directive and its origins.

.dc.a was invented for use in our testsuite.  No other use is
supported, which is why it hasn't been documented.  I won't stand in
the way of anyone who wants to change mips .dc.a behaviour,
particularly if doing so makes it easier to write generic tests for
mips.

-- 
Alan Modra
Australia Development Lab, IBM

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

[Maciej W. Rozycki]

On Mon, 15 Nov 2010, Alan Modra wrote:

> >  OK, cc-ed Alan Modra, the original offender. ;)  Perhaps he'll be able to 
> > provide input about the directive and its origins.
> 
> .dc.a was invented for use in our testsuite.  No other use is
> supported, which is why it hasn't been documented.  I won't stand in
> the way of anyone who wants to change mips .dc.a behaviour,
> particularly if doing so makes it easier to write generic tests for
> mips.

 Alan, thanks for the clarification.

 Richard, I believe the above explanation clears your concerns.  Here's a 
change to make .dc.a emit a 32-bit or 64-bit entity as expected:

$ cat comm-data2.s
	.text
	.globl	_start
	.globl	__start
_start:
__start:
	.comm	foo, 4, 4
	.section .debug_foo,"",%progbits
	.balign	16
	.dc.a	foo
	.balign	16
$ mips-sde-elf-objdump -r comm-data2-32.o

comm-data2-32.o:     file format elf32-tradbigmips

RELOCATION RECORDS FOR [.debug_foo]:
OFFSET   TYPE              VALUE 
00000000 R_MIPS_32         foo


$ mips-sde-elf-objdump -r comm-data2-n32.o

comm-data2-n32.o:     file format elf32-ntradbigmips

RELOCATION RECORDS FOR [.debug_foo]:
OFFSET   TYPE              VALUE 
00000000 R_MIPS_32         foo


$ mips-sde-elf-objdump -r comm-data2-64.o

comm-data2-64.o:     file format elf64-tradbigmips

RELOCATION RECORDS FOR [.debug_foo]:
OFFSET           TYPE              VALUE 
0000000000000000 R_MIPS_64         foo
0000000000000000 R_MIPS_NONE       *ABS*
0000000000000000 R_MIPS_NONE       *ABS*


$

 Regression tested with mips-sde-elf and mips-gnu-linux.

2010-12-07  Maciej W. Rozycki  <macro@codesourcery.com>

	gas/
	* config/tc-mips.h (TC_ADDRESS_BYTES): New macro.
	(mips_address_bytes): New prototype.
	* config/tc-mips.c (mips_address_bytes): New function.

 OK to apply?

  Maciej

binutils-gas-mips-address-bits.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2010-12-07 18:03:09.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2010-12-07 18:21:46.000000000 +0000
@@ -1235,6 +1235,15 @@ static const pseudo_typeS mips_nonecoff_
   { NULL, NULL, 0 },
 };
 
+/* Export the ABI address size for use by TC_ADDRESS_BYTES for the
+   purpose of the `.dc.a' internal pseudo-op.  */
+
+int
+mips_address_bytes (void)
+{
+  return HAVE_64BIT_ADDRESSES ? 8 : 4;
+}
+
 extern void pop_insert (const pseudo_typeS *);
 
 void
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.h
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.h	2010-12-07 18:03:09.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.h	2010-12-07 18:13:47.000000000 +0000
@@ -42,6 +42,9 @@ struct expressionS;
 #define MAX_RELOC_EXPANSION 3
 #define LOCAL_LABELS_FB 1
 
+#define TC_ADDRESS_BYTES mips_address_bytes
+extern int mips_address_bytes (void);
+
 /* Maximum symbol offset that can be encoded in a BFD_RELOC_GPREL16
    relocation.  */
 #define MAX_GPREL_OFFSET (0x7FF0)

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

[Richard Sandiford]

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> 2010-12-07  Maciej W. Rozycki  <macro@codesourcery.com>
>
> 	gas/
> 	* config/tc-mips.h (TC_ADDRESS_BYTES): New macro.
> 	(mips_address_bytes): New prototype.
> 	* config/tc-mips.c (mips_address_bytes): New function.

OK

Richard

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

[Maciej W. Rozycki]

On Thu, 9 Dec 2010, Richard Sandiford wrote:

> > 2010-12-07  Maciej W. Rozycki  <macro@codesourcery.com>
> >
> > 	gas/
> > 	* config/tc-mips.h (TC_ADDRESS_BYTES): New macro.
> > 	(mips_address_bytes): New prototype.
> > 	* config/tc-mips.c (mips_address_bytes): New function.
> 
> OK

 Thanks, I'll be applying the following change on top of that change then, 
unless you have any further concerns.

2010-12-10  Maciej W. Rozycki  <macro@codesourcery.com>

	PR ld/10144

	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-20101210-mips-debug-reloc.patch
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data.exp	2010-12-09 02:34:50.000000000 +0000
@@ -0,0 +1,70 @@
+# 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 favorite 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" \
+    ] \
+]
+
+# 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" \
+	"" \
+	{ comm-data2.s } \
+	{ \
+	    { readelf -s comm-data2.sd } \
+	    { readelf -r comm-data2.rd } \
+	    { readelf "-x .debug_foo" comm-data2.xd } \
+	} \
+	"comm-data" \
+    ] \
+]
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data1.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data1.s	2010-12-09 02:34:50.000000000 +0000
@@ -0,0 +1,6 @@
+	.section .rodata,"a",%progbits
+	.balign	8
+	.globl	foo
+	.type	foo,%object
+foo:
+	.skip	4, 0
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data1.sd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data1.sd	2010-12-09 02:34:50.000000000 +0000
@@ -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: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.rd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.rd	2010-12-09 02:34:50.000000000 +0000
@@ -0,0 +1 @@
+There are no relocations in this file\.
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.s	2010-12-09 02:34:50.000000000 +0000
@@ -0,0 +1,10 @@
+	.text
+	.globl	_start
+	.globl	__start
+_start:
+__start:
+	.comm	foo, 4, 4
+	.section .debug_foo,"",%progbits
+	.balign	16
+	.dc.a	foo
+	.balign	16
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.sd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.sd	2010-12-09 02:34:50.000000000 +0000
@@ -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: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.xd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.xd	2010-12-09 02:34:50.000000000 +0000
@@ -0,0 +1,2 @@
+Hex dump of section '\.debug_foo':
+ +0x0+ +00000000 00000000 00000000 00000000 +\.\.\.\.\.\.\.\.\.\.\.\.\.\.\.\.
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/comm-data.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/comm-data.exp	2010-12-09 02:34:50.000000000 +0000
@@ -0,0 +1,78 @@
+# 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-Linux targets; feel free to include your favorite 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" \
+	] \
+    ]
+
+    # 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: binutils-fsf-trunk-quilt/ld/testsuite/lib/ld-lib.exp
===================================================================
--- binutils-fsf-trunk-quilt.orig/ld/testsuite/lib/ld-lib.exp	2010-12-09 02:34:47.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/lib/ld-lib.exp	2010-12-09 02:34:50.000000000 +0000
@@ -961,11 +961,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
@@ -1171,7 +1172,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
@@ -1290,7 +1292,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

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

[Richard Sandiford]

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> On Thu, 9 Dec 2010, Richard Sandiford wrote:
>
>> > 2010-12-07  Maciej W. Rozycki  <macro@codesourcery.com>
>> >
>> > 	gas/
>> > 	* config/tc-mips.h (TC_ADDRESS_BYTES): New macro.
>> > 	(mips_address_bytes): New prototype.
>> > 	* config/tc-mips.c (mips_address_bytes): New function.
>> 
>> OK
>
>  Thanks, I'll be applying the following change on top of that change then, 
> unless you have any further concerns.

I think this is likely to break other targets that have 32-bit ABIs
and 64-bit architectures.  A general fix along the lines of the
bfd.c:is32bit thing that I mentioned upthread would be needed first.

(That fix would also have worked for your three examples, without
the need for a MIPS-specific patch.  The reason I didn't insist
is that a MIPS-specific change is needed for EABI64 (32-bit ELF,
64-bit addresses, ick.).)

Richard

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

[Maciej W. Rozycki]

On Sat, 11 Dec 2010, Richard Sandiford wrote:

> >  Thanks, I'll be applying the following change on top of that change then, 
> > unless you have any further concerns.
> 
> I think this is likely to break other targets that have 32-bit ABIs
> and 64-bit architectures.  A general fix along the lines of the
> bfd.c:is32bit thing that I mentioned upthread would be needed first.

 TBH I don't think putting a lot of infrastructure into an internal 
feature meant for testing only is worth the effort unless proved 
otherwise.  I propose to include my testcases as is now and then the 
respective platform maintainers to implement TC_ADDRESS_BYTES as a need 
arises, i.e. someone notices breakage or they feel like looking for work.  
We've got around a year until the next release now, so there's plenty of 
time to catch any problems and I find the idea of rejecting a test case 
because "it may reveal breakage somewhere" backwards, sorry.  This is 
exactly what test cases are for.

 If enough evidence is found this is worth abstracting, then the 
bfd.c:is32bit idea you propose may be implemented (though I'd be more 
comfortable with a function returning a quantitative result -- there are 
undoubtedly addressing submode variations other to 64/32, e.g. 32/16, 
24/16, etc.).  The TC_ADDRESS_BYTES macro is distinct enough for all the 
places to be easily identified at that stage.

 As I believe these test cases are good enough in the current form, I will 
not make any further development in this area.  I'll be happy to commit my 
proposal as posted, but otherwise I'll dismiss it -- feel free to pick it 
up yourself, share with somebody or discard altogether.

> (That fix would also have worked for your three examples, without
> the need for a MIPS-specific patch.  The reason I didn't insist
> is that a MIPS-specific change is needed for EABI64 (32-bit ELF,
> 64-bit addresses, ick.).)

 Well, I believe TC_ADDRESS_BYTES is the least of concern with getting the 
address size right.

 While at it: M R Swami, as the CR16 maintainer would you please have a 
look at CR16 implementation of l_cons() in gas/config/tc-cr16.c?  This 
function refers to TC_ADDRESS_BYTES that is nowhere defined in that 
configuration and the .dc.a pseudo-op is therefore likely broken for that 
target.  I suspect the function has been copied and pasted from gas/read.c 
without the bit referring to TC_ADDRESS_BYTES updated accordingly.  Thank 
you.

  Maciej

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

[Maciej W. Rozycki]

Hi Richard,

On Mon, 13 Dec 2010, Maciej W. Rozycki wrote:

> On Sat, 11 Dec 2010, Richard Sandiford wrote:
> 
> > >  Thanks, I'll be applying the following change on top of that change then, 
> > > unless you have any further concerns.
> > 
> > I think this is likely to break other targets that have 32-bit ABIs
> > and 64-bit architectures.  A general fix along the lines of the
> > bfd.c:is32bit thing that I mentioned upthread would be needed first.
> 
>  TBH I don't think putting a lot of infrastructure into an internal 
> feature meant for testing only is worth the effort unless proved 
> otherwise.  I propose to include my testcases as is now and then the 
> respective platform maintainers to implement TC_ADDRESS_BYTES as a need 
> arises, i.e. someone notices breakage or they feel like looking for work.  
> We've got around a year until the next release now, so there's plenty of 
> time to catch any problems and I find the idea of rejecting a test case 
> because "it may reveal breakage somewhere" backwards, sorry.  This is 
> exactly what test cases are for.
> 
>  If enough evidence is found this is worth abstracting, then the 
> bfd.c:is32bit idea you propose may be implemented (though I'd be more 
> comfortable with a function returning a quantitative result -- there are 
> undoubtedly addressing submode variations other to 64/32, e.g. 32/16, 
> 24/16, etc.).  The TC_ADDRESS_BYTES macro is distinct enough for all the 
> places to be easily identified at that stage.
> 
>  As I believe these test cases are good enough in the current form, I will 
> not make any further development in this area.  I'll be happy to commit my 
> proposal as posted, but otherwise I'll dismiss it -- feel free to pick it 
> up yourself, share with somebody or discard altogether.
> 
> > (That fix would also have worked for your three examples, without
> > the need for a MIPS-specific patch.  The reason I didn't insist
> > is that a MIPS-specific change is needed for EABI64 (32-bit ELF,
> > 64-bit addresses, ick.).)
> 
>  Well, I believe TC_ADDRESS_BYTES is the least of concern with getting the 
> address size right.

 Since you've clearly been unconvinced, here's a regenerated version of 
the proposal with code switched back to the ELF64 conditional.  There's 
clearly no point in holding an otherwise useful test case over such an 
unimportant detail.

 Aside from that and copyright years, ld-mips-elf/comm-data.exp has been 
updated to handle a recent change to reject unknown -z options.

 Regression re-tested with mips-sde-elf and mips-linux-gnu.  OK to apply?

2011-10-31  Maciej W. Rozycki  <macro@codesourcery.com>

	PR ld/10144
	* 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-mips-debug-reloc.patch
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data.exp	2011-10-31 11:51:16.915860689 +0000
@@ -0,0 +1,76 @@
+# Expect script for common symbol override.
+#
+#   Copyright 2011  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 favorite 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 flavor.
+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: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data1.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data1.s	2011-10-31 00:00:00.000000000 +0000
@@ -0,0 +1,6 @@
+	.section .rodata,"a",%progbits
+	.balign	8
+	.globl	foo
+	.type	foo,%object
+foo:
+	.skip	4, 0
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data1.sd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data1.sd	2011-10-31 00:00:00.000000000 +0000
@@ -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: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.rd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.rd	2011-10-31 00:00:00.000000000 +0000
@@ -0,0 +1 @@
+There are no relocations in this file\.
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.s	2011-10-31 00:00:00.000000000 +0000
@@ -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: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.sd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.sd	2011-10-31 00:00:00.000000000 +0000
@@ -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: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.xd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/comm-data2.xd	2011-10-31 00:00:00.000000000 +0000
@@ -0,0 +1,2 @@
+Hex dump of section '\.debug_foo':
+ +0x0+ +00000000 00000000 00000000 00000000 +\.\.\.\.\.\.\.\.\.\.\.\.\.\.\.\.
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/comm-data.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/comm-data.exp	2011-10-31 11:53:50.935864982 +0000
@@ -0,0 +1,86 @@
+# Expect script for common symbol override, MIPS variation.
+#
+#   Copyright 2011  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-Linux targets; feel free to include your favorite 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"
+
+    # There's no "-z copyreloc" option, deal with it.
+    set ZFLAG [string map [list copyreloc "" nocopyreloc "-z $reloc"] $reloc]
+    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 $ZFLAG -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: binutils-fsf-trunk-quilt/ld/testsuite/lib/ld-lib.exp
===================================================================
--- binutils-fsf-trunk-quilt.orig/ld/testsuite/lib/ld-lib.exp	2011-10-27 19:48:39.000000000 +0100
+++ binutils-fsf-trunk-quilt/ld/testsuite/lib/ld-lib.exp	2011-10-31 11:50:18.235861872 +0000
@@ -964,11 +964,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
@@ -1162,7 +1163,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
@@ -1284,7 +1286,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

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

[Richard Sandiford]

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> 2011-10-31  Maciej W. Rozycki  <macro@codesourcery.com>
>
> 	PR ld/10144
> 	* 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.

Looks good, thanks.

Richard

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

[Maciej W. Rozycki]

On Thu, 24 Nov 2011, Richard Sandiford wrote:

> > 	PR ld/10144
> > 	* 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.
> 
> Looks good, thanks.

 Applied now, thanks.

  Maciej

Fix CRIS bug exposed by "MIPS/BFD: Don't make debug section relocs dynamic"

[Hans-Peter Nilsson]

> From: "Maciej W. Rozycki" <macro@codesourcery.com>
> Date: Mon, 31 Oct 2011 13:21:52 +0100

> 2011-10-31  Maciej W. Rozycki  <macro@codesourcery.com>
> 
>         PR ld/10144
>         * 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.

This new test caused the following failure to appear for
cris-axis-linux-gnu:

Running /tmp/hpautotest-binutils/bsrc/src/ld/testsuite/ld-elf/comm-data.exp ...
FAIL: Common symbol override test

...but it turned out to be a target bug, so...thanks, I guess. :)

I see this test fails for m68k-linux too, if someone feels pity
(no listed maintainer).

No regressions tested cris-elf cris-linux.
Can I put this on the 2.22 branch too?

bfd:
	* elf32-cris.c (cris_elf_check_relocs) <plt accounting for
	R_CRIS_8, R_CRIS_16, and R_CRIS_32>: Move early break for
	non-SEC_ALLOC sections before GOT and PLT accounting.

Index: elf32-cris.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-cris.c,v
retrieving revision 1.117
diff -p -u -r1.117 elf32-cris.c
--- elf32-cris.c	19 Oct 2011 07:17:13 -0000	1.117
+++ elf32-cris.c	1 Dec 2011 02:47:49 -0000
@@ -3583,6 +3583,12 @@ cris_elf_check_relocs (bfd *abfd,
 		 sec,
 		 cris_elf_howto_table[r_type].name);
 	    }
+
+	  /* We don't need to handle relocs into sections not going into
+	     the "real" output.  */
+	  if ((sec->flags & SEC_ALLOC) == 0)
+	    break;
+
 	  if (h != NULL)
 	    {
 	      h->non_got_ref = 1;
@@ -3612,11 +3618,6 @@ cris_elf_check_relocs (bfd *abfd,
 	  if (! info->shared)
 	    break;
 
-	  /* We don't need to handle relocs into sections not going into
-	     the "real" output.  */
-	  if ((sec->flags & SEC_ALLOC) == 0)
-	    break;
-
 	  /* We may need to create a reloc section in the dynobj and made room
 	     for this reloc.  */
 	  if (sreloc == NULL)

brgds, H-P

Re: Fix CRIS bug exposed by "MIPS/BFD: Don't make debug section relocs dynamic"

[Tristan Gingold]

On Dec 1, 2011, at 3:52 AM, Hans-Peter Nilsson wrote:

>> From: "Maciej W. Rozycki" <macro@codesourcery.com>
>> Date: Mon, 31 Oct 2011 13:21:52 +0100
> 
>> 2011-10-31  Maciej W. Rozycki  <macro@codesourcery.com>
>> 
>>        PR ld/10144
>>        * 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.
> 
> This new test caused the following failure to appear for
> cris-axis-linux-gnu:
> 
> Running /tmp/hpautotest-binutils/bsrc/src/ld/testsuite/ld-elf/comm-data.exp ...
> FAIL: Common symbol override test
> 
> ...but it turned out to be a target bug, so...thanks, I guess. :)
> 
> I see this test fails for m68k-linux too, if someone feels pity
> (no listed maintainer).
> 
> No regressions tested cris-elf cris-linux.
> Can I put this on the 2.22 branch too?

Sure.

> 
> bfd:
> 	* elf32-cris.c (cris_elf_check_relocs) <plt accounting for
> 	R_CRIS_8, R_CRIS_16, and R_CRIS_32>: Move early break for
> 	non-SEC_ALLOC sections before GOT and PLT accounting.
> 
> Index: elf32-cris.c
> ===================================================================
> RCS file: /cvs/src/src/bfd/elf32-cris.c,v
> retrieving revision 1.117
> diff -p -u -r1.117 elf32-cris.c
> --- elf32-cris.c	19 Oct 2011 07:17:13 -0000	1.117
> +++ elf32-cris.c	1 Dec 2011 02:47:49 -0000
> @@ -3583,6 +3583,12 @@ cris_elf_check_relocs (bfd *abfd,
> 		 sec,
> 		 cris_elf_howto_table[r_type].name);
> 	    }
> +
> +	  /* We don't need to handle relocs into sections not going into
> +	     the "real" output.  */
> +	  if ((sec->flags & SEC_ALLOC) == 0)
> +	    break;
> +
> 	  if (h != NULL)
> 	    {
> 	      h->non_got_ref = 1;
> @@ -3612,11 +3618,6 @@ cris_elf_check_relocs (bfd *abfd,
> 	  if (! info->shared)
> 	    break;
> 
> -	  /* We don't need to handle relocs into sections not going into
> -	     the "real" output.  */
> -	  if ((sec->flags & SEC_ALLOC) == 0)
> -	    break;
> -
> 	  /* We may need to create a reloc section in the dynobj and made room
> 	     for this reloc.  */
> 	  if (sreloc == NULL)
> 
> brgds, H-P

Re: Fix CRIS bug exposed by "MIPS/BFD: Don't make debug section relocs dynamic"

[Mikael Pettersson]

Hans-Peter Nilsson writes:
 > > From: "Maciej W. Rozycki" <macro@codesourcery.com>
 > > Date: Mon, 31 Oct 2011 13:21:52 +0100
 > 
 > > 2011-10-31  Maciej W. Rozycki  <macro@codesourcery.com>
 > > 
 > >         PR ld/10144
 > >         * 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.
 > 
 > This new test caused the following failure to appear for
 > cris-axis-linux-gnu:
 > 
 > Running /tmp/hpautotest-binutils/bsrc/src/ld/testsuite/ld-elf/comm-data.exp ...
 > FAIL: Common symbol override test
 > 
 > ...but it turned out to be a target bug, so...thanks, I guess. :)
 > 
 > I see this test fails for m68k-linux too, if someone feels pity
 > (no listed maintainer).
 > 
 > No regressions tested cris-elf cris-linux.
 > Can I put this on the 2.22 branch too?
 > 
 > bfd:
 > 	* elf32-cris.c (cris_elf_check_relocs) <plt accounting for
 > 	R_CRIS_8, R_CRIS_16, and R_CRIS_32>: Move early break for
 > 	non-SEC_ALLOC sections before GOT and PLT accounting.
 > 
 > Index: elf32-cris.c
 > ===================================================================
 > RCS file: /cvs/src/src/bfd/elf32-cris.c,v
 > retrieving revision 1.117
 > diff -p -u -r1.117 elf32-cris.c
 > --- elf32-cris.c	19 Oct 2011 07:17:13 -0000	1.117
 > +++ elf32-cris.c	1 Dec 2011 02:47:49 -0000
 > @@ -3583,6 +3583,12 @@ cris_elf_check_relocs (bfd *abfd,
 >  		 sec,
 >  		 cris_elf_howto_table[r_type].name);
 >  	    }
 > +
 > +	  /* We don't need to handle relocs into sections not going into
 > +	     the "real" output.  */
 > +	  if ((sec->flags & SEC_ALLOC) == 0)
 > +	    break;
 > +
 >  	  if (h != NULL)
 >  	    {
 >  	      h->non_got_ref = 1;
 > @@ -3612,11 +3618,6 @@ cris_elf_check_relocs (bfd *abfd,
 >  	  if (! info->shared)
 >  	    break;
 >  
 > -	  /* We don't need to handle relocs into sections not going into
 > -	     the "real" output.  */
 > -	  if ((sec->flags & SEC_ALLOC) == 0)
 > -	    break;
 > -
 >  	  /* We may need to create a reloc section in the dynobj and made room
 >  	     for this reloc.  */
 >  	  if (sreloc == NULL)
 > 
 > brgds, H-P

I've implemented a similar change for elf32-m68k.c which fixes
ld-elf/comm-data.exp for m68k-linux, with no testsuite regressions
on head or the 2.22 release.

As for m68k maintainers, I looked around in recent ChangeLogs,
and both Andreas Schwab and Maxim Kuvyrkov seem likely candidates
so I've added them to the Cc: list.

Ok for head and 2.22 branch?

(If approved I'll need for someone else to do the commits.)

/Mikael


bfd/

2011-12-01  Mikael Pettersson  <mikpe@it.uu.se>

	* elf32-m68k.c (elf_m68k_check_relocs) <R_68K_8, R68K_16, R_68K_32>: For
	non-SEC_ALLOC sections break before GOT and PLT accounting.

--- binutils-2.22.51/bfd/elf32-m68k.c.~1~	2011-10-19 09:17:14.000000000 +0200
+++ binutils-2.22.51/bfd/elf32-m68k.c	2011-12-01 10:41:31.000000000 +0100
@@ -2816,6 +2816,11 @@ elf_m68k_check_relocs (abfd, info, sec, 
 	case R_68K_8:
 	case R_68K_16:
 	case R_68K_32:
+	  /* We don't need to handle relocs into sections not going into
+	     the "real" output.  */
+	  if ((sec->flags & SEC_ALLOC) == 0)
+	      break;
+
 	  if (h != NULL)
 	    {
 	      /* Make sure a plt entry is created for this symbol if it
@@ -2829,8 +2834,7 @@ elf_m68k_check_relocs (abfd, info, sec, 
 
 	  /* If we are creating a shared library, we need to copy the
 	     reloc into the shared library.  */
-	  if (info->shared
-	      && (sec->flags & SEC_ALLOC) != 0)
+	  if (info->shared)
 	    {
 	      /* When creating a shared object, we must copy these
 		 reloc types into the output file.  We create a reloc

Re: Fix CRIS bug exposed by "MIPS/BFD: Don't make debug section relocs dynamic"

[nick clifton]

Hi Mikael,

> 2011-12-01  Mikael Pettersson<mikpe@it.uu.se>
>
> 	* elf32-m68k.c (elf_m68k_check_relocs)<R_68K_8, R68K_16, R_68K_32>: For
> 	non-SEC_ALLOC sections break before GOT and PLT accounting.

Approved and applied.

Cheers
   Nick

Re: Fix CRIS bug exposed by "MIPS/BFD: Don't make debug section relocs dynamic"

[Tristan Gingold]

On Dec 1, 2011, at 11:46 AM, Mikael Pettersson wrote:

> Hans-Peter Nilsson writes:
>>> From: "Maciej W. Rozycki" <macro@codesourcery.com>
>>> Date: Mon, 31 Oct 2011 13:21:52 +0100
>> 
>>> 2011-10-31  Maciej W. Rozycki  <macro@codesourcery.com>
>>> 
>>>        PR ld/10144
>>>        * 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.
>> 
>> This new test caused the following failure to appear for
>> cris-axis-linux-gnu:
>> 
>> Running /tmp/hpautotest-binutils/bsrc/src/ld/testsuite/ld-elf/comm-data.exp ...
>> FAIL: Common symbol override test
>> 
>> ...but it turned out to be a target bug, so...thanks, I guess. :)
>> 
>> I see this test fails for m68k-linux too, if someone feels pity
>> (no listed maintainer).
>> 
>> No regressions tested cris-elf cris-linux.
>> Can I put this on the 2.22 branch too?

Yes.

>> 
>> bfd:
>> 	* elf32-cris.c (cris_elf_check_relocs) <plt accounting for
>> 	R_CRIS_8, R_CRIS_16, and R_CRIS_32>: Move early break for
>> 	non-SEC_ALLOC sections before GOT and PLT accounting.
>> 
>> Index: elf32-cris.c
>> ===================================================================
>> RCS file: /cvs/src/src/bfd/elf32-cris.c,v
>> retrieving revision 1.117
>> diff -p -u -r1.117 elf32-cris.c
>> --- elf32-cris.c	19 Oct 2011 07:17:13 -0000	1.117
>> +++ elf32-cris.c	1 Dec 2011 02:47:49 -0000
>> @@ -3583,6 +3583,12 @@ cris_elf_check_relocs (bfd *abfd,
>> 		 sec,
>> 		 cris_elf_howto_table[r_type].name);
>> 	    }
>> +
>> +	  /* We don't need to handle relocs into sections not going into
>> +	     the "real" output.  */
>> +	  if ((sec->flags & SEC_ALLOC) == 0)
>> +	    break;
>> +
>> 	  if (h != NULL)
>> 	    {
>> 	      h->non_got_ref = 1;
>> @@ -3612,11 +3618,6 @@ cris_elf_check_relocs (bfd *abfd,
>> 	  if (! info->shared)
>> 	    break;
>> 
>> -	  /* We don't need to handle relocs into sections not going into
>> -	     the "real" output.  */
>> -	  if ((sec->flags & SEC_ALLOC) == 0)
>> -	    break;
>> -
>> 	  /* We may need to create a reloc section in the dynobj and made room
>> 	     for this reloc.  */
>> 	  if (sreloc == NULL)
>> 
>> brgds, H-P
> 
> I've implemented a similar change for elf32-m68k.c which fixes
> ld-elf/comm-data.exp for m68k-linux, with no testsuite regressions
> on head or the 2.22 release.
> 
> As for m68k maintainers, I looked around in recent ChangeLogs,
> and both Andreas Schwab and Maxim Kuvyrkov seem likely candidates
> so I've added them to the Cc: list.
> 
> Ok for head and 2.22 branch?
> 
> (If approved I'll need for someone else to do the commits.)
> 
> /Mikael
> 
> 
> bfd/
> 
> 2011-12-01  Mikael Pettersson  <mikpe@it.uu.se>
> 
> 	* elf32-m68k.c (elf_m68k_check_relocs) <R_68K_8, R68K_16, R_68K_32>: For
> 	non-SEC_ALLOC sections break before GOT and PLT accounting.
> 
> --- binutils-2.22.51/bfd/elf32-m68k.c.~1~	2011-10-19 09:17:14.000000000 +0200
> +++ binutils-2.22.51/bfd/elf32-m68k.c	2011-12-01 10:41:31.000000000 +0100
> @@ -2816,6 +2816,11 @@ elf_m68k_check_relocs (abfd, info, sec, 
> 	case R_68K_8:
> 	case R_68K_16:
> 	case R_68K_32:
> +	  /* We don't need to handle relocs into sections not going into
> +	     the "real" output.  */
> +	  if ((sec->flags & SEC_ALLOC) == 0)
> +	      break;
> +
> 	  if (h != NULL)
> 	    {
> 	      /* Make sure a plt entry is created for this symbol if it
> @@ -2829,8 +2834,7 @@ elf_m68k_check_relocs (abfd, info, sec, 
> 
> 	  /* If we are creating a shared library, we need to copy the
> 	     reloc into the shared library.  */
> -	  if (info->shared
> -	      && (sec->flags & SEC_ALLOC) != 0)
> +	  if (info->shared)
> 	    {
> 	      /* When creating a shared object, we must copy these
> 		 reloc types into the output file.  We create a reloc

Re: Fix CRIS bug exposed by "MIPS/BFD: Don't make debug section relocs dynamic"

[Hans-Peter Nilsson]

> From: Tristan Gingold <gingold@adacore.com>
> Date: Fri, 2 Dec 2011 09:12:12 +0100

> On Dec 1, 2011, at 11:46 AM, Mikael Pettersson wrote:
> 
> > Hans-Peter Nilsson writes:
> >> I see this test fails for m68k-linux too, if someone feels pity
> >> (no listed maintainer).
> >> 
> >> No regressions tested cris-elf cris-linux.
> >> Can I put this on the 2.22 branch too?
> 
> Yes.

Did you mean to reply to Mikael Pettersson's patch and request
for 2.22?  (More likely than replying to me a second time, but
better safe than sorry. :)

Mikael, I don't see you mentioned in copyright.list, so if
unless it's already in progress, I'd suggest you take care of
the GNU copyright assignment paperwork, even if IMHO this
particular patch was a small enough.

> >> bfd:
> >> 	* elf32-cris.c (cris_elf_check_relocs) <plt accounting for
> >> 	R_CRIS_8, R_CRIS_16, and R_CRIS_32>: Move early break for
> >> 	non-SEC_ALLOC sections before GOT and PLT accounting.

> > I've implemented a similar change for elf32-m68k.c which fixes
> > ld-elf/comm-data.exp for m68k-linux, with no testsuite regressions
> > on head or the 2.22 release.
> > 
> > As for m68k maintainers, I looked around in recent ChangeLogs,
> > and both Andreas Schwab and Maxim Kuvyrkov seem likely candidates
> > so I've added them to the Cc: list.
> > 
> > Ok for head and 2.22 branch?

> > 2011-12-01  Mikael Pettersson  <mikpe@it.uu.se>
> > 
> > 	* elf32-m68k.c (elf_m68k_check_relocs) <R_68K_8, R68K_16, R_68K_32>: For
> > 	non-SEC_ALLOC sections break before GOT and PLT accounting.

brgds, H-P

Re: Fix CRIS bug exposed by "MIPS/BFD: Don't make debug section relocs dynamic"

[Tristan Gingold]

On Dec 2, 2011, at 1:12 PM, Hans-Peter Nilsson wrote:

>> From: Tristan Gingold <gingold@adacore.com>
>> Date: Fri, 2 Dec 2011 09:12:12 +0100
> 
>> On Dec 1, 2011, at 11:46 AM, Mikael Pettersson wrote:
>> 
>>> Hans-Peter Nilsson writes:
>>>> I see this test fails for m68k-linux too, if someone feels pity
>>>> (no listed maintainer).
>>>> 
>>>> No regressions tested cris-elf cris-linux.
>>>> Can I put this on the 2.22 branch too?
>> 
>> Yes.
> 
> Did you mean to reply to Mikael Pettersson's patch and request
> for 2.22?  (More likely than replying to me a second time, but
> better safe than sorry. :)

I ok'ed the backport for 2.22.  I can't approve the patch for trunk.
(sorry for not having been clear).

> Mikael, I don't see you mentioned in copyright.list, so if
> unless it's already in progress, I'd suggest you take care of
> the GNU copyright assignment paperwork, even if IMHO this
> particular patch was a small enough.
> 
>>>> bfd:
>>>> 	* elf32-cris.c (cris_elf_check_relocs) <plt accounting for
>>>> 	R_CRIS_8, R_CRIS_16, and R_CRIS_32>: Move early break for
>>>> 	non-SEC_ALLOC sections before GOT and PLT accounting.
> 
>>> I've implemented a similar change for elf32-m68k.c which fixes
>>> ld-elf/comm-data.exp for m68k-linux, with no testsuite regressions
>>> on head or the 2.22 release.
>>> 
>>> As for m68k maintainers, I looked around in recent ChangeLogs,
>>> and both Andreas Schwab and Maxim Kuvyrkov seem likely candidates
>>> so I've added them to the Cc: list.
>>> 
>>> Ok for head and 2.22 branch?
> 
>>> 2011-12-01  Mikael Pettersson  <mikpe@it.uu.se>
>>> 
>>> 	* elf32-m68k.c (elf_m68k_check_relocs) <R_68K_8, R68K_16, R_68K_32>: For
>>> 	non-SEC_ALLOC sections break before GOT and PLT accounting.
> 
> brgds, H-P

Re: Fix CRIS bug exposed by "MIPS/BFD: Don't make debug section relocs dynamic"

[Hans-Peter Nilsson]

> From: Tristan Gingold <gingold@adacore.com>
> Date: Fri, 2 Dec 2011 13:50:28 +0100
> On Dec 2, 2011, at 1:12 PM, Hans-Peter Nilsson wrote:
> > Did you mean to reply to Mikael Pettersson's patch and request
> > for 2.22?  (More likely than replying to me a second time, but
> > better safe than sorry. :)
> 
> I ok'ed the backport for 2.22.  I can't approve the patch for trunk.

Nick approved and committed it to trunk and I've applied it to
the 2.22-branch now.  Thanks.

brgds, H-P

[PATCH] de-Linuxification

[Thomas Schwinge]

From: Thomas Schwinge <thomas@schwinge.name>

ld/testsuite/

	* ld-elf/comm-data.exp: Do not return for GNU systems.

---
 ld/testsuite/ld-elf/comm-data.exp |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/ld/testsuite/ld-elf/comm-data.exp b/ld/testsuite/ld-elf/comm-data.exp
index 86e95d1..3bc8484 100644
--- a/ld/testsuite/ld-elf/comm-data.exp
+++ b/ld/testsuite/ld-elf/comm-data.exp
@@ -1,6 +1,6 @@
 # Expect script for common symbol override.
 #
-#   Copyright 2011  Free Software Foundation, Inc.
+# Copyright 2011, 2012 Free Software Foundation, Inc.
 #
 # This file is part of the GNU Binutils.
 #
@@ -29,9 +29,10 @@ if ![is_elf_format] {
     return
 }
 
-# Exclude non-Linux targets; feel free to include your favorite one
+# Exclude some more targets; feel free to include your favorite one
 # if you like.
-if ![istarget *-*-linux*] {
+if { ![istarget *-*-linux*]
+     && ![istarget *-*-gnu*] } {
     return
 }
 
-- 
tg: (fe30cfd..) t/de-Linuxification (depends on: baseline)

Re: [PATCH] de-Linuxification

[Alan Modra]

On Thu, Feb 16, 2012 at 12:02:54AM +0100, Thomas Schwinge wrote:
> 	* ld-elf/comm-data.exp: Do not return for GNU systems.

Applied.

-- 
Alan Modra
Australia Development Lab, IBM