From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15486 invoked by alias); 13 Aug 2012 21:41:05 -0000 Received: (qmail 15474 invoked by uid 22791); 13 Aug 2012 21:41:02 -0000 X-SWARE-Spam-Status: No, hits=-3.7 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 13 Aug 2012 21:40:47 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1T12NS-0003CK-6F from Maciej_Rozycki@mentor.com for binutils@sourceware.org; Mon, 13 Aug 2012 14:40:46 -0700 Received: from SVR-IES-FEM-02.mgc.mentorg.com ([137.202.0.106]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Mon, 13 Aug 2012 14:40:46 -0700 Received: from [172.30.6.46] (137.202.0.76) by SVR-IES-FEM-02.mgc.mentorg.com (137.202.0.106) with Microsoft SMTP Server id 14.1.289.1; Mon, 13 Aug 2012 22:40:44 +0100 Date: Tue, 14 Aug 2012 00:14:00 -0000 From: "Maciej W. Rozycki" To: Subject: [PING] LD: Hidden/internal versioned symbol preemption bug Message-ID: User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2012-08/txt/msg00240.txt.bz2 Hi, 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 from Wednesday. Included below verbatim for easy reference and the complete series posted here: http://sourceware.org/ml/binutils/2012-08/msg00159.html http://sourceware.org/ml/binutils/2012-08/msg00160.html http://sourceware.org/ml/binutils/2012-08/msg00161.html I will appreciate your feedback. Maciej ---------- Forwarded message ---------- Message-ID: Date: Wed, 8 Aug 2012 13:19:40 +0100 From: Maciej W. Rozycki To: binutils@sourceware.org Cc: Alan Modra Subject: [PATCH 1/3] LD: Hidden/internal versioned symbol preemption bug Hi, 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 n64): -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 (...) 2.22.52.20120308 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 (...) 2.22.52.20120308 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); and: 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 address. 2. For MIPS an assertion failure is triggered as noted above and the link fails. 3. For Power a broken binary is produced that has an incorrect dynamic relocation: 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 correctly: http://sourceware.org/ml/binutils/2012-05/msg00383.html 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; and: 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; or: 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 above. 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 bfd/ * elflink.c (_bfd_elf_merge_symbol): Also override the version a dynamic symbol defaulted to if preempted with a hidden or internal definition. Maciej binutils-bfd-preempt-indirect.diff Index: binutils-fsf-trunk-quilt/bfd/elflink.c =================================================================== --- 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; } else - { - 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; } else