On 8 Dec 2024, at 11:10, Simon Martin wrote:
> Hi Richard,
>
> On 8 Dec 2024, at 10:27, Richard Biener wrote:
>
>> 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?
> Sure, here’s the updated version that I’m currently testing. Ok for
> trunk and gcc-14 assuming the testing comes back all green?
FYI the testing on x86_64-pc-linux-gnu of the updated patch was successful.
Simon
