[Bug c++/50282] New: pointer-to-member cast works incorrectly

[Bug c++/50282] New: pointer-to-member cast works incorrectly

[imzhuli at vip dot qq.com]

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

             Bug #: 50282
           Summary: pointer-to-member cast works incorrectly
    Classification: Unclassified
           Product: gcc
           Version: 4.1.2
            Status: UNCONFIRMED
          Severity: critical
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: imzhuli@vip.qq.com


Created attachment 25180
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25180
compile-info (command line, and g++ output),  t.cpp (source), t.ii

in ISO/IEC 14882 (cpp2003) <also in n3224(cpp0x)>, chapter 5.2.10 about
reinterpret_cast, the rule 9 says:
"
An rvalue of type “pointer to member of X of type T1” can be explicitly
converted to an rvalue of type
“pointer to member of Y of type T2” if T1 and T2 are both function types or
both object types.66) The null
member pointer value (4.11) is converted to the null member pointer value of
the destination type. The
result of this conversion is unspecified, except in the following cases:
— converting an rvalue of type “pointer to member function” to a different
pointer to member function
type and back to its original type yields the original pointer to member value.
— converting an rvalue of type “pointer to data member of X of type T1” to the
type “pointer to data member
of Y of type T2” (where the alignment requirements of T2 are no stricter than
those of T1) and back
to its original type yields the original pointer to member value.
"

but as i tested using G++ 4.1.2 (SUSE LINUX 11) / G++ 4.5.5 (UBUNTU 11.04), 
this is not exactly implemented. 

detailed comments are in the attachments.

attachment:
1.compile info.txt 
2.t.ii /test-case ii file
3.tiny single test-case source-file which includes ONLY c++ standered headers,
detailed commented.

[Bug c++/50282] pointer-to-member cast works incorrectly

[imzhuli at vip dot qq.com]

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

zhuli <imzhuli at vip dot qq.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|critical                    |major

[Bug c++/50282] pointer-to-member cast works incorrectly

[redi at gcc dot gnu.org]

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

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-09-03 08:55:38 UTC ---
uint32_t isn't big enough to hold a pointer-to-member value, and that
conversion isn't valid anyway

[Bug c++/50282] pointer-to-member cast works incorrectly

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|major                       |normal

[Bug c++/50282] pointer-to-member cast works incorrectly

[imzhuli at vip dot qq.com]

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

--- Comment #2 from zhuli <imzhuli at vip dot qq.com> 2011-09-03 11:31:57 UTC ---
I've never used any unint32_t tu hold any pointer-to-member, 
I just tested and find out the sizeof (pointer-to-member-function) is 16, 
then i used a pointer-to-uint32_t four times to dump the value of the
pointer-to-member, 32bits by 32bits.

According my understanding to the standard, this conversion should be valid, or
there must be any valid way . 

