[Bug tree-optimization/16115] New: 3.5: problem with argument passing via temporary (breaks auto_ptr)

[Bug tree-optimization/16115] New: 3.5: problem with argument passing via temporary (breaks auto_ptr)

[gcc-bugzilla at gcc dot gnu dot org]

The code y1.cc below seems to produce the wrong result if it is compiled
with -O2.  (It works ok, however, with either -O1 or -O3.)


$ g++ -O2 -o y1 y1.cc
$ ./y1
a bfffb690
d 0 0
dtor 1 bfffb690
$

In the last line of output, i expect the value of a printed by the destructor
to be 0; however, it is still 1.


Here is the generated code for the foo() and main() functions (passed
through c++filt, omitting exception handling labels and cleanups):

foo(auto_ptr):
	pushl	%ebp
	xorl	%edx, %edx
	movl	%esp, %ebp
	xorl	%eax, %eax
	subl	$24, %esp
	movl	%edx, 8(%esp)
	movl	%eax, 4(%esp)
	movl	$.LC0, (%esp)
	call	printf
	leave
	ret


main:
	pushl	%ebp
	movl	%esp, %ebp
	subl	$40, %esp
	andl	$-16, %esp
	subl	$16, %esp
	movl	%esi, -4(%ebp)
	leal	-24(%ebp), %esi
	movl	%ebx, -8(%ebp)
	movl	$1, -24(%ebp)
	movl	%esi, 4(%esp)
	movl	$.LC1, (%esp)
	call	printf
	movl	%esi, (%esp)
	call	foo(auto_ptr)
	movl	-24(%ebp), %eax
	movl	%esi, 8(%esp)
	movl	$.LC2, (%esp)
	movl	%eax, 4(%esp)
	call	printf
	movl	-8(%ebp), %ebx
	xorl	%eax, %eax
	movl	-4(%ebp), %esi
	movl	%ebp, %esp
	popl	%ebp
	ret
...
.LC0:
	.string	"d %x %x\n"
.LC1:
	.string	"a %x\n"
.LC2:
	.string	"dtor %x %x\n"


It looks like something is going wrong with handling the temporary object
that gets passed to foo().

With gcc 3.4, this code works as expected, so this is a regression.


FYI, here is an earlier stage in the reduction of the test case, with
the auto_ptr class left intact.  Here, the problem shows up as a double
deletion of the allocated object.  Again, it shows up at -O2, but not
at -O1 or -O3.

$ g++ -O2 -o y y.cc
$ ./y
a ctor 804a008
a dtor 804a008
a dtor 804a008
$


y.cc -------------------------------------------
#include <memory>

struct A
{
  A() { printf ("a ctor %x\n", this); }
  ~A() { printf ("a dtor %x\n", this); }
};


void foo (std::auto_ptr<A> a)
{
  delete a.release();
}


int main ()
{
  foo (std::auto_ptr<A> (new A));
}
------------------------------------------------

Environment:
System: Linux karma 2.6.6 #15 Thu May 13 15:07:54 EDT 2004 i686 i686 i386 GNU/Linux
Architecture: i686

	<machine, os, target, libraries (multiple lines)>
host: i686-pc-linux-gnu
build: i686-pc-linux-gnu
target: i686-pc-linux-gnu
configured with: /home/sss/gcc/gcc/configure --prefix=/usr/local/gcc --enable-threads=posix --enable-long-long --enable-languages=c,c++,f95

How-To-Repeat:

Compile the following with -O2 and run:

y1.cc ------------------------------------------
extern "C" int printf(const char*, ...);

struct auto_ptr
{
  int _M_ptr;
  auto_ptr(int p) : _M_ptr(1) { printf ("a %x\n", this); }
  ~auto_ptr() { printf ("dtor %x %x\n", _M_ptr, this); }
  void release() {  _M_ptr = 0;  }
  auto_ptr(const auto_ptr&a) : _M_ptr (a._M_ptr) { printf ("b %x\n", this); }
};


void foo (auto_ptr a)
{
  a.release();
  printf ("d %x %x\n", a._M_ptr, 0);
}

int main ()
{
  foo (0);
}
------------------------------------------------
------- Additional Comments From snyder at fnal dot gov  2004-06-21 16:43 -------
Fix:
	<how to correct or work around the problem, if known (multiple lines)>

