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

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

[Ilya Enkovich]

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.


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;

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

[Jan Hubicka]

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

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

[Ilya Enkovich]

On 02 Apr 22:45, Jan Hubicka wrote:
> > 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 a lot for looking into it!  Seems originally I mostly got problems due to no DECL_WEAK for generated var, make_decl_one_only seems to work for my tests.  Will run more testing to make sure it's OK.  Here is a new patch version.  Is it OK?

Thanks,
Ilya
--
gcc/

2015-04-03  Ilya Enkovich  <ilya.enkovich@intel.com>

	* tree-chkp.c (chkp_find_const_bounds_var): Search
	by assembler name.
	(chkp_make_static_const_bounds): Use make_decl_one_only.

gcc/testsuite/

2015-04-03  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..596e551
--- /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 } } } */
+
+const 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 03f75b3..4daeab6 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -1876,26 +1876,17 @@ chkp_add_bounds_to_call_stmt (gimple_stmt_iterator *gsi)
 /* Return constant static bounds var with specified LB and UB
    if such var exists in varpool.  Return NULL otherwise.  */
 static tree
-chkp_find_const_bounds_var (HOST_WIDE_INT lb,
-			    HOST_WIDE_INT ub)
+chkp_find_const_bounds_var (tree id)
 {
-  tree val = targetm.chkp_make_bounds_constant (lb, ub);
-  struct varpool_node *node;
-
-  /* We expect bounds constant is represented as a complex value
-     of two pointer sized integers.  */
-  gcc_assert (TREE_CODE (val) == COMPLEX_CST);
+  struct symtab_node *node = symtab_node::get_for_asmname (id);
 
-  FOR_EACH_VARIABLE (node)
-    if (POINTER_BOUNDS_P (node->decl)
-	&& TREE_READONLY (node->decl)
-	&& DECL_INITIAL (node->decl)
-	&& TREE_CODE (DECL_INITIAL (node->decl)) == COMPLEX_CST
-	&& tree_int_cst_equal (TREE_REALPART (DECL_INITIAL (node->decl)),
-			       TREE_REALPART (val))
-	&& tree_int_cst_equal (TREE_IMAGPART (DECL_INITIAL (node->decl)),
-			       TREE_IMAGPART (val)))
+  if (node)
+    {
+      /* We don't allow this symbol usage for non bounds.  */
+      gcc_assert (node->type == SYMTAB_VARIABLE);
+      gcc_assert (POINTER_BOUNDS_P (node->decl));
       return node->decl;
+    }
 
   return NULL;
 }
