Your bfd & ld patch breaks --as-needed on SPARC (and some other targets)

classic Classic list List threaded Threaded
11 messages Options
Nix
Reply | Threaded
Open this post in threaded view
|

Your bfd & ld patch breaks --as-needed on SPARC (and some other targets)

Nix
Specifically, this patch from late January:

revision 1.2862
date: 2005/01/25 01:39:58;  author: amodra;  state: Exp;  lines: +13 -0
bfd/
        * elflink.c (elf_link_add_object_symbols): Don't create link dynamic
        sections immediately when linking shared libs.  Instead, wait until
        we know a lib is needed.
        (_bfd_elf_link_create_dynstrtab): Extract from..
        (_bfd_elf_link_create_dynamic_sections_): ..here.
        (elf_add_dt_needed_tag): Call _bfd_elf_link_create_dynstrtab and
        _bfd_elf_link_create_dynamic_sections.  Add abfd param.  Allow
        for non-existent .dynamic.
        (elf_link_output_extsym): Don't complain about undefined symbols
        in as-needed dynamic libs that aren't actually linked.
ld/
        * emultempl/elf32.em (gld${EMULATION_NAME}_try_needed): Formatting.
        (gld${EMULATION_NAME}_after_open): Ignore needed libs if they were
        only needed by an as-needed lib that didn't get linked.

makes links against one or more shared libraries (discounting libc) with
--as-needed fail:

cat > hello.c << EOF
#include <stdio.h>

int main() {
        printf("hello world!\n");
}
EOF
gcc -Wl,--as-needed -o hello hello.c -lssl
ld: Disabling relaxation: it will not work with multiple definitions
/usr/lib/../lib/crt1.o:(.plt+0x0): multiple definition of `_PROCEDURE_LINKAGE_TABLE_'
collect2: ld returned 1 exit status

Obviously this error is silly: every shared library can be expected
to have a PLT, and on want_plt_sym targets like SPARC they'll all have
duplicate copies of this symbol. It's pretty trivial to reproduce
the error with _DYNAMIC, too:

nix@amaterasu 525 /tmp% nm -D /usr/lib/libImlib.so | grep ' A '
0003a014 A _DYNAMIC
0003a124 A _GLOBAL_OFFSET_TABLE_
0003a500 A _PROCEDURE_LINKAGE_TABLE_
0003b101 A __bss_start
0003b101 A _edata
0003b120 A _end

nix@amaterasu 526 /tmp% /usr/bin/gcc -B/usr/packages/binutils/sparc-amaterasu/ld -o hello hello.c -L. -lImlib
/usr/lib/../lib/crt1.o:(.dynamic+0x0): multiple definition of `_DYNAMIC'
lt-ld-new: Disabling relaxation: it will not work with multiple definitions
/usr/lib/../lib/crt1.o:(.plt+0x0): multiple definition of `_PROCEDURE_LINKAGE_TABLE_'

And voila, there are multiple references to those symbols in the
library's transitive dependencies:

nix@amaterasu 538 /tmp% ldd /usr/lib/libImlib.so | awk '{print $3;}' | sort -u | xargs nm -D | grep -E '_DYNAMIC|_GLOBAL_OFFSET_TABLE_|_PROCEDURE_LINKAGE_TABLE_'
000248c4 A _DYNAMIC
00024cd0 A _PROCEDURE_LINKAGE_TABLE_
000180ec A _DYNAMIC
000182dc A _PROCEDURE_LINKAGE_TABLE_
000e2378 A _DYNAMIC
000e3760 A _PROCEDURE_LINKAGE_TABLE_
0001eddc A _DYNAMIC
0001f008 A _PROCEDURE_LINKAGE_TABLE_
000413f0 A _DYNAMIC
000417e8 A _PROCEDURE_LINKAGE_TABLE_
0003c86c A _DYNAMIC
0003cfa4 A _PROCEDURE_LINKAGE_TABLE_
00062f28 A _DYNAMIC
00064000 A _PROCEDURE_LINKAGE_TABLE_
00017130 A _DYNAMIC
000174d0 A _GLOBAL_OFFSET_TABLE_
00017214 A _PROCEDURE_LINKAGE_TABLE_
0002172c A _DYNAMIC
00021924 A _PROCEDURE_LINKAGE_TABLE_

__bss_start, _edata and _end seem to not trip this bug. I also can't make
it happen with _GLOBAL_OFFSET_TABLE_.


This is Debian bug #320697 and Ubuntu bug #12822: reports there indicate
that MIPS is broken similarly. I suspect that all want_plt_sym targets
are broken, but haven't tested that yet. (I hope to get to it this
lunchtime, but as this isn't my day job other things must supervene
until then.)


(Let me add my voice to the chorus praising Janis for reghunt. Simple
yet elegant and so, so useful.)

--
`I work in computers so, of course, I'm an expert on everything.'
                                                     --- Simon Rumble
Reply | Threaded
Open this post in threaded view
|

Re: Your bfd & ld patch breaks --as-needed on SPARC (and some other targets)

Alan Modra
I made a followup patch to fix a similar problem with _DYNAMIC.  Looks
like we need to do the same for other syms.

This patch also makes _DYANAMIC and _PROCEDURE_LINKAGE_TABLE_ hidden,
as Hans-Peter Nilsson did for _GLOBAL_OFFSET_TABLE_ on 2004-11-02.
I think this is a good thing to do, but haven't yet done much
testing..  In any case, this change will require another bunch of ld
testsuite updates, because we'll have fewer dynamic symbols.  I won't
apply it until I've also fixed the testsuite, but could you please test
it in the meantime?

        * elflink.c (define_linkage_sym): New function, extracted from..
        (_bfd_elf_create_got_section): ..here.
        (_bfd_elf_link_create_dynamic_sections): Call it for _DYNAMIC.
        (_bfd_elf_create_dynamic_sections): ..and _PROCEDURE_LINKAGE_TABLE_.

Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.187
diff -u -p -r1.187 elflink.c
--- bfd/elflink.c 4 Aug 2005 06:22:06 -0000 1.187
+++ bfd/elflink.c 12 Aug 2005 12:58:39 -0000
@@ -27,13 +27,51 @@
 #include "safe-ctype.h"
 #include "libiberty.h"
 
