Launchpad has imported 7 comments from the remote bug at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70674.
If you reply to an imported comment from within Launchpad, your comment will be sent to the remote bug automatically. Read more about Launchpad's inter-bugtracker facilities at https://documentation.ubuntu.com/launchpad/user/reference/bugs/multi-project-bugs/about-multi-project-bugs/#bugs-in-external-trackers. ------------------------------------------------------------------------ On 2016-04-15T03:35:07+00:00 Krebbel wrote: Created attachment 38276 Fix proposal t.c: void foo (void) { volatile int a = 5; (void) a; } cc1 -O2 -fno-omit-frame-pointer -march=z10 -mtune=z196 t.c The assignment to a is moved by the scheduler *after* the stack pointer restore. While not being a problem in this example in other circumstances this might cause data corruption if e.g. a signal handler gets triggered in between. foo: ldgr %f2,%r11 ldgr %f0,%r15 lay %r15,-168(%r15) lgr %r11,%r15 lgdr %r15,%f0 <----- stack pointer restore mvhi 164(%r11),5 <----- stack write for variable a l %r1,164(%r11) lgdr %r11,%f2 br %r14 The variable access is done through the framepointer which does not conflict with the restore of r15. The problem was latent in the backend but was so far hidden by doing the restore of r11 and r15 in the same instruction - a load multiple. However, there always was the potential problem of doing the stack access with a temporary register assigned by the compiler. Reply at: https://bugs.launchpad.net/ubuntu/+source/gcc-5/+bug/1572613/comments/0 ------------------------------------------------------------------------ On 2016-04-15T07:16:59+00:00 Rguenth wrote: please fill in known-to-work/fail fields. If it's present in 4.9 it can't be P1 (but it is P2). If it's only latent on branches and exposed on trunk then it can be P1. Reply at: https://bugs.launchpad.net/ubuntu/+source/gcc-5/+bug/1572613/comments/1 ------------------------------------------------------------------------ On 2016-04-20T07:10:04+00:00 Krebbel wrote: Author: krebbel Date: Wed Apr 20 07:09:32 2016 New Revision: 235233 URL: https://gcc.gnu.org/viewcvs?rev=235233&root=gcc&view=rev Log: PR70674: S/390: Add memory barrier to stack pointer restore from fpr. This patches fixes a problem with stack variable accesses being scheduled after the stack pointer restore instructions. In the testcase this happened with the stack variable 'a' accessed through the frame pointer. The existing stack_tie we have in the backend is basically useless when trying to block stack variable accesses from being scheduled across an insn. The alias set of stack variables and the frame alias set usually differ and hence aren't in conflict with each other. The solution appears to be a magic MEM term with a scratch register which is handled as a full memory barrier when analyzing scheduling dependencies. With the patch a (clobber (mem:BLK (scratch))) is being added to the restore instruction in order to prevent any memory operations to be scheduled across the insn. The patch does that only for the one case where the stack pointer is restored from an FPR. Theoretically this might happen also in the case where the stack pointer gets restored using a load multiple. However, triggering that problem with load-multiple appears to be much harder since the load-multiple will restore the frame pointer as well. So in order to see the problem a different call-clobbered register would need to be used as temporary stack pointer. Another case which needs to be handled some day is the stack pointer allocation part. It needs to be a memory barrier as well. Bootstrapped and regression tested with --with-arch z196 and z13 on s390 and s390x. gcc/ChangeLog: 2016-04-20 Andreas Krebbel <[email protected]> Backport from mainline 2016-04-20 Andreas Krebbel <[email protected]> PR target/70674 * config/s390/s390.c (s390_restore_gprs_from_fprs): Pick the new stack_restore_from_fpr pattern when restoring r15. (s390_optimize_prologue): Strip away the memory barrier in the parallel when trying to get rid of restore insns. * config/s390/s390.md ("stack_restore_from_fpr"): New insn definition for loading the stack pointer from an FPR. Compared to the normal move insn this pattern includes a full memory barrier. gcc/testsuite/ChangeLog: 2016-04-20 Andreas Krebbel <[email protected]> Backport from mainline 2016-04-20 Andreas Krebbel <[email protected]> PR target/70674 * gcc.target/s390/pr70674.c: New test. Added: branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/pr70674.c Modified: branches/gcc-5-branch/gcc/ChangeLog branches/gcc-5-branch/gcc/config/s390/s390.c branches/gcc-5-branch/gcc/config/s390/s390.md branches/gcc-5-branch/gcc/testsuite/ChangeLog Reply at: https://bugs.launchpad.net/ubuntu/+source/gcc-5/+bug/1572613/comments/2 ------------------------------------------------------------------------ On 2016-04-20T07:13:55+00:00 Krebbel wrote: Author: krebbel Date: Wed Apr 20 07:13:23 2016 New Revision: 235234 URL: https://gcc.gnu.org/viewcvs?rev=235234&root=gcc&view=rev Log: PR70674: S/390: Add memory barrier to stack pointer restore from fpr. This patches fixes a problem with stack variable accesses being scheduled after the stack pointer restore instructions. In the testcase this happened with the stack variable 'a' accessed through the frame pointer. The existing stack_tie we have in the backend is basically useless when trying to block stack variable accesses from being scheduled across an insn. The alias set of stack variables and the frame alias set usually differ and hence aren't in conflict with each other. The solution appears to be a magic MEM term with a scratch register which is handled as a full memory barrier when analyzing scheduling dependencies. With the patch a (clobber (mem:BLK (scratch))) is being added to the restore instruction in order to prevent any memory operations to be scheduled across the insn. The patch does that only for the one case where the stack pointer is restored from an FPR. Theoretically this might happen also in the case where the stack pointer gets restored using a load multiple. However, triggering that problem with load-multiple appears to be much harder since the load-multiple will restore the frame pointer as well. So in order to see the problem a different call-clobbered register would need to be used as temporary stack pointer. Another case which needs to be handled some day is the stack pointer allocation part. It needs to be a memory barrier as well. Bootstrapped and regression tested with --with-arch z196 and z13 on s390 and s390x. -Andreas- gcc/ChangeLog: 2016-04-20 Andreas Krebbel <[email protected]> PR target/70674 * config/s390/s390.c (s390_restore_gprs_from_fprs): Pick the new stack_restore_from_fpr pattern when restoring r15. (s390_optimize_prologue): Strip away the memory barrier in the parallel when trying to get rid of restore insns. * config/s390/s390.md ("stack_restore_from_fpr"): New insn definition for loading the stack pointer from an FPR. Compared to the normal move insn this pattern includes a full memory barrier. gcc/testsuite/ChangeLog: 2016-04-20 Andreas Krebbel <[email protected]> PR target/70674 * gcc.target/s390/pr70674.c: New test. Added: trunk/gcc/testsuite/gcc.target/s390/pr70674.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/s390/s390.c trunk/gcc/config/s390/s390.md trunk/gcc/testsuite/ChangeLog Reply at: https://bugs.launchpad.net/ubuntu/+source/gcc-5/+bug/1572613/comments/3 ------------------------------------------------------------------------ On 2016-04-20T07:16:36+00:00 Krebbel wrote: Author: krebbel Date: Wed Apr 20 07:16:03 2016 New Revision: 235235 URL: https://gcc.gnu.org/viewcvs?rev=235235&root=gcc&view=rev Log: PR70674: S/390: Add memory barrier to stack pointer restore from fpr. This patches fixes a problem with stack variable accesses being scheduled after the stack pointer restore instructions. In the testcase this happened with the stack variable 'a' accessed through the frame pointer. The existing stack_tie we have in the backend is basically useless when trying to block stack variable accesses from being scheduled across an insn. The alias set of stack variables and the frame alias set usually differ and hence aren't in conflict with each other. The solution appears to be a magic MEM term with a scratch register which is handled as a full memory barrier when analyzing scheduling dependencies. With the patch a (clobber (mem:BLK (scratch))) is being added to the restore instruction in order to prevent any memory operations to be scheduled across the insn. The patch does that only for the one case where the stack pointer is restored from an FPR. Theoretically this might happen also in the case where the stack pointer gets restored using a load multiple. However, triggering that problem with load-multiple appears to be much harder since the load-multiple will restore the frame pointer as well. So in order to see the problem a different call-clobbered register would need to be used as temporary stack pointer. Another case which needs to be handled some day is the stack pointer allocation part. It needs to be a memory barrier as well. Bootstrapped and regression tested with --with-arch z196 and z13 on s390 and s390x. gcc/ChangeLog: 2016-04-20 Andreas Krebbel <[email protected]> Backport from mainline 2016-04-20 Andreas Krebbel <[email protected]> PR target/70674 * config/s390/s390.c (s390_restore_gprs_from_fprs): Pick the new stack_restore_from_fpr pattern when restoring r15. (s390_optimize_prologue): Strip away the memory barrier in the parallel when trying to get rid of restore insns. * config/s390/s390.md ("stack_restore_from_fpr"): New insn definition for loading the stack pointer from an FPR. Compared to the normal move insn this pattern includes a full memory barrier. gcc/testsuite/ChangeLog: 2016-04-20 Andreas Krebbel <[email protected]> Backport from mainline 2016-04-20 Andreas Krebbel <[email protected]> PR target/70674 * gcc.target/s390/pr70674.c: New test. Added: branches/gcc-6-branch/gcc/testsuite/gcc.target/s390/pr70674.c Modified: branches/gcc-6-branch/gcc/ChangeLog branches/gcc-6-branch/gcc/config/s390/s390.c branches/gcc-6-branch/gcc/config/s390/s390.md branches/gcc-6-branch/gcc/testsuite/ChangeLog Reply at: https://bugs.launchpad.net/ubuntu/+source/gcc-5/+bug/1572613/comments/4 ------------------------------------------------------------------------ On 2016-04-21T11:50:53+00:00 Krebbel wrote: Author: krebbel Date: Thu Apr 21 11:50:22 2016 New Revision: 235334 URL: https://gcc.gnu.org/viewcvs?rev=235334&root=gcc&view=rev Log: PR70674: S/390: Add memory barrier to stack pointer restore from fpr. This patches fixes a problem with stack variable accesses being scheduled after the stack pointer restore instructions. In the testcase this happened with the stack variable 'a' accessed through the frame pointer. The existing stack_tie we have in the backend is basically useless when trying to block stack variable accesses from being scheduled across an insn. The alias set of stack variables and the frame alias set usually differ and hence aren't in conflict with each other. The solution appears to be a magic MEM term with a scratch register which is handled as a full memory barrier when analyzing scheduling dependencies. With the patch a (clobber (mem:BLK (scratch))) is being added to the restore instruction in order to prevent any memory operations to be scheduled across the insn. The patch does that only for the one case where the stack pointer is restored from an FPR. Theoretically this might happen also in the case where the stack pointer gets restored using a load multiple. However, triggering that problem with load-multiple appears to be much harder since the load-multiple will restore the frame pointer as well. So in order to see the problem a different call-clobbered register would need to be used as temporary stack pointer. Another case which needs to be handled some day is the stack pointer allocation part. It needs to be a memory barrier as well. gcc/ChangeLog: 2016-04-21 Andreas Krebbel <[email protected]> Backport from mainline 2016-04-20 Andreas Krebbel <[email protected]> PR target/70674 * config/s390/s390.c (s390_restore_gprs_from_fprs): Pick the new stack_restore_from_fpr pattern when restoring r15. (s390_optimize_prologue): Strip away the memory barrier in the parallel when trying to get rid of restore insns. * config/s390/s390.md ("stack_restore_from_fpr"): New insn definition for loading the stack pointer from an FPR. Compared to the normal move insn this pattern includes a full memory barrier. gcc/testsuite/ChangeLog: 2016-04-21 Andreas Krebbel <[email protected]> Backport from mainline 2016-04-20 Andreas Krebbel <[email protected]> PR target/70674 * gcc.target/s390/pr70674.c: New test. Added: branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/pr70674.c Modified: branches/gcc-4_9-branch/gcc/ChangeLog branches/gcc-4_9-branch/gcc/config/s390/s390.c branches/gcc-4_9-branch/gcc/config/s390/s390.md branches/gcc-4_9-branch/gcc/testsuite/ChangeLog Reply at: https://bugs.launchpad.net/ubuntu/+source/gcc-5/+bug/1572613/comments/15 ------------------------------------------------------------------------ On 2016-04-21T11:51:57+00:00 Krebbel wrote: Fixed with the committed patch. Reply at: https://bugs.launchpad.net/ubuntu/+source/gcc-5/+bug/1572613/comments/16 ** Changed in: gcc Status: Unknown => Fix Released ** Changed in: gcc Importance: Unknown => Medium -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1572613 Title: GCC stack access scheduled after stack deallocation To manage notifications about this bug go to: https://bugs.launchpad.net/gcc/+bug/1572613/+subscriptions -- ubuntu-bugs mailing list [email protected] https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
