[PR45875] Fix get_binfo_at_offset

Submitted by Martin Jambor on Oct. 21, 2010, 12:51 p.m.

Details

Message ID 20101021125133.GA21785@virgil.arch.suse.de
State New
Headers show

Commit Message

Martin Jambor Oct. 21, 2010, 12:51 p.m.
Hi,

the patch below fixes the original problem reported in PR 45875.  The
if statement is not only totally unnecessary but also wrong because it
does not take into account the expected_type parameter.

This currently leads to some IPA_JF_ANCESTOR jump functions which
should not be there, which then leads to propagations which should not
happen and wrong binfos being used for virtual method lookups.

Bootstrapped and tested on x86_64-linux.  OK for trunk?

Thanks,

Martin


2010-10-20  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/45875
	* tree.c (get_binfo_at_offset): Remove initial zero offset test.

	* testsuite/g++.dg/ipa/pr45875.C: New test.

Comments

Richard Guenther Oct. 21, 2010, 1:20 p.m.
On Thu, 21 Oct 2010, Martin Jambor wrote:

> Hi,
> 
> the patch below fixes the original problem reported in PR 45875.  The
> if statement is not only totally unnecessary but also wrong because it
> does not take into account the expected_type parameter.
> 
> This currently leads to some IPA_JF_ANCESTOR jump functions which
> should not be there, which then leads to propagations which should not
> happen and wrong binfos being used for virtual method lookups.
> 
> Bootstrapped and tested on x86_64-linux.  OK for trunk?

Ok.

Thanks,
Richard.

> Thanks,
> 
> Martin
> 
> 
> 2010-10-20  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR tree-optimization/45875
> 	* tree.c (get_binfo_at_offset): Remove initial zero offset test.
> 
> 	* testsuite/g++.dg/ipa/pr45875.C: New test.
> 
> Index: icln/gcc/testsuite/g++.dg/ipa/pr45875.C
> ===================================================================
> --- /dev/null
> +++ icln/gcc/testsuite/g++.dg/ipa/pr45875.C
> @@ -0,0 +1,48 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -fno-early-inlining -fno-ipa-cp"  } */
> +
> +extern "C" void abort (void);
> +
> +class A
> +{
> +public:
> +  virtual int foo (int i);
> +};
> +
> +class B
> +{
> +public:
> +  class A confusion;
> +};
> +
> +int A::foo (int i)
> +{
> +  return i + 1;
> +}
> +
> +int __attribute__ ((noinline,noclone)) get_input(void)
> +{
> +  return 1;
> +}
> +
> +static int middleman_a (class A *obj, int i)
> +{
> +  return obj->foo (i);
> +}
> +
> +static int middleman_b (class B *obj, int i)
> +{
> +  return middleman_a (&obj->confusion, i);
> +}
> +
> +
> +int main (int argc, char *argv[])
> +{
> +  class B b;
> +  int i, j = get_input ();
> +
> +  for (i = 0; i < j; i++)
> +    if (middleman_b (&b, j) != 2)
> +      abort ();
> +  return 0;
> +}
> Index: icln/gcc/tree.c
> ===================================================================
> --- icln.orig/gcc/tree.c
> +++ icln/gcc/tree.c
> @@ -10884,9 +10884,6 @@ get_binfo_at_offset (tree binfo, HOST_WI
>  {
>    tree type;
>  
> -  if (offset == 0)
> -    return binfo;
> -
>    type = TREE_TYPE (binfo);
>    while (offset > 0)
>      {
> 
>
H.J. Lu Oct. 21, 2010, 5:54 p.m.
On Thu, Oct 21, 2010 at 5:51 AM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> the patch below fixes the original problem reported in PR 45875.  The
> if statement is not only totally unnecessary but also wrong because it
> does not take into account the expected_type parameter.
>
> This currently leads to some IPA_JF_ANCESTOR jump functions which
> should not be there, which then leads to propagations which should not
> happen and wrong binfos being used for virtual method lookups.
>
> Bootstrapped and tested on x86_64-linux.  OK for trunk?
>
> Thanks,
>
> Martin
>
>
> 2010-10-20  Martin Jambor  <mjambor@suse.cz>
>
>        PR tree-optimization/45875
>        * tree.c (get_binfo_at_offset): Remove initial zero offset test.
>
>        * testsuite/g++.dg/ipa/pr45875.C: New test.
>

This patch caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46120

g++.dg/ipa/ivinline-?.C and tree.c (get_binfo_at_offset) are new with:

http://gcc.gnu.org/ml/gcc-cvs/2010-05/msg00559.html

Since you changed get_binfo_at_offset, are those tests still valid?

Patch hide | download patch | download mbox

Index: icln/gcc/testsuite/g++.dg/ipa/pr45875.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/pr45875.C
@@ -0,0 +1,48 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O3 -fno-early-inlining -fno-ipa-cp"  } */
+
+extern "C" void abort (void);
+
+class A
+{
+public:
+  virtual int foo (int i);
+};
+
+class B
+{
+public:
+  class A confusion;
+};
+
+int A::foo (int i)
+{
+  return i + 1;
+}
+
+int __attribute__ ((noinline,noclone)) get_input(void)
+{
+  return 1;
+}
+
+static int middleman_a (class A *obj, int i)
+{
+  return obj->foo (i);
+}
+
+static int middleman_b (class B *obj, int i)
+{
+  return middleman_a (&obj->confusion, i);
+}
+
+
+int main (int argc, char *argv[])
+{
+  class B b;
+  int i, j = get_input ();
+
+  for (i = 0; i < j; i++)
+    if (middleman_b (&b, j) != 2)
+      abort ();
+  return 0;
+}
Index: icln/gcc/tree.c
===================================================================
--- icln.orig/gcc/tree.c
+++ icln/gcc/tree.c
@@ -10884,9 +10884,6 @@  get_binfo_at_offset (tree binfo, HOST_WI
 {
   tree type;
 
-  if (offset == 0)
-    return binfo;
-
   type = TREE_TYPE (binfo);
   while (offset > 0)
     {