[Bug c++/37016] New: member function pointer failure with optimization

[Bug c++/37016] New: member function pointer failure with optimization

[rjharrison at gmail dot com]

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

#include <iostream>

/*                                                                              
  Basic design concept is that WorldObject implements remote method call        
  functionality using the "curiously recurring template pattern" to enable      
  forwarding calls from the generic base class that implements the transport    
  layer to the derived class.                                                   

  The specific failure was in forwarding calls to items in a container.         
  This is emulated here by wrapping a single item.                              

  In the main program there are two forms of the call.  In the last             
  (uncommented) form the member function pointer is for clarity                 
  assigned to a variable (itemfunptr) before making the call.                   
  With 4.3.0 and 4.3.1 this code compiles incorrectly with -O1 or greater       
  to produce this warning                                                       

  reproduce.cc: In function ‘int main()’:                                       
  reproduce.cc:26: warning: ‘itemfunptr.void (Container::*)(void
(Item::*)(int), int)::__pfn’ is used uninitialized in this function             
  reproduce.cc:47: note: ‘itemfunptr.void (Container::*)(void (Item::*)(int),
int)::__pfn’ was declared here                                                  

  and the resulting executable segvs.  It works correctly with -O0.             

  With 4.2.3 and earlier it works correctly with optimization.                  

  In the first (commented out) form of the call in the main program             
  we directly refer to desired member function.  This compiles                  
  and executes correctly with all tested versions.                              
*/

template <class Derived>
struct WorldObject {
    template <typename memfunT, typename arg1T, typename arg2T>
    void forward(memfunT memfun, arg1T arg1, arg2T arg2) {
        Derived* obj = static_cast<Derived*>(this);
        (obj->*memfun)(arg1, arg2);
    }
};

struct Item {
    void fred(int a) {
        std::cout << "a=" << a << std::endl;
    }
};

struct Container : public WorldObject<Container> {
    Item item;
    template <typename itemfunT, typename arg1T>
    void itemfun(itemfunT itemfun, int a) {
        (item.*itemfun)(a);
    }
};

int main() {
    typedef void (Item::*itemfun)(int);

    Container t;

    // This call compiles and executes correctly with all versions tested       
    //t.forward(&Container::itemfun<itemfun,int>, &Item::fred, 1);              

    // This call compiles with a warning and segvs on execution if using        
    // -O1 or greater with 4.3.*.  4.2.* is correct.                            
    void (Container::*itemfunptr)(itemfun, int) =
&Container::itemfun<itemfun,int>;
    t.forward(itemfunptr, &Item::fred, 1);

    return 0;
}


-- 
           Summary: member function pointer failure with optimization
           Product: gcc
           Version: 4.3.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: rjharrison at gmail dot com
 GCC build triplet: x86_64-unknown-linux-gnu
  GCC host triplet: x86_64-unknown-linux-gnu
GCC target triplet: x86_64-unknown-linux-gnu


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2008-08-03 21:47 -------
Confirmed.

SRA messing up again when there is different FIELD decls.
  itemfunptrD.21695.__pfnD.21700 ={v} itemfunD.21696;
  memfunD.24341 ={v} itemfunptrD.21695;
  memfun$__pfnD.24346_4 = memfunD.24341.__pfnD.21692;

You can see how the two different field decls are allocated.  I don't know if I
want to mark this a front bug or a middle one.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |major
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
           Keywords|                            |wrong-code
   Last reconfirmed|0000-00-00 00:00:00         |2008-08-03 21:47:37
               date|                            |
            Summary|member function pointer     |[4.3/4.4 Regression] member
                   |failure with optimization   |function pointer failure
                   |                            |with optimization
   Target Milestone|---                         |4.3.2


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2008-08-04 09:10 -------
Uhm.  Lovely SRA.


-- 

rguenth at gcc dot gnu dot org changed:

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


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2008-08-04 09:18 -------
Ok, so SRA does

-  itemfunptrD.2084.__pfnD.2089 ={v} itemfunD.2085;
-  itemfunptrD.2084.__deltaD.2082 ={v} 0;
-  memfunD.2158 ={v} itemfunptrD.2084;

+  itemfunptr$__pfnD.2176_1 = itemfunD.2085;
+  itemfunptr$__deltaD.2175_21 = 0;
+  memfun$__pfnD.2180_25 = itemfunptr$__pfnD.2177_24(D);
+  memfun$__deltaD.2181_26 = itemfunptr$__deltaD.2175_21;

