[Bug c++/43120] New: Diamond virtual inheritence with covariant return type confuses GCC

[Bug c++/43120] New: Diamond virtual inheritence with covariant return type confuses GCC

[goeran at uddeborg dot se]

Diamond shaped class structure with a covariant return method causes GCC to
fail to adjust base pointers.  In the attached program, the output is a huge
number instead of the expected 123.

Looking at what happens in GDB, it appears that the this pointer in the call of
A(const A &other) gets a B reference rather than an A reference.  The place
where it expects to find the member A::a is actually outside of the object of
type F.  If I'm reading the output from GDB correctly.


-- 
           Summary: Diamond virtual inheritence with covariant return type
                    confuses GCC
           Product: gcc
           Version: 4.4.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: goeran at uddeborg dot se


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

[Bug c++/43120] Diamond virtual inheritence with covariant return type confuses GCC

[goeran at uddeborg dot se]

------- Comment #1 from goeran at uddeborg dot se  2010-02-19 14:24 -------
Created an attachment (id=19921)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=19921&action=view)
Test case

No special flags, just compile with "c++ thunk.cc -o thunk".


-- 


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

[Bug c++/43120] Diamond virtual inheritence with covariant return type confuses GCC

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2010-02-19 14:39 -------
Confirmed.  Broken since it is supported.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
           Keywords|                            |wrong-code
      Known to fail|                            |3.4.0 4.5.0
   Last reconfirmed|0000-00-00 00:00:00         |2010-02-19 14:39:04
               date|                            |


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

[Bug c++/43120] Diamond virtual inheritence with covariant return type confuses GCC

[jason at gcc dot gnu dot org]

------- Comment #3 from jason at gcc dot gnu dot org  2010-02-23 01:45 -------
Reduced a bit:

extern "C" void abort ();

struct A {
  virtual void dummy() {}
};

struct B {
  virtual B *clone() = 0;
};

struct C : public virtual B { };
struct D : public virtual B { };

struct E : public C, public D {
  virtual E *clone() = 0;
};

struct F* fp;
struct F : public A, public E {
  F(int) { fp = this; }

  virtual E *clone() {
    if (fp != this)
      abort ();
  }
};

int main() {
  F *a = new F(123);
  B *c = a;
  c->clone();
}


-- 


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

[Bug c++/43120] Diamond virtual inheritence with covariant return type confuses GCC

[goeran at uddeborg dot se]

------- Comment #4 from goeran at uddeborg dot se  2010-02-24 11:02 -------
Created an attachment (id=19944)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=19944&action=view)
A different simplification, avoiding the diamond inheritance

The diamond inheritance does not seem to be a requirement.  Here is a case
simplified in a different way, where there is no diamond inheritance any more. 
(This one does print "33" rather than a huge number.  Still wrong, obviously.)


-- 


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

[Bug c++/43120] Virtual inheritence with covariant return type confuses GCC

[jason at gcc dot gnu dot org]

-- 

jason at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |jason at gcc dot gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2010-02-19 14:39:04         |2010-07-01 19:57:07
               date|                            |


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

[Bug c++/43120] Virtual inheritence with covariant return type confuses GCC

[jason at gcc dot gnu dot org]

------- Comment #5 from jason at gcc dot gnu dot org  2010-07-02 21:12 -------
This seems to have been broken by

2003-01-27  Nathan Sidwell  <nathan@codesourcery.com>

        * class.c (update_vtable_entry_for_fn): Add index parameter.
        Generate vcall thunk for covariant overriding from a virtual
        primary base.
        (dfs_modify_vtables): Adjust.

Reverting that patch fixes the problem and doesn't break any of the existing
covariant tests.  Nathan, I couldn't figure out what the comment you added was
talking about; I don't suppose you have any idea after all these years?


-- 

jason at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nathan at codesourcery dot
                   |                            |com


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

[Bug c++/43120] Virtual inheritence with covariant return type confuses GCC

[nathan at codesourcery dot com]

------- Comment #6 from nathan at codesourcery dot com  2010-07-05 10:56 -------
Subject: Re:  Virtual inheritence with covariant return type
 confuses GCC

Jason,
> Reverting that patch fixes the problem and doesn't break any of the existing
> covariant tests.  Nathan, I couldn't figure out what the comment you added was
> talking about; I don't suppose you have any idea after all these years?

the patch email is at http://gcc.gnu.org/ml/gcc-patches/2003-01/msg02157.html 
and I see you and I discussed the patch then too, and it seems you took some 
convincing then too :)

Does that email thread help, or would you like me to dig deeper?

nathan


-- 


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

[Bug c++/43120] Virtual inheritence with covariant return type confuses GCC

[jason at gcc dot gnu dot org]