@@ -1907,37 +1898,34 @@ chkp_make_static_const_bounds (HOST_WIDE_INT lb,
 			       HOST_WIDE_INT ub,
 			       const char *name)
 {
+  tree id = get_identifier (name);
   tree var;
+  varpool_node *node;
 
   /* With LTO we may have constant bounds already in varpool.
      Try to find it.  */
-  var = chkp_find_const_bounds_var (lb, ub);
+  var = chkp_find_const_bounds_var (id);
 
   if (var)
     return var;
 
-  var  = build_decl (UNKNOWN_LOCATION, VAR_DECL,
-		     get_identifier (name), pointer_bounds_type_node);
+  var  = build_decl (UNKNOWN_LOCATION, VAR_DECL, id,
+		     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;
+  DECL_INITIAL (var) = targetm.chkp_make_bounds_constant (lb, ub);
+  make_decl_one_only (var, DECL_ASSEMBLER_NAME (var));
   /* 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->force_output = 1;
+
   varpool_node::finalize_decl (var);
 
   return var;

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

[Jan Hubicka]

> 
> Thanks a lot for looking into it!  Seems originally I mostly got problems due to no DECL_WEAK for generated var, make_decl_one_only seems to work for my tests.  Will run more testing to make sure it's OK.  Here is a new patch version.  Is it OK?

Good ;)
> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
> index 03f75b3..4daeab6 100644
> --- a/gcc/tree-chkp.c
> +++ b/gcc/tree-chkp.c
> @@ -1876,26 +1876,17 @@ chkp_add_bounds_to_call_stmt (gimple_stmt_iterator *gsi)
>  /* Return constant static bounds var with specified LB and UB
>     if such var exists in varpool.  Return NULL otherwise.  */
>  static tree
> -chkp_find_const_bounds_var (HOST_WIDE_INT lb,
> -			    HOST_WIDE_INT ub)
> +chkp_find_const_bounds_var (tree id)
>  {
> -  tree val = targetm.chkp_make_bounds_constant (lb, ub);
> -  struct varpool_node *node;
> -
> -  /* We expect bounds constant is represented as a complex value
> -     of two pointer sized integers.  */
> -  gcc_assert (TREE_CODE (val) == COMPLEX_CST);
> +  struct symtab_node *node = symtab_node::get_for_asmname (id);

Sadly I think this won't work on targets that adds prefix to assembler name,
like djgpp/mingw.
To obtain mangled assembler name you need to call
id = targetm.mangle_decl_assembler_name (decl, DECL_NAME (decl));
that in turn requires decl to already exist.  That is why I guess you may want
to just build the new var and see if already exists....

Honza
>  
> -  FOR_EACH_VARIABLE (node)
> -    if (POINTER_BOUNDS_P (node->decl)
> -	&& TREE_READONLY (node->decl)
> -	&& DECL_INITIAL (node->decl)
> -	&& TREE_CODE (DECL_INITIAL (node->decl)) == COMPLEX_CST
> -	&& tree_int_cst_equal (TREE_REALPART (DECL_INITIAL (node->decl)),
> -			       TREE_REALPART (val))
> -	&& tree_int_cst_equal (TREE_IMAGPART (DECL_INITIAL (node->decl)),
> -			       TREE_IMAGPART (val)))
> +  if (node)
> +    {
> +      /* We don't allow this symbol usage for non bounds.  */
> +      gcc_assert (node->type == SYMTAB_VARIABLE);
> +      gcc_assert (POINTER_BOUNDS_P (node->decl));
>        return node->decl;
> +    }
>  
>    return NULL;
>  }
> @@ -1907,37 +1898,34 @@ chkp_make_static_const_bounds (HOST_WIDE_INT lb,
>  			       HOST_WIDE_INT ub,
>  			       const char *name)
>  {
> +  tree id = get_identifier (name);
>    tree var;
> +  varpool_node *node;
>  
>    /* With LTO we may have constant bounds already in varpool.
>       Try to find it.  */
> -  var = chkp_find_const_bounds_var (lb, ub);
> +  var = chkp_find_const_bounds_var (id);
>  
>    if (var)
>      return var;
>  
> -  var  = build_decl (UNKNOWN_LOCATION, VAR_DECL,
> -		     get_identifier (name), pointer_bounds_type_node);
> +  var  = build_decl (UNKNOWN_LOCATION, VAR_DECL, id,
> +		     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;
> +  DECL_INITIAL (var) = targetm.chkp_make_bounds_constant (lb, ub);
> +  make_decl_one_only (var, DECL_ASSEMBLER_NAME (var));
>    /* 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->force_output = 1;
> +
>    varpool_node::finalize_decl (var);
>  
>    return var;

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

[Ilya Enkovich]

On 03 Apr 19:01, Jan Hubicka wrote:
> 
> Sadly I think this won't work on targets that adds prefix to assembler name,
> like djgpp/mingw.
> To obtain mangled assembler name you need to call
> id = targetm.mangle_decl_assembler_name (decl, DECL_NAME (decl));
> that in turn requires decl to already exist.  That is why I guess you may want
> to just build the new var and see if already exists....
> 
> Honza

I see.  Thank you for review!  Here is an updated version.  Testing passed.  Does it look OK?

Thanks,
Ilya
--
gcc/

2015-04-07  Ilya Enkovich  <ilya.enkovich@intel.com>

	* tree-chkp.c (chkp_find_const_bounds_var): Remove.
	(chkp_make_static_const_bounds): Search existing
	symbol by assembler name.  Use make_decl_one_only.

gcc/testsuite/

2015-04-07  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..596e551
--- /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 } } } */
+
+const 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 03f75b3..f5accce 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -1873,33 +1873,6 @@ chkp_add_bounds_to_call_stmt (gimple_stmt_iterator *gsi)
   gimple_call_set_with_bounds (new_call, true);
 }
 
-/* Return constant static bounds var with specified LB and UB
-   if such var exists in varpool.  Return NULL otherwise.  */
-static tree
-chkp_find_const_bounds_var (HOST_WIDE_INT lb,
-			    HOST_WIDE_INT ub)
-{
-  tree val = targetm.chkp_make_bounds_constant (lb, ub);
-  struct varpool_node *node;
-
-  /* We expect bounds constant is represented as a complex value
-     of two pointer sized integers.  */
-  gcc_assert (TREE_CODE (val) == COMPLEX_CST);
-
-  FOR_EACH_VARIABLE (node)
-    if (POINTER_BOUNDS_P (node->decl)
-	&& TREE_READONLY (node->decl)
-	&& DECL_INITIAL (node->decl)
-	&& TREE_CODE (DECL_INITIAL (node->decl)) == COMPLEX_CST
-	&& tree_int_cst_equal (TREE_REALPART (DECL_INITIAL (node->decl)),
-			       TREE_REALPART (val))
-	&& tree_int_cst_equal (TREE_IMAGPART (DECL_INITIAL (node->decl)),
-			       TREE_IMAGPART (val)))
-      return node->decl;
-
-  return NULL;
-}
-
 /* Return constant static bounds var with specified bounds LB and UB.
    If such var does not exists then new var is created with specified NAME.  */
 static tree
@@ -1907,37 +1880,38 @@ chkp_make_static_const_bounds (HOST_WIDE_INT lb,
 			       HOST_WIDE_INT ub,
 			       const char *name)
 {
+  tree id = get_identifier (name);
   tree var;
+  varpool_node *node;
+  symtab_node *snode;
+
+  var  = build_decl (UNKNOWN_LOCATION, VAR_DECL, id,
+		     pointer_bounds_type_node);
+  TREE_STATIC (var) = 1;
 
   /* 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);
+  if ((snode = symtab_node::get_for_asmname (DECL_ASSEMBLER_NAME (var))))
+    {
+      /* We don't allow this symbol usage for non bounds.  */
+      gcc_assert (snode->type == SYMTAB_VARIABLE);
+      gcc_assert (POINTER_BOUNDS_P (snode->decl));
+      return snode->decl;
+    }
 
-  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;
+  DECL_INITIAL (var) = targetm.chkp_make_bounds_constant (lb, ub);
+  make_decl_one_only (var, DECL_ASSEMBLER_NAME (var));
   /* 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->force_output = 1;
+
   varpool_node::finalize_decl (var);
 
   return var;

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

[Jan Hubicka]

> 
> 2015-04-07  Ilya Enkovich  <ilya.enkovich@intel.com>
> 
> 	* tree-chkp.c (chkp_find_const_bounds_var): Remove.
> 	(chkp_make_static_const_bounds): Search existing
> 	symbol by assembler name.  Use make_decl_one_only.
> 
> gcc/testsuite/
> 
> 2015-04-07  Ilya Enkovich  <ilya.enkovich@intel.com>
> 
> 	* gcc.dg/lto/chkp-static-bounds_0.c: New.

OK, thanks!
> +  if ((snode = symtab_node::get_for_asmname (DECL_ASSEMBLER_NAME (var))))
> +    {
> +      /* We don't allow this symbol usage for non bounds.  */
> +      gcc_assert (snode->type == SYMTAB_VARIABLE);
> +      gcc_assert (POINTER_BOUNDS_P (snode->decl));
This probably allows users to trigger ICE by declaring function of conflicting
name.  What about sorry ("...") message instead?
> +      return snode->decl;
> +    }
>  
> -  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;
> +  DECL_INITIAL (var) = targetm.chkp_make_bounds_constant (lb, ub);
> +  make_decl_one_only (var, DECL_ASSEMBLER_NAME (var));
>    /* 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->force_output = 1;
> +
>    varpool_node::finalize_decl (var);
>  
>    return var;

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

[Ilya Enkovich]

On 07 Apr 22:43, Jan Hubicka wrote:
> > 
> > 2015-04-07  Ilya Enkovich  <ilya.enkovich@intel.com>
> > 
> > 	* tree-chkp.c (chkp_find_const_bounds_var): Remove.
> > 	(chkp_make_static_const_bounds): Search existing
> > 	symbol by assembler name.  Use make_decl_one_only.
> > 
> > gcc/testsuite/
> > 
> > 2015-04-07  Ilya Enkovich  <ilya.enkovich@intel.com>
> > 
> > 	* gcc.dg/lto/chkp-static-bounds_0.c: New.
> 
> OK, thanks!
> > +  if ((snode = symtab_node::get_for_asmname (DECL_ASSEMBLER_NAME (var))))
> > +    {
> > +      /* We don't allow this symbol usage for non bounds.  */
> > +      gcc_assert (snode->type == SYMTAB_VARIABLE);
> > +      gcc_assert (POINTER_BOUNDS_P (snode->decl));
> This probably allows users to trigger ICE by declaring function of conflicting
> name.  What about sorry ("...") message instead?

Here is installed version.

Thanks,
Ilya
--
gcc/

2015-04-08  Ilya Enkovich  <ilya.enkovich@intel.com>

	* tree-chkp.c (chkp_find_const_bounds_var): Remove.
	(chkp_make_static_const_bounds): Search existing
	symbol by assembler name.  Use make_decl_one_only.
	(chkp_get_zero_bounds_var): Remove node	search which
	is now performed in chkp_make_static_const_bounds.
	(chkp_get_none_bounds_var): Likewise.

gcc/testsuite/

2015-04-08  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..596e551
--- /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 } } } */
+
+const 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 03f75b3..4c8379f 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -1873,33 +1873,6 @@ chkp_add_bounds_to_call_stmt (gimple_stmt_iterator *gsi)
   gimple_call_set_with_bounds (new_call, true);
 }
 
