PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

classic Classic list List threaded Threaded
30 messages Options
12
Reply | Threaded
Open this post in threaded view
|

PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

H.J. Lu-27
We should treat global symbols in PIE like shared library. There is
no regression on Linux/x86/x86-64/ia64.


H.J.
----
2006-01-26  H.J. Lu  <[hidden email]>

        PR ld/2218
        * elflink.c (elf_link_add_object_symbols): Mark global symbol
        in shared library dynamic.

--- bfd/elflink.c.pie 2006-01-25 09:02:12.000000000 -0800
+++ bfd/elflink.c 2006-01-26 21:42:37.000000000 -0800
@@ -4050,7 +4050,7 @@ elf_link_add_object_symbols (bfd *abfd,
  }
       else
  h->def_regular = 1;
-      if (! info->executable
+      if (info->shared
   || h->def_dynamic
   || h->ref_dynamic)
  dynsym = TRUE;
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

Alan Modra
On Thu, Jan 26, 2006 at 11:03:35PM -0800, H. J. Lu wrote:
> We should treat global symbols in PIE like shared library.

Perhaps.  But please explain why the place you are patching is the
correct place to fix a problem with weak syms.  I don't think your patch
is correct.

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

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

H.J. Lu-27
In reply to this post by H.J. Lu-27
On Thu, Jan 26, 2006 at 11:03:35PM -0800, H. J. Lu wrote:
> We should treat global symbols in PIE like shared library. There is
> no regression on Linux/x86/x86-64/ia64.
>

Here is a patch for testcase.

H.J.
----
2006-01-27  H.J. Lu  <[hidden email]>

        PR ld/2218
        * ld-pie/pie.exp: New file.
        * ld-pie/weakundef.c: Likewise.
        * ld-pie/weakundef.out: Likewise.

        * lib/ld-lib.exp (run_ld_link_exec_tests): Fix nesting. Support
        building PIE and shared library.

--- ld/testsuite/ld-pie/pie.exp.pie 2006-01-26 22:27:43.000000000 -0800
+++ ld/testsuite/ld-pie/pie.exp 2006-01-26 22:51:37.000000000 -0800
@@ -0,0 +1,31 @@
+# Expect script for various PIE tests.
+#   Copyright 2006 Free Software Foundation, Inc.
+#
+# This file 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 2 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.
+#
+
+# This test can only be run if ld generates native executables.
+if ![isnative] then {return}
+
+# Run on Linux only.
+if { ![istarget *-*-linux*] } {
+    return
+}
+
+set array_tests {
+    {"weak undefined" "-pie" "" {weakundef.c} "weakundef" "weakundef.out" "-fPIC"}
+}
+
+run_ld_link_exec_tests [] $array_tests
--- ld/testsuite/ld-pie/weakundef.c.pie 2006-01-26 22:27:49.000000000 -0800
+++ ld/testsuite/ld-pie/weakundef.c 2006-01-26 22:39:51.000000000 -0800
@@ -0,0 +1,15 @@
+#include <stdio.h>
+
+#pragma weak undef_func
+
+extern int undef_func (void);
+int (*ptr_to_func)(void) = undef_func;
+
+int
+main (void)
+{
+  if (ptr_to_func == NULL)
+    printf ("PASSED\n");
+
+  return 0;
+}
--- ld/testsuite/ld-pie/weakundef.out.pie 2006-01-26 22:40:33.000000000 -0800
+++ ld/testsuite/ld-pie/weakundef.out 2006-01-26 22:39:51.000000000 -0800
@@ -0,0 +1 @@
+PASSED
--- ld/testsuite/lib/ld-lib.exp.pie 2005-08-24 08:27:20.000000000 -0700
+++ ld/testsuite/lib/ld-lib.exp 2006-01-27 06:13:32.000000000 -0800
@@ -1313,33 +1313,44 @@ proc run_ld_link_exec_tests { targets_to
     set objfile "tmpdir/[file rootname $src_file].o"
     lappend objfiles $objfile
 
- # We ignore warnings since some compilers may generate
- # incorrect section attributes and the assembler will warn
- # them.
- ld_compile "$CC -c $CFLAGS $cflags" $srcdir/$subdir/$src_file $objfile
+    # We ignore warnings since some compilers may generate
+    # incorrect section attributes and the assembler will warn
+    # them.
+    ld_compile "$CC -c $CFLAGS $cflags" $srcdir/$subdir/$src_file $objfile
 
- if ![ld_link $ld $binfile "-L$srcdir/$subdir $ld_options $objfiles"] {
-    set failed 1
- } else {
-    set failed 0
-    send_log "Running: $binfile > $binfile.out\n"
-    verbose "Running: $binfile > $binfile.out"
-    catch "exec $binfile > $binfile.out" exec_output
-    
-    if ![string match "" $exec_output] then {
- send_log "$exec_output\n"
- verbose "$exec_output" 1
- set failed 1
+    # We have to use $CC to build PIE and shared library.
+    if { [ string match "-shared" $ld_options ] \
+ || [ string match "-pie" $ld_options ] } {
+ set link_proc ld_simple_link
+ set link_cmd $CC
     } else {
- send_log "diff $binfile.out $srcdir/$subdir/$expfile\n"
- verbose "diff $binfile.out $srcdir/$subdir/$expfile"
- catch "exec diff $binfile.out $srcdir/$subdir/$expfile" exec_output
- set exec_output [prune_warnings $exec_output]
+ set link_proc ld_link
+ set link_cmd $ld
+    }
 
+    if ![$link_proc $link_cmd $binfile "-L$srcdir/$subdir $ld_options $objfiles"] {
+ set failed 1
+    } else {
+ set failed 0
+ send_log "Running: $binfile > $binfile.out\n"
+ verbose "Running: $binfile > $binfile.out"
+ catch "exec $binfile > $binfile.out" exec_output
+    
  if ![string match "" $exec_output] then {
     send_log "$exec_output\n"
     verbose "$exec_output" 1
     set failed 1
+ } else {
+    send_log "diff $binfile.out $srcdir/$subdir/$expfile\n"
+    verbose "diff $binfile.out $srcdir/$subdir/$expfile"
+    catch "exec diff $binfile.out $srcdir/$subdir/$expfile" exec_output
+    set exec_output [prune_warnings $exec_output]
+
+    if ![string match "" $exec_output] then {
+ send_log "$exec_output\n"
+ verbose "$exec_output" 1
+ set failed 1
+    }
  }
     }
 
@@ -1348,7 +1359,7 @@ proc run_ld_link_exec_tests { targets_to
     } else {
  set errcnt 0
  pass $testname
-    } }
+    }
  }
     }
 }
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

