Bug 50906 - e500 exception unwinding under "-Os" causes SIGSEGV
Summary: e500 exception unwinding under "-Os" causes SIGSEGV
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.6.1
: P3 normal
Target Milestone: ---
Assignee: Alan Modra
URL: http://gcc.gnu.org/ml/gcc-patches/201...
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-28 20:29 UTC by Kyle Moffett
Modified: 2012-02-25 09:58 UTC (History)
1 user (show)

See Also:
Host: powerpc-linux-gnuspe
Target: powerpc-linux-gnuspe
Build: powerpc-linux-gnuspe
Known to work:
Known to fail: 4.6.1
Last reconfirmed: 2011-11-01 00:00:00


Attachments
Test case, part 1 (535 bytes, application/octet-stream)
2011-10-28 20:29 UTC, Kyle Moffett
Details
Test case, part 2 (471 bytes, application/octet-stream)
2011-10-28 20:29 UTC, Kyle Moffett
Details
Assembled unwindtestfunc.cc (with -O2) (9.64 KB, text/plain)
2011-10-28 20:31 UTC, Kyle Moffett
Details
Assembled unwindtestfunc.cc (with -Os) (8.62 KB, text/plain)
2011-10-28 20:32 UTC, Kyle Moffett
Details
Archive of RTL dumps of failing testcase built with "-Os" (173.68 KB, application/x-tgz)
2011-11-02 18:24 UTC, Kyle Moffett
Details
Proposed mainline fix (3.11 KB, patch)
2011-11-03 12:54 UTC, Alan Modra
Details | Diff
gcc-4.6 fix (2.20 KB, patch)
2011-11-03 12:55 UTC, Alan Modra
Details | Diff
mainline fix (3.12 KB, patch)
2011-11-04 10:00 UTC, Alan Modra
Details | Diff
gcc-4.6 fix (2.21 KB, patch)
2011-11-04 10:01 UTC, Alan Modra
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle Moffett 2011-10-28 20:29:18 UTC
Created attachment 25649 [details]
Test case, part 1

The libffi test-suite is failing on an e500v2 build of GCC 4.6.1 with a segmentation fault in libgcc (called from "throw()" in the "unwindtest.cc" file).

The target GCC configure options:
  --build/--host/--target=powerpc-linux-gnuspe
  --with-cpu=8548
  --enable-e500_double
  --with-long-double-128

I have extracted it to the attached minimal test-case, compiled as follows:

  $ g++ -Wall -Wextra -Werror -ggdb3 -Os -c unwindtest.cc -o unwindtest.o
  $ g++ -Wall -Wextra -Werror -ggdb3 -Os -c unwindtestfunc.cc -o unwindtestfunc.o
  $ g++ -ggdb3 -Os -o unwindtest unwindtest.o unwindtestfunc.o

The test fails if "unwindtestfunc.cc" is built with "-Os", but not if it is built with "-O2".  The compile options for unwindtest.cc have no effect on the success or failure of the test.

Cheers,
Kyle Moffett

--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
Comment 1 Kyle Moffett 2011-10-28 20:29:53 UTC
Created attachment 25650 [details]
Test case, part 2
Comment 2 Kyle Moffett 2011-10-28 20:31:54 UTC
Created attachment 25651 [details]
Assembled unwindtestfunc.cc (with -O2)
Comment 3 Kyle Moffett 2011-10-28 20:32:18 UTC
Created attachment 25652 [details]
Assembled unwindtestfunc.cc (with -Os)
Comment 4 Alan Modra 2011-11-01 23:44:04 UTC
_Z16closure_test_fn1P7ffi_cifPvPS1_S1_:
.LFB0:
	.file 1 "unwindtestfunc.cc"
	.loc 1 17 0
	.cfi_startproc
.LVL0:
	stwu 1,-128(1)
.LCFI0:
	.cfi_def_cfa_offset 128
	mflr 0
	mr 3,6
.LVL1:
	stw 0,132(1)
	addi 11,1,-16