-/* Return constant static bounds var with specified LB and UB
-   if such var exists in varpool.  Return NULL otherwise.  */
-static tree
-chkp_find_const_bounds_var (HOST_WIDE_INT lb,
-			    HOST_WIDE_INT ub)
-{
-  tree val = targetm.chkp_make_bounds_constant (lb, ub);
-  struct varpool_node *node;
-
-  /* We expect bounds constant is represented as a complex value
-     of two pointer sized integers.  */
-  gcc_assert (TREE_CODE (val) == COMPLEX_CST);
-
-  FOR_EACH_VARIABLE (node)
-    if (POINTER_BOUNDS_P (node->decl)
-	&& TREE_READONLY (node->decl)
-	&& DECL_INITIAL (node->decl)
-	&& TREE_CODE (DECL_INITIAL (node->decl)) == COMPLEX_CST
-	&& tree_int_cst_equal (TREE_REALPART (DECL_INITIAL (node->decl)),
-			       TREE_REALPART (val))
-	&& tree_int_cst_equal (TREE_IMAGPART (DECL_INITIAL (node->decl)),
-			       TREE_IMAGPART (val)))
-      return node->decl;
-
-  return NULL;
-}
-
 /* Return constant static bounds var with specified bounds LB and UB.
    If such var does not exists then new var is created with specified NAME.  */
 static tree
