Re: [PATCH 2/2] libstdc++: Move stream initialization into compiled library [PR44952]

[Patrick Palka <ppalka@redhat.com>]

On Fri, 4 Nov 2022, Jonathan Wakely wrote:

> On Fri, 4 Nov 2022 at 15:08, Patrick Palka via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
> >
> > This patch moves the global object for constructing the standard streams
> > out from <iostream> and into the compiled library on targets that support
> > the init_priority attribute.  This means that <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 to force that the stream initialization runs first before any
> > user-defined global initializer in programs that that use a static
> > libstdc++.a.
> >
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look right?
> > Unfortunately I don't have access to a system that truly doesn't support
> > init priorities, so I instead tested that situation by artificially
> > disabling the init_priority attribute on x86_64.
> >
> >         PR libstdc++/44952
> >         PR libstdc++/98108
> >
> > libstdc++-v3/ChangeLog:
> >
> >         * include/bits/c++config (_GLIBCXX_HAS_ATTRIBUTE): Define.
> >         (_GLIBCXX_HAVE_ATTRIBUTE_INIT_PRIORITY): Define.
> >         * include/std/iostream (__ioinit): Define only if init_priority
> >         attribute isn't usable.
> >         * src/c++98/ios_init.cc (__ioinit): Define here instead if
> >         the init_priority is usable.
> >         * src/c++98/ios_base_init.h: New file.
> > ---
> >  libstdc++-v3/include/bits/c++config    | 12 ++++++++++++
> >  libstdc++-v3/include/std/iostream      |  4 ++++
> >  libstdc++-v3/src/c++98/ios_base_init.h |  9 +++++++++
> >  libstdc++-v3/src/c++98/ios_init.cc     |  4 ++++
> >  4 files changed, 29 insertions(+)
> >  create mode 100644 libstdc++-v3/src/c++98/ios_base_init.h
> >
> > diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
> > index 50406066afe..f93076191d9 100644
> > --- a/libstdc++-v3/include/bits/c++config
> > +++ b/libstdc++-v3/include/bits/c++config
> > @@ -837,6 +837,18 @@ namespace __gnu_cxx
> >
> >  #undef _GLIBCXX_HAS_BUILTIN
> >
> > +#ifdef __has_attribute
> 
> Do we need this?
> I think all the compilers we support implemented this long ago (clang
> had it before gcc, and Intel has it for gcc compat, and any others had
> better have it by now too).
> 
> So we can just use #if __has_attribute directly, instead of defining
> these extra macros.

Sounds good, I wasn't sure if we can assume support for __has_attribute
or not.

> 
> > +# define _GLIBCXX_HAS_ATTRIBUTE(B) __has_attribute(B)
> > +#else
> > +# define _GLIBCXX_HAS_ATTRIBUTE(B) 0
> > +#endif
> > +
> > +#if _GLIBCXX_HAS_ATTRIBUTE(init_priority)
> > +# define _GLIBCXX_HAVE_ATTRIBUTE_INIT_PRIORITY 1
> > +#endif
> > +
> > +#undef _GLIBCXX_HAS_ATTRIBUTE
> > +
> >  // Mark code that should be ignored by the compiler, but seen by Doxygen.
> >  #define _GLIBCXX_DOXYGEN_ONLY(X)
> >
> > diff --git a/libstdc++-v3/include/std/iostream b/libstdc++-v3/include/std/iostream
> > index 70318a45891..5eaa9755d9a 100644
> > --- a/libstdc++-v3/include/std/iostream
> > +++ b/libstdc++-v3/include/std/iostream
> > @@ -73,7 +73,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >    ///@}
> >
> >    // For construction of filebuffers for cout, cin, cerr, clog et. al.
> > +  // When the init_priority attribute is usable, we do this initialization
> > +  // in the compiled library instead (see src/c++98/ios_init.cc).
> > +#if !_GLIBCXX_HAVE_ATTRIBUTE_INIT_PRIORITY
> >    static ios_base::Init __ioinit;
> > +#endif
> >
> >  _GLIBCXX_END_NAMESPACE_VERSION
> >  } // namespace
> > diff --git a/libstdc++-v3/src/c++98/ios_base_init.h b/libstdc++-v3/src/c++98/ios_base_init.h
> > new file mode 100644
> > index 00000000000..f3087d1da3c
> > --- /dev/null
> > +++ b/libstdc++-v3/src/c++98/ios_base_init.h
> > @@ -0,0 +1,9 @@
> > +// This is only in a header so we can use the system_header pragma,
> > +// to suppress the warning caused by using a reserved init_priority.
> > +#pragma GCC system_header
> 
> Ugh, that's annoying.

