[PATCH] libstdc++, Darwin: Handle a linker warning [PR112397].

[PATCH] libstdc++, Darwin: Handle a linker warning [PR112397].

[Iain Sandoe]

Tested on i686-darwin9, x86_64-darwin14,17,19,21,23, x86_64-linux,
aarch64-linux-gnu,

OK for trunk?
eventual back-ports?
thanks
Iain

--- 8< ---

Darwin's linker warns when we make a direct branch to code that is
in a weak definition (citing that if a different implementation of
the weak function is chosen by the dynamic linker this would be an
error).

As the analysis in the PR shows, this can happen when we have hot/
cold partitioning and there is an error path that is primarily cold
but makes use of epilogue code in the hot section.  In this simple
case, we can easily deduce that the code is in fact safe; however
that is not something we can realistically implement in the linker.

Since the user-replaceable allocators are implemented using weak
definitions, this is a warning that is frequently flagged up in both
the testsuite and end-user code.

The chosen solution here is to suppress the hot/cold partitioning for
these cases (it is unlikely to impact performance much c.f. the
actual allocation).

	PR target/112397

libstdc++-v3/ChangeLog:

	* configure: Regenerate.
	* configure.ac: Detect if we are building for Darwin.
	* libsupc++/Makefile.am: If we are building for Darwin, then
	suppress hot/cold partitioning for the array allocators.
	* libsupc++/Makefile.in: Regenerated.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
Co-authored-by: Jonathan Wakely <jwakely@redhat.com>
---
 libstdc++-v3/configure             | 35 +++++++++++++++++++++++-------
 libstdc++-v3/configure.ac          |  6 +++++
 libstdc++-v3/libsupc++/Makefile.am |  8 +++++++
 libstdc++-v3/libsupc++/Makefile.in |  6 +++++
 4 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index c68cac4f345..37396bd6ebb 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -109,6 +109,12 @@ ACX_LT_HOST_FLAGS
 AC_SUBST(enable_shared)
 AC_SUBST(enable_static)
 AM_CONDITIONAL([ENABLE_DARWIN_AT_RPATH], [test x$enable_darwin_at_rpath = xyes])
+os_is_darwin=no
+case ${host_os} in
+  darwin*) os_is_darwin=yes ;;
+  *) ;;
+esac
+AM_CONDITIONAL([OS_IS_DARWIN], [test x${os_is_darwin} = xyes])
 
 if test "$enable_vtable_verify" = yes; then
   predep_objects_CXX="${predep_objects_CXX} ${glibcxx_builddir}/../libgcc/vtv_start.o"
diff --git a/libstdc++-v3/libsupc++/Makefile.am b/libstdc++-v3/libsupc++/Makefile.am
index d0e1618507e..e151ce7a1fe 100644
--- a/libstdc++-v3/libsupc++/Makefile.am
+++ b/libstdc++-v3/libsupc++/Makefile.am
@@ -132,6 +132,14 @@ atomicity_file = ${glibcxx_srcdir}/$(ATOMICITY_SRCDIR)/atomicity.h
 atomicity.cc: ${atomicity_file}
 	$(LN_S) ${atomicity_file} ./atomicity.cc || true
 
+if OS_IS_DARWIN
+# See PR 112397
+new_opvnt.lo: new_opvnt.cc
+	$(LTCXXCOMPILE) -fno-reorder-blocks-and-partition -I. -c $<
+new_opvnt.o: new_opvnt.cc
+	$(CXXCOMPILE) -fno-reorder-blocks-and-partition -I. -c $<
+endif
+
 # AM_CXXFLAGS needs to be in each subdirectory so that it can be
 # modified in a per-library or per-sub-library way.  Need to manually
 # set this option because CONFIG_CXXFLAGS has to be after
-- 
2.39.2 (Apple Git-143)

Re: [PATCH] libstdc++, Darwin: Handle a linker warning [PR112397].

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 3436 bytes --]

On Sun, 18 Feb 2024, 16:15 Iain Sandoe, <iains.gcc@gmail.com> wrote:

