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?
Thanks!
SimonFrom b7d2918b249b57e2ca236acac66cc3503f5bddeb Mon Sep 17 00:00:00 2001
From: Simon Martin <si...@nasilyan.com>
Date: Fri, 6 Dec 2024 11:04:24 +0100
Subject: [PATCH] tree-eh: Don't crash on GIMPLE_TRY_FINALLY with empty cleanup
sequence [PR117845]
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 | 5 +++--
3 files changed, 27 insertions(+), 2 deletions(-)
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..e8af5fb8989 100644
--- a/gcc/tree-eh.cc
+++ b/gcc/tree-eh.cc
@@ -1025,8 +1025,9 @@ honor_protect_cleanup_actions (struct leh_state
*outer_state,
terminate before we get to it, so strip it away before adding the
MUST_NOT_THROW filter. */
gimple_stmt_iterator gsi = gsi_start (finally);
- gimple *x = gsi_stmt (gsi);
- if (gimple_code (x) == GIMPLE_TRY
+ gimple *x = !gsi_end_p (gsi) ? gsi_stmt (gsi) : NULL;
+ if (x
+ && gimple_code (x) == GIMPLE_TRY
&& gimple_try_kind (x) == GIMPLE_TRY_CATCH
&& gimple_try_catch_is_cleanup (x))
{
--
2.44.0
