Re: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)
On Thu, 31 Aug 2023 at 21:43, Jonathan Wakely wrote:
> On Thu, 31 Aug 2023 at 18:42, Jonathan Wakely wrote:
> >
> > On Thu, 31 Aug 2023 at 16:26, Christophe Lyon
> > wrote:
> > >
> > > As discussed in PR104167 (comments #8 and below), and PR111238, using
> > > -Wl,-gc-sections in the libstdc++ testsuite for arm-eabi
> > > (cross-toolchain) avoids link failures for a few tests:
> > >
> > > 27_io/filesystem/path/108636.cc
> >
> > I think this one probably just needs { dg-require-filesystem-ts "" }
> > because there's no point testing that we can link to the
> > std::filesystem definitions if some of those definitions are unusable
> > on the target.
> >
> > // { dg-additional-options "-Wl,--gc-sections" { target gc_sections } }
> >
> > For the rest of them, does the attached patch help?
>
> I've tested the patch for an arm-eabi cross compiler, and it fixes the
> linker errors.
>
Indeed, I just checked too, thanks!
> It doesn't change the fact that almost any use of the std::filesystem
> APIs will hit the linker errors and so require users to link with
> --gc-sections (or provide stubs for the missing functions) but that's
> for users to deal with (if anybody using newlib targets is even making
> use of those std::filesystem APIs anyway). With the patch to tzdb.cc
> we don't need to change how libstdc++ is tested for the arm-eabi cross
> target.
>
I think it's better to keep the current status (ie. drop my patch
proposal), so that we are aware of similar issues in the future.
I'd say this should be documented, not sure where?
Actually it might also be worth considering removing -gc-sections from
native builds, if there's no clear reason to use it ;-)
Thanks,
Christophe
>
> > If arm-eabi
> > doesn't define _GLIBCXX_HAVE_READLINK then there's no point even
> > trying to call filesystem::read_symlink. We can avoid a useless
> > dependency on it by reusing the same preprocessor condition that
> > filesystem::read_symlink uses.
> >
> > > std/time/clock/gps/1.cc
> > > std/time/clock/gps/io.cc
> > > std/time/clock/tai/1.cc
> > > std/time/clock/tai/io.cc
> > > std/time/clock/utc/1.cc
> > > std/time/clock/utc/io.cc
> > > std/time/clock/utc/leap_second_info.cc
> > > std/time/exceptions.cc
> > > std/time/format.cc
> > > std/time/time_zone/get_info_local.cc
> > > std/time/time_zone/get_info_sys.cc
> > > std/time/tzdb/1.cc
> > > std/time/tzdb/leap_seconds.cc
> > > std/time/tzdb_list/1.cc
> > > std/time/zoned_time/1.cc
> > > std/time/zoned_time/custom.cc
> > > std/time/zoned_time/io.cc
> > > std/time/zoned_traits.cc
> > >
> > > This patch achieves this by calling GLIBCXX_CHECK_LINKER_FEATURES in
> > > cross-build cases, like we already do for native builds. We keep not
> > > doing so in Canadian-cross builds.
> > >
> > > However, this would hide the fact that libstdc++ somehow forces the
> > > user to use -Wl,-gc-sections to avoid undefined references to chdir,
> > > mkdir, chmod, pathconf, ... so maybe it's better to keep the status
> > > quo and not apply this patch?
> >
> > I'm undecided about this for now, but let's wait for HP's cris-elf
> > testing anyway.
>
>
Re: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)
On Thu, 31 Aug 2023 at 18:42, Jonathan Wakely wrote:
>
> On Thu, 31 Aug 2023 at 16:26, Christophe Lyon
> wrote:
> >
> > As discussed in PR104167 (comments #8 and below), and PR111238, using
> > -Wl,-gc-sections in the libstdc++ testsuite for arm-eabi
> > (cross-toolchain) avoids link failures for a few tests:
> >
> > 27_io/filesystem/path/108636.cc
>
> I think this one probably just needs { dg-require-filesystem-ts "" }
> because there's no point testing that we can link to the
> std::filesystem definitions if some of those definitions are unusable
> on the target.
>
> // { dg-additional-options "-Wl,--gc-sections" { target gc_sections } }
>
> For the rest of them, does the attached patch help?
I've tested the patch for an arm-eabi cross compiler, and it fixes the
linker errors.
It doesn't change the fact that almost any use of the std::filesystem
APIs will hit the linker errors and so require users to link with
--gc-sections (or provide stubs for the missing functions) but that's
for users to deal with (if anybody using newlib targets is even making
use of those std::filesystem APIs anyway). With the patch to tzdb.cc
we don't need to change how libstdc++ is tested for the arm-eabi cross
target.
> If arm-eabi
> doesn't define _GLIBCXX_HAVE_READLINK then there's no point even
> trying to call filesystem::read_symlink. We can avoid a useless
> dependency on it by reusing the same preprocessor condition that
> filesystem::read_symlink uses.
>
> > std/time/clock/gps/1.cc
> > std/time/clock/gps/io.cc
> > std/time/clock/tai/1.cc
> > std/time/clock/tai/io.cc
> > std/time/clock/utc/1.cc
> > std/time/clock/utc/io.cc
> > std/time/clock/utc/leap_second_info.cc
> > std/time/exceptions.cc
> > std/time/format.cc
> > std/time/time_zone/get_info_local.cc
> > std/time/time_zone/get_info_sys.cc
> > std/time/tzdb/1.cc
> > std/time/tzdb/leap_seconds.cc
> > std/time/tzdb_list/1.cc
> > std/time/zoned_time/1.cc
> > std/time/zoned_time/custom.cc
> > std/time/zoned_time/io.cc
> > std/time/zoned_traits.cc
> >
> > This patch achieves this by calling GLIBCXX_CHECK_LINKER_FEATURES in
> > cross-build cases, like we already do for native builds. We keep not
> > doing so in Canadian-cross builds.
> >
> > However, this would hide the fact that libstdc++ somehow forces the
> > user to use -Wl,-gc-sections to avoid undefined references to chdir,
> > mkdir, chmod, pathconf, ... so maybe it's better to keep the status
> > quo and not apply this patch?
>
> I'm undecided about this for now, but let's wait for HP's cris-elf
> testing anyway.
Re: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)
> From: Hans-Peter Nilsson > Date: Thu, 31 Aug 2023 19:05:19 +0200 > > Date: Thu, 31 Aug 2023 17:25:45 +0200 > > From: Christophe Lyon via Gcc-patches > > However, this would hide the fact that libstdc++ somehow forces the > > user to use -Wl,-gc-sections to avoid undefined references to chdir, > > mkdir, chmod, pathconf, ... so maybe it's better to keep the status > > quo and not apply this patch? I agree with the sentiment, but maybe --gc-sections should instead be passed by default for arm-eabi when linking, with way to opt-out; as for cris-elf per below. > Datapoint: no failures for cris-elf in the listed tests - > but it instead passes --gc-sections if -O2 or -O3 is seen > for linking; see cris/cris.h. It's been like that forever, > modulo a patch in 2002 not passing it if "-r" is seen. > > Incidentally, I've been sort-of investigating a recent-ish > commit to newlib (8/8) that added a stub for getpriority, > which was apparently added due to testsuite breakage for > libstdc++ and arm-eabi, but that instead broke testsuite > results for *other* targets, as warning at link-time. Film > at 11. > > > 2023-08-31 Christophe Lyon > > > > libstdc++-v3/ChangeLog: > > > > PR libstdc++/111238 > > * configure: Regenerate. > > * configure.ac: Call GLIBCXX_CHECK_LINKER_FEATURES in cross, > > non-Canadian builds. > > On this actual patch, I can't say yay or nay though (but > leaning towards yay), but I'll test for cris-elf. Would you > mind holding off committing for a day or two? No regressions for cris-elf with this patch. Still, on one thought I'm also not wild about libstdc++ this way overriding the target, and on the other hand, I'll likely to suggest something similar (adding options) to "improve" GCC_TRY_COMPILE_OR_LINK (more targets actually linking). brgds, H-P
Re: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)
On Thu, 31 Aug 2023, 18:43 Jonathan Wakely via Libstdc++, <
libstd...@gcc.gnu.org> wrote:
> On Thu, 31 Aug 2023 at 16:26, Christophe Lyon
> wrote:
> >
> > As discussed in PR104167 (comments #8 and below), and PR111238, using
> > -Wl,-gc-sections in the libstdc++ testsuite for arm-eabi
> > (cross-toolchain) avoids link failures for a few tests:
> >
> > 27_io/filesystem/path/108636.cc
>
> I think this one probably just needs { dg-require-filesystem-ts "" }
> because there's no point testing that we can link to the
> std::filesystem definitions if some of those definitions are unusable
> on the target.
>
> // { dg-additional-options "-Wl,--gc-sections" { target gc_sections } }
>
Oops, ignore this line! I was going to suggest that we could work try
adding this line, but I think it's better to use dg-require for the
108636.cc test, and make the ones below just work.
> For the rest of them, does the attached patch help? If arm-eabi
> doesn't define _GLIBCXX_HAVE_READLINK then there's no point even
> trying to call filesystem::read_symlink. We can avoid a useless
> dependency on it by reusing the same preprocessor condition that
> filesystem::read_symlink uses.
>
> > std/time/clock/gps/1.cc
> > std/time/clock/gps/io.cc
> > std/time/clock/tai/1.cc
> > std/time/clock/tai/io.cc
> > std/time/clock/utc/1.cc
> > std/time/clock/utc/io.cc
> > std/time/clock/utc/leap_second_info.cc
> > std/time/exceptions.cc
> > std/time/format.cc
> > std/time/time_zone/get_info_local.cc
> > std/time/time_zone/get_info_sys.cc
> > std/time/tzdb/1.cc
> > std/time/tzdb/leap_seconds.cc
> > std/time/tzdb_list/1.cc
> > std/time/zoned_time/1.cc
> > std/time/zoned_time/custom.cc
> > std/time/zoned_time/io.cc
> > std/time/zoned_traits.cc
> >
> > This patch achieves this by calling GLIBCXX_CHECK_LINKER_FEATURES in
> > cross-build cases, like we already do for native builds. We keep not
> > doing so in Canadian-cross builds.
> >
> > However, this would hide the fact that libstdc++ somehow forces the
> > user to use -Wl,-gc-sections to avoid undefined references to chdir,
> > mkdir, chmod, pathconf, ... so maybe it's better to keep the status
> > quo and not apply this patch?
>
> I'm undecided about this for now, but let's wait for HP's cris-elf
> testing anyway.
>
Re: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)
On Thu, 31 Aug 2023 at 16:26, Christophe Lyon
wrote:
>
> As discussed in PR104167 (comments #8 and below), and PR111238, using
> -Wl,-gc-sections in the libstdc++ testsuite for arm-eabi
> (cross-toolchain) avoids link failures for a few tests:
>
> 27_io/filesystem/path/108636.cc
I think this one probably just needs { dg-require-filesystem-ts "" }
because there's no point testing that we can link to the
std::filesystem definitions if some of those definitions are unusable
on the target.
// { dg-additional-options "-Wl,--gc-sections" { target gc_sections } }
For the rest of them, does the attached patch help? If arm-eabi
doesn't define _GLIBCXX_HAVE_READLINK then there's no point even
trying to call filesystem::read_symlink. We can avoid a useless
dependency on it by reusing the same preprocessor condition that
filesystem::read_symlink uses.
> std/time/clock/gps/1.cc
> std/time/clock/gps/io.cc
> std/time/clock/tai/1.cc
> std/time/clock/tai/io.cc
> std/time/clock/utc/1.cc
> std/time/clock/utc/io.cc
> std/time/clock/utc/leap_second_info.cc
> std/time/exceptions.cc
> std/time/format.cc
> std/time/time_zone/get_info_local.cc
> std/time/time_zone/get_info_sys.cc
> std/time/tzdb/1.cc
> std/time/tzdb/leap_seconds.cc
> std/time/tzdb_list/1.cc
> std/time/zoned_time/1.cc
> std/time/zoned_time/custom.cc
> std/time/zoned_time/io.cc
> std/time/zoned_traits.cc
>
> This patch achieves this by calling GLIBCXX_CHECK_LINKER_FEATURES in
> cross-build cases, like we already do for native builds. We keep not
> doing so in Canadian-cross builds.
>
> However, this would hide the fact that libstdc++ somehow forces the
> user to use -Wl,-gc-sections to avoid undefined references to chdir,
> mkdir, chmod, pathconf, ... so maybe it's better to keep the status
> quo and not apply this patch?
I'm undecided about this for now, but let's wait for HP's cris-elf
testing anyway.
commit eea73ea3bdd44a8f7d8c0f54b15bfba9058f6ce8
Author: Jonathan Wakely
Date: Thu Aug 31 18:31:32 2023
libstdc++: Avoid useless dependency on read_symlink from tzdb
chrono::tzdb::current_zone uses filesystem::read_symlink, which creates
a dependency on the fs_ops.o object in libstdc++.a, which then creates
dependencies on several OS functions if --gc-sections isn't used.
In the cases where that causes linker failures, we probably don't have
readlink anyway, so the filesystem::read_symlink call will always fail.
Repeat the preprocessor conditions for filesystem::read_symlink in the
body of chrono::tzdb::current_zone so that we don't create the
dependency on fs_ops.o if it's not even going to be able to read the
symlink.
libstdc++-v3/ChangeLog:
* src/c++20/tzdb.cc (tzdb::current_zone): Check configure macros
for POSIX readlink before using filesystem::read_symlink.
diff --git a/libstdc++-v3/src/c++20/tzdb.cc b/libstdc++-v3/src/c++20/tzdb.cc
index 0fcbf6a4824..24044bb60f8 100644
--- a/libstdc++-v3/src/c++20/tzdb.cc
+++ b/libstdc++-v3/src/c++20/tzdb.cc
@@ -1635,6 +1635,9 @@ namespace std::chrono
// TODO cache this function's result?
#ifndef _AIX
+// Repeat the preprocessor condition used by filesystem::read_symlink,
+// to avoid a dependency on src/c++17/tzdb.o if it won't work anyway.
+#if defined(_GLIBCXX_HAVE_READLINK) && defined(_GLIBCXX_HAVE_SYS_STAT_H)
error_code ec;
// This should be a symlink to e.g. /usr/share/zoneinfo/Europe/London
auto path = filesystem::read_symlink("/etc/localtime", ec);
@@ -1653,6 +1656,7 @@ namespace std::chrono
return tz;
}
}
+#endif
// Otherwise, look for a file naming the time zone.
string_view files[] {
"/etc/timezone",// Debian derivates
Re: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)
> Date: Thu, 31 Aug 2023 17:25:45 +0200 > From: Christophe Lyon via Gcc-patches > As discussed in PR104167 (comments #8 and below), and PR111238, using > -Wl,-gc-sections in the libstdc++ testsuite for arm-eabi > (cross-toolchain) avoids link failures for a few tests: > > 27_io/filesystem/path/108636.cc > std/time/clock/gps/1.cc > std/time/clock/gps/io.cc > std/time/clock/tai/1.cc > std/time/clock/tai/io.cc > std/time/clock/utc/1.cc > std/time/clock/utc/io.cc > std/time/clock/utc/leap_second_info.cc > std/time/exceptions.cc > std/time/format.cc > std/time/time_zone/get_info_local.cc > std/time/time_zone/get_info_sys.cc > std/time/tzdb/1.cc > std/time/tzdb/leap_seconds.cc > std/time/tzdb_list/1.cc > std/time/zoned_time/1.cc > std/time/zoned_time/custom.cc > std/time/zoned_time/io.cc > std/time/zoned_traits.cc > > This patch achieves this by calling GLIBCXX_CHECK_LINKER_FEATURES in > cross-build cases, like we already do for native builds. We keep not > doing so in Canadian-cross builds. > > However, this would hide the fact that libstdc++ somehow forces the > user to use -Wl,-gc-sections to avoid undefined references to chdir, > mkdir, chmod, pathconf, ... so maybe it's better to keep the status > quo and not apply this patch? Datapoint: no failures for cris-elf in the listed tests - but it instead passes --gc-sections if -O2 or -O3 is seen for linking; see cris/cris.h. It's been like that forever, modulo a patch in 2002 not passing it if "-r" is seen. Incidentally, I've been sort-of investigating a recent-ish commit to newlib (8/8) that added a stub for getpriority, which was apparently added due to testsuite breakage for libstdc++ and arm-eabi, but that instead broke testsuite results for *other* targets, as warning at link-time. Film at 11. > 2023-08-31 Christophe Lyon > > libstdc++-v3/ChangeLog: > > PR libstdc++/111238 > * configure: Regenerate. > * configure.ac: Call GLIBCXX_CHECK_LINKER_FEATURES in cross, > non-Canadian builds. On this actual patch, I can't say yay or nay though (but leaning towards yay), but I'll test for cris-elf. Would you mind holding off committing for a day or two? brgds, H-P