------- Comment #7 from jason at gcc dot gnu dot org  2010-07-07 13:24 -------
That thread does help, thanks.  But part of my confusion was that the testcase
you added with that patch (abi/covariant1) didn't actually seem to test the
bug.  When I add a definition of c14::f17, the test starts failing; even 3.4
refers to the no-this-adjustment covariant thunk in the c14 vtable.  So I'm not
sure what the patch was actually doing.


-- 


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

[Bug c++/43120] Virtual inheritence with covariant return type confuses GCC

[nathan at gcc dot gnu dot org]

------- Comment #8 from nathan at gcc dot gnu dot org  2010-07-07 13:56 -------
Hm, I guess I must have flubbed the testcase.  After all this time, I don't
have a better recollection.  Sorry.


-- 


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

[Bug c++/43120] Virtual inheritance with covariant return type confuses GCC

[jason at gcc dot gnu dot org]

------- Comment #9 from jason at gcc dot gnu dot org  2010-07-08 14:00 -------
Subject: Bug 43120

Author: jason
Date: Thu Jul  8 14:00:26 2010
New Revision: 161954

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161954
Log:
        PR c++/43120
        * class.c (update_vtable_entry_for_fn): Fix handling of dummy
        virtual bases for covariant thunks.

Added:
    trunk/gcc/testsuite/g++.dg/inherit/covariant17.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/class.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/abi/covariant1.C


-- 


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

[Bug c++/43120] Virtual inheritance with covariant return type confuses GCC

[jason at gcc dot gnu dot org]

------- Comment #10 from jason at gcc dot gnu dot org  2010-07-09 19:36 -------
Subject: Bug 43120

Author: jason
Date: Fri Jul  9 19:36:19 2010
New Revision: 162008

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162008
Log:
        PR c++/43120
        * cp-tree.h (BV_LOST_PRIMARY): New macro.
        * class.c (update_vtable_entry_for_fn): Fix covariant thunk logic.
        Set BV_LOST_PRIMARY.
        (build_vtbl_initializer): Check BV_LOST_PRIMARY.

Added:
    trunk/gcc/testsuite/g++.dg/abi/covariant6.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/class.c
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/abi/covariant1.C
    trunk/gcc/testsuite/g++.dg/inherit/covariant17.C
    trunk/gcc/testsuite/g++.dg/inherit/covariant7.C


-- 


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

[Bug c++/43120] Virtual inheritance with covariant return type confuses GCC

[jason at gcc dot gnu dot org]

------- Comment #11 from jason at gcc dot gnu dot org  2010-07-09 19:41 -------
Subject: Bug 43120

Author: jason
Date: Fri Jul  9 19:40:39 2010
New Revision: 162010

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162010
Log:
        PR c++/43120
        * class.c (update_vtable_entry_for_fn): Fix handling of dummy
        virtual bases for covariant thunks.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/g++.dg/inherit/covariant17.C
Modified:
    branches/gcc-4_5-branch/gcc/cp/ChangeLog
    branches/gcc-4_5-branch/gcc/cp/class.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_5-branch/gcc/testsuite/g++.dg/abi/covariant1.C


-- 


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

[Bug c++/43120] Virtual inheritance with covariant return type confuses GCC

[jason at gcc dot gnu dot org]

------- Comment #12 from jason at gcc dot gnu dot org  2010-07-09 19:46 -------
Subject: Bug 43120

Author: jason
Date: Fri Jul  9 19:45:53 2010
New Revision: 162011

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162011
Log:
        PR c++/43120
        * class.c (update_vtable_entry_for_fn): Fix handling of dummy
        virtual bases for covariant thunks.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/inherit/covariant17.C
Modified:
    branches/gcc-4_4-branch/gcc/cp/ChangeLog
    branches/gcc-4_4-branch/gcc/cp/class.c
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/abi/covariant1.C


-- 


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

[Bug c++/43120] Virtual inheritance with covariant return type confuses GCC

[jason at gcc dot gnu dot org]

------- Comment #13 from jason at gcc dot gnu dot org  2010-07-09 19:50 -------
Subject: Bug 43120

Author: jason
Date: Fri Jul  9 19:50:25 2010
New Revision: 162013

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162013
Log:
Revert "        PR c++/43120"

Removed:
    branches/gcc-4_5-branch/gcc/testsuite/g++.dg/inherit/covariant17.C
Modified:
    branches/gcc-4_5-branch/gcc/cp/ChangeLog
    branches/gcc-4_5-branch/gcc/cp/class.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_5-branch/gcc/testsuite/g++.dg/abi/covariant1.C


-- 


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

[Bug c++/43120] Virtual inheritance with covariant return type confuses GCC

[jason at gcc dot gnu dot org]

------- Comment #14 from jason at gcc dot gnu dot org  2010-07-09 20:12 -------
Fixed for 4.6.  This isn't a regression, and the fix changes the ABI (for code
that is badly broken without it), so I'm not comfortable applying it to the
release branches.  I did apply it, but then reverted it again.


-- 

jason at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.6.0


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