-- 
           Summary: 3.5: problem with argument passing via temporary (breaks
                    auto_ptr)
           Product: gcc
           Version: 3.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: snyder at fnal dot gov
                CC: gcc-bugs at gcc dot gnu dot org
 GCC build triplet: i686-pc-linux-gnu
  GCC host triplet: i686-pc-linux-gnu
GCC target triplet: i686-pc-linux-gnu


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

[Bug tree-optimization/16115] [3.5 regression] problem with argument passing via temporary (breaks auto_ptr)

[bangerth at dealii dot org]

------- Additional Comments From bangerth at dealii dot org  2004-06-21 16:55 -------
Confirmed. With this minimally modified testcase 
------------------------ 
#include <memory> 
 
struct A 
{ 
    A() { printf ("a ctor %x\n", this); } 
    A(const A&) { printf ("a copy ctor %x\n", this); } 
    ~A() { printf ("a dtor %x\n", this); } 
}; 
 
 
void foo (std::auto_ptr<A> a) 
{ 
  delete a.release(); 
} 
 
 
int main () 
{ 
  foo (std::auto_ptr<A> (new A)); 
} 
----------------------- 
we can also see that the copy constructor is not called, and that thus 
the number of con- and destructor calls do not match -- bad... 
 
g/x> /home/bangerth/bin/gcc-3.5-pre/bin/c++ x.cc -O2 
g/x> ./a.out  
a ctor 804a008 
a dtor 804a008 
a dtor 804a008 
 
W. 

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
           Keywords|                            |wrong-code
   Last reconfirmed|0000-00-00 00:00:00         |2004-06-21 16:55:31
               date|                            |
            Summary|3.5: problem with argument  |[3.5 regression] problem
                   |passing via temporary       |with argument passing via
                   |(breaks auto_ptr)           |temporary (breaks auto_ptr)
   Target Milestone|---                         |3.5.0


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

[Bug tree-optimization/16115] [3.5 regression] double-destruction problem with argument passing via temporary (breaks auto_ptr)

[bangerth at dealii dot org]

------- Additional Comments From bangerth at dealii dot org  2004-06-21 18:26 -------
Here's a reduced testcase: 
--------------------- 
extern "C" void abort(); 
 
int count = 0; 
 
struct T { 
    T() { count++; } 
    T(const T&) { count++; } 
    ~T() { if (count==0) abort(); --count; } 
}; 
 
struct auto_ptr { 
    T* p; 
 
    auto_ptr(T* __p) : p(__p) { } 
    ~auto_ptr() { delete p; } 
 
    T* release() { 
      T* t = p; 
      p = 0; 
      return t; 
    } 
}; 
 
void destroy (auto_ptr a) { 
  delete a.release(); 
} 
 
 
int main () 
{ 
  destroy (new T); 
} 
----------------------------- 
 
For this, I get 
g/x> /home/bangerth/bin/gcc-3.5-pre/bin/c++ x.cc -O2 && ./a.out  
Aborted 
 
We indeed call operator delete twice, once inside destroy, and once 
inside the destructor of the argument to destroy. However, since release 
sets p=0, the second time we call operator delete we shouldn't get a call 
to the destructor of class T. Alas, we do, thus the abort. 
 
W. 

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[3.5 regression] problem    |[3.5 regression] double-
                   |with argument passing via   |destruction problem with
                   |temporary (breaks auto_ptr) |argument passing via
                   |                            |temporary (breaks auto_ptr)


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

[Bug tree-optimization/16115] [3.5 regression] double-destruction problem with argument passing via temporary (breaks auto_ptr)

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-22 06:34 -------
Richard or Jason (more likely Richard),
This bug is most likely comes from one of your recent patches.
Could either of you two look into it?

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rth at gcc dot gnu dot org,
                   |                            |jason at gcc dot gnu dot org


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

[Bug tree-optimization/16115] [3.5 regression] double-destruction problem with argument passing via temporary (breaks auto_ptr)

[rth at gcc dot gnu dot org]

