| Message ID | 20150218184020.GP1746@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On 02/18/15 11:40, Jakub Jelinek wrote: > Hi! > > Richard's GIMPLE EH rewrite in r151696 regressed following testcase. > The problem is that when lowering: > [gcov-15.C:14:5] try > { > [gcov-15.C:18:12] D.2335 = __cxa_allocate_exception (4); > [gcov-15.C:18:12] try > { > [gcov-15.C:18:12] [gcov-15.C:18:12] MEM[(int *)D.2335] = 5; > } > catch > { > [gcov-15.C:18:11] __cxa_free_exception (D.2335); > } > [gcov-15.C:18:11] __cxa_throw (D.2335, &_ZTIi, 0B); > } > catch > { > [gcov-15.C:20:3] catch (NULL) > { > [gcov-15.C:20:3] try > { > [gcov-15.C:20:10] D.2340 = __builtin_eh_pointer (0); > [gcov-15.C:20:10] __cxa_begin_catch (D.2340); > [gcov-15.C:22:15] catchEx (); > } > finally > { > [gcov-15.C:20:10] __cxa_end_catch (); > } > } > } > we put the cleanup of the catch cleanup in front of the catch cleanup > in the EH sequence: > [gcov-15.C:18:12] D.2335 = __cxa_allocate_exception (4); > [gcov-15.C:18:12] [gcov-15.C:18:12] MEM[(int *)D.2335] = 5; > [gcov-15.C:18:11] __cxa_throw (D.2335, &_ZTIi, 0B); > <D.2345>: > [gcov-15.C:24:1] D.2341 = 0; > [gcov-15.C:24:1] goto <D.2342>; > <D.2342>: > [gcov-15.C:24:1] return D.2341; > <D.2343>: > [gcov-15.C:20:10] __cxa_end_catch (); > resx 3 > <D.2346>: > eh_dispatch 1 > resx 1 > <D.2344>: > [gcov-15.C:20:10] D.2340 = __builtin_eh_pointer (1); > [gcov-15.C:20:10] __cxa_begin_catch (D.2340); > [gcov-15.C:22:15] catchEx (); > [gcov-15.C:20:10] __cxa_end_catch (); > goto <D.2345>; > and as the __cxa_end_catch () is the first bb for line 20, > gcov without -a considers that bb count as the one to be shown. > Before the gimple EH rewrite and also with this patch we instead > order the cleanup (__cxa_end_catch ()) after the __cxa_begin_catch (): > [gcov-15.C:18:12] D.2335 = __cxa_allocate_exception (4); > [gcov-15.C:18:12] [gcov-15.C:18:12] MEM[(int *)D.2335] = 5; > [gcov-15.C:18:11] __cxa_throw (D.2335, &_ZTIi, 0B); > <D.2345>: > [gcov-15.C:24:1] D.2341 = 0; > [gcov-15.C:24:1] goto <D.2342>; > <D.2342>: > [gcov-15.C:24:1] return D.2341; > <D.2346>: > eh_dispatch 1 > resx 1 > <D.2344>: > [gcov-15.C:20:10] D.2340 = __builtin_eh_pointer (1); > [gcov-15.C:20:10] __cxa_begin_catch (D.2340); > [gcov-15.C:22:15] catchEx (); > [gcov-15.C:20:10] __cxa_end_catch (); > goto <D.2345>; > <D.2343>: > [gcov-15.C:20:10] __cxa_end_catch (); > resx 3 > > Bootstrapped/regtested on x86_64-linux and i686-linux, > libstdc++.so.6 compiled without and with the patch is identical on > x86_64-linux. The testcase also has identical generated code both at -O0 > and -O2 when compiled without coverage. Ok for trunk? > > 2015-02-18 Jakub Jelinek <jakub@redhat.com> > > PR gcov-profile/64634 > * tree-eh.c (frob_into_branch_around): Fix up typos > in function comment. > (lower_catch): Put eh_seq resulting from EH lowering of > the cleanup sequence after the cleanup rather than before > it. > > * g++.dg/gcov/gcov-15.C: New test. OK. jeff
--- gcc/tree-eh.c.jj 2015-02-12 08:57:35.000000000 +0100 +++ gcc/tree-eh.c 2015-02-18 16:46:14.878887862 +0100 @@ -884,10 +884,10 @@ eh_region_may_contain_throw (eh_region r /* We want to transform try { body; } catch { stuff; } to - normal_seqence: + normal_sequence: body; over: - eh_seqence: + eh_sequence: landing_pad: stuff; goto over; @@ -1813,6 +1813,12 @@ lower_catch (struct leh_state *state, gt this_state.cur_region = state->cur_region; this_state.ehp_region = try_region; + /* Add eh_seq from lowering EH in the cleanup sequence after the cleanup + itself, so that e.g. for coverage purposes the nested cleanups don't + appear before the cleanup body. See PR64634 for details. */ + gimple_seq old_eh_seq = eh_seq; + eh_seq = NULL; + out_label = NULL; cleanup = gimple_try_cleanup (tp); for (gsi = gsi_start (cleanup); @@ -1849,7 +1855,11 @@ lower_catch (struct leh_state *state, gt gimple_try_set_cleanup (tp, new_seq); - return frob_into_branch_around (tp, try_region, out_label); + gimple_seq new_eh_seq = eh_seq; + eh_seq = old_eh_seq; + gimple_seq ret_seq = frob_into_branch_around (tp, try_region, out_label); + gimple_seq_add_seq (&eh_seq, new_eh_seq); + return ret_seq; } /* A subroutine of lower_eh_constructs_1. Lower a GIMPLE_TRY with a --- gcc/testsuite/g++.dg/gcov/gcov-15.C.jj 2015-02-18 17:06:35.599727342 +0100 +++ gcc/testsuite/g++.dg/gcov/gcov-15.C 2015-02-18 17:17:04.483358209 +0100 @@ -0,0 +1,26 @@ +// PR gcov-profile/64634 +// { dg-options "-fprofile-arcs -ftest-coverage" } +// { dg-do run { target native } } + +void catchEx () // count(1) +{ + __builtin_exit (0); // count(1) + try + {} + catch (int) + {} +} + +int main () // count(1) +{ + try + { + throw 5; // count(1) + } + catch (...) // count(1) + { + catchEx (); // count(1) + } +} + +// { dg-final { run-gcov gcov-15.C } }
Hi! Richard's GIMPLE EH rewrite in r151696 regressed following testcase. The problem is that when lowering: [gcov-15.C:14:5] try { [gcov-15.C:18:12] D.2335 = __cxa_allocate_exception (4); [gcov-15.C:18:12] try { [gcov-15.C:18:12] [gcov-15.C:18:12] MEM[(int *)D.2335] = 5; } catch { [gcov-15.C:18:11] __cxa_free_exception (D.2335); } [gcov-15.C:18:11] __cxa_throw (D.2335, &_ZTIi, 0B); } catch { [gcov-15.C:20:3] catch (NULL) { [gcov-15.C:20:3] try { [gcov-15.C:20:10] D.2340 = __builtin_eh_pointer (0); [gcov-15.C:20:10] __cxa_begin_catch (D.2340); [gcov-15.C:22:15] catchEx (); } finally { [gcov-15.C:20:10] __cxa_end_catch (); } } } we put the cleanup of the catch cleanup in front of the catch cleanup in the EH sequence: [gcov-15.C:18:12] D.2335 = __cxa_allocate_exception (4); [gcov-15.C:18:12] [gcov-15.C:18:12] MEM[(int *)D.2335] = 5; [gcov-15.C:18:11] __cxa_throw (D.2335, &_ZTIi, 0B); <D.2345>: [gcov-15.C:24:1] D.2341 = 0; [gcov-15.C:24:1] goto <D.2342>; <D.2342>: [gcov-15.C:24:1] return D.2341; <D.2343>: [gcov-15.C:20:10] __cxa_end_catch (); resx 3 <D.2346>: eh_dispatch 1 resx 1 <D.2344>: [gcov-15.C:20:10] D.2340 = __builtin_eh_pointer (1); [gcov-15.C:20:10] __cxa_begin_catch (D.2340); [gcov-15.C:22:15] catchEx (); [gcov-15.C:20:10] __cxa_end_catch (); goto <D.2345>; and as the __cxa_end_catch () is the first bb for line 20, gcov without -a considers that bb count as the one to be shown. Before the gimple EH rewrite and also with this patch we instead order the cleanup (__cxa_end_catch ()) after the __cxa_begin_catch (): [gcov-15.C:18:12] D.2335 = __cxa_allocate_exception (4); [gcov-15.C:18:12] [gcov-15.C:18:12] MEM[(int *)D.2335] = 5; [gcov-15.C:18:11] __cxa_throw (D.2335, &_ZTIi, 0B); <D.2345>: [gcov-15.C:24:1] D.2341 = 0; [gcov-15.C:24:1] goto <D.2342>; <D.2342>: [gcov-15.C:24:1] return D.2341; <D.2346>: eh_dispatch 1 resx 1 <D.2344>: [gcov-15.C:20:10] D.2340 = __builtin_eh_pointer (1); [gcov-15.C:20:10] __cxa_begin_catch (D.2340); [gcov-15.C:22:15] catchEx (); [gcov-15.C:20:10] __cxa_end_catch (); goto <D.2345>; <D.2343>: [gcov-15.C:20:10] __cxa_end_catch (); resx 3 Bootstrapped/regtested on x86_64-linux and i686-linux, libstdc++.so.6 compiled without and with the patch is identical on x86_64-linux. The testcase also has identical generated code both at -O0 and -O2 when compiled without coverage. Ok for trunk? 2015-02-18 Jakub Jelinek <jakub@redhat.com> PR gcov-profile/64634 * tree-eh.c (frob_into_branch_around): Fix up typos in function comment. (lower_catch): Put eh_seq resulting from EH lowering of the cleanup sequence after the cleanup rather than before it. * g++.dg/gcov/gcov-15.C: New test. Jakub