.LCFI1:
	.cfi_def_cfa 11, 160

That last .cfi directive is wrong.  To match the instructions it should be
	.cfi_def_cfa 11, 144
Comment 5 Alan Modra 2011-11-02 00:05:42 UTC
	bl _save64gpr_24
	.loc 1 17 0
	mr 31,4
	.cfi_offset 31, -100
	.cfi_offset 1231, -104
	.cfi_offset 30, -108
	.cfi_offset 1230, -112
	.cfi_offset 29, -116
	.cfi_offset 1229, -120
	.cfi_offset 28, -124
	.cfi_offset 1228, -128
	.cfi_offset 27, -132
	.cfi_offset 1227, -136
	.cfi_offset 26, -140
	.cfi_offset 1226, -144
	.cfi_offset 25, -148
	.cfi_offset 1225, -152
	.cfi_offset 24, -156
	.cfi_offset 1224, -160

These are all wrong too, I think.

	.loc 1 26 0
	lwz 9,0(5)
	.loc 1 17 0
	mr 11,5

Yikes!!  Using r11 for something else, but no cfi directive to say the frame is no longer in r11.
Comment 6 Kyle Moffett 2011-11-02 18:24:02 UTC
Created attachment 25692 [details]
Archive of RTL dumps of failing testcase built with "-Os"

I spent some time looking through the RTL dump.  It looks like the stack references first show up in 194r.split2, but I couldn't understand enough of what I was looking at to match it up with the assembly output or figure out where the CFI information went wrong.

I've attached a tar.gz of the RTL dumps in case that helps isolate the issue.

Cheers,
Kyle Moffett
Comment 7 Alan Modra 2011-11-03 04:17:09 UTC
There are multiple problems in TARGET_SPE_ABI parts of rs6000_emit_prologue.  Hacking on it.
Comment 8 Alan Modra 2011-11-03 12:54:32 UTC
Created attachment 25702 [details]
Proposed mainline fix
Comment 9 Alan Modra 2011-11-03 12:55:29 UTC
Created attachment 25703 [details]
gcc-4.6 fix
Comment 10 Alan Modra 2011-11-03 12:59:10 UTC
Please test out these patches.  bootstrap and regression tests with -Os in BOOT_CFLAGS on spe would be ideal.  I'll be running a powerpc-linux regression test, but can't do that for spe.
Comment 11 Kyle Moffett 2011-11-03 16:48:43 UTC
Ok, I'm running a "bootstrap-lean" + "make check" by way of a full Debian GCC package build with this patch added.  The first build will just do C/C++/ObjC/ObjC++/Fortran; if that works out OK I will also do a Java build in a little while.

I applied a 1-line patch to make the package build set "BOOT_CFLAGS := -g -O2" as you requested, so hopefully any fallout will be obvious during the GCC build itself.

There are 2 other patches I have applied on top of the Debian GCC 4.6.2-3 package to fix the SPE build:
  * http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=647288
  * http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=647324

The libffi one applies on top of these patches already in the Debian package:
  * http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=644338

Based on the speed of this board and the fact that I am using it for other testing as well, I expect to have the test results in roughly 6-8 hours.

Cheers,
Kyle Moffett

--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
Comment 12 Alan Modra 2011-11-04 10:00:43 UTC
Created attachment 25711 [details]
mainline fix

Revised patch.  I'd forgotten that you can't compare hard reg rtx's, ever since

2002-06-16  Jeff Law <law@redhat.com>

	* emit-rtl.c (gen_rtx_REG): Temporarily turn off automatic
	sharing of hard registers.

Some "temporarily" that turned out to be. :)
Comment 13 Alan Modra 2011-11-04 10:01:34 UTC
Created attachment 25712 [details]
gcc-4.6 fix
Comment 14 Kyle Moffett 2011-11-04 21:20:18 UTC
I'm now rebuilding the Debian GCC 4.6.2-4 (which now includes the other 2 patches I mentioned previously) with the updated 4.6 patch, I'll let you know what the results are (including testsuite deltas between unpatched-O2, patched-O2, and patched-Os).

