public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug libstdc++/99537] New: Wrong memory_order used in stop_token ref-counting
@ 2021-03-11 2:00 lewissbaker.opensource at gmail dot com
2021-03-11 13:34 ` [Bug libstdc++/99537] " redi at gcc dot gnu.org
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: lewissbaker.opensource at gmail dot com @ 2021-03-11 2:00 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99537
Bug ID: 99537
Summary: Wrong memory_order used in stop_token ref-counting
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: libstdc++
Assignee: unassigned at gcc dot gnu.org
Reporter: lewissbaker.opensource at gmail dot com
Target Milestone: ---
In the implementation of stop_token the _Stop_state_t implements
reference-counting for tracking shared ownership of the stop-state.
This is done via two methods:
void
_M_add_owner() noexcept
{
_M_owners.fetch_add(1, memory_order::relaxed);
}
void
_M_release_ownership() noexcept
{
if (_M_owners.fetch_sub(1, memory_order::release) == 1)
delete this;
}
Other than initialising the _M_owners atomic value to 1, these are the only two
accesses of the _M_owners variable.
The 'fetch_sub()' operation in _M_release_ownership() should be using
memory_order::acq_rel instead of memory_order::release. The use of 'release'
only is insufficient as it does not synchronise with any corresponding
'acquire' operation.
With the current implementation, it's possible that a prior write to one of the
_M_value or _M_head data-members by a thread releasing the second-to-last
reference might not be visible to another thread that releases the last
reference and frees the memory, resulting in potential write to freed memory.
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug libstdc++/99537] Wrong memory_order used in stop_token ref-counting
2021-03-11 2:00 [Bug libstdc++/99537] New: Wrong memory_order used in stop_token ref-counting lewissbaker.opensource at gmail dot com
@ 2021-03-11 13:34 ` redi at gcc dot gnu.org
2021-03-11 17:53 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: redi at gcc dot gnu.org @ 2021-03-11 13:34 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99537
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2021-03-11
Ever confirmed|0 |1
Status|UNCONFIRMED |NEW
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug libstdc++/99537] Wrong memory_order used in stop_token ref-counting
2021-03-11 2:00 [Bug libstdc++/99537] New: Wrong memory_order used in stop_token ref-counting lewissbaker.opensource at gmail dot com
2021-03-11 13:34 ` [Bug libstdc++/99537] " redi at gcc dot gnu.org
@ 2021-03-11 17:53 ` cvs-commit at gcc dot gnu.org
2021-03-11 17:53 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-03-11 17:53 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99537
--- Comment #1 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:8cfb387388a90730ab36ac24d9049677db633a11
commit r11-7628-g8cfb387388a90730ab36ac24d9049677db633a11
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Mar 11 16:43:51 2021 +0000
libstdc++: Handle EPERM for filesystem access errors on MacOS [PR 99537]
Contrary to what POSIX says, some directory operations on MacOS can fail
with EPERM instead of EACCES, so we need to handle both.
libstdc++-v3/ChangeLog:
PR libstdc++/99537
* src/c++17/fs_dir.cc (recursive_directory_iterator): Use new
helper function to check for permission denied errors.
* src/filesystem/dir.cc (recursive_directory_iterator):
Likewise.
* src/filesystem/dir-common.h (is_permission_denied_error): New
helper function.
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug libstdc++/99537] Wrong memory_order used in stop_token ref-counting
2021-03-11 2:00 [Bug libstdc++/99537] New: Wrong memory_order used in stop_token ref-counting lewissbaker.opensource at gmail dot com
2021-03-11 13:34 ` [Bug libstdc++/99537] " redi at gcc dot gnu.org
2021-03-11 17:53 ` cvs-commit at gcc dot gnu.org
@ 2021-03-11 17:53 ` cvs-commit at gcc dot gnu.org
2021-03-29 20:03 ` cvs-commit at gcc dot gnu.org
2021-03-29 21:36 ` redi at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-03-11 17:53 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99537
--- Comment #2 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:15825b17cf3fbf28181c51fe94a2898f448f915c
commit r11-7629-g15825b17cf3fbf28181c51fe94a2898f448f915c
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Mar 11 16:44:57 2021 +0000
libstdc++: Use acq_rel memory ordering [PR 99537]
As Lewis Baker wrote in the PR:
> The 'fetch_sub()' operation in _M_release_ownership() should be using
> memory_order::acq_rel instead of memory_order::release. The use of
> 'release' only is insufficient as it does not synchronise with any
> corresponding 'acquire' operation.
> With the current implementation, it's possible that a prior write to
> one of the _M_value or _M_head data-members by a thread releasing the
> second-to-last reference might not be visible to another thread that
> releases the last reference and frees the memory, resulting in
> potential write to freed memory.
This simply changes the memory order to acq_rel as suggested.
libstdc++-v3/ChangeLog:
PR libstdc++/99537
* include/std/stop_token (_Stop_state_t::_M_release_ownership):
Use acq_rel memory ordering.
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug libstdc++/99537] Wrong memory_order used in stop_token ref-counting
2021-03-11 2:00 [Bug libstdc++/99537] New: Wrong memory_order used in stop_token ref-counting lewissbaker.opensource at gmail dot com
` (2 preceding siblings ...)
2021-03-11 17:53 ` cvs-commit at gcc dot gnu.org
@ 2021-03-29 20:03 ` cvs-commit at gcc dot gnu.org
2021-03-29 21:36 ` redi at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-03-29 20:03 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99537
--- Comment #3 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:ad9ae82ed1c33e1803f33bb367b0b4c9ee2e3e63
commit r10-9594-gad9ae82ed1c33e1803f33bb367b0b4c9ee2e3e63
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Mar 11 16:44:57 2021 +0000
libstdc++: Use acq_rel memory ordering [PR 99537]
As Lewis Baker wrote in the PR:
> The 'fetch_sub()' operation in _M_release_ownership() should be using
> memory_order::acq_rel instead of memory_order::release. The use of
> 'release' only is insufficient as it does not synchronise with any
> corresponding 'acquire' operation.
> With the current implementation, it's possible that a prior write to
> one of the _M_value or _M_head data-members by a thread releasing the
> second-to-last reference might not be visible to another thread that
> releases the last reference and frees the memory, resulting in
> potential write to freed memory.
This simply changes the memory order to acq_rel as suggested.
libstdc++-v3/ChangeLog:
PR libstdc++/99537
* include/std/stop_token (_Stop_state_t::_M_release_ownership):
Use acq_rel memory ordering.
(cherry picked from commit 15825b17cf3fbf28181c51fe94a2898f448f915c)
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug libstdc++/99537] Wrong memory_order used in stop_token ref-counting
2021-03-11 2:00 [Bug libstdc++/99537] New: Wrong memory_order used in stop_token ref-counting lewissbaker.opensource at gmail dot com
` (3 preceding siblings ...)
2021-03-29 20:03 ` cvs-commit at gcc dot gnu.org
@ 2021-03-29 21:36 ` redi at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: redi at gcc dot gnu.org @ 2021-03-29 21:36 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99537
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
Target Milestone|--- |10.3
--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for 10.3
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-29 21:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11 2:00 [Bug libstdc++/99537] New: Wrong memory_order used in stop_token ref-counting lewissbaker.opensource at gmail dot com
2021-03-11 13:34 ` [Bug libstdc++/99537] " redi at gcc dot gnu.org
2021-03-11 17:53 ` cvs-commit at gcc dot gnu.org
2021-03-11 17:53 ` cvs-commit at gcc dot gnu.org
2021-03-29 20:03 ` cvs-commit at gcc dot gnu.org
2021-03-29 21:36 ` redi at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).