Re: [PATCH] Don't instrument DECL_INITIAL of statics (PR sanitizer/66190)

[Marek Polacek <polacek at redhat dot com>]

Ping.

On Thu, May 21, 2015 at 09:36:59PM +0200, Marek Polacek wrote:
> In this PR, we find ourselves instrumenting a static initializer and
> then crashing when expanding an unlowered UBSAN_NULL.  Jakub suggests
> to not instrument DECL_INITIAL of a static variable.  The following
> patch is an attempt to do that.  Note that we're still able to sanitize
> similar cases (they don't have DECL_INITIAL but something else).
> 
> Bootstrap/regtest/bootstrap-ubsan passed on x86_64-linux, ok for trunk?
> 
> 2015-05-21  Marek Polacek  <polacek@redhat.com>
> 
> 	PR sanitizer/66190
> 	* cp-gimplify.c (struct cp_genericize_data): Add no_sanitize_p.
> 	(cp_genericize_r): Don't instrument static initializers.
> 	(cp_genericize_tree): Initialize wtd.no_sanitize_p.
> 
> 	* g++.dg/ubsan/static-init-1.C: New test.
> 	* g++.dg/ubsan/static-init-2.C: New test.
> 	* g++.dg/ubsan/static-init-3.C: New test.
> 
> diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c
> index d5a64fc..778d8f3 100644
> --- gcc/cp/cp-gimplify.c
> +++ gcc/cp/cp-gimplify.c
> @@ -906,6 +906,7 @@ struct cp_genericize_data
>    vec<tree> bind_expr_stack;
>    struct cp_genericize_omp_taskreg *omp_ctx;
>    tree try_block;
> +  bool no_sanitize_p;
>  };
>  
>  /* Perform any pre-gimplification lowering of C++ front end trees to
> @@ -1150,6 +1151,21 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void *data)
>        *stmt_p = build1 (NOP_EXPR, void_type_node, integer_zero_node);
>        *walk_subtrees = 0;
>      }
> +  else if ((flag_sanitize
> +	    & (SANITIZE_NULL | SANITIZE_ALIGNMENT | SANITIZE_VPTR))
> +	   && TREE_CODE (stmt) == DECL_EXPR
> +	   && VAR_P (DECL_EXPR_DECL (stmt))
> +	   && TREE_STATIC (DECL_EXPR_DECL (stmt))
> +	   && DECL_INITIAL (DECL_EXPR_DECL (stmt)))
> +    {
> +      *walk_subtrees = 0;
> +      /* The point here is to not sanitize static initializers.  */
> +      bool no_sanitize_p = wtd->no_sanitize_p;
> +      wtd->no_sanitize_p = true;
> +      cp_walk_tree (&DECL_INITIAL (DECL_EXPR_DECL (stmt)), cp_genericize_r,
> +		    data, NULL);
> +      wtd->no_sanitize_p = no_sanitize_p;
> +    }
>    else if (TREE_CODE (stmt) == OMP_PARALLEL || TREE_CODE (stmt) == OMP_TASK)
>      {
>        struct cp_genericize_omp_taskreg omp_ctx;
> @@ -1275,9 +1291,10 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void *data)
>        if (*stmt_p == error_mark_node)
>  	*stmt_p = size_one_node;
>        return NULL;
> -    }    
> -  else if (flag_sanitize
> -	   & (SANITIZE_NULL | SANITIZE_ALIGNMENT | SANITIZE_VPTR))
> +    }
> +  else if ((flag_sanitize
> +	    & (SANITIZE_NULL | SANITIZE_ALIGNMENT | SANITIZE_VPTR))
> +	   && !wtd->no_sanitize_p)
>      {
>        if ((flag_sanitize & (SANITIZE_NULL | SANITIZE_ALIGNMENT))
>  	  && TREE_CODE (stmt) == NOP_EXPR
> @@ -1319,6 +1336,7 @@ cp_genericize_tree (tree* t_p)
>    wtd.bind_expr_stack.create (0);
>    wtd.omp_ctx = NULL;
>    wtd.try_block = NULL_TREE;
> +  wtd.no_sanitize_p = false;
>    cp_walk_tree (t_p, cp_genericize_r, &wtd, NULL);
>    delete wtd.p_set;
>    wtd.bind_expr_stack.release ();
> diff --git gcc/testsuite/g++.dg/ubsan/static-init-1.C gcc/testsuite/g++.dg/ubsan/static-init-1.C
> index e69de29..0b424c0 100644
> --- gcc/testsuite/g++.dg/ubsan/static-init-1.C
> +++ gcc/testsuite/g++.dg/ubsan/static-init-1.C
> @@ -0,0 +1,21 @@
> +// PR sanitizer/66190
> +// { dg-do compile }
> +// { dg-options "-fsanitize=null -std=c++11" }
> +
> +class A {
> +public:
> +  void fn1 (int);
> +};
> +
> +class G {
> +  ~G ();
> +  A t;
> +  virtual void fn2 () {
> +    static int a;
> +    static int &b = a;
> +    static int &c (a);
> +    static int &d {a};
> +    t.fn1 (b);
> +  }
> +};
> +G ::~G () {}
> diff --git gcc/testsuite/g++.dg/ubsan/static-init-2.C gcc/testsuite/g++.dg/ubsan/static-init-2.C
> index e69de29..d046b33 100644
> --- gcc/testsuite/g++.dg/ubsan/static-init-2.C
> +++ gcc/testsuite/g++.dg/ubsan/static-init-2.C
> @@ -0,0 +1,17 @@
> +// PR sanitizer/66190
> +// { dg-do run }
> +// { dg-options "-fsanitize=null -std=c++11" }
> +
> +int
> +main ()
> +{
> +  static int *a;
> +  static int &b = *a;
> +  static int &c (*a);
> +  static int &d {*a};
> +  return 0;
> +}
> +
> +// { dg-output "reference binding to null pointer of type 'int'(\n|\r\n|\r)" }
> +// { dg-output "\[^\n\r]*reference binding to null pointer of type 'int'(\n|\r\n|\r)" }
> +// { dg-output "\[^\n\r]*reference binding to null pointer of type 'int'" }
> diff --git gcc/testsuite/g++.dg/ubsan/static-init-3.C gcc/testsuite/g++.dg/ubsan/static-init-3.C
> index e69de29..7fd6cbd 100644
> --- gcc/testsuite/g++.dg/ubsan/static-init-3.C
> +++ gcc/testsuite/g++.dg/ubsan/static-init-3.C
> @@ -0,0 +1,19 @@
> +// PR sanitizer/66190
> +// { dg-do run }
> +// { dg-options "-fsanitize=null -std=c++11" }
> +
> +int *fn (void) { return 0; }
> +
> +int
> +main ()
> +{
> +  static int a;
> +  static int &b = *fn ();
> +  static int &c (*fn ());
> +  static int &d {*fn ()};
> +  return 0;
> +}
> +
> +// { dg-output "reference binding to null pointer of type 'int'(\n|\r\n|\r)" }
> +// { dg-output "\[^\n\r]*reference binding to null pointer of type 'int'(\n|\r\n|\r)" }
> +// { dg-output "\[^\n\r]*reference binding to null pointer of type 'int'" }
> 
> 	Marek

	Marek