* [PATCH] libstdc++: Fix iostream init for Clang on darwin [PR110432]
@ 2023-06-30 14:10 Jonathan Wakely
2023-06-30 14:28 ` Patrick Palka
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Wakely @ 2023-06-30 14:10 UTC (permalink / raw)
To: gcc-patches, libstdc++; +Cc: Patrick Palka
Tested x86_64-linux. Patrick, PTAL.
-- >8 --
The __has_attribute(init_priority) check in <iostream> is true for Clang
on darwin, which means that user code including <iostream> thinks the
library will initialize the global streams. However, when libstdc++ is
built by GCC on darwin, the __has_attribute(init_priority) check is
false, which means that the library thinks that user code will do the
initialization when <iostream> is included. This means that the
initialization is never done.
Add an autoconf check so that the header and the library both make their
decision based on the static properties of GCC at build time, with a
consistent outcome.
As a belt and braces check, also do the initialization in <iostream> if
the compiler including that header doesn't support the attribute (even
if the library also containers the initialization). This might result in
redundant initialization done in <iostream>, but ensures the
initialization happens somewhere if there's any doubt about the
attribute working correctly due to missing linker support.
libstdc++-v3/ChangeLog:
PR libstdc++/110432
* acinclude.m4 (GLIBCXX_CHECK_INIT_PRIORITY): New.
* config.h.in: Regenerate.
* configure: Regenerate.
* configure.ac: Use GLIBCXX_CHECK_INIT_PRIORITY.
* include/std/iostream:
* src/c++98/ios_base_init.h: Use new autoconf macro instead of
__has_attribute.
---
libstdc++-v3/acinclude.m4 | 27 ++++++++++++++
libstdc++-v3/config.h.in | 3 ++
libstdc++-v3/configure | 51 ++++++++++++++++++++++++++
libstdc++-v3/configure.ac | 3 ++
libstdc++-v3/include/std/iostream | 2 +-
libstdc++-v3/src/c++98/ios_base_init.h | 2 +-
6 files changed, 86 insertions(+), 2 deletions(-)
diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 277ae10e031..823832f97d4 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -5680,6 +5680,33 @@ AC_DEFUN([GLIBCXX_CHECK_ALIGNAS_CACHELINE], [
AC_LANG_RESTORE
])
+dnl
+dnl Check whether iostream initialization should be done in the library,
+dnl using the init_priority attribute.
+dnl
+dnl Defines:
+dnl _GLIBCXX_USE_INIT_PRIORITY_ATTRIBUTE if GCC supports the init_priority
+dnl attribute for the target.
+dnl
+AC_DEFUN([GLIBCXX_CHECK_INIT_PRIORITY], [
+AC_LANG_SAVE
+ AC_LANG_CPLUSPLUS
+
+ AC_MSG_CHECKING([whether init_priority attribute is supported])
+ AC_TRY_COMPILE(, [
+ #if ! __has_attribute(init_priority)
+ #error init_priority not supported
+ #endif
+ ], [ac_init_priority=yes], [ac_init_priority=no])
+ if test "$ac_init_priority" = yes; then
+ AC_DEFINE_UNQUOTED(_GLIBCXX_USE_INIT_PRIORITY_ATTRIBUTE, 1,
+ [Define if init_priority should be used for iostream initialization.])
+ fi
+ AC_MSG_RESULT($ac_init_priority)
+
+ AC_LANG_RESTORE
+])
+
# Macros from the top-level gcc directory.
m4_include([../config/gc++filt.m4])
m4_include([../config/tls.m4])
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index 9770c178767..fc0f2522027 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -550,6 +550,9 @@ GLIBCXX_ZONEINFO_DIR
# For src/c++11/shared_ptr.cc alignment.
GLIBCXX_CHECK_ALIGNAS_CACHELINE
+# For using init_priority in ios_init.cc
+GLIBCXX_CHECK_INIT_PRIORITY
+
# Define documentation rules conditionally.
# See if makeinfo has been installed and is modern enough
diff --git a/libstdc++-v3/include/std/iostream b/libstdc++-v3/include/std/iostream
index cfd124dcf43..ec337cf89dd 100644
--- a/libstdc++-v3/include/std/iostream
+++ b/libstdc++-v3/include/std/iostream
@@ -75,7 +75,7 @@ _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/globals_io.cc).
-#if !__has_attribute(__init_priority__)
+#if !(_GLIBCXX_USE_INIT_PRIORITY_ATTRIBUTE && __has_attribute(init_priority))
static ios_base::Init __ioinit;
#elif defined(_GLIBCXX_SYMVER_GNU)
__extension__ __asm (".globl _ZSt21ios_base_library_initv");
diff --git a/libstdc++-v3/src/c++98/ios_base_init.h b/libstdc++-v3/src/c++98/ios_base_init.h
index b600ec3298e..f7edfc84625 100644
--- a/libstdc++-v3/src/c++98/ios_base_init.h
+++ b/libstdc++-v3/src/c++98/ios_base_init.h
@@ -8,6 +8,6 @@
// constructor when statically linking with libstdc++.a), instead of
// doing so in (each TU that includes) <iostream>.
// This needs to be done in the same TU that defines the stream objects.
-#if __has_attribute(init_priority)
+#if _GLIBCXX_USE_INIT_PRIORITY_ATTRIBUTE
static ios_base::Init __ioinit __attribute__((init_priority(90)));
#endif
--
2.41.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] libstdc++: Fix iostream init for Clang on darwin [PR110432]
2023-06-30 14:10 [PATCH] libstdc++: Fix iostream init for Clang on darwin [PR110432] Jonathan Wakely
@ 2023-06-30 14:28 ` Patrick Palka
2023-06-30 14:37 ` Jonathan Wakely
0 siblings, 1 reply; 3+ messages in thread
From: Patrick Palka @ 2023-06-30 14:28 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: gcc-patches, libstdc++, Patrick Palka
On Fri, 30 Jun 2023, Jonathan Wakely wrote:
> Tested x86_64-linux. Patrick, PTAL.
>
> -- >8 --
>
> The __has_attribute(init_priority) check in <iostream> is true for Clang
> on darwin, which means that user code including <iostream> thinks the
> library will initialize the global streams. However, when libstdc++ is
> built by GCC on darwin, the __has_attribute(init_priority) check is
> false, which means that the library thinks that user code will do the
> initialization when <iostream> is included. This means that the
> initialization is never done.
>
> Add an autoconf check so that the header and the library both make their
> decision based on the static properties of GCC at build time, with a
> consistent outcome.
>
> As a belt and braces check, also do the initialization in <iostream> if
> the compiler including that header doesn't support the attribute (even
> if the library also containers the initialization). This might result in
> redundant initialization done in <iostream>, but ensures the
> initialization happens somewhere if there's any doubt about the
> attribute working correctly due to missing linker support.
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/110432
> * acinclude.m4 (GLIBCXX_CHECK_INIT_PRIORITY): New.
> * config.h.in: Regenerate.
> * configure: Regenerate.
> * configure.ac: Use GLIBCXX_CHECK_INIT_PRIORITY.
> * include/std/iostream:
Missing ChangeLog entry?
> * src/c++98/ios_base_init.h: Use new autoconf macro instead of
> __has_attribute.
> ---
> libstdc++-v3/acinclude.m4 | 27 ++++++++++++++
> libstdc++-v3/config.h.in | 3 ++
> libstdc++-v3/configure | 51 ++++++++++++++++++++++++++
> libstdc++-v3/configure.ac | 3 ++
> libstdc++-v3/include/std/iostream | 2 +-
> libstdc++-v3/src/c++98/ios_base_init.h | 2 +-
> 6 files changed, 86 insertions(+), 2 deletions(-)
>
> diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
> index 277ae10e031..823832f97d4 100644
> --- a/libstdc++-v3/acinclude.m4
> +++ b/libstdc++-v3/acinclude.m4
> @@ -5680,6 +5680,33 @@ AC_DEFUN([GLIBCXX_CHECK_ALIGNAS_CACHELINE], [
> AC_LANG_RESTORE
> ])
>
> +dnl
> +dnl Check whether iostream initialization should be done in the library,
> +dnl using the init_priority attribute.
> +dnl
> +dnl Defines:
> +dnl _GLIBCXX_USE_INIT_PRIORITY_ATTRIBUTE if GCC supports the init_priority
> +dnl attribute for the target.
> +dnl
> +AC_DEFUN([GLIBCXX_CHECK_INIT_PRIORITY], [
> +AC_LANG_SAVE
> + AC_LANG_CPLUSPLUS
> +
> + AC_MSG_CHECKING([whether init_priority attribute is supported])
> + AC_TRY_COMPILE(, [
> + #if ! __has_attribute(init_priority)
> + #error init_priority not supported
> + #endif
> + ], [ac_init_priority=yes], [ac_init_priority=no])
> + if test "$ac_init_priority" = yes; then
> + AC_DEFINE_UNQUOTED(_GLIBCXX_USE_INIT_PRIORITY_ATTRIBUTE, 1,
> + [Define if init_priority should be used for iostream initialization.])
> + fi
> + AC_MSG_RESULT($ac_init_priority)
> +
> + AC_LANG_RESTORE
> +])
> +
> # Macros from the top-level gcc directory.
> m4_include([../config/gc++filt.m4])
> m4_include([../config/tls.m4])
> diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
> index 9770c178767..fc0f2522027 100644
> --- a/libstdc++-v3/configure.ac
> +++ b/libstdc++-v3/configure.ac
> @@ -550,6 +550,9 @@ GLIBCXX_ZONEINFO_DIR
> # For src/c++11/shared_ptr.cc alignment.
> GLIBCXX_CHECK_ALIGNAS_CACHELINE
>
> +# For using init_priority in ios_init.cc
> +GLIBCXX_CHECK_INIT_PRIORITY
> +
> # Define documentation rules conditionally.
>
> # See if makeinfo has been installed and is modern enough
> diff --git a/libstdc++-v3/include/std/iostream b/libstdc++-v3/include/std/iostream
> index cfd124dcf43..ec337cf89dd 100644
> --- a/libstdc++-v3/include/std/iostream
> +++ b/libstdc++-v3/include/std/iostream
> @@ -75,7 +75,7 @@ _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/globals_io.cc).
> -#if !__has_attribute(__init_priority__)
> +#if !(_GLIBCXX_USE_INIT_PRIORITY_ATTRIBUTE && __has_attribute(init_priority))
This should check __init_priority__ since init_priority is a non-reserved
name I think? LGTM otherwise.
> static ios_base::Init __ioinit;
> #elif defined(_GLIBCXX_SYMVER_GNU)
> __extension__ __asm (".globl _ZSt21ios_base_library_initv");
> diff --git a/libstdc++-v3/src/c++98/ios_base_init.h b/libstdc++-v3/src/c++98/ios_base_init.h
> index b600ec3298e..f7edfc84625 100644
> --- a/libstdc++-v3/src/c++98/ios_base_init.h
> +++ b/libstdc++-v3/src/c++98/ios_base_init.h
> @@ -8,6 +8,6 @@
> // constructor when statically linking with libstdc++.a), instead of
> // doing so in (each TU that includes) <iostream>.
> // This needs to be done in the same TU that defines the stream objects.
> -#if __has_attribute(init_priority)
> +#if _GLIBCXX_USE_INIT_PRIORITY_ATTRIBUTE
> static ios_base::Init __ioinit __attribute__((init_priority(90)));
> #endif
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] libstdc++: Fix iostream init for Clang on darwin [PR110432]
2023-06-30 14:28 ` Patrick Palka
@ 2023-06-30 14:37 ` Jonathan Wakely
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Wakely @ 2023-06-30 14:37 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, libstdc++
On Fri, 30 Jun 2023 at 15:29, Patrick Palka <ppalka@redhat.com> wrote:
>
> On Fri, 30 Jun 2023, Jonathan Wakely wrote:
>
> > Tested x86_64-linux. Patrick, PTAL.
> >
> > -- >8 --
> >
> > The __has_attribute(init_priority) check in <iostream> is true for Clang
> > on darwin, which means that user code including <iostream> thinks the
> > library will initialize the global streams. However, when libstdc++ is
> > built by GCC on darwin, the __has_attribute(init_priority) check is
> > false, which means that the library thinks that user code will do the
> > initialization when <iostream> is included. This means that the
> > initialization is never done.
> >
> > Add an autoconf check so that the header and the library both make their
> > decision based on the static properties of GCC at build time, with a
> > consistent outcome.
> >
> > As a belt and braces check, also do the initialization in <iostream> if
> > the compiler including that header doesn't support the attribute (even
> > if the library also containers the initialization). This might result in
> > redundant initialization done in <iostream>, but ensures the
> > initialization happens somewhere if there's any doubt about the
> > attribute working correctly due to missing linker support.
> >
> > libstdc++-v3/ChangeLog:
> >
> > PR libstdc++/110432
> > * acinclude.m4 (GLIBCXX_CHECK_INIT_PRIORITY): New.
> > * config.h.in: Regenerate.
> > * configure: Regenerate.
> > * configure.ac: Use GLIBCXX_CHECK_INIT_PRIORITY.
> > * include/std/iostream:
>
> Missing ChangeLog entry?
>
> > * src/c++98/ios_base_init.h: Use new autoconf macro instead of
> > __has_attribute.
> > ---
> > libstdc++-v3/acinclude.m4 | 27 ++++++++++++++
> > libstdc++-v3/config.h.in | 3 ++
> > libstdc++-v3/configure | 51 ++++++++++++++++++++++++++
> > libstdc++-v3/configure.ac | 3 ++
> > libstdc++-v3/include/std/iostream | 2 +-
> > libstdc++-v3/src/c++98/ios_base_init.h | 2 +-
> > 6 files changed, 86 insertions(+), 2 deletions(-)
> >
> > diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
> > index 277ae10e031..823832f97d4 100644
> > --- a/libstdc++-v3/acinclude.m4
> > +++ b/libstdc++-v3/acinclude.m4
> > @@ -5680,6 +5680,33 @@ AC_DEFUN([GLIBCXX_CHECK_ALIGNAS_CACHELINE], [
> > AC_LANG_RESTORE
> > ])
> >
> > +dnl
> > +dnl Check whether iostream initialization should be done in the library,
> > +dnl using the init_priority attribute.
> > +dnl
> > +dnl Defines:
> > +dnl _GLIBCXX_USE_INIT_PRIORITY_ATTRIBUTE if GCC supports the init_priority
> > +dnl attribute for the target.
> > +dnl
> > +AC_DEFUN([GLIBCXX_CHECK_INIT_PRIORITY], [
> > +AC_LANG_SAVE
> > + AC_LANG_CPLUSPLUS
> > +
> > + AC_MSG_CHECKING([whether init_priority attribute is supported])
> > + AC_TRY_COMPILE(, [
> > + #if ! __has_attribute(init_priority)
> > + #error init_priority not supported
> > + #endif
> > + ], [ac_init_priority=yes], [ac_init_priority=no])
> > + if test "$ac_init_priority" = yes; then
> > + AC_DEFINE_UNQUOTED(_GLIBCXX_USE_INIT_PRIORITY_ATTRIBUTE, 1,
> > + [Define if init_priority should be used for iostream initialization.])
> > + fi
> > + AC_MSG_RESULT($ac_init_priority)
> > +
> > + AC_LANG_RESTORE
> > +])
> > +
> > # Macros from the top-level gcc directory.
> > m4_include([../config/gc++filt.m4])
> > m4_include([../config/tls.m4])
> > diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
> > index 9770c178767..fc0f2522027 100644
> > --- a/libstdc++-v3/configure.ac
> > +++ b/libstdc++-v3/configure.ac
> > @@ -550,6 +550,9 @@ GLIBCXX_ZONEINFO_DIR
> > # For src/c++11/shared_ptr.cc alignment.
> > GLIBCXX_CHECK_ALIGNAS_CACHELINE
> >
> > +# For using init_priority in ios_init.cc
> > +GLIBCXX_CHECK_INIT_PRIORITY
> > +
> > # Define documentation rules conditionally.
> >
> > # See if makeinfo has been installed and is modern enough
> > diff --git a/libstdc++-v3/include/std/iostream b/libstdc++-v3/include/std/iostream
> > index cfd124dcf43..ec337cf89dd 100644
> > --- a/libstdc++-v3/include/std/iostream
> > +++ b/libstdc++-v3/include/std/iostream
> > @@ -75,7 +75,7 @@ _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/globals_io.cc).
> > -#if !__has_attribute(__init_priority__)
> > +#if !(_GLIBCXX_USE_INIT_PRIORITY_ATTRIBUTE && __has_attribute(init_priority))
>
> This should check __init_priority__ since init_priority is a non-reserved
> name I think? LGTM otherwise.
Thanks, fixed and pushed to trunk. Backport to gcc-13 to follow.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-06-30 14:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-30 14:10 [PATCH] libstdc++: Fix iostream init for Clang on darwin [PR110432] Jonathan Wakely
2023-06-30 14:28 ` Patrick Palka
2023-06-30 14:37 ` Jonathan Wakely
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).