[PATCH 1/2] c++: correct __has_attribute(init_priority)

[PATCH 1/2] c++: correct __has_attribute(init_priority)

[Patrick Palka]

Currently __has_attribute(init_priority) always returns true, even on
targets that don't actually support init priorities, and when using
the attribute on such targets, we issue a hard error that init
priorities are unsupported.  This makes it impossible to conditionally
use the attribute by querying __has_attribute.

This patch fixes this by adding the attribute to the attribute table
only if the target supports init priorities, so that __has_attribute
returns false appropriately.  Thus on such targets we'll now treat it as
just another unrecognized attribute, so using it gives a -Wattribute
warning instead of an error.

gcc/cp/ChangeLog:

	* tree.cc (cxx_attribute_table): Add entry for init_priority
	only if SUPPORTS_INIT_PRIORITY.
	(handle_init_priority_attribute): Assert SUPPORTS_INIT_PRIORITY
	is true.

gcc/testsuite/ChangeLog:

	* g++.dg/special/initpri3.C: New test.
---
 gcc/cp/tree.cc                          | 20 +++++++-------------
 gcc/testsuite/g++.dg/special/initpri3.C |  9 +++++++++
 2 files changed, 16 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/special/initpri3.C

diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 45348c58bb6..c30bbeb0839 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -5010,8 +5010,10 @@ const struct attribute_spec cxx_attribute_table[] =
 {
   /* { name, min_len, max_len, decl_req, type_req, fn_type_req,
        affects_type_identity, handler, exclude } */
+#if SUPPORTS_INIT_PRIORITY
   { "init_priority",  1, 1, true,  false, false, false,
     handle_init_priority_attribute, NULL },
+#endif
   { "abi_tag", 1, -1, false, false, false, true,
     handle_abi_tag_attribute, NULL },
   { NULL, 0, 0, false, false, false, false, NULL, NULL }
@@ -5039,7 +5041,7 @@ const struct attribute_spec std_attribute_table[] =
 
 /* Handle an "init_priority" attribute; arguments as in
    struct attribute_spec.handler.  */
-static tree
+ATTRIBUTE_UNUSED static tree
 handle_init_priority_attribute (tree* node,
 				tree name,
 				tree args,
@@ -5103,18 +5105,10 @@ handle_init_priority_attribute (tree* node,
 	 pri);
     }
 
-  if (SUPPORTS_INIT_PRIORITY)
-    {
-      SET_DECL_INIT_PRIORITY (decl, pri);
-      DECL_HAS_INIT_PRIORITY_P (decl) = 1;
-      return NULL_TREE;
-    }
-  else
-    {
-      error ("%qE attribute is not supported on this platform", name);
-      *no_add_attrs = true;
-      return NULL_TREE;
-    }
+  gcc_assert (SUPPORTS_INIT_PRIORITY);
+  SET_DECL_INIT_PRIORITY (decl, pri);
+  DECL_HAS_INIT_PRIORITY_P (decl) = 1;
+  return NULL_TREE;
 }
 
 /* DECL is being redeclared; the old declaration had the abi tags in OLD,
diff --git a/gcc/testsuite/g++.dg/special/initpri3.C b/gcc/testsuite/g++.dg/special/initpri3.C
new file mode 100644
index 00000000000..a181abdd0b1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/special/initpri3.C
@@ -0,0 +1,9 @@
+// Verify __has_attribute(init_priority) is false whenever the platform
+// doesn't support it, and is treated as an unrecognized attribute.
+
+#if !__has_attribute(init_priority)
+#error init_priority /* { dg-error "" "" { target { ! init_priority } } } */
+#endif
+
+struct A { A(); } a __attribute__((init_priority(500)));
+// { dg-warning "attribute directive ignored" "" { target { ! init_priority } } .-1 }
-- 
2.38.1.385.g3b08839926

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

[Patrick Palka]

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
+# 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
+
+#if !_GLIBCXX_HAVE_ATTRIBUTE_INIT_PRIORITY
+# error "This file should not be included for this build"
+#endif
+
+static ios_base::Init __ioinit __attribute__((init_priority(90)));
diff --git a/libstdc++-v3/src/c++98/ios_init.cc b/libstdc++-v3/src/c++98/ios_init.cc
index 1b5132f1c2d..954fa9f29cf 100644
--- a/libstdc++-v3/src/c++98/ios_init.cc
+++ b/libstdc++-v3/src/c++98/ios_init.cc
@@ -75,6 +75,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   extern wostream wclog;
 #endif
 
