diff mbox

C PATCH to fix ASAN ICE with a compound literal (PR sanitizer/80659)

Message ID 20170516134051.GE24582@redhat.com
State New
Headers show

Commit Message

Marek Polacek May 16, 2017, 1:40 p.m. UTC
Martin L. asked me to have a look at this ICE with ASAN.  This is an ICE with a
compound literal in a switch.  A hash map gimplify_ctxp->live_switch_vars is
filled when processing DECL_EXPRs with their DECL_EXPR_DECLs.  Such decls should
then be removed from the hash map when gimplifying a BIND_EXPR.

In C, non-static compound literals aren't pushed into any scope, so they were
never found by gimplify_bind_expr, so they stayed in the hash map resulting in
a crash on
 2299       if (gimplify_ctxp->live_switch_vars)
 2300         {
 2301           gcc_assert (gimplify_ctxp->live_switch_vars->elements () == 0);
 2302           delete gimplify_ctxp->live_switch_vars;
 2303         }

We don't add artificial decls to the hash map:
 1647           if (!DECL_ARTIFICIAL (decl) && gimplify_ctxp->live_switch_vars)
 1648             gimplify_ctxp->live_switch_vars->add (decl);

build_compound_literal only marks a decl of a compound literal as artificial
when the compound literal is static.  I think there's no harm in marking
decls of non-static compound literals as artificial, too (provided it's fine to
skip asan instrumentation of compound literals).

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2017-05-16  Marek Polacek  <polacek@redhat.com>

	PR sanitizer/80659
	* c-decl.c (build_compound_literal): Set DECL_ARTIFICIAL even for
	non-static compound literals.

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


	Marek

Comments

Richard Biener May 17, 2017, 7:42 a.m. UTC | #1
On Tue, 16 May 2017, Marek Polacek wrote:

> Martin L. asked me to have a look at this ICE with ASAN.  This is an ICE with a
> compound literal in a switch.  A hash map gimplify_ctxp->live_switch_vars is
> filled when processing DECL_EXPRs with their DECL_EXPR_DECLs.  Such decls should
> then be removed from the hash map when gimplifying a BIND_EXPR.
> 
> In C, non-static compound literals aren't pushed into any scope, so they were
> never found by gimplify_bind_expr, so they stayed in the hash map resulting in
> a crash on
>  2299       if (gimplify_ctxp->live_switch_vars)
>  2300         {
>  2301           gcc_assert (gimplify_ctxp->live_switch_vars->elements () == 0);
>  2302           delete gimplify_ctxp->live_switch_vars;
>  2303         }
> 
> We don't add artificial decls to the hash map:
>  1647           if (!DECL_ARTIFICIAL (decl) && gimplify_ctxp->live_switch_vars)
>  1648             gimplify_ctxp->live_switch_vars->add (decl);
> 
> build_compound_literal only marks a decl of a compound literal as artificial
> when the compound literal is static.  I think there's no harm in marking
> decls of non-static compound literals as artificial, too (provided it's fine to
> skip asan instrumentation of compound literals).

But probably DECL_IGNORED_P as well at the same time?

> Bootstrapped/regtested on x86_64-linux, ok for trunk?

Ok with moving that as well (and re-testing).

Thanks,
Richard.

> 2017-05-16  Marek Polacek  <polacek@redhat.com>
> 
> 	PR sanitizer/80659
> 	* c-decl.c (build_compound_literal): Set DECL_ARTIFICIAL even for
> 	non-static compound literals.
> 
> 	* gcc.dg/asan/pr80659.c: New test.
> 
> diff --git gcc/c/c-decl.c gcc/c/c-decl.c
> index b779d37..887e95d 100644
> --- gcc/c/c-decl.c
> +++ gcc/c/c-decl.c
> @@ -5261,6 +5261,7 @@ build_compound_literal (location_t loc, tree type, tree init, bool non_const)
>    DECL_CONTEXT (decl) = current_function_decl;
>    TREE_USED (decl) = 1;
>    DECL_READ_P (decl) = 1;
> +  DECL_ARTIFICIAL (decl) = 1;
>    TREE_TYPE (decl) = type;
>    TREE_READONLY (decl) = (TYPE_READONLY (type)
>  			  || (TREE_CODE (type) == ARRAY_TYPE
> @@ -5297,7 +5298,6 @@ build_compound_literal (location_t loc, tree type, tree init, bool non_const)
>        set_compound_literal_name (decl);
>        DECL_DEFER_OUTPUT (decl) = 1;
>        DECL_COMDAT (decl) = 1;
> -      DECL_ARTIFICIAL (decl) = 1;
>        DECL_IGNORED_P (decl) = 1;
>        pushdecl (decl);
>        rest_of_decl_compilation (decl, 1, 0);
> diff --git gcc/testsuite/gcc.dg/asan/pr80659.c gcc/testsuite/gcc.dg/asan/pr80659.c
> index e69de29..0cbf2e4 100644
> --- gcc/testsuite/gcc.dg/asan/pr80659.c
> +++ gcc/testsuite/gcc.dg/asan/pr80659.c
> @@ -0,0 +1,13 @@
> +/* PR sanitizer/80659 */
> +/* { dg-do compile } */
> +
> +void
> +foo (int a)
> +{
> +  switch (a)
> +    {
> +    case 0:
> +      (int[3]) { };
> +      int h;
> +    }
> +}
> 
> 	Marek
> 
>
diff mbox

Patch

diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index b779d37..887e95d 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -5261,6 +5261,7 @@  build_compound_literal (location_t loc, tree type, tree init, bool non_const)
   DECL_CONTEXT (decl) = current_function_decl;
   TREE_USED (decl) = 1;
   DECL_READ_P (decl) = 1;
+  DECL_ARTIFICIAL (decl) = 1;
   TREE_TYPE (decl) = type;
   TREE_READONLY (decl) = (TYPE_READONLY (type)
 			  || (TREE_CODE (type) == ARRAY_TYPE
@@ -5297,7 +5298,6 @@  build_compound_literal (location_t loc, tree type, tree init, bool non_const)
       set_compound_literal_name (decl);
       DECL_DEFER_OUTPUT (decl) = 1;
       DECL_COMDAT (decl) = 1;
-      DECL_ARTIFICIAL (decl) = 1;
       DECL_IGNORED_P (decl) = 1;
       pushdecl (decl);
       rest_of_decl_compilation (decl, 1, 0);
diff --git gcc/testsuite/gcc.dg/asan/pr80659.c gcc/testsuite/gcc.dg/asan/pr80659.c
index e69de29..0cbf2e4 100644
--- gcc/testsuite/gcc.dg/asan/pr80659.c
+++ gcc/testsuite/gcc.dg/asan/pr80659.c
@@ -0,0 +1,13 @@ 
+/* PR sanitizer/80659 */
+/* { dg-do compile } */
+
+void
+foo (int a)
+{
+  switch (a)
+    {
+    case 0:
+      (int[3]) { };
+      int h;
+    }
+}