From 3cb251b77933671acc8717ede172be28d839927f Mon Sep 17 00:00:00 2001 From: Jan Hubicka Date: Sun, 7 Dec 2014 08:35:11 +0100 Subject: [PATCH] symtab.c (symtab_node::equal_address_to): New function. * symtab.c (symtab_node::equal_address_to): New function. * cgraph.h (symtab_node::equal_address_to): Declare. * fold-const.c (fold_comparison, fold_binary_loc): Use it. * c-family/c-common.c: Refuse weaks for symbols that can not change visibility. * gcc.dg/addr_equal-1.c: New testcase. From-SVN: r218462 --- gcc/ChangeLog | 8 +++ gcc/c-family/c-common.c | 7 +- gcc/cgraph.h | 5 ++ gcc/fold-const.c | 61 +++++++---------- gcc/symtab.c | 87 ++++++++++++++++++++++++ gcc/testsuite/ChangeLog | 4 ++ gcc/testsuite/gcc.dg/addr_equal-1.c | 101 ++++++++++++++++++++++++++++ 7 files changed, 235 insertions(+), 38 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/addr_equal-1.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 676ffaebced..e233e6582aa 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,11 @@ +2014-12-07 Jan Hubicka + + * symtab.c (symtab_node::equal_address_to): New function. + * cgraph.h (symtab_node::equal_address_to): Declare. + * fold-const.c (fold_comparison, fold_binary_loc): Use it. + * c-family/c-common.c: Refuse weaks for symbols that can not change + visibility. + 2014-12-07 Jonathan Wakely * doc/invoke.texi (Warning Options): Fix spelling and grammar. diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 65256cfecd9..1066c6b96a7 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -7781,7 +7781,12 @@ handle_weak_attribute (tree *node, tree name, } else if (TREE_CODE (*node) == FUNCTION_DECL || TREE_CODE (*node) == VAR_DECL) - declare_weak (*node); + { + struct symtab_node *n = symtab_node::get (*node); + if (n && n->refuse_visibility_changes) + error ("%+D declared weak after being used", *node); + declare_weak (*node); + } else warning (OPT_Wattributes, "%qE attribute ignored", name); diff --git a/gcc/cgraph.h b/gcc/cgraph.h index 54ee7489416..997414ce894 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -332,6 +332,11 @@ public: /* Return true if symbol is known to be nonzero. */ bool nonzero_address (); + /* Return 0 if symbol is known to have different address than S2, + Return 1 if symbol is known to have same address as S2, + return 2 otherwise. */ + int equal_address_to (symtab_node *s2); + /* Return symbol table node associated with DECL, if any, and NULL otherwise. */ static inline symtab_node *get (const_tree decl) diff --git a/gcc/fold-const.c b/gcc/fold-const.c index c268007415c..94d1cbfc2b2 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -8985,7 +8985,7 @@ fold_comparison (location_t loc, enum tree_code code, tree type, } } /* For non-equal bases we can simplify if they are addresses - of local binding decls or constants. */ + declarations with different addresses. */ else if (indirect_base0 && indirect_base1 /* We know that !operand_equal_p (base0, base1, 0) because the if condition was false. But make @@ -8993,16 +8993,13 @@ fold_comparison (location_t loc, enum tree_code code, tree type, && base0 != base1 && TREE_CODE (arg0) == ADDR_EXPR && TREE_CODE (arg1) == ADDR_EXPR - && (((TREE_CODE (base0) == VAR_DECL - || TREE_CODE (base0) == PARM_DECL) - && (targetm.binds_local_p (base0) - || CONSTANT_CLASS_P (base1))) - || CONSTANT_CLASS_P (base0)) - && (((TREE_CODE (base1) == VAR_DECL - || TREE_CODE (base1) == PARM_DECL) - && (targetm.binds_local_p (base1) - || CONSTANT_CLASS_P (base0))) - || CONSTANT_CLASS_P (base1))) + && DECL_P (base0) + && DECL_P (base1) + /* Watch for aliases. */ + && (!decl_in_symtab_p (base0) + || !decl_in_symtab_p (base1) + || !symtab_node::get_create (base0)->equal_address_to + (symtab_node::get_create (base1)))) { if (code == EQ_EXPR) return omit_two_operands_loc (loc, type, boolean_false_node, @@ -12257,33 +12254,23 @@ fold_binary_loc (location_t loc, unaliased symbols neither of which are extern (since we do not have access to attributes for externs), then we know the result. */ if (TREE_CODE (arg0) == ADDR_EXPR - && VAR_OR_FUNCTION_DECL_P (TREE_OPERAND (arg0, 0)) - && ! DECL_WEAK (TREE_OPERAND (arg0, 0)) - && ! lookup_attribute ("alias", - DECL_ATTRIBUTES (TREE_OPERAND (arg0, 0))) - && ! DECL_EXTERNAL (TREE_OPERAND (arg0, 0)) + && DECL_P (TREE_OPERAND (arg0, 0)) && TREE_CODE (arg1) == ADDR_EXPR - && VAR_OR_FUNCTION_DECL_P (TREE_OPERAND (arg1, 0)) - && ! DECL_WEAK (TREE_OPERAND (arg1, 0)) - && ! lookup_attribute ("alias", - DECL_ATTRIBUTES (TREE_OPERAND (arg1, 0))) - && ! DECL_EXTERNAL (TREE_OPERAND (arg1, 0))) - { - /* We know that we're looking at the address of two - non-weak, unaliased, static _DECL nodes. - - It is both wasteful and incorrect to call operand_equal_p - to compare the two ADDR_EXPR nodes. It is wasteful in that - all we need to do is test pointer equality for the arguments - to the two ADDR_EXPR nodes. It is incorrect to use - operand_equal_p as that function is NOT equivalent to a - C equality test. It can in fact return false for two - objects which would test as equal using the C equality - operator. */ - bool equal = TREE_OPERAND (arg0, 0) == TREE_OPERAND (arg1, 0); - return constant_boolean_node (equal - ? code == EQ_EXPR : code != EQ_EXPR, - type); + && DECL_P (TREE_OPERAND (arg1, 0))) + { + int equal; + + if (decl_in_symtab_p (TREE_OPERAND (arg0, 0)) + && decl_in_symtab_p (TREE_OPERAND (arg1, 0))) + equal = symtab_node::get_create (TREE_OPERAND (arg0, 0)) + ->equal_address_to (symtab_node::get_create + (TREE_OPERAND (arg1, 0))); + else + equal = TREE_OPERAND (arg0, 0) == TREE_OPERAND (arg1, 0); + if (equal != 2) + return constant_boolean_node (equal + ? code == EQ_EXPR : code != EQ_EXPR, + type); } /* Similarly for a NEGATE_EXPR. */ diff --git a/gcc/symtab.c b/gcc/symtab.c index 29839e676f0..3eceb88340f 100644 --- a/gcc/symtab.c +++ b/gcc/symtab.c @@ -1860,3 +1860,90 @@ symtab_node::nonzero_address () return true; return false; } + +/* Return 0 if symbol is known to have different address than S2, + Return 1 if symbol is known to have same address as S2, + return 2 otherwise. */ +int +symtab_node::equal_address_to (symtab_node *s2) +{ + enum availability avail1, avail2; + + /* A Shortcut: equivalent symbols are always equivalent. */ + if (this == s2) + return 1; + + /* For non-interposable aliases, lookup and compare their actual definitions. + Also check if the symbol needs to bind to given definition. */ + symtab_node *rs1 = ultimate_alias_target (&avail1); + symtab_node *rs2 = s2->ultimate_alias_target (&avail2); + bool binds_local1 = rs1->analyzed && decl_binds_to_current_def_p (this->decl); + bool binds_local2 = rs2->analyzed && decl_binds_to_current_def_p (s2->decl); + bool really_binds_local1 = binds_local1; + bool really_binds_local2 = binds_local2; + + /* Addresses of vtables and virtual functions can not be used by user + code and are used only within speculation. In this case we may make + symbol equivalent to its alias even if interposition may break this + rule. Doing so will allow us to turn speculative inlining into + non-speculative more agressively. */ + if (DECL_VIRTUAL_P (this->decl) && avail1 >= AVAIL_AVAILABLE) + binds_local1 = true; + if (DECL_VIRTUAL_P (s2->decl) && avail2 >= AVAIL_AVAILABLE) + binds_local2 = true; + + /* If both definitions are available we know that even if they are bound + to other unit they must be defined same way and therefore we can use + equivalence test. */ + if (rs1 != rs2 && avail1 >= AVAIL_AVAILABLE && avail2 >= AVAIL_AVAILABLE) + binds_local1 = binds_local2 = true; + + if ((binds_local1 ? rs1 : this) + == (binds_local2 ? rs2 : s2)) + { + /* We made use of the fact that alias is not weak. */ + if (binds_local1 && rs1 != this) + refuse_visibility_changes = true; + if (binds_local2 && rs2 != s2) + s2->refuse_visibility_changes = true; + return 1; + } + + /* If both symbols may resolve to NULL, we can not really prove them different. */ + if (!nonzero_address () && !s2->nonzero_address ()) + return 2; + + /* Except for NULL, functions and variables never overlap. */ + if (TREE_CODE (decl) != TREE_CODE (s2->decl)) + return 0; + + /* If one of the symbols is unresolved alias, punt. */ + if (rs1->alias || rs2->alias) + return 2; + + /* If we have a non-interposale definition of at least one of the symbols + and the other symbol is different, we know other unit can not interpose + it to the first symbol; all aliases of the definition needs to be + present in the current unit. */ + if (((really_binds_local1 || really_binds_local2) + /* If we have both definitions and they are different, we know they + will be different even in units they binds to. */ + || (binds_local1 && binds_local2)) + && rs1 != rs2) + { + /* We make use of the fact that one symbol is not alias of the other + and that the definition is non-interposable. */ + refuse_visibility_changes = true; + s2->refuse_visibility_changes = true; + rs1->refuse_visibility_changes = true; + rs2->refuse_visibility_changes = true; + return 0; + } + + /* TODO: Alias oracle basically assume that addresses of global variables + are different unless they are declared as alias of one to another. + We probably should be consistent and use this fact here, too, and update + alias oracle to use this predicate. */ + + return 2; +} diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 4c8939757be..d96c4ec68a0 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2014-12-07 Jan Hubicka + + * gcc.dg/addr_equal-1.c: New testcase. + 2014-12-06 James Greenhalgh Sebastian Pop Brian Rzycki diff --git a/gcc/testsuite/gcc.dg/addr_equal-1.c b/gcc/testsuite/gcc.dg/addr_equal-1.c new file mode 100644 index 00000000000..b033f50bfe7 --- /dev/null +++ b/gcc/testsuite/gcc.dg/addr_equal-1.c @@ -0,0 +1,101 @@ +/* { dg-do run } */ +/* { dg-require-weak "" } */ +/* { dg-require-alias "" } */ +/* { dg-options "-O2" } */ +void abort (void); +extern int undef_var0, undef_var1; +extern __attribute__ ((weak)) int weak_undef_var0; +extern __attribute__ ((weak)) int weak_undef_var1; +__attribute__ ((weak)) int weak_def_var0; +int def_var0=0, def_var1=0; +static int alias_var0 __attribute__ ((alias("def_var0"))); +extern int weak_alias_var0 __attribute__ ((alias("def_var0"))) __attribute__ ((weak)); +void undef_fn0(void); +void undef_fn1(void); +void def_fn0(void) +{ +} +void def_fn1(void) +{ +} +__attribute__ ((weak)) +void weak_def_fn0(void) +{ +} +__attribute__ ((weak)) +void weak_def_fn1(void) +{ +} +__attribute__ ((weak)) void weak_undef_fn0(void); + +inline +void inline_fn0(void) +{ +} +inline +void inline_fn1(void) +{ +} + +int +main(int argc, char **argv) +{ + /* Two definitions are always different unless they can be interposed. */ + if (!__builtin_constant_p (def_fn0 == def_fn1)) + abort(); + if (def_fn0 == def_fn1) + abort(); + + if (!__builtin_constant_p (&def_var0 == &def_var1)) + abort(); + if (&def_var0 == &def_var1) + abort(); + + /* Same symbol is the same no matter on interposition. */ + if (!__builtin_constant_p (undef_fn0 != undef_fn0)) + abort (); + if (undef_fn0 != undef_fn0) + abort (); + + /* Do not get confused by same offset. */ + if (!__builtin_constant_p (&undef_var0 + argc != &undef_var0 + argc)) + abort (); + if (&undef_var0 + argc != &undef_var0 + argc) + abort (); + + /* Alias and its target is equivalent unless one of them can be interposed. */ + if (!__builtin_constant_p (&def_var0 != &alias_var0)) + abort (); + if (&def_var0 != &alias_var0 ) + abort (); + + if (__builtin_constant_p (&def_var0 != &weak_alias_var0)) + abort (); + if (&def_var0 != &weak_alias_var0) + abort (); + + /* Weak definitions may be both NULL. */ + if (__builtin_constant_p ((void *)weak_undef_fn0 == (void *)&weak_undef_var0)) + abort (); + if ((void *)weak_undef_fn0 != (void *)&weak_undef_var0) + abort (); + + /* Variables and functions do not share same memory locations otherwise. */ + if (!__builtin_constant_p ((void *)undef_fn0 == (void *)&undef_var0)) + abort (); + if ((void *)undef_fn0 == (void *)&undef_var0) + abort (); + + /* This works for cases where one object is just weakly defined, too. */ + if (!__builtin_constant_p ((void *)weak_undef_fn0 == (void *)&weak_def_var0)) + abort (); + if ((void *)weak_undef_fn0 == (void *)&weak_def_var0) + abort (); + + /* Inline functions are known to be different. */ + if (!__builtin_constant_p (inline_fn0 != inline_fn1)) + abort (); + if (inline_fn0 == inline_fn1) + abort (); + return 0; +} -- 2.30.2