Bug 65802 - [6 Regression] ICE in redirect_eh_edge_1, at tree-eh.c:2335
Summary: [6 Regression] ICE in redirect_eh_edge_1, at tree-eh.c:2335
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 6.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-18 18:28 UTC by Dmitry G. Dyachenko
Modified: 2015-04-24 14:19 UTC (History)
2 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry G. Dyachenko 2015-04-18 18:28:24 UTC
r222031 PASS
r222198 FAIL

$ cat x.ii
typedef int tf();
struct S {
  tf m_fn1;
} a;

void fn1()
{
  try {
    __builtin_va_list c;
    {
      int *d = __builtin_va_arg(c, int *);
      int **e = &d;
      __asm__("" : "=d"(e));
      a.m_fn1();
    }
    a.m_fn1();
  }

  catch (...) {
  }
}

$ g++ -fpreprocessed -Wall -c x.ii
x.ii: In function 'void fn1()':
x.ii:6:6: internal compiler error: in redirect_eh_edge_1, at tree-eh.c:2335
 void fn1()
      ^
0xe7050b redirect_eh_edge_1
	/home/dimhen/src/gcc_current/gcc/tree-eh.c:2335
0xe70c4c cleanup_empty_eh_merge_phis
	/home/dimhen/src/gcc_current/gcc/tree-eh.c:4259
0xe714e9 cleanup_empty_eh
	/home/dimhen/src/gcc_current/gcc/tree-eh.c:4508
0xe714e9 cleanup_all_empty_eh
	/home/dimhen/src/gcc_current/gcc/tree-eh.c:4551
0xe714e9 execute_cleanup_eh_1
	/home/dimhen/src/gcc_current/gcc/tree-eh.c:4581
0xe714e9 execute
	/home/dimhen/src/gcc_current/gcc/tree-eh.c:4639
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.

$ g++ -v
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/usr/local/gcc_current/libexec/gcc/x86_64-unknown-linux-gnu/6.0.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: /home/dimhen/src/gcc_current/configure --prefix=/usr/local/gcc_current --enable-static --enable-checking=yes,df,fold,rtl --enable-languages=c,c++,lto --enable-plugin --disable-libstdcxx-dual-abi --disable-multilib
Thread model: posix
gcc version 6.0.0 20150417 (experimental) [trunk revision 222198] (GCC)
Comment 1 Dmitry G. Dyachenko 2015-04-18 19:04:48 UTC
r222168 PASS
r222174 FAIL
Comment 2 Tom de Vries 2015-04-19 08:59:13 UTC
Confirmed, I see the failure as well.
Comment 3 Tom de Vries 2015-04-19 17:42:08 UTC
This patch allows the example to pass:
...
diff --git a/gcc/passes.def b/gcc/passes.def
index ffa63b5..041197c 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -344,7 +344,6 @@ along with GCC; see the file COPYING3.  If not see
       NEXT_PASS (pass_tm_edges);
   POP_INSERT_PASSES ()
   NEXT_PASS (pass_vtable_verify);
-  NEXT_PASS (pass_lower_vaarg);
   NEXT_PASS (pass_lower_vector);
   NEXT_PASS (pass_lower_complex_O0);
   NEXT_PASS (pass_asan_O0);
@@ -352,6 +351,7 @@ along with GCC; see the file COPYING3.  If not see
   NEXT_PASS (pass_sanopt);
   NEXT_PASS (pass_cleanup_eh);
   NEXT_PASS (pass_lower_resx);
+  NEXT_PASS (pass_lower_vaarg);
   NEXT_PASS (pass_nrv);
   NEXT_PASS (pass_cleanup_cfg_post_optimizing);
   NEXT_PASS (pass_warn_function_noreturn);
...

I have no knowledge of the exception handling implementation, so:
1. a proper root cause analysis would take me some time.
2. I have no idea whether the patch is actually correct.  I'll try a bootstrap
   and reg-test though, that'll gives us at least more information.