------- Additional Comments From rth at gcc dot gnu dot org  2004-06-22 08:00 -------
This isn't my bug.  The best I can figure has to do with aliasing changes, but
I'm not even 100% certain that there's bugs there either.  Jason needs to answer
the question of how the ABI works for passing TREE_ADDRESSABLE type arguments.

What has happened is that the aliasing code saw that the address of PARM_DECL a
was taken (this = &a, where this came from inlining of release into destroy).
The optimizers do not, however, consider the PARM_DECL to overlap global memory,
so we are able to delete the store to a.p as dead.

If passing a TREE_ADDRESSABLE type argument is supposed to pass by invisible
reference, and magically overlap with the original temporary, then the tree
level representation of this is wrong.  We should be using a reference type.

My best guess, however, is that the reduced test case is broken.

-- 


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

[Bug tree-optimization/16115] [3.5 regression] double-destruction problem with argument passing via temporary (breaks auto_ptr)

[rth at gcc dot gnu dot org]

------- Additional Comments From rth at gcc dot gnu dot org  2004-06-22 21:08 -------
Jason confirms that Magic is involved with these types.

My plan to fix involves moving the magic so that the reference-ness of these
types such that it is exposed to the optimizers.  Which will allow them to see
that the code currently being deleted is not in fact dead.

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


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

[Bug tree-optimization/16115] [3.5 regression] double-destruction problem with argument passing via temporary (breaks auto_ptr)

[rth at gcc dot gnu dot org]

------- Additional Comments From rth at gcc dot gnu dot org  2004-06-22 21:48 -------
Horrible mangling issues.  Passed to c++ guru.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|rth at gcc dot gnu dot org  |jason at gcc dot gnu dot org


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

[Bug tree-optimization/16115] [3.5 regression] double-destruction problem with argument passing via temporary (breaks auto_ptr)

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-06-24 20:07 -------
Subject: Bug 16115

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	jason@gcc.gnu.org	2004-06-24 20:06:58

Modified files:
	gcc/cp         : ChangeLog decl.c 

Log message:
	PR c++/16115
	* decl.c (grokparms): Give the PARM_DECL reference type if the
	parameter is passed by invisible reference.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.4126&r2=1.4127
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/decl.c.diff?cvsroot=gcc&r1=1.1225&r2=1.1226



-- 


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

[Bug tree-optimization/16115] [3.5 regression] double-destruction problem with argument passing via temporary (breaks auto_ptr)

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-06-24 20:07 -------
Subject: Bug 16115

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	jason@gcc.gnu.org	2004-06-24 20:07:23

Added files:
	gcc/testsuite/g++.dg/init: call1.C 

Log message:
	PR c++/16115
	* decl.c (grokparms): Give the PARM_DECL reference type if the
	parameter is passed by invisible reference.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/init/call1.C.diff?cvsroot=gcc&r1=NONE&r2=1.1



-- 


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

[Bug tree-optimization/16115] [3.5 regression] double-destruction problem with argument passing via temporary (breaks auto_ptr)

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-24 21:08 -------
Fixed.

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


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

[Bug tree-optimization/16115] [3.5 regression] double-destruction problem with argument passing via temporary (breaks auto_ptr)

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-07-06 07:16 -------
Subject: Bug 16115

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	mmitchel@gcc.gnu.org	2004-07-06 07:16:37

Modified files:
	gcc/cp         : ChangeLog decl.c 

Log message:
	Revert:
	2004-06-24  Jason Merrill  <jason@redhat.com>
	PR c++/16115
	* decl.c (grokparms): Give the PARM_DECL reference type if the
	parameter is passed by invisible reference.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.4167&r2=1.4168
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/decl.c.diff?cvsroot=gcc&r1=1.1235&r2=1.1236



-- 


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

[Bug tree-optimization/16115] [3.5 regression] double-destruction problem with argument passing via temporary (breaks auto_ptr)

[mmitchel at gcc dot gnu dot org]

------- Additional Comments From mmitchel at gcc dot gnu dot org  2004-07-06 07:23 -------
This patch is incorrect, on several levels.

First, it changes the type of the PARM_DECL to be different from the type that
C++ assigns that declaration.  That is incorrect; the C++ front end's assignment
of types should match the language.  As we've discussed previously, for example,
it's a bug that we have expressions with REFERENCE_TYPE; the standard says that
no expressions have that type.  (Declarations can, of course, have
REFERENCE_TYPE -- but these parameters do not.)  We should be trying to minimize
the extent to which ABI details appear in the front end.

