> Hello. > > Following patch is a fix for PR ipa/64693, where ICF merges a pair of > functions > that somehow take a references to a different functions. Unfortunately, such > function > pointer can be used for function comparison that can break an executable. > > Tested on x86_64-linux-pc and no regression is added. > Apart from that, I'm able to run tests on LTO bootstraped compiler. > > Ready for trunk? > Thanks, > Martin
> >From ff65c1bb4ba1593ea42aec5d6cbedf2330d8a866 Mon Sep 17 00:00:00 2001 > From: mliska <mli...@suse.cz> > Date: Fri, 23 Jan 2015 14:58:36 +0100 > Subject: [PATCH] Fix PR ipa/64693. > > gcc/ChangeLog: > > 2015-01-23 Martin Liska <mli...@suse.cz> > > PR ipa/64693 > * ipa-icf.c (sem_function::equals_private): Handle references > for a pair of compared functions. I think you can copy a logic whether given function needs thunk - in particular you can still merge if functions are DECL_VIRTUAL. I also think you have same problem with variable initializers - if you merge two variables pointing to equivalent functions that however need different addresses, you still need to prevent merging. Honza > > gcc/testsuite/ChangeLog: > > 2015-01-23 Martin Liska <mli...@suse.cz> > > * gcc.dg/ipa/ipa-icf-26.c: Fix expected results. > * gcc.dg/ipa/ipa-icf-33.c: Remove reduced line. > * gcc.dg/ipa/ipa-icf-34.c: New test. > --- > gcc/ipa-icf.c | 27 +++++++++++++++++++++- > gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c | 3 +-- > gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c | 1 - > gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c | 43 > +++++++++++++++++++++++++++++++++++ > 4 files changed, 70 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c > > diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c > index afb5be5..4bea08d 100644 > --- a/gcc/ipa-icf.c > +++ b/gcc/ipa-icf.c > @@ -525,13 +525,38 @@ sem_function::equals_private (sem_item *item, > if (decl1 != decl2) > return return_false(); > > - > for (arg1 = DECL_ARGUMENTS (decl), > arg2 = DECL_ARGUMENTS (m_compared_func->decl); > arg1; arg1 = DECL_CHAIN (arg1), arg2 = DECL_CHAIN (arg2)) > if (!m_checker->compare_decl (arg1, arg2)) > return return_false (); > > + /* Compare if this function contains any function references > + and compare them. */ > + cgraph_node *source_node = get_node (); > + cgraph_node *target_node = m_compared_func->get_node (); > + if (source_node->num_references () != target_node->num_references ()) > + return return_false_with_msg ("different number of references"); > + > + ipa_ref *r1 = NULL, *r2 = NULL; > + > + for (unsigned i = 0; i < source_node->num_references (); i++) > + { > + symtab_node *ref1 = source_node->iterate_reference (i, r1)->referred; > + symtab_node *ref2 = target_node->iterate_reference (i, r2)->referred; > + > + bool is_cgraph1 = is_a<cgraph_node *> (ref1); > + bool is_cgraph2 = is_a<cgraph_node *> (ref2); > + > + if (!is_cgraph1 && !is_cgraph2) > + continue; > + > + if ((is_cgraph1 && is_cgraph2 && ref1 != ref2) > + || (!is_cgraph1 && is_cgraph2) > + || (is_cgraph1 && !is_cgraph2)) > + return return_false_with_msg ("different target of a reference"); > + } > + > /* Fill-up label dictionary. */ > for (unsigned i = 0; i < bb_sorted.length (); ++i) > { > diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c > b/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c > index 0c5a5a6..f48c040 100644 > --- a/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c > +++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c > @@ -38,7 +38,6 @@ int main() > return 0; > } > > -/* { dg-final { scan-ipa-dump "Semantic equality hit:bar->foo" "icf" } } */ > /* { dg-final { scan-ipa-dump "Semantic equality hit:remove->destroy" "icf" > } } */ > -/* { dg-final { scan-ipa-dump "Equal symbols: 2" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */ > /* { dg-final { cleanup-ipa-dump "icf" } } */ > diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c > b/gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c > index d7e814d..7b6a8ae 100644 > --- a/gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c > +++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c > @@ -23,5 +23,4 @@ int main() > } > > /* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */ > -/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */ > /* { dg-final { cleanup-ipa-dump "icf" } } */ > diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c > b/gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c > new file mode 100644 > index 0000000..c8507c7 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c > @@ -0,0 +1,43 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O0 -fipa-icf -fdump-ipa-icf-details" } */ > + > +#include <stdlib.h> > +#include <assert.h> > + > +int callback1(int a) > +{ > + return a * a; > +} > + > +int callback2(int a) > +{ > + return a * a; > +} > + > +static int test(int (*callback) (int)) > +{ > + if (callback == callback1) > + return 1; > + > + return 0; > +} > + > +int foo() > +{ > + return test(&callback1); > +} > + > +int bar() > +{ > + return test(&callback2); > +} > + > +int main() > +{ > + assert (foo() != bar()); > + > + return 0; > +} > + > +/* { dg-final { scan-ipa-dump "different target of a reference" "icf" } } */ > +/* { dg-final { cleanup-ipa-dump "icf" } } */ > -- > 2.1.2 >