Global C++ IO streams are broken (cout, cin, cerr)

[Sebastian Huber]

The global C++ IO stream objects are initialized here

​https://gcc.gnu.org/viewcvs/gcc/trunk/libstdc%2B%2B-v3/src/c%2B%2B98/ios_init.cc?view=markup#l85

via a placement new. The "stdout" etc. is thread-local in Newlib

#define    stdout  (_REENT->_stdout)

Using this for a global object like std::cout is quite broken. Which FILE object should be used instead? Potential fix:

diff --git a/libstdc++-v3/src/c++98/ios_init.cc b/libstdc++-v3/src/c++98/ios_init.cc
index c5bcc83..7470c44 100644
--- a/libstdc++-v3/src/c++98/ios_init.cc
+++ b/libstdc++-v3/src/c++98/ios_init.cc
@@ -33,6 +33,15 @@
 #include <ext/stdio_filebuf.h>
 #include <ext/stdio_sync_filebuf.h>
 
+#ifdef __rtems__
+#undef stdout
+#undef stdin
+#undef stderr
+#define stdout (_GLOBAL_REENT->_stdout)
+#define stdin (_GLOBAL_REENT->_stdout)
+#define stderr (_GLOBAL_REENT->_stdout)
+#endif
+
 namespace __gnu_internal _GLIBCXX_VISIBILITY(hidden)
 {
   using namespace __gnu_cxx;

diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
index 83d3dc5..7d50951 100644
--- a/newlib/libc/stdio/findfp.c
+++ b/newlib/libc/stdio/findfp.c
@@ -259,6 +259,12 @@ _DEFUN(__sinit, (s),
   __sinit_lock_release ();
 }
 
+static void __attribute__((__constructor__(0)))
+_global_reent_init(void)
+{
+  __sinit (_GLOBAL_REENT);
+}
+
 #ifndef __SINGLE_THREAD__
 
 __LOCK_INIT_RECURSIVE(static, __sfp_recursive_mutex);

[Chris Johns]

This approach concerns me because it breaks std::cout in the other direction. If this is fine for C++'s stdout it should be ok for a thread's C stdout and given newlib has:

#define      stdout  (_REENT->_stdout)

we are not consistent.

Is moving at run-time the address of stdout, which newlib does, conflicting with another standard like C++? I assume the stdc++ code is capturing a reference to stdout so closing and then opening a file and assigning it to stdio works.

This is a difficult issue.

[Sebastian Huber]

Another option would be to use thread-local cout, etc. This would require that all RTEMS targets support thread-local storage.

[Chris Johns]

I have considered this and it is attractive. Is this something libstdc++ supports out of the box or do we need to make changes?

I would like to understand the issue a little more and how cygwin solves the problem. Joel has been testing some code on cygwin and it would be nice to get it to run on RTEMS and see how it breaks.

I am concerned the current implementation's behavior is undefined if a thread calls std::cout and then is deleted leaving libstdc++ with a reference to invalid memory.

[Sebastian Huber]

Replying to Chris Johns:

I have considered this and it is attractive. Is this something libstdc++ supports out of the box or do we need to make changes?

No, I guess it needs a some work and it is not clear if this is acceptable for libstdc++. The maintainer have a strong view if it comes to standard compliance.

I would like to understand the issue a little more and how cygwin solves the problem.

I guess the main thread is never deleted, so cout can use its stdout safely.

Joel has been testing some code on cygwin and it would be nice to get it to run on RTEMS and see how it breaks.

I am concerned the current implementation's behavior is undefined if a thread calls std::cout and then is deleted leaving libstdc++ with a reference to invalid memory.

This is why one option is to use the _GLOBAL_REENT.

[Sebastian Huber]

The Ada run-time support has the same problem (thread-local IO streams used like a global object).

[Sebastian Huber]

Maybe we could add an RTEMS-specific change to Newlib so that we still have the thread-local stdio streams, but we initialize them to global FILE objects by default, e.g. remove struct _reent::sf for RTEMS. This would also reduce the amount of per-thread storage.

If we do this, then we have to be careful with fclose(), since this would close the FILE for all threads. There is no reference counting in the FILE objects.

[Sebastian Huber <sebastian.huber@…>]

In 5ede1c7/rtems-source-builder:

4.12: Enable global stdio streams

Update #3012.

[Sebastian Huber <sebastian.huber@…>]

In 9b07f5e/rtems:

newlib01: Use fopen() instead of freopen()

With global stdio streams, a freopen() would close the global stream
object.

Update #3012.

[Chris Johns]

Has the recent change in newlib to have a single stdout, stderr, etc object and per thread references in the TLS fixed this problem?

If the first use of the C++ standard streams is before any code redirects and overwrites a thread TLS pointer the standard streams should be the global instance.

[Sebastian Huber]

From my point of view this problem is fixed.

The application could alter the stdio stream references in a attribute((constructor(123))) function if it knows what to do.

[Sebastian Huber]

Milestone renamed