Created attachment 5270 [details] Compiler output demonstrating the problem Gas sometimes emits a short branch and an R_ARM_THUMB_JUMP11 relocation for an unqualified branch ("b") to a global symbol defined in the same source file. However, there's no guarantee that R_ARM_THM_JUMP11 relocation can be resolved correctly if the symbol gets preempted. Observed in binutils-arm-linux-gnueabi 2.20.51.20100908-0ubuntu2cross1.52. Observed on binutils trunk on 2011-02-25. The problem seems to strike in practice when the compiler optimises a sibling call to "b <label>", for which the assembler generates a b.n. Handed-coded assembler can also cause the same problem. As a side-effect of the presence of this relocation, Thumb-2 kernel modules may fail to load, since the kernel doesn't support fixing up this relocation. I believe the kernel doesn't do any symbol preemption processing when loading modules -- if so, the relocation is actually redundant. But there's no way for the kernel to detect at module load time that the relocation can safely be ignored. In kernel builds, about 1 out every 6 modules suffers from this problem. It's not clear to me whether the method used by the kernel to link .ko objects is wrong or not. It seems that either the tools should support symbol preemption in this scenario (implying that a short branch is not adequate to guarantee that preemption will work -- i.e., a gas bug) or not (in this case the fixup should be done locally and there should presumably be no relocation -- i.e., a different gas bug). It appears that passing -fno-optimize-sibling-calls to GCC can work around the problem by avoiding the generation of local branches to global symbols, but this is clearly not the correct long-term fix.
Created attachment 5271 [details] sample disassembly and relocation info
Created attachment 5662 [details] Do not relax relocations to preemptable global symbols
Hi Dave, I think that we should be supporting the possibility of symbol preemption. So please could you try out the uploaded patch and let me know if it resolves the problem for you. Cheers Nick PS. Hmm, I wonder if we should support a command line feature to enable/disable this behaviour, and what the default setting should be...
(In reply to comment #3) > Hi Dave, > > I think that we should be supporting the possibility of symbol preemption. So > please could you try out the uploaded patch and let me know if it resolves the > problem for you. > > Cheers > Nick > > PS. Hmm, I wonder if we should support a command line feature to > enable/disable this behaviour, and what the default setting should be... Your patch in http://sourceware.org/bugzilla/show_bug.cgi?id=12532#c2 seems to fix my instance of the problem. Do you know when this is likely to get merged? I don't see it in binutils trunk yet. Cheers ---Dave
CVSROOT: /cvs/src Module name: src Changes by: nickc@sourceware.org 2011-04-12 11:47:38 Modified files: gas : ChangeLog gas/config : tc-arm.c Log message: PR gas/12532 * config/tc-arm.c (relax_branch): Do not relax branches to preemptable global symbols. Patches: http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/ChangeLog.diff?cvsroot=src&r1=1.4464&r2=1.4465 http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/config/tc-arm.c.diff?cvsroot=src&r1=1.479&r2=1.480
Hi Dave > Do you know when this is likely to get merged? I don't see it in binutils > trunk yet. That would be because I was waiting for your agreement that the patch worked... Anyway it is all checked in now. Cheers Nick
On Tue, Apr 12, 2011 at 12:49 PM, nickc at redhat dot com <sourceware-bugzilla@sourceware.org> wrote: > http://sourceware.org/bugzilla/show_bug.cgi?id=12532 > > Nick Clifton <nickc at redhat dot com> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > Status|WAITING |RESOLVED > Resolution| |FIXED > > --- Comment #6 from Nick Clifton <nickc at redhat dot com> 2011-04-12 11:49:05 UTC --- > Hi Dave > >> Do you know when this is likely to get merged? I don't see it in binutils >> trunk yet. > > That would be because I was waiting for your agreement that the patch worked... > > Anyway it is all checked in now. OK! Thanks ---Dave
CVSROOT: /cvs/src Module name: src Changes by: nickc@sourceware.org 2011-04-12 15:44:36 Modified files: gas/testsuite : ChangeLog gas/testsuite/gas/arm: weakdef-1.d plt-1.d thumb2_bcond.d Log message: PR gas/12532 * gas/arm/plt-1.d: Update expected disassembly. * gas/arm/thumb2_bcond.d: Likewise. * gas/arm/weakdef-1.d: Likewise. Patches: http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1882&r2=1.1883 http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/weakdef-1.d.diff?cvsroot=src&r1=1.1&r2=1.2 http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/plt-1.d.diff?cvsroot=src&r1=1.1&r2=1.2 http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/thumb2_bcond.d.diff?cvsroot=src&r1=1.6&r2=1.7
This problem still exists on binutils 2.33 when -fvisibility=hidden is passed to cflags. I imagine this is so due to some conflicting code where the forced B.W is only generated for static functions, since non-static ones will be relocated differently, but then because of -fvisibility=hidden, they get treated like statics, only B is used instead of the forced B.W, causing this issue to crop up again. OpenWRT experienced this when including WireGuard on a new board. I fixed it like this: https://git.zx2c4.com/wireguard-linux-compat/commit/?id=178cdfffb99f2fd6fb4a5bfd2f9319461d93f53b
Tracking the new bug here now: https://sourceware.org/bugzilla/show_bug.cgi?id=26141