public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)
@ 2023-08-31 15:25 Christophe Lyon
  2023-08-31 17:05 ` Hans-Peter Nilsson
  2023-08-31 17:42 ` Jonathan Wakely
  0 siblings, 2 replies; 7+ messages in thread
From: Christophe Lyon @ 2023-08-31 15:25 UTC (permalink / raw)
  To: GCC Patches, Jonathan Wakely, libstdc++


[-- Attachment #1.1: Type: text/plain, Size: 1349 bytes --]

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?

2023-08-31  Christophe Lyon  <christophe.lyon@linaro.org>

libstdc++-v3/ChangeLog:

        PR libstdc++/111238
        * configure: Regenerate.
        * configure.ac: Call GLIBCXX_CHECK_LINKER_FEATURES in cross,
        non-Canadian builds.

[-- Attachment #2: 0001-libstdc-Use-GLIBCXX_CHECK_LINKER_FEATURES-for-cross-.patch --]
[-- Type: text/x-patch, Size: 7553 bytes --]

From 026b173107f19d4a1bf4e1cd05befa97c65d01f4 Mon Sep 17 00:00:00 2001
From: Christophe Lyon <christophe.lyon@linaro.org>
Date: Thu, 31 Aug 2023 13:50:16 +0000
Subject: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds
 (PR111238)

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?

2023-08-31  Christophe Lyon  <christophe.lyon@linaro.org>

libstdc++-v3/ChangeLog:

	PR libstdc++/111238
	* configure: Regenerate.
	* configure.ac: Call GLIBCXX_CHECK_LINKER_FEATURES in cross,
	non-Canadian builds.
---
 libstdc++-v3/configure    | 154 ++++++++++++++++++++++++++++++++++++++
 libstdc++-v3/configure.ac |   4 +
 2 files changed, 158 insertions(+)

diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index c4da56c3042..948dab4f9a0 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -29823,6 +29823,160 @@ else
     CANADIAN=yes
   else
     CANADIAN=no
+  fi
+
+  if test $CANADIAN = no; then
+
+  # If we're not using GNU ld, then there's no point in even trying these
+  # tests.  Check for that first.  We should have already tested for gld
+  # by now (in libtool), but require it now just to be safe...
+  test -z "$SECTION_LDFLAGS" && SECTION_LDFLAGS=''
+  test -z "$OPT_LDFLAGS" && OPT_LDFLAGS=''
+
+
+
+  # The name set by libtool depends on the version of libtool.  Shame on us
+  # for depending on an impl detail, but c'est la vie.  Older versions used
+  # ac_cv_prog_gnu_ld, but now it's lt_cv_prog_gnu_ld, and is copied back on
+  # top of with_gnu_ld (which is also set by --with-gnu-ld, so that actually
+  # makes sense).  We'll test with_gnu_ld everywhere else, so if that isn't
+  # set (hence we're using an older libtool), then set it.
+  if test x${with_gnu_ld+set} != xset; then
+    if test x${ac_cv_prog_gnu_ld+set} != xset; then
+      # We got through "ac_require(ac_prog_ld)" and still not set?  Huh?
+      with_gnu_ld=no
+    else
+      with_gnu_ld=$ac_cv_prog_gnu_ld
+    fi
+  fi
+
+  # Start by getting the version number.  I think the libtool test already
+  # does some of this, but throws away the result.
+  glibcxx_ld_is_gold=no
+  glibcxx_ld_is_mold=no
+  if test x"$with_gnu_ld" = x"yes"; then
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for ld version" >&5
+$as_echo_n "checking for ld version... " >&6; }
+
+    if $LD --version 2>/dev/null | grep 'GNU gold' >/dev/null 2>&1; then
+      glibcxx_ld_is_gold=yes
+    elif $LD --version 2>/dev/null | grep 'mold' >/dev/null 2>&1; then
+      glibcxx_ld_is_mold=yes
+    fi
+    ldver=`$LD --version 2>/dev/null |
+	   sed -e 's/[. ][0-9]\{8\}$//;s/.* \([^ ]\{1,\}\)$/\1/; q'`
+
+    glibcxx_gnu_ld_version=`echo $ldver | \
+	   $AWK -F. '{ if (NF<3) $3=0; print ($1*100+$2)*100+$3 }'`
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: $glibcxx_gnu_ld_version" >&5
+$as_echo "$glibcxx_gnu_ld_version" >&6; }
+  fi
+
+  # Set --gc-sections.
+  glibcxx_have_gc_sections=no
+  if test "$glibcxx_ld_is_gold" = "yes" || test "$glibcxx_ld_is_mold" = "yes" ; then
+    if $LD --help 2>/dev/null | grep gc-sections >/dev/null 2>&1; then
+      glibcxx_have_gc_sections=yes
+    fi
+  else
+    glibcxx_gcsections_min_ld=21602
+    if test x"$with_gnu_ld" = x"yes" &&
+	test $glibcxx_gnu_ld_version -gt $glibcxx_gcsections_min_ld ; then
+      glibcxx_have_gc_sections=yes
+    fi
+  fi
+  if test "$glibcxx_have_gc_sections" = "yes"; then
+    # Sufficiently young GNU ld it is!  Joy and bunny rabbits!
+    # NB: This flag only works reliably after 2.16.1. Configure tests
+    # for this are difficult, so hard wire a value that should work.
+
+    ac_test_CFLAGS="${CFLAGS+set}"
+    ac_save_CFLAGS="$CFLAGS"
+    CFLAGS='-Wl,--gc-sections'
+
+    # Check for -Wl,--gc-sections
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for ld that supports -Wl,--gc-sections" >&5
+$as_echo_n "checking for ld that supports -Wl,--gc-sections... " >&6; }
+    if test x$gcc_no_link = xyes; then
+  as_fn_error $? "Link tests are not allowed after GCC_NO_EXECUTABLES." "$LINENO" 5
+fi
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+ int one(void) { return 1; }
+     int two(void) { return 2; }
+
+int
+main ()
+{
+ two();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  ac_gcsections=yes
+else
+  ac_gcsections=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+    if test "$ac_gcsections" = "yes"; then
+      rm -f conftest.c
+      touch conftest.c
+      if $CC -c conftest.c; then
+	if $LD --gc-sections -o conftest conftest.o 2>&1 | \
+	   grep "Warning: gc-sections option ignored" > /dev/null; then
+	  ac_gcsections=no
+	fi
+      fi
+      rm -f conftest.c conftest.o conftest
+    fi
+    if test "$ac_gcsections" = "yes"; then
+      SECTION_LDFLAGS="-Wl,--gc-sections $SECTION_LDFLAGS"
+    fi
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_gcsections" >&5
+$as_echo "$ac_gcsections" >&6; }
+
+    if test "$ac_test_CFLAGS" = set; then
+      CFLAGS="$ac_save_CFLAGS"
+    else
+      # this is the suspicious part
+      CFLAGS=''
+    fi
+  fi
+
+  # Set -z,relro.
+  # Note this is only for shared objects.
+  ac_ld_relro=no
+  if test x"$with_gnu_ld" = x"yes"; then
+    # cygwin and mingw uses PE, which has no ELF relro support,
+    # multi target ld may confuse configure machinery
+    case "$host" in
+    *-*-cygwin*)
+     ;;
+    *-*-mingw*)
+     ;;
+    *)
+      { $as_echo "$as_me:${as_lineno-$LINENO}: checking for ld that supports -Wl,-z,relro" >&5
+$as_echo_n "checking for ld that supports -Wl,-z,relro... " >&6; }
+      cxx_z_relo=`$LD -v --help 2>/dev/null | grep "z relro"`
+      if test -n "$cxx_z_relo"; then
+        OPT_LDFLAGS="-Wl,-z,relro"
+        ac_ld_relro=yes
+      fi
+      { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_ld_relro" >&5
+$as_echo "$ac_ld_relro" >&6; }
+    esac
+  fi
+
+  # Set linker optimization flags.
+  if test x"$with_gnu_ld" = x"yes"; then
+    OPT_LDFLAGS="-Wl,-O1 $OPT_LDFLAGS"
+  fi
+
+
+
+
   fi
 
   # Construct crosses by hand, eliminating bits that need ld...
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index fc0f2522027..49472353435 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -325,6 +325,10 @@ else
     CANADIAN=no
   fi
 
