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 <pola...@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..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'" } Marek