From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18771 invoked by alias); 25 Aug 2009 18:52:11 -0000 Received: (qmail 18760 invoked by uid 22791); 25 Aug 2009 18:52:10 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from NaN.false.org (HELO nan.false.org) (208.75.86.248) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 25 Aug 2009 18:52:02 +0000 Received: from nan.false.org (localhost [127.0.0.1]) by nan.false.org (Postfix) with ESMTP id 88EEC10661; Tue, 25 Aug 2009 18:51:59 +0000 (GMT) Received: from caradoc.them.org (209.195.188.212.nauticom.net [209.195.188.212]) by nan.false.org (Postfix) with ESMTP id 500E71065D; Tue, 25 Aug 2009 18:51:59 +0000 (GMT) Received: from drow by caradoc.them.org with local (Exim 4.69) (envelope-from ) id 1Mg17e-0006OM-Fb; Tue, 25 Aug 2009 14:51:58 -0400 Date: Wed, 26 Aug 2009 01:21:00 -0000 From: Daniel Jacobowitz To: Christophe LYON , binutils@sourceware.org Subject: Fix Thumb-2 shared libraries Message-ID: <20090825185158.GA2712@caradoc.them.org> Mail-Followup-To: Christophe LYON , binutils@sourceware.org References: <4A324BAC.7090702@redhat.com> <4A3255D9.9030906@st.com> <20090612140642.GA31950@caradoc.them.org> <4A32626C.8090407@st.com> <4A3658C4.3000800@st.com> <20090615175935.GA22200@caradoc.them.org> <4A390CF5.3000505@st.com> <20090617162337.GA9315@caradoc.them.org> <4A3A4DEF.5070704@st.com> <4A3A4E62.8030603@st.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4A3A4E62.8030603@st.com> User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes 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: 2009-08/txt/msg00465.txt.bz2 On Thu, Jun 18, 2009 at 04:25:38PM +0200, Christophe LYON wrote: > Oops! I forgot to attach the patch, sorry. > > > On 18.06.2009 16:23, Christophe LYON wrote: > >Hi, > > > >After a bit more work, I am able to propose a new patch for this issue. > >I have enriched the tests to be sure to check calls inside shared > >libs to defined/undefined and ARM/Thumb symbols. > > > >Checked with arm-none-eabi and arm-linux-gnueabi with no regression. > > > >Christophe. Unfortunately, this patch breaks Thumb-2 shared libraries. > @@ -11273,14 +11291,14 @@ allocate_dynrelocs (struct elf_link_hash > { > h->root.u.def.section = s; > h->root.u.def.value = h->plt.offset; > - > - /* Make sure the function is not marked as Thumb, in case > - it is the target of an ABS32 relocation, which will > - point to the PLT entry. */ > - if (ELF_ST_TYPE (h->type) == STT_ARM_TFUNC) > - h->type = ELF_ST_INFO (ELF_ST_BIND (h->type), STT_FUNC); > } > > + /* Make sure the function is not marked as Thumb, in case > + it is the target of an ABS32 relocation, which will > + point to the PLT entry. */ > + if (ELF_ST_TYPE (h->type) == STT_ARM_TFUNC) > + h->type = ELF_ST_INFO (ELF_ST_BIND (h->type), STT_FUNC); > + The comment doesn't make sense outside of the previous if block. What used to happen, inside that block, was we'd mark the function definition in the ELF symbol table as pointing at the PLT entry. At that point, the address of the PLT entry is ARM code, so we had to adjust the function type. But now the symbol table still points to the Thumb code, and we've marked the entry as STT_FUNC. It ends up without the ISA bit set. This causes R_ARM_JUMP_SLOT relocations to jump to the target function as if it were ARM code - does not work. If I revert just this part of your patch, there is one additional failure in the testsuite (thanks, as usual, for test cases!). We use blx to branch to the PLT entry, so I assume that's what this hunk was for. We have to change the use of the symbol in this case, rather than the definition. The test still needs a fixup, because the version that's there now would not work at runtime. It has: .* <__lib_func3_from_thumb>: .*: e59fc000 ldr ip, \[pc, #0\] ; 1000364 <__lib_func3_from_thumb\+0x8> .*: e08ff00c add pc, pc, ip .*: feffff90 .word 0xfeffff90 That will branch to lib_func3 in ARM mode, but lib_func3 is a Thumb function. Here's a patch I'm testing for this failure. -- Daniel Jacobowitz CodeSourcery 2009-08-25 Daniel Jacobowitz * elf32-arm.c (elf32_arm_final_link_relocate): Set sym_flags for the mode of target PLT entries. (allocate_dynrelocs): Only adjust symbol type if setting its value. Index: elf32-arm.c =================================================================== --- elf32-arm.c (revision 259038) +++ elf32-arm.c (working copy) @@ -7330,6 +7330,9 @@ elf32_arm_final_link_relocate (reloc_how + splt->output_offset + h->plt.offset); *unresolved_reloc_p = FALSE; + /* The PLT entry is in ARM mode, regardless of the + target function. */ + sym_flags = STT_FUNC; } /* A branch to an undefined weak symbol is turned into a @@ -7756,10 +7759,14 @@ elf32_arm_final_link_relocate (reloc_how /* If the Thumb BLX instruction is available, convert the BL to a BLX instruction to call the ARM-mode PLT entry. */ lower_insn = (lower_insn & ~0x1000) | 0x0800; + sym_flags = STT_FUNC; } else - /* Target the Thumb stub before the ARM PLT entry. */ - value -= PLT_THUMB_STUB_SIZE; + { + /* Target the Thumb stub before the ARM PLT entry. */ + value -= PLT_THUMB_STUB_SIZE; + sym_flags = STT_ARM_TFUNC; + } *unresolved_reloc_p = FALSE; } @@ -11753,13 +11760,13 @@ allocate_dynrelocs (struct elf_link_hash { h->root.u.def.section = s; h->root.u.def.value = h->plt.offset; - } - /* Make sure the function is not marked as Thumb, in case - it is the target of an ABS32 relocation, which will - point to the PLT entry. */ - if (ELF_ST_TYPE (h->type) == STT_ARM_TFUNC) - h->type = ELF_ST_INFO (ELF_ST_BIND (h->type), STT_FUNC); + /* Make sure the function is not marked as Thumb, in case + it is the target of an ABS32 relocation, which will + point to the PLT entry. */ + if (ELF_ST_TYPE (h->type) == STT_ARM_TFUNC) + h->type = ELF_ST_INFO (ELF_ST_BIND (h->type), STT_FUNC); + } /* Make room for this entry. */ s->size += htab->plt_entry_size;