From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5687 invoked by alias); 3 Jun 2015 16:32:14 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 5672 invoked by uid 89); 3 Jun 2015 16:32:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.2 required=5.0 tests=AWL,BAYES_05,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD,UNWANTED_LANGUAGE_BODY autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 03 Jun 2015 16:32:13 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 34BDE374A2C for ; Wed, 3 Jun 2015 16:32:12 +0000 (UTC) Received: from redhat.com (ovpn-204-24.brq.redhat.com [10.40.204.24]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t53GW3r4001521 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NO); Wed, 3 Jun 2015 12:32:09 -0400 Date: Wed, 03 Jun 2015 16:33:00 -0000 From: Marek Polacek To: Jakub Jelinek Cc: GCC Patches Subject: Re: [PATCH] Don't instrument DECL_INITIAL of statics (PR sanitizer/66190) Message-ID: <20150603163201.GH2756@redhat.com> References: <20150521193658.GQ27320@redhat.com> <20150529084134.GG27320@redhat.com> <20150529102639.GW10247@tucnak.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150529102639.GW10247@tucnak.redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2015-06/txt/msg00343.txt.bz2 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 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 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