I probably won't have any results for you before Monday.

Cheers,
Kyle Moffett

--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
Comment 15 Kyle Moffett 2011-11-08 19:12:04 UTC
Sorry for the delay.  I've suddenly started experiencing odd kernel panics on my test boards in the middle of the test-suite runs and I'm in the process of tracking them down.  Based on where the kernel is crashing I don't think it's caused by the GCC changes, just bad luck that I didn't notice this bug before.

From the test suites that have completed so far, I'm not seeing any new FAILs relative to the old GCC, but I have to wait for them all to finish in order to be able to diff properly.

I'll let you know when I have something to report.

Cheers,
Kyle Moffett
Comment 16 David Kühling 2011-11-11 09:51:05 UTC
@Kyle: wrt random kernel panics, I had them too on my P2020RDB and attributed them to RAM errors.  Before they happen, the kernel already complains about corrupt data in the logs and GCC randomly crashes when running a sufficiently large parallel build.  After power-cycling the GCC crashes usually vanish, so maybe there's a small chance that DDR RAM is slightly miscalibrated by u-boot?  Reducing the DDR clock might help.
Comment 17 Kyle Moffett 2011-11-22 18:50:20 UTC
Ok, a new kernel based on 3.2-rc1 resolved my crashing issues entirely.  I wasn't too worried about my DDR clocks since I have ECC memory and EDAC never reported any errors.

Using the gcc-4.6 fix on top of 4.6.2, I get the following diffs in the testsuite summary between 4.6.2-unpatched and 4.6.2-patched.  I'm in the process of running a second build with BOOT_CFLAGS="-Os", but I'll be out of the office for Thanksgiving until next Monday and probably won't be able to check on it during that time.

These appear to be EH bugs fixed by your changes:
-FAIL: g++.dg/torture/stackalign/eh-vararg-1.C -Os execution test
-FAIL: g++.dg/torture/stackalign/eh-vararg-2.C -Os execution test
-FAIL: g++.dg/torture/stackalign/eh-vararg-1.C -Os execution test
-FAIL: g++.dg/torture/stackalign/eh-vararg-2.C -Os execution test

These are tests that shouldn't be run on e500/SPE as they build with "-mcpu=power5".  These tests fail with SIGILL while executing an "lfd" opcode; I'm not sure why they passed before:
+FAIL: gcc.target/powerpc/ppc-fma-5.c execution test
+FAIL: gfortran.dg/pr47614.f  -O0 execution test
+FAIL: gfortran.dg/pr47614.f  -O1 execution test
+FAIL: gfortran.dg/pr47614.f  -O2 execution test
+FAIL: gfortran.dg/pr47614.f  -O3 -fomit-frame-pointer execution test
+FAIL: gfortran.dg/pr47614.f  -O3 -fomit-frame-pointer -funroll-loops
+FAIL: gfortran.dg/pr47614.f  -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions execution test
+FAIL: gfortran.dg/pr47614.f  -O3 -g execution test
+FAIL: gfortran.dg/pr47614.f  -Os execution test

There's no other delta in the testsuite summary, so I feel pretty confident that there were no regressions introduced by this patch for e500 at least.

Thanks again for your help!

Cheers,
Kyle Moffett
Comment 18 Kyle Moffett 2011-11-28 19:30:07 UTC
I am happy to report that your updated 4.6.2 patch seems to be 100% functional on e500/SPE.

I get identical "test-summary" reports for patched-4.6.2 with and without BOOT_CFLAGS="-g -Os", apart from the different build directory paths (EG: /srv/build/gcc-4.6-normal versus /srv/build/gcc-4.6-Os), 

As before, the following testsuite failures are fixed relative to unpatched 4.6.2:
-FAIL: g++.dg/torture/stackalign/eh-vararg-1.C -Os execution test
-FAIL: g++.dg/torture/stackalign/eh-vararg-2.C -Os execution test
-FAIL: g++.dg/torture/stackalign/eh-vararg-1.C -Os execution test
-FAIL: g++.dg/torture/stackalign/eh-vararg-2.C -Os execution test

