This is the mail archive of the
mailing list for the binutils project.
[PING] LD: Hidden/internal versioned symbol preemption bug
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: <binutils at sourceware dot org>
- Date: Mon, 13 Aug 2012 22:40:35 +0100
- Subject: [PING] LD: Hidden/internal versioned symbol preemption bug
Is there anything I could do to help with the review of this change?
It is a pretty fatal failure affecting all targets, so I would like to
see it fixed in the upcoming release which I have been told is happening
by the end of this month and I'll be disappearing for a fortnight starting
Included below verbatim for easy reference and the complete series posted
I will appreciate your feedback.
---------- Forwarded message ----------
Date: Wed, 8 Aug 2012 13:19:40 +0100
From: Maciej W. Rozycki <email@example.com>
Cc: Alan Modra <firstname.lastname@example.org>
Subject: [PATCH 1/3] LD: Hidden/internal versioned symbol preemption bug
While working on the MIPS _gp special symbol scope bug I have addressed
recently I ran the glibc test suite to double-check the bug fix did not
have any adverse effect on the library and stumbled across this mysterious
progression for PLT (o32) multilibs only (the test already succeeded for
-FAIL: build dlfcn/bug-atexit3-lib.so
for PLT multilibs only (the test already succeeded for n64). Upon a
closer inspection I have discovered the following link failure,
mysteriously, has been removed:
.../mips-linux-gnu/bin/ld: BFD (...) 126.96.36.19920308 assertion fail .../bfd/elfxx-mips.c:3475
.../lib/gcc/mips-linux-gnu/4.6.3/libgcc_eh.a(unwind-dw2.o): In function `_Unwind_FindEnclosingFunction':
.../gcc/unwind-dw2.c:311:(.text+0x35b4): relocation truncated to fit: R_MIPS_CALL16 against`_Unwind_Find_FDE@@GCC_3.0'
collect2: ld returned 1 exit status
and the test fully passed. There was another assertion failure
immediately preceding though:
.../mips-linux-gnu/bin/ld: BFD (...) 188.8.131.5220308 assertion fail .../bfd/elfxx-mips.c:3471
that had been retained and the test harness is not smart enough to choke
on it (there was no link failure, apparently LD included in the somewhat
older version of binutils did not terminate on assertion failures).
These assertions are:
BFD_ASSERT (got_index < htab->sgot->size);
BFD_ASSERT (h->dynindx >= global_got_dynindx);
respectively in mips_elf_global_got_index and the latter assertion
triggered across all multilibs including SVR4 stubs ones (n64).
This is quite obscure, but I was able to reduce the scenario as follows.
A shared library is being linked. There are a couple of shared libraries
involved in the link and there are also a number of static archives pulled
from. There is a symbol reference from one of these static archives that
is satisfied with a versioned symbol defined in one of the shared
libraries. A GOT entry is assigned for the symbol reference.
Later on, another archive member is pulled that overrides the versioned
symbol with a hidden definition. The GOT entry for the old reference is
discarded and the linker is supposed to have overwritten the definition
from the shared library with the hidden one that will be ultimately used
within this shared library being linked.
However when the dynamic object is finally layed out, the original
reference to the versioned symbol turns out to have been retained and a
GOT reference is attempted to be finalised to satisfy the respective
relocation. This ends up unsuccessful as the entry assignment has already
beed discarded (dynindx set to -1). This causes the assertion failure,
and also a link failure in our current sources.
On further debugging this issue I made the following observations. The
bug affects all Linux targets (with a representative sample actually
examined), causing different failure modes:
1. For ARM a broken binary is produced that uses the wrong symbol's final
2. For MIPS an assertion failure is triggered as noted above and the link
3. For Power a broken binary is produced that has an incorrect dynamic
000101d0 ffffff01 R_PPC_ADDR32 bad symbol index: 00ffffff
4. For x86 the link fails:
unresolvable R_386_32 relocation against symbol `_Unwind_Find_FDE@@GCC_3.0'
final link failed: Nonrepresentable section on output
The bug can be reproduced with a simple test case involving three symbols
and four objects used in the link -- three relocatables and one shared
library. If a non-versioned symbol is used instead, then the binary
produced is correct.
I have finally tracked down the cause to a piece of code in
_bfd_elf_merge_symbol. Interestingly enough this code has been recently
updated to handle the case of overriding with a protected symbol
but the corresponding case of a hidden/internal symbol has not been
adjusted accordingly. The rules of preemption are the same for all the
three non-default export classes (any dynamic definition must be removed
from the scope, it won't be used in the module produced in this static
link) and the fix below therefore borrows from code immediately following
that is used to override the indirect dynamic symbol (or just The Symbol
when no versioning has been applied).
Please note that as a side effect this change also removes some code that
became dead as a result of the change referred to in the previous
paragraph, specifically these assignments:
vh->dynamic_def = 1;
vh->ref_dynamic = 1;
vh->ref_dynamic = 0;
in the respective conditional blocks, because at the conclusion of the
containing block this assignment is made:
h = vh;
and a piece of code below then sets:
h->ref_dynamic = 0;
h->ref_dynamic = 1;
as appropriate and:
h->dynamic_def = 0;
Likewise there's no need to call:
(*bed->elf_backend_hide_symbol) (info, vh, TRUE);
that's already made for h below under the same condition.
This fix removes the failures noted above for the four targets and causes
the correct value of the hidden/internal symbol to be used for relocation.
It causes no regressions for a superset of Alan's favourite targets. It
also causes no regressions in i686-linux, x86_64-linux, mipsel-linux or
mips64el-linux native testing (regrettably most export class tests are run
in native testing only; I can't test non-x86 non-MIPS native targets
reasonably easily or at all).
OK to apply?
I'll follow up with two other changes:
1. A piece to add a set of assertions to the Power target to guard against
dynindx being minus one similar to what MIPS and other targets use.
One of these assertions triggers as appropriate (depending on the
relocation type involved) and prevents the value of minus one to
propagate to the final binary's dynamic relocation table as observed
2. A set of test cases to cover non-default export class preemption of a
versioned symbol. These have been adapted for the LD test suite from
code reduced from the case included in glibc.
2012-08-08 Maciej W. Rozycki <email@example.com>
* elflink.c (_bfd_elf_merge_symbol): Also override the version
a dynamic symbol defaulted to if preempted with a hidden or
--- binutils-fsf-trunk-quilt.orig/bfd/elflink.c 2012-07-19 00:13:38.630594071 +0100
+++ binutils-fsf-trunk-quilt/bfd/elflink.c 2012-07-19 00:20:21.130618721 +0100
@@ -1210,23 +1210,25 @@ _bfd_elf_merge_symbol (bfd *abfd,
vh->root.type = h->root.type;
h->root.type = bfd_link_hash_indirect;
(*bed->elf_backend_copy_indirect_symbol) (info, vh, h);
- /* Protected symbols will override the dynamic definition
- with default version. */
- if (ELF_ST_VISIBILITY (sym->st_other) == STV_PROTECTED)
+ h->root.u.i.link = (struct bfd_link_hash_entry *) vh;
+ if (ELF_ST_VISIBILITY (sym->st_other) != STV_PROTECTED)
- h->root.u.i.link = (struct bfd_link_hash_entry *) vh;
- vh->dynamic_def = 1;
- vh->ref_dynamic = 1;
+ /* If the new symbol is hidden or internal, completely undo
+ any dynamic link state. */
+ (*bed->elf_backend_hide_symbol) (info, h, TRUE);
+ h->forced_local = 0;
+ h->ref_dynamic = 0;
- h->root.type = vh->root.type;
- vh->ref_dynamic = 0;
- /* We have to hide it here since it was made dynamic
- global with extra bits when the symbol info was
- copied from the old dynamic definition. */
- (*bed->elf_backend_hide_symbol) (info, vh, TRUE);
+ h->ref_dynamic = 1;
+ h->def_dynamic = 0;
+ h->dynamic_def = 0;
+ /* FIXME: Should we check type and size for protected symbol? */
+ h->size = 0;
+ h->type = 0;
h = vh;