elf32-ppc ld bug with symbol versioning

elf32-ppc ld bug with symbol versioning

[Roland McGrath]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 776 bytes --]

The attached shell archive unpacks a few files and a Makefile that
demonstrates a bug now biting glibc on powerpc-unknown-linux-gnu.
This shows up in mainline of today, as well as other recent versions.

With these files, "make" will give you:

/usr/local/bin/ld: lose.so: undefined versioned symbol name foo@@VERS_1

This boils down to an ld -shared that is resolving against a .so and a .a
with versioned references between them.  The crucial factor is probably
that bar.a(bar.o) has a weak defn of bar that should be in force despite
the presence of a bar symbol in foo.so earlier in the link order.  foo.so's
defn is ignored because it is a versioned defn with @ rather than @@,
but having done that seems to make ld fail to see bar.a(bar.o)'s own defn.


Thanks,
Roland


[-- Attachment #2: shell archive of ld test case --]
[-- Type: text/plain, Size: 3031 bytes --]

#!/bin/sh
# This is a shell archive (produced by GNU sharutils 4.2.1).
# To extract the files from this archive, save it to some FILE, remove
# everything before the `!/bin/sh' line above, then type `sh FILE'.
#
# Made on 2003-04-22 16:30 PDT by <roland@debian.sf.frob.com>.
# Source directory was `/mnt/home/roland/redhat/ld-bug'.
#
# Existing files will *not* be overwritten unless `-c' is specified.
# This format requires very little intelligence at unshar time.
# "if test", "echo", "mkdir", and "sed" may be needed.
#
# This shar contains:
# length mode       name
# ------ ---------- ------------------------------------------
#    332 -rw-rw-rw- Makefile
#    122 -rw-rw-rw- foo.c
#     44 -rw-rw-rw- foo.v
#     82 -rw-rw-rw- bar.c
#     25 -rw-rw-rw- lose.c
#
echo=echo
if mkdir _sh01948; then
  $echo 'x -' 'creating lock directory'
else
  $echo 'failed to create lock directory'
  exit 1
fi
# ============= Makefile ==============
if test -f 'Makefile' && test "$first_param" != -c; then
  $echo 'x -' SKIPPING 'Makefile' '(file already exists)'
else
  $echo 'x -' extracting 'Makefile' '(text)'
  sed 's/^X//' << 'SHAR_EOF' > 'Makefile' &&
Xlose.so: lose.c foo.so bar.a
X	$(CC) -o $@ -shared $^
Xfoo.so: foo.o foo.v
X	$(CC) -o $@ -shared -Wl,--version-script=foo.v foo.o
Xbar.a: bar.o
X	rm -f $@
X	ar cq $@ $^
X
Xfoo.o lose.o: CFLAGS = -fPIC
X
Xbug.tar: Makefile foo.c foo.v bar.c lose.c
X	rm -f $@
X	tar cf $@ $^
Xbug.shar: Makefile foo.c foo.v bar.c lose.c
X	rm -f $@
X	shar -V $^ > $@
SHAR_EOF
  : || $echo 'restore of' 'Makefile' 'failed'
fi
# ============= foo.c ==============
if test -f 'foo.c' && test "$first_param" != -c; then
  $echo 'x -' SKIPPING 'foo.c' '(file already exists)'
else
  $echo 'x -' extracting 'foo.c' '(text)'
  sed 's/^X//' << 'SHAR_EOF' > 'foo.c' &&
Xvoid __attribute__((weak)) foo () {}
Xvoid __attribute__((weak)) old_bar () { foo (); }
Xasm(".symver old_bar,bar@VERS_1");
SHAR_EOF
  : || $echo 'restore of' 'foo.c' 'failed'
fi
# ============= foo.v ==============
if test -f 'foo.v' && test "$first_param" != -c; then
  $echo 'x -' SKIPPING 'foo.v' '(file already exists)'
else
  $echo 'x -' extracting 'foo.v' '(text)'
  sed 's/^X//' << 'SHAR_EOF' > 'foo.v' &&
XVERS_1 {
X  global: foo; bar;
X  local: *;
X};
SHAR_EOF
  : || $echo 'restore of' 'foo.v' 'failed'
fi
# ============= bar.c ==============
if test -f 'bar.c' && test "$first_param" != -c; then
  $echo 'x -' SKIPPING 'bar.c' '(file already exists)'
else
  $echo 'x -' extracting 'bar.c' '(text)'
  sed 's/^X//' << 'SHAR_EOF' > 'bar.c' &&
Xvoid __attribute__((weak)) foo () {}
Xvoid __attribute__((weak)) bar () { foo(); }
SHAR_EOF
  : || $echo 'restore of' 'bar.c' 'failed'
fi
# ============= lose.c ==============
if test -f 'lose.c' && test "$first_param" != -c; then
  $echo 'x -' SKIPPING 'lose.c' '(file already exists)'
else
  $echo 'x -' extracting 'lose.c' '(text)'
  sed 's/^X//' << 'SHAR_EOF' > 'lose.c' &&
Xvoid lose () { bar (); }
SHAR_EOF
  : || $echo 'restore of' 'lose.c' 'failed'
fi
rm -fr _sh01948
exit 0

Re: elf32-ppc ld bug with symbol versioning

[H. J. Lu]

On Tue, Apr 22, 2003 at 04:40:16PM -0700, Roland McGrath wrote:
Content-Description: message body text
> The attached shell archive unpacks a few files and a Makefile that
> demonstrates a bug now biting glibc on powerpc-unknown-linux-gnu.
> This shows up in mainline of today, as well as other recent versions.
> 
> With these files, "make" will give you:
> 
> /usr/local/bin/ld: lose.so: undefined versioned symbol name foo@@VERS_1
> 
> This boils down to an ld -shared that is resolving against a .so and a .a
> with versioned references between them.  The crucial factor is probably
> that bar.a(bar.o) has a weak defn of bar that should be in force despite
> the presence of a bar symbol in foo.so earlier in the link order.  foo.so's
> defn is ignored because it is a versioned defn with @ rather than @@,
> but having done that seems to make ld fail to see bar.a(bar.o)'s own defn.
> 

We should be careful about weak symbol changes. I have a patch to fix
weak symbols which will change how we handle weak symbols. But it
depends on:

http://sources.redhat.com/ml/binutils/2003-04/msg00239.html


H.J.

Re: elf32-ppc ld bug with symbol versioning

[Roland McGrath]

Actually removing all the weakness doesn't affect this test case.
I think this bug just has to do with symbol versioning.

Re: elf32-ppc ld bug with symbol versioning

[Daniel Jacobowitz]

On Tue, Apr 22, 2003 at 04:40:16PM -0700, Roland McGrath wrote:
Content-Description: message body text
> The attached shell archive unpacks a few files and a Makefile that
> demonstrates a bug now biting glibc on powerpc-unknown-linux-gnu.
> This shows up in mainline of today, as well as other recent versions.
> 
> With these files, "make" will give you:
> 
> /usr/local/bin/ld: lose.so: undefined versioned symbol name foo@@VERS_1
> 
> This boils down to an ld -shared that is resolving against a .so and a .a
> with versioned references between them.  The crucial factor is probably
> that bar.a(bar.o) has a weak defn of bar that should be in force despite
> the presence of a bar symbol in foo.so earlier in the link order.  foo.so's
> defn is ignored because it is a versioned defn with @ rather than @@,
> but having done that seems to make ld fail to see bar.a(bar.o)'s own defn.

This sounds awfully familiar.  Could you do me a favor and try he patch
in <http://sources.redhat.com/ml/binutils/2003-02/msg00419.html>?


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: elf32-ppc ld bug with symbol versioning

[H. J. Lu]

On Tue, Apr 22, 2003 at 05:03:55PM -0700, Roland McGrath wrote:
> Actually removing all the weakness doesn't affect this test case.
> I think this bug just has to do with symbol versioning.

The subject is misleading. The bug is not elf32-ppc specific. I will
look into it.

H.J.

Re: elf32-ppc ld bug with symbol versioning

[Roland McGrath]

> On Tue, Apr 22, 2003 at 05:03:55PM -0700, Roland McGrath wrote:
> > Actually removing all the weakness doesn't affect this test case.
> > I think this bug just has to do with symbol versioning.
> 
> The subject is misleading. The bug is not elf32-ppc specific. I will
> look into it.

I cannot reproduce it on elf32-i386 (i686-linux).

Re: elf32-ppc ld bug with symbol versioning

[Roland McGrath]

> This sounds awfully familiar.  Could you do me a favor and try he patch
> in <http://sources.redhat.com/ml/binutils/2003-02/msg00419.html>?

I think that fixed it.

Re: elf32-ppc ld bug with symbol versioning

[H. J. Lu]

On Tue, Apr 22, 2003 at 10:29:17PM -0700, Roland McGrath wrote:
> > On Tue, Apr 22, 2003 at 05:03:55PM -0700, Roland McGrath wrote:
> > > Actually removing all the weakness doesn't affect this test case.
> > > I think this bug just has to do with symbol versioning.
> > 
> > The subject is misleading. The bug is not elf32-ppc specific. I will
> > look into it.
> 
> I cannot reproduce it on elf32-i386 (i686-linux).

I can reproduce it on i686-linux with a even smaller testcase. The
error message is kind of bogus.


H.J.

Re: elf32-ppc ld bug with symbol versioning

[H. J. Lu]

On Tue, Apr 22, 2003 at 10:32:05PM -0700, Roland McGrath wrote:
> > This sounds awfully familiar.  Could you do me a favor and try he patch
> > in <http://sources.redhat.com/ml/binutils/2003-02/msg00419.html>?
> 
> I think that fixed it.

I don't think it is the right fix. I have 2 small testcases. One of
them still fails even with that patch.

BTW, does anyone have any objections to this patch:

http://sources.redhat.com/ml/binutils/2003-04/msg00342.html

I'd like to use it in bfd.


H.J.

Re: elf32-ppc ld bug with symbol versioning

[H. J. Lu]

On Tue, Apr 22, 2003 at 11:40:01PM -0700, H. J. Lu wrote:
> On Tue, Apr 22, 2003 at 10:32:05PM -0700, Roland McGrath wrote:
> > > This sounds awfully familiar.  Could you do me a favor and try he patch
> > > in <http://sources.redhat.com/ml/binutils/2003-02/msg00419.html>?
> > 
> > I think that fixed it.
> 
> I don't think it is the right fix. I have 2 small testcases. One of
> them still fails even with that patch.
> 

Oops. I took it back. It is ok.

> BTW, does anyone have any objections to this patch:
> 
> http://sources.redhat.com/ml/binutils/2003-04/msg00342.html
> 
> I'd like to use it in bfd.
> 
> 
> H.J.

PATCH: A testcase for ld bug with symbol versioning

[H. J. Lu]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

On Tue, Apr 22, 2003 at 11:04:09PM -0700, H. J. Lu wrote:
> On Tue, Apr 22, 2003 at 10:29:17PM -0700, Roland McGrath wrote:
> > > On Tue, Apr 22, 2003 at 05:03:55PM -0700, Roland McGrath wrote:
> > > > Actually removing all the weakness doesn't affect this test case.
> > > > I think this bug just has to do with symbol versioning.
> > > 
> > > The subject is misleading. The bug is not elf32-ppc specific. I will
> > > look into it.
> > 
> > I cannot reproduce it on elf32-i386 (i686-linux).
> 
> I can reproduce it on i686-linux with a even smaller testcase. The
> error message is kind of bogus.
> 
> 

This is the testcase I will check in shortly.


H.J.

[-- Attachment #2: ld-version-ind.patch --]
[-- Type: text/plain, Size: 2800 bytes --]

2003-04-23  H.J. Lu <hjl@gnu.org>

	* ld-elfvers/vers.exp (objdump_dynsymstuff): Support empty
	files.
	(objdump_versionstuff): Likewise.
	Add tests vers25a, vers25b1 and vers25b2 for versioned
	definition vs. normal definition in different files.

	* ld-elfvers/vers25a.c: New file.
	* ld-elfvers/vers25a.dsym: Likewise.
	* ld-elfvers/vers25a.map: Likewise.
	* ld-elfvers/vers25a.ver: Likewise.

	* ld-elfvers/vers25b.c: New empty file.
	* ld-elfvers/vers25b.dsym: Likewise.
	* ld-elfvers/vers25b.ver: Likewise.

--- ld/testsuite/ld-elfvers/vers.exp.ind	2003-02-13 14:14:11.000000000 -0800
+++ ld/testsuite/ld-elfvers/vers.exp	2003-04-23 08:43:35.000000000 -0700
@@ -351,6 +351,11 @@ proc objdump_dynsymstuff { objdump objec
 	}
 	close $file_b
 	
+	# Support empty files.
+	if { ![info exists list_a] && ![info exists list_b] } then {
+	    return 1
+	}
+
 	for { set i 0 } { $i < [llength $list_b] } { incr i } {
 	    set line_b [lindex $list_b $i]
 	    
@@ -446,6 +451,11 @@ proc objdump_versionstuff { objdump obje
 	close $f1
 	close $f2
 
+	# Support empty expected file.
+	if [string match "" $l2] then {
+	    return 1
+	}
+
 	verbose -log "Did not find \"$l2\""
 	set f1 [open $tmpdir/objdump.out r]
 	while { [gets $f1 l1] != -1 } {
@@ -863,3 +873,8 @@ run_ld_link_tests [list "\"vers24c\"
 			 \"-shared --version-script $srcdir/$subdir/vers24.map\"
 			 \"$as_pic_flags\" {vers24c.c} { { readelf -Wrs vers24.rd } }
 			 \"libvers24c.so\" \"-fpic\""]
+
+# Test versioned definition vs. normal definition in different files.
+build_vers_lib "vers25a" vers25a.c vers25a "" vers25a.map vers25a.ver vers25a.dsym ""
+build_vers_lib "vers25b1" vers25b.c vers25b1 "vers25a.o vers25a.so" "" vers25b.ver vers25b.dsym ""
+build_vers_lib "vers25b2" vers25b.c vers25b2 "vers25a.so vers25a.o" "" vers25b.ver vers25b.dsym ""
--- ld/testsuite/ld-elfvers/vers25a.c.ind	2003-04-23 08:37:10.000000000 -0700
+++ ld/testsuite/ld-elfvers/vers25a.c	2003-04-23 07:42:35.000000000 -0700
@@ -0,0 +1 @@
+void foo () {}
--- ld/testsuite/ld-elfvers/vers25a.dsym.ind	2003-04-23 08:37:10.000000000 -0700
+++ ld/testsuite/ld-elfvers/vers25a.dsym	2003-04-23 07:48:27.000000000 -0700
@@ -0,0 +1,2 @@
+[0]*[ 	]+g[ 	]+DO[ 	]+\*ABS\*[ 	]+[0]*[ 	]+VERS.0[ 	]+VERS.0
+[0-9a-f]*[ 	]+g[ 	]+DF[ 	]+.text[	]+[0-9a-f]*[ 	]+VERS.0[ 	]+(0x[0-9a-f]*|)[ 	]*foo
--- ld/testsuite/ld-elfvers/vers25a.map.ind	2003-04-23 08:37:10.000000000 -0700
+++ ld/testsuite/ld-elfvers/vers25a.map	2003-04-23 07:45:42.000000000 -0700
@@ -0,0 +1,5 @@
+VERS.0 {
+  global:
+    foo;
+  local: *;
+};
--- ld/testsuite/ld-elfvers/vers25a.ver.ind	2003-04-23 08:37:10.000000000 -0700
+++ ld/testsuite/ld-elfvers/vers25a.ver	2003-04-23 07:47:17.000000000 -0700
@@ -0,0 +1,4 @@
+Version definitions:
+1 0x01 0x065d090f vers25a.so
+2 0x00 0x05aa7610 VERS.0
+

Re: elf32-ppc ld bug with symbol versioning

[Ian Lance Taylor]

"H. J. Lu" <hjl@lucon.org> writes:

> BTW, does anyone have any objections to this patch:
> 
> http://sources.redhat.com/ml/binutils/2003-04/msg00342.html
> 
> I'd like to use it in bfd.

If you're going to change the formats accepted by vfinfo you need to
change the comment above it describing it.

Note that vfinfo implements %v, so I'm not sure why you need %x and
%lx.  If you don't really need %ld and %lu, I would vote against
including them.  If we're going to reimplement printf, we should use a
different approach.

I have no basic problem with adding the callback.

Ian

PATCH: Add error handler to linker callback

[H. J. Lu]

[-- Attachment #1: Type: text/plain, Size: 707 bytes --]

On Wed, Apr 23, 2003 at 11:34:25AM -0700, Ian Lance Taylor wrote:
> "H. J. Lu" <hjl@lucon.org> writes:
> 
> > BTW, does anyone have any objections to this patch:
> > 
> > http://sources.redhat.com/ml/binutils/2003-04/msg00342.html
> > 
> > I'd like to use it in bfd.
> 
> If you're going to change the formats accepted by vfinfo you need to
> change the comment above it describing it.
> 
> Note that vfinfo implements %v, so I'm not sure why you need %x and
> %lx.  If you don't really need %ld and %lu, I would vote against
> including them.  If we're going to reimplement printf, we should use a
> different approach.
> 
> I have no basic problem with adding the callback.
> 

How about this one?


H.J.

[-- Attachment #2: binutils-ld-cb.patch --]
[-- Type: text/plain, Size: 2107 bytes --]

include/

2003-04-17  H.J. Lu <hjl@gnu.org>

	* bfdlink.h (bfd_link_callbacks): Add error_handler.

ld/

2003-04-17  H.J. Lu <hjl@gnu.org>

	* ldmain.c (link_callbacks): Initialize error_handler.

	* ldmisc.c (error_handler): New function.

	* ldmisc.h (error_handler): New prototype.

--- binutils/include/bfdlink.h.cb	2003-01-02 09:57:40.000000000 -0800
+++ binutils/include/bfdlink.h	2003-04-17 15:40:47.000000000 -0700
@@ -481,6 +481,11 @@ struct bfd_link_callbacks
   bfd_boolean (*notice)
     PARAMS ((struct bfd_link_info *, const char *name,
 	     bfd *abfd, asection *section, bfd_vma address));
+  /* A function which is called for reporting a linker error. ID is the
+     error identifier. The remaining input is the same as einfo () in
+     ld.  */
+  bfd_boolean (*error_handler)
+    PARAMS ((int id, const char * fmt, ...));
 };
 \f
 /* The linker builds link_order structures which tell the code how to
--- binutils/ld/ldmain.c.cb	2003-04-01 14:29:10.000000000 -0800
+++ binutils/ld/ldmain.c	2003-04-17 15:53:06.000000000 -0700
@@ -155,7 +155,8 @@ static struct bfd_link_callbacks link_ca
   reloc_overflow,
   reloc_dangerous,
   unattached_reloc,
-  notice
+  notice,
+  error_handler
 };
 
 struct bfd_link_info link_info;
--- binutils/ld/ldmisc.c.cb	2003-04-01 14:29:10.000000000 -0800
+++ binutils/ld/ldmisc.c	2003-04-17 21:23:39.000000000 -0700
@@ -506,3 +529,14 @@ ld_abort (file, line, fn)
   einfo (_("%P%F: please report this bug\n"));
   xexit (1);
 }
+
+bfd_boolean
+error_handler VPARAMS ((int id ATTRIBUTE_UNUSED, const char *fmt, ...))
+{
+  VA_OPEN (arg, fmt);
+  VA_FIXEDARG (arg, const char *, fmt);
+
+  vfinfo (stderr, fmt, arg);
+  VA_CLOSE (arg);
+  return TRUE;
+}
--- binutils/ld/ldmisc.h.cb	2001-09-27 10:28:34.000000000 -0700
+++ binutils/ld/ldmisc.h	2003-04-17 15:54:30.000000000 -0700
@@ -22,6 +22,7 @@
 #ifndef LDMISC_H
 #define LDMISC_H
 
+extern bfd_boolean error_handler PARAMS ((int, const char *, ...));
 extern void einfo PARAMS ((const char *, ...));
 extern void minfo PARAMS ((const char *, ...));
 extern void info_msg PARAMS ((const char *, ...));

Re: PATCH: Add error handler to linker callback

[Ian Lance Taylor]

"H. J. Lu" <hjl@lucon.org> writes:

> > I have no basic problem with adding the callback.
> > 
> 
> How about this one?

This is fine with me.  These days I'd rather have Nick or Alan or
somebody approve it for real, though.

Ian

Re: PATCH: Add error handler to linker callback

[Nick Clifton]

Hi Ian,

> > How about this one?
> 
> This is fine with me.  These days I'd rather have Nick or Alan or
> somebody approve it for real, though.

The patch is fine with me too.

I am sure that neither Alan nor I have any problems with you approving
patches either Ian.  After all you know more about the linker and bfd
than any other living soul today.

Cheers
        Nick