Patchwork [PR45875] Fix get_binfo_at_offset

login
register
mail settings
Submitter Martin Jambor
Date Oct. 21, 2010, 12:51 p.m.
Message ID <20101021125133.GA21785@virgil.arch.suse.de>
Download mbox | patch
Permalink /patch/68603/
State New
Headers show

Comments

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.
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

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)
     {