[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397 Iain Sandoe changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #15 from Iain Sandoe --- fixed on open branches.
[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397 --- Comment #14 from GCC Commits --- The releases/gcc-11 branch has been updated by Iain D Sandoe : https://gcc.gnu.org/g:8c19cb9c6186b65f1858c91d423238a00ffe0c01 commit r11-11403-g8c19cb9c6186b65f1858c91d423238a00ffe0c01 Author: Iain Sandoe Date: Thu Feb 8 17:54:31 2024 + 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 Co-authored-by: Jonathan Wakely (cherry picked from commit 1609fdff16f17ead37666f6d0e801800ee3d04d2)
[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397 --- Comment #13 from GCC Commits --- The releases/gcc-12 branch has been updated by Iain D Sandoe : https://gcc.gnu.org/g:77f17e405a0669db9a6c8af69bde6eb1170f48bd commit r12-10376-g77f17e405a0669db9a6c8af69bde6eb1170f48bd Author: Iain Sandoe Date: Thu Feb 8 17:54:31 2024 + 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 Co-authored-by: Jonathan Wakely (cherry picked from commit 1609fdff16f17ead37666f6d0e801800ee3d04d2)
[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397 --- Comment #12 from GCC Commits --- The releases/gcc-13 branch has been updated by Iain D Sandoe : https://gcc.gnu.org/g:ae11f0154116f4e5fa8769b1ea1600b1b1c22958 commit r13-8577-gae11f0154116f4e5fa8769b1ea1600b1b1c22958 Author: Iain Sandoe Date: Thu Feb 8 17:54:31 2024 + 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 Co-authored-by: Jonathan Wakely (cherry picked from commit 1609fdff16f17ead37666f6d0e801800ee3d04d2)
[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397 --- Comment #11 from GCC Commits --- The master branch has been updated by Iain D Sandoe : https://gcc.gnu.org/g:1609fdff16f17ead37666f6d0e801800ee3d04d2 commit r14-9073-g1609fdff16f17ead37666f6d0e801800ee3d04d2 Author: Iain Sandoe Date: Thu Feb 8 17:54:31 2024 + 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 Co-authored-by: Jonathan Wakely
[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397
--- Comment #10 from Iain Sandoe ---
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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397 --- Comment #9 from Jakub Jelinek --- 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397
--- Comment #8 from Jakub Jelinek ---
(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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397
--- Comment #7 from Iain Sandoe ---
(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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397
Jakub Jelinek changed:
What|Removed |Added
CC||jakub at gcc dot gnu.org
--- Comment #6 from Jakub Jelinek ---
(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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397
--- Comment #5 from Jonathan Wakely ---
Maybe:
ifneq ($(filter %-darwin%,${host_alias}),)
[Bug target/112397] Two persistent libstdc++ test failures on x86_64-apple-darwin
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397
--- Comment #4 from Jonathan Wakely ---
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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397 Iain Sandoe changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2024-02-07 Ever confirmed|0 |1 --- Comment #3 from Iain Sandoe --- (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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112397 Iain Sandoe changed: What|Removed |Added Component|libstdc++ |target --- Comment #2 from Iain Sandoe --- (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.
