This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Restore .gnu.linkonce.r. g++-3.4 linking


Hi Alan,

Using notation `.prefix.*' for `.prefix.A' and `.prefix.B' but not `.prefix'.
Using notation `.prefix*'  for `.prefix.A' and `.prefix.B' and even `.prefix'.

On Fri, 24 Oct 2008 08:57:08 +0200, Alan Modra wrote:
> On Thu, Oct 23, 2008 at 01:59:14AM +0200, Jan Kratochvil wrote:
> > the attached testcase reproduces a regression of ld on the output of g++-3.4:
> > 
> > binutils-2.15.92.0.2:
> > `.gnu.linkonce.t.foo' referenced in section `.gnu.linkonce.r.foo' of
> > /tmp/2j.o: defined in discarded section `.gnu.linkonce.t.foo' of /tmp/2j.o
> > output produced  (.rela.gnu.linkonce.r.foo present with cleared relocations)
> > 
> > CVS HEAD:
> > `.gnu.linkonce.t.foo' referenced in section `.gnu.linkonce.r.foo' of
> > /tmp/2j.o: defined in discarded section `.gnu.linkonce.t.foo' of /tmp/2j.o
> > no output
> 
> If you want output, you can use --noinhibit-exec.

It is clear the suggested flag is not viable for normal day-to-day compilations.
Regular missing function definitions etc. would create a crashing output.


> > patched CVS HEAD:
> > (no warnings)
> > output produced (.rela.gnu.linkonce.r.foo present with cleared relocations)
> 
> ld should not silently produce what could be buggy output.  See PR 858
> which resulted in references like the above becoming hard errors.

The patch does not permit arbitrary unresolved relocations.

The patch affects only the sections `.gnu.linkonce.r.*' which are used only by
g++-3.4.

g++-4.3 already creates only `.text.*' (and `.rodata.*') sections in COMDAT
groups (and therefore with no `.gnu.linkonce' prefix) - unrelated to this
patch.

Even a relocation in `.gnu.linkonce.r.*' referencing an undefined symbol is
still an error with this patch as the _target_ section is not being discarded:
http://people.redhat.com/jkratoch/rodataX.C
(also a part of the new testcase)

A relocation in `.gnu.linkonce.r.A' referencing a function symbol in
`.gnu.linkonce.t.B' being discarded is not an error with the patch as the
referenced symbol is not STT_SECTION and therefore it is replaced by its kept
instance in `.gnu.linkonce.t.B' in some other file.
http://people.redhat.com/jkratoch/cross1.C
http://people.redhat.com/jkratoch/cross2.C
(also a part of the new testcase)

The case of a relocation in (kept) `.gnu.linkonce.r.A' referencing an
STT_SECTION symbol from (discarded) `.gnu.linkonce.t.B':
I do not think it is possible to produce such output from C/C++.  The former
patch silently discarded such relocation and according to your comment the new
patch will error out (as before) on such relocation.
(also a part of the new testcase)


> > /* g++-3.x specific - before COMDAT groups were supported.  .gnu.linkonce.r.*
> >    is present only sometimes, depending on the compilation options.
> >    .gnu.linkonce.r.* is like .rodata for its .gnu.linkonce.t.* so there is no
> >    use for it if it did not exist in the file from which we chose
> >    .gnu.linkonce.t.* as the one not being discarded.  We would also fail to
> >    PRETEND the symbols as the other .gnu.linkonce.r.* section has different
> >    size.  */
> > 
> > g++-3.x introduced .gnu.linkonce.r.* by:
> > http://gcc.gnu.org/ml/gcc-patches/2004-08/msg00653.html
> 
> So for 3.4.1 we ought to allow references from .rodata too?  Where do
> we draw the line?

No, because `.rodata' section are unrelated to the discarding process.
gcc-3.4/gcc/varasm.c uses .rodata* only for !DECL_ONE_ONLY sections.

`.rodata.*' sections are used only for -ffunction-sections -fdata-sections
compilation and such sections are joined (not one-only discarded).  These
sections should not be and are not affected by this patch.


No regressions.  The new testcase is not PASSing for:

#notarget: arc* d30v* dlx* openrisc* or32* pj*
as while being ELF they do not use the ELF linker (no
elf_backend_relocate_section defined for them).

These targets crx-elf h8300-elf mn10200-elf mn10300-elf FAIL as they have
bogus label in the error message:
`.L2^B1' referenced in section `.gnu.linkonce.r.foo' of tmpdir/dump1.o: defined in discarded section `.gnu.linkonce.t.bar' of tmpdir/dump1.o

hppa64-hp-hpux11.23 FAILs (local labels `1' and `2' FAIL there):
ld-elf/linkoncerdiff2.s:2: Error: junk at end of line, first unrecognized character is `1'

mmix-elf FAILs (other linkonce* also FAIL):
ld-elf/linkoncerdiff1.s:1: Error: junk at end of line, first unrecognized character is `,'