And again, as before, the following 64-bit-only tests are erroneously run on 32-bit e500 CPUs, although they were inexplicably successful during a previous build.
+FAIL: gcc.target/powerpc/ppc-fma-5.c execution test
+FAIL: gfortran.dg/pr47614.f  -O0 execution test
+FAIL: gfortran.dg/pr47614.f  -O1 execution test
+FAIL: gfortran.dg/pr47614.f  -O2 execution test
+FAIL: gfortran.dg/pr47614.f  -O3 -fomit-frame-pointer execution test
+FAIL: gfortran.dg/pr47614.f  -O3 -fomit-frame-pointer -funroll-loops
+FAIL: gfortran.dg/pr47614.f  -O3 -fomit-frame-pointer -funroll-all-loops
-finline-functions execution test
+FAIL: gfortran.dg/pr47614.f  -O3 -g execution test
+FAIL: gfortran.dg/pr47614.f  -Os execution test

Thanks again for your help!

Cheers,
Kyle Moffett
Comment 19 Alan Modra 2011-12-06 03:41:49 UTC
Author: amodra
Date: Tue Dec  6 03:41:44 2011
New Revision: 182039

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182039
Log:
	PR target/50906
	* config/rs6000/rs6000.c (rs6000_emit_prologue <TARGET_SPE_ABI>):
	Do not mark r11 setup as frame-related.  Pass correct offset to
	rs6000_emit_savres_rtx.  Correct out-of-line rs6000_frame_related
	arguments.  Correct sp_offset.  Remove "offset" fudge from
	in-line rs6000_frame_related call.  Rename misleading variable.
	Fix comments and whitespace.  Tidy some expressions.
	(rs6000_emit_epilogue <TARGET_SPE_ABI>): Always set frame_reg_rtx
	to r11 in out-of-line case.  Correct sp_offset.  Pass correct
	offset to rs6000_emit_savres_rtx.  Rename misleading variable.
	Fix comments and whitespace.  Tidy some expressions.
	(rs6000_emit_epilogue <non-TARGET_SPE_ABI>): Add sp_offset
	adjustment when !saving_GPRs_inline.  Correct register mode
	used in address calcs.
	(rs6000_emit_epilogue <non-TARGET_SPE_ABI>): Similarly when
	!restoring_GPRs_inline.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.c
Comment 20 Alan Modra 2011-12-06 03:47:40 UTC
Author: amodra
Date: Tue Dec  6 03:47:37 2011
New Revision: 182040

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182040
Log:
	PR target/50906
	* config/rs6000/rs6000.c (rs6000_emit_prologue <TARGET_SPE_ABI>):
	Do not mark r11 setup as frame-related.  Pass correct offset to
	rs6000_emit_savres_rtx.  Correct out-of-line rs6000_frame_related
	arguments.  Correct sp_offset.  Remove "offset" fudge from
	in-line rs6000_frame_related call.  Rename misleading variable.
	Fix comments and whitespace.  Tidy some expressions.
	(rs6000_emit_epilogue <TARGET_SPE_ABI>): Always set frame_reg_rtx
	to r11 in out-of-line case.  Correct sp_offset.  Pass correct
	offset to rs6000_emit_savres_rtx.  Rename misleading variable.
	Fix comments and whitespace.  Tidy some expressions.
	(rs6000_emit_epilogue <non-TARGET_SPE_ABI>): Add sp_offset
	adjustment when !saving_GPRs_inline.  Correct register mode
	used in address calcs.
	(rs6000_emit_epilogue <non-TARGET_SPE_ABI>): Similarly when
	!restoring_GPRs_inline.


Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/config/rs6000/rs6000.c
Comment 21 Alan Modra 2011-12-06 03:48:32 UTC
patch applied 4.7 and 4.6