Bug 65735 - [5 Regression] ICE (in duplicate_thread_path, at tree-ssa-threadupdate.c)
Summary: [5 Regression] ICE (in duplicate_thread_path, at tree-ssa-threadupdate.c)
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 5.0
: P1 normal
Target Milestone: 5.0
Assignee: Jakub Jelinek
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2015-04-10 21:37 UTC by Matthias Klose
Modified: 2015-10-28 09:01 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.9.2
Known to fail: 5.0
Last reconfirmed: 2015-04-10 00:00:00


Attachments
preprocessed source (80.13 KB, application/x-xz)
2015-04-10 21:37 UTC, Matthias Klose
Details
gcc5-pr65735.patch (1023 bytes, patch)
2015-04-11 14:22 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Klose 2015-04-10 21:37:22 UTC
Created attachment 35290 [details]
preprocessed source

seen with r221867 on arm-linux-gnueabihf

$ g++ -c -g -O2 menu.ii 
menu.ii: In function 'void fn2()':
menu.ii:2:6: internal compiler error: in duplicate_thread_path, at tree-ssa-threadupdate.c:2445
 void fn2() {
      ^
0x7c2d8f duplicate_thread_path
        ../../src/gcc/tree-ssa-threadupdate.c:2444
0x7c2d8f thread_through_all_blocks(bool)
        ../../src/gcc/tree-ssa-threadupdate.c:2594
0x74020d execute
        ../../src/gcc/tree-ssa-dom.c:938
Please submit a full bug report,
with preprocessed source if appropriate.

$ cat menu.ii
int fn1();
void fn2() {
  int a, b, c;
  while (!a) {
    c = fn1();
    if (c == 7)
      c = b;
    switch (c) {
    case 1:
      a = b++;
      if (b)
        b = 1;
    }
  }
}

$ g++ -v
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/usr/lib/gcc/arm-linux-gnueabihf/5/lto-wrapper
Target: arm-linux-gnueabihf
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 5-20150404-0ubuntu11' --with-bugurl=file:///usr/share/doc/gcc-5/README.Bugs --enable-languages=c,ada,c++,java,go,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-5 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=c++98 --enable-gnu-unique-object --disable-libitm --disable-libquadmath --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-5-armhf/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-5-armhf --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-5-armhf --with-arch-directory=arm --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --enable-multilib --disable-sjlj-exceptions --with-arch=armv7-a --with-fpu=vfpv3-d16 --with-float=hard --with-mode=thumb --disable-werror --enable-multilib --enable-checking=yes --build=arm-linux-gnueabihf --host=arm-linux-gnueabihf --target=arm-linux-gnueabihf
Thread model: posix
gcc version 5.0.0 20150404 (experimental) [trunk revision 221867] (Ubuntu 5-20150404-0ubuntu11)
Comment 1 Matthias Klose 2015-04-10 23:22:06 UTC
fails on x86_64-linux-gnu too.
Comment 2 Jakub Jelinek 2015-04-10 23:27:01 UTC
Started with r221675.
Comment 3 Jakub Jelinek 2015-04-11 08:59:22 UTC
Slightly cleaned up testcase (so it doesn't use ununinitialized vars).  -O2 is enough.
The ICE is on:
2443		  /* Make sure the successor is the next node in the path.  */
2444		  gcc_assert (i + 1 == n_region
2445			      || region_copy[i + 1] == single_succ_edge (bb)->dest);

i is 0, n_region is 5.

int foo (void);

void
bar (int a, int b, int c)
{
  while (!a)
    {
      c = foo ();
      if (c == 7)
	c = b;
      switch (c)
	{
	case 1:
	  a = b++;
	  if (b)
	    b = 1;
	}
    }
}
Comment 4 Jakub Jelinek 2015-04-11 10:07:30 UTC
(In reply to Jakub Jelinek from comment #3)
> Slightly cleaned up testcase (so it doesn't use ununinitialized vars).  -O2
> is enough.
> The ICE is on:
> 2443		  /* Make sure the successor is the next node in the path.  */
> 2444		  gcc_assert (i + 1 == n_region
> 2445			      || region_copy[i + 1] == single_succ_edge (bb)->dest);
> 
> i is 0, n_region is 5.
> 
> int foo (void);
> 
> void
> bar (int a, int b, int c)
> {
>   while (!a)
>     {
>       c = foo ();
>       if (c == 7)
> 	c = b;
>       switch (c)
> 	{
> 	case 1:
> 	  a = b++;
> 	  if (b)
> 	    b = 1;
> 	}
>     }
> }

Don't know anything about the FSM code, but in the second
thread_through_all_blocks
the first path looks like:
(gdb) p *paths.m_vec->m_vecdata[0].m_vec->m_vecdata[0]
$127 = {e = <edge 0x7ffff18669a0 (9 -> 4)>, type = EDGE_FSM_THREAD}
(gdb) p *paths.m_vec->m_vecdata[0].m_vec->m_vecdata[1]
$128 = {e = <edge 0x7ffff1866850 (4 -> 5)>, type = EDGE_FSM_THREAD}
(gdb) p *paths.m_vec->m_vecdata[0].m_vec->m_vecdata[2]
$129 = {e = <edge 0x7ffff1866498 (5 -> 12)>, type = EDGE_FSM_THREAD}
(gdb) p *paths.m_vec->m_vecdata[0].m_vec->m_vecdata[3]
$130 = {e = <edge 0x7ffff18665b0 (12 -> 14)>, type = EDGE_FSM_THREAD}
(gdb) p *paths.m_vec->m_vecdata[0].m_vec->m_vecdata[4]
$131 = {e = <edge 0x7ffff18663f0 (14 -> 5)>, type = EDGE_FSM_THREAD}
(gdb) p *paths.m_vec->m_vecdata[0].m_vec->m_vecdata[5]
$132 = {e = <edge 0x7ffff1866498 (5 -> 12)>, type = EDGE_NO_COPY_SRC_BLOCK}
Having the same block (bb5) twice in the path sounds really suspicious to me.

Guess a quick hack could be to check for bbs appearing more than once in
valid_jump_thread_path.  Dunno if bb->flags & BB_VISITED could be used for that.
Comment 5 Jakub Jelinek 2015-04-11 10:16:55 UTC
It shows up already when registering the path:
  Registering FSM jump thread: (9, 4) incoming edge;  (4, 5)  (5, 12)  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12) 
  Registering FSM jump thread: (9, 12) incoming edge;  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12) 
  Registering FSM jump thread: (9, 4) incoming edge;  (4, 5)  (5, 12)  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12) 
  Registering FSM jump thread: (9, 12) incoming edge;  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12) 
  Registering FSM jump thread: (9, 4) incoming edge;  (4, 5)  (5, 12)  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12) 
  Registering FSM jump thread: (9, 12) incoming edge;  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12) 
  Registering FSM jump thread: (9, 4) incoming edge;  (4, 5)  (5, 12)  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12) 
  Registering FSM jump thread: (9, 12) incoming edge;  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12) 
  Registering FSM jump thread: (9, 4) incoming edge;  (4, 5)  (5, 12)  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12) 
  Registering FSM jump thread: (9, 12) incoming edge;  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12) 
  Registering FSM jump thread: (9, 4) incoming edge;  (4, 5)  (5, 12)  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12) 
  Registering FSM jump thread: (9, 12) incoming edge;  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12) 
  Registering FSM jump thread: (9, 4) incoming edge;  (4, 5)  (5, 12)  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12) 
  Registering FSM jump thread: (9, 12) incoming edge;  (12, 14)  (14, 5)  (5, 12) nocopy; (5, 12)
