This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: PATCH: PR ld/14591: Wrong symbol type with common symbol and weak function
- From: "H.J. Lu" <hongjiu dot lu at intel dot com>
- To: binutils at sourceware dot org
- Date: Tue, 18 Sep 2012 07:21:16 -0700
- Subject: Re: PATCH: PR ld/14591: Wrong symbol type with common symbol and weak function
- References: <20120917203333.GA24337@intel.com>
- Reply-to: "H.J. Lu" <hjl dot tools at gmail dot com>
On Mon, Sep 17, 2012 at 01:33:33PM -0700, H.J. Lu wrote:
> Hi,
>
> We allow a common symbol to override a weak function. But we didn't
> handle symbol type properly, which leads to STT_FUNC type for common
> symbol. This patch avoids updating common symbol type from a weak
> definition and updates symbol type from a common symbol when overriding
> a weak symbol. OK to install?
>
> Thanks.
>
>
> H.J.
> ---
> bfd/
>
> 2012-09-17 H.J. Lu <hongjiu.lu@intel.com>
>
> PR ld/14591
> * elf-bfd.h (_bfd_elf_merge_symbol): Add an argument to return
> if the old symbol is weak.
> (_bfd_elf_add_default_symbol): Update _bfd_elf_merge_symbol.
> (elf_link_add_object_symbols): Don't update symbol type from a
> weak definition. Update symbol type from a common symbol when
> overriding a weak symbol.
>
Here is the updated patch with additinal tests. OK to install?
Thanks.
---
bfd/
2012-09-18 H.J. Lu <hongjiu.lu@intel.com>
PR ld/14591
* elf-bfd.h (_bfd_elf_merge_symbol): Add an argument to return
if the old symbol is weak.
(_bfd_elf_add_default_symbol): Update _bfd_elf_merge_symbol.
(elf_link_add_object_symbols): Don't update symbol type from a
weak definition. Update symbol type from a common symbol when
overriding a weak symbol.
ld/testsuite/
2012-09-18 H.J. Lu <hongjiu.lu@intel.com>
PR ld/14591
* ld-elf/comm-data.exp (run_ld_link_tests): Add comm-data3a and
comm-data3b tests.
* ld-ifunc/ifunc.exp (run_ld_link_exec_tests): New.
* ld-elf/comm-data3.sd: New file.
* ld-elf/comm-data3a.s: Likewise.
* ld-elf/comm-data3b.s: Likewise.
* ld-ifunc/ifunc-17a-i386.d: Likewise.
* ld-ifunc/ifunc-17a-x86-64.d: Likewise.
* ld-ifunc/ifunc-17a.s: Likewise.
* ld-ifunc/ifunc-17b-i386.d: Likewise.
* ld-ifunc/ifunc-17b-x86-64.d: Likewise.
* ld-ifunc/ifunc-17b.s: Likewise.
* ld-ifunc/ifunc-common-1.out: Likewise.
* ld-ifunc/ifunc-common-1a.c: Likewise.
* ld-ifunc/ifunc-common-1b.c: Likewise.
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 1225036..3709af9 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1970,7 +1970,7 @@ extern bfd_boolean _bfd_elf_maybe_strip_eh_frame_hdr
extern bfd_boolean _bfd_elf_merge_symbol
(bfd *, struct bfd_link_info *, const char *, Elf_Internal_Sym *,
- asection **, bfd_vma *, unsigned int *,
+ asection **, bfd_vma *, bfd_boolean *, unsigned int *,
struct elf_link_hash_entry **, bfd_boolean *,
bfd_boolean *, bfd_boolean *, bfd_boolean *);
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 9446e7d..232c0b1 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -941,6 +941,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
Elf_Internal_Sym *sym,
asection **psec,
bfd_vma *pvalue,
+ bfd_boolean *pold_weak,
unsigned int *pold_alignment,
struct elf_link_hash_entry **sym_hash,
bfd_boolean *skip,
@@ -1043,6 +1044,8 @@ _bfd_elf_merge_symbol (bfd *abfd,
newweak = bind == STB_WEAK;
oldweak = (h->root.type == bfd_link_hash_defweak
|| h->root.type == bfd_link_hash_undefweak);
+ if (pold_weak)
+ *pold_weak = oldweak;
/* In cases involving weak versioned symbols, we may wind up trying
to merge a symbol with itself. Catch that here, to avoid the
@@ -1665,7 +1668,7 @@ _bfd_elf_add_default_symbol (bfd *abfd,
size_change_ok = FALSE;
sec = *psec;
if (!_bfd_elf_merge_symbol (abfd, info, shortname, sym, &sec, value,
- NULL, &hi, &skip, &override,
+ NULL, NULL, &hi, &skip, &override,
&type_change_ok, &size_change_ok))
return FALSE;
@@ -1774,7 +1777,7 @@ nondefault:
size_change_ok = FALSE;
sec = *psec;
if (!_bfd_elf_merge_symbol (abfd, info, shortname, sym, &sec, value,
- NULL, &hi, &skip, &override,
+ NULL, NULL, &hi, &skip, &override,
&type_change_ok, &size_change_ok))
return FALSE;
@@ -3868,6 +3871,8 @@ error_free_dyn:
bfd_boolean size_change_ok;
bfd_boolean type_change_ok;
bfd_boolean new_weakdef;
+ bfd_boolean new_weak;
+ bfd_boolean old_weak;
bfd_boolean override;
bfd_boolean common;
unsigned int old_alignment;
@@ -4000,6 +4005,7 @@ error_free_dyn:
size_change_ok = FALSE;
type_change_ok = bed->type_change_ok;
+ old_weak = FALSE;
old_alignment = 0;
old_bfd = NULL;
new_sec = sec;
@@ -4145,7 +4151,7 @@ error_free_dyn:
}
if (!_bfd_elf_merge_symbol (abfd, info, name, isym, &sec,
- &value, &old_alignment,
+ &value, &old_weak, &old_alignment,
sym_hash, &skip, &override,
&type_change_ok, &size_change_ok))
goto error_free_vers;
@@ -4207,10 +4213,11 @@ error_free_dyn:
if (is_elf_hash_table (htab))
h->unique_global = (flags & BSF_GNU_UNIQUE) != 0;
+ new_weak = (flags & BSF_WEAK) != 0;
new_weakdef = FALSE;
if (dynamic
&& definition
- && (flags & BSF_WEAK) != 0
+ && new_weak
&& !bed->is_function_type (ELF_ST_TYPE (isym->st_info))
&& is_elf_hash_table (htab)
&& h->u.weakdef == NULL)
@@ -4339,7 +4346,9 @@ error_free_dyn:
h->size = h->root.u.c.size;
if (ELF_ST_TYPE (isym->st_info) != STT_NOTYPE
- && (definition || h->type == STT_NOTYPE))
+ && ((definition && !new_weak)
+ || (old_weak && h->root.type == bfd_link_hash_common)
+ || h->type == STT_NOTYPE))
{
unsigned int type = ELF_ST_TYPE (isym->st_info);
diff --git a/ld/testsuite/ld-elf/comm-data.exp b/ld/testsuite/ld-elf/comm-data.exp
index 2258afb..1c75f55 100644
--- a/ld/testsuite/ld-elf/comm-data.exp
+++ b/ld/testsuite/ld-elf/comm-data.exp
@@ -75,4 +75,24 @@ run_ld_link_tests [list \
} \
"comm-data" \
] \
+ [list \
+ "$testname 3a" \
+ "-static" \
+ "" \
+ { comm-data3a.s comm-data3b.s } \
+ { \
+ { readelf -s comm-data3.sd } \
+ } \
+ "comm-data3a" \
+ ] \
+ [list \
+ "$testname 3b" \
+ "-static" \
+ "" \
+ { comm-data3b.s comm-data3a.s } \
+ { \
+ { readelf -s comm-data3.sd } \
+ } \
+ "comm-data3b" \
+ ] \
]
diff --git a/ld/testsuite/ld-elf/comm-data3.sd b/ld/testsuite/ld-elf/comm-data3.sd
new file mode 100644
index 0000000..5a96ed7
--- /dev/null
+++ b/ld/testsuite/ld-elf/comm-data3.sd
@@ -0,0 +1,3 @@
+#...
+ +[0-9]+: +[0-9a-f]+ +4 +OBJECT +GLOBAL +DEFAULT +[1-9] foo
+#pass
diff --git a/ld/testsuite/ld-elf/comm-data3a.s b/ld/testsuite/ld-elf/comm-data3a.s
new file mode 100644
index 0000000..e0bde49
--- /dev/null
+++ b/ld/testsuite/ld-elf/comm-data3a.s
@@ -0,0 +1,11 @@
+ .globl main
+ .globl start
+ .globl _start
+ .globl __start
+ .text
+main:
+start:
+_start:
+__start:
+ .byte 0
+ .common foo,4,4
diff --git a/ld/testsuite/ld-elf/comm-data3b.s b/ld/testsuite/ld-elf/comm-data3b.s
new file mode 100644
index 0000000..837a099
--- /dev/null
+++ b/ld/testsuite/ld-elf/comm-data3b.s
@@ -0,0 +1,6 @@
+ .weak foo
+ .type foo,%function
+ .size foo,1
+ .text
+foo:
+ .byte 1
diff --git a/ld/testsuite/ld-ifunc/ifunc-17a-i386.d b/ld/testsuite/ld-ifunc/ifunc-17a-i386.d
new file mode 100644
index 0000000..303b54e
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/ifunc-17a-i386.d
@@ -0,0 +1,10 @@
+#source: ifunc-17a.s
+#source: ifunc-17b.s
+#ld: -static -m elf_i386
+#as: --32
+#readelf: -s --wide
+#target: x86_64-*-* i?86-*-*
+
+#...
+ +[0-9]+: +[0-9a-f]+ +4 +OBJECT +GLOBAL +DEFAULT +[1-9] foo
+#pass
diff --git a/ld/testsuite/ld-ifunc/ifunc-17a-x86-64.d b/ld/testsuite/ld-ifunc/ifunc-17a-x86-64.d
new file mode 100644
index 0000000..965ef63
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/ifunc-17a-x86-64.d
@@ -0,0 +1,10 @@
+#source: ifunc-17a.s
+#source: ifunc-17b.s
+#ld: -static -m elf_x86_64
+#as: --64
+#readelf: -s --wide
+#target: x86_64-*-*
+
+#...
+ +[0-9]+: +[0-9a-f]+ +4 +OBJECT +GLOBAL +DEFAULT +[1-9] foo
+#pass
diff --git a/ld/testsuite/ld-ifunc/ifunc-17a.s b/ld/testsuite/ld-ifunc/ifunc-17a.s
new file mode 100644
index 0000000..e0bde49
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/ifunc-17a.s
@@ -0,0 +1,11 @@
+ .globl main
+ .globl start
+ .globl _start
+ .globl __start
+ .text
+main:
+start:
+_start:
+__start:
+ .byte 0
+ .common foo,4,4
diff --git a/ld/testsuite/ld-ifunc/ifunc-17b-i386.d b/ld/testsuite/ld-ifunc/ifunc-17b-i386.d
new file mode 100644
index 0000000..82ccac2
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/ifunc-17b-i386.d
@@ -0,0 +1,10 @@
+#source: ifunc-17b.s
+#source: ifunc-17a.s
+#ld: -static -m elf_i386
+#as: --32
+#readelf: -s --wide
+#target: x86_64-*-* i?86-*-*
+
+#...
+ +[0-9]+: +[0-9a-f]+ +4 +OBJECT +GLOBAL +DEFAULT +[1-9] foo
+#pass
diff --git a/ld/testsuite/ld-ifunc/ifunc-17b-x86-64.d b/ld/testsuite/ld-ifunc/ifunc-17b-x86-64.d
new file mode 100644
index 0000000..7527eae
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/ifunc-17b-x86-64.d
@@ -0,0 +1,10 @@
+#source: ifunc-17b.s
+#source: ifunc-17a.s
+#ld: -static -m elf_x86_64
+#as: --64
+#readelf: -s --wide
+#target: x86_64-*-*
+
+#...
+ +[0-9]+: +[0-9a-f]+ +4 +OBJECT +GLOBAL +DEFAULT +[1-9] foo
+#pass
diff --git a/ld/testsuite/ld-ifunc/ifunc-17b.s b/ld/testsuite/ld-ifunc/ifunc-17b.s
new file mode 100644
index 0000000..66abe04
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/ifunc-17b.s
@@ -0,0 +1,6 @@
+ .weak foo
+ .type foo, %gnu_indirect_function
+ .size foo,1
+ .text
+foo:
+ .byte 1
diff --git a/ld/testsuite/ld-ifunc/ifunc-common-1.out b/ld/testsuite/ld-ifunc/ifunc-common-1.out
new file mode 100644
index 0000000..31a3ce2
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/ifunc-common-1.out
@@ -0,0 +1 @@
+PASSED: 0
diff --git a/ld/testsuite/ld-ifunc/ifunc-common-1a.c b/ld/testsuite/ld-ifunc/ifunc-common-1a.c
new file mode 100644
index 0000000..794ca45
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/ifunc-common-1a.c
@@ -0,0 +1,9 @@
+#include <stdio.h>
+
+int foo;
+int
+main ()
+{
+ printf ("PASSED: %d\n", foo);
+ return 0;
+}
diff --git a/ld/testsuite/ld-ifunc/ifunc-common-1b.c b/ld/testsuite/ld-ifunc/ifunc-common-1b.c
new file mode 100644
index 0000000..1235942
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/ifunc-common-1b.c
@@ -0,0 +1,12 @@
+void alt (void) { }
+
+void foo (void);
+void * foo_ifunc (void) __asm__ ("foo");
+__asm__(".type foo, %gnu_indirect_function");
+__asm__(".weak foo");
+
+void *
+foo_ifunc (void)
+{
+ return alt;
+}
diff --git a/ld/testsuite/ld-ifunc/ifunc.exp b/ld/testsuite/ld-ifunc/ifunc.exp
index 3eef3a9..df913d8 100644
--- a/ld/testsuite/ld-ifunc/ifunc.exp
+++ b/ld/testsuite/ld-ifunc/ifunc.exp
@@ -347,6 +347,27 @@ if { $verbose < 1 } {
remote_file host delete "tmpdir/static_nonifunc_prog"
}
+run_ld_link_exec_tests [] [list \
+ [list \
+ "Common symbol override ifunc test 1a" \
+ "-static" \
+ "" \
+ { ifunc-common-1a.c ifunc-common-1b.c } \
+ "ifunc-common-1a" \
+ "ifunc-common-1.out" \
+ "-g" \
+ ] \
+ [list \
+ "Common symbol override ifunc test 1b" \
+ "-static" \
+ "" \
+ { ifunc-common-1b.c ifunc-common-1a.c } \
+ "ifunc-common-1b" \
+ "ifunc-common-1.out" \
+ "-g" \
+ ] \
+]
+
set test_list [lsort [glob -nocomplain $srcdir/$subdir/*.d]]
foreach t $test_list {
# We need to strip the ".d", but can leave the dirname.