H.J. Lu-27
In reply to this post by Alan Modra
On Fri, Jan 27, 2006 at 08:34:51PM +1030, Alan Modra wrote:
> On Thu, Jan 26, 2006 at 11:03:35PM -0800, H. J. Lu wrote:
> > We should treat global symbols in PIE like shared library.
>
> Perhaps.  But please explain why the place you are patching is the
> correct place to fix a problem with weak syms.  I don't think your patch
> is correct.

PIE is like DSO when the undefined weak symbols are concerned. They
have to be dynamic symbols so that ld.so can handle them properly. I
will check in my testcase shortly so that you can check it out.


H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

H.J. Lu-27
On Fri, Jan 27, 2006 at 06:28:06AM -0800, H. J. Lu wrote:

> On Fri, Jan 27, 2006 at 08:34:51PM +1030, Alan Modra wrote:
> > On Thu, Jan 26, 2006 at 11:03:35PM -0800, H. J. Lu wrote:
> > > We should treat global symbols in PIE like shared library.
> >
> > Perhaps.  But please explain why the place you are patching is the
> > correct place to fix a problem with weak syms.  I don't think your patch
> > is correct.
>
> PIE is like DSO when the undefined weak symbols are concerned. They
> have to be dynamic symbols so that ld.so can handle them properly. I
> will check in my testcase shortly so that you can check it out.
>

The patch probably is incorrect. I think we have an issue with dynamic
symbols in PIE:

bash-3.00$ cat x.c
main (){ }
bash-3.00$ gcc -c x.c
bash-3.00$ gcc x.o
bash-3.00$ readelf -Ds a.out

Symbol table for image:
  Num Buc:    Value  Size   Type   Bind Vis      Ndx Name
    4   0: 00000000     0  NOTYPE   WEAK DEFAULT UND __gmon_start__
    3   0: 00000000     0  NOTYPE   WEAK DEFAULT UND _Jv_RegisterClasses
    1   1: 00000000   221    FUNC GLOBAL DEFAULT UND __libc_start_main
    2   2: 08048430     4  OBJECT GLOBAL DEFAULT  14 _IO_stdin_used
bash-3.00$ gcc x.o -pie
bash-3.00$ readelf -Ds a.out

Symbol table for image:
  Num Buc:    Value  Size   Type   Bind Vis      Ndx Name
   16   1: 000017c0     0  NOTYPE GLOBAL DEFAULT ABS _edata
   15   1: 00000000   231    FUNC   WEAK DEFAULT UND __cxa_finalize
   19   3: 00000000     0  NOTYPE   WEAK DEFAULT UND _Jv_RegisterClasses
   14   7: 00000000   221    FUNC GLOBAL DEFAULT UND __libc_start_main
   13  10: 0000059c    30    FUNC GLOBAL DEFAULT  12 main
   12  11: 000017c0     0  NOTYPE GLOBAL DEFAULT ABS __bss_start
   18  13: 000006ac     4  OBJECT GLOBAL DEFAULT  14 _IO_stdin_used
   17  13: 000017c4     0  NOTYPE GLOBAL DEFAULT ABS _end
   20  15: 00000000     0  NOTYPE   WEAK DEFAULT UND __gmon_start__
bash-3.00$

We are putting more symbols into dynamic symbol table in PIE without
marking them hidden. That means we export more symbols in PIE. I will
see what I can do.


H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

H.J. Lu-27
On Fri, Jan 27, 2006 at 07:07:27AM -0800, H. J. Lu wrote:

> On Fri, Jan 27, 2006 at 06:28:06AM -0800, H. J. Lu wrote:
> > On Fri, Jan 27, 2006 at 08:34:51PM +1030, Alan Modra wrote:
> > > On Thu, Jan 26, 2006 at 11:03:35PM -0800, H. J. Lu wrote:
> > > > We should treat global symbols in PIE like shared library.
> > >
> > > Perhaps.  But please explain why the place you are patching is the
> > > correct place to fix a problem with weak syms.  I don't think your patch
> > > is correct.
> >
> > PIE is like DSO when the undefined weak symbols are concerned. They
> > have to be dynamic symbols so that ld.so can handle them properly. I
> > will check in my testcase shortly so that you can check it out.
> >
>
> The patch probably is incorrect. I think we have an issue with dynamic
> symbols in PIE:
>
> bash-3.00$ cat x.c
> main (){ }
> bash-3.00$ gcc -c x.c
> bash-3.00$ gcc x.o
> bash-3.00$ readelf -Ds a.out
>
> Symbol table for image:
>   Num Buc:    Value  Size   Type   Bind Vis      Ndx Name
>     4   0: 00000000     0  NOTYPE   WEAK DEFAULT UND __gmon_start__
>     3   0: 00000000     0  NOTYPE   WEAK DEFAULT UND _Jv_RegisterClasses
>     1   1: 00000000   221    FUNC GLOBAL DEFAULT UND __libc_start_main
>     2   2: 08048430     4  OBJECT GLOBAL DEFAULT  14 _IO_stdin_used
> bash-3.00$ gcc x.o -pie
> bash-3.00$ readelf -Ds a.out
>
> Symbol table for image:
>   Num Buc:    Value  Size   Type   Bind Vis      Ndx Name
>    16   1: 000017c0     0  NOTYPE GLOBAL DEFAULT ABS _edata
>    15   1: 00000000   231    FUNC   WEAK DEFAULT UND __cxa_finalize
>    19   3: 00000000     0  NOTYPE   WEAK DEFAULT UND _Jv_RegisterClasses
>    14   7: 00000000   221    FUNC GLOBAL DEFAULT UND __libc_start_main
>    13  10: 0000059c    30    FUNC GLOBAL DEFAULT  12 main
>    12  11: 000017c0     0  NOTYPE GLOBAL DEFAULT ABS __bss_start
>    18  13: 000006ac     4  OBJECT GLOBAL DEFAULT  14 _IO_stdin_used
>    17  13: 000017c4     0  NOTYPE GLOBAL DEFAULT ABS _end
>    20  15: 00000000     0  NOTYPE   WEAK DEFAULT UND __gmon_start__
> bash-3.00$
>
> We are putting more symbols into dynamic symbol table in PIE without
> marking them hidden. That means we export more symbols in PIE. I will
> see what I can do.
>