+/* Define a symbol in a dynamic linkage section.  */
+
+static struct elf_link_hash_entry *
+define_linkage_sym (bfd *abfd,
+    struct bfd_link_info *info,
+    asection *sec,
+    const char *name)
+{
+  struct elf_link_hash_entry *h;
+  struct bfd_link_hash_entry *bh;
+
+  h = elf_link_hash_lookup (elf_hash_table (info), name, FALSE, FALSE, FALSE);
+  if (h != NULL)
+    {
+      /* Zap symbol defined in an as-needed lib that wasn't linked.
+ This is a symptom of a larger problem:  Absolute symbols
+ defined in shared libraries can't be overridden, because we
+ lose the link to the bfd which is via the symbol section.  */
+      h->root.type = bfd_link_hash_new;
+    }
+
+  bh = &h->root;
+  if (!_bfd_generic_link_add_one_symbol (info, abfd, name, BSF_GLOBAL,
+ sec, 0, NULL, FALSE,
+ get_elf_backend_data (abfd)->collect,
+ &bh))
+    return NULL;
+  h = (struct elf_link_hash_entry *) bh;
+  h->def_regular = 1;
+  h->type = STT_OBJECT;
+  h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_HIDDEN;
+
+  if (!info->executable
+      && !bfd_elf_link_record_dynamic_symbol (info, h))
+    return NULL;
+
+  return h;
+}
+
 bfd_boolean
 _bfd_elf_create_got_section (bfd *abfd, struct bfd_link_info *info)
 {
   flagword flags;
   asection *s;
   struct elf_link_hash_entry *h;
-  struct bfd_link_hash_entry *bh;
   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
   int ptralign;
 
@@ -78,21 +116,10 @@ _bfd_elf_create_got_section (bfd *abfd,
  (or .got.plt) section.  We don't do this in the linker script
  because we don't want to define the symbol if we are not creating
  a global offset table.  */
-      bh = NULL;
-      if (!(_bfd_generic_link_add_one_symbol
-    (info, abfd, "_GLOBAL_OFFSET_TABLE_", BSF_GLOBAL, s,
-     0, NULL, FALSE, bed->collect, &bh)))
- return FALSE;
-      h = (struct elf_link_hash_entry *) bh;
-      h->def_regular = 1;
-      h->type = STT_OBJECT;
-      h->other = STV_HIDDEN;
-
-      if (! info->executable
-  && ! bfd_elf_link_record_dynamic_symbol (info, h))
- return FALSE;
-
+      h = define_linkage_sym (abfd, info, s, "_GLOBAL_OFFSET_TABLE_");
       elf_hash_table (info)->hgot = h;
+      if (h == NULL)
+ return FALSE;
     }
 
   /* The first bit of the global offset table is the header.  */
@@ -132,8 +159,6 @@ _bfd_elf_link_create_dynamic_sections (b
 {
   flagword flags;
   register asection *s;
-  struct elf_link_hash_entry *h;
-  struct bfd_link_hash_entry *bh;
   const struct elf_backend_data *bed;
 
   if (! is_elf_hash_table (info->hash))
@@ -212,27 +237,7 @@ _bfd_elf_link_create_dynamic_sections (b
      section.  We don't want to define it if there is no .dynamic
      section, since on some ELF platforms the start up code examines it
      to decide how to initialize the process.  */
-  h = elf_link_hash_lookup (elf_hash_table (info), "_DYNAMIC",
-    FALSE, FALSE, FALSE);
-  if (h != NULL)
-    {
-      /* Zap symbol defined in an as-needed lib that wasn't linked.
- This is a symptom of a larger problem:  Absolute symbols
- defined in shared libraries can't be overridden, because we
- lose the link to the bfd which is via the symbol section.  */
-      h->root.type = bfd_link_hash_new;
-    }
-  bh = &h->root;
-  if (! (_bfd_generic_link_add_one_symbol
- (info, abfd, "_DYNAMIC", BSF_GLOBAL, s, 0, NULL, FALSE,
-  get_elf_backend_data (abfd)->collect, &bh)))
-    return FALSE;
-  h = (struct elf_link_hash_entry *) bh;
-  h->def_regular = 1;
-  h->type = STT_OBJECT;
-
-  if (! info->executable
-      && ! bfd_elf_link_record_dynamic_symbol (info, h))
+  if (!define_linkage_sym (abfd, info, s, "_DYNAMIC"))
     return FALSE;
 
   s = bfd_make_section_with_flags (abfd, ".hash",
@@ -287,18 +292,9 @@ _bfd_elf_create_dynamic_sections (bfd *a
       /* Define the symbol _PROCEDURE_LINKAGE_TABLE_ at the start of the
  .plt section.  */
       struct elf_link_hash_entry *h;
-      struct bfd_link_hash_entry *bh = NULL;
-
-      if (! (_bfd_generic_link_add_one_symbol
-     (info, abfd, "_PROCEDURE_LINKAGE_TABLE_", BSF_GLOBAL, s, 0, NULL,
-      FALSE, get_elf_backend_data (abfd)->collect, &bh)))
- return FALSE;
-      h = (struct elf_link_hash_entry *) bh;
-      h->def_regular = 1;
-      h->type = STT_OBJECT;
 
-      if (! info->executable
-  && ! bfd_elf_link_record_dynamic_symbol (info, h))
+      h = define_linkage_sym (abfd, info, s, "_PROCEDURE_LINKAGE_TABLE_");
+      if (h == NULL)
  return FALSE;
     }
 

--
Alan Modra
IBM OzLabs - Linux Technology Centre
Nix
Reply | Threaded
Open this post in threaded view
|

Re: Your bfd & ld patch breaks --as-needed on SPARC (and some other targets)

Nix
On Fri, 12 Aug 2005, Alan Modra uttered the following:

> I made a followup patch to fix a similar problem with _DYNAMIC.  Looks
> like we need to do the same for other syms.
>
> This patch also makes _DYANAMIC and _PROCEDURE_LINKAGE_TABLE_ hidden,
> as Hans-Peter Nilsson did for _GLOBAL_OFFSET_TABLE_ on 2004-11-02.
> I think this is a good thing to do, but haven't yet done much
> testing..  In any case, this change will require another bunch of ld
> testsuite updates, because we'll have fewer dynamic symbols.  I won't
> apply it until I've also fixed the testsuite, but could you please test
> it in the meantime?

It seems to fix it for me: the _DYNAMIC and _PROCEDURE_LINKAGE_TABLE_
errors both disappear.

(Nice patch, too, cleaning up a bit of duplication...)

I'll update the Debian and Ubuntu bugs when the patch goes in.


Thank you :)

--
`I work in computers so, of course, I'm an expert on everything.'
                                                     --- Simon Rumble
Reply | Threaded
Open this post in threaded view
|

Re: Your bfd & ld patch breaks --as-needed on SPARC (and some other targets)

Alan Modra
Trying again, this time without the ld testsuite changes, since
sources.redhat.com complained that my message exceeded a 40000 line
limit.  I guess no one wants to look at such a diff anyway.  :)

On Fri, Aug 12, 2005 at 03:34:49PM +0100, Nix wrote:
> On Fri, 12 Aug 2005, Alan Modra uttered the following:
> > This patch also makes _DYANAMIC and _PROCEDURE_LINKAGE_TABLE_ hidden,
> > as Hans-Peter Nilsson did for _GLOBAL_OFFSET_TABLE_ on 2004-11-02.
> > I think this is a good thing to do, but haven't yet done much
> > testing..
>
> It seems to fix it for me: the _DYNAMIC and _PROCEDURE_LINKAGE_TABLE_
> errors both disappear.

As threatened.  I haven't done anything about elfxx-mips.c yet, which
probably ought to use _bfd_elf_define_linkage_sym.  Would one of the
mips maintainers please take a look?

There are also some SH64 ld test failures that I haven't investigated.
If I'm to blame, I'm willing to fix, but some analysis from SH people
would be appreciated.  Also, I left one failing cris test, because it
deliberately checks for even .text addresses, but at first glance
nothing seems to guarantee that.

bfd/
        * elf-bfd.h (_bfd_elf_define_linkage_sym): Declare.
        * elflink.c (_bfd_elf_define_linkage_sym): New function, extracted
        from..
        (_bfd_elf_create_got_section): ..here.
        (_bfd_elf_link_create_dynamic_sections): Call it for _DYNAMIC.
        (_bfd_elf_create_dynamic_sections): ..and _PROCEDURE_LINKAGE_TABLE_.
        * elf-m10300.c (_bfd_mn10300_elf_create_got_section): Use
        _bfd_elf_define_linkage_sym.
        * elf32-frv.c (_frv_create_got_section): Likewise.
        * elf64-alpha.c (elf64_alpha_create_dynamic_sections): Likewise.

ld/testsuite/
        * ld-alpha/tlsbin.rd: Adjust for dynamic sym changes.
        * ld-alpha/tlsbinr.rd: Likewise.
        * ld-alpha/tlspic.rd: Likewise.
        * ld-arm/mixed-app.d: Likewise.
        * ld-arm/mixed-app.sym: Likewise.
        * ld-arm/mixed-lib.sym: Likewise.
        * ld-arm/tls-app.d: Likewise.
        * ld-arm/tls-app.r: Likewise.
        * ld-cris/expdyn5.d: Likewise.
        * ld-cris/expdyn6.d: Likewise.
        * ld-cris/expdyn7.d: Likewise.
        * ld-cris/gotplt1.d: Likewise.
        * ld-cris/gotplt2.d: Likewise.
        * ld-cris/gotplt3.d: Likewise.
        * ld-cris/hiddef1.d: Likewise.
        * ld-cris/libdso-10.d: Likewise.
        * ld-cris/libdso-11.d: Likewise.
        * ld-cris/libdso-12.d: Likewise.
        * ld-cris/libdso-13.d: Likewise.
        * ld-cris/libdso-14.d: Likewise.
        * ld-cris/libdso-2.d: Likewise.
        * ld-cris/pv32-1.d: Likewise.
        * ld-cris/weakref2.d: Likewise.
        * ld-frv/fdpic-pie-1.d: Likewise.
        * ld-frv/fdpic-pie-2.d: Likewise.
        * ld-frv/fdpic-pie-6.d: Likewise.
        * ld-frv/fdpic-pie-7.d: Likewise.
        * ld-frv/fdpic-pie-8.d: Likewise.
        * ld-frv/fdpic-shared-1.d: Likewise.
        * ld-frv/fdpic-shared-2.d: Likewise.
        * ld-frv/fdpic-shared-3.d: Likewise.
        * ld-frv/fdpic-shared-4.d: Likewise.
        * ld-frv/fdpic-shared-5.d: Likewise.
        * ld-frv/fdpic-shared-6.d: Likewise.
        * ld-frv/fdpic-shared-7.d: Likewise.
        * ld-frv/fdpic-shared-8.d: Likewise.
        * ld-frv/fdpic-shared-local-2.d: Likewise.
        * ld-frv/fdpic-shared-local-8.d: Likewise.
        * ld-frv/fdpic-static-1.d: Likewise.
        * ld-frv/fdpic-static-2.d: Likewise.
        * ld-frv/fdpic-static-6.d: Likewise.
        * ld-frv/fdpic-static-7.d: Likewise.
        * ld-frv/fdpic-static-8.d: Likewise.
        * ld-frv/tls-dynamic-1.d: Likewise.
        * ld-frv/tls-dynamic-2.d: Likewise.
        * ld-frv/tls-dynamic-3.d: Likewise.
        * ld-frv/tls-initial-shared-2.d: Likewise.
        * ld-frv/tls-pie-1.d: Likewise.
        * ld-frv/tls-pie-3.d: Likewise.
        * ld-frv/tls-relax-dynamic-1.d: Likewise.
        * ld-frv/tls-relax-dynamic-2.d: Likewise.
        * ld-frv/tls-relax-dynamic-3.d: Likewise.
        * ld-frv/tls-relax-initial-shared-2.d: Likewise.
        * ld-frv/tls-relax-pie-1.d: Likewise.
        * ld-frv/tls-relax-pie-3.d: Likewise.
        * ld-frv/tls-relax-shared-1.d: Likewise.
        * ld-frv/tls-relax-shared-2.d: Likewise.
        * ld-frv/tls-relax-shared-3.d: Likewise.
        * ld-frv/tls-relax-static-1.d: Likewise.
        * ld-frv/tls-shared-1.d: Likewise.
        * ld-frv/tls-shared-2.d: Likewise.
        * ld-frv/tls-shared-3.d: Likewise.
        * ld-frv/tls-static-1.d: Likewise.
        * ld-frv/tls-static-3.d: Likewise.
        * ld-i386/tlsbin.rd: Likewise.
        * ld-i386/tlsnopic.rd: Likewise.
        * ld-i386/tlspic.rd: Likewise.
        * ld-ia64/tlsbin.dd: Likewise.
        * ld-ia64/tlsbin.rd: Likewise.
        * ld-ia64/tlspic.dd: Likewise.
        * ld-ia64/tlspic.rd: Likewise.
        * ld-powerpc/tlsexe.g: Likewise.
        * ld-powerpc/tlsexe.r: Likewise.
        * ld-powerpc/tlsexe32.d: Likewise.
        * ld-powerpc/tlsexe32.g: Likewise.
        * ld-powerpc/tlsexe32.r: Likewise.
        * ld-powerpc/tlsexetoc.g: Likewise.
        * ld-powerpc/tlsexetoc.r: Likewise.
        * ld-powerpc/tlsso.g: Likewise.
        * ld-powerpc/tlsso.r: Likewise.
        * ld-powerpc/tlsso32.d: Likewise.
        * ld-powerpc/tlsso32.g: Likewise.
        * ld-powerpc/tlsso32.r: Likewise.
        * ld-powerpc/tlstocso.g: Likewise.
        * ld-powerpc/tlstocso.r: Likewise.
        * ld-s390/tlsbin.rd: Likewise.
        * ld-s390/tlsbin_64.rd: Likewise.
        * ld-s390/tlspic.rd: Likewise.
        * ld-s390/tlspic_64.rd: Likewise.
        * ld-sh/shared-1.d: Likewise.
        * ld-sh/tlsbin-2.d: Likewise.
        * ld-sh/tlsbin-3.d: Likewise.
        * ld-sh/tlsbin-4.d: Likewise.
        * ld-sh/tlspic-2.d: Likewise.
        * ld-sh/sh64/abi32.xd: Likewise.
        * ld-sh/sh64/abi64.xd: Likewise.
        * ld-sh/sh64/cmpct1.xd: Likewise.
        * ld-sh/sh64/crange1.rd: Likewise.
        * ld-sh/sh64/crange2.rd: Likewise.
        * ld-sh/sh64/crange3-cmpct.rd: Likewise.
        * ld-sh/sh64/crange3-media.rd: Likewise.
        * ld-sh/sh64/crange3.rd: Likewise.
        * ld-sh/sh64/gotplt.d: Likewise.
        * ld-sh/sh64/init-cmpct.d: Likewise.
        * ld-sh/sh64/init-media.d: Likewise.
        * ld-sh/sh64/init64.d: Likewise.
        * ld-sh/sh64/mix1.xd: Likewise.
        * ld-sh/sh64/mix2.xd: Likewise.
        * ld-sh/sh64/sh64.exp: Likewise.
        * ld-sh/sh64/shdl32.xd: Likewise.
        * ld-sh/sh64/shdl64.xd: Likewise.
        * ld-sparc/tlssunbin32.rd: Likewise.
        * ld-sparc/tlssunbin64.rd: Likewise.
        * ld-sparc/tlssunnopic32.rd: Likewise.
        * ld-sparc/tlssunnopic64.rd: Likewise.
        * ld-sparc/tlssunpic32.rd: Likewise.
        * ld-sparc/tlssunpic64.rd: Likewise.
        * ld-x86-64/tlsbin.rd: Likewise.
        * ld-x86-64/tlspic.dd: Likewise.
        * ld-x86-64/tlspic.rd: Likewise.

Index: bfd/elf-bfd.h
===================================================================
RCS file: /cvs/src/src/bfd/elf-bfd.h,v
retrieving revision 1.194
diff -u -p -r1.194 elf-bfd.h
--- bfd/elf-bfd.h 29 Jul 2005 02:46:02 -0000 1.194
+++ bfd/elf-bfd.h 15 Aug 2005 13:33:42 -0000
@@ -1639,6 +1639,8 @@ extern bfd_boolean _bfd_elf_create_dynam
   (bfd *, struct bfd_link_info *);
 extern bfd_boolean _bfd_elf_create_got_section
   (bfd *, struct bfd_link_info *);
+extern struct elf_link_hash_entry *_bfd_elf_define_linkage_sym
+  (bfd *, struct bfd_link_info *, asection *, const char *);
 
 extern bfd_boolean _bfd_elfcore_make_pseudosection
   (bfd *, char *, size_t, ufile_ptr);
Index: bfd/elf-m10300.c
===================================================================
RCS file: /cvs/src/src/bfd/elf-m10300.c,v
retrieving revision 1.67
diff -u -p -r1.67 elf-m10300.c
--- bfd/elf-m10300.c 8 Jul 2005 06:19:56 -0000 1.67
+++ bfd/elf-m10300.c 15 Aug 2005 13:33:44 -0000
@@ -542,7 +542,6 @@ _bfd_mn10300_elf_create_got_section (abf
   flagword   flags;
   flagword   pltflags;
   asection * s;
-  struct bfd_link_hash_entry * bh;
   struct elf_link_hash_entry * h;
   const struct elf_backend_data * bed = get_elf_backend_data (abfd);
   int ptralign;
@@ -581,24 +580,12 @@ _bfd_mn10300_elf_create_got_section (abf
       || ! bfd_set_section_alignment (abfd, s, bed->plt_alignment))
     return FALSE;
 
-  if (bed->want_plt_sym)
-    {
-      /* Define the symbol _PROCEDURE_LINKAGE_TABLE_ at the start of the
- .plt section.  */
-      bh = NULL;
-      if (! (_bfd_generic_link_add_one_symbol
-     (info, abfd, "_PROCEDURE_LINKAGE_TABLE_", BSF_GLOBAL, s,
-      (bfd_vma) 0, (const char *) NULL, FALSE,
-      get_elf_backend_data (abfd)->collect, &bh)))
- return FALSE;
-      h = (struct elf_link_hash_entry *) bh;
-      h->def_regular = 1;
-      h->type = STT_OBJECT;
-
-      if (info->shared
-  && ! bfd_elf_link_record_dynamic_symbol (info, h))
- return FALSE;
-    }
+  /* Define the symbol _PROCEDURE_LINKAGE_TABLE_ at the start of the
+     .plt section.  */
+  if (bed->want_plt_sym
+      && !_bfd_elf_define_linkage_sym (abfd, info, s,
+       "_PROCEDURE_LINKAGE_TABLE_"))
+    return FALSE;
 
   s = bfd_make_section_with_flags (abfd, ".got", flags);
   if (s == NULL
@@ -617,20 +604,10 @@ _bfd_mn10300_elf_create_got_section (abf
      (or .got.plt) section.  We don't do this in the linker script
      because we don't want to define the symbol if we are not creating
      a global offset table.  */
-  bh = NULL;
-  if (!(_bfd_generic_link_add_one_symbol
- (info, abfd, "_GLOBAL_OFFSET_TABLE_", BSF_GLOBAL, s,
- 0, (const char *) NULL, FALSE, bed->collect, &bh)))
-    return FALSE;
-  h = (struct elf_link_hash_entry *) bh;
-  h->def_regular = 1;
-  h->type = STT_OBJECT;
-
-  if (info->shared
-      && ! bfd_elf_link_record_dynamic_symbol (info, h))
-    return FALSE;
-
+  h = _bfd_elf_define_linkage_sym (abfd, info, s, "_GLOBAL_OFFSET_TABLE_");
   elf_hash_table (info)->hgot = h;
+  if (h == NULL)
+    return FALSE;
 
   /* The first bit of the global offset table is the header.  */
   s->size += bed->got_header_size;
Index: bfd/elf32-frv.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-frv.c,v
retrieving revision 1.42
diff -u -p -r1.42 elf32-frv.c
--- bfd/elf32-frv.c 20 Jun 2005 18:12:06 -0000 1.42
+++ bfd/elf32-frv.c 15 Aug 2005 13:33:48 -0000
@@ -4301,22 +4301,15 @@ _frv_create_got_section (bfd *abfd, stru
  (or .got.plt) section.  We don't do this in the linker script
  because we don't want to define the symbol if we are not creating
  a global offset table.  */
-      bh = NULL;
-      if (!(_bfd_generic_link_add_one_symbol
-    (info, abfd, "_GLOBAL_OFFSET_TABLE_", BSF_GLOBAL, s,
-     0, (const char *) NULL, FALSE, bed->collect, &bh)))
+      h = _bfd_elf_define_linkage_sym (abfd, info, s, "_GLOBAL_OFFSET_TABLE_");
+      elf_hash_table (info)->hgot = h;
+      if (h == NULL)
  return FALSE;
-      h = (struct elf_link_hash_entry *) bh;
-      h->def_regular = 1;
-      h->type = STT_OBJECT;
-      /* h->other = STV_HIDDEN; */ /* Should we?  */
 
       /* Machine-specific: we want the symbol for executables as
  well.  */
       if (! bfd_elf_link_record_dynamic_symbol (info, h))
  return FALSE;
-
-      elf_hash_table (info)->hgot = h;
     }
 
   /* The first bit of the global offset table is the header.  */
@@ -4399,26 +4392,12 @@ _frv_create_got_section (bfd *abfd, stru
   /* FRV-specific: remember it.  */
   frvfdpic_plt_section (info) = s;
 
-  if (bed->want_plt_sym)
-    {
-      /* Define the symbol _PROCEDURE_LINKAGE_TABLE_ at the start of the
- .plt section.  */
-      struct elf_link_hash_entry *h;
-      struct bfd_link_hash_entry *bh = NULL;
-
-      if (! (_bfd_generic_link_add_one_symbol
-     (info, abfd, "_PROCEDURE_LINKAGE_TABLE_", BSF_GLOBAL, s, 0, NULL,
-      FALSE, get_elf_backend_data (abfd)->collect, &bh)))
- return FALSE;
-      h = (struct elf_link_hash_entry *) bh;
-      h->def_regular = 1;
-      h->type = STT_OBJECT;
-      /* h->other = STV_HIDDEN; */ /* Should we?  */
-
-      if (! info->executable
-  && ! bfd_elf_link_record_dynamic_symbol (info, h))
- return FALSE;
-    }
+  /* Define the symbol _PROCEDURE_LINKAGE_TABLE_ at the start of the
+     .plt section.  */
+  if (bed->want_plt_sym
+      && !_bfd_elf_define_linkage_sym (abfd, info, s,
+       "_PROCEDURE_LINKAGE_TABLE_"))
+    return FALSE;
 
   /* FRV-specific: we want rel relocations for the plt.  */
   s = bfd_make_section_with_flags (abfd, ".rel.plt",
Index: bfd/elf64-alpha.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-alpha.c,v
retrieving revision 1.146
diff -u -p -r1.146 elf64-alpha.c
--- bfd/elf64-alpha.c 8 Jul 2005 06:20:01 -0000 1.146
+++ bfd/elf64-alpha.c 15 Aug 2005 13:33:51 -0000
@@ -1232,7 +1232,6 @@ elf64_alpha_create_dynamic_sections (bfd
   asection *s;
   flagword flags;
   struct elf_link_hash_entry *h;
-  struct bfd_link_hash_entry *bh;
 
   /* We need to create .plt, .rela.plt, .got, and .rela.got sections.  */
 
@@ -1245,17 +1244,8 @@ elf64_alpha_create_dynamic_sections (bfd
 
   /* Define the symbol _PROCEDURE_LINKAGE_TABLE_ at the start of the
      .plt section.  */
-  bh = NULL;
-  if (! (_bfd_generic_link_add_one_symbol
- (info, abfd, "_PROCEDURE_LINKAGE_TABLE_", BSF_GLOBAL, s,
-  (bfd_vma) 0, (const char *) NULL, FALSE,
-  get_elf_backend_data (abfd)->collect, &bh)))
-    return FALSE;
-  h = (struct elf_link_hash_entry *) bh;
-  h->def_regular = 1;
-  h->type = STT_OBJECT;
-
-  if (info->shared && ! bfd_elf_link_record_dynamic_symbol (info, h))
+  if (!_bfd_elf_define_linkage_sym (abfd, info, s,
+    "_PROCEDURE_LINKAGE_TABLE_"))
     return FALSE;
 
   flags = (SEC_ALLOC | SEC_LOAD | SEC_HAS_CONTENTS | SEC_IN_MEMORY
@@ -1292,21 +1282,11 @@ elf64_alpha_create_dynamic_sections (bfd
      dynobj's .got section.  We don't do this in the linker script
      because we don't want to define the symbol if we are not creating
      a global offset table.  */
-  bh = NULL;
-  if (!(_bfd_generic_link_add_one_symbol
- (info, abfd, "_GLOBAL_OFFSET_TABLE_", BSF_GLOBAL,
- alpha_elf_tdata(abfd)->got, (bfd_vma) 0, (const char *) NULL,
- FALSE, get_elf_backend_data (abfd)->collect, &bh)))
-    return FALSE;
-  h = (struct elf_link_hash_entry *) bh;
-  h->def_regular = 1;
-  h->type = STT_OBJECT;
-
-  if (info->shared
-      && ! bfd_elf_link_record_dynamic_symbol (info, h))
-    return FALSE;
-
+  h = _bfd_elf_define_linkage_sym (abfd, info, alpha_elf_tdata(abfd)->got,
+   "_GLOBAL_OFFSET_TABLE_");
   elf_hash_table (info)->hgot = h;
+  if (h == NULL)
+    return FALSE;
 
   return TRUE;
 }
Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.187
diff -u -p -r1.187 elflink.c
--- bfd/elflink.c 4 Aug 2005 06:22:06 -0000 1.187
+++ bfd/elflink.c 15 Aug 2005 13:34:01 -0000
@@ -27,13 +27,51 @@
 #include "safe-ctype.h"
 #include "libiberty.h"
 
+/* Define a symbol in a dynamic linkage section.  */
+
+struct elf_link_hash_entry *
+_bfd_elf_define_linkage_sym (bfd *abfd,
+     struct bfd_link_info *info,
+     asection *sec,
+     const char *name)
+{
+  struct elf_link_hash_entry *h;
+  struct bfd_link_hash_entry *bh;
+
+  h = elf_link_hash_lookup (elf_hash_table (info), name, FALSE, FALSE, FALSE);
+  if (h != NULL)
+    {
+      /* Zap symbol defined in an as-needed lib that wasn't linked.
+ This is a symptom of a larger problem:  Absolute symbols
+ defined in shared libraries can't be overridden, because we
+ lose the link to the bfd which is via the symbol section.  */
+      h->root.type = bfd_link_hash_new;
+    }
+
+  bh = &h->root;
+  if (!_bfd_generic_link_add_one_symbol (info, abfd, name, BSF_GLOBAL,
+ sec, 0, NULL, FALSE,
+ get_elf_backend_data (abfd)->collect,
+ &bh))
+    return NULL;
+  h = (struct elf_link_hash_entry *) bh;
+  h->def_regular = 1;
+  h->type = STT_OBJECT;
+  h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_HIDDEN;
+
+  if (!info->executable
+      && !bfd_elf_link_record_dynamic_symbol (info, h))
+    return NULL;
+
+  return h;
+}
+
 bfd_boolean
 _bfd_elf_create_got_section (bfd *abfd, struct bfd_link_info *info)
 {
   flagword flags;
   asection *s;
   struct elf_link_hash_entry *h;
-  struct bfd_link_hash_entry *bh;
   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
   int ptralign;
 
@@ -78,21 +116,10 @@ _bfd_elf_create_got_section (bfd *abfd,
  (or .got.plt) section.  We don't do this in the linker script
  because we don't want to define the symbol if we are not creating
  a global offset table.  */
-      bh = NULL;
-      if (!(_bfd_generic_link_add_one_symbol
-    (info, abfd, "_GLOBAL_OFFSET_TABLE_", BSF_GLOBAL, s,
-     0, NULL, FALSE, bed->collect, &bh)))
- return FALSE;
-      h = (struct elf_link_hash_entry *) bh;
-      h->def_regular = 1;
-      h->type = STT_OBJECT;
-      h->other = STV_HIDDEN;
-
-      if (! info->executable
-  && ! bfd_elf_link_record_dynamic_symbol (info, h))
- return FALSE;
-
+      h = _bfd_elf_define_linkage_sym (abfd, info, s, "_GLOBAL_OFFSET_TABLE_");
       elf_hash_table (info)->hgot = h;
+      if (h == NULL)
+ return FALSE;
     }
 
   /* The first bit of the global offset table is the header.  */
@@ -132,8 +159,6 @@ _bfd_elf_link_create_dynamic_sections (b
 {
   flagword flags;
   register asection *s;
-  struct elf_link_hash_entry *h;
-  struct bfd_link_hash_entry *bh;
   const struct elf_backend_data *bed;
 
   if (! is_elf_hash_table (info->hash))
@@ -212,27 +237,7 @@ _bfd_elf_link_create_dynamic_sections (b
      section.  We don't want to define it if there is no .dynamic
      section, since on some ELF platforms the start up code examines it
      to decide how to initialize the process.  */
-  h = elf_link_hash_lookup (elf_hash_table (info), "_DYNAMIC",
-    FALSE, FALSE, FALSE);
-  if (h != NULL)
-    {
-      /* Zap symbol defined in an as-needed lib that wasn't linked.
- This is a symptom of a larger problem:  Absolute symbols
- defined in shared libraries can't be overridden, because we
- lose the link to the bfd which is via the symbol section.  */
-      h->root.type = bfd_link_hash_new;
-    }
-  bh = &h->root;
-  if (! (_bfd_generic_link_add_one_symbol
- (info, abfd, "_DYNAMIC", BSF_GLOBAL, s, 0, NULL, FALSE,
-  get_elf_backend_data (abfd)->collect, &bh)))
-    return FALSE;
-  h = (struct elf_link_hash_entry *) bh;
-  h->def_regular = 1;
-  h->type = STT_OBJECT;
-
-  if (! info->executable
-      && ! bfd_elf_link_record_dynamic_symbol (info, h))
+  if (!_bfd_elf_define_linkage_sym (abfd, info, s, "_DYNAMIC"))
     return FALSE;
 
   s = bfd_make_section_with_flags (abfd, ".hash",
@@ -282,25 +287,12 @@ _bfd_elf_create_dynamic_sections (bfd *a
       || ! bfd_set_section_alignment (abfd, s, bed->plt_alignment))
     return FALSE;
 
-  if (bed->want_plt_sym)
-    {
-      /* Define the symbol _PROCEDURE_LINKAGE_TABLE_ at the start of the
- .plt section.  */
-      struct elf_link_hash_entry *h;
-      struct bfd_link_hash_entry *bh = NULL;
-
-      if (! (_bfd_generic_link_add_one_symbol
-     (info, abfd, "_PROCEDURE_LINKAGE_TABLE_", BSF_GLOBAL, s, 0, NULL,
-      FALSE, get_elf_backend_data (abfd)->collect, &bh)))
- return FALSE;
-      h = (struct elf_link_hash_entry *) bh;
-      h->def_regular = 1;
-      h->type = STT_OBJECT;
-
-      if (! info->executable
-  && ! bfd_elf_link_record_dynamic_symbol (info, h))
- return FALSE;
-    }
+  /* Define the symbol _PROCEDURE_LINKAGE_TABLE_ at the start of the
+     .plt section.  */
+  if (bed->want_plt_sym
+      && !_bfd_elf_define_linkage_sym (abfd, info, s,
+       "_PROCEDURE_LINKAGE_TABLE_"))
+    return FALSE;
 
   s = bfd_make_section_with_flags (abfd,
    (bed->default_use_rela_p

--
Alan Modra
IBM OzLabs - Linux Technology Centre
Reply | Threaded
Open this post in threaded view
|

Re: Your bfd & ld patch breaks --as-needed on SPARC (and some other targets)

Hans-Peter Nilsson
> Date: Tue, 16 Aug 2005 01:45:35 +0930
> From: Alan Modra <[hidden email]>

> Also, I left one failing cris test, because it
> deliberately checks for even .text addresses, but at first glance
> nothing seems to guarantee that.

Right, it's a missing a ".p2align 1" in dso-1.s and has worked
by accident before.  I guess this is caused by _DYNAMIC\0
(odd-length string) now gone from .dynsym?

(JFTR, my autotester alerted me.  I actually had to wait a bit
for the binutils@ message that I'm now replying to. :-)

brgds, H-P
Nix
Reply | Threaded
Open this post in threaded view
|

Re: Your bfd & ld patch breaks --as-needed on SPARC (and some other targets)

Nix
In reply to this post by Alan Modra
On 15 Aug 2005, Alan Modra spake:
> Trying again, this time without the ld testsuite changes, since
> sources.redhat.com complained that my message exceeded a 40000 line
> limit.

Odd. Gnus says it was 13702 lines long... still a >500K patch,
though. Maybe its real limit is 512K or something?

>        I guess no one wants to look at such a diff anyway.  :)

I looked in some horror at the volume of changes you'd had to make.
I had no idea I was lumbering you with a monster of this magnitude!

--
`I work in computers so, of course, I'm an expert on everything.'
                                                     --- Simon Rumble
Reply | Threaded
Open this post in threaded view
|

Re: Your bfd & ld patch breaks --as-needed on SPARC (and some other targets)

Alan Modra
In reply to this post by Hans-Peter Nilsson
On Mon, Aug 15, 2005 at 06:25:36PM +0200, Hans-Peter Nilsson wrote:
> Right, it's a missing a ".p2align 1" in dso-1.s and has worked
> by accident before.  I guess this is caused by _DYNAMIC\0
> (odd-length string) now gone from .dynsym?

Exactly.

--
Alan Modra
IBM OzLabs - Linux Technology Centre
Reply | Threaded
Open this post in threaded view
|

Re: Your bfd & ld patch breaks --as-needed on SPARC (and some other targets)

Kaz Kojima
In reply to this post by Alan Modra
Alan Modra <[hidden email]> wrote:
> There are also some SH64 ld test failures that I haven't investigated.
> If I'm to blame, I'm willing to fix, but some analysis from SH people
> would be appreciated.

I've got 3 ld test failures for sh64-unknown-elf.

FAIL: Mixing SH64 assembler-generated with linker-generated .cranges, partial linking
FAIL: Sorted SH64 .cranges, entry at SHcompact code
FAIL: Sorted SH64 .cranges, entry at SHmedia code

It seems that the first failure is caused with the wrong ref_count for
.cranges section and the latter 2 errors are caused with the wrong rawsize
of .cranges.  I'll check the attached patch in to fix them.

Regards,
        kaz
-
2005-08-17  Kaz Kojima  <[hidden email]>

        * emultempl/sh64elf.em (sh64_elf_${EMULATION_NAME}_after_allocation):
        Don't increment rel_count of .cranges here.  Set rawsize of .cranges.

diff -uprN ORIG/src/ld/emultempl/sh64elf.em LOCAL/src/ld/emultempl/sh64elf.em
--- ORIG/src/ld/emultempl/sh64elf.em 2005-05-17 08:29:12.000000000 +0900
+++ LOCAL/src/ld/emultempl/sh64elf.em 2005-08-17 10:15:59.000000000 +0900
@@ -520,13 +520,6 @@ sh64_elf_${EMULATION_NAME}_after_allocat
  bfd_put_32 (output_bfd, isec->output_offset,
     crangesp + SH64_CRANGE_CR_ADDR_OFFSET);
  cr_addr_order->u.reloc.p->addend = 0;
-
- /* We must update the number of relocations here,
-   since the elf linker does not take link orders
-   into account when setting header sizes.  The
-   actual relocation orders are however executed
-   correctly.  */
- elf_section_data(cranges)->rel_count++;
       }
     else
       bfd_put_32 (output_bfd,
@@ -564,4 +557,5 @@ sh64_elf_${EMULATION_NAME}_after_allocat
   sh64_elf_section_data (cranges)->sh64_info->cranges_growth
     = crangesp - cranges->contents - cranges->size;
   cranges->size = crangesp - cranges->contents;
+  cranges->rawsize = cranges->size;
 }
Reply | Threaded
Open this post in threaded view
|

Re: Your bfd & ld patch breaks --as-needed on SPARC (and some other targets)

Alan Modra
Hi Kaz,

On Wed, Aug 17, 2005 at 11:56:20AM +0900, Kaz Kojima wrote:
> It seems that the first failure is caused with the wrong ref_count for
> .cranges section and the latter 2 errors are caused with the wrong rawsize
> of .cranges.  I'll check the attached patch in to fix them.

Thanks for looking into this.  I'm curious as to why you need to set
rawsize, as there might be a generic bug somewhere.  Care to explain?

--
Alan Modra
IBM OzLabs - Linux Technology Centre
Reply | Threaded
Open this post in threaded view
|

Re: Your bfd & ld patch breaks --as-needed on SPARC (and some other targets)

Kaz Kojima
Alan Modra <[hidden email]> wrote:
> Thanks for looking into this.  I'm curious as to why you need to set
> rawsize, as there might be a generic bug somewhere.  Care to explain?

I don't look at it deeply.  There is a comment before the code in
problem (emultempl/sh64elf.em: 550):

  /* The .cranges section will have this size, no larger or smaller.
     Since relocs (if relocatable linking) will be emitted into the
     "extended" size, we must set the raw size to the total.  We have to
     keep track of the number of new .cranges entries.

     Sorting before writing is done by sh64_elf_final_write_processing.  */

In the erroneous case, the size and the rawsize of .cranges are
the different positive numbers and bfd_malloc_and_get_section
returns the data which is correct only up to rawsize bytes.  So I
thought that we must set the rawsize to the total size there,
though I could be wrong about this.

Regards,
        kaz
Reply | Threaded
Open this post in threaded view
|

Re: Your bfd & ld patch breaks --as-needed on SPARC (and some other targets)

Alan Modra
On Wed, Aug 17, 2005 at 01:09:17PM +0900, Kaz Kojima wrote:
> In the erroneous case, the size and the rawsize of .cranges are
> the different positive numbers and bfd_malloc_and_get_section
> returns the data which is correct only up to rawsize bytes.  So I
> thought that we must set the rawsize to the total size there,
> though I could be wrong about this.

Yes, or set rawsize to zero, either of which would work.  My original
intent with the bfd_malloc_and_get_section logic was to support
re-reading of section contents from disk, where "rawsize" would stay as
the original on disk size, and "size" be that after relaxation.  The
idea didn't quite work out though, since multiple relax passes copy
"size" to "rawsize", which is useful to some of the backend relaxation
functions.

--
Alan Modra
IBM OzLabs - Linux Technology Centre