@@ -1907,37 +1880,43 @@ chkp_make_static_const_bounds (HOST_WIDE_INT lb,
 			       HOST_WIDE_INT ub,
 			       const char *name)
 {
+  tree id = get_identifier (name);
   tree var;
+  varpool_node *node;
+  symtab_node *snode;
+
+  var  = build_decl (UNKNOWN_LOCATION, VAR_DECL, id,
+		     pointer_bounds_type_node);
+  TREE_STATIC (var) = 1;
+  TREE_PUBLIC (var) = 1;
 
   /* 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);
+  if ((snode = symtab_node::get_for_asmname (DECL_ASSEMBLER_NAME (var))))
+    {
+      /* We don't allow this symbol usage for non bounds.  */
+      if (snode->type != SYMTAB_VARIABLE
+	  || !POINTER_BOUNDS_P (snode->decl))
+	sorry ("-fcheck-pointer-bounds requires '%s' "
+	       "name for internal usage",
+	       IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (var)));
+
+      return snode->decl;
+    }
 
-  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;
+  DECL_INITIAL (var) = targetm.chkp_make_bounds_constant (lb, ub);
+  make_decl_one_only (var, DECL_ASSEMBLER_NAME (var));
   /* 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->force_output = 1;
+
   varpool_node::finalize_decl (var);
 
   return var;
@@ -2001,14 +1980,6 @@ tree
 chkp_get_zero_bounds_var (void)
 {
   if (!chkp_zero_bounds_var)
-    {
-      tree id = get_identifier (CHKP_ZERO_BOUNDS_VAR_NAME);
-      symtab_node *node = symtab_node::get_for_asmname (id);
-      if (node)
-	chkp_zero_bounds_var = node->decl;
-    }
-
-  if (!chkp_zero_bounds_var)
     chkp_zero_bounds_var
       = chkp_make_static_const_bounds (0, -1,
 				       CHKP_ZERO_BOUNDS_VAR_NAME);
@@ -2020,14 +1991,6 @@ tree
 chkp_get_none_bounds_var (void)
 {
   if (!chkp_none_bounds_var)
-    {
-      tree id = get_identifier (CHKP_NONE_BOUNDS_VAR_NAME);
-      symtab_node *node = symtab_node::get_for_asmname (id);
-      if (node)
-	chkp_none_bounds_var = node->decl;
-    }
-
-  if (!chkp_none_bounds_var)
     chkp_none_bounds_var
       = chkp_make_static_const_bounds (-1, 0,
 				       CHKP_NONE_BOUNDS_VAR_NAME);