Re: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)

[Christophe Lyon <christophe.lyon@linaro.org>]

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

On Thu, 31 Aug 2023 at 21:43, Jonathan Wakely <jwakely@redhat.com> wrote:

> On Thu, 31 Aug 2023 at 18:42, Jonathan Wakely <jwakely@redhat.com> wrote:
> >
> > On Thu, 31 Aug 2023 at 16:26, Christophe Lyon
> > <christophe.lyon@linaro.org> 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.
>
>