[Bug libstdc++/112397] New: Two persistent libstdc++ test failures on x86_64-apple-darwin

[Bug libstdc++/112397] New: Two persistent libstdc++ test failures on x86_64-apple-darwin

[fxcoudert at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

            Bug ID: 112397
           Summary: Two persistent libstdc++ test failures on
                    x86_64-apple-darwin
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: fxcoudert at gcc dot gnu.org
  Target Milestone: ---

FAIL: 17_intro/static.cc  -std=gnu++17 (test for excess errors)
FAIL: 20_util/to_chars/4.cc  -std=gnu++17 (test for excess errors)

The both have the same linker warning:

ld: warning: direct access in function 'operator new[](unsigned long,
std::nothrow_t const&) (.cold)' from file '/Users/fx/ibin-2023
1105/x86_64-apple-darwin21/./libstdc++-v3/src/.libs/libstdc++.a(new_opvnt.o)'
to global weak symbol 'operator new[](unsigned long, s
td::nothrow_t const&)' from file
'/Users/fx/ibin-20231105/x86_64-apple-darwin21/./libstdc++-v3/src/.libs/libstdc++.a(new_opvnt.o)'
m
eans the weak symbol cannot be overridden at runtime. This was likely caused by
different translation units being compiled with diff
erent visibility settings.

ld: warning: direct access in function 'operator new[](unsigned long,
std::nothrow_t const&) (.cold)' from file
'/Users/fx/ibin-20231105/x86_64-apple-darwin21/./libstdc++-v3/src/.libs/libstdc++.a(new_opvnt.o)'
to global weak symbol 'operator new[](unsigned long, std::nothrow_t const&)'
from file
'/Users/fx/ibin-20231105/x86_64-apple-darwin21/./libstdc++-v3/src/.libs/libstdc++.a(new_opvnt.o)'
means the weak symbol cannot be overridden at runtime. This was likely caused
by different translation units being compiled with different visibility
settings.

I am not sure what the "different visibility settings" are, or if it's just a
red herring, but the testcase is compiled with:

Executing on host: /Users/fx/ibin-20231105/./gcc/xg++ -shared-libgcc
-B/Users/fx/ibin-20231105/./gcc -nostdinc++
-L/Users/fx/ibin-20231105/x86_64-apple-darwin21/libstdc++-v3/src
-L/Users/fx/ibin-20231105/x86_64-apple-darwin21/libstdc++-v3/src/.libs
-L/Users/fx/ibin-20231105/x86_64-apple-darwin21/libstdc++-v3/libsupc++/.libs
-B/Users/fx/irun-20231105/x86_64-apple-darwin21/bin/
-B/Users/fx/irun-20231105/x86_64-apple-darwin21/lib/ -isystem
/Users/fx/irun-20231105/x86_64-apple-darwin21/include -isystem
/Users/fx/irun-20231105/x86_64-apple-darwin21/sys-include -fchecking=1
-B/Users/fx/ibin-20231105/x86_64-apple-darwin21/./libstdc++-v3/src/.libs
-fmessage-length=0 -fno-show-column -ffunction-sections -fdata-sections -g -O2
-DLOCALEDIR="." -nostdinc++
-I/Users/fx/ibin-20231105/x86_64-apple-darwin21/libstdc++-v3/include/x86_64-apple-darwin21
-I/Users/fx/ibin-20231105/x86_64-apple-darwin21/libstdc++-v3/include
-I/Users/fx/gcc-upstream/libstdc++-v3/libsupc++
-I/Users/fx/gcc-upstream/libstdc++-v3/include/backward
-I/Users/fx/gcc-upstream/libstdc++-v3/testsuite/util 
/Users/fx/gcc-upstream/libstdc++-v3/testsuite/17_intro/static.cc   
-std=gnu++17 -static-libstdc++ -fdiagnostics-plain-output ./libtestc++.a
-liconv
-L/Users/fx/ibin-20231105/x86_64-apple-darwin21/libstdc++-v3/src/filesystem/.libs
-L/Users/fx/ibin-20231105/x86_64-apple-darwin21/libstdc++-v3/src/experimental/.libs
 -lm  -o ./static.exe    (timeout = 360)

