[Bug tree-optimization/64330] [5 Regression] IPA-ICF merges const exported vars that could be addressable in other TUs

2014-12-18 Thread marxin at gcc dot gnu.org 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64330

--- Comment #10 from Martin Liška marxin at gcc dot gnu.org ---
Author: marxin
Date: Thu Dec 18 13:32:18 2014
New Revision: 218864

URL: https://gcc.gnu.org/viewcvs?rev=218864root=gccview=rev
Log:
Fix for PR64330.

PR tree-optimization/64330
* ipa-icf.c (sem_variable::parse): Add checking
for externally visible symbols and do not introduce
an alias for an external declaration.


Modified:
trunk/gcc/ChangeLog
trunk/gcc/ipa-icf.c

[Bug tree-optimization/64330] [5 Regression] IPA-ICF merges const exported vars that could be addressable in other TUs

2014-12-18 Thread marxin at gcc dot gnu.org 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64330

Martin Liška marxin at gcc dot gnu.org changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #11 from Martin Liška marxin at gcc dot gnu.org ---
Fixed in 5.0.0.

[Bug tree-optimization/64330] [5 Regression] IPA-ICF merges const exported vars that could be addressable in other TUs

2014-12-17 Thread y.gribov at samsung dot com 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64330

Yury Gribov y.gribov at samsung dot com changed:

   What|Removed |Added

 CC||y.gribov at samsung dot com

--- Comment #6 from Yury Gribov y.gribov at samsung dot com ---
(In reply to Jakub Jelinek from comment #5)
 (In reply to Kostya Serebryany from comment #4)
  So, maybe the ODR checker in the current form is not that useless.
  Sorry, couldn't resist :)
 
 But it isn't really an ODR checker.

Basically a symbol aliasing checker.

[Bug tree-optimization/64330] [5 Regression] IPA-ICF merges const exported vars that could be addressable in other TUs

2014-12-17 Thread hubicka at gcc dot gnu.org 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64330

Jan Hubicka hubicka at gcc dot gnu.org changed:

   What|Removed |Added

 CC||hubicka at gcc dot gnu.org

--- Comment #7 from Jan Hubicka hubicka at gcc dot gnu.org ---
I think the test should be something like
 VIRTUAL || -fmerge-all-constants || (!TREE_ADDRESSABLE  !
externally_visible)

te current code seems to miss the externally_visible flag.  Indeed you can not
merge symbols that may have address taken.

How does asan behave with aliases?

Honza

[Bug tree-optimization/64330] [5 Regression] IPA-ICF merges const exported vars that could be addressable in other TUs

2014-12-17 Thread marxin at gcc dot gnu.org 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64330

--- Comment #8 from Martin Liška marxin at gcc dot gnu.org ---
Thank you for the missing externally visible attribute.

I've been testing following patch:

diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index b193200..0685019 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -1131,8 +1131,12 @@ sem_variable::parse (varpool_node *node, bitmap_obstack
*stack)
   tree decl = node-decl;

   bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl);
-  bool can_handle = readonly  (DECL_VIRTUAL_P (decl)
-|| !TREE_ADDRESSABLE (decl));
+  if (!readonly)
+return NULL;
+
+  bool can_handle = DECL_VIRTUAL_P (decl)
+   || flag_merge_constants = 2
+   || (!TREE_ADDRESSABLE (decl)  !node-externally_visible);

   if (!can_handle)
 return NULL;

As soon as tests finish, I will send it to ML.

Martin

[Bug tree-optimization/64330] [5 Regression] IPA-ICF merges const exported vars that could be addressable in other TUs

2014-12-17 Thread hubicka at ucw dot cz 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64330

--- Comment #9 from Jan Hubicka hubicka at ucw dot cz ---
 Thank you for the missing externally visible attribute.
 
 I've been testing following patch:
 
 diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
 index b193200..0685019 100644
 --- a/gcc/ipa-icf.c
 +++ b/gcc/ipa-icf.c
 @@ -1131,8 +1131,12 @@ sem_variable::parse (varpool_node *node, bitmap_obstack
 *stack)
tree decl = node-decl;
 
bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY 
 (decl);
 -  bool can_handle = readonly  (DECL_VIRTUAL_P (decl)
 -|| !TREE_ADDRESSABLE (decl));
 +  if (!readonly)
 +return NULL;
 +
 +  bool can_handle = DECL_VIRTUAL_P (decl)
 +   || flag_merge_constants = 2
 +   || (!TREE_ADDRESSABLE (decl)  
 !node-externally_visible);
 
if (!can_handle)
  return NULL;
 
 As soon as tests finish, I will send it to ML.

Thanks, the patch is OK if it passes.
Also please be sure that we won't merge DECL_EXTERNAL at all.  Producing an
extenral
alias is not going to save any code.

Honza
 
 Martin
 
 -- 
 You are receiving this mail because:
 You are on the CC list for the bug.

[Bug tree-optimization/64330] [5 Regression] IPA-ICF merges const exported vars that could be addressable in other TUs

2014-12-16 Thread jakub at gcc dot gnu.org 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64330

Jakub Jelinek jakub at gcc dot gnu.org changed:

   What|Removed |Added

   Priority|P3  |P1
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2014-12-16
  Component|sanitizer   |tree-optimization
   Target Milestone|--- |5.0
Summary|[ASAN] Bogus|[5 Regression] IPA-ICF
   |AddressSanitizer:  |merges const exported vars
   |odr-violation  |that could be addressable
   ||in other TUs
 Ever confirmed|0   |1

--- Comment #3 from Jakub Jelinek jakub at gcc dot gnu.org ---
sem_variable *
sem_variable::parse (varpool_node *node, bitmap_obstack *stack)
{
  tree decl = node-decl;

  bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl);
  bool can_handle = readonly  (DECL_VIRTUAL_P (decl)
 || !TREE_ADDRESSABLE (decl));

So it indeed doesn't consider the case when the variable is accessible by other
TUs where the variable could be addressable in them.

[Bug tree-optimization/64330] [5 Regression] IPA-ICF merges const exported vars that could be addressable in other TUs

2014-12-16 Thread kcc at gcc dot gnu.org 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64330

--- Comment #4 from Kostya Serebryany kcc at gcc dot gnu.org ---
So, maybe the ODR checker in the current form is not that useless.
Sorry, couldn't resist :)

[Bug tree-optimization/64330] [5 Regression] IPA-ICF merges const exported vars that could be addressable in other TUs

2014-12-16 Thread jakub at gcc dot gnu.org 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64330

--- Comment #5 from Jakub Jelinek jakub at gcc dot gnu.org ---
(In reply to Kostya Serebryany from comment #4)
 So, maybe the ODR checker in the current form is not that useless.
 Sorry, couldn't resist :)

But it isn't really an ODR checker.  Here it complains if two different symbols
share the same storage.  Which is fine if they aren't address taken and can't
be address taken elsewhere.