[Bug c/64379] New: VFP register restore in ARM epilogue can break indirect tailcalls

[Bug c/64379] New: VFP register restore in ARM epilogue can break indirect tailcalls

[donn.seeley at windriver dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

            Bug ID: 64379
           Summary: VFP register restore in ARM epilogue can break
                    indirect tailcalls
           Product: gcc
           Version: 4.9.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: donn.seeley at windriver dot com

Created attachment 34314
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34314&action=edit
reduced from content.c in xfsdump

In the example, the compiler uses a VFP/NEON register to hold a 64-bit scalar. 
Restoring the saved VFP/NEON register from the stack stomps on IP.  But the
indirect tailcall uses all 4 argument registers, so the function pointer is in
IP, which got trashed:

        sub     ip, fp, #44
        fldmfdd ip!, {d8}
        sub     sp, fp, #36
        ldmfd   sp, {r4, r5, r6, r7, r8, r9, fp, sp, lr}
        bx      ip      @ indirect register sibling call

The code branches to a stack location and the program segfaults.

[Bug c/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[donn.seeley at windriver dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

--- Comment #1 from Donn Seeley <donn.seeley at windriver dot com> ---
Created attachment 34315
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34315&action=edit
generated assembly code for content.i

[Bug target/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[donn.seeley at windriver dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

--- Comment #2 from Donn Seeley <donn.seeley at windriver dot com> ---
Created attachment 34316
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34316&action=edit
compiler parameters for the build

[Bug target/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[donn.seeley at windriver dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

--- Comment #4 from Donn Seeley <donn.seeley at windriver dot com> ---
Created attachment 34319
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34319&action=edit
main.i to go with content.i

I copied content.i to main.i and added just enough material to get it to
compile and (hopefully) run.  I then built two versions of the example:

  arm-wrs-linux-gnueabi-gcc -march=armv7-a -mfloat-abi=softfp -mfpu=neon -marm
-mthumb-interwork -O1 -pipe -mapcs-frame -c content.i
  arm-wrs-linux-gnueabi-gcc -march=armv7-a -mfloat-abi=softfp -mfpu=neon -marm
-mthumb-interwork -O1 -pipe -mapcs-frame -c main.i
  arm-wrs-linux-gnueabi-gcc -march=armv7-a -mfloat-abi=softfp -mfpu=neon -marm
-mthumb-interwork -O1 -pipe -mapcs-frame
--sysroot=/home/donn/c/7.x/wrl-projects/qemuarma9-neon-standard-glibc_std/bitbake_build/tmp/sysroots/qemuarma9
content.o main.o -o without-sibcalls

  arm-wrs-linux-gnueabi-gcc -march=armv7-a -mfloat-abi=softfp -mfpu=neon -marm
-mthumb-interwork -O1 -foptimize-sibling-calls -pipe -mapcs-frame -c content.i
  arm-wrs-linux-gnueabi-gcc -march=armv7-a -mfloat-abi=softfp -mfpu=neon -marm
-mthumb-interwork -O1 -foptimize-sibling-calls -pipe -mapcs-frame -c main.i
  arm-wrs-linux-gnueabi-gcc -march=armv7-a -mfloat-abi=softfp -mfpu=neon -marm
-mthumb-interwork -O1 -foptimize-sibling-calls -pipe -mapcs-frame
--sysroot=/home/donn/c/7.x/wrl-projects/qemuarma9-neon-standard-glibc_std/bitbake_build/tmp/sysroots/qemuarma9
content.o main.o -o with-sibcalls

I booted up QEMU and ran the programs:

  root@qemu0:~# ./without-sibcalls
  root@qemu0:~# ./with-sibcalls
  Segmentation fault
  root@qemu0:~#

[Bug target/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[mikpelinux at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

--- Comment #5 from Mikael Pettersson <mikpelinux at gmail dot com> ---
I can reproduce the wrong-code with gcc-4.9.2 on armv7l-linux-gnueabi.  Appears
to need both -foptimize-sibling-calls and -mapcs-frame to trigger.

[Bug target/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[mikpelinux at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

Mikael Pettersson <mikpelinux at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikpelinux at gmail dot com,
                   |                            |vmakarov at gcc dot gnu.org

--- Comment #6 from Mikael Pettersson <mikpelinux at gmail dot com> ---
Started with r204752, still occurs with today's trunk.  Author CC:d.

Note this may turn out to be a target bug, like PR62662 which also started at
this revision.

Note 2: r204752 causes cc1 to crash and hang deep inside glibc on my system, I
also have to apply r204771 to unbreak it.

[Bug target/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[donn.seeley at windriver dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

--- Comment #7 from Donn Seeley <donn.seeley at windriver dot com> ---
Re -mapcs-frame:  Yes, the failure is due to code in
arm_expand_epilogue_apcs_frame() that explicitly requires / stomps on IP in
order to restore VFP/NEON registers.  Sorry, I should have mentioned that.  The
indirect tailcall register is chosen from CALLER_SAVE_REGS via the "Cs"
constraint, and CALLER_SAVE_REGS contains the arg regs plus IP; if the arg regs
are in use, then the tailcall register ends up being IP.  Hence the conflict.

[Bug target/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-01-13
                 CC|                            |ramana at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #8 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
(In reply to Donn Seeley from comment #7)
> Re -mapcs-frame:  Yes, the failure is due to code in
> arm_expand_epilogue_apcs_frame() that explicitly requires / stomps on IP in
> order to restore VFP/NEON registers.  Sorry, I should have mentioned that. 
> The indirect tailcall register is chosen from CALLER_SAVE_REGS via the "Cs"
> constraint, and CALLER_SAVE_REGS contains the arg regs plus IP; if the arg
> regs are in use, then the tailcall register ends up being IP.  Hence the
> conflict.


Uggh - these ancient options in the backend. mapcs-frame is quite ancient and
isn't really something that's tested very often, no wonder it's rotting. In
this case we seem to have regressed something that worked earlier, I do wonder
if the fix here is to just turn IP regnum into a fixed register in the presence
of mapcs-frame.


regards
Ramana

[Bug target/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu.org

--- Comment #9 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Uggh - these ancient options in the backend. mapcs-frame is quite ancient
> and isn't really something that's tested very often, no wonder it's rotting.
> In this case we seem to have regressed something that worked earlier,

Yes, indirect tailcalls used to be rejected by the back-end.

> I do wonder if the fix here is to just turn IP regnum into a fixed register in
> the presence of mapcs-frame.

This would only turn the problem into an ICE here.  I think this needs to be
fixed in arm_function_ok_for_sibcall, either in a big-hammer-ish way (disable
again indirect tailcalls with -mapcs-frame) or in a more precise way.

[Bug target/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

--- Comment #10 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
(In reply to Eric Botcazou from comment #9)
> > Uggh - these ancient options in the backend. mapcs-frame is quite ancient
> > and isn't really something that's tested very often, no wonder it's rotting.
> > In this case we seem to have regressed something that worked earlier,
> 
> Yes, indirect tailcalls used to be rejected by the back-end.

Indeed.

> 
> > I do wonder if the fix here is to just turn IP regnum into a fixed register in
> > the presence of mapcs-frame.
> 
> This would only turn the problem into an ICE here.  

Hmmmm not sure.

> I think this needs to be
> fixed in arm_function_ok_for_sibcall, either in a big-hammer-ish way
> (disable again indirect tailcalls with -mapcs-frame) or in a more precise
> way.

Sure that would work too but it feels like a rather big hammer. In the absence
of anything else, I'm happy to review a patch if someone could post this
upstream and get it tested, I won't have time to work on this bug for the next
week or 2.

While we are here, what's the thought on deprecating mapcs-frame in the
arm-wrs-vxworks community ?  It is not really something that's part of the ABI
(it predates EABI), doesn't work with Thumb2 and not tested regularly and we
keep finding issues with this.

Can arm-wrs-vxworks move to using the EHABI information and unwinding
information from there to handle such backtraces - after all libunwind has the
capability now for unwinding ARM EHABI tables if you needed a generic library
to do this ? 


regards
Ramana

[Bug target/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

--- Comment #11 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> While we are here, what's the thought on deprecating mapcs-frame in the
> arm-wrs-vxworks community ?  It is not really something that's part of the
> ABI (it predates EABI), doesn't work with Thumb2 and not tested regularly
> and we keep finding issues with this.
> 
> Can arm-wrs-vxworks move to using the EHABI information and unwinding
> information from there to handle such backtraces - after all libunwind has
> the capability now for unwinding ARM EHABI tables if you needed a generic
> library to do this ? 

Let me check what the status of the just released VxWorks 7 is with the
resident expert here at AdaCore.  IIUC there is a clear move to EABI, but I'm
not sure if it is complete.

[Bug target/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[donn.seeley at windriver dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

--- Comment #12 from Donn Seeley <donn.seeley at windriver dot com> ---
I talked to the folks who added -mapcs-frame to our builds, and it appears that
we don't actually need it any more.  We'll remove it and see what happens.

[Bug target/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[donn.seeley at windriver dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

--- Comment #13 from Donn Seeley <donn.seeley at windriver dot com> ---
BTW, this issue cropped up in Wind River Linux testing, not VxWorks.  I have no
idea whether the VxWorks folks are using -mapcs-frame.  WR Linux will remove
the -mapcs-frame flag from future builds; it was originally added to support
patches to oprofile that we have since dropped.

[Bug target/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

--- Comment #14 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
(In reply to Donn Seeley from comment #13)
> BTW, this issue cropped up in Wind River Linux testing, not VxWorks.  I have
> no idea whether the VxWorks folks are using -mapcs-frame.  WR Linux will
> remove the -mapcs-frame flag from future builds; it was originally added to
> support patches to oprofile that we have since dropped.

Yes I've been asking the Linux kernel folks why they are using mapcs-frame. I'd
think if this has been added only for oprofile, this should now be dropped from
usage.

Thanks for the input on VxWorks, we'll draft something up about deprecating
this option.

Ramana

[Bug target/64379] VFP register restore in ARM epilogue can break indirect tailcalls

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64379

--- Comment #15 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Thanks for the input on VxWorks, we'll draft something up about deprecating
> this option.

Note that the system compiler uses APCS frames and DWARF2 EH on VxWorks 6,
which is the mainstream version as of this writing, so this cannot be done too
quickly
for the sake of compatibility with it.