+  if test $CANADIAN = no; then
+    GLIBCXX_CHECK_LINKER_FEATURES
+  fi
+
   # Construct crosses by hand, eliminating bits that need ld...
   # GLIBCXX_CHECK_MATH_SUPPORT
 
-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)
  2023-08-31 15:25 [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238) Christophe Lyon
@ 2023-08-31 17:05 ` Hans-Peter Nilsson
  2023-08-31 19:39   ` Hans-Peter Nilsson
  2023-08-31 17:42 ` Jonathan Wakely
  1 sibling, 1 reply; 7+ messages in thread
From: Hans-Peter Nilsson @ 2023-08-31 17:05 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: gcc-patches, jwakely, libstdc++

> Date: Thu, 31 Aug 2023 17:25:45 +0200
> From: Christophe Lyon via Gcc-patches <gcc-patches@gcc.gnu.org>

> 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  <christophe.lyon@linaro.org>
> 
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)
  2023-08-31 15:25 [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238) Christophe Lyon
  2023-08-31 17:05 ` Hans-Peter Nilsson
@ 2023-08-31 17:42 ` Jonathan Wakely
  2023-08-31 17:53   ` Jonathan Wakely
  2023-08-31 19:43   ` Jonathan Wakely
  1 sibling, 2 replies; 7+ messages in thread
From: Jonathan Wakely @ 2023-08-31 17:42 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: GCC Patches, libstdc++

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

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? 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.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1883 bytes --]

commit eea73ea3bdd44a8f7d8c0f54b15bfba9058f6ce8
Author: Jonathan Wakely <jwakely@redhat.com>
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)
  2023-08-31 17:42 ` Jonathan Wakely