Comment 6 Jakub Jelinek 2015-04-11 14:22:10 UTC
Created attachment 35300 [details]
gcc5-pr65735.patch

Untested fix.

It seems there is a guard against recursion, but it only checks that we don't recurse through the same PHI more than once.  But e.g. on the bb5 in the testcase there are two different PHI nodes, and we walk one of the PHI nodes first and then walk the other PHI node the second time, which is when we add the same basic block to the path again.  At least short term, I don't think the paths with loops in them are really supportable.
Tested so far just on the new testcase and on tree-ssa.exp=*dom*, will do full bootstrap/regtest momentarily.
Comment 7 Jakub Jelinek 2015-04-11 17:33:25 UTC
Author: jakub
Date: Sat Apr 11 17:32:54 2015
New Revision: 222011

URL: https://gcc.gnu.org/viewcvs?rev=222011&root=gcc&view=rev
Log:
	PR tree-optimization/65735
	* tree-ssa-threadedge.c (fsm_find_control_statement_thread_paths):
	Remove visited_phis argument, add visited_bbs, avoid recursing into the
	same bb rather than just into the same phi node.
	(thread_through_normal_block): Adjust caller.

	* gcc.c-torture/compile/pr65735.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/compile/pr65735.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-threadedge.c
Comment 8 Jakub Jelinek 2015-04-12 19:12:53 UTC
Fixed.
Comment 9 Yvan Roux 2015-10-28 09:01:19 UTC
Author: yroux
Date: Wed Oct 28 09:00:47 2015
New Revision: 229478