[Bug libstdc++/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |iains at gcc dot gnu.org

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The .cold symbol is created by gcc, I don't see any way to control its
visibility in the source. So maybe a target bug in the compiler, not a library
bug.

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|libstdc++                   |target

--- Comment #2 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #1)
> The .cold symbol is created by gcc, I don't see any way to control its
> visibility in the source. So maybe a target bug in the compiler, not a
> library bug.

Agreed its a target issue - it's a question of convincing the linker that the
label is not reachable from a different TU if the implementation of the
non-cold symbol comes from that.

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2024-02-07
     Ever confirmed|0                           |1

--- Comment #3 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #2)
> (In reply to Jonathan Wakely from comment #1)
> > The .cold symbol is created by gcc, I don't see any way to control its
> > visibility in the source. So maybe a target bug in the compiler, not a
> > library bug.
> 
> Agreed its a target issue - it's a question of convincing the linker that
> the label is not reachable from a different TU if the implementation of the
> non-cold symbol comes from that.

The problem is this:

We partition the function into hot/cold.

so we have

  weak definition
  hot
 ..... do stuff
 ... make a call

Landingpad1 OK... return
   epilogue_code:
   ....

Landingpad2 bad -> jump to error_case

   cold
error_case:
   ... do stuff
   jump epilogue_code.


====

The linker says "oh you have a direct jump to that epilogue code in a weak
definition which would be broken if a different weak definition was chosen by
the dynamic linker".

On the face of it, the linker is correct ...
... but what it cannot see is that the only way to be executing the code that
makes this jump is if it came from Landingpad2.

So actually, we do not have a real error - but I am not sure how we could go
about teaching the linker to DTRT (even if we could persuade Apple to do the
same for the new closed source one).

I'm tempted to suggest that we consider building libstdc++ with
`-fno-reorder-blocks-and-partition` and see if there's any measurable
performance decrease.

NOTE; that Darwin's linker (using subsections_by_symbols) is also able to
partition and reorder code without help from the compiler .. (although
whether/how much it does I"m not sure).

If there's a way to add that [-fno-reorder-blocks-and-partition] flag to that
single source for Darwin-only, that would be a short-term workaround.

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Something like this, but I don't know what condition to use to check if the
target is darwin:

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

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Maybe:

ifneq ($(filter %-darwin%,${host_alias}),)

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #5)
> Maybe:
> 
> ifneq ($(filter %-darwin%,${host_alias}),)

${target_os} instead maybe?

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

--- Comment #7 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #6)
> (In reply to Jonathan Wakely from comment #5)
> > Maybe:
> > 
> > ifneq ($(filter %-darwin%,${host_alias}),)
> 
> ${target_os} instead maybe?

I'll give that a try (we have similar stanzas in the Ada build stuff)

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #7)
> (In reply to Jakub Jelinek from comment #6)
> > (In reply to Jonathan Wakely from comment #5)
> > > Maybe:
> > > 
> > > ifneq ($(filter %-darwin%,${host_alias}),)
> > 
> > ${target_os} instead maybe?
> 
> I'll give that a try (we have similar stanzas in the Ada build stuff)

Of course for ${target_os} one would need to leave out the - before darwin,
because target_os will be darwinsomething, so
ifneq ($(filter darwin%,${target_os}),)
Or host_os?  Never know what one should use in target libraries, maybe host and
target is the same there, just build might be different?

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
https://gcc.gnu.org/onlinedocs/gccint/Configure-Terms.html
says that in target libraries target doesn't apply, so host_os it is then.

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

--- Comment #10 from Iain Sandoe <iains at gcc dot gnu.org> ---
automake if is limited to testing a single variable, so we have to introduce an
AM_CONDITIONAL to say the OS is DARWIN (we did not seem to have one already,
but I could have missed something else usable).

I'm testing this - if it's not too invasive (but I'll also try, at some point,
to see if applying this to the whole library build makes any measurable
performance change)

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

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

--- Comment #11 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>:

https://gcc.gnu.org/g:1609fdff16f17ead37666f6d0e801800ee3d04d2

commit r14-9073-g1609fdff16f17ead37666f6d0e801800ee3d04d2
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Thu Feb 8 17:54:31 2024 +0000

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

    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>

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

--- Comment #12 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Iain D Sandoe
<iains@gcc.gnu.org>:

https://gcc.gnu.org/g:ae11f0154116f4e5fa8769b1ea1600b1b1c22958

commit r13-8577-gae11f0154116f4e5fa8769b1ea1600b1b1c22958
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Thu Feb 8 17:54:31 2024 +0000

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

    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>
    (cherry picked from commit 1609fdff16f17ead37666f6d0e801800ee3d04d2)

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

--- Comment #13 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Iain D Sandoe
<iains@gcc.gnu.org>:

https://gcc.gnu.org/g:77f17e405a0669db9a6c8af69bde6eb1170f48bd

commit r12-10376-g77f17e405a0669db9a6c8af69bde6eb1170f48bd
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Thu Feb 8 17:54:31 2024 +0000

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

    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>
    (cherry picked from commit 1609fdff16f17ead37666f6d0e801800ee3d04d2)

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

--- Comment #14 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Iain D Sandoe
<iains@gcc.gnu.org>:

https://gcc.gnu.org/g:8c19cb9c6186b65f1858c91d423238a00ffe0c01

commit r11-11403-g8c19cb9c6186b65f1858c91d423238a00ffe0c01
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Thu Feb 8 17:54:31 2024 +0000

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

    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>
    (cherry picked from commit 1609fdff16f17ead37666f6d0e801800ee3d04d2)

[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #15 from Iain Sandoe <iains at gcc dot gnu.org> ---
fixed on open branches.