[Bug fortran/44596] New: Dynamic dispatch uses broken types

[Bug fortran/44596] New: Dynamic dispatch uses broken types

[rguenth at gcc dot gnu dot org]

On mem-ref2 branch gfortran.dg/dynamic_dispatch_6.f03 is miscompiled due
to TBAA issues as the Frontend uses two completely unrelated structure
types to access the same vtable pointer.

That of course can't work.


-- 
           Summary: Dynamic dispatch uses broken types
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: rguenth at gcc dot gnu dot org


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

[Bug fortran/44596] Dynamic dispatch uses broken types

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2010-06-19 19:16 -------
  static struct vtype$periodic_5th_factory vtab$periodic_5th_factory =
{.$hash=9935896, .$size=0, .$extends=&vtab$field_factory};

...

  if (vtab$periodic_5th_factory.create == 0B)
    {
      vtab$periodic_5th_factory.create = new_periodic_5th_order;
    }
  field_creator.$vptr = (struct vtype$field_factory *)
&vtab$periodic_5th_factory;
  u = VIEW_CONVERT_EXPR<struct class$field_p>(field_creator.$vptr->create
(&field_creator));

is broken.  You access struct vtype$periodic_5th_factory via a pointer
of type struct vtype$field_factory but they are not in any way related.
Look into alias.c:record_component_aliases and provide proper BINFOs,
or if that is not possible make sure to disable TBAA for vtables
by making all pointers to them ref-all.


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[burnus at gcc dot gnu dot org]

------- Comment #2 from burnus at gcc dot gnu dot org  2010-06-22 15:00 -------
CC Paul as he might have an idea how to handle it (cf. comment 1).

BTW: The mem-ref2 branch (cf. http://gcc.gnu.org/wiki/MemRef ) will be merged
soon.


-- 

burnus at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pault at gcc dot gnu dot org


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[paul dot richard dot thomas at gmail dot com]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1617 bytes --]



------- Comment #3 from paul dot richard dot thomas at gmail dot com  2010-06-22 19:15 -------
Subject: Re:  [OOP] Dynamic dispatch uses broken types

Dear Tobias,

> ------- Comment #2 from burnus at gcc dot gnu dot org  2010-06-22 15:00 -------
> CC Paul as he might have an idea how to handle it (cf. comment 1).

It seems that our attempt to use the front end to describe
polymorphism is running into difficulties and is facing us with what I
had hoped to avoid:  Coming to grips with the way that g++ does
things.  The reason that I had hoped to avoid this is obvious - none
of us have the time to do it.

Here is some partial documentation of what Richard is talking about:

http://www.delorie.com/gnu/docs/gcc/gccint_40.html

http://docs.freebsd.org/info/gxxint/gxxint.info.Macros.html

http://www.math.utah.edu/docs/info/gxxint_1.html

(some repetition).

On the other hand, surely our front-endery can be persuaded to work?
I guess that we have to ensure that the inherited vtables are all of
the same type as the base type.  Since all the vtables in a class
consist of procedure pointers and pointers to generic vtables with
exactly the same structure, this should be straightforward.  I guess
that the error is to add to the vtable all the extra methods of the
inherited type, rather than sticking to overloading.