URL: https://gcc.gnu.org/viewcvs?rev=229478&root=gcc&view=rev
Log:
gcc/
2015-10-28 Yvan Roux  <yvan.roux@linaro.org>
	   Sebastian Pop  <s.pop@samsung.com>

	Backport from trunk r221007, r221675, r222011.
	2015-04-11  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/65735
	* tree-ssa-threadedge.c (fsm_find_control_statement_thread_paths):
	Remove visited_phis argument, add visited_bbs, avoid recursing into the
	same bb rather than just into the same phi node.
	(thread_through_normal_block): Adjust caller.

	2015-03-25  Sebastian Pop  <s.pop@samsung.com>

	PR tree-optimization/65177
	* tree-ssa-threadupdate.c (verify_seme): Renamed verify_jump_thread.
	(bb_in_bbs): New.
	(duplicate_seme_region): Renamed duplicate_thread_path.  Redirect all
	edges not adjacent on the path to the original code.

	2015-02-26  Sebastian Pop  <s.pop@samsung.com>

	PR tree-optimization/65048
	* tree-ssa-threadupdate.c (valid_jump_thread_path): New.
	(thread_through_all_blocks): Call valid_jump_thread_path.
	Remove invalid FSM jump-thread paths.

gcc/testsuite/
2015-10-28 Yvan Roux  <yvan.roux@linaro.org>
	   Sebastian Pop  <s.pop@samsung.com>

	Backport from trunk r221007, r221675, r222011.
	2015-04-11  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/65735
	* gcc.c-torture/compile/pr65735.c: New test.

	2015-03-25  Sebastian Pop  <s.pop@samsung.com>

	PR tree-optimization/65177
	* gcc.dg/tree-ssa/ssa-dom-thread-10.c: New.

	2015-02-26  Sebastian Pop  <s.pop@samsung.com>

	PR tree-optimization/65048
	* gcc.dg/tree-ssa/ssa-dom-thread-9.c: New.


Added:
    branches/linaro/gcc-4_9-branch/gcc/testsuite/gcc.c-torture/compile/pr65735.c
    branches/linaro/gcc-4_9-branch/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-10.c
    branches/linaro/gcc-4_9-branch/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-9.c
Modified:
    branches/linaro/gcc-4_9-branch/gcc/ChangeLog.linaro
    branches/linaro/gcc-4_9-branch/gcc/testsuite/ChangeLog.linaro
    branches/linaro/gcc-4_9-branch/gcc/tree-ssa-threadedge.c
    branches/linaro/gcc-4_9-branch/gcc/tree-ssa-threadupdate.c