From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 117501 invoked by alias); 2 Apr 2015 20:45:17 -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 117488 invoked by uid 89); 2 Apr 2015 20:45:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.6 required=5.0 tests=AWL,BAYES_50,KAM_LAZY_DOMAIN_SECURITY,KAM_STOCKGEN,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: atrey.karlin.mff.cuni.cz Received: from atrey.karlin.mff.cuni.cz (HELO atrey.karlin.mff.cuni.cz) (195.113.26.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 02 Apr 2015 20:45:15 +0000 Received: by atrey.karlin.mff.cuni.cz (Postfix, from userid 4018) id 611BB81EFA; Thu, 2 Apr 2015 22:45:13 +0200 (CEST) Date: Thu, 02 Apr 2015 20:45:00 -0000 From: Jan Hubicka To: Ilya Enkovich Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH, CHKP] Fix static const bounds creation in LTO Message-ID: <20150402204513.GI21276@atrey.karlin.mff.cuni.cz> References: <20150402152154.GC6244@msticlxl57.ims.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150402152154.GC6244@msticlxl57.ims.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2015-04/txt/msg00088.txt.bz2 > Hi, > > Current in LTO static const bounds are created as external symbol. It doesn't work in case original symbols were removed and privatized. This patch introduces a separate comdat symbol to be used in LTO. Bootstrapped and tested on x86_64-unknown-linux-gnu. Does this approach look OK? As I understand it, you want to create a static variables, like __chkp_zero_bounds which should be shared across translation units. Currently the function does: static tree chkp_make_static_const_bounds (HOST_WIDE_INT lb, HOST_WIDE_INT ub, const char *name) { tree var; /* With LTO we may have constant bounds already in varpool. Try to find it. */ var = chkp_find_const_bounds_var (lb, ub); if (var) return var; var = build_decl (UNKNOWN_LOCATION, VAR_DECL, get_identifier (name), pointer_bounds_type_node); TREE_PUBLIC (var) = 1; TREE_USED (var) = 1; TREE_READONLY (var) = 1; TREE_STATIC (var) = 1; TREE_ADDRESSABLE (var) = 0; DECL_ARTIFICIAL (var) = 1; DECL_READ_P (var) = 1; /* We may use this symbol during ctors generation in chkp_finish_file when all symbols are emitted. Force output to avoid undefined symbols in ctors. */ if (!in_lto_p) { DECL_INITIAL (var) = targetm.chkp_make_bounds_constant (lb, ub); DECL_COMDAT (var) = 1; varpool_node::get_create (var)->set_comdat_group (DECL_ASSEMBLER_NAME (var)); varpool_node::get_create (var)->force_output = 1; } else DECL_EXTERNAL (var) = 1; varpool_node::finalize_decl (var); } You should not set comdat group by hand, or we get into troubles on non-ELF targets. Just call make_decl_one_only (var, DECL_ASSEMBLER_NAME (var)); Now in LTO I guess you want to check if the symbol is provided by the source compilation unit, i.e. call symtab_node::get_for_asmname (DECL_ASSEMBLER_NAME (var)) and if it is there, check it have proper type and fatal_error otherwise and if it does, discar var and use existing variable This avoid a duplicate declaration of a symbol that is invalid in symtab. (once we solve the transparent alias thing, I will add verification for that) Because you already set force_output, the symbol should not be privatized and renamed in any way. If there are cases the symbol can get legally optimized out, I guess you can also avoid setting force_output and we can stream references to the decls into optimization summary in a case we want ot bind for it in WPA, but that can wait for next stage1. Honza > > Thanks, > Ilya > -- > gcc/ > > 2015-04-02 Ilya Enkovich > > * tree-chkp.c (CHKP_LTO_SYMBOL_SUFFIX): New. > (chkp_make_static_const_bounds): Use another > symbol in LTO. > > gcc/testsuite/ > > 2015-04-02 Ilya Enkovich > > * gcc.dg/lto/chkp-static-bounds_0.c: New. > > > diff --git a/gcc/testsuite/gcc.dg/lto/chkp-static-bounds_0.c b/gcc/testsuite/gcc.dg/lto/chkp-static-bounds_0.c > new file mode 100644 > index 0000000..e896eb1 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/lto/chkp-static-bounds_0.c > @@ -0,0 +1,26 @@ > +/* { dg-lto-do link } */ > +/* { dg-require-effective-target mpx } */ > +/* { dg-lto-options { { -flto -flto-partition=max -fcheck-pointer-bounds -mmpx } } } */ > + > +char *cc; > + > +int test1 (const char *c) > +{ > + c = __builtin___bnd_init_ptr_bounds (c); > + cc = c; > + return c[0] * 2; > +} > + > +struct S > +{ > + int (*fnptr) (const char *); > +} S; > + > +struct S s1 = {test1}; > +struct S s2 = {test1}; > +struct S s3 = {test1}; > + > +int main (int argc, const char **argv) > +{ > + return s1.fnptr (argv[0]) + s2.fnptr (argv[1]); > +} > diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c > index d2df4ba..0578936 100644 > --- a/gcc/tree-chkp.c > +++ b/gcc/tree-chkp.c > @@ -421,6 +421,7 @@ static bool in_chkp_pass; > #define CHKP_VAR_BOUNDS_PREFIX "__chkp_var_bounds_" > #define CHKP_ZERO_BOUNDS_VAR_NAME "__chkp_zero_bounds" > #define CHKP_NONE_BOUNDS_VAR_NAME "__chkp_none_bounds" > +#define CHKP_LTO_SYMBOL_SUFFIX ".lto" > > /* Static checker constructors may become very large and their > compilation with optimization may take too much time. > @@ -1906,7 +1907,8 @@ chkp_make_static_const_bounds (HOST_WIDE_INT lb, > HOST_WIDE_INT ub, > const char *name) > { > - tree var; > + tree var, id; > + varpool_node *node; > > /* With LTO we may have constant bounds already in varpool. > Try to find it. */ > @@ -1915,8 +1917,22 @@ chkp_make_static_const_bounds (HOST_WIDE_INT lb, > if (var) > return var; > > - var = build_decl (UNKNOWN_LOCATION, VAR_DECL, > - get_identifier (name), pointer_bounds_type_node); > + /* In LTO we may have symbol with changed visibility, comdat > + group etc. Therefore we shouldn't recreate the same symbol. > + Use LTO version instead. */ > + if (in_lto_p) > + { > + int len = strlen (name) + strlen (CHKP_LTO_SYMBOL_SUFFIX) + 1; > + char *new_name = XALLOCAVEC (char, len); > + strcpy (new_name, name); > + strcat (new_name, CHKP_LTO_SYMBOL_SUFFIX); > + id = get_identifier (new_name); > + } > + else > + id = get_identifier (name); > + > + var = build_decl (UNKNOWN_LOCATION, VAR_DECL, id, > + pointer_bounds_type_node); > > TREE_PUBLIC (var) = 1; > TREE_USED (var) = 1; > @@ -1925,18 +1941,17 @@ chkp_make_static_const_bounds (HOST_WIDE_INT lb, > TREE_ADDRESSABLE (var) = 0; > DECL_ARTIFICIAL (var) = 1; > DECL_READ_P (var) = 1; > + DECL_INITIAL (var) = targetm.chkp_make_bounds_constant (lb, ub); > + DECL_COMDAT (var) = 1; > + DECL_WEAK (var) = 1; > /* We may use this symbol during ctors generation in chkp_finish_file > when all symbols are emitted. Force output to avoid undefined > symbols in ctors. */ > - if (!in_lto_p) > - { > - DECL_INITIAL (var) = targetm.chkp_make_bounds_constant (lb, ub); > - DECL_COMDAT (var) = 1; > - varpool_node::get_create (var)->set_comdat_group (DECL_ASSEMBLER_NAME (var)); > - varpool_node::get_create (var)->force_output = 1; > - } > - else > - DECL_EXTERNAL (var) = 1; > + node = varpool_node::get_create (var); > + node->set_comdat_group (DECL_ASSEMBLER_NAME (var)); > + node->force_output = 1; > + node->externally_visible = 1; > + > varpool_node::finalize_decl (var); > > return var;