> Tested on i686-darwin9, x86_64-darwin14,17,19,21,23, x86_64-linux,
> aarch64-linux-gnu,
>
> OK for trunk?
> eventual back-ports?
>

Yup, ok for all.


thanks
> Iain
>
> --- 8< ---
>
> Darwin's linker warns when we make a direct branch to code that is
> in a weak definition (citing that if a different implementation of
> the weak function is chosen by the dynamic linker this would be an
> error).
>
> As the analysis in the PR shows, this can happen when we have hot/
> cold partitioning and there is an error path that is primarily cold
> but makes use of epilogue code in the hot section.  In this simple
> case, we can easily deduce that the code is in fact safe; however
> that is not something we can realistically implement in the linker.
>
> Since the user-replaceable allocators are implemented using weak
> definitions, this is a warning that is frequently flagged up in both
> the testsuite and end-user code.
>
> The chosen solution here is to suppress the hot/cold partitioning for
> these cases (it is unlikely to impact performance much c.f. the
> actual allocation).
>
>         PR target/112397
>
> libstdc++-v3/ChangeLog:
>
>         * configure: Regenerate.
>         * configure.ac: Detect if we are building for Darwin.
>         * libsupc++/Makefile.am: If we are building for Darwin, then
>         suppress hot/cold partitioning for the array allocators.
>         * libsupc++/Makefile.in: Regenerated.
>
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
> Co-authored-by: Jonathan Wakely <jwakely@redhat.com>
> ---
>  libstdc++-v3/configure             | 35 +++++++++++++++++++++++-------
>  libstdc++-v3/configure.ac          |  6 +++++
>  libstdc++-v3/libsupc++/Makefile.am |  8 +++++++
>  libstdc++-v3/libsupc++/Makefile.in |  6 +++++
>  4 files changed, 47 insertions(+), 8 deletions(-)
>
> diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
> index c68cac4f345..37396bd6ebb 100644
> --- a/libstdc++-v3/configure.ac
> +++ b/libstdc++-v3/configure.ac
> @@ -109,6 +109,12 @@ ACX_LT_HOST_FLAGS
>  AC_SUBST(enable_shared)
>  AC_SUBST(enable_static)
>  AM_CONDITIONAL([ENABLE_DARWIN_AT_RPATH], [test x$enable_darwin_at_rpath =
> xyes])
> +os_is_darwin=no
> +case ${host_os} in
> +  darwin*) os_is_darwin=yes ;;
> +  *) ;;
> +esac
> +AM_CONDITIONAL([OS_IS_DARWIN], [test x${os_is_darwin} = xyes])
>
>  if test "$enable_vtable_verify" = yes; then
>    predep_objects_CXX="${predep_objects_CXX}
> ${glibcxx_builddir}/../libgcc/vtv_start.o"
> diff --git a/libstdc++-v3/libsupc++/Makefile.am
> b/libstdc++-v3/libsupc++/Makefile.am
> index d0e1618507e..e151ce7a1fe 100644
> --- a/libstdc++-v3/libsupc++/Makefile.am
> +++ b/libstdc++-v3/libsupc++/Makefile.am
> @@ -132,6 +132,14 @@ atomicity_file =
> ${glibcxx_srcdir}/$(ATOMICITY_SRCDIR)/atomicity.h
>  atomicity.cc: ${atomicity_file}
>         $(LN_S) ${atomicity_file} ./atomicity.cc || true
>
> +if OS_IS_DARWIN
> +# See PR 112397
> +new_opvnt.lo: new_opvnt.cc
> +       $(LTCXXCOMPILE) -fno-reorder-blocks-and-partition -I. -c $<
> +new_opvnt.o: new_opvnt.cc
> +       $(CXXCOMPILE) -fno-reorder-blocks-and-partition -I. -c $<
> +endif
> +
>  # AM_CXXFLAGS needs to be in each subdirectory so that it can be
>  # modified in a per-library or per-sub-library way.  Need to manually
>  # set this option because CONFIG_CXXFLAGS has to be after
> --
> 2.39.2 (Apple Git-143)
>
>