[Bug libstdc++/53202] New: Copy constructor not called when starting a thread

[Bug libstdc++/53202] New: Copy constructor not called when starting a thread

[anthony.ajw at gmail dot com]

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

             Bug #: 53202
           Summary: Copy constructor not called when starting a thread
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: anthony.ajw@gmail.com


Created attachment 27291
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27291
Simple test program

When compiling and running the attached program the copy constructor for
background_hello should be called at least once. It is not being called at all,
but the function call operator is showing a different this pointer, and the
destructor is called 3 times.

~$ g++ --version
g++ (Ubuntu/Linaro 4.7.0-7ubuntu1) 4.7.0
Copyright (C) 2012 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

~$ g++ -std=c++0x -pthread t.cpp && ./a.out
default ctor called, this=0x7fffc2b05bdf
destructor called, this=0x7fffc2b05b8f
void background_hello::operator()() called, this=0x922040
destructor called, this=0x922040
destructor called, this=0x7fffc2b05bdf

[Bug libstdc++/53202] Copy constructor not called when starting a thread

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-05-02
         AssignedTo|unassigned at gcc dot       |redi at gcc dot gnu.org
                   |gnu.org                     |
     Ever Confirmed|0                           |1

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-05-02 21:25:19 UTC ---
Ouch.

With 4.6 I see three ctors and three dtors:

default ctor called, this=0x7fffafe073df
copy ctor called
move ctor called
destructor called, this=0x7fffafe073a0
void background_hello::operator()() called, this=0x15f0048
destructor called, this=0x15f0048
destructor called, this=0x7fffafe073df

[Bug c++/53202] Copy constructor not called when starting a thread

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
                 CC|                            |jason at gcc dot gnu.org
          Component|libstdc++                   |c++
         AssignedTo|redi at gcc dot gnu.org     |unassigned at gcc dot
                   |                            |gnu.org

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-05-03 00:45:01 UTC ---
Here's a reduced version that only needs -std=c++11 and not -pthread

#include <tuple>

template<typename Callable>
  struct Bind_simple
  {
    explicit
    Bind_simple(const Callable& callable)
    : _M_bound(callable)
    { }

    Bind_simple(const Bind_simple&) = default;
    Bind_simple(Bind_simple&&) = default;

    auto operator()() -> decltype((*(Callable*)0)())
    {
      return std::get<0>(_M_bound)();
    }

  private:

    std::tuple<Callable> _M_bound;
  };

template<typename Callable>
  Bind_simple<Callable>
  bind_simple(Callable& callable)
  {
    return Bind_simple<Callable>(callable);
  }

struct thread
{
  struct ImplBase { };

  template<typename T>
    struct Impl : ImplBase {
      T t;
      Impl(T&& t) : t(std::move(t)) { }
    };

  template<typename T>
    thread(T& t)
    {
      auto p = make_routine(bind_simple(t));

      p->t();

      delete p;
    }

  template<typename Callable>
    Impl<Callable>*
    make_routine(Callable&& f)
    {
      return new Impl<Callable>(std::forward<Callable>(f));
    }
};


class background_hello
{
public:
    background_hello()
    {
        __builtin_printf("default ctor called, this=%p\n", this);
    }

    background_hello(const background_hello &)
    {
        __builtin_printf("copy ctor called\n");
    }

    background_hello(background_hello &&)
    {
        __builtin_printf("move ctor called\n");
    }

    void operator ()() const
    {
        __builtin_printf("void background_hello::operator()() called,
this=%p\n", this);
    }

    ~background_hello()
    {
        __builtin_printf("destructor called, this=%p\n", this);
    }

};

int main()
{
    background_hello bh;
    thread t(bh);
}

This still produces the wrong result:

default ctor called, this=0x7fffe4a2c3bf
destructor called, this=0x7fffe4a2c38f
void background_hello::operator()() called, this=0xd9d028
destructor called, this=0xd9d028
destructor called, this=0x7fffe4a2c3bf


Looking at the gimple dump I see that the
  thread::Impl<Bind_simple<background_hello()>>
constructor doesn't initialize its Bind_simple member:


thread::Impl<T>::Impl(T&&) [with T = Bind_simple<background_hello>] (struct
Impl * const this, struct Bind_simple & t)
{
  struct Bind_simple * D.8909;

  thread::ImplBase::ImplBase (this);
  try
    {

    }
  catch
    {
      D.8909 = &this->t;
      Bind_simple<background_hello>::~Bind_simple (D.8909);
    }
}


If the Bind_simple object isn't move-constructed then its background_hello
object won't be either, which explains the missing constructor calls.

If Bind_simple<Callable> stores a Callable directly instead of as
tuple<Callable> then the problem goes away and I see three constructors called.


I think at this point it looks like a G++ issue

[Bug c++/53202] Copy constructor not called when starting a thread

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-05-03 01:14:28 UTC ---
If I change some of the functions to forward lvalues instead of using perfect
forwarding then I see one call to a copy constructor

e.g. just changing the thread::Impl ctor to