Here is the updated patch.


H.J.
----
2006-01-27  H.J. Lu  <[hidden email]>

        PR ld/2218
        * elflink.c (elf_link_add_object_symbols): Mark global symbol
        in shared library dynamic.
        (elf_link_output_extsym): Mark a symbol in PIE hidden when
        appropriate.

--- bfd/elflink.c.pie 2006-01-27 06:53:51.000000000 -0800
+++ bfd/elflink.c 2006-01-27 09:37:15.000000000 -0800
@@ -4050,7 +4050,12 @@ elf_link_add_object_symbols (bfd *abfd,
  }
       else
  h->def_regular = 1;
-      if (! info->executable
+
+      /* The global symbols in DSO and PIE are put in dynamic
+ symbol table by default.  We will mark the ones in PIE
+ as hidden in elf_link_output_extsym if they aren't
+ defined in nor referenced by any dynamic object.  */
+      if (info->shared
   || h->def_dynamic
   || h->ref_dynamic)
  dynsym = TRUE;
@@ -6487,6 +6492,15 @@ elf_link_output_extsym (struct elf_link_
   else
     sym.st_info = ELF_ST_INFO (STB_GLOBAL, h->type);
 
+  /* If we don't export all symbols into the dynamic symbol table, a
+     symbol in PIE, which isn't defined in nor referenced by any
+     dynamic object, has the hidden visibility.  */
+  if (finfo->info->pie
+      && !finfo->info->export_dynamic
+      && !h->def_dynamic
+      && !h->ref_dynamic)
+    sym.st_other = STV_HIDDEN | (h->other & ~ ELF_ST_VISIBILITY (-1));
+
   switch (h->root.type)
     {
     default:
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

H.J. Lu-27
On Fri, Jan 27, 2006 at 09:41:56AM -0800, H. J. Lu wrote:

> On Fri, Jan 27, 2006 at 07:07:27AM -0800, H. J. Lu wrote:
> > On Fri, Jan 27, 2006 at 06:28:06AM -0800, H. J. Lu wrote:
> > > On Fri, Jan 27, 2006 at 08:34:51PM +1030, Alan Modra wrote:
> > > > On Thu, Jan 26, 2006 at 11:03:35PM -0800, H. J. Lu wrote:
> > > > > We should treat global symbols in PIE like shared library.
> > > >
> > > > Perhaps.  But please explain why the place you are patching is the
> > > > correct place to fix a problem with weak syms.  I don't think your patch
> > > > is correct.
> > >
> > > PIE is like DSO when the undefined weak symbols are concerned. They
> > > have to be dynamic symbols so that ld.so can handle them properly. I
> > > will check in my testcase shortly so that you can check it out.
> > >
> >
> > The patch probably is incorrect. I think we have an issue with dynamic
> > symbols in PIE:
> >
> > bash-3.00$ cat x.c
> > main (){ }
> > bash-3.00$ gcc -c x.c
> > bash-3.00$ gcc x.o
> > bash-3.00$ readelf -Ds a.out
> >
> > Symbol table for image:
> >   Num Buc:    Value  Size   Type   Bind Vis      Ndx Name
> >     4   0: 00000000     0  NOTYPE   WEAK DEFAULT UND __gmon_start__
> >     3   0: 00000000     0  NOTYPE   WEAK DEFAULT UND _Jv_RegisterClasses
> >     1   1: 00000000   221    FUNC GLOBAL DEFAULT UND __libc_start_main
> >     2   2: 08048430     4  OBJECT GLOBAL DEFAULT  14 _IO_stdin_used
> > bash-3.00$ gcc x.o -pie
> > bash-3.00$ readelf -Ds a.out
> >
> > Symbol table for image:
> >   Num Buc:    Value  Size   Type   Bind Vis      Ndx Name
> >    16   1: 000017c0     0  NOTYPE GLOBAL DEFAULT ABS _edata
> >    15   1: 00000000   231    FUNC   WEAK DEFAULT UND __cxa_finalize
> >    19   3: 00000000     0  NOTYPE   WEAK DEFAULT UND _Jv_RegisterClasses
> >    14   7: 00000000   221    FUNC GLOBAL DEFAULT UND __libc_start_main
> >    13  10: 0000059c    30    FUNC GLOBAL DEFAULT  12 main
> >    12  11: 000017c0     0  NOTYPE GLOBAL DEFAULT ABS __bss_start
> >    18  13: 000006ac     4  OBJECT GLOBAL DEFAULT  14 _IO_stdin_used
> >    17  13: 000017c4     0  NOTYPE GLOBAL DEFAULT ABS _end
> >    20  15: 00000000     0  NOTYPE   WEAK DEFAULT UND __gmon_start__
> > bash-3.00$
> >
> > We are putting more symbols into dynamic symbol table in PIE without
> > marking them hidden. That means we export more symbols in PIE. I will
> > see what I can do.
> >
>
> Here is the updated patch.
>
>

A slight update. If a symbol in PIE is defined or not referenced, we
don't need to put it in dynamic symbol table.


H.J.
----
2006-01-27  H.J. Lu  <[hidden email]>

        PR ld/2218
        * elflink.c (elf_link_add_object_symbols): Put undefined,
        referenced symbol in PIE into dynamic symbol table.
        (elf_link_output_extsym): Mark a symbol in PIE hidden if
        it isn't defined in nor referenced by any dynamic object.

--- bfd/elflink.c.pie 2006-01-27 06:53:51.000000000 -0800
+++ bfd/elflink.c 2006-01-27 15:37:07.000000000 -0800
@@ -4050,7 +4050,14 @@ elf_link_add_object_symbols (bfd *abfd,
  }
       else
  h->def_regular = 1;
+
+      /* If an undefined symbol in PIE is referenced by a
+ non-shared object, we put it in dynamic symbol table.
+ We will mark it as hidden in elf_link_output_extsym
+ if it isn't defined in nor referenced by any dynamic
+ object.  */
       if (! info->executable
+  || (info->pie && !h->def_regular && h->ref_regular)
   || h->def_dynamic
   || h->ref_dynamic)
  dynsym = TRUE;
@@ -6487,6 +6494,15 @@ elf_link_output_extsym (struct elf_link_
   else
     sym.st_info = ELF_ST_INFO (STB_GLOBAL, h->type);
 
+  /* If we don't export all symbols into the dynamic symbol table, a
+     symbol in PIE, which isn't defined in nor referenced by any
+     dynamic object, has the hidden visibility.  */
+  if (finfo->info->pie
+      && !finfo->info->export_dynamic
+      && !h->def_dynamic
+      && !h->ref_dynamic)
+    sym.st_other = STV_HIDDEN | (h->other & ~ ELF_ST_VISIBILITY (-1));
+
   switch (h->root.type)
     {
     default:
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

H.J. Lu-27
Could someone please take a look at

http://sourceware.org/ml/binutils/2006-01/msg00224.html

Thanks.


H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

Alan Modra
In reply to this post by H.J. Lu-27
On Fri, Jan 27, 2006 at 03:41:25PM -0800, H. J. Lu wrote:

> On Fri, Jan 27, 2006 at 09:41:56AM -0800, H. J. Lu wrote:
> > On Fri, Jan 27, 2006 at 07:07:27AM -0800, H. J. Lu wrote:
> > > On Fri, Jan 27, 2006 at 06:28:06AM -0800, H. J. Lu wrote:
> > > > On Fri, Jan 27, 2006 at 08:34:51PM +1030, Alan Modra wrote:
> > > > > On Thu, Jan 26, 2006 at 11:03:35PM -0800, H. J. Lu wrote:
> > > > > > We should treat global symbols in PIE like shared library.
> > > > >
> > > > > Perhaps.  But please explain why the place you are patching is the
> > > > > correct place to fix a problem with weak syms.  I don't think your patch
> > > > > is correct.
> > > >
> > > > PIE is like DSO when the undefined weak symbols are concerned. They
> > > > have to be dynamic symbols so that ld.so can handle them properly. I
> > > > will check in my testcase shortly so that you can check it out.

You didn't answer my question.  I asked why you were patching that
particular place to fix undefined weak symbols in PIEs.  See the patch
I've just committed for powerpc64.

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

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

H.J. Lu-27
On Fri, Feb 17, 2006 at 03:24:13PM +1030, Alan Modra wrote:

> On Fri, Jan 27, 2006 at 03:41:25PM -0800, H. J. Lu wrote:
> > On Fri, Jan 27, 2006 at 09:41:56AM -0800, H. J. Lu wrote:
> > > On Fri, Jan 27, 2006 at 07:07:27AM -0800, H. J. Lu wrote:
> > > > On Fri, Jan 27, 2006 at 06:28:06AM -0800, H. J. Lu wrote:
> > > > > On Fri, Jan 27, 2006 at 08:34:51PM +1030, Alan Modra wrote:
> > > > > > On Thu, Jan 26, 2006 at 11:03:35PM -0800, H. J. Lu wrote:
> > > > > > > We should treat global symbols in PIE like shared library.
> > > > > >
> > > > > > Perhaps.  But please explain why the place you are patching is the
> > > > > > correct place to fix a problem with weak syms.  I don't think your patch
> > > > > > is correct.
> > > > >
> > > > > PIE is like DSO when the undefined weak symbols are concerned. They
> > > > > have to be dynamic symbols so that ld.so can handle them properly. I
> > > > > will check in my testcase shortly so that you can check it out.
>
> You didn't answer my question.  I asked why you were patching that
> particular place to fix undefined weak symbols in PIEs.  See the patch
> I've just committed for powerpc64.

It is a generic ELF problem. Why does each backend have to handle it
separately?


H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

Alan Modra
On Thu, Feb 16, 2006 at 10:14:31PM -0800, H. J. Lu wrote:
> On Fri, Feb 17, 2006 at 03:24:13PM +1030, Alan Modra wrote:
> > You didn't answer my question.  I asked why you were patching that
> > particular place to fix undefined weak symbols in PIEs.  See the patch
> > I've just committed for powerpc64.
>
> It is a generic ELF problem. Why does each backend have to handle it
> separately?

Well, if you are clever you might be able to make a generic patch that
handles undef weak in pie.  The right place to do it is after all
symbols have been loaded.

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

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

H.J. Lu-27
On Fri, Feb 17, 2006 at 05:15:06PM +1030, Alan Modra wrote:

> On Thu, Feb 16, 2006 at 10:14:31PM -0800, H. J. Lu wrote:
> > On Fri, Feb 17, 2006 at 03:24:13PM +1030, Alan Modra wrote:
> > > You didn't answer my question.  I asked why you were patching that
> > > particular place to fix undefined weak symbols in PIEs.  See the patch
> > > I've just committed for powerpc64.
> >
> > It is a generic ELF problem. Why does each backend have to handle it
> > separately?
>
> Well, if you are clever you might be able to make a generic patch that
> handles undef weak in pie.  The right place to do it is after all
> symbols have been loaded.
>

Thanks for the pointer. Here is the updated patch.


H.J.
-----
2006-02-17  H.J. Lu  <[hidden email]>

        PR ld/2218
        * elflink.c (_bfd_elf_fix_symbol_flags): Make weak undefined
        symbols in PIE dynamic.
        (elf_link_output_extsym): Mark a symbol in PIE hidden if
        it isn't defined in nor referenced by any dynamic object.

--- bfd/elflink.c.pie 2006-02-15 14:02:41.000000000 -0800
+++ bfd/elflink.c 2006-02-17 06:42:26.000000000 -0800
@@ -2255,6 +2255,16 @@ _bfd_elf_fix_symbol_flags (struct elf_li
  h->def_regular = 1;
     }
 
+  /* Weak undefined symbols in PIE have to be dynamic.  */
+  if (eif->info->pie
+      && h->dynindx == -1
+      && h->root.type == bfd_link_hash_undefweak
+      && ! bfd_elf_link_record_dynamic_symbol (eif->info, h))
+    {
+      eif->failed = TRUE;
+      return FALSE;
+    }
+
   /* If this is a final link, and the symbol was defined as a common
      symbol in a regular object file, and there was no definition in
      any dynamic object, then the linker will have allocated space for
@@ -6443,6 +6453,15 @@ elf_link_output_extsym (struct elf_link_
   else
     sym.st_info = ELF_ST_INFO (STB_GLOBAL, h->type);
 
+  /* If we don't export all symbols into the dynamic symbol table, a
+     symbol in PIE, which isn't defined in nor referenced by any
+     dynamic object, has the hidden visibility.  */
+  if (finfo->info->pie
+      && !finfo->info->export_dynamic
+      && !h->def_dynamic
+      && !h->ref_dynamic)
+    sym.st_other = STV_HIDDEN | (h->other & ~ ELF_ST_VISIBILITY (-1));
+
   switch (h->root.type)
     {
     default:
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

H.J. Lu-27
On Fri, Feb 17, 2006 at 06:46:56AM -0800, H. J. Lu wrote:

> On Fri, Feb 17, 2006 at 05:15:06PM +1030, Alan Modra wrote:
> > On Thu, Feb 16, 2006 at 10:14:31PM -0800, H. J. Lu wrote:
> > > On Fri, Feb 17, 2006 at 03:24:13PM +1030, Alan Modra wrote:
> > > > You didn't answer my question.  I asked why you were patching that
> > > > particular place to fix undefined weak symbols in PIEs.  See the patch
> > > > I've just committed for powerpc64.
> > >
> > > It is a generic ELF problem. Why does each backend have to handle it
> > > separately?
> >
> > Well, if you are clever you might be able to make a generic patch that
> > handles undef weak in pie.  The right place to do it is after all
> > symbols have been loaded.
> >
>
> Thanks for the pointer. Here is the updated patch.
>
>

I added the h->ref_regular check. It may not be strictly necessary
since it isn't easy to generate an unreferenced weak undefined
symbol :-(.


H.J.
---
2006-02-17  H.J. Lu  <[hidden email]>

        PR ld/2218
        * elflink.c (_bfd_elf_fix_symbol_flags): Make weak undefined
        symbols in PIE dynamic.
        (elf_link_output_extsym): Mark a symbol in PIE hidden if
        it isn't defined in nor referenced by any dynamic object.

--- bfd/elflink.c.pie 2006-02-17 07:39:52.000000000 -0800
+++ bfd/elflink.c 2006-02-17 07:44:40.000000000 -0800
@@ -2255,6 +2255,17 @@ _bfd_elf_fix_symbol_flags (struct elf_li
  h->def_regular = 1;
     }
 
+  /* A referenced weak undefined symbol in PIE has to be dynamic.  */
+  if (eif->info->pie
+      && h->dynindx == -1
+      && h->ref_regular
+      && h->root.type == bfd_link_hash_undefweak
+      && ! bfd_elf_link_record_dynamic_symbol (eif->info, h))
+    {
+      eif->failed = TRUE;
+      return FALSE;
+    }
+
   /* If this is a final link, and the symbol was defined as a common
      symbol in a regular object file, and there was no definition in
      any dynamic object, then the linker will have allocated space for
@@ -6443,6 +6454,15 @@ elf_link_output_extsym (struct elf_link_
   else
     sym.st_info = ELF_ST_INFO (STB_GLOBAL, h->type);
 
+  /* If we don't export all symbols into the dynamic symbol table, a
+     symbol in PIE, which isn't defined in nor referenced by any
+     dynamic object, has the hidden visibility.  */
+  if (finfo->info->pie
+      && !finfo->info->export_dynamic
+      && !h->def_dynamic
+      && !h->ref_dynamic)
+    sym.st_other = STV_HIDDEN | (h->other & ~ ELF_ST_VISIBILITY (-1));
+
   switch (h->root.type)
     {
     default:
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

Alan Modra
On Fri, Feb 17, 2006 at 07:47:56AM -0800, H. J. Lu wrote:

> On Fri, Feb 17, 2006 at 06:46:56AM -0800, H. J. Lu wrote:
> > On Fri, Feb 17, 2006 at 05:15:06PM +1030, Alan Modra wrote:
> > > On Thu, Feb 16, 2006 at 10:14:31PM -0800, H. J. Lu wrote:
> > > > It is a generic ELF problem. Why does each backend have to handle it
> > > > separately?
> > >
> > > Well, if you are clever you might be able to make a generic patch that
> > > handles undef weak in pie.  The right place to do it is after all
> > > symbols have been loaded.
> >
> > Thanks for the pointer. Here is the updated patch.
>
> I added the h->ref_regular check. It may not be strictly necessary
> since it isn't easy to generate an unreferenced weak undefined
> symbol :-(.

After giving this some more thought, I'm inclined to say that this patch
does not belong in the generic ELF support.  The reason is that the
behaviour of weak undefined symbols is not well defined.  At least, I'm
not aware of any standard that says an undefined weak sym should be made
dynamic when dynamic objects are involved in the link.  I think it would
be quite reasonable for an ELF target to choose to resolve undefined
weak syms in an executable to zero at link time.

So I think we ought to patch each backend individually, painful as that
might be.  Note that many backends already handle undef weak syms
specially, eg. calling bfd_elf_link_record_dynamic_symbol for plt and
got references (even in non-pie exes).

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

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

H.J. Lu-27
On Mon, Feb 20, 2006 at 10:19:39AM +1030, Alan Modra wrote:

>
> After giving this some more thought, I'm inclined to say that this patch
> does not belong in the generic ELF support.  The reason is that the
> behaviour of weak undefined symbols is not well defined.  At least, I'm
> not aware of any standard that says an undefined weak sym should be made
> dynamic when dynamic objects are involved in the link.  I think it would
> be quite reasonable for an ELF target to choose to resolve undefined
> weak syms in an executable to zero at link time.
>
> So I think we ought to patch each backend individually, painful as that
> might be.  Note that many backends already handle undef weak syms
> specially, eg. calling bfd_elf_link_record_dynamic_symbol for plt and
> got references (even in non-pie exes).

I have no strong opionions on either approach as long as we can get
undefined weak symbol in PIE to work correctly. If we want to make it
target dependent, I'd like to add 2 hooks to ELF backend:

1. fixup_undefined_weak_symbol, which will be called to fix up
the undefined weak symbol. The default will be NULL.
2. pie_fixup_output_extsym, which be called to fix up output symbol
for PIE. The default will also be NULL.

2 generic versions will be provided and x86/x86-64/ia64 will use them.


H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

Alan Modra
On Sun, Feb 19, 2006 at 04:40:41PM -0800, H. J. Lu wrote:

> On Mon, Feb 20, 2006 at 10:19:39AM +1030, Alan Modra wrote:
> >
> > After giving this some more thought, I'm inclined to say that this patch
> > does not belong in the generic ELF support.  The reason is that the
> > behaviour of weak undefined symbols is not well defined.  At least, I'm
> > not aware of any standard that says an undefined weak sym should be made
> > dynamic when dynamic objects are involved in the link.  I think it would
> > be quite reasonable for an ELF target to choose to resolve undefined
> > weak syms in an executable to zero at link time.
> >
> > So I think we ought to patch each backend individually, painful as that
> > might be.  Note that many backends already handle undef weak syms
> > specially, eg. calling bfd_elf_link_record_dynamic_symbol for plt and
> > got references (even in non-pie exes).
>
> I have no strong opionions on either approach as long as we can get
> undefined weak symbol in PIE to work correctly. If we want to make it
> target dependent, I'd like to add 2 hooks to ELF backend:
>
> 1. fixup_undefined_weak_symbol, which will be called to fix up
> the undefined weak symbol. The default will be NULL.
> 2. pie_fixup_output_extsym, which be called to fix up output symbol
> for PIE. The default will also be NULL.
>
> 2 generic versions will be provided and x86/x86-64/ia64 will use them.

I don't see that these are needed.  I'm testing the following.

        PR ld/2218
        * elf32-arm.c (allocate_dynrelocs): Ensure undef weak sym in pie
        is dynamic.
        * elf32-hppa.c (allocate_dynrelocs): Likewise.
        * elf32-i386.c (allocate_dynrelocs): Likewise.
        * elf32-s390.c (allocate_dynrelocs): Likewise.
        * elf32-sh.c (allocate_dynrelocs): Likewise.
        * elf64-s390.c (allocate_dynrelocs): Likewise.
        * elf64-x86-64.c (allocate_dynrelocs): Likewise.
        * elf32-m32r.c (allocate_dynrelocs): Likewise.  Discard relocs
        on undef weak with non-default visibility too.
        * elfxx-sparc.c (allocate_dynrelocs): Ditto.

Index: bfd/elf32-arm.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-arm.c,v
retrieving revision 1.63
diff -u -p -r1.63 elf32-arm.c
--- bfd/elf32-arm.c 27 Jan 2006 14:11:43 -0000 1.63
+++ bfd/elf32-arm.c 20 Feb 2006 00:45:28 -0000
@@ -6333,9 +6333,22 @@ allocate_dynrelocs (struct elf_link_hash
 
       /* Also discard relocs on undefined weak syms with non-default
          visibility.  */
-      if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
+      if (eh->relocs_copied != NULL
   && h->root.type == bfd_link_hash_undefweak)
- eh->relocs_copied = NULL;
+ {
+  if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT)
+    eh->relocs_copied = NULL;
+
+  /* Make sure undefined weak symbols are output as a dynamic
+     symbol in PIEs.  */
+  else if (h->dynindx == -1
+   && !h->forced_local)
+    {
+      if (! bfd_elf_link_record_dynamic_symbol (info, h))
+ return FALSE;
+    }
+ }
+
       else if (htab->root.is_relocatable_executable && h->dynindx == -1
        && h->root.type == bfd_link_hash_new)
  {
Index: bfd/elf32-hppa.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-hppa.c,v
retrieving revision 1.141
diff -u -p -r1.141 elf32-hppa.c
--- bfd/elf32-hppa.c 27 Dec 2005 22:50:08 -0000 1.141
+++ bfd/elf32-hppa.c 20 Feb 2006 00:45:30 -0000
@@ -2020,9 +2020,21 @@ allocate_dynrelocs (struct elf_link_hash
 
       /* Also discard relocs on undefined weak syms with non-default
  visibility.  */
-      if (ELF_ST_VISIBILITY (eh->other) != STV_DEFAULT
+      if (hh->dyn_relocs != NULL
   && eh->root.type == bfd_link_hash_undefweak)
- hh->dyn_relocs = NULL;
+ {
+  if (ELF_ST_VISIBILITY (eh->other) != STV_DEFAULT)
+    hh->dyn_relocs = NULL;
+
+  /* Make sure undefined weak symbols are output as a dynamic
+     symbol in PIEs.  */
+  else if (eh->dynindx == -1
+   && !eh->forced_local)
+    {
+      if (! bfd_elf_link_record_dynamic_symbol (info, eh))
+ return FALSE;
+    }
+ }
     }
   else
     {
Index: bfd/elf32-i386.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-i386.c,v
retrieving revision 1.151
diff -u -p -r1.151 elf32-i386.c
--- bfd/elf32-i386.c 18 Jan 2006 21:07:48 -0000 1.151
+++ bfd/elf32-i386.c 20 Feb 2006 00:45:33 -0000
@@ -1779,9 +1779,21 @@ allocate_dynrelocs (struct elf_link_hash
 
       /* Also discard relocs on undefined weak syms with non-default
  visibility.  */
-      if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
+      if (eh->dyn_relocs != NULL
   && h->root.type == bfd_link_hash_undefweak)
- eh->dyn_relocs = NULL;
+ {
+  if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT)
+    eh->dyn_relocs = NULL;
+
+  /* Make sure undefined weak symbols are output as a dynamic
+     symbol in PIEs.  */
+  else if (h->dynindx == -1
+   && !h->forced_local)
+    {
+      if (! bfd_elf_link_record_dynamic_symbol (info, h))
+ return FALSE;
+    }
+ }
     }
   else if (ELIMINATE_COPY_RELOCS)
     {
Index: bfd/elf32-m32r.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-m32r.c,v
retrieving revision 1.76
diff -u -p -r1.76 elf32-m32r.c
--- bfd/elf32-m32r.c 1 Dec 2005 04:48:13 -0000 1.76
+++ bfd/elf32-m32r.c 20 Feb 2006 00:45:35 -0000
@@ -2101,6 +2101,24 @@ allocate_dynrelocs (struct elf_link_hash
                 pp = &p->next;
             }
         }
+
+      /* Also discard relocs on undefined weak syms with non-default
+ visibility.  */
+      if (eh->dyn_relocs != NULL
+  && h->root.type == bfd_link_hash_undefweak)
+ {
+  if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT)
+    eh->dyn_relocs = NULL;
+
+  /* Make sure undefined weak symbols are output as a dynamic
+     symbol in PIEs.  */
+  else if (h->dynindx == -1
+   && !h->forced_local)
+    {
+      if (! bfd_elf_link_record_dynamic_symbol (info, h))
+ return FALSE;
+    }
+ }
     }
   else
     {
Index: bfd/elf32-s390.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-s390.c,v
retrieving revision 1.79
diff -u -p -r1.79 elf32-s390.c
--- bfd/elf32-s390.c 25 Oct 2005 16:19:06 -0000 1.79
+++ bfd/elf32-s390.c 20 Feb 2006 00:45:36 -0000
@@ -1908,9 +1908,21 @@ allocate_dynrelocs (h, inf)
 
       /* Also discard relocs on undefined weak syms with non-default
  visibility.  */
-      if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
+      if (eh->dyn_relocs != NULL
   && h->root.type == bfd_link_hash_undefweak)
- eh->dyn_relocs = NULL;
+ {
+  if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT)
+    eh->dyn_relocs = NULL;
+
+  /* Make sure undefined weak symbols are output as a dynamic
+     symbol in PIEs.  */
+  else if (h->dynindx == -1
+   && !h->forced_local)
+    {
+      if (! bfd_elf_link_record_dynamic_symbol (info, h))
+ return FALSE;
+    }
+ }
     }
   else if (ELIMINATE_COPY_RELOCS)
     {
Index: bfd/elf32-sh.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-sh.c,v
retrieving revision 1.132
diff -u -p -r1.132 elf32-sh.c
--- bfd/elf32-sh.c 31 Dec 2005 16:23:13 -0000 1.132
+++ bfd/elf32-sh.c 20 Feb 2006 00:45:40 -0000
@@ -4160,9 +4160,21 @@ allocate_dynrelocs (struct elf_link_hash
 
       /* Also discard relocs on undefined weak syms with non-default
  visibility.  */
-      if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
+      if (eh->dyn_relocs != NULL
   && h->root.type == bfd_link_hash_undefweak)
- eh->dyn_relocs = NULL;
+ {
+  if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT)
+    eh->dyn_relocs = NULL;
+
+  /* Make sure undefined weak symbols are output as a dynamic
+     symbol in PIEs.  */
+  else if (h->dynindx == -1
+   && !h->forced_local)
+    {
+      if (! bfd_elf_link_record_dynamic_symbol (info, h))
+ return FALSE;
+    }
+ }
     }
   else
     {
Index: bfd/elf64-s390.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-s390.c,v
retrieving revision 1.79
diff -u -p -r1.79 elf64-s390.c
--- bfd/elf64-s390.c 25 Oct 2005 16:19:07 -0000 1.79
+++ bfd/elf64-s390.c 20 Feb 2006 00:45:41 -0000
@@ -1881,9 +1881,21 @@ allocate_dynrelocs (h, inf)
 
       /* Also discard relocs on undefined weak syms with non-default
  visibility.  */
-      if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
+      if (eh->dyn_relocs != NULL
   && h->root.type == bfd_link_hash_undefweak)
- eh->dyn_relocs = NULL;
+ {
+  if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT)
+    eh->dyn_relocs = NULL;
+
+  /* Make sure undefined weak symbols are output as a dynamic
+     symbol in PIEs.  */
+  else if (h->dynindx == -1
+   && !h->forced_local)
+    {
+      if (! bfd_elf_link_record_dynamic_symbol (info, h))
+ return FALSE;
+    }
+ }
     }
   else if (ELIMINATE_COPY_RELOCS)
     {
Index: bfd/elf64-x86-64.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-x86-64.c,v
retrieving revision 1.109
diff -u -p -r1.109 elf64-x86-64.c
--- bfd/elf64-x86-64.c 18 Jan 2006 21:07:48 -0000 1.109
+++ bfd/elf64-x86-64.c 20 Feb 2006 00:45:43 -0000
@@ -1565,9 +1565,21 @@ allocate_dynrelocs (struct elf_link_hash
 
       /* Also discard relocs on undefined weak syms with non-default
  visibility.  */
-      if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
+      if (eh->dyn_relocs != NULL
   && h->root.type == bfd_link_hash_undefweak)
- eh->dyn_relocs = NULL;
+ {
+  if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT)
+    eh->dyn_relocs = NULL;
+
+  /* Make sure undefined weak symbols are output as a dynamic
+     symbol in PIEs.  */
+  else if (h->dynindx == -1
+   && !h->forced_local)
+    {
+      if (! bfd_elf_link_record_dynamic_symbol (info, h))
+ return FALSE;
+    }
+ }
     }
   else if (ELIMINATE_COPY_RELOCS)
     {
Index: bfd/elfxx-sparc.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-sparc.c,v
retrieving revision 1.17
diff -u -p -r1.17 elfxx-sparc.c
--- bfd/elfxx-sparc.c 1 Feb 2006 22:03:38 -0000 1.17
+++ bfd/elfxx-sparc.c 20 Feb 2006 00:45:46 -0000
@@ -1935,6 +1935,24 @@ allocate_dynrelocs (struct elf_link_hash
  pp = &p->next;
     }
  }
+
+      /* Also discard relocs on undefined weak syms with non-default
+ visibility.  */
+      if (eh->dyn_relocs != NULL
+  && h->root.type == bfd_link_hash_undefweak)
+ {
+  if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT)
+    eh->dyn_relocs = NULL;
+
+  /* Make sure undefined weak symbols are output as a dynamic
+     symbol in PIEs.  */
+  else if (h->dynindx == -1
+   && !h->forced_local)
+    {
+      if (! bfd_elf_link_record_dynamic_symbol (info, h))
+ return FALSE;
+    }
+ }
     }
   else
     {

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

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

Daniel Jacobowitz-2
In reply to this post by Alan Modra
On Mon, Feb 20, 2006 at 10:19:39AM +1030, Alan Modra wrote:

> After giving this some more thought, I'm inclined to say that this patch
> does not belong in the generic ELF support.  The reason is that the
> behaviour of weak undefined symbols is not well defined.  At least, I'm
> not aware of any standard that says an undefined weak sym should be made
> dynamic when dynamic objects are involved in the link.  I think it would
> be quite reasonable for an ELF target to choose to resolve undefined
> weak syms in an executable to zero at link time.
>
> So I think we ought to patch each backend individually, painful as that
> might be.  Note that many backends already handle undef weak syms
> specially, eg. calling bfd_elf_link_record_dynamic_symbol for plt and
> got references (even in non-pie exes).

I don't think I follow your logic here.  We've got a Linux port to the
lion's share of architectures that we have an ELF backend and dynamic
linking for; we're going to end up implementing the same thing in each
and every one of those backends, to be consistent.

Are you worried about compatibility with some as-yet-unknown tools that
don't do this?  If so, can we have a backend flag to turn it off, but
enable it by default and keep the code in common?

I'm generally distressed with how much platform-specific code there is
in all this; I think we should be trying to drive that down, not up.

--
Daniel Jacobowitz
CodeSourcery
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

H.J. Lu-27
In reply to this post by Alan Modra
On Mon, Feb 20, 2006 at 11:30:20AM +1030, Alan Modra wrote:

> On Sun, Feb 19, 2006 at 04:40:41PM -0800, H. J. Lu wrote:
> > On Mon, Feb 20, 2006 at 10:19:39AM +1030, Alan Modra wrote:
> > >
> > > After giving this some more thought, I'm inclined to say that this patch
> > > does not belong in the generic ELF support.  The reason is that the
> > > behaviour of weak undefined symbols is not well defined.  At least, I'm
> > > not aware of any standard that says an undefined weak sym should be made
> > > dynamic when dynamic objects are involved in the link.  I think it would
> > > be quite reasonable for an ELF target to choose to resolve undefined
> > > weak syms in an executable to zero at link time.
> > >
> > > So I think we ought to patch each backend individually, painful as that
> > > might be.  Note that many backends already handle undef weak syms
> > > specially, eg. calling bfd_elf_link_record_dynamic_symbol for plt and
> > > got references (even in non-pie exes).
> >
> > I have no strong opionions on either approach as long as we can get
> > undefined weak symbol in PIE to work correctly. If we want to make it
> > target dependent, I'd like to add 2 hooks to ELF backend:
> >
> > 1. fixup_undefined_weak_symbol, which will be called to fix up
> > the undefined weak symbol. The default will be NULL.
> > 2. pie_fixup_output_extsym, which be called to fix up output symbol
> > for PIE. The default will also be NULL.
> >
> > 2 generic versions will be provided and x86/x86-64/ia64 will use them.
>
> I don't see that these are needed.  I'm testing the following.
>
> PR ld/2218
> * elf32-arm.c (allocate_dynrelocs): Ensure undef weak sym in pie
> is dynamic.
> * elf32-hppa.c (allocate_dynrelocs): Likewise.
> * elf32-i386.c (allocate_dynrelocs): Likewise.
> * elf32-s390.c (allocate_dynrelocs): Likewise.
> * elf32-sh.c (allocate_dynrelocs): Likewise.
> * elf64-s390.c (allocate_dynrelocs): Likewise.
> * elf64-x86-64.c (allocate_dynrelocs): Likewise.
> * elf32-m32r.c (allocate_dynrelocs): Likewise.  Discard relocs
> on undef weak with non-default visibility too.
> * elfxx-sparc.c (allocate_dynrelocs): Ditto.
>

Do you have a patch for ia64? Also how do you address

http://sourceware.org/bugzilla/show_bug.cgi?id=2251

My patch changes to if we don't export all symbols into the dynamic
symbol table, a symbol in PIE, which isn't defined in nor referenced
by any dynamic object, has the hidden visibility.


H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

Alan Modra
On Sun, Feb 19, 2006 at 06:10:01PM -0800, H. J. Lu wrote:
> Do you have a patch for ia64?

Nope.  I was leaving that to you.

> Also how do you address
>
> http://sourceware.org/bugzilla/show_bug.cgi?id=2251

Since that is a ld.so problem, I don't understand why you are asking.

> My patch changes to if we don't export all symbols into the dynamic
> symbol table, a symbol in PIE, which isn't defined in nor referenced
> by any dynamic object, has the hidden visibility.

Why is this a good idea, and why should this change only apply to pie?
ie. Why not do the same for non-pie executables.  Wasn't this part of
your patch just a hack to fix up the fact that you made too many symbols
dynamic in the first place?

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

Re: PATCH: ld/2218: Weak undefined symbol doesn't work properly with PIE

Alan Modra
In reply to this post by Daniel Jacobowitz-2
On Sun, Feb 19, 2006 at 08:02:54PM -0500, Daniel Jacobowitz wrote:

> On Mon, Feb 20, 2006 at 10:19:39AM +1030, Alan Modra wrote:
> > After giving this some more thought, I'm inclined to say that this patch
> > does not belong in the generic ELF support.  The reason is that the
> > behaviour of weak undefined symbols is not well defined.  At least, I'm
> > not aware of any standard that says an undefined weak sym should be made
> > dynamic when dynamic objects are involved in the link.  I think it would
> > be quite reasonable for an ELF target to choose to resolve undefined
> > weak syms in an executable to zero at link time.
> >
> > So I think we ought to patch each backend individually, painful as that
> > might be.  Note that many backends already handle undef weak syms
> > specially, eg. calling bfd_elf_link_record_dynamic_symbol for plt and
> > got references (even in non-pie exes).
>
> I don't think I follow your logic here.  We've got a Linux port to the
> lion's share of architectures that we have an ELF backend and dynamic
> linking for; we're going to end up implementing the same thing in each
> and every one of those backends, to be consistent.
>
> Are you worried about compatibility with some as-yet-unknown tools that
> don't do this?

Yes, and the possibility that some existing target already handles
undefined weak differently, and will be broken by a patch like HJ's.

>  If so, can we have a backend flag to turn it off, but
> enable it by default and keep the code in common?

We could, but take a look at a typical allocate_dynrelocs function.
Undefined weak are treated specially for plt, got, and when we try to
eliminate copy relocs.  I don't think it makes sense to just handle
undefined weak in pie in generic ELF code without handling the other
cases.   And, since we have quite different plt/got schemes, I think it
will be quite awkward to try to handle them all in generic code.

--
Alan Modra
IBM OzLabs - Linux Technology Centre
12