| Message ID | 20240711200820.502745-1-polacek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | eh: ICE with std::initializer_list and ASan [PR115865] | expand |
On Thu, Jul 11, 2024 at 04:08:20PM -0400, Marek Polacek wrote: > and we ICE on the empty finally in lower_try_finally_onedest while > getting get_eh_else. > > Rather than checking everywhere that a GIMPLE_TRY_FINALLY is not empty, > I thought we could process it as if it were unreachable. I'm not sure we need to check everywhere, the only spot is get_eh_else from what I can see and IMHO that should be fixed with something like: --- gcc/tree-eh.cc.jj 2024-01-03 11:51:27.490787274 +0100 +++ gcc/tree-eh.cc 2024-07-17 13:21:01.093840884 +0200 @@ -950,7 +950,7 @@ static inline geh_else * get_eh_else (gimple_seq finally) { gimple *x = gimple_seq_first_stmt (finally); - if (gimple_code (x) == GIMPLE_EH_ELSE) + if (x && gimple_code (x) == GIMPLE_EH_ELSE) { gcc_assert (gimple_seq_singleton_p (finally)); return as_a <geh_else *> (x); because empty finally sequence certainly doesn't contain GIMPLE_EH_ELSE. Looking at other spots, I see it is able to deal with empty sequences: if (gimple_seq_singleton_p (new_seq) && gimple_code (gimple_seq_first_stmt (new_seq)) == GIMPLE_GOTO) gimple_seq_singleton_p is false for empty new_seq. inner = gimple_seq_first_stmt (gimple_try_cleanup (tp)); if (flag_exceptions) { this_region = gen_eh_region_allowed (state->cur_region, gimple_eh_filter_types (inner)); in lower_eh_filter will not work with empty gimple_try_cleanup, but caller will only call it if it is non-empty and starts with GIMPLE_EH_FILTER. gimple *inner = gimple_seq_first_stmt (gimple_try_cleanup (tp)); eh_region this_region; this_region = gen_eh_region_must_not_throw (state->cur_region); this_region->u.must_not_throw.failure_decl = gimple_eh_must_not_throw_fndecl ( as_a <geh_mnt *> (inner)); Similarly (lower_eh_must_not_throw and GIMPLE_EH_MUST_NOT_THROW). x = gimple_seq_first_stmt (gimple_try_cleanup (try_stmt)); if (!x) { replace = gimple_try_eval (try_stmt); lower_eh_constructs_1 (state, &replace); } else switch (gimple_code (x)) { This case shows special case for empty cleanup and otherwise calls lower_eh_* etc. depending on what the cleanup first statement is. x = gimple_seq_last_stmt (finally); finally_loc = x ? gimple_location (x) : tf_loc; This allows for finally to be empty again. > > PR c++/115865 > > gcc/ChangeLog: > > * tree-eh.cc (lower_try_finally): If the FINALLY block is empty, treat > it as if it were not reachable. > > gcc/testsuite/ChangeLog: > > * g++.dg/asan/initlist2.C: New test. > --- > gcc/testsuite/g++.dg/asan/initlist2.C | 16 ++++++++++++++++ > gcc/tree-eh.cc | 4 ++-- > 2 files changed, 18 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/asan/initlist2.C > > diff --git a/gcc/testsuite/g++.dg/asan/initlist2.C b/gcc/testsuite/g++.dg/asan/initlist2.C > new file mode 100644 > index 00000000000..bce5410be33 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/asan/initlist2.C > @@ -0,0 +1,16 @@ > +// PR c++/115865 > +// { dg-do compile } > +// { dg-options "-fsanitize=address" } > + > +typedef decltype(sizeof(char)) size_t; > + > +namespace std { > +template <class> class initializer_list { > + int *_M_array; > + size_t _M_len; > +}; > +} > + > +int main() { > + std::initializer_list x = { 1, 2, 3 }; > +} > diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc > index a776ad5c92b..6ae6dab223e 100644 > --- a/gcc/tree-eh.cc > +++ b/gcc/tree-eh.cc > @@ -1703,8 +1703,8 @@ lower_try_finally (struct leh_state *state, gtry *tp) > ndests += this_tf.may_return; > ndests += this_tf.may_throw; > > - /* If the FINALLY block is not reachable, dike it out. */ > - if (ndests == 0) > + /* If the FINALLY block is not reachable or empty, dike it out. */ > + if (ndests == 0 || gimple_seq_empty_p (gimple_try_cleanup (tp))) > { > gimple_seq_add_seq (&this_tf.top_p_seq, gimple_try_eval (tp)); > gimple_try_set_cleanup (tp, NULL); > > base-commit: 74d8accaf88f83bfcab1150bf9be5140e7ac0e94 > -- > 2.45.2 Jakub
diff --git a/gcc/testsuite/g++.dg/asan/initlist2.C b/gcc/testsuite/g++.dg/asan/initlist2.C new file mode 100644 index 00000000000..bce5410be33 --- /dev/null +++ b/gcc/testsuite/g++.dg/asan/initlist2.C @@ -0,0 +1,16 @@ +// PR c++/115865 +// { dg-do compile } +// { dg-options "-fsanitize=address" } + +typedef decltype(sizeof(char)) size_t; + +namespace std { +template <class> class initializer_list { + int *_M_array; + size_t _M_len; +}; +} + +int main() { + std::initializer_list x = { 1, 2, 3 }; +} diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc index a776ad5c92b..6ae6dab223e 100644 --- a/gcc/tree-eh.cc +++ b/gcc/tree-eh.cc @@ -1703,8 +1703,8 @@ lower_try_finally (struct leh_state *state, gtry *tp) ndests += this_tf.may_return; ndests += this_tf.may_throw; - /* If the FINALLY block is not reachable, dike it out. */ - if (ndests == 0) + /* If the FINALLY block is not reachable or empty, dike it out. */ + if (ndests == 0 || gimple_seq_empty_p (gimple_try_cleanup (tp))) { gimple_seq_add_seq (&this_tf.top_p_seq, gimple_try_eval (tp)); gimple_try_set_cleanup (tp, NULL);
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- Here we ICE with -fsanitize=address on std::initializer_list x = { 1, 2, 3 }; since r14-8681, which removed .ASAN_MARK calls on TREE_STATIC variables. That means that lower_try_finally now instead of try { .ASAN_MARK (UNPOISON, &C.0, 12); x = {}; x._M_len = 3; x._M_array = &C.0; } finally { .ASAN_MARK (POISON, &C.0, 12); } gets: try { x = {}; x._M_len = 3; x._M_array = &C.0; } finally { } and we ICE on the empty finally in lower_try_finally_onedest while getting get_eh_else. Rather than checking everywhere that a GIMPLE_TRY_FINALLY is not empty, I thought we could process it as if it were unreachable. PR c++/115865 gcc/ChangeLog: * tree-eh.cc (lower_try_finally): If the FINALLY block is empty, treat it as if it were not reachable. gcc/testsuite/ChangeLog: * g++.dg/asan/initlist2.C: New test. --- gcc/testsuite/g++.dg/asan/initlist2.C | 16 ++++++++++++++++ gcc/tree-eh.cc | 4 ++-- 2 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/asan/initlist2.C base-commit: 74d8accaf88f83bfcab1150bf9be5140e7ac0e94