Furthermore, this test case:

  template <typename T>
  struct S {
    ~S();
  };
                                                                                
  struct T {
    T(S<int>);
                                                                                
    S<int> s_;
  };
                                                                                
  T::T(S<int> s) : s_(s) {
  }

now ICEs the compiler on (at least) powerpc-apple-darwin7.4.0 due to some
problem with cloned functions.

Also, this change is potentially pessimizing code in that it will no longer be
obvious to the optimizers that the static type of the parameter must in fact be
the declared type.  So, we may not be able to resolve virtual function invoked
on the parameter.

Finally, if we were going to make this change in the front end, the right place
would be in grokdeclarator (where the PARM_DECL is created) rather than in
grokparms, where we must then wastefully create a second PARM_DECL.

If the optimizers needs these parameters to have reference type, then that
change needs to happen after the front end processing is complete, in the course
of generating code for this function.  I'm not sure what the tradeoffs are vis a
vis (re)teaching the optimizers about TREE_ADDRESSABLE on types.

I've reverted this patch.  

I'll be happy to help with an alternate solution in whatever way I can.

(Parenthetically, I'm not sure the call1.C test is checking anything useful,
since it does not fail even after I reverted the patch.)

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


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

[Bug tree-optimization/16115] [3.5 regression] double-destruction problem with argument passing via temporary (breaks auto_ptr)

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-07-18 05:44 -------
Subject: Bug 16115

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	jason@gcc.gnu.org	2004-07-18 05:44:20

Modified files:
	gcc            : ChangeLog stor-layout.c tree.h 
	gcc/cp         : ChangeLog call.c cp-gimplify.c decl.c 

Log message:
	PR c++/16115
	* stor-layout.c (relayout_decl): New fn.
	* tree.h: Declare it.
	(DECL_BY_REFERENCE): New macro.
	* cp/call.c (type_passed_as): Make the invisible reference type
	__restrict.
	* cp/cp-gimplify.c (gimplify_cleanup_stmt): Rename to
	cp_genericize_r.  Handle invisible reference lowering.
	(is_invisiref_parm): New fn.
	(cp_genericize): Adjust the types of invisible reference parms.
	Don't repeat the walk for clones.
	* cp/decl.c (store_parm_decls): Don't generate any code for clones.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.4590&r2=2.4591
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/stor-layout.c.diff?cvsroot=gcc&r1=1.195&r2=1.196
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree.h.diff?cvsroot=gcc&r1=1.561&r2=1.562
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.4215&r2=1.4216
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/call.c.diff?cvsroot=gcc&r1=1.494&r2=1.495
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/cp-gimplify.c.diff?cvsroot=gcc&r1=1.6&r2=1.7
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/decl.c.diff?cvsroot=gcc&r1=1.1254&r2=1.1255



-- 


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

[Bug tree-optimization/16115] [3.5 regression] double-destruction problem with argument passing via temporary (breaks auto_ptr)

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-07-18 13:41 -------
Subject: Bug 16115

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	jason@gcc.gnu.org	2004-07-18 13:41:02

Modified files:
	gcc/testsuite/g++.dg/init: call1.C 

Log message:
	PR c++/16115
	* stor-layout.c (relayout_decl): New fn.
	* tree.h: Declare it.
	(DECL_BY_REFERENCE): New macro.
	* cp/call.c (type_passed_as): Make the invisible reference type
	__restrict.
	* cp/cp-gimplify.c (gimplify_cleanup_stmt): Rename to
	cp_genericize_r.  Handle invisible reference lowering.
	(is_invisiref_parm): New fn.
	(cp_genericize): Adjust the types of invisible reference parms.
	Don't repeat the walk for clones.
	* cp/decl.c (store_parm_decls): Don't generate any code for clones.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/init/call1.C.diff?cvsroot=gcc&r1=1.3&r2=1.4



-- 


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

[Bug tree-optimization/16115] [3.5 regression] double-destruction problem with argument passing via temporary (breaks auto_ptr)

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-07-18 14:15 -------
Fixed.

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


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