+#if _GLIBCXX_HAVE_ATTRIBUTE_INIT_PRIORITY
+# include "ios_base_init.h"
+#endif
+
   ios_base::Init::Init()
   {
     if (__gnu_cxx::__exchange_and_add_dispatch(&_S_refcount, 1) == 0)
-- 
2.38.1.385.g3b08839926

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

[Jonathan Wakely]

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.

> +# 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.

> +
> +#if !_GLIBCXX_HAVE_ATTRIBUTE_INIT_PRIORITY
> +# error "This file should not be included for this build"
> +#endif
> +
> +static ios_base::Init __ioinit __attribute__((init_priority(90)));
> diff --git a/libstdc++-v3/src/c++98/ios_init.cc b/libstdc++-v3/src/c++98/ios_init.cc
> index 1b5132f1c2d..954fa9f29cf 100644
> --- a/libstdc++-v3/src/c++98/ios_init.cc
> +++ b/libstdc++-v3/src/c++98/ios_init.cc
> @@ -75,6 +75,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>    extern wostream wclog;
>  #endif
>
> +#if _GLIBCXX_HAVE_ATTRIBUTE_INIT_PRIORITY
> +# include "ios_base_init.h"
> +#endif
> +
>    ios_base::Init::Init()
>    {
>      if (__gnu_cxx::__exchange_and_add_dispatch(&_S_refcount, 1) == 0)
> --
> 2.38.1.385.g3b08839926
>

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

[Patrick Palka]

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

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

[Jonathan Wakely]

On Fri, 4 Nov 2022 at 16:31, Patrick Palka <ppalka@redhat.com> wrote:
>
> 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:

OK for trunk assuming testing passes.

Thanks for working on it.


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

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

[Florian Weimer]

* Patrick Palka via Gcc-patches:

> 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.

I think this breaks initialization of iostreams of shared objects that
are preloaded with LD_PRELOAD.  With the constructor, they initialize
iostreams once they are loaded via their own ELF constructors (even
before libstdc++'s ELF constructors run).  Without the constructor in
<iostream>, that no longer happens.

Thanks,
Florian

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

[Patrick Palka]

On Fri, 4 Nov 2022, Florian Weimer wrote:

> * Patrick Palka via Gcc-patches:
> 
> > 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.
> 
> I think this breaks initialization of iostreams of shared objects that
> are preloaded with LD_PRELOAD.  With the constructor, they initialize
> iostreams once they are loaded via their own ELF constructors (even
> before libstdc++'s ELF constructors run).  Without the constructor in
> <iostream>, that no longer happens.

IIUC wouldn't that shared object depend on libstdc++.so and hence
libstdc++'s constructors would still run before the shared object's?

> 
> Thanks,
> Florian
> 
> 

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

[Florian Weimer]

* Patrick Palka:

> On Fri, 4 Nov 2022, Florian Weimer wrote:
>
>> * Patrick Palka via Gcc-patches:
>> 
>> > 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.
>> 
>> I think this breaks initialization of iostreams of shared objects that
>> are preloaded with LD_PRELOAD.  With the constructor, they initialize
>> iostreams once they are loaded via their own ELF constructors (even
>> before libstdc++'s ELF constructors run).  Without the constructor in
>> <iostream>, that no longer happens.
>
> IIUC wouldn't that shared object depend on libstdc++.so and hence
> libstdc++'s constructors would still run before the shared object's?

Hmm, right, we only reorder the symbol binding order, not the
initialization order.  The preloaded object will not participate in a
cycle with libstdc++, so I think this should indeed be a safe change.

Thanks,
Florian

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

[Hans-Peter Nilsson]

> From: Patrick Palka via Gcc-patches <gcc-patches@gcc.gnu.org>
> Date: Fri, 4 Nov 2022 16:05:25 +0100

> 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.

This (r13-3707-g4e4e3ffd10f53e) broke statically linked programs
using iostreams (affected embedded targets + "native" with
-static).  For me it manifests as adding some 100+ fails to my
cris-elf autotester also repeatable using a native Debian 11
x86_64 build running the test-suite with -static like "make
check-gcc-c++ 'RUNTESTFLAGS=--target_board=unix/-static
old-deja.exp=15071.C'".

I opened PR107701 for it.

brgds, H-P

Re: [PATCH 1/2] c++: correct __has_attribute(init_priority)

[Jason Merrill]

On 11/4/22 11:05, Patrick Palka wrote:
> Currently __has_attribute(init_priority) always returns true, even on
> targets that don't actually support init priorities, and when using
> the attribute on such targets, we issue a hard error that init
> priorities are unsupported.  This makes it impossible to conditionally
> use the attribute by querying __has_attribute.
> 
> This patch fixes this by adding the attribute to the attribute table
> only if the target supports init priorities, so that __has_attribute
> returns false appropriately.  Thus on such targets we'll now treat it as
> just another unrecognized attribute, so using it gives a -Wattribute
> warning instead of an error.

OK.

> gcc/cp/ChangeLog:
> 
> 	* tree.cc (cxx_attribute_table): Add entry for init_priority
> 	only if SUPPORTS_INIT_PRIORITY.
> 	(handle_init_priority_attribute): Assert SUPPORTS_INIT_PRIORITY
> 	is true.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/special/initpri3.C: New test.
> ---
>   gcc/cp/tree.cc                          | 20 +++++++-------------
>   gcc/testsuite/g++.dg/special/initpri3.C |  9 +++++++++
>   2 files changed, 16 insertions(+), 13 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/special/initpri3.C
> 
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index 45348c58bb6..c30bbeb0839 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -5010,8 +5010,10 @@ const struct attribute_spec cxx_attribute_table[] =
>   {
>     /* { name, min_len, max_len, decl_req, type_req, fn_type_req,
>          affects_type_identity, handler, exclude } */
> +#if SUPPORTS_INIT_PRIORITY
>     { "init_priority",  1, 1, true,  false, false, false,
>       handle_init_priority_attribute, NULL },
> +#endif
>     { "abi_tag", 1, -1, false, false, false, true,
>       handle_abi_tag_attribute, NULL },
>     { NULL, 0, 0, false, false, false, false, NULL, NULL }
> @@ -5039,7 +5041,7 @@ const struct attribute_spec std_attribute_table[] =
>   
>   /* Handle an "init_priority" attribute; arguments as in
>      struct attribute_spec.handler.  */
> -static tree
> +ATTRIBUTE_UNUSED static tree
>   handle_init_priority_attribute (tree* node,
>   				tree name,
>   				tree args,
> @@ -5103,18 +5105,10 @@ handle_init_priority_attribute (tree* node,
>   	 pri);
>       }
>   
> -  if (SUPPORTS_INIT_PRIORITY)
> -    {
> -      SET_DECL_INIT_PRIORITY (decl, pri);
> -      DECL_HAS_INIT_PRIORITY_P (decl) = 1;
> -      return NULL_TREE;
> -    }
> -  else
> -    {
> -      error ("%qE attribute is not supported on this platform", name);
> -      *no_add_attrs = true;
> -      return NULL_TREE;
> -    }
> +  gcc_assert (SUPPORTS_INIT_PRIORITY);
> +  SET_DECL_INIT_PRIORITY (decl, pri);
> +  DECL_HAS_INIT_PRIORITY_P (decl) = 1;
> +  return NULL_TREE;
>   }
>   
>   /* DECL is being redeclared; the old declaration had the abi tags in OLD,
> diff --git a/gcc/testsuite/g++.dg/special/initpri3.C b/gcc/testsuite/g++.dg/special/initpri3.C
> new file mode 100644
> index 00000000000..a181abdd0b1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/special/initpri3.C
> @@ -0,0 +1,9 @@
> +// Verify __has_attribute(init_priority) is false whenever the platform
> +// doesn't support it, and is treated as an unrecognized attribute.
> +
> +#if !__has_attribute(init_priority)
> +#error init_priority /* { dg-error "" "" { target { ! init_priority } } } */
> +#endif
> +
> +struct A { A(); } a __attribute__((init_priority(500)));
> +// { dg-warning "attribute directive ignored" "" { target { ! init_priority } } .-1 }