diff mbox

Don't instrument DECL_INITIAL of statics (PR sanitizer/66190)

Message ID 20150603163201.GH2756@redhat.com
State New
Headers show

Commit Message

Marek Polacek June 3, 2015, 4:32 p.m. UTC 
On Fri, May 29, 2015 at 12:26:39PM +0200, Jakub Jelinek wrote:
> This seems strange.  Normally DECL_INITIAL of vars isn't walked when
> processing DECL_EXPRs, so IMHO you shouldn't either.
> I think it would be much better to handle this case where the tree.c
> code handles it, thus in cp_genericize_r's BIND_EXPR handling.
> Just do there something along the lines:
>   if (flag_sanitize
>       & (SANITIZE_NULL | SANITIZE_ALIGNMENT | SANITIZE_VPTR))
>     {
>       bool no_sanitize_p = wtd->no_sanitize_p;
>       wtd->no_sanitize_p = true;
>       for (tree decl = BIND_EXPR_VARS (*tp); decl; decl = DECL_CHAIN (decl))
> 	if (VAR_P (decl)
> 	    && TREE_STATIC (decl)
> 	    && DECL_INITIAL (decl))
> 	  cp_walk_tree (&DECL_INITIAL (decl), cp_genericize_r, data, NULL);
>       wtd->no_sanitize_p = no_sanitize_p;
>     }
> with some appripriate comments.  As cp_genericize_r gives up early for
> expressions it has walked already, this should DTRT then.

All right, that seems to work well.  Done in the below.

Bootstrap-ubsaned/regtested on x86_64-linux, ok for trunk?

2015-06-03  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.


	Marek

Comments

Jakub Jelinek June 3, 2015, 4:33 p.m. UTC | #1 
On Wed, Jun 03, 2015 at 06:32:01PM +0200, Marek Polacek wrote:
> All right, that seems to work well.  Done in the below.
> 
> Bootstrap-ubsaned/regtested on x86_64-linux, ok for trunk?

Ok, thanks.

> 2015-06-03  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.

	Jakub
Marek Polacek June 3, 2015, 4:59 p.m. UTC | #2 
On Wed, Jun 03, 2015 at 06:33:20PM +0200, Jakub Jelinek wrote:
> Ok, thanks.

Forgot to ask - can I also backport the fix to gcc-5 branch?

	Marek
Jakub Jelinek June 3, 2015, 5 p.m. UTC | #3 
On Wed, Jun 03, 2015 at 06:59:55PM +0200, Marek Polacek wrote:
> On Wed, Jun 03, 2015 at 06:33:20PM +0200, Jakub Jelinek wrote:
> > Ok, thanks.
> 
> Forgot to ask - can I also backport the fix to gcc-5 branch?

Sure.

	Jakub
diff mbox

Patch

diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c
index d5a64fc..69fd53b 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
@@ -1105,6 +1106,21 @@  cp_genericize_r (tree *stmt_p, int *walk_subtrees, void *data)
 				     : OMP_CLAUSE_DEFAULT_PRIVATE);
 	      }
 	}
+      if (flag_sanitize
+	  & (SANITIZE_NULL | SANITIZE_ALIGNMENT | SANITIZE_VPTR))
+	{
+	  /* The point here is to not sanitize static initializers.  */
+	  bool no_sanitize_p = wtd->no_sanitize_p;
+	  wtd->no_sanitize_p = true;
+	  for (tree decl = BIND_EXPR_VARS (stmt);
+	       decl;
+	       decl = DECL_CHAIN (decl))
+	    if (VAR_P (decl)
+		&& TREE_STATIC (decl)
+		&& DECL_INITIAL (decl))
+	      cp_walk_tree (&DECL_INITIAL (decl), cp_genericize_r, data, NULL);
+	  wtd->no_sanitize_p = no_sanitize_p;
+	}
       wtd->bind_expr_stack.safe_push (stmt);
       cp_walk_tree (&BIND_EXPR_BODY (stmt),
 		    cp_genericize_r, data, NULL);
@@ -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..36c6007 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'" }