| Message ID | 475cb650-8a3d-8395-9f93-da12e8e841a1@suse.cz |
|---|---|
| State | New |
| Headers | show |
On Wed, Nov 09, 2016 at 02:16:45PM +0100, Martin Liška wrote: > As shown in the attached test-case, the assert cannot always be true. > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Ready to be installed? > Martin > >From b55459461f3f7396a094be6801082715ddb4b30d Mon Sep 17 00:00:00 2001 > From: marxin <mliska@suse.cz> > Date: Wed, 9 Nov 2016 11:52:00 +0100 > Subject: [PATCH] Remove unneeded gcc_assert in gimplifier (PR sanitizer/78270) > > gcc/ChangeLog: > > 2016-11-09 Martin Liska <mliska@suse.cz> > > PR sanitizer/78270 > * gimplify.c (gimplify_switch_expr): No description on what you've changed. That said, I'm not 100% sure it is the right fix. As the testcase shows, for switch without GIMPLE_BIND wrapping the body we can have variables that are in scope from the switch onwards. I bet we could also have variables that go out of scope, say if in the compound literal's initializer there is ({ ... }) that declares variables. I doubt you can have a valid case/default label in those though, so perhaps it would be simpler not to create live_switch_vars at all if SWITCH_BODY is not a BIND_EXPR? Jakub
On 11/09/2016 02:29 PM, Jakub Jelinek wrote: > On Wed, Nov 09, 2016 at 02:16:45PM +0100, Martin Liška wrote: >> As shown in the attached test-case, the assert cannot always be true. >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >> >> Ready to be installed? >> Martin > >> >From b55459461f3f7396a094be6801082715ddb4b30d Mon Sep 17 00:00:00 2001 >> From: marxin <mliska@suse.cz> >> Date: Wed, 9 Nov 2016 11:52:00 +0100 >> Subject: [PATCH] Remove unneeded gcc_assert in gimplifier (PR sanitizer/78270) >> >> gcc/ChangeLog: >> >> 2016-11-09 Martin Liska <mliska@suse.cz> >> >> PR sanitizer/78270 >> * gimplify.c (gimplify_switch_expr): > > No description on what you've changed. > > That said, I'm not 100% sure it is the right fix. > As the testcase shows, for switch without GIMPLE_BIND wrapping the body > we can have variables that are in scope from the switch onwards. > I bet we could also have variables that go out of scope, say if in the > compound literal's initializer there is ({ ... }) that declares variables. > I doubt you can have a valid case/default label in those though, so > perhaps it would be simpler not to create live_switch_vars at all > if SWITCH_BODY is not a BIND_EXPR? I like the approach you introduced! I'll re-trigger regression tests and send a newer version of patch. Martin > > Jakub >
From b55459461f3f7396a094be6801082715ddb4b30d Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Wed, 9 Nov 2016 11:52:00 +0100 Subject: [PATCH] Remove unneeded gcc_assert in gimplifier (PR sanitizer/78270) gcc/ChangeLog: 2016-11-09 Martin Liska <mliska@suse.cz> PR sanitizer/78270 * gimplify.c (gimplify_switch_expr): gcc/testsuite/ChangeLog: 2016-11-09 Martin Liska <mliska@suse.cz> PR sanitizer/78270 * gcc.dg/asan/pr78269.c: New test. --- gcc/gimplify.c | 1 - gcc/testsuite/gcc.dg/asan/pr78269.c | 13 +++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.dg/asan/pr78269.c diff --git a/gcc/gimplify.c b/gcc/gimplify.c index e5930e6..15976e2 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -2266,7 +2266,6 @@ gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p) labels = gimplify_ctxp->case_labels; gimplify_ctxp->case_labels = saved_labels; - gcc_assert (gimplify_ctxp->live_switch_vars->elements () == 0); delete gimplify_ctxp->live_switch_vars; gimplify_ctxp->live_switch_vars = saved_live_switch_vars; diff --git a/gcc/testsuite/gcc.dg/asan/pr78269.c b/gcc/testsuite/gcc.dg/asan/pr78269.c new file mode 100644 index 0000000..55840b0 --- /dev/null +++ b/gcc/testsuite/gcc.dg/asan/pr78269.c @@ -0,0 +1,13 @@ +// { dg-do compile } +// { dg-additional-options "-Wno-switch-unreachable" } + +typedef struct +{ +} bdaddr_t; + +int a; +void fn1 () +{ + switch (a) + &(bdaddr_t){}; +} -- 2.10.1