Yeah :( we employ the same workaround in
src/c++17/{memory_resource.cc,default_resource.h} too.

Here's v2 which gets rid of the new macros, adds more comments and other
minor improvements.  Only smoke tested so far, full bootstrap and
regtesting in progress:

-- >8 --

Subject: [PATCH 2/2] libstdc++: Move stream initialization into compiled
 library [PR44952]

This patch moves the global object for constructing the standard streams
out from <iostream> and into the compiled library on targets that support
init priorities.  This means that <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 force the stream initialization to run first before any
user-defined global initializer, particularly for programs that use a
static libstdc++.a.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look right?
Unfortunately I don't have access to a system that truly doesn't support
init priorities, so I instead tested that situation by artificially
disabling the init_priority attribute on x86_64.

	PR libstdc++/44952
	PR libstdc++/98108

libstdc++-v3/ChangeLog:

	* include/std/iostream (__ioinit): No longer define here if
	init_priority attribute is usable.
	* src/c++98/ios_init.cc (__ioinit): Define here instead if
	the init_priority is usable.
	* src/c++98/ios_base_init.h: New file.
---
 libstdc++-v3/include/std/iostream      |  4 ++++
 libstdc++-v3/src/c++98/ios_base_init.h | 12 ++++++++++++
 libstdc++-v3/src/c++98/ios_init.cc     |  2 ++
 3 files changed, 18 insertions(+)
 create mode 100644 libstdc++-v3/src/c++98/ios_base_init.h

diff --git a/libstdc++-v3/include/std/iostream b/libstdc++-v3/include/std/iostream
index 70318a45891..ff78e1cfb87 100644
--- a/libstdc++-v3/include/std/iostream
+++ b/libstdc++-v3/include/std/iostream
@@ -73,7 +73,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   ///@}
 
   // For construction of filebuffers for cout, cin, cerr, clog et. al.
+  // When the init_priority attribute is usable, we do this initialization
+  // in the compiled library instead (src/c++98/ios_init.cc).
+#if !__has_attribute(__init_priority__)
   static ios_base::Init __ioinit;
+#endif
 
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
diff --git a/libstdc++-v3/src/c++98/ios_base_init.h b/libstdc++-v3/src/c++98/ios_base_init.h
new file mode 100644
index 00000000000..fe45e0e98a9
--- /dev/null
+++ b/libstdc++-v3/src/c++98/ios_base_init.h
@@ -0,0 +1,12 @@
+// This is only in a header so we can use the system_header pragma,
+// to suppress the warning caused by using a reserved init_priority.
+#pragma GCC system_header
+
+// If the target supports init priorities, set up a static object in the
+// compiled library to perform the <iostream> initialization once and
+// sufficiently early (so that the initialization runs first before any
+// other global constructor when staticaly linking with libstdc++.a),
+// instead of having to do so in each TU that includes <iostream>.
+#if __has_attribute(init_priority)
+static ios_base::Init __ioinit __attribute__((init_priority(90)));
+#endif
diff --git a/libstdc++-v3/src/c++98/ios_init.cc b/libstdc++-v3/src/c++98/ios_init.cc
index 1b5132f1c2d..4016fcab785 100644
--- a/libstdc++-v3/src/c++98/ios_init.cc
+++ b/libstdc++-v3/src/c++98/ios_init.cc
@@ -75,6 +75,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   extern wostream wclog;
 #endif
 
+#include "ios_base_init.h"
+
   ios_base::Init::Init()
   {
     if (__gnu_cxx::__exchange_and_add_dispatch(&_S_refcount, 1) == 0)
-- 
2.38.1.385.g3b08839926