Thanks for checking the patch,
Jan


2008-10-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* elflink.c (elf_link_input_bfd): Handle pre-COMDAT g++-3.4 relocations
	in `.gnu.linkonce.r.*' referencing its `.gnu.linkonce.t.*'.

2008-10-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* ld-elf/linkoncerdiff.d, ld-elf/linkoncerdiff1.s,
	ld-elf/linkoncerdiff2.s: New.

--- bfd/elflink.c	20 Oct 2008 10:57:33 -0000	1.315
+++ bfd/elflink.c	26 Oct 2008 16:38:02 -0000
@@ -9273,7 +9273,23 @@ elf_link_input_bfd (struct elf_final_lin
 		  if ((sec = *ps) != NULL && elf_discarded_section (sec))
 		    {
 		      BFD_ASSERT (r_symndx != 0);
-		      if (action_discarded & COMPLAIN)
+
+		      /* Do not complain on unresolved relocations in
+			 `.gnu.linkonce.r.F' referencing its discarded
+			 `.gnu.linkonce.t.F' counterpart - g++-3.4 specific as
+			 g++-4.x is using COMDAT groups (without the
+			 `.gnu.linkonce' prefix) instead.  `.gnu.linkonce.r.*'
+			 were the `.rodata' part of its matching
+			 `.gnu.linkonce.t.*'.  If `.gnu.linkonce.r.F' is not
+			 discarded with its `.gnu.linkonce.t.F' being discarded
+			 means we chose one-only `.gnu.linkonce.t.F' section
+			 from an other file not needing any `.gnu.linkonce.r.F'
+			 and thus `.gnu.linkonce.r.F' could be in fact also
+			 discarded.  */
+		      if ((action_discarded & COMPLAIN)
+		          && !(CONST_STRNEQ (o->name, ".gnu.linkonce.r.")
+			       && CONST_STRNEQ (sec->name, ".gnu.linkonce.t.")
+			       && strcmp (o->name + 16, sec->name + 16) == 0))
 			(*finfo->info->callbacks->einfo)
 			  (_("%X`%s' referenced in section `%A' of %B: "
 			     "defined in discarded section `%A' of %B\n"),
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ld/testsuite/ld-elf/linkoncerdiff.d	26 Oct 2008 16:38:06 -0000
@@ -0,0 +1,5 @@
+#source: linkoncerdiff1.s
+#source: linkoncerdiff2.s
+#notarget: arc* d30v* dlx* openrisc* or32* pj*
+#ld: -r
+#error: `.gnu.linkonce.t.bar' referenced in section `.gnu.linkonce.r.foo' of tmpdir/dump1.o: defined in discarded section `.gnu.linkonce.t.bar' of tmpdir/dump1.o
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ld/testsuite/ld-elf/linkoncerdiff1.s	26 Oct 2008 16:38:06 -0000
@@ -0,0 +1,7 @@
+	.section	.gnu.linkonce.t.foo, "a", %progbits
+	.globl	symfoo
+symfoo:
+
+	.section	.gnu.linkonce.t.bar, "a", %progbits
+	.globl	symbar
+symbar:
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ld/testsuite/ld-elf/linkoncerdiff2.s	26 Oct 2008 16:38:06 -0000
@@ -0,0 +1,17 @@
+	.section	.gnu.linkonce.t.foo, "a", %progbits
+1:
+	.globl	symfoo
+symfoo:
+	.long	0
+
+	.section	.gnu.linkonce.t.bar, "a", %progbits
+2:
+	.globl	symbar
+symbar:
+	.long	0
+
+	.section	.gnu.linkonce.r.foo, "a", %progbits
+	.long	1b
+	.long	symfoo
+	.long	2b
+	.long	symbar


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]