> 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.enkov...@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.enkov...@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;
