* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
@ 2021-12-29 10:12 ` redi at gcc dot gnu.org
2021-12-29 10:22 ` redi at gcc dot gnu.org
` (12 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: redi at gcc dot gnu.org @ 2021-12-29 10:12 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2021-12-29
Status|UNCONFIRMED |NEW
Ever confirmed|0 |1
--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Pavel I. Kryukov from comment #0)
> C++ standard does not require 'do nothing' if std::forward_list::merge
> argument equals to 'this', however it does not say 'undefined behavior' or
> 'infinite loop' too.
It does say undefined behaviour for the overload your patch changes.
[res.on.arguments] says:
> If a function argument is bound to an rvalue reference parameter, the
> implementation may assume that this parameter is a unique reference to this
> argument, except that the argument passed to a move-assignment operator may
> be a reference to *this ([lib.types.movedfrom]).
Passing *this as the first parameter of merge(forward_list&&) or
merge(forward_list&&, Comp) breaks this assumption, resulting in undefined
behaviour.
The check is only needed for the overloads taking forward_list& parameters:
--- a/libstdc++-v3/include/bits/forward_list.h
+++ b/libstdc++-v3/include/bits/forward_list.h
@@ -1267,7 +1267,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
merge(forward_list& __list)
- { merge(std::move(__list)); }
+ {
+ if (std::__addressof(__list) == this)
+ return;
+
+ merge(std::move(__list));
+ }
/**
* @brief Merge sorted lists according to comparison function.
@@ -1287,7 +1292,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
template<typename _Comp>
void
merge(forward_list& __list, _Comp __comp)
- { merge(std::move(__list), __comp); }
+ {
+ if (std::__addressof(__list) == this)
+ return;
+
+ merge(std::move(__list), __comp);
+ }
/**
* @brief Sort the elements of the list.
> I've attached my patch to fix and test it, but since I'm not an experienced
> GCC contributor, I don't know how to run the tests. Following instructions
> on CONTRIBUTING page builds whole GCC ending with some problems with my
> machine. I would be grateful if you tell how I can test only libstdc++.
You need to build the whole of GCC to test libstdc++, so if you can't get that
working, you should fix that first.
Testing libstdc++ is documented at
https://gcc.gnu.org/onlinedocs/libstdc++/manual/test.html#test.run
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
2021-12-29 10:12 ` [Bug libstdc++/103853] " redi at gcc dot gnu.org
@ 2021-12-29 10:22 ` redi at gcc dot gnu.org
2022-01-05 10:06 ` redi at gcc dot gnu.org
` (11 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: redi at gcc dot gnu.org @ 2021-12-29 10:22 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The new test should also check the overload taking a comparison function.
And if we do want to add the check for the rvalue overloads (which isn't
required, but might be more user friendly, with a small overhead) those should
be checked in the test as well.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
2021-12-29 10:12 ` [Bug libstdc++/103853] " redi at gcc dot gnu.org
2021-12-29 10:22 ` redi at gcc dot gnu.org
@ 2022-01-05 10:06 ` redi at gcc dot gnu.org
2022-01-05 10:48 ` redi at gcc dot gnu.org
` (10 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: redi at gcc dot gnu.org @ 2022-01-05 10:06 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
N.B. this is https://cplusplus.github.io/LWG/issue3088
Currently the standard actually requires that x.merge(x) does x.clear() which
is obviously not what we want.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
` (2 preceding siblings ...)
2022-01-05 10:06 ` redi at gcc dot gnu.org
@ 2022-01-05 10:48 ` redi at gcc dot gnu.org
2022-01-05 14:40 ` pavel.kryukov at phystech dot edu
` (9 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: redi at gcc dot gnu.org @ 2022-01-05 10:48 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Pavel, I'm going to use your original patch, or parts of it anyway (the
addition to the changelog is wrong, we do not patch the changelog by hand it's
auto-generated).
Are you willing and able to certify the origin of the code under the terms of
https://gcc.gnu.org/dco.html ?
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
` (3 preceding siblings ...)
2022-01-05 10:48 ` redi at gcc dot gnu.org
@ 2022-01-05 14:40 ` pavel.kryukov at phystech dot edu
2022-01-05 14:47 ` redi at gcc dot gnu.org
` (8 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: pavel.kryukov at phystech dot edu @ 2022-01-05 14:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
--- Comment #5 from Pavel I. Kryukov <pavel.kryukov at phystech dot edu> ---
> Are you willing and able to certify the origin of the code under the terms of https://gcc.gnu.org/dco.html ?
Yes. Do I need to resubmit a signed-off patch?
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
` (4 preceding siblings ...)
2022-01-05 14:40 ` pavel.kryukov at phystech dot edu
@ 2022-01-05 14:47 ` redi at gcc dot gnu.org
2022-01-06 10:50 ` pavel.kryukov at phystech dot edu
` (7 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: redi at gcc dot gnu.org @ 2022-01-05 14:47 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I think I can probably just add the Signed-off-by: tag based on your comment
above, and push it. But it wouldn't hurt to attach a signed-off patch here.
Thanks!
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
` (5 preceding siblings ...)
2022-01-05 14:47 ` redi at gcc dot gnu.org
@ 2022-01-06 10:50 ` pavel.kryukov at phystech dot edu
2022-01-06 15:00 ` cvs-commit at gcc dot gnu.org
` (6 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: pavel.kryukov at phystech dot edu @ 2022-01-06 10:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
Pavel I. Kryukov <pavel.kryukov at phystech dot edu> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #52080|0 |1
is obsolete| |
--- Comment #7 from Pavel I. Kryukov <pavel.kryukov at phystech dot edu> ---
Created attachment 52133
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52133&action=edit
check if __list==this in std::forward_list::merge
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
` (6 preceding siblings ...)
2022-01-06 10:50 ` pavel.kryukov at phystech dot edu
@ 2022-01-06 15:00 ` cvs-commit at gcc dot gnu.org
2022-01-06 15:39 ` redi at gcc dot gnu.org
` (5 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-01-06 15:00 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
--- Comment #8 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:52ebc2be0990d6d3a46bb716164f9cef6f661021
commit r12-6284-g52ebc2be0990d6d3a46bb716164f9cef6f661021
Author: Pavel I. Kryukov <pavel.kryukov@phystech.edu>
Date: Thu Jan 6 12:32:36 2022 +0000
libstdc++: Add self-merge check to std::forward_list::merge [PR103853]
This implements the proposed resolution of LWG 3088, so that x.merge(x)
is a no-op, consistent with std::list::merge.
Signed-off-by: Pavel I. Kryukov <pavel.kryukov@phystech.edu>
Co-authored-by: Jonathan Wakely <jwakely@redhat.com>
libstdc++-v3/ChangeLog:
PR libstdc++/103853
* include/bits/forward_list.tcc (forward_list::merge): Check for
self-merge.
* testsuite/23_containers/forward_list/operations/merge.cc: New
test.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
` (7 preceding siblings ...)
2022-01-06 15:00 ` cvs-commit at gcc dot gnu.org
@ 2022-01-06 15:39 ` redi at gcc dot gnu.org
2022-01-06 19:02 ` pavel.kryukov at phystech dot edu
` (4 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: redi at gcc dot gnu.org @ 2022-01-06 15:39 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Thanks! This is fixed on trunk now, and will be backported to the release
branches soon.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
` (8 preceding siblings ...)
2022-01-06 15:39 ` redi at gcc dot gnu.org
@ 2022-01-06 19:02 ` pavel.kryukov at phystech dot edu
2022-04-21 12:33 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: pavel.kryukov at phystech dot edu @ 2022-01-06 19:02 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
--- Comment #10 from Pavel I. Kryukov <pavel.kryukov at phystech dot edu> ---
Thanks a lot!
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
` (9 preceding siblings ...)
2022-01-06 19:02 ` pavel.kryukov at phystech dot edu
@ 2022-04-21 12:33 ` cvs-commit at gcc dot gnu.org
2022-04-26 13:11 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-04-21 12:33 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
--- Comment #11 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:2089886d5743c20ce0b41d68f0a4cbe097b46704
commit r11-9909-g2089886d5743c20ce0b41d68f0a4cbe097b46704
Author: Pavel I. Kryukov <pavel.kryukov@phystech.edu>
Date: Thu Jan 6 12:32:36 2022 +0000
libstdc++: Add self-merge check to std::forward_list::merge [PR103853]
This implements the proposed resolution of LWG 3088, so that x.merge(x)
is a no-op, consistent with std::list::merge.
Signed-off-by: Pavel I. Kryukov <pavel.kryukov@phystech.edu>
Co-authored-by: Jonathan Wakely <jwakely@redhat.com>
libstdc++-v3/ChangeLog:
PR libstdc++/103853
* include/bits/forward_list.tcc (forward_list::merge): Check for
self-merge.
* testsuite/23_containers/forward_list/operations/merge.cc: New
test.
(cherry picked from commit 52ebc2be0990d6d3a46bb716164f9cef6f661021)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
` (10 preceding siblings ...)
2022-04-21 12:33 ` cvs-commit at gcc dot gnu.org
@ 2022-04-26 13:11 ` cvs-commit at gcc dot gnu.org
2022-05-09 16:40 ` cvs-commit at gcc dot gnu.org
2022-05-09 19:45 ` redi at gcc dot gnu.org
13 siblings, 0 replies; 15+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-04-26 13:11 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
--- Comment #12 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:93d720f0ec7651e6a402dda33553817c1ad14ded
commit r10-10559-g93d720f0ec7651e6a402dda33553817c1ad14ded
Author: Pavel I. Kryukov <pavel.kryukov@phystech.edu>
Date: Thu Jan 6 12:32:36 2022 +0000
libstdc++: Add self-merge check to std::forward_list::merge [PR103853]
This implements the proposed resolution of LWG 3088, so that x.merge(x)
is a no-op, consistent with std::list::merge.
Signed-off-by: Pavel I. Kryukov <pavel.kryukov@phystech.edu>
Co-authored-by: Jonathan Wakely <jwakely@redhat.com>
libstdc++-v3/ChangeLog:
PR libstdc++/103853
* include/bits/forward_list.tcc (forward_list::merge): Check for
self-merge.
* testsuite/23_containers/forward_list/operations/merge.cc: New
test.
(cherry picked from commit 52ebc2be0990d6d3a46bb716164f9cef6f661021)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
` (11 preceding siblings ...)
2022-04-26 13:11 ` cvs-commit at gcc dot gnu.org
@ 2022-05-09 16:40 ` cvs-commit at gcc dot gnu.org
2022-05-09 19:45 ` redi at gcc dot gnu.org
13 siblings, 0 replies; 15+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-05-09 16:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:
https://gcc.gnu.org/g:79b7b824346d3cb3101d1896f81ed16392b7471c
commit r9-10056-g79b7b824346d3cb3101d1896f81ed16392b7471c
Author: Pavel I. Kryukov <pavel.kryukov@phystech.edu>
Date: Thu Jan 6 12:32:36 2022 +0000
libstdc++: Add self-merge check to std::forward_list::merge [PR103853]
This implements the proposed resolution of LWG 3088, so that x.merge(x)
is a no-op, consistent with std::list::merge.
Signed-off-by: Pavel I. Kryukov <pavel.kryukov@phystech.edu>
Co-authored-by: Jonathan Wakely <jwakely@redhat.com>
libstdc++-v3/ChangeLog:
PR libstdc++/103853
* include/bits/forward_list.tcc (forward_list::merge): Check for
self-merge.
* testsuite/23_containers/forward_list/operations/merge.cc: New
test.
(cherry picked from commit 52ebc2be0990d6d3a46bb716164f9cef6f661021)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Bug libstdc++/103853] std::forward_list::merge should check if __list != this
2021-12-28 20:43 [Bug libstdc++/103853] New: std::forward_list::merge should check if __list != this pavel.kryukov at phystech dot edu
` (12 preceding siblings ...)
2022-05-09 16:40 ` cvs-commit at gcc dot gnu.org
@ 2022-05-09 19:45 ` redi at gcc dot gnu.org
13 siblings, 0 replies; 15+ messages in thread
From: redi at gcc dot gnu.org @ 2022-05-09 19:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103853
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |9.5
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for 9.5, 10.4, 11.4 and 12.1
^ permalink raw reply [flat|nested] 15+ messages in thread