#3012 closed defect (fixed)

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

Reported by: Sebastian Huber Owned by: Chris Johns
Priority: normal Milestone: 5.1
Component: tool/newlib Version:
Severity: normal Keywords:
Cc: Blocked By:
Blocking:

Description

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);

Change History (12)

comment:1 Changed on May 10, 2017 at 12:21:49 AM by 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.

comment:2 Changed on May 10, 2017 at 5:08:39 AM by Sebastian Huber

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

comment:3 Changed on May 10, 2017 at 5:59:36 AM by 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.

comment:4 in reply to:  3 Changed on May 10, 2017 at 6:05:58 AM by 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.

comment:5 Changed on May 23, 2017 at 9:45:20 AM by Sebastian Huber

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

comment:6 Changed on Jun 22, 2017 at 6:36:49 AM by 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.

comment:7 Changed on Jun 30, 2017 at 12:57:05 PM by Sebastian Huber <sebastian.huber@…>

In 5ede1c7/rtems-source-builder:

4.12: Enable global stdio streams

Update #3012.

comment:8 Changed on Jun 30, 2017 at 12:57:49 PM by 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.

comment:9 Changed on Jul 6, 2017 at 1:08:35 AM by 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.

comment:10 Changed on Jul 6, 2017 at 5:08:03 AM by Sebastian Huber

Component: GCCNewlib
Milestone: Indefinite4.12.0
Resolution: fixed
Status: assignedclosed

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.

comment:11 Changed on Jul 6, 2017 at 5:08:25 AM by Sebastian Huber

Version: 4.12

comment:12 Changed on Nov 9, 2017 at 6:27:14 AM by Sebastian Huber

Milestone: 4.12.05.1

Milestone renamed

Note: See TracTickets for help on using tickets.