Comment 4 Tom de Vries 2015-04-19 21:29:55 UTC
The assert triggering is:
...
(gdb) 
#5  0x00000000011325fe in redirect_eh_edge_1 (edge_in=0x7ffff64af4d0, new_bb=0x7ffff64ae4e0, change_region=true) at src/gcc/tree-eh.c:2335
2335	  gcc_assert (lookup_stmt_eh_lp (throw_stmt) == old_lp_nr);
...

Indeed the comparison fails:
...
(gdb) p old_lp_nr
$1 = 1
(gdb) p lookup_stmt_eh_lp (throw_stmt)
$2 = 0
...

And lookup_stmt_eh_lp (throw_stmt) is 0, because throw_stmt is NULL, which presumably is not intended to be NULL here:
...
(gdb) p throw_stmt
$3 = (gimple) 0x0
...

throw_stmt is initialized here:
...
2334	  throw_stmt = last_stmt (edge_in->src);
...

In fact edge_in->src is an empty bb:
...
(gdb) call debug_bb (edge_in->src)
;; basic block 15, loop depth 0, count 0, freq 0, maybe hot
;;  prev block 14, next block 3, flags: (NEW)
;;  pred:       14 (FALLTHRU)
;;  succ:       7 (EH)
;;              3 (FALLTHRU)
...

Before pass_lower_vaarg, we have this definition in bb2 definining for _6:
...
;;   basic block 2, loop depth 0, count 0, freq 0, maybe hot
;;    prev block 0, next block 3, flags: (NEW, REACHABLE)
;;    pred:       ENTRY (FALLTHRU)
  [LP 1] # .MEM_5 = VDEF <.MEM_4(D)>
  # USE = anything
  # CLB = anything
  _6 = VA_ARG (&cD.2333, 0B);
;;    succ:       7 (EH)
;;                3 (FALLTHRU)
...

After pass_lower_vaarg, it's spread over several basic blocks:
...
;;   basic block 2, loop depth 0, count 0, freq 0, maybe hot
;;    prev block 0, next block 11, flags: (NEW, REACHABLE)
;;    pred:       ENTRY (FALLTHRU)
;;    succ:       11 [100.0%]  (FALLTHRU)

;;   basic block 11, loop depth 0, count 0, freq 0, maybe hot
;;    prev block 2, next block 12, flags: (NEW)
;;    pred:       2 [100.0%]  (FALLTHRU)
  # VUSE <.MEM_4(D)>
  _22 = cD.2333.gp_offsetD.5;
  if (_22 >= 48)
    goto <bb 13> (<L6>);
  else
    goto <bb 12> (<L5>);
;;    succ:       13 (TRUE_VALUE)
;;                12 (FALSE_VALUE)

;;   basic block 12, loop depth 0, count 0, freq 0, maybe hot
;;    prev block 11, next block 13, flags: (NEW)
;;    pred:       11 (FALSE_VALUE)
<L5>:
  # VUSE <.MEM_4(D)>
  _23 = cD.2333.reg_save_areaD.8;
  # VUSE <.MEM_4(D)>
  _24 = cD.2333.gp_offsetD.5;
  _25 = (sizetype) _24;
  addr.1_26 = _23 + _25;
  # VUSE <.MEM_4(D)>
  _27 = cD.2333.gp_offsetD.5;
  _28 = _27 + 8;
  # .MEM_29 = VDEF <.MEM_4(D)>
  cD.2333.gp_offsetD.5 = _28;
  goto <bb 14> (<L7>);
;;    succ:       14 (FALLTHRU)

;;   basic block 13, loop depth 0, count 0, freq 0, maybe hot
;;    prev block 12, next block 14, flags: (NEW)
;;    pred:       11 (TRUE_VALUE)
<L6>:
  # VUSE <.MEM_4(D)>
  _30 = cD.2333.overflow_arg_areaD.7;
  addr.1_31 = _30;
  _32 = _30 + 8;
  # .MEM_33 = VDEF <.MEM_4(D)>
  cD.2333.overflow_arg_areaD.7 = _32;
;;    succ:       14 (FALLTHRU)

;;   basic block 14, loop depth 0, count 0, freq 0, maybe hot
;;    prev block 13, next block 15, flags: (NEW)
;;    pred:       12 (FALLTHRU)
;;                13 (FALLTHRU)
  # .MEM_20 = PHI <.MEM_29(12), .MEM_33(13)>
  # addr.1_21 = PHI <addr.1_26(12), addr.1_31(13)>
