diff mbox

Remove unneeded gcc_assert in gimplifier (PR sanitizer/78270)

Message ID 4a129694-400b-9899-de79-4f0f07ed6311@suse.cz
State New
Headers show

Commit Message

Martin Liška Nov. 10, 2016, 11:02 a.m. UTC 
On 11/09/2016 02:47 PM, Martin Liška wrote:
> 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


Sending the patch.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

> 

>>

>> 	Jakub

>>

>

Comments

Jakub Jelinek Nov. 10, 2016, 11:07 a.m. UTC | #1 
On Thu, Nov 10, 2016 at 12:02:45PM +0100, Martin Liška wrote:
> >From fb4b852a17656309e6acfb8da97cf9bce4b3b176 Mon Sep 17 00:00:00 2001

> From: marxin <mliska@suse.cz>

> Date: Wed, 9 Nov 2016 11:52:00 +0100

> Subject: [PATCH] Create live_switch_vars conditionally (PR sanitizer/78270)

> 

> gcc/testsuite/ChangeLog:

> 

> 2016-11-09  Martin Liska  <mliska@suse.cz>

> 


Missing
	PR sanitizer/78270
note.

> 	* gcc.dg/asan/pr78269.c: New test.


Misnamed test, should be pr78270.c.
> 

> gcc/ChangeLog:

> 

> 2016-11-09  Martin Liska  <mliska@suse.cz>

> 


Also missing the PR line.

> 	* gimplify.c (gimplify_switch_expr): Create live_switch_vars

> 	only when SWITCH_BODY is a BIND_EXPR.


Ok with those nits fixed.

	Jakub
diff mbox

Patch

From fb4b852a17656309e6acfb8da97cf9bce4b3b176 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 9 Nov 2016 11:52:00 +0100
Subject: [PATCH] Create live_switch_vars conditionally (PR sanitizer/78270)

gcc/testsuite/ChangeLog:

2016-11-09  Martin Liska  <mliska@suse.cz>

	* gcc.dg/asan/pr78269.c: New test.

gcc/ChangeLog:

2016-11-09  Martin Liska  <mliska@suse.cz>

	* gimplify.c (gimplify_switch_expr): Create live_switch_vars
	only when SWITCH_BODY is a BIND_EXPR.
---
 gcc/gimplify.c                      | 20 +++++++++++++++-----
 gcc/testsuite/gcc.dg/asan/pr78269.c | 13 +++++++++++++
 2 files changed, 28 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/asan/pr78269.c

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index d392450..da60c05 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -2241,7 +2241,7 @@  gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p)
     {
       vec<tree> labels;
       vec<tree> saved_labels;
-      hash_set<tree> *saved_live_switch_vars;
+      hash_set<tree> *saved_live_switch_vars = NULL;
       tree default_case = NULL_TREE;
       gswitch *switch_stmt;
 
@@ -2253,8 +2253,14 @@  gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p)
          labels.  Save all the things from the switch body to append after.  */
       saved_labels = gimplify_ctxp->case_labels;
       gimplify_ctxp->case_labels.create (8);
-      saved_live_switch_vars = gimplify_ctxp->live_switch_vars;
-      gimplify_ctxp->live_switch_vars = new hash_set<tree> (4);
+
+      /* Do not create live_switch_vars if SWITCH_BODY is not a BIND_EXPR.  */
+      if (TREE_CODE (SWITCH_BODY (switch_expr)) == BIND_EXPR)
+	{
+	  saved_live_switch_vars = gimplify_ctxp->live_switch_vars;
+	  gimplify_ctxp->live_switch_vars = new hash_set<tree> (4);
+	}
+
       bool old_in_switch_expr = gimplify_ctxp->in_switch_expr;
       gimplify_ctxp->in_switch_expr = true;
 
@@ -2269,8 +2275,12 @@  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;
+
+      if (gimplify_ctxp->live_switch_vars)
+	{
+	  gcc_assert (gimplify_ctxp->live_switch_vars->elements () == 0);
+	  delete gimplify_ctxp->live_switch_vars;
+	}
       gimplify_ctxp->live_switch_vars = saved_live_switch_vars;
 
       preprocess_case_label_vec_for_gimple (labels, index_type,
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