From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26888 invoked by alias); 19 Mar 2015 08:30:01 -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 26842 invoked by uid 89); 19 Mar 2015 08:30:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=3.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KAM_FROM_URIBL_PCCC,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-wi0-f173.google.com Received: from mail-wi0-f173.google.com (HELO mail-wi0-f173.google.com) (209.85.212.173) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 19 Mar 2015 08:29:58 +0000 Received: by wixw10 with SMTP id w10so62159481wix.0 for ; Thu, 19 Mar 2015 01:29:55 -0700 (PDT) X-Received: by 10.181.27.199 with SMTP id ji7mr13860155wid.76.1426753795644; Thu, 19 Mar 2015 01:29:55 -0700 (PDT) Received: from msticlxl57.ims.intel.com ([192.55.54.40]) by mx.google.com with ESMTPSA id m4sm941684wjb.25.2015.03.19.01.29.52 for (version=TLSv1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 19 Mar 2015 01:29:54 -0700 (PDT) Date: Thu, 19 Mar 2015 08:30:00 -0000 From: Ilya Enkovich To: gcc-patches@gcc.gnu.org Subject: Re: [CHKP, PATCH] Fix instrumented indirect calls with propagated pointers Message-ID: <20150319082944.GC64546@msticlxl57.ims.intel.com> References: <20150312100931.GK27860@msticlxl57.ims.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150312100931.GK27860@msticlxl57.ims.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00991.txt.bz2 On 12 Mar 13:09, Ilya Enkovich wrote: > Hi, > > Instrumented function pointer may be propagated into not instrumented indirect call and vice versa. It requires additional call modifications (either remove bounds or change callee). Bootstrapped and tested on x86_64-unknown-linux-gnu. OK for trunk? > > Here is an updated version where we don't remove bounds args in case all args are to be removed anyway. Otherwise new statement is created and EH is not cleaned for it in redirect_all_calls. New test is added. Bootstrapped and tested on x86_64-unknown-linux-gnu. OK for trunk? Thanks, Ilya -- gcc/ 2015-03-19 Ilya Enkovich * cgraph.c (cgraph_edge::redirect_call_stmt_to_callee): Add redirection for instrumented calls. * tree-chkp.h (chkp_copy_call_skip_bounds): New. (chkp_redirect_edge): New. * tree-chkp.c (chkp_copy_call_skip_bounds): New. (chkp_redirect_edge): New. gcc/testsuite/ 2015-03-19 Ilya Enkovich * gcc.target/i386/mpx/chkp-fix-calls-1.c: New. * gcc.target/i386/mpx/chkp-fix-calls-2.c: New. * gcc.target/i386/mpx/chkp-fix-calls-3.c: New. diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 5ca1901..a0b0465 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1278,14 +1278,25 @@ cgraph_edge::redirect_call_stmt_to_callee (void) { cgraph_edge *e = this; - tree decl = gimple_call_fndecl (e->call_stmt); - tree lhs = gimple_call_lhs (e->call_stmt); + tree decl; + tree lhs; gcall *new_stmt; gimple_stmt_iterator gsi; + bool skip_bounds = false; #ifdef ENABLE_CHECKING cgraph_node *node; #endif + /* We might propagate instrumented function pointer into + not instrumented function and vice versa. In such a + case we need to either fix function declaration or + remove bounds from call statement. */ + if (callee) + skip_bounds = chkp_redirect_edge (e); + + decl = gimple_call_fndecl (e->call_stmt); + lhs = gimple_call_lhs (e->call_stmt); + if (e->speculative) { cgraph_edge *e2; @@ -1391,7 +1402,8 @@ cgraph_edge::redirect_call_stmt_to_callee (void) } if (e->indirect_unknown_callee - || decl == e->callee->decl) + || (decl == e->callee->decl + && !skip_bounds)) return e->call_stmt; #ifdef ENABLE_CHECKING @@ -1416,13 +1428,19 @@ cgraph_edge::redirect_call_stmt_to_callee (void) } } - if (e->callee->clone.combined_args_to_skip) + if (e->callee->clone.combined_args_to_skip + || skip_bounds) { int lp_nr; - new_stmt - = gimple_call_copy_skip_args (e->call_stmt, - e->callee->clone.combined_args_to_skip); + new_stmt = e->call_stmt; + if (e->callee->clone.combined_args_to_skip) + new_stmt + = gimple_call_copy_skip_args (new_stmt, + e->callee->clone.combined_args_to_skip); + if (skip_bounds) + new_stmt = chkp_copy_call_skip_bounds (new_stmt); + gimple_call_set_fndecl (new_stmt, e->callee->decl); gimple_call_set_fntype (new_stmt, gimple_call_fntype (e->call_stmt)); diff --git a/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-1.c b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-1.c new file mode 100644 index 0000000..cb4d229 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-1.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fcheck-pointer-bounds -mmpx" } */ + +#include "math.h" + +double +test1 (double x, double y, double (*fn)(double, double)) +{ + return fn (x, y); +} + +double +test2 (double x, double y) +{ + return test1 (x, y, copysign); +} diff --git a/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-2.c b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-2.c new file mode 100644 index 0000000..951e7de --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-2.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-O3 -fcheck-pointer-bounds -mmpx -fno-inline" } */ + +#include "math.h" + +double +test1 (double x, double y, double (*fn)(double, double)) +{ + return fn (x, y); +} + +double +test2 (double x, double y) +{ + return test1 (x, y, copysign); +} diff --git a/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-3.c b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-3.c new file mode 100755 index 0000000..439f631 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-3.c @@ -0,0 +1,33 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fexceptions -fcheck-pointer-bounds -mmpx" } */ + +extern int f2 (const char*, int, ...); +extern long int f3 (int *); +extern void err (void) __attribute__((__error__("error"))); + +extern __inline __attribute__ ((__always_inline__)) int +f1 (int i, ...) +{ + if (__builtin_constant_p (i)) + { + if (i) + err (); + return f2 ("", i); + } + + return f2 ("", i); +} + +int +test () +{ + int i; + + if (f1 (0)) + if (f3 (&i)) + i = 0; + + return i; +} + + diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c index d2df4ba..b9508d8 100644 --- a/gcc/tree-chkp.c +++ b/gcc/tree-chkp.c @@ -500,6 +500,71 @@ chkp_expand_bounds_reset_for_mem (tree mem, tree ptr) expand_normal (bndstx); } +/* Build a GIMPLE_CALL identical to CALL but skipping bounds + arguments. */ + +gcall * +chkp_copy_call_skip_bounds (gcall *call) +{ + bitmap bounds; + unsigned i; + + bitmap_obstack_initialize (NULL); + bounds = BITMAP_ALLOC (NULL); + + for (i = 0; i < gimple_call_num_args (call); i++) + if (POINTER_BOUNDS_P (gimple_call_arg (call, i))) + bitmap_set_bit (bounds, i); + + if (bitmap_empty_p (bounds)) + return call; + + call = gimple_call_copy_skip_args (call, bounds); + gimple_call_set_with_bounds (call, false); + + BITMAP_FREE (bounds); + bitmap_obstack_release (NULL); + + return call; +} + +/* Redirect edge E to the correct node according to call_stmt. + Return 1 if bounds removal from call_stmt should be done + instead of redirection. */ + +bool +chkp_redirect_edge (cgraph_edge *e) +{ + bool instrumented = false; + tree decl = e->callee->decl; + + if (e->callee->instrumentation_clone + || chkp_function_instrumented_p (decl)) + instrumented = true; + + if (instrumented + && !gimple_call_with_bounds_p (e->call_stmt)) + e->redirect_callee (cgraph_node::get_create (e->callee->orig_decl)); + else if (!instrumented + && !chkp_gimple_call_builtin_p (e->call_stmt, BUILT_IN_CHKP_BNDCL) + && !chkp_gimple_call_builtin_p (e->call_stmt, BUILT_IN_CHKP_BNDCU) + && !chkp_gimple_call_builtin_p (e->call_stmt, BUILT_IN_CHKP_BNDSTX) + && gimple_call_with_bounds_p (e->call_stmt)) + { + if (e->callee->instrumented_version) + e->redirect_callee (e->callee->instrumented_version); + else + { + tree args = TYPE_ARG_TYPES (TREE_TYPE (decl)); + /* Avoid bounds removal if all args will be removed. */ + if (!args || TREE_VALUE (args) != void_type_node) + return true; + } + } + + return false; +} + /* Mark statement S to not be instrumented. */ static void chkp_mark_stmt (gimple s) diff --git a/gcc/tree-chkp.h b/gcc/tree-chkp.h index 86f3618..40e2489 100644 --- a/gcc/tree-chkp.h +++ b/gcc/tree-chkp.h @@ -54,5 +54,7 @@ extern void chkp_copy_bounds_for_assign (gimple assign, extern bool chkp_gimple_call_builtin_p (gimple call, enum built_in_function code); extern void chkp_expand_bounds_reset_for_mem (tree mem, tree ptr); +extern gcall *chkp_copy_call_skip_bounds (gcall *call); +extern bool chkp_redirect_edge (cgraph_edge *e); #endif /* GCC_TREE_CHKP_H */