[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?

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.

> 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;