This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Revert pr16467 change
- From: Alan Modra <amodra at gmail dot com>
- To: binutils at sourceware dot org
- Cc: "H.J. Lu" <hjl dot tools at gmail dot com>
- Date: Wed, 1 Jun 2016 22:58:39 +0930
- Subject: Revert pr16467 change
- Authentication-results: sourceware.org; auth=none
This reverts the pr16467 change, which was incorrect due to faulty
analysis of the pr16467 testcase. The failure was not due to a
mismatch in symbol type (ifunc/non-ifunc) but due to a symbol loop
being set up. I've added a pr16467d.c that shows the same symbol
loop without ifunc being involved.
I'm not 100% happy with this fix, due to not always having symbol
versions around early enough. See the comment. However, comparing
versions directly addresses the problem exposed by the testcase, and I
don't see any other approach that will work in all cases.
The strtab save/restore change is necessary due to
_bfd_elf_add_default_symbol now calling elf_backend_hide_symbol, which
decrements dynstr strtab refcounts. Previously we only added new
dynamic symbols.
I'll commit this in a few days if no more problems like the strtab
refcounting one appear.
bfd/
PR ld/20159
PR ld/16467
* elflink.c (_bfd_elf_merge_symbol): Revert PR16467 change.
(_bfd_elf_add_default_symbol): Don't indirect to/from defined
symbol given a version by a script different to the version
of the symbol being added.
(elf_link_add_object_symbols): Use _bfd_elf_strtab_save and
_bfd_elf_strtab_restore. Don't fudge dynstr references.
* elf-strtab.c (_bfd_elf_strtab_restore_size): Delete.
(struct strtab_save): New.
(_bfd_elf_strtab_save, _bfd_elf_strtab_restore): New functions.
* elf-bfd.h (_bfd_elf_strtab_restore_size): Delete.
(_bfd_elf_strtab_save, _bfd_elf_strtab_restore): Declare.
ld/
* testsuite/ld-ifunc/pr16467d.c: New file.
* testsuite/ld-ifunc/pr20159.out: New file.
* testsuite/ld-ifunc/pr20159a.c: New file.
* testsuite/ld-ifunc/pr20159b.c: New file.
* testsuite/ld-ifunc/ifunc.exp: Run new tests.
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 3184e57..99dbbce 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2058,9 +2058,11 @@ extern void _bfd_elf_strtab_delref
extern unsigned int _bfd_elf_strtab_refcount
(struct elf_strtab_hash *, bfd_size_type);
extern void _bfd_elf_strtab_clear_all_refs
- (struct elf_strtab_hash *tab);
-extern void _bfd_elf_strtab_restore_size
- (struct elf_strtab_hash *, bfd_size_type);
+ (struct elf_strtab_hash *);
+extern void *_bfd_elf_strtab_save
+ (struct elf_strtab_hash *);
+extern void _bfd_elf_strtab_restore
+ (struct elf_strtab_hash *, void *);
extern bfd_size_type _bfd_elf_strtab_size
(struct elf_strtab_hash *);
extern bfd_size_type _bfd_elf_strtab_offset
diff --git a/bfd/elf-strtab.c b/bfd/elf-strtab.c
index ca8ac33..19b0ad8 100644
--- a/bfd/elf-strtab.c
+++ b/bfd/elf-strtab.c
@@ -215,16 +215,45 @@ _bfd_elf_strtab_clear_all_refs (struct elf_strtab_hash *tab)
tab->array[idx]->refcount = 0;
}
-/* Downsizes strtab. Entries from IDX up to the current size are
- removed from the array. */
+/* Save strtab refcounts prior to adding --as-needed library. */
+
+struct strtab_save
+{
+ bfd_size_type size;
+ unsigned int refcount[1];
+};
+
+void *
+_bfd_elf_strtab_save (struct elf_strtab_hash *tab)
+{
+ struct strtab_save *save;
+ bfd_size_type idx, size;
+
+ size = sizeof (*save) + (tab->size - 1) * sizeof (save->refcount[0]);
+ save = bfd_malloc (size);
+ if (save == NULL)
+ return save;
+
+ save->size = tab->size;
+ for (idx = 1; idx < tab->size; idx++)
+ save->refcount[idx] = tab->array[idx]->refcount;
+ return save;
+}
+
+/* Restore strtab refcounts on finding --as-needed library not needed. */
+
void
-_bfd_elf_strtab_restore_size (struct elf_strtab_hash *tab, bfd_size_type idx)
+_bfd_elf_strtab_restore (struct elf_strtab_hash *tab, void *buf)
{
- bfd_size_type curr_size = tab->size;
+ bfd_size_type idx, curr_size = tab->size;
+ struct strtab_save *save = (struct strtab_save *) buf;
BFD_ASSERT (tab->sec_size == 0);
- BFD_ASSERT (idx <= curr_size);
- tab->size = idx;
+ BFD_ASSERT (save->size <= curr_size);
+ tab->size = save->size;
+ for (idx = 1; idx < save->size; ++idx)
+ tab->array[idx]->refcount = save->refcount[idx];
+
for (; idx < curr_size; ++idx)
{
/* We don't remove entries from the hash table, just set their
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 93e7dd2..8bb1066 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1202,21 +1202,20 @@ _bfd_elf_merge_symbol (bfd *abfd,
oldfunc = (h->type != STT_NOTYPE
&& bed->is_function_type (h->type));
- /* When we try to create a default indirect symbol from the dynamic
- definition with the default version, we skip it if its type and
- the type of existing regular definition mismatch. */
+ /* If creating a default indirect symbol ("foo" or "foo@") from a
+ dynamic versioned definition ("foo@@") skip doing so if there is
+ an existing regular definition with a different type. We don't
+ want, for example, a "time" variable in the executable overriding
+ a "time" function in a shared library. */
if (pold_alignment == NULL
&& newdyn
&& newdef
&& !olddyn
- && (((olddef || h->root.type == bfd_link_hash_common)
- && ELF_ST_TYPE (sym->st_info) != h->type
- && ELF_ST_TYPE (sym->st_info) != STT_NOTYPE
- && h->type != STT_NOTYPE
- && !(newfunc && oldfunc))
- || (olddef
- && ((h->type == STT_GNU_IFUNC)
- != (ELF_ST_TYPE (sym->st_info) == STT_GNU_IFUNC)))))
+ && (olddef || h->root.type == bfd_link_hash_common)
+ && ELF_ST_TYPE (sym->st_info) != h->type
+ && ELF_ST_TYPE (sym->st_info) != STT_NOTYPE
+ && h->type != STT_NOTYPE
+ && !(newfunc && oldfunc))
{
*skip = TRUE;
return TRUE;
@@ -1781,6 +1780,31 @@ _bfd_elf_add_default_symbol (bfd *abfd,
if (skip)
goto nondefault;
+ if (hi->def_regular)
+ {
+ /* If the undecorated symbol will have a version added by a
+ script different to H, then don't indirect to/from the
+ undecorated symbol. This isn't ideal because we may not yet
+ have seen symbol versions, if given by a script on the
+ command line rather than via --version-script. */
+ if (hi->verinfo.vertree == NULL && info->version_info != NULL)
+ {
+ bfd_boolean hide;
+
+ hi->verinfo.vertree
+ = bfd_find_version_for_sym (info->version_info,
+ hi->root.root.string, &hide);
+ if (hi->verinfo.vertree != NULL && hide)
+ {
+ (*bed->elf_backend_hide_symbol) (info, hi, TRUE);
+ goto nondefault;
+ }
+ }
+ if (hi->verinfo.vertree != NULL
+ && strcmp (p + 1 + (p[1] == '@'), hi->verinfo.vertree->name) != 0)
+ goto nondefault;
+ }
+
if (! override)
{
/* Add the default symbol if not performing a relocatable link. */
@@ -3591,8 +3615,7 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
void *old_ent;
struct bfd_link_hash_entry *old_undefs = NULL;
struct bfd_link_hash_entry *old_undefs_tail = NULL;
- long old_dynsymcount = 0;
- bfd_size_type old_dynstr_size = 0;
+ void *old_strtab = NULL;
size_t tabsize = 0;
asection *s;
bfd_boolean just_syms;
@@ -4036,8 +4059,9 @@ error_free_dyn:
old_table = htab->root.table.table;
old_size = htab->root.table.size;
old_count = htab->root.table.count;
- old_dynsymcount = htab->dynsymcount;
- old_dynstr_size = _bfd_elf_strtab_size (htab->dynstr);
+ old_strtab = _bfd_elf_strtab_save (htab->dynstr);
+ if (old_strtab == NULL)
+ goto error_free_vers;
for (i = 0; i < htab->root.table.size; i++)
{
@@ -4762,7 +4786,9 @@ error_free_dyn:
memcpy (htab->root.table.table, old_tab, tabsize);
htab->root.undefs = old_undefs;
htab->root.undefs_tail = old_undefs_tail;
- _bfd_elf_strtab_restore_size (htab->dynstr, old_dynstr_size);
+ _bfd_elf_strtab_restore (htab->dynstr, old_strtab);
+ free (old_strtab);
+ old_strtab = NULL;
for (i = 0; i < htab->root.table.size; i++)
{
struct bfd_hash_entry *p;
@@ -4775,9 +4801,6 @@ error_free_dyn:
h = (struct elf_link_hash_entry *) p;
if (h->root.type == bfd_link_hash_warning)
h = (struct elf_link_hash_entry *) h->root.u.i.link;
- if (h->dynindx >= old_dynsymcount
- && h->dynstr_index < old_dynstr_size)
- _bfd_elf_strtab_delref (htab->dynstr, h->dynstr_index);
/* Preserve the maximum alignment and size for common
symbols even if this dynamic lib isn't on DT_NEEDED
@@ -5099,6 +5122,8 @@ error_free_dyn:
error_free_vers:
if (old_tab != NULL)
free (old_tab);
+ if (old_strtab != NULL)
+ free (old_strtab);
if (nondeflt_vers != NULL)
free (nondeflt_vers);
if (extversym != NULL)
diff --git a/ld/testsuite/ld-ifunc/ifunc.exp b/ld/testsuite/ld-ifunc/ifunc.exp
index e860f36..e01e264 100644
--- a/ld/testsuite/ld-ifunc/ifunc.exp
+++ b/ld/testsuite/ld-ifunc/ifunc.exp
@@ -384,6 +384,31 @@ run_cc_link_tests [list \
{} \
"libpr16467c.a" \
] \
+ [list \
+ "Build libpr16467d.a" \
+ "" \
+ "-fPIC" \
+ { pr16467d.c } \
+ {} \
+ "libpr16467d.a" \
+ ] \
+ [list \
+ "Build libpr16467d.so" \
+ "-shared tmpdir/pr16467d.o tmpdir/libpr16467a.so \
+ -Wl,--version-script=pr16467b.map" \
+ "-fPIC" \
+ { dummy.c } \
+ {} \
+ "libpr16467d.so" \
+ ] \
+ [list \
+ "Build libpr20159.so" \
+ "-shared" \
+ "-fPIC" \
+ { pr20159b.c } \
+ {} \
+ "libpr20159.so" \
+ ] \
]
run_ld_link_exec_tests [] [list \
@@ -441,6 +466,22 @@ run_ld_link_exec_tests [] [list \
"" \
] \
[list \
+ "Run pr16467d" \
+ "tmpdir/pr16467c.o tmpdir/libpr16467d.so tmpdir/libpr16467a.so" \
+ "" \
+ { dummy.c } \
+ "pr16467d" \
+ "pr16467.out" \
+ ] \
+ [list \
+ "Run pr20159" \
+ "tmpdir/libpr20159.so" \
+ "" \
+ { pr20159a.c } \
+ "pr20159" \
+ "pr20159.out" \
+ ] \
+ [list \
"Run ifunc-main" \
"tmpdir/libifunc-lib.so" \
"" \
diff --git a/ld/testsuite/ld-ifunc/pr16467d.c b/ld/testsuite/ld-ifunc/pr16467d.c
new file mode 100644
index 0000000..c8d662b
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr16467d.c
@@ -0,0 +1,11 @@
+const char *new_sd_get_seats (void);
+__asm__ (".symver new_sd_get_seats,sd_get_seats@LIBSYSTEMD_209");
+
+const char *(*sd_get_seats_p) (void) = new_sd_get_seats;
+
+const char *sd_get_seats (void)
+{
+ if (sd_get_seats_p == &sd_get_seats)
+ return "LOOP";
+ return (*sd_get_seats_p) ();
+}
diff --git a/ld/testsuite/ld-ifunc/pr20159.out b/ld/testsuite/ld-ifunc/pr20159.out
new file mode 100644
index 0000000..e69de29
diff --git a/ld/testsuite/ld-ifunc/pr20159a.c b/ld/testsuite/ld-ifunc/pr20159a.c
new file mode 100644
index 0000000..3fcb051
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr20159a.c
@@ -0,0 +1,20 @@
+#include <stdlib.h>
+#include <time.h>
+
+extern time_t libtime (void);
+
+time_t
+time (time_t *tloc)
+{
+ return 42;
+}
+
+int
+main (void)
+{
+ if (time (NULL) != 42)
+ abort ();
+ if (libtime () != 42)
+ abort ();
+ return 0;
+}
diff --git a/ld/testsuite/ld-ifunc/pr20159b.c b/ld/testsuite/ld-ifunc/pr20159b.c
new file mode 100644
index 0000000..5c17004
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr20159b.c
@@ -0,0 +1,7 @@
+#include <time.h>
+
+time_t
+libtime (void)
+{
+ return time (NULL);
+}
--
Alan Modra
Australia Development Lab, IBM