User account creation filtered due to spam.
A tail-call of weak function produces incorrect code, compiled with -O2 or -O3. Target: arm-eabi Version: GCC 4.6.2 and 4.6.1 ***** Test case: t00x1.c <<<<< extern void __attribute__((weak)) wfunc(void); void main(void) { wfunc(); // tail weak call fails!!! // __asm__ volatile ("nop":::"memory"); // workaround } >>>>> $ /opt/arm/gnuarm-4.6.2/bin/arm-eabi-gcc -c -S -o t00x1.S -O2 t00x1.c ***** Contents of t00x1.S: .file "t00x1.c" .section .text.startup,"ax",%progbits .align 2 .global main .type main, %function main: @ args = 0, pretend = 0, frame = 0 @ frame_needed = 0, uses_anonymous_args = 0 @ link register save eliminated. b wfunc @@@@ <<< Will be replaced by "NOP" in linker!! @@@@ so, no return sequence! .size main, .-main .weak wfunc .ident "GCC: (GNU) 4.6.2" ***** Disassembled (objdump) code: t00x1.lst: 00000240 <main>: 240: e1a00000 nop ; (mov r0, r0) @@@@ really, NOP 00000244 <__do_global_dtors_aux>: 244: e92d4010 push {r4, lr} 248: e59f4028 ldr r4, [pc, #40] ; 278 ....... ****** Call to function "wfunc" was converted to NOP by linker, as expected. As a result, no return sequence from main(), falling througt to next code line. For reference: the same program compiled with -O1 ******* .file "t00x1.c" .text .align 2 .global main .type main, %function main: @ args = 0, pretend = 0, frame = 0 @ frame_needed = 0, uses_anonymous_args = 0 stmfd sp!, {r3, lr} bl wfunc ldmfd sp!, {r3, pc} .size main, .-main .weak wfunc .ident "GCC: (GNU) 4.6.2" Code with workaround, compiled with -O2 correct: $ /opt/arm/gnuarm-4.6.2/bin/arm-eabi-gcc -c -S -o t00x1.S -O2 t00x1.c ******* .file "t00x1.c" .section .text.startup,"ax",%progbits .align 2 .global main .type main, %function main: @ args = 0, pretend = 0, frame = 0 @ frame_needed = 0, uses_anonymous_args = 0 stmfd sp!, {r3, lr} bl wfunc @ 5 "t00x1.c" 1 nop @ 0 "" 2 ldmfd sp!, {r3, pc} .size main, .-main .weak wfunc .ident "GCC: (GNU) 4.6.2" ******* Compiler options: $ /opt/arm/gnuarm-4.6.2/bin/arm-eabi-gcc -v Using built-in specs. COLLECT_GCC=/opt/arm/gnuarm-4.6.2/bin/arm-eabi-gcc COLLECT_LTO_WRAPPER=/opt/arm/gnuarm-4.6.2/libexec/gcc/arm-eabi/4.6.2/lto-wrapper Target: arm-eabi Configured with: ../../src/gcc-4.6.2/configure --target=arm-eabi --prefix=/opt/arm/gnuarm-4.6.2 --enable-multilib --enable-interwork --enable-biendian --enable-fpu --with-newlib --with-gnu-ld --with-gnu-as --disable-nls --disable-shared --with-arch=armv5te --with-fpu=vfp --with-float=softfp --with-abi=aapcs-linux --enable-lto --enable-languages=c,c++ --disable-threads --enable-ppl --enable-cloog --enable-gmp --enable-mpfr --enable-lto -with-headers=/opt/arm/gnuarm-4.6.2/arm-eabi/include Thread model: single gcc version 4.6.2 (GCC)
I think this code is undefined. You are calling a weak function without checking if it is null.
Also the linker seems funny to replace a branch to null with a nop.
This behavior is explicitly defined in ARM RealView compiler, and GCC seems try to follow this convention.
From ARM EABI specification (doc: ARM IHI 0044A) "On platforms that do not support dynamic pre-emption of symbols an unresolved weak reference to a symbol relocated by R_ARM_CALL shall be treated as a jump to the next instruction (the call becomes a no-op). The behaviour of R_ARM_JUMP24 in these conditions is unspecified." At least, tail call + epilogue shall not be replaced by (undefined) JUMP.
On Tue, 20 Dec 2011, sipych at gmail dot com wrote: > "On platforms that do not support dynamic pre-emption of symbols an unresolved > weak reference to a symbol relocated by R_ARM_CALL shall be treated as a jump > to the next instruction (the call becomes a no-op). The behaviour of > R_ARM_JUMP24 in these conditions is unspecified." > > At least, tail call + epilogue shall not be replaced by (undefined) JUMP. That's an ABI specifying relocation handling, not an API specifying semantics of C source code. The C semantics are as specified in the C standard, i.e. undefined behavior if the call is executed; C is a high-level language not a portable assembler and the semantics are those of the C standard not those of particular instructions you might guess have some relation to particular C operations. The EABI semantics say what happens with a relocation that might have been generated from code where the call is properly conditional in the C source, or might have been generated from code with undefined behavior. It would be valid for the compiler to rely on the EABI semantics to optimize code generation for "if (wfunc) wfunc ();", for example, but those semantics do not change the undefined nature of code that doesn't condition the call.
It seems reasonable to expect minimal consistency, either generating invalid (zero for example) reference for any direct weak function call, better marking it with "warning" or even "error" message, or not depending on the optimization options. In any case, the semantics of weak referencies is beyond of the C standard scope, so even "if( wfunc ) wfunc();" may be treated only in terms of reasonable consistency. EABI specification follows a simple rule: "call to undefined weak function do nothing". Just the same do "if( wfunc ) wfunc();", a little bit longer, if we believe that undefined wfunc == 0 always, but not 0xDEADBEEF eventually when optimized.
Technically this bug is invalid (undefined is undefined, not consistent). ARM maintainers may want to disallow tail/sibling calls to possibly weak targets. The ABI is inconsistent itself, btw, as weak_fn (); beahaves different than *(&weak_fn) (); as I presume the linker cannot do anything for indirect calls to weak functions.
Author: rearnsha Date: Thu Dec 22 14:13:09 2011 New Revision: 182621 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182621 Log: PR target/51643 * arm.c (arm_function_ok_for_sibcall): Don't try to tailcall a weak function on bare-metal EABI targets. * gcc.target/arm/sibcall-2.c: New test. Added: trunk/gcc/testsuite/gcc.target/arm/sibcall-2.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/arm/arm.c trunk/gcc/testsuite/ChangeLog
Author: rearnsha Date: Thu Dec 22 14:28:39 2011 New Revision: 182622 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182622 Log: PR target/51643 * arm.c (arm_function_ok_for_sibcall): Don't try to tailcall a weak function on bare-metal EABI targets. * gcc.target/arm/sibcall-2.c: New test. Added: branches/gcc-4_6-branch/gcc/testsuite/gcc.target/arm/sibcall-2.c Modified: branches/gcc-4_6-branch/gcc/ChangeLog branches/gcc-4_6-branch/gcc/config/arm/arm.c branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
Fixed on 4.6 and trunk.
Author: rearnsha Date: Thu Dec 22 17:31:50 2011 New Revision: 182628 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182628 Log: PR target/51643 * arm.c (arm_function_ok_for_sibcall): Use DECL_WEAK in previous change. Modified: trunk/gcc/ChangeLog trunk/gcc/config/arm/arm.c
Author: rearnsha Date: Thu Dec 22 17:32:58 2011 New Revision: 182629 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182629 Log: PR target/51643 * arm.c (arm_function_ok_for_sibcall): Use DECL_WEAK in previous change. Modified: branches/gcc-4_6-branch/gcc/ChangeLog branches/gcc-4_6-branch/gcc/config/arm/arm.c
Thanks, Richard! It now works fine with -mabi=aapcs Perhaps I don't understand some details, could you tell me why the -mabi=aapcs-linux is not included in this fix?
Because the ABI says it only works for bare metal. On a system with shared libraries, you can't tell at static link time if a weak symbol will be resolved by a shared library, so it has to left up to the dynamic linker which will fill in a PLT stub. Once you have those, it's pretty hard to make the call become a stub (and even if it were, it wouldn't be a NOP (the linker would have to turn the call into something that simply returned as the PLT sequence can't be patched). Tail-calling PLT sequences is perfectly safe, so there's no reason not to perform the optimization.
Oh, I see, thanks!
Author: jye2 Date: Fri Jun 8 08:57:53 2012 New Revision: 188332 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188332 Log: 2012-06-08 Joey Ye <joey.ye@arm.com> Backport r184442 from mainline 2012-02-21 Richard Earnshaw <rearnsha@arm.com> PR target/52294 * thumb2.md (thumb2_shiftsi3_short): Split register and immediate shifts. For register shifts tie operands 0 and 1. (peephole2 for above): Check that register-controlled shifts have suitably tied operands. Backport r183756 from mainline 2012-01-31 Matthew Gretton-Dann <matthew.gretton-dann@arm.com> * config/arm/thumb2.md (thumb2_mov_notscc): Use MVN for true condition. Backport r183349 from mainline 2012-01-20 Jakub Jelinek <jakub@redhat.com> PR target/51915 * config/arm/arm.c (arm_count_output_move_double_insns): Call output_move_double on a copy of operands array. Backport r183095 from mainline 2012-01-11 Matthew Gretton-Dann <matthew.gretton-dann@arm.com> * config/arm/arm.md (mov_notscc): Use MVN for false condition. Backport r182628 from mainline 2011-12-21 Richard Earnshaw <rearnsha@arm.com> PR target/51643 * arm.c (arm_function_ok_for_sibcall): Use DECL_WEAK in previous change. Backport r182621 from mainline 2011-12-21 Richard Earnshaw <rearnsha@arm.com> PR target/51643 * arm.c (arm_function_ok_for_sibcall): Don't try to tailcall a weak function on bare-metal EABI targets. Testsuite: Backport r183349 from mainline 2012-01-20 Jakub Jelinek <jakub@redhat.com> PR target/51915 * gcc.target/arm/pr51915.c: New test. Backport r183095 from mainline 2012-01-11 Matthew Gretton-Dann <matthew.gretton-dann@arm.com> * gcc.c-torture/execute/20120110-1.c: New testcase. Backport r182621 from mainline 2011-12-21 Richard Earnshaw <rearnsha@arm.com> PR target/51643 * gcc.target/arm/sibcall-2.c: New test. Added: branches/ARM/embedded-4_6-branch/gcc/testsuite/gcc.c-torture/execute/20120111-1.c branches/ARM/embedded-4_6-branch/gcc/testsuite/gcc.target/arm/pr51915.c branches/ARM/embedded-4_6-branch/gcc/testsuite/gcc.target/arm/sibcall-2.c Modified: branches/ARM/embedded-4_6-branch/gcc/ChangeLog.arm branches/ARM/embedded-4_6-branch/gcc/config/arm/arm.c branches/ARM/embedded-4_6-branch/gcc/config/arm/arm.md branches/ARM/embedded-4_6-branch/gcc/config/arm/thumb2.md branches/ARM/embedded-4_6-branch/gcc/testsuite/ChangeLog.arm