public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug libstdc++/98108] New: Broken Schwarz counter for iostreams initialization
@ 2020-12-02 21:32 i.hamsa at gmail dot com
2020-12-03 7:28 ` [Bug libstdc++/98108] " rguenth at gcc dot gnu.org
` (9 more replies)
0 siblings, 10 replies; 11+ messages in thread
From: i.hamsa at gmail dot com @ 2020-12-02 21:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98108
Bug ID: 98108
Summary: Broken Schwarz counter for iostreams initialization
Product: gcc
Version: 10.2.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: libstdc++
Assignee: unassigned at gcc dot gnu.org
Reporter: i.hamsa at gmail dot com
Target Milestone: ---
Iostreams initialization doesn't work in multithreaded environments.
To reproduce one needs two source files:
// file1.cc
#include <thread>
void threadfunc();
struct StaticThread {
std::thread t;
StaticThread() : t(threadfunc) {}
~StaticThread() { t.join(); }
};
static StaticThread thread1, thread2;
//file2.cc
#include <iostream>
void threadfunc() {
std::cout << "Printing\n";
}
int main() {}
//compile
g++ -pthread file1.cc file2.cc
It is important that <iostream> is NOT included in file1.cc.
The resulting executable always crashes on my machine (Gentoo Linux). Add
<iostream> to file1.cc, the crash disappears. Change the order of source files
in the command line, the crash disappears.
---
The culprit is ios_base::Init::_S_refcount. Basically it is an atomic nifty
counter. The problem is, the nifty counter idiom doesn't work with
multithreading. Making it atomic achieves absolutely nothing. If we have two or
more threads that can enter Init::Init() at the same time, one should proceed
and the rest must WAIT. It is the same thing as initialization of a static
object in a block scope, and should be interlocked in exactly the same way.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/98108] Broken Schwarz counter for iostreams initialization
2020-12-02 21:32 [Bug libstdc++/98108] New: Broken Schwarz counter for iostreams initialization i.hamsa at gmail dot com
@ 2020-12-03 7:28 ` rguenth at gcc dot gnu.org
2020-12-03 9:19 ` i.hamsa at gmail dot com
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-12-03 7:28 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98108
--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think this is undefined since global object initialization order is not
well-defined between TUs so thread1/thread2 and std::cout construction are not
well-ordered.
It probably works (by accident) when doing
g++ -pthread file2.cc file1.cc
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/98108] Broken Schwarz counter for iostreams initialization
2020-12-02 21:32 [Bug libstdc++/98108] New: Broken Schwarz counter for iostreams initialization i.hamsa at gmail dot com
2020-12-03 7:28 ` [Bug libstdc++/98108] " rguenth at gcc dot gnu.org
@ 2020-12-03 9:19 ` i.hamsa at gmail dot com
2020-12-09 15:52 ` redi at gcc dot gnu.org
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: i.hamsa at gmail dot com @ 2020-12-03 9:19 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98108
--- Comment #2 from i.hamsa at gmail dot com ---
The standard streams are not user defined objects. They need to be initialized
before any user code touches them, before `main`, before everything, no matter
what. This is exactly the purpose of the "nifty counter". It works perfectly
well in single-threaded environments.
The problem is that libstdc++ implementation of it purports to be thread-safe.
It isn't.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/98108] Broken Schwarz counter for iostreams initialization
2020-12-02 21:32 [Bug libstdc++/98108] New: Broken Schwarz counter for iostreams initialization i.hamsa at gmail dot com
2020-12-03 7:28 ` [Bug libstdc++/98108] " rguenth at gcc dot gnu.org
2020-12-03 9:19 ` i.hamsa at gmail dot com
@ 2020-12-09 15:52 ` redi at gcc dot gnu.org
2020-12-15 11:45 ` cvs-commit at gcc dot gnu.org
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: redi at gcc dot gnu.org @ 2020-12-09 15:52 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98108
--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I think it's QoI whether this works.
"The objects are constructed and the associations are established at some time
prior to or during the first time an object of class ios_base::Init is
constructed, ..."
"The results of including <iostream> in a translation unit shall be as if
<iostream> defined an instance of ios_base::Init with static storage duration."
So there's no guarantee that the global iostreams will be usable during the
static initialization phase, because there's no guarantee that a given
constructor will be before the first ios_base::Init constructor.
The problem is *not* that the counters use atomics rather than waiting.
Even if the initialization is done as in the patch below, it still crashes:
--- a/libstdc++-v3/src/c++98/ios_init.cc
+++ b/libstdc++-v3/src/c++98/ios_init.cc
@@ -77,7 +77,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
ios_base::Init::Init()
{
- if (__gnu_cxx::__exchange_and_add_dispatch(&_S_refcount, 1) == 0)
+ struct do_init
+ {
+ do_init()
{
// Standard streams default to synced with "C" operations.
_S_synced_with_stdio = true;
@@ -116,8 +118,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// streams are not re-initialized with uses of ios_base::Init
// besides <iostream> static object, ie just using <ios> with
// ios_base::Init objects.
- __gnu_cxx::__atomic_add_dispatch(&_S_refcount, 1);
+ _S_refcount = 1;
}
+ };
+#ifndef __cpp_threadsafe_static_init
+# warning "ios_base::Init::Init() initialization is not thread-safe"
+#endif
+ static const do_init once __attribute__((unused)) = do_init();
+
+ __gnu_cxx::__exchange_and_add_dispatch(&_S_refcount, 1);
}
ios_base::Init::~Init()
This doesn't help, because the thread1 and thread2 objects in file1.cc are
still constructed before the static ios_base::Init object in file2.cc and so
they still try to use std::cout before the first _S_refcount increment has
happened.
Waiting for the iostream initialization to finish doesn't help if it hasn't
started yet. And isn't relevant in your program, because only one
ios_base::Init constructor ever runs in the whole program. Making the second
one wait for the first doesn't help if there is only one.
What does make your program work is:
--- a/libstdc++-v3/include/std/iostream
+++ b/libstdc++-v3/include/std/iostream
@@ -71,7 +71,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
//@}
// For construction of filebuffers for cout, cin, cerr, clog et. al.
- static ios_base::Init __ioinit;
+ static ios_base::Init __ioinit __attribute__((__init_priority__(50)));
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace
And this works with or without changing the Schwarz counter.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/98108] Broken Schwarz counter for iostreams initialization
2020-12-02 21:32 [Bug libstdc++/98108] New: Broken Schwarz counter for iostreams initialization i.hamsa at gmail dot com
` (2 preceding siblings ...)
2020-12-09 15:52 ` redi at gcc dot gnu.org
@ 2020-12-15 11:45 ` cvs-commit at gcc dot gnu.org
2020-12-15 11:53 ` redi at gcc dot gnu.org
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-12-15 11:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98108
--- Comment #4 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:cf4ed3b41594b6935a337fe0aaf8149eadf88751
commit r11-6072-gcf4ed3b41594b6935a337fe0aaf8149eadf88751
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue Dec 15 11:40:06 2020 +0000
libstdc++: Use init_priority attribute for Init object [PR 98108]
This causes the global objects that run the <iostream> initialization
code to be constructed earlier, which avoids some bugs in user code due
to incorrectly relying on static initialization order.
libstdc++-v3/ChangeLog:
PR libstdc++/98108
* include/std/iostream (__ioinit): Add init_priority attribute.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/98108] Broken Schwarz counter for iostreams initialization
2020-12-02 21:32 [Bug libstdc++/98108] New: Broken Schwarz counter for iostreams initialization i.hamsa at gmail dot com
` (3 preceding siblings ...)
2020-12-15 11:45 ` cvs-commit at gcc dot gnu.org
@ 2020-12-15 11:53 ` redi at gcc dot gnu.org
2020-12-15 18:42 ` cvs-commit at gcc dot gnu.org
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: redi at gcc dot gnu.org @ 2020-12-15 11:53 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98108
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |11.0
Status|UNCONFIRMED |RESOLVED
Resolution|--- |FIXED
--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I've added the init_priority attribute on trunk, which makes this (broken)
program work as expected.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/98108] Broken Schwarz counter for iostreams initialization
2020-12-02 21:32 [Bug libstdc++/98108] New: Broken Schwarz counter for iostreams initialization i.hamsa at gmail dot com
` (4 preceding siblings ...)
2020-12-15 11:53 ` redi at gcc dot gnu.org
@ 2020-12-15 18:42 ` cvs-commit at gcc dot gnu.org
2020-12-16 16:16 ` redi at gcc dot gnu.org
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-12-15 18:42 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98108
--- 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:433703843b3fa76bcbba4f1fd782c7ef319b64a8
commit r11-6091-g433703843b3fa76bcbba4f1fd782c7ef319b64a8
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue Dec 15 18:40:28 2020 +0000
libstdc++: Remove init_priority attribute for Init object [PR 98108]
This reverts commit cf4ed3b41594b6935a337fe0aaf8149eadf88751.
libstdc++-v3/ChangeLog:
PR libstdc++/98108
* include/std/iostream (__ioinit): Remove init_priority attribute.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/98108] Broken Schwarz counter for iostreams initialization
2020-12-02 21:32 [Bug libstdc++/98108] New: Broken Schwarz counter for iostreams initialization i.hamsa at gmail dot com
` (5 preceding siblings ...)
2020-12-15 18:42 ` cvs-commit at gcc dot gnu.org
@ 2020-12-16 16:16 ` redi at gcc dot gnu.org
2020-12-16 17:41 ` i.hamsa at gmail dot com
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: redi at gcc dot gnu.org @ 2020-12-16 16:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98108
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|FIXED |INVALID
--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I've reverted the change because it breaks the build for some targes.
So you'll just have to fix your code. The simplest way is to ensure you include
<iostream> before defining a global that wants to use std::cout.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/98108] Broken Schwarz counter for iostreams initialization
2020-12-02 21:32 [Bug libstdc++/98108] New: Broken Schwarz counter for iostreams initialization i.hamsa at gmail dot com
` (6 preceding siblings ...)
2020-12-16 16:16 ` redi at gcc dot gnu.org
@ 2020-12-16 17:41 ` i.hamsa at gmail dot com
2020-12-16 23:32 ` redi at gcc dot gnu.org
2022-11-06 16:16 ` cvs-commit at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: i.hamsa at gmail dot com @ 2020-12-16 17:41 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98108
--- Comment #8 from i.hamsa at gmail dot com ---
After re-reading the relevant parts of the standard, I have convinced myself
that the program is indeed not required to work unless <iostreams> is included,
even in a single-threaded version. Even though the standard encourages
implementations to initialise the standard iostreams objects earlier than
required, it is not mandatory.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/98108] Broken Schwarz counter for iostreams initialization
2020-12-02 21:32 [Bug libstdc++/98108] New: Broken Schwarz counter for iostreams initialization i.hamsa at gmail dot com
` (7 preceding siblings ...)
2020-12-16 17:41 ` i.hamsa at gmail dot com
@ 2020-12-16 23:32 ` redi at gcc dot gnu.org
2022-11-06 16:16 ` cvs-commit at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: redi at gcc dot gnu.org @ 2020-12-16 23:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98108
--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Right, this actually has nothing to do with threads.
The behaviour is exactly the same as if file1.cc contained this instead:
void threadfunc();
struct StaticThread {
StaticThread() { threadfunc(); }
};
static StaticThread thread1;
A global constructor calls threadfunc() before std::cout has been initialized.
It doesn't matter whether that happens in a separate thread or not, and in
either case the solution is to include <iostream> so that a std::ios_base::Init
object is constructed before your global.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug libstdc++/98108] Broken Schwarz counter for iostreams initialization
2020-12-02 21:32 [Bug libstdc++/98108] New: Broken Schwarz counter for iostreams initialization i.hamsa at gmail dot com
` (8 preceding siblings ...)
2020-12-16 23:32 ` redi at gcc dot gnu.org
@ 2022-11-06 16:16 ` cvs-commit at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-11-06 16:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98108
--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:
https://gcc.gnu.org/g:4e4e3ffd10f53ef71696bc728ab40258751a2df4
commit r13-3707-g4e4e3ffd10f53ef71696bc728ab40258751a2df4
Author: Patrick Palka <ppalka@redhat.com>
Date: Sun Nov 6 11:16:00 2022 -0500
libstdc++: Move stream initialization into compiled library [PR44952]
This patch moves the static object for constructing the standard streams
out from <iostream> and into the compiled library on systems that support
init priorities. This'll mean <iostream> no longer introduces a separate
global constructor in each TU that includes it.
We can do this only if the init_priority attribute is supported because
we need a way to ensure the stream initialization runs first before any
user global initializer, particularly when linking with a static
libstdc++.a.
PR libstdc++/44952
PR libstdc++/39796
PR libstdc++/98108
libstdc++-v3/ChangeLog:
* include/std/iostream (__ioinit): No longer define here if
the init_priority attribute is usable.
* src/c++98/ios_init.cc (__ioinit): Define here instead if
init_priority is usable, via ...
* src/c++98/ios_base_init.h: ... this new file.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-11-06 16:16 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-02 21:32 [Bug libstdc++/98108] New: Broken Schwarz counter for iostreams initialization i.hamsa at gmail dot com
2020-12-03 7:28 ` [Bug libstdc++/98108] " rguenth at gcc dot gnu.org
2020-12-03 9:19 ` i.hamsa at gmail dot com
2020-12-09 15:52 ` redi at gcc dot gnu.org
2020-12-15 11:45 ` cvs-commit at gcc dot gnu.org
2020-12-15 11:53 ` redi at gcc dot gnu.org
2020-12-15 18:42 ` cvs-commit at gcc dot gnu.org
2020-12-16 16:16 ` redi at gcc dot gnu.org
2020-12-16 17:41 ` i.hamsa at gmail dot com
2020-12-16 23:32 ` redi at gcc dot gnu.org
2022-11-06 16:16 ` cvs-commit 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).