* [PATCH] sanopt: Return TODO_cleanup_cfg if any .{UB,HWA,A}SAN_* calls were lowered [PR106190]
@ 2023-03-28 8:04 Jakub Jelinek
2023-03-28 8:46 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2023-03-28 8:04 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
Hi!
The following testcase ICEs, because without optimization eh lowering
decides not to duplicate finally block of try/finally and so we end up
with variable guarded cleanup. The sanopt pass creates a cfg that ought
to be cleaned up (some IFN_UBSAN_* functions are lowered in this case with
constant conditions in gcond and when not allowing recovery some bbs which
end with noreturn calls actually have successor edges), but the cfg cleanup
is actually (it is -O0) done only during the optimized pass. We notice
there that the d[1][a] = 0; statement which has an EH edge is unreachable
(because ubsan would always abort on the out of bounds d[1] access), remove
the EH landing pad and block, but because that block just sets a variable
and jumps to another one which tests that variable and that one is reachable
from normal control flow, the __builtin_eh_pointer (1) later in there is
kept in the IL and we ICE during expansion of that statement because the
EH region has been removed.
The following patch fixes it by doing the cfg cleanup already during
sanopt pass if we create something that might need it, while the EH
landing pad is then removed already during sanopt pass, there is ehcleanup
later and we don't ICE anymore.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2023-03-28 Jakub Jelinek <jakub@redhat.com>
PR middle-end/106190
* sanopt.cc (pass_sanopt::execute): Return TODO_cleanup_cfg if any
of the IFN_{UB,HWA,A}SAN_* internal fns are lowered.
* gcc.dg/asan/pr106190.c: New test.
--- gcc/sanopt.cc.jj 2023-02-15 09:23:27.832389821 +0100
+++ gcc/sanopt.cc 2023-03-27 16:00:23.758621014 +0200
@@ -1300,6 +1300,7 @@ pass_sanopt::execute (function *fun)
basic_block bb;
int asan_num_accesses = 0;
bool contains_asan_mark = false;
+ int ret = 0;
/* Try to remove redundant checks. */
if (optimize
@@ -1352,6 +1353,7 @@ pass_sanopt::execute (function *fun)
if (gimple_call_internal_p (stmt))
{
enum internal_fn ifn = gimple_call_internal_fn (stmt);
+ int this_ret = TODO_cleanup_cfg;
switch (ifn)
{
case IFN_UBSAN_NULL:
@@ -1387,8 +1389,10 @@ pass_sanopt::execute (function *fun)
no_next = hwasan_expand_mark_ifn (&gsi);
break;
default:
+ this_ret = 0;
break;
}
+ ret |= this_ret;
}
else if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
{
@@ -1418,7 +1422,7 @@ pass_sanopt::execute (function *fun)
if (need_commit_edge_insert)
gsi_commit_edge_inserts ();
- return 0;
+ return ret;
}
} // anon namespace
--- gcc/testsuite/gcc.dg/asan/pr106190.c.jj 2023-03-27 16:50:15.685501029 +0200
+++ gcc/testsuite/gcc.dg/asan/pr106190.c 2023-03-27 16:51:25.187499082 +0200
@@ -0,0 +1,15 @@
+/* PR middle-end/106190 */
+/* { dg-do compile } */
+/* { dg-options "-fnon-call-exceptions -fsanitize=address,undefined -fno-sanitize-recover=all" } */
+
+int
+main ()
+{
+ int a;
+ int *b[1];
+ int c[10];
+ int d[1][1];
+ for (a = 0; a < 1; a++)
+ d[1][a] = 0;
+ return 0;
+}
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] sanopt: Return TODO_cleanup_cfg if any .{UB,HWA,A}SAN_* calls were lowered [PR106190]
2023-03-28 8:04 [PATCH] sanopt: Return TODO_cleanup_cfg if any .{UB,HWA,A}SAN_* calls were lowered [PR106190] Jakub Jelinek
@ 2023-03-28 8:46 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2023-03-28 8:46 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Tue, 28 Mar 2023, Jakub Jelinek wrote:
> Hi!
>
> The following testcase ICEs, because without optimization eh lowering
> decides not to duplicate finally block of try/finally and so we end up
> with variable guarded cleanup. The sanopt pass creates a cfg that ought
> to be cleaned up (some IFN_UBSAN_* functions are lowered in this case with
> constant conditions in gcond and when not allowing recovery some bbs which
> end with noreturn calls actually have successor edges), but the cfg cleanup
> is actually (it is -O0) done only during the optimized pass. We notice
> there that the d[1][a] = 0; statement which has an EH edge is unreachable
> (because ubsan would always abort on the out of bounds d[1] access), remove
> the EH landing pad and block, but because that block just sets a variable
> and jumps to another one which tests that variable and that one is reachable
> from normal control flow, the __builtin_eh_pointer (1) later in there is
> kept in the IL and we ICE during expansion of that statement because the
> EH region has been removed.
>
> The following patch fixes it by doing the cfg cleanup already during
> sanopt pass if we create something that might need it, while the EH
> landing pad is then removed already during sanopt pass, there is ehcleanup
> later and we don't ICE anymore.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2023-03-28 Jakub Jelinek <jakub@redhat.com>
>
> PR middle-end/106190
> * sanopt.cc (pass_sanopt::execute): Return TODO_cleanup_cfg if any
> of the IFN_{UB,HWA,A}SAN_* internal fns are lowered.
>
> * gcc.dg/asan/pr106190.c: New test.
>
> --- gcc/sanopt.cc.jj 2023-02-15 09:23:27.832389821 +0100
> +++ gcc/sanopt.cc 2023-03-27 16:00:23.758621014 +0200
> @@ -1300,6 +1300,7 @@ pass_sanopt::execute (function *fun)
> basic_block bb;
> int asan_num_accesses = 0;
> bool contains_asan_mark = false;
> + int ret = 0;
>
> /* Try to remove redundant checks. */
> if (optimize
> @@ -1352,6 +1353,7 @@ pass_sanopt::execute (function *fun)
> if (gimple_call_internal_p (stmt))
> {
> enum internal_fn ifn = gimple_call_internal_fn (stmt);
> + int this_ret = TODO_cleanup_cfg;
> switch (ifn)
> {
> case IFN_UBSAN_NULL:
> @@ -1387,8 +1389,10 @@ pass_sanopt::execute (function *fun)
> no_next = hwasan_expand_mark_ifn (&gsi);
> break;
> default:
> + this_ret = 0;
> break;
> }
> + ret |= this_ret;
> }
> else if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
> {
> @@ -1418,7 +1422,7 @@ pass_sanopt::execute (function *fun)
> if (need_commit_edge_insert)
> gsi_commit_edge_inserts ();
>
> - return 0;
> + return ret;
> }
>
> } // anon namespace
> --- gcc/testsuite/gcc.dg/asan/pr106190.c.jj 2023-03-27 16:50:15.685501029 +0200
> +++ gcc/testsuite/gcc.dg/asan/pr106190.c 2023-03-27 16:51:25.187499082 +0200
> @@ -0,0 +1,15 @@
> +/* PR middle-end/106190 */
> +/* { dg-do compile } */
> +/* { dg-options "-fnon-call-exceptions -fsanitize=address,undefined -fno-sanitize-recover=all" } */
> +
> +int
> +main ()
> +{
> + int a;
> + int *b[1];
> + int c[10];
> + int d[1][1];
> + for (a = 0; a < 1; a++)
> + d[1][a] = 0;
> + return 0;
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-03-28 8:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-28 8:04 [PATCH] sanopt: Return TODO_cleanup_cfg if any .{UB,HWA,A}SAN_* calls were lowered [PR106190] Jakub Jelinek
2023-03-28 8:46 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).