@ 2023-08-31 17:53   ` Jonathan Wakely
  2023-08-31 19:43   ` Jonathan Wakely
  1 sibling, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2023-08-31 17:53 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Christophe Lyon, GCC Patches, libstdc++

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

On Thu, 31 Aug 2023, 18:43 Jonathan Wakely via Libstdc++, <
libstdc++@gcc.gnu.org> 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 } }
>

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)
  2023-08-31 17:05 ` Hans-Peter Nilsson
@ 2023-08-31 19:39   ` Hans-Peter Nilsson
  0 siblings, 0 replies; 7+ messages in thread
From: Hans-Peter Nilsson @ 2023-08-31 19:39 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: christophe.lyon, gcc-patches, jwakely, libstdc++

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Thu, 31 Aug 2023 19:05:19 +0200

> > Date: Thu, 31 Aug 2023 17:25:45 +0200
> > From: Christophe Lyon via Gcc-patches <gcc-patches@gcc.gnu.org>

> > 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  <christophe.lyon@linaro.org>
> > 
> > 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)
  2023-08-31 17:42 ` Jonathan Wakely
  2023-08-31 17:53   ` Jonathan Wakely
@ 2023-08-31 19:43   ` Jonathan Wakely
  2023-09-01  8:27     ` Christophe Lyon
  1 sibling, 1 reply; 7+ messages in thread
From: Jonathan Wakely @ 2023-08-31 19:43 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: GCC Patches, libstdc++

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.

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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238)
  2023-08-31 19:43   ` Jonathan Wakely
@ 2023-09-01  8:27     ` Christophe Lyon
  0 siblings, 0 replies; 7+ messages in thread
From: Christophe Lyon @ 2023-09-01  8:27 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: GCC Patches, libstdc++

[-- 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.
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-09-01  8:27 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-31 15:25 [PATCH] libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds (PR111238) Christophe Lyon
2023-08-31 17:05 ` Hans-Peter Nilsson
2023-08-31 19:39   ` Hans-Peter Nilsson
2023-08-31 17:42 ` Jonathan Wakely
2023-08-31 17:53   ` Jonathan Wakely
2023-08-31 19:43   ` Jonathan Wakely
2023-09-01  8:27     ` Christophe Lyon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).