<L7>:
  # VUSE <.MEM_20>
  _6 = MEM[(intD.9 * * {ref-all})addr.1_21];
;;    succ:       15 (FALLTHRU)

;;   basic block 15, loop depth 0, count 0, freq 0, maybe hot
;;    prev block 14, next block 3, flags: (NEW)
;;    pred:       14 (FALLTHRU)
;;    succ:       7 (EH)
;;                3 (FALLTHRU)
...

Before the expansion, the ifn_va_arg is the statement that could throw:
...
(gdb) call debug_bb_n (2)
;; basic block 2, loop depth 0, count 0, freq 0, maybe hot
;;  prev block 0, next block 3, flags: (NEW, REACHABLE)
;;  pred:       ENTRY (FALLTHRU)
[LP 1] # .MEM_5 = VDEF <.MEM_4(D)>
# USE = anything 
# CLB = anything 
_6 = VA_ARG (&cD.2333, 0B);
;;  succ:       7 (EH)
;;              3 (FALLTHRU)

$1 = (basic_block_def *) 0x7ffff64ae270
(gdb) call stmt_could_throw_p ( last_stmt ($1 ) )
$3 = true
...

But the last statement before bb15 cannot throw:
...
(gdb) call debug_bb_n (14)
;; basic block 14, loop depth 0, count 0, freq 0, maybe hot
;;  prev block 13, next block 15, flags: (NEW)
;;  pred:       12 (FALLTHRU)
;;              13 (FALLTHRU)
# .MEM_20 = PHI <.MEM_29(12), .MEM_33(13)>
# addr.1_21 = PHI <addr.1_26(12), addr.1_31(13)>
<L7>:
# VUSE <.MEM_20>
_6 = MEM[(intD.9 * * {ref-all})addr.1_21];
;;  succ:       15 (FALLTHRU)

$4 = (basic_block_def *) 0x7ffff64aea90
(gdb) call stmt_could_throw_p ( last_stmt ($4 ) )
$5 = false
...

So it does not seem to be just a question of removing the last empty bb.
Comment 5 Tom de Vries 2015-04-19 21:37:45 UTC
Before the patch series, at 011.cfg we had this representation:
...
;;   basic block 2, loop depth 0, count 0, freq 0, maybe hot
;;    prev block 0, next block 3, flags: (NEW, REACHABLE)
;;    pred:       ENTRY (FALLTHRU)
  D.2342 = cD.2333.gp_offsetD.5;
  if (D.2342 >= 48)
    goto <bb 4>;
  else
    goto <bb 3>;
;;    succ:       4 (TRUE_VALUE)
;;                3 (FALSE_VALUE)

;;   basic block 3, loop depth 0, count 0, freq 0, maybe hot
;;    prev block 2, next block 4, flags: (NEW, REACHABLE)
;;    pred:       2 (FALSE_VALUE)
  D.2344 = cD.2333.reg_save_areaD.8;
  D.2345 = cD.2333.gp_offsetD.5;
  D.2346 = (sizetype) D.2345;
  addr.0D.2339 = D.2344 + D.2346;
  D.2347 = cD.2333.gp_offsetD.5;
  D.2348 = D.2347 + 8;
  cD.2333.gp_offsetD.5 = D.2348;
  goto <bb 5>;
;;    succ:       5 (FALLTHRU)

;;   basic block 4, loop depth 0, count 0, freq 0, maybe hot
;;    prev block 3, next block 5, flags: (NEW, REACHABLE)
;;    pred:       2 (TRUE_VALUE)
  D.2349 = cD.2333.overflow_arg_areaD.7;
  addr.0D.2339 = D.2349;
  D.2350 = D.2349 + 8;
  cD.2333.overflow_arg_areaD.7 = D.2350;
;;    succ:       5 (FALLTHRU)

