| Message ID | 20120413135236.GA15120@virgil.arch.suse.de |
|---|---|
| State | New |
| Headers | show |
On Fri, 13 Apr 2012, Martin Jambor wrote: > Hi, > > currently we ICE when attempting to devirtualize a call to a virtual > method introduced in a descendant but with a base which is an ancestor > which does not have it. This is because fold_ctor_reference returns > constant zero when it cannot find the particular value in the provided > constructor which is something gimple_get_virt_method_for_binfo cannot > cope with. The patch below avoids it by simply testing for that > value and bailing out. > > Bootstrapped and tested on x86_64-linux, OK for trunk? Ok. Thanks, Richard. > Thanks, > > Martin > > > 2012-04-12 Martin Jambor <mjambor@suse.cz> > > PR middle-end/52939 > * gimple-fold.c (gimple_get_virt_method_for_binfo): Bail out if > fold_ctor_reference returns a zero constant. > > * testsuite/g++.dg/ipa/pr52939.C: New test. > > Index: src/gcc/gimple-fold.c > =================================================================== > --- src.orig/gcc/gimple-fold.c > +++ src/gcc/gimple-fold.c > @@ -3087,7 +3087,7 @@ gimple_get_virt_method_for_binfo (HOST_W > offset += token * size; > fn = fold_ctor_reference (TREE_TYPE (TREE_TYPE (v)), DECL_INITIAL (v), > offset, size); > - if (!fn) > + if (!fn || integer_zerop (fn)) > return NULL_TREE; > gcc_assert (TREE_CODE (fn) == ADDR_EXPR > || TREE_CODE (fn) == FDESC_EXPR); > Index: src/gcc/testsuite/g++.dg/ipa/pr52939.C > =================================================================== > --- /dev/null > +++ src/gcc/testsuite/g++.dg/ipa/pr52939.C > @@ -0,0 +1,58 @@ > +/* Verify that we do not ICE on invalid devirtualizations (which might > + be OK at run-time because never executed). */ > +/* { dg-do run } */ > +/* { dg-options "-O3 -fno-early-inlining -fno-inline" } */ > + > +extern "C" void abort (void); > + > +class A > +{ > +public: > + int data; > + virtual int foo (int i); > +}; > + > +class B : public A > +{ > +public: > + virtual int foo (int i); > + virtual int bar (int i); > +}; > + > +int A::foo (int i) > +{ > + return i + 1; > +} > + > +int B::foo (int i) > +{ > + return i + 2; > +} > + > +int B::bar (int i) > +{ > + return i + 3; > +} > + > +static int middleman (class A *obj, int i) > +{ > + class B *b = (class B *) obj; > + > + if (i != 1) > + return b->bar (i); > + else > + return i; > +} > + > +int __attribute__ ((noinline,noclone)) get_input(void) > +{ > + return 1; > +} > + > +int main (int argc, char *argv[]) > +{ > + class A o; > + if (middleman (&o, get_input ()) != 1) > + abort (); > + return 0; > +} > >
Hi, On Fri, Apr 13, 2012 at 04:13:13PM +0200, Richard Guenther wrote: > On Fri, 13 Apr 2012, Martin Jambor wrote: > > > Hi, > > > > currently we ICE when attempting to devirtualize a call to a virtual > > method introduced in a descendant but with a base which is an ancestor > > which does not have it. This is because fold_ctor_reference returns > > constant zero when it cannot find the particular value in the provided > > constructor which is something gimple_get_virt_method_for_binfo cannot > > cope with. The patch below avoids it by simply testing for that > > value and bailing out. > > > > Bootstrapped and tested on x86_64-linux, OK for trunk? > > Ok. I forgot to ask permission also to commit it to the 4.7 branch but I suppose that I can do that too and will go ahead with the same patch soon (after bootstrap and testing on the branch). Thanks, Martin > > Thanks, > Richard. > > > Thanks, > > > > Martin > > > > > > 2012-04-12 Martin Jambor <mjambor@suse.cz> > > > > PR middle-end/52939 > > * gimple-fold.c (gimple_get_virt_method_for_binfo): Bail out if > > fold_ctor_reference returns a zero constant. > > > > * testsuite/g++.dg/ipa/pr52939.C: New test. > > > > Index: src/gcc/gimple-fold.c > > =================================================================== > > --- src.orig/gcc/gimple-fold.c > > +++ src/gcc/gimple-fold.c > > @@ -3087,7 +3087,7 @@ gimple_get_virt_method_for_binfo (HOST_W > > offset += token * size; > > fn = fold_ctor_reference (TREE_TYPE (TREE_TYPE (v)), DECL_INITIAL (v), > > offset, size); > > - if (!fn) > > + if (!fn || integer_zerop (fn)) > > return NULL_TREE; > > gcc_assert (TREE_CODE (fn) == ADDR_EXPR > > || TREE_CODE (fn) == FDESC_EXPR); > > Index: src/gcc/testsuite/g++.dg/ipa/pr52939.C > > =================================================================== > > --- /dev/null > > +++ src/gcc/testsuite/g++.dg/ipa/pr52939.C > > @@ -0,0 +1,58 @@ > > +/* Verify that we do not ICE on invalid devirtualizations (which might > > + be OK at run-time because never executed). */ > > +/* { dg-do run } */ > > +/* { dg-options "-O3 -fno-early-inlining -fno-inline" } */ > > + > > +extern "C" void abort (void); > > + > > +class A > > +{ > > +public: > > + int data; > > + virtual int foo (int i); > > +}; > > + > > +class B : public A > > +{ > > +public: > > + virtual int foo (int i); > > + virtual int bar (int i); > > +}; > > + > > +int A::foo (int i) > > +{ > > + return i + 1; > > +} > > + > > +int B::foo (int i) > > +{ > > + return i + 2; > > +} > > + > > +int B::bar (int i) > > +{ > > + return i + 3; > > +} > > + > > +static int middleman (class A *obj, int i) > > +{ > > + class B *b = (class B *) obj; > > + > > + if (i != 1) > > + return b->bar (i); > > + else > > + return i; > > +} > > + > > +int __attribute__ ((noinline,noclone)) get_input(void) > > +{ > > + return 1; > > +} > > + > > +int main (int argc, char *argv[]) > > +{ > > + class A o; > > + if (middleman (&o, get_input ()) != 1) > > + abort (); > > + return 0; > > +} > > > > > > -- > Richard Guenther <rguenther@suse.de> > SUSE / SUSE Labs > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
Index: src/gcc/gimple-fold.c =================================================================== --- src.orig/gcc/gimple-fold.c +++ src/gcc/gimple-fold.c @@ -3087,7 +3087,7 @@ gimple_get_virt_method_for_binfo (HOST_W offset += token * size; fn = fold_ctor_reference (TREE_TYPE (TREE_TYPE (v)), DECL_INITIAL (v), offset, size); - if (!fn) + if (!fn || integer_zerop (fn)) return NULL_TREE; gcc_assert (TREE_CODE (fn) == ADDR_EXPR || TREE_CODE (fn) == FDESC_EXPR); Index: src/gcc/testsuite/g++.dg/ipa/pr52939.C =================================================================== --- /dev/null +++ src/gcc/testsuite/g++.dg/ipa/pr52939.C @@ -0,0 +1,58 @@ +/* Verify that we do not ICE on invalid devirtualizations (which might + be OK at run-time because never executed). */ +/* { dg-do run } */ +/* { dg-options "-O3 -fno-early-inlining -fno-inline" } */ + +extern "C" void abort (void); + +class A +{ +public: + int data; + virtual int foo (int i); +}; + +class B : public A +{ +public: + virtual int foo (int i); + virtual int bar (int i); +}; + +int A::foo (int i) +{ + return i + 1; +} + +int B::foo (int i) +{ + return i + 2; +} + +int B::bar (int i) +{ + return i + 3; +} + +static int middleman (class A *obj, int i) +{ + class B *b = (class B *) obj; + + if (i != 1) + return b->bar (i); + else + return i; +} + +int __attribute__ ((noinline,noclone)) get_input(void) +{ + return 1; +} + +int main (int argc, char *argv[]) +{ + class A o; + if (middleman (&o, get_input ()) != 1) + abort (); + return 0; +}