On Sat, Dec 7, 2024 at 9:29 PM Simon Martin <si...@nasilyan.com> wrote:
>
> The following valid code triggers an ICE with -fsanitize=address
>
> === cut here ===
> void l() {
> auto const ints = {0,1,2,3,4,5};
> for (auto i : { 3 } ) {
> __builtin_printf("%d ", i);
> }
> }
> === cut here ===
>
> The problem is that honor_protect_cleanup_actions does not expect the
> cleanup sequence of a GIMPLE_TRY_FINALLY to be empty. It is however the
> case here since r14-8681-gceb242f5302027, because lower_stmt removes the
> only statement in the sequence: a ASAN_MARK statement for the array that
> backs the initializer_list).
>
> This patch simply checks that the finally block is not 0 before
> accessing it in honor_protect_cleanup_actions.
>
> Successfully tested on x86_64-pc-linux-gnu. OK for trunk and gcc-14?
>
> PR c++/117845
>
> gcc/ChangeLog:
>
> * tree-eh.cc (honor_protect_cleanup_actions): Support empty
> finally sequences.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/asan/pr117845-2.C: New test.
> * g++.dg/asan/pr117845.C: New test.
>
> ---
> gcc/testsuite/g++.dg/asan/pr117845-2.C | 12 ++++++++++++
> gcc/testsuite/g++.dg/asan/pr117845.C | 12 ++++++++++++
> gcc/tree-eh.cc | 3 ++-
> 3 files changed, 26 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.dg/asan/pr117845-2.C
> create mode 100644 gcc/testsuite/g++.dg/asan/pr117845.C
>
> diff --git a/gcc/testsuite/g++.dg/asan/pr117845-2.C
> b/gcc/testsuite/g++.dg/asan/pr117845-2.C
> new file mode 100644
> index 00000000000..c0556397009
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/asan/pr117845-2.C
> @@ -0,0 +1,12 @@
> +// PR c++/117845 - Actually valid variant
> +// { dg-do "compile" }
> +// { dg-options "-fsanitize=address" }
> +
> +#include <initializer_list>
> +
> +void l() {
> + auto const ints = {0,1,2,3,4,5};
> + for (auto i : { 3 } ) {
> + __builtin_printf("%d ", i);
> + }
> +}
> diff --git a/gcc/testsuite/g++.dg/asan/pr117845.C
> b/gcc/testsuite/g++.dg/asan/pr117845.C
> new file mode 100644
> index 00000000000..d90d351e270
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/asan/pr117845.C
> @@ -0,0 +1,12 @@
> +// PR c++/117845 - Initially reported case.
> +// { dg-do "compile" }
> +// { dg-options "-fsanitize=address" }
> +
> +#include <initializer_list>
> +
> +void l() {
> + auto const ints = {0,1,2,3,4,5};
> + for (int i : ints | h) { // { dg-error "was not declared" }
> + __builtin_printf("%d ", i);
> + }
> +}
> diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc
> index 769785fad2b..dc920de9b38 100644
> --- a/gcc/tree-eh.cc
> +++ b/gcc/tree-eh.cc
> @@ -1026,7 +1026,8 @@ honor_protect_cleanup_actions (struct leh_state
> *outer_state,
> MUST_NOT_THROW filter. */
> gimple_stmt_iterator gsi = gsi_start (finally);
> gimple *x = gsi_stmt (gsi);
> - if (gimple_code (x) == GIMPLE_TRY
> + if (x
style-wise you should check for gsi_end_p (gsi) before
calling gsi_stmt on the iterator. Implementation-wise
your patch has the same effect, of course.
Can you still refactor it this way?
Thanks,
Richard.
> + && gimple_code (x) == GIMPLE_TRY
> && gimple_try_kind (x) == GIMPLE_TRY_CATCH
> && gimple_try_catch_is_cleanup (x))
> {
> --
> 2.44.0
>