      Impl(const T& t) : t(t) { }

produces:

default ctor called, this=0x7fffd878f35f
copy ctor called
destructor called, this=0x7fffd878f327
void background_hello::operator()() called, this=0x1258010
destructor called, this=0x1258010
destructor called, this=0x7fffd878f35f

The gimple dump shows:

thread::Impl<T>::Impl(const T&) [with T = Bind_simple<background_hello>]
(struct Impl * const this, const struct Bind_simple & t)
{
  struct Bind_simple * D.8913;
  struct Bind_simple * D.8914;

  thread::ImplBase::ImplBase (this);
  D.8913 = &this->t;
  Bind_simple<background_hello>::Bind_simple (D.8913, t);
  try
    {

    }
  catch
    {
      D.8914 = &this->t;
      Bind_simple<background_hello>::~Bind_simple (D.8914);
    }
}

Note the copy construction of the Bind_simple member that was missing
previously

[Bug c++/53202] Copy constructor not called when starting a thread

[redi at gcc dot gnu.org]

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-05-03 08:22:51 UTC ---
Created attachment 27301
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27301
preprocessed source

This preprocessed source was created by G++ version 4.6.3 20120306 (Red Hat
4.6.3-2)

When compiled with 4.6.3 it gives:

default ctor called, this=0x7fffba05109f
copy ctor called, this=0x7fffba051067
move ctor called, this=0x253d010
destructor called, this=0x7fffba051067
void background_hello::operator()() called, this=0x253d010
destructor called, this=0x253d010
destructor called, this=0x7fffba05109f

When the same preprocessed source is compiled with the 4.7 branch or trunk it
gives:

default ctor called, this=0x7fff8a2f7daf
move ctor called, this=0x1d56010
destructor called, this=0x7fff8a2f7d77
void background_hello::operator()() called, this=0x1d56010
destructor called, this=0x1d56010
destructor called, this=0x7fff8a2f7daf

N.B. there is only one missing constructor here, compared to two when the
original source is compiled with trunk, I guess that's due to a change in
<tuple> between 4.6 and 4.8, but the same preprocessed input should still
produce the same result with both versions, so I think that rules out library
changes being the sole cause of this bug

[Bug c++/53202] Copy constructor not called when starting a thread

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-05-03 08:30:38 UTC ---
For that preprocessed source the difference is in the Bind_simple constructor,
for 4.6 it is:

Bind_simple<Callable>::Bind_simple(const Callable&) [with Callable =
background_hello] (struct Bind_simple * const this, const struct
background_hello & callable)
{
  struct tuple * D.5382;
  struct tuple * D.5383;

  D.5382 = &this->_M_bound;
  std::tuple<background_hello>::tuple (D.5382, callable);
  try
    {

    }
  catch
    {
      D.5383 = &this->_M_bound;
      std::tuple<background_hello>::~tuple (D.5383);
    }
}

But for 4.8 it fails to initialise its member _M_bound:

Bind_simple<Callable>::Bind_simple(const Callable&) [with Callable =
background_hello] (struct Bind_simple * const this, const struct
background_hello & callable)
{
  struct tuple * D.6070;

  try
    {

    }
  catch
    {
      D.6070 = &this->_M_bound;
      std::tuple<background_hello>::~tuple (D.6070);
    }
}

[Bug c++/53202] [4.7/4.8 Regression] Copy constructor not called when starting a thread

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.6.3
            Summary|Copy constructor not called |[4.7/4.8 Regression] Copy
                   |when starting a thread      |constructor not called when
                   |                            |starting a thread
      Known to fail|                            |4.7.1, 4.8.0

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-06-15 00:41:31 UTC ---
Based on comment 4 I'm marking this as a regression since 4.6

[Bug c++/53202] [4.7/4.8 Regression] Copy constructor not called when starting a thread

[hjl.tools at gmail dot com]

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

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.7.2

--- Comment #7 from H.J. Lu <hjl.tools at gmail dot com> 2012-06-15 03:04:39 UTC ---
It is caused by revision 178518:

http://gcc.gnu.org/ml/gcc-cvs/2011-09/msg00129.html

[Bug c++/53202] [4.7/4.8 Regression] Copy constructor not called when starting a thread

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

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

[Bug c++/53202] [4.7/4.8 Regression] Copy constructor not called when starting a thread

[jason at gcc dot gnu.org]

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

--- Comment #8 from Jason Merrill <jason at gcc dot gnu.org> 2012-06-25 15:18:04 UTC ---
Author: jason
Date: Mon Jun 25 15:17:59 2012
New Revision: 188940

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188940
Log:
    PR c++/53202
    * semantics.c (build_data_member_initialization): Always keep
    initializer for empty base.
    (cxx_eval_bare_aggregate): Discard it here.

Added:
    trunk/gcc/testsuite/g++.dg/cpp0x/constexpr-tuple.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/semantics.c
    trunk/gcc/testsuite/ChangeLog

[Bug c++/53202] [4.7/4.8 Regression] Copy constructor not called when starting a thread

[jason at gcc dot gnu.org]

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

--- Comment #9 from Jason Merrill <jason at gcc dot gnu.org> 2012-06-25 15:41:18 UTC ---
Author: jason
Date: Mon Jun 25 15:41:13 2012
New Revision: 188941

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188941
Log:
    PR c++/53202
    * semantics.c (build_data_member_initialization): Always keep
    initializer for empty base.
    (cxx_eval_bare_aggregate): Discard it here.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/g++.dg/cpp0x/constexpr-tuple.C
Modified:
    branches/gcc-4_7-branch/gcc/cp/ChangeLog
    branches/gcc-4_7-branch/gcc/cp/semantics.c
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog

[Bug c++/53202] [4.7/4.8 Regression] Copy constructor not called when starting a thread

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

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

--- Comment #10 from Jason Merrill <jason at gcc dot gnu.org> 2012-06-25 15:45:39 UTC ---
Fixed for 4.7.2