[Bug libstdc++/108118] New: std::weak_ptr lacks a self-usurp check on move

[Bug libstdc++/108118] New: std::weak_ptr lacks a self-usurp check on move

[b.stanimirov at abv dot bg]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108118

            Bug ID: 108118
           Summary: std::weak_ptr lacks a self-usurp check on move
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: b.stanimirov at abv dot bg
  Target Milestone: ---

If one somehow move-assigns a weak pointer to itself, it gets destroyed.

Demo: https://godbolt.org/z/eWdhYMhYe

[Bug libstdc++/108118] std::weak_ptr lacks a self-usurp check on move

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108118

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 54102
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54102&action=edit
testcase

Please next time attach the testcase rather than just linking to godbolt.

[Bug libstdc++/108118] std::weak_ptr lacks a self-usurp check on move

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108118

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Basically you have:
w = std::move(w);

I think that is just undefined code at this point ... But I am not 100% sure.

[Bug libstdc++/108118] std::weak_ptr lacks a self-usurp check on move

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108118

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Unless https://cplusplus.github.io/LWG/issue2315 is not implemented ...

[Bug libstdc++/108118] std::weak_ptr lacks a self-usurp check on move

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108118

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #2)
> Basically you have:
> w = std::move(w);
> 
> I think that is just undefined code at this point ... But I am not 100% sure.

Nope, it's allowed. But it's not required to be a no-op.

(In reply to Andrew Pinski from comment #3)
> Unless https://cplusplus.github.io/LWG/issue2315 is not implemented ...

That was done years ago: r5-4842-gf871d7f9f458d8

[Bug libstdc++/108118] std::weak_ptr lacks a self-usurp check on move

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108118

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-12-15
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Actually for this type the standard does require it to be effectively a no-op:

weak_ptr& operator=(weak_ptr&& r) noexcept;

Effects: Equivalent to weak_ptr(std::move(r)).swap(*this).

[Bug libstdc++/108118] std::weak_ptr lacks a self-usurp check on move

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108118

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:92eb0adc14a5f84acce7e5bc780b81b1544b24aa

commit r13-4745-g92eb0adc14a5f84acce7e5bc780b81b1544b24aa
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Dec 15 09:52:48 2022 +0000

    libstdc++: Fix self-move for std::weak_ptr [PR108118]

    I think an alternative fix would be something like:

      _M_ptr = std::exchange(rhs._M_ptr, nullptr);
      _M_refcount = std::move(rhs._M_refcount);

    The standard's move-and-swap implementation generates smaller code at
    all levels except -O0 and -Og, so it seems simplest to just do what the
    standard says.

    libstdc++-v3/ChangeLog:

            PR libstdc++/108118
            * include/bits/shared_ptr_base.h (weak_ptr::operator=):
            Implement as move-and-swap exactly as specified in the standard.
            * testsuite/20_util/weak_ptr/cons/self_move.cc: New test.

[Bug libstdc++/108118] std::weak_ptr lacks a self-usurp check on move

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108118

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.4
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

[Bug libstdc++/108118] std::weak_ptr lacks a self-usurp check on move

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108118

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:629ed996f32cb03dd712789eede1f7f2036e497b

commit r12-9351-g629ed996f32cb03dd712789eede1f7f2036e497b
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Dec 15 09:52:48 2022 +0000

    libstdc++: Fix self-move for std::weak_ptr [PR108118]

    I think an alternative fix would be something like:

      _M_ptr = std::exchange(rhs._M_ptr, nullptr);
      _M_refcount = std::move(rhs._M_refcount);

    The standard's move-and-swap implementation generates smaller code at
    all levels except -O0 and -Og, so it seems simplest to just do what the
    standard says.

    libstdc++-v3/ChangeLog:

            PR libstdc++/108118
            * include/bits/shared_ptr_base.h (weak_ptr::operator=):
            Implement as move-and-swap exactly as specified in the standard.
            * testsuite/20_util/weak_ptr/cons/self_move.cc: New test.

    (cherry picked from commit 92eb0adc14a5f84acce7e5bc780b81b1544b24aa)

[Bug libstdc++/108118] std::weak_ptr lacks a self-usurp check on move

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108118

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:9578570e305e77b8c105583506781158c137f743

commit r11-10768-g9578570e305e77b8c105583506781158c137f743
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Dec 15 09:52:48 2022 +0000

    libstdc++: Fix self-move for std::weak_ptr [PR108118]

    I think an alternative fix would be something like:

      _M_ptr = std::exchange(rhs._M_ptr, nullptr);
      _M_refcount = std::move(rhs._M_refcount);

    The standard's move-and-swap implementation generates smaller code at
    all levels except -O0 and -Og, so it seems simplest to just do what the
    standard says.

    libstdc++-v3/ChangeLog:

            PR libstdc++/108118
            * include/bits/shared_ptr_base.h (weak_ptr::operator=):
            Implement as move-and-swap exactly as specified in the standard.
            * testsuite/20_util/weak_ptr/cons/self_move.cc: New test.

    (cherry picked from commit 92eb0adc14a5f84acce7e5bc780b81b1544b24aa)

[Bug libstdc++/108118] std::weak_ptr lacks a self-usurp check on move

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108118

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

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

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for 11.4, 12.3 and 13.1

[Bug libstdc++/108118] std::weak_ptr lacks a self-usurp check on move

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108118

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:b272bc0fbf84454cae8c02e7404cfb23641380cd

commit r10-11469-gb272bc0fbf84454cae8c02e7404cfb23641380cd
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Dec 15 09:52:48 2022 +0000

    libstdc++: Fix self-move for std::weak_ptr [PR108118]

    I think an alternative fix would be something like:

      _M_ptr = std::exchange(rhs._M_ptr, nullptr);
      _M_refcount = std::move(rhs._M_refcount);

    The standard's move-and-swap implementation generates smaller code at
    all levels except -O0 and -Og, so it seems simplest to just do what the
    standard says.

    libstdc++-v3/ChangeLog:

            PR libstdc++/108118
            * include/bits/shared_ptr_base.h (weak_ptr::operator=):
            Implement as move-and-swap exactly as specified in the standard.
            * testsuite/20_util/weak_ptr/cons/self_move.cc: New test.

    (cherry picked from commit 92eb0adc14a5f84acce7e5bc780b81b1544b24aa)

[Bug libstdc++/108118] std::weak_ptr lacks a self-usurp check on move

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108118

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.4                        |10.5

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Backported for 10.5 too.