> BTW: The mem-ref2 branch (cf. http://gcc.gnu.org/wiki/MemRef ) will be merged
> soon.

Oh bother, I feel another rush coming on, just when I was hoping that
we could take stock of OOP and consider how to get it right :-(

Paul


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[paul dot richard dot thomas at gmail dot com]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 690 bytes --]



------- Comment #4 from paul dot richard dot thomas at gmail dot com  2010-06-23 17:36 -------
Subject: Re:  [OOP] Dynamic dispatch uses broken types

Tobias,

On Tue, Jun 22, 2010 at 5:00 PM, burnus at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #2 from burnus at gcc dot gnu dot org  2010-06-22 15:00 -------
> CC Paul as he might have an idea how to handle it (cf. comment 1).

It's "obvious", the vtables for a given class can all be of the same
type, thus eliminating the problem. :-)
>
> BTW: The mem-ref2 branch (cf. http://gcc.gnu.org/wiki/MemRef ) will be merged
> soon.
>

Cheers

Paul


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2010-07-01 14:22 -------
*** Bug 44746 has been marked as a duplicate of this bug. ***


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2010-07-01 14:21 -------
*** Bug 44745 has been marked as a duplicate of this bug. ***


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hjl dot tools at gmail dot
                   |                            |com


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[dominiq at lps dot ens dot fr]

------- Comment #7 from dominiq at lps dot ens dot fr  2010-07-01 14:25 -------
This may be a duplicate of pr44662. Could you try the patch in
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44662#c2 ?


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[rguenth at gcc dot gnu dot org]

------- Comment #8 from rguenth at gcc dot gnu dot org  2010-07-01 14:34 -------
(In reply to comment #7)
> This may be a duplicate of pr44662. Could you try the patch in
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44662#c2 ?

It is not.


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[dominiq at lps dot ens dot fr]

------- Comment #9 from dominiq at lps dot ens dot fr  2010-07-01 15:57 -------
(In reply to comment #8)
> > This may be a duplicate of pr44662. Could you try the patch in
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44662#c2 ?
>
> It is not.

Agreed for this pr (and pr44745 is a duplicate). However I think pr44746 is a
duplicate of pr44596 and should be fixed by the patch in its comment #2.


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[pault at gcc dot gnu dot org]

------- Comment #10 from pault at gcc dot gnu dot org  2010-07-01 18:27 -------
(In reply to comment #9)
> (In reply to comment #8)

I'm on the road for a few days - I'll update the tree on my laptop and see what
I can do :-)

Cheers

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |pault at gcc dot gnu dot org
                   |dot org                     |
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2010-07-01 18:27:06
               date|                            |


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[pault at gcc dot gnu dot org]

------- Comment #11 from pault at gcc dot gnu dot org  2010-07-04 14:40 -------
Subject: Bug 44596

Author: pault
Date: Sun Jul  4 14:40:34 2010
New Revision: 161801

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161801
Log:
2010-07-04  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/44596
        PR fortran/44745
        * trans-types.c (gfc_get_derived_type): Derived type fields
        with the vtype attribute must have TYPE_REF_CAN_ALIAS_ALL set.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-types.c


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[rguenth at gcc dot gnu dot org]

------- Comment #12 from rguenth at gcc dot gnu dot org  2010-07-05 12:32 -------
        field_type = build_pointer_type (field_type);

+      /* vtype fields can point to different types to the base type.  */
+      if (c->ts.type == BT_DERIVED && c->ts.u.derived->attr.vtype)
+       TYPE_REF_CAN_ALIAS_ALL (field_type) = true;
+

this is wrong.  You shouldn't change bits in (shared) types.  Instead do

        field_type = build_pointer_type_for_mode
            (field_type, ptr_mode,
             c->ts.type == BT_DERIVED && c->ts.u.derived->attr.vtype);


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[burnus at gcc dot gnu dot org]

------- Comment #13 from burnus at gcc dot gnu dot org  2010-07-05 13:14 -------
(In reply to comment #12)
> You shouldn't change bits in (shared) types.  Instead do
> 
>         field_type = build_pointer_type_for_mode
>             (field_type, ptr_mode,
>              c->ts.type == BT_DERIVED && c->ts.u.derived->attr.vtype);

Thanks for checking, Richard!


For those wondering, like me, where ptr_mode is defined: gcc/machmode.h has:
  extern enum machine_mode ptr_mode;

The the function itself is (tree.c):

/* Construct, lay out and return the type of pointers to TO_TYPE with
   mode MODE.  If CAN_ALIAS_ALL is TRUE, indicate this type can
   reference all of memory. If such a type has already been
   constructed, reuse it.  */

tree
build_pointer_type_for_mode (tree to_type, enum machine_mode mode,
                             bool can_alias_all)


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[paul dot richard dot thomas at gmail dot com]

------- Comment #14 from paul dot richard dot thomas at gmail dot com  2010-07-05 13:52 -------
Subject: Re:  [OOP] Dynamic dispatch uses broken types

On Mon, Jul 5, 2010 at 3:14 PM, burnus at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:

> Thanks for checking, Richard!

Indeed, seconded by me.

Is there anywhere that these rather more subtle aspects of gcc are
documented in a structured way?  I am afraid that I just do not have
the time to do the archeology that seems to be necessary.

I'll do the necessary asap.

Thanks

Paul


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[rguenther at suse dot de]

------- Comment #15 from rguenther at suse dot de  2010-07-05 14:06 -------
Subject: Re:  [OOP] Dynamic dispatch uses broken types

On Mon, 5 Jul 2010, paul dot richard dot thomas at gmail dot com wrote:

> ------- Comment #14 from paul dot richard dot thomas at gmail dot com  2010-07-05 13:52 -------
> Subject: Re:  [OOP] Dynamic dispatch uses broken types
> 
> On Mon, Jul 5, 2010 at 3:14 PM, burnus at gcc dot gnu dot org
> <gcc-bugzilla@gcc.gnu.org> wrote:
> 
> > Thanks for checking, Richard!
> 
> Indeed, seconded by me.
> 
> Is there anywhere that these rather more subtle aspects of gcc are
> documented in a structured way?  I am afraid that I just do not have
> the time to do the archeology that seems to be necessary.

Well, it's the general invariant that all TYPEs and DECLs are
shared, likewise some constants (notably INTEGER_CSTs).
You should never modify them directly (unless you know
exactly what you are doing), but use construction functions from tree.c

> I'll do the necessary asap.

Thanks.


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[paul dot richard dot thomas at gmail dot com]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 430 bytes --]



------- Comment #16 from paul dot richard dot thomas at gmail dot com  2010-07-05 15:47 -------
Subject: Re:  [OOP] Dynamic dispatch uses broken types

> ------- Comment #15 from rguenther at suse dot de  2010-07-05 14:06 -------

Now I take a look at build_pointer_type_for_mode and it's uses in
gfortran, it seems rather obvious.  Thanks for the helping hand.

Paul


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[pault at gcc dot gnu dot org]

------- Comment #17 from pault at gcc dot gnu dot org  2010-07-05 19:26 -------
Subject: Bug 44596

Author: pault
Date: Mon Jul  5 19:26:12 2010
New Revision: 161848

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161848
Log:
2010-07-05  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/44596
        * trans-types.c (gfc_get_derived_type): Derived type fields
        with the vtype attribute must have TYPE_REF_CAN_ALIAS_ALL set
        but build_pointer_type_for_mode must be used for this.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-types.c


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[burnus at gcc dot gnu dot org]

------- Comment #18 from burnus at gcc dot gnu dot org  2010-07-06 15:20 -------
Paul, thanks for the check in. Do you plan to backport it to 4.5, which sems to
use the same code?


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[paul dot richard dot thomas at gmail dot com]

------- Comment #19 from paul dot richard dot thomas at gmail dot com  2010-07-06 15:28 -------
Subject: Re:  [OOP] Dynamic dispatch uses broken types

Dear Tobias,

> Paul, thanks for the check in. Do you plan to backport it to 4.5, which sems to
> use the same code?

Yes, I could do that on Thursday, when I am back in Barcelona.  I do
not have 4.5 on the laptop.

Cheers

Paul


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[pault at gcc dot gnu dot org]

------- Comment #20 from pault at gcc dot gnu dot org  2010-07-11 17:45 -------
(In reply to comment #19)
> Subject: Re:  [OOP] Dynamic dispatch uses broken types
> 
> Dear Tobias,
> 
> > Paul, thanks for the check in. Do you plan to backport it to 4.5, which sems to
> > use the same code?

When I add the vtype attribute and the above patch, almost every OOP test fails
with, for example:

/svn/gcc-4_5-branch/gcc/testsuite/gfortran.dg/class_6.f03:7:0: internal
compiler error: in gimplify_expr, at gimplify.c:7346
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.

I think that, since we do not need this (yet!), I am disinclined to spend any
more time on it, unless Richard understands what is happening.

Paul


-- 


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

[Bug fortran/44596] [OOP] Dynamic dispatch uses broken types

[burnus at gcc dot gnu dot org]

------- Comment #21 from burnus at gcc dot gnu dot org  2010-07-13 10:01 -------
(In reply to comment #19)
> > Paul, thanks for the check in. Do you plan to backport it to 4.5, which
> > sems to use the same code?
> 
> Yes, I could do that on Thursday, when I am back in Barcelona.  I do
> not have 4.5 on the laptop.
[...]
> When I add the vtype attribute and the above patch, almost every OOP test
> fails

Well, if it does not want to be fixed then we don't fix it. I had hoped that it
would be trivial - which turned out to be not the case.

OOP was announced as experimental (in 4.5) - thus, OOP users can easily jump
directly to 4.6.

> I think that, since we do not need this (yet!)

Then close it as FIXED (on the 4.6 trunk).


-- 

burnus at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


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