This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, CHKP] Fix static const bounds creation in LTO


> 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  <ilya.enkovich@intel.com>
> 
> 	* 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  <ilya.enkovich@intel.com>
> 
> 	* 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;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]