(In reply to comment #1)
> uint32_t isn't big enough to hold a pointer-to-member value, and that
> conversion isn't valid anyway

[Bug c++/50282] pointer-to-member cast works incorrectly

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-09-03 12:00:49 UTC ---
(In reply to comment #2)
> I've never used any unint32_t tu hold any pointer-to-member, 
> I just tested and find out the sizeof (pointer-to-member-function) is 16, 
> then i used a pointer-to-uint32_t four times to dump the value of the
> pointer-to-member, 32bits by 32bits.
>
> According my understanding to the standard, this conversion should be valid, or
> there must be any valid way . 

The above violates C++ aliasing rules but would be supported by GCC as an
extension with -O[01] or -fno-strict-aliasing.  The standard only allows
accessing the storage via a character type (thus, dump 16 individual bytes via
a char * pointer).

> (In reply to comment #1)
> > uint32_t isn't big enough to hold a pointer-to-member value, and that
> > conversion isn't valid anyway

[Bug c++/50282] pointer-to-member cast works incorrectly

[imzhuli at vip dot qq.com]

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

--- Comment #4 from zhuli <imzhuli at vip dot qq.com> 2011-09-03 12:17:35 UTC ---
yes, you are right. 
As an addition, compiling with -O2 might yield an warning, as you metioned.

but i checked my env(32-bit) and used default compiler option, so i just simply
assumed 4byte-object & 16byte-object share the same alignment. and as this is
not the point of the issue, i didnt take much care about that. In fact, i
seldom use such code, even to debug.
Anyway, you comment is advisable, thanks.



(In reply to comment #3)
> (In reply to comment #2)
> > I've never used any unint32_t tu hold any pointer-to-member, 
> > I just tested and find out the sizeof (pointer-to-member-function) is 16, 
> > then i used a pointer-to-uint32_t four times to dump the value of the
> > pointer-to-member, 32bits by 32bits.
> >
> > According my understanding to the standard, this conversion should be valid, or
> > there must be any valid way . 
> 
> The above violates C++ aliasing rules but would be supported by GCC as an
> extension with -O[01] or -fno-strict-aliasing.  The standard only allows
> accessing the storage via a character type (thus, dump 16 individual bytes via
> a char * pointer).
> 
> > (In reply to comment #1)
> > > uint32_t isn't big enough to hold a pointer-to-member value, and that
> > > conversion isn't valid anyway

[Bug c++/50282] pointer-to-member cast works incorrectly

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-09-04 18:45:24 UTC ---
The problem is is your code, not gcc

The type of &T4::f is void(T2::*)() not void(T4::*)() so when you cast the
pointer to void(T4::*)() you are not casting back to the original type.

You can fix the code by using reinterpret_cast<void(T2::*)()> for the cases
that use the expression &T4::f, instead of reinterpret_cast<void(T4::*)()>

For what it's worth, clang gives exactly the same result as g++ for a reduced
version of your program:

// define class N with no member,
// we just need a type void(N::*)() of which an object can hold a
pointer-to-member-function
class N {} ;

// define class T1 & T3 with some member data/functions
// so that an object of class T4 which derives from T1 & T2 & T3 
// will has its base object of type T2 have different address from the object
itself.
class T1
{
public:
  char c ;
  int i ; 
} ;

class T3:virtual public T1
{
public:
  int i3 ;
public:
  virtual void f2() {
    __builtin_printf("Foo3 !! this=%p\n", (void*)this);
  } ;
} ;

// define class T2 
// T2 has a memmber function f, which our pointer-to-member-function will point
to.
// as T4 derives from T2 as a public base type, this function is an accessable
from T4 ;
// the function outputs the value of 'this' pointer, which i expect it always
points to an object of T2.
class T2
{
public:
  int i2 ;
public:
  virtual void f() {
    __builtin_printf("Foo2 !! this=%p\n", (void*)this);
  } ;
} ;

// define class T4, which simply derives from T1&T2&T3
class T4:public virtual T1, public T3, public T2
{} ;



int main(int, char**)
{
  T4 t4 ;
  void (N::*pfn)() = 0;
  void(T4::*pf4)() = &T4::f ;

  // this line shows the address of t4 and its base object t4.t2 differ ;
  __builtin_printf("AddressOf t4=%p, t4.t2=%p\n", (void*)(&t4),
(void*)(&(T2&)(t4)));

  {
    __builtin_printf("\nwhat i expect:\n");
    // the following lines show what i expect to see:
    // no matter what form the function call is, the function tells me the
address of t4.t2
    t4.f() ;
    (t4.*(&T4::f))() ;
    (t4.*pf4)() ;
  }

  {    
    __builtin_printf("\nTestCase1:\n");
    __builtin_printf("pfn assignment: pfn =
reinterpret_cast<void(N::*)()>(&T4::f) \n");
    __builtin_printf("Function call form: (t4.*reinterpret_cast<void(T4::*)()>(
pfn )() \n");
    // Case1: 
    // pfn is assigned directly from &T4::f, 
    // but actually, its value shows that is &T2::f
    pfn= reinterpret_cast<void(N::*)()>(&T4::f) ;

    // comparing with the result i memtioned above, this is not what i want.
    (t4.*reinterpret_cast<void(T4::*)()>(pfn))() ;
  }

  {
    __builtin_printf("\nTestCase2:\n");
    __builtin_printf( "pfn assignment: pfn =
reinterpret_cast<void(N::*)()>(pf4) \n");
    __builtin_printf("Function call form: (t4.*reinterpret_cast<void(T4::*)()>(
pfn )() \n");
    // Case2: pfn is transfromed from pf4, which is defined as type
void(T4::*)(), and its value
    // has been correctly assigned as &T4::f, not &T2::f
    pfn = reinterpret_cast<void(N::*)()>(pf4) ;

    // this time, the function call works correctly
    (t4.*reinterpret_cast<void(T4::*)()>(pfn))() ;
  }

  {
    __builtin_printf("\nTestCase3:\n");
    // Case3:
    // this case exactly follows the standered draft, if anyone might say
    // in the above casese i used a lvalue(pfn), 
    // this time, i have rvalue only, but it's obviously not functioning.
    __builtin_printf("Function call form: (t4.*reinterpret_cast<void(T4::*)()>(
reinterpret_cast<void(N::*)()>(&T4::f)))() \n");
    (t4.*reinterpret_cast<void(T4::*)()>(
                                         reinterpret_cast<void(N::*)()>(&T4::f)
                                        ))() ;
  }
}


When the 1st and 3rd tests are altered to use reinterpret_cast<void(T2::*)()>
it gives the results you expect

so I think this is invalid

[Bug c++/50282] pointer-to-member cast works incorrectly

[imzhuli at vip dot qq.com]

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

--- Comment #6 from zhuli <imzhuli at vip dot qq.com> 2011-09-05 03:12:55 UTC ---
I guess we have some agreement on HOW gcc works. the following are what i got
through the test-cases:
1.simply using &T4::f, will only get a value of T2::f, as the function f
accessed through T4,is actually defined in T2. NO CONVERSION WILL TAKE PLACE.
2.directly assign &T4::f to an object of type void(T4::*)(void), will
IMPLICITELY cause an conversion from void(T2::*)(void) to void (T4::*)(void), 

That means:
If I want to store a pointer-to-member-function, i should use its original
type, as T2::* ,or T4::T2::* which is only verbose but makes no difference in
current g++ implementation), in the case, and that's what i usually have to
have done in my programs (but not in the test-cases).

BUT:
1.This form is anti-literal. becaouse the programmer should always be aware of
where the function is implemented so as to store it in a generic way, or he
should use template.
2.To be more anti-literal, considering the following cases:
   assume i implement a function T::f4, and f4 is originally a member-function
of T4, and i give two assignment:
  void (T2::*pf2)() = &T4::f ;  // 1> OK, because f is implemented in T2
  void (T2::*pf42)() = &T4::f4 ; // 2> Error, because f4 is implemented in T4
as we both know how gcc works, and we know the second assignment voilabe the
c++ standard about "No casting from D::f to B::f", so i say we feel no strange
about the result, but IT IS STILL CONFUSING, right ? event option -Wall will
not yield any warning or suggesting about the first line. At least I think such
a warning is much useful than a warning like "unused variable".

So, I can accepte the way gcc implements the conversions, for I just tested
several cases and i can find out which way I can follow. But I have some
suggestions: 

For the compiler always knows from the codes that f is accessable through T4,
we can have two forms of assign &T4::f to objects of type T4::*, 
1. (void)(T4::*pf4)() = &T4::T2::f,  or
2, (void)(T4::*pf4)() = static_cast<void (T4::*)()>(&T2::f) ;
and:
3 (void)(T4::*pf4)() = &T2::f, will yield an warning. 

I thinks this will make programmers feel easier and happier.


(In reply to comment #5)
> The problem is is your code, not gcc
> The type of &T4::f is void(T2::*)() not void(T4::*)() so when you cast the
> pointer to void(T4::*)() you are not casting back to the original type.
> You can fix the code by using reinterpret_cast<void(T2::*)()> for the cases
> that use the expression &T4::f, instead of reinterpret_cast<void(T4::*)()>
> For what it's worth, clang gives exactly the same result as g++ for a reduced
> version of your program:
> // define class N with no member,
> // we just need a type void(N::*)() of which an object can hold a
> pointer-to-member-function
> class N {} ;
> // define class T1 & T3 with some member data/functions
> // so that an object of class T4 which derives from T1 & T2 & T3 
> // will has its base object of type T2 have different address from the object
> itself.
> class T1
> {
> public:
>   char c ;
>   int i ; 
> } ;
> class T3:virtual public T1
> {
> public:
>   int i3 ;
> public:
>   virtual void f2() {
>     __builtin_printf("Foo3 !! this=%p\n", (void*)this);
>   } ;
> } ;
> // define class T2 
> // T2 has a memmber function f, which our pointer-to-member-function will point
> to.
> // as T4 derives from T2 as a public base type, this function is an accessable
> from T4 ;
> // the function outputs the value of 'this' pointer, which i expect it always
> points to an object of T2.
> class T2
> {
> public:
>   int i2 ;
> public:
>   virtual void f() {
>     __builtin_printf("Foo2 !! this=%p\n", (void*)this);
>   } ;
> } ;
> // define class T4, which simply derives from T1&T2&T3
> class T4:public virtual T1, public T3, public T2
> {} ;
> int main(int, char**)
> {
>   T4 t4 ;
>   void (N::*pfn)() = 0;
>   void(T4::*pf4)() = &T4::f ;
>   // this line shows the address of t4 and its base object t4.t2 differ ;
>   __builtin_printf("AddressOf t4=%p, t4.t2=%p\n", (void*)(&t4),
> (void*)(&(T2&)(t4)));
>   {
>     __builtin_printf("\nwhat i expect:\n");
>     // the following lines show what i expect to see:
>     // no matter what form the function call is, the function tells me the
> address of t4.t2
>     t4.f() ;
>     (t4.*(&T4::f))() ;
>     (t4.*pf4)() ;
>   }
>   {    
>     __builtin_printf("\nTestCase1:\n");
>     __builtin_printf("pfn assignment: pfn =
> reinterpret_cast<void(N::*)()>(&T4::f) \n");
>     __builtin_printf("Function call form: (t4.*reinterpret_cast<void(T4::*)()>(
> pfn )() \n");
>     // Case1: 
>     // pfn is assigned directly from &T4::f, 
>     // but actually, its value shows that is &T2::f
>     pfn= reinterpret_cast<void(N::*)()>(&T4::f) ;
>     // comparing with the result i memtioned above, this is not what i want.
>     (t4.*reinterpret_cast<void(T4::*)()>(pfn))() ;
>   }
>   {
>     __builtin_printf("\nTestCase2:\n");
>     __builtin_printf( "pfn assignment: pfn =
> reinterpret_cast<void(N::*)()>(pf4) \n");
>     __builtin_printf("Function call form: (t4.*reinterpret_cast<void(T4::*)()>(
> pfn )() \n");
>     // Case2: pfn is transfromed from pf4, which is defined as type
> void(T4::*)(), and its value
>     // has been correctly assigned as &T4::f, not &T2::f
>     pfn = reinterpret_cast<void(N::*)()>(pf4) ;
>     // this time, the function call works correctly
>     (t4.*reinterpret_cast<void(T4::*)()>(pfn))() ;
>   }
>   {
>     __builtin_printf("\nTestCase3:\n");
>     // Case3:
>     // this case exactly follows the standered draft, if anyone might say
>     // in the above casese i used a lvalue(pfn), 
>     // this time, i have rvalue only, but it's obviously not functioning.
>     __builtin_printf("Function call form: (t4.*reinterpret_cast<void(T4::*)()>(
> reinterpret_cast<void(N::*)()>(&T4::f)))() \n");
>     (t4.*reinterpret_cast<void(T4::*)()>(
>                                          reinterpret_cast<void(N::*)()>(&T4::f)
>                                         ))() ;
>   }
> }
> When the 1st and 3rd tests are altered to use reinterpret_cast<void(T2::*)()>
> it gives the results you expect
> so I think this is invalid

[Bug c++/50282] pointer-to-member cast works incorrectly

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-09-05 08:08:08 UTC ---
(In reply to comment #6)
> I guess we have some agreement on HOW gcc works. the following are what i got
> through the test-cases:
> 1.simply using &T4::f, will only get a value of T2::f, as the function f
> accessed through T4,is actually defined in T2. NO CONVERSION WILL TAKE PLACE.
> 2.directly assign &T4::f to an object of type void(T4::*)(void), will
> IMPLICITELY cause an conversion from void(T2::*)(void) to void (T4::*)(void), 

Yes, the point is that implicit conversion is safe, and done correctly by the
compiler, which knows what it is doing.  Your uses of reinterpret_cast were not
all safe, and lie to the compiler - you should always be very careful when
using reinterpret_cast.

> That means:
> If I want to store a pointer-to-member-function, i should use its original
> type, as T2::* ,or T4::T2::* which is only verbose but makes no difference in
> current g++ implementation), in the case, and that's what i usually have to
> have done in my programs (but not in the test-cases).

You can make use of the implicit conversion to void(T4::*)() BEFORE you cast to
void(N::*)(), and then it is safe to cast BACK to void(T4::*)().  That's what
your TestCase2 does.
That implicitly converts from P1 to P2 which is safe, then you use
reinterpret_cast to go from P2 to P3 and back to P2, which is OK

What you cannot do is reinterpret_cast from P1 to P3 to P2 and expect the right
result, because you have not cast back to the original type.

> So, I can accepte the way gcc implements the conversions, for I just tested
> several cases and i can find out which way I can follow. But I have some
> suggestions: 
> 
> For the compiler always knows from the codes that f is accessable through T4,
> we can have two forms of assign &T4::f to objects of type T4::*, 
> 1. (void)(T4::*pf4)() = &T4::T2::f,  or
> 2, (void)(T4::*pf4)() = static_cast<void (T4::*)()>(&T2::f) ;
> and:
> 3 (void)(T4::*pf4)() = &T2::f, will yield an warning. 
> 
> I thinks this will make programmers feel easier and happier.

Did you mean &T4::f in the third case?  If not, it wouldn't make me happier,
it's obvious that there is an implicit conversion, because the declarator says
"T4" and the initializer says "T2"

Closing this as invalid, if you want an enhancement for a new warning please
file a separate request with a clear explanation of what you want.