;;   basic block 5, loop depth 0, count 0, freq 0, maybe hot
;;    prev block 4, next block 6, flags: (NEW, REACHABLE)
;;    pred:       3 (FALLTHRU)
;;                4 (FALLTHRU)
  d.1D.2351 = MEM[(intD.9 * * {ref-all})addr.0D.2339];
  dD.2334 = d.1D.2351;
  eD.2335 = &dD.2334;
  __asm__("" : "=d" eD.2335);
  [LP 1] # USE = anything 
  # CLB = anything 
  m_fn1D.2327 (&aD.2330);
;;    succ:       9 (EH)
;;                6 (FALLTHRU)
...

There's no exception connected to the whole va_arg sequence.

So the question is: should ifn_va_arg have ECF_NOTHROW?
Comment 6 Tom de Vries 2015-04-20 07:23:20 UTC
(In reply to vries from comment #5)
> So the question is: should ifn_va_arg have ECF_NOTHROW?

Adding ECF_NOTHROW to ifn_va_arg also fixes the ICE.

And at gimplify_va_arg_expr, the VA_ARG_EXPR tree is non-throwing:
...
(gdb) call debug_generic_expr (*expr_p )
VA_ARG_EXPR <c>
(gdb) call debug_tree (*expr_p )
 <va_arg_expr 0x7ffff661e9a0
    type <pointer_type 0x7ffff64c97e0
        type <integer_type 0x7ffff64a7690 int public SI
            size <integer_cst 0x7ffff64c50a8 constant 32>
            unit size <integer_cst 0x7ffff64c50c0 constant 4>
            align 32 symtab 0 alias set -1 canonical type 0x7ffff64a7690 precision 32 min <integer_cst 0x7ffff64c5060 -2147483648> max <integer_cst 0x7ffff64c5078 2147483647>
            pointer_to_this <pointer_type 0x7ffff64c97e0>>
        sizes-gimplified unsigned DI
        size <integer_cst 0x7ffff64a3e58 constant 64>
        unit size <integer_cst 0x7ffff64a3e70 constant 8>
        align 64 symtab 0 alias set -1 canonical type 0x7ffff64c97e0
        pointer_to_this <pointer_type 0x7ffff6622540>>
    side-effects
    arg 0 <var_decl 0x7ffff64b0c60 c
        type <array_type 0x7ffff64ccbd0 __builtin_va_list type <record_type 0x7ffff64cc738 __va_list_tag>
            sizes-gimplified BLK
            size <integer_cst 0x7ffff64c5318 constant 192>
            unit size <integer_cst 0x7ffff64c52e8 constant 24>
            align 64 symtab 0 alias set -1 canonical type 0x7ffff64cc888 domain <integer_type 0x7ffff64cc7e0>>
        used tree_1 decl_5 BLK file test.cc line 13 col 25 size <integer_cst 0x7ffff64c5318 192> unit size <integer_cst 0x7ffff64c52e8 24>
        align 64 context <function_decl 0x7ffff66240d8 fn1>>
    test.cc:15:32>
(gdb) call tree_could_throw_p (*expr_p )
$1 = false
...

I'll bootstrap and reg-test this fix.
Comment 7 Richard Biener 2015-04-20 08:24:08 UTC
(In reply to vries from comment #6)
> (In reply to vries from comment #5)
> > So the question is: should ifn_va_arg have ECF_NOTHROW?
> 
> Adding ECF_NOTHROW to ifn_va_arg also fixes the ICE.
> 
> And at gimplify_va_arg_expr, the VA_ARG_EXPR tree is non-throwing:

Is that always the case (try -fnon-call-exceptions)?  Then doing that looks obvious.

Richard.

> (gdb) call debug_generic_expr (*expr_p )
> VA_ARG_EXPR <c>
> (gdb) call debug_tree (*expr_p )
>  <va_arg_expr 0x7ffff661e9a0
>     type <pointer_type 0x7ffff64c97e0
>         type <integer_type 0x7ffff64a7690 int public SI
>             size <integer_cst 0x7ffff64c50a8 constant 32>
>             unit size <integer_cst 0x7ffff64c50c0 constant 4>
>             align 32 symtab 0 alias set -1 canonical type 0x7ffff64a7690
> precision 32 min <integer_cst 0x7ffff64c5060 -2147483648> max <integer_cst
> 0x7ffff64c5078 2147483647>
>             pointer_to_this <pointer_type 0x7ffff64c97e0>>
>         sizes-gimplified unsigned DI
>         size <integer_cst 0x7ffff64a3e58 constant 64>
>         unit size <integer_cst 0x7ffff64a3e70 constant 8>
>         align 64 symtab 0 alias set -1 canonical type 0x7ffff64c97e0
>         pointer_to_this <pointer_type 0x7ffff6622540>>
>     side-effects
>     arg 0 <var_decl 0x7ffff64b0c60 c
>         type <array_type 0x7ffff64ccbd0 __builtin_va_list type <record_type
> 0x7ffff64cc738 __va_list_tag>
>             sizes-gimplified BLK
>             size <integer_cst 0x7ffff64c5318 constant 192>
>             unit size <integer_cst 0x7ffff64c52e8 constant 24>
>             align 64 symtab 0 alias set -1 canonical type 0x7ffff64cc888
> domain <integer_type 0x7ffff64cc7e0>>
>         used tree_1 decl_5 BLK file test.cc line 13 col 25 size <integer_cst
> 0x7ffff64c5318 192> unit size <integer_cst 0x7ffff64c52e8 24>
>         align 64 context <function_decl 0x7ffff66240d8 fn1>>
>     test.cc:15:32>
> (gdb) call tree_could_throw_p (*expr_p )
> $1 = false
> ...
> 
> I'll bootstrap and reg-test this fix.
Comment 8 Tom de Vries 2015-04-20 08:42:55 UTC
(In reply to Richard Biener from comment #7)
> (In reply to vries from comment #6)
> > (In reply to vries from comment #5)
> > > So the question is: should ifn_va_arg have ECF_NOTHROW?
> > 
> > Adding ECF_NOTHROW to ifn_va_arg also fixes the ICE.
> > 
> > And at gimplify_va_arg_expr, the VA_ARG_EXPR tree is non-throwing:
> 
> Is that always the case (try -fnon-call-exceptions)?  Then doing that looks
> obvious.
> 
> Richard.
> 

Yep, that's also the case with -fnon-call-exceptions:
...
(gdb) call debug_generic_expr (*expr_p)
VA_ARG_EXPR <c>
(gdb) call tree_could_throw_p (*expr_p )
$1 = false
(gdb) p flag_non_call_exceptions
$2 = 1
...
Comment 9 Tom de Vries 2015-04-21 08:43:39 UTC
Author: vries
Date: Tue Apr 21 08:43:07 2015
New Revision: 222259

URL: https://gcc.gnu.org/viewcvs?rev=222259&root=gcc&view=rev
Log:
Mark ifn_va_arg with ECF_NOTHROW

2015-04-21  Tom de Vries  <tom@codesourcery.com>

	PR tree-optimization/65802
	* internal-fn.def (VA_ARG): Add ECF_NOTROW to flags.

	* g++.dg/pr65802.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/pr65802.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/internal-fn.def
    trunk/gcc/testsuite/ChangeLog
Comment 10 Tom de Vries 2015-04-21 08:46:48 UTC
patch with test-case committed, marking resolved fixed.
Comment 11 Tom de Vries 2015-04-24 14:19:29 UTC
Author: vries
Date: Fri Apr 24 14:18:57 2015
New Revision: 222413

URL: https://gcc.gnu.org/viewcvs?rev=222413&root=gcc&view=rev
Log:
Replace g++.dg/pr65802.C with gcc.dg/pr65802.c

2015-04-24  Tom de Vries  <tom@codesourcery.com>

	PR tree-optimization/65802
	* g++.dg/pr65802.C: Move to ...
	* gcc.dg/pr65802.c: ... here.  Add -fexceptions to dg-options. Include
	stdarg.h.  Rewrite for C.
	(fn1): Use va_list and va_arg.  Make variable args function.  Add use of
	va_start and va_end.  Remove unnecessary inline asm.

Added:
    trunk/gcc/testsuite/gcc.dg/pr65802.c
Removed:
    trunk/gcc/testsuite/g++.dg/pr65802.C
Modified:
    trunk/gcc/testsuite/ChangeLog