using a new scalar element for itemfunptr$__pfn instead of the just-assigned
one (which means, somehow the element lookup doesn't work).  I'll investigate.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |rguenth at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2008-08-03 21:47:37         |2008-08-04 09:18:24
               date|                            |


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2008-08-04 11:54 -------
This is yet another case of the C++ frontend using two different record types
for the same PFN structure.

For example we have

 <component_ref 0x7ffff7ec18c0
...
    arg 0 <var_decl 0x7ffff759c8c0 itemfunptr
        type <record_type 0x7ffff75a5300 sizes-gimplified type_6 BLK
...
    arg 1 <field_decl 0x7ffff759c960 __pfn type <pointer_type 0x7ffff75a56c0>
...
        bit offset <integer_cst 0x7ffff7edc300 constant 0> context <record_type
0x7ffff75a5780>

where you can see the type of the variable indexed is not of the same type
as the field context type of the field we index it by ...

Because TYPE_GET_PTRMEMFUNC_TYPE for itemfun is a different type than the
type of itemfunptr, so we build a "wrong" PTRMEM_CST object.

The following "fixes" this, but I'd like to have a C++ maintainer have a look
here... (there doesn't seem to exist an easy way to fix or workaround this
in SRA, but eventually gimplification of the CONSTRUCTOR could insert a
conversion)

Index: cp/class.c
===================================================================
--- cp/class.c  (revision 138552)
+++ cp/class.c  (working copy)
@@ -6197,7 +6197,7 @@ resolve_address_of_overloaded_function (
     }

   if (TYPE_PTRFN_P (target_type) || TYPE_PTRMEMFUNC_P (target_type))
-    return cp_build_unary_op (ADDR_EXPR, fn, 0, flags);
+    return make_ptrmem_cst (target_type, fn);
   else
     {
       /* The target must be a REFERENCE_TYPE.  Above, cp_build_unary_op


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|rguenth at gcc dot gnu dot  |unassigned at gcc dot gnu
                   |org                         |dot org
             Status|ASSIGNED                    |NEW


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2008-08-04 11:56 -------
I'm testing that patch, but I'm sure some side-effects of cp_build_unary_op
are necessary...


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu dot org
           Priority|P3                          |P1


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2008-08-04 13:41 -------
Of course it didn't work.  The following seems to

Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c      (revision 138611)
+++ gcc/cp/class.c      (working copy)
@@ -6197,7 +6197,12 @@ resolve_address_of_overloaded_function (
     }

   if (TYPE_PTRFN_P (target_type) || TYPE_PTRMEMFUNC_P (target_type))
-    return cp_build_unary_op (ADDR_EXPR, fn, 0, flags);
+    {
+      tree tem = cp_build_unary_op (ADDR_EXPR, fn, 0, flags);
+      if (TREE_CODE (tem) == PTRMEM_CST)
+       return make_ptrmem_cst (target_type, fn);
+      return tem;
+    }
   else
     {
       /* The target must be a REFERENCE_TYPE.  Above, cp_build_unary_op


we might be able to unconditionally do return make_ptrmem_cst (target_type,
fn);
for TYPE_PTRMEMFUNC_P (target_type) though.


-- 


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[rguenth at gcc dot gnu dot org]

------- Comment #7 from rguenth at gcc dot gnu dot org  2008-08-04 14:00 -------
Causes

FAIL: g++.dg/template/ptrmem8.C  (test for errors, line 18)
FAIL: g++.dg/template/ptrmem8.C  (test for errors, line 19)

Jason - can you help here?  Thanks.


-- 


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[jason at gcc dot gnu dot org]

------- Comment #8 from jason at gcc dot gnu dot org  2008-08-04 19:37 -------
In general, it seems like SRA needs to be more conservative with
TYPE_STRUCTURAL_EQUALITY_P types such as these.


-- 


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[jason at gcc dot gnu dot org]

------- Comment #9 from jason at gcc dot gnu dot org  2008-08-04 20:17 -------
However, it's not clear to me that pointers to member functions need to have
TYPE_STRUCTURAL_EQUALITY_P in general; I'm testing a patch to set
TYPE_CANONICAL appropriately.


-- 


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[rguenth at gcc dot gnu dot org]

------- Comment #10 from rguenth at gcc dot gnu dot org  2008-08-04 20:27 -------
The middle-end expects that if there is variable of record type T that for
a component-reference it can walk its fields and find the one used as
operand 1 in the component-reference.

If you look at the tree in commend #4 as it reaches the middle-end then
this is not the case.  SRA tries to deal with this by doing

    case FIELD_DECL:
      /* Fields are unique within a record, but not between
         compatible records.  */
      if (DECL_FIELD_CONTEXT (ae) == DECL_FIELD_CONTEXT (be))
        return false;
      return fields_compatible_p (ae, be);

but fields_compatible_p returns false because

  if (!types_compatible_p (TREE_TYPE (f1), TREE_TYPE (f2)))
    return false;

returns false.


-- 


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[jason at gcc dot gnu dot org]

------- Comment #11 from jason at gcc dot gnu dot org  2008-08-04 21:02 -------
Created an attachment (id=16017)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16017&action=view)
Patch to check TYPE_STRUCTURAL_EQUALITY_P in useless_type_conversion


-- 


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[jason at gcc dot gnu dot org]

------- Comment #12 from jason at gcc dot gnu dot org  2008-08-04 21:03 -------
Well, that's the problem then: we have two POINTER_TYPE to METHOD_TYPE which
are considered equivalent in the front end, but not equivalent in the tree-ssa
types_compatible_p.

The specific bug seems to be that useless_type_conversion_1 doesn't expect to
need to do structural comparison of non-aggregate types.  That's an invalid
assumption; it should check TYPE_STRUCTURAL_EQUALITY_P.  Does the attached
patch seem appropriate to you?  It fixes the bug for me, though I haven't
regression tested it.


-- 

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|2008-08-04 09:18:24         |2008-08-04 21:03:50
               date|                            |


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[rguenther at suse dot de]

------- Comment #13 from rguenther at suse dot de  2008-08-04 21:11 -------
Subject: Re:  [4.3/4.4 Regression] member function pointer
 failure with optimization

On Mon, 4 Aug 2008, jason at gcc dot gnu dot org wrote:

> ------- Comment #12 from jason at gcc dot gnu dot org  2008-08-04 21:03 -------
> Well, that's the problem then: we have two POINTER_TYPE to METHOD_TYPE which
> are considered equivalent in the front end, but not equivalent in the tree-ssa
> types_compatible_p.
> 
> The specific bug seems to be that useless_type_conversion_1 doesn't expect to
> need to do structural comparison of non-aggregate types.  That's an invalid
> assumption; it should check TYPE_STRUCTURAL_EQUALITY_P.  Does the attached
> patch seem appropriate to you?  It fixes the bug for me, though I haven't
> regression tested it.

Duh, if TYPE_STRUCTURAL_EQUALITY_P is used on non-aggregates then yes,
this is a correct patch (the code currently makes FUNCTION_TYPEs and
METHOD_TYPEs distinct if they are not pointer-equivalent or linked
via TYPE_CANONICAL / TYPE_MAIN_VARIANT).  So I would prefer if
you add those to the AGGREGATE_TYPE_P case instead of letting
it fall-through to the "default" case - we should probably add
some gcc_unreachable () to make sure we handled all cases.

Thanks,
Richard.


-- 


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[rguenther at suse dot de]

------- Comment #14 from rguenther at suse dot de  2008-08-04 21:13 -------
Subject: Re:  [4.3/4.4 Regression] member function pointer
 failure with optimization

On Mon, 4 Aug 2008, rguenther at suse dot de wrote:

> ------- Comment #13 from rguenther at suse dot de  2008-08-04 21:11 -------
> Subject: Re:  [4.3/4.4 Regression] member function pointer
>  failure with optimization
> 
>  So I would prefer if
> you add those to the AGGREGATE_TYPE_P case instead of letting
> it fall-through to the "default" case - we should probably add
> some gcc_unreachable () to make sure we handled all cases.

On a second thought this is probably not so good, but a
/* Fallthrough.  */ comment would be nice for the AGGREGATE_TYPE_P case.

Thanks,
Richard.


-- 


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[jason at gcc dot gnu dot org]

------- Comment #15 from jason at gcc dot gnu dot org  2008-08-05 22:23 -------
Subject: Bug 37016

Author: jason
Date: Tue Aug  5 22:22:00 2008
New Revision: 138740

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=138740
Log:
        PR c++/37016
        * tree-ssa.c (useless_type_conversion_p_1): Call langhook
        if TYPE_STRUCTURAL_EQUALITY_P is true for both types.

Added:
    trunk/gcc/testsuite/g++.dg/opt/pmf1.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa.c


-- 


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[jason at gcc dot gnu dot org]

------- Comment #16 from jason at gcc dot gnu dot org  2008-08-06 01:55 -------
Subject: Bug 37016

Author: jason
Date: Wed Aug  6 01:54:31 2008
New Revision: 138755

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=138755
Log:
        PR c++/37016
        * tree-ssa.c (useless_type_conversion_p_1): Call langhook
        if TYPE_STRUCTURAL_EQUALITY_P is true for both types.

Added:
    branches/gcc-4_3-branch/gcc/testsuite/g++.dg/opt/pmf1.C
      - copied unchanged from r138740, trunk/gcc/testsuite/g++.dg/opt/pmf1.C
Modified:
    branches/gcc-4_3-branch/gcc/ChangeLog
    branches/gcc-4_3-branch/gcc/tree-ssa.c


-- 


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[jason at gcc dot gnu dot org]

------- Comment #17 from jason at gcc dot gnu dot org  2008-08-06 02:26 -------
Subject: Bug 37016

Author: jason
Date: Wed Aug  6 02:25:20 2008
New Revision: 138756

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=138756
Log:
        PR c++/37016
        * decl.c (build_ptrmemfunc_type): Don't require structural
        comparison of PMF types.
        * tree.c (cp_build_qualified_type_real): Don't clear
        a valid TYPE_PTRMEMFUNC_TYPE.
        * typeck.c (cp_build_unary_op): Still do build_ptrmemfunc in
        templates.

Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c
    trunk/gcc/cp/tree.c
    trunk/gcc/cp/typeck.c


-- 


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

[Bug c++/37016] [4.3/4.4 Regression] member function pointer failure with optimization

[jason at gcc dot gnu dot org]

------- Comment #18 from jason at gcc dot gnu dot org  2008-08-06 03:14 -------
Fixed in two different ways.


-- 

jason at gcc dot gnu dot org changed:

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


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