[v2] gdbserver/linux: probe for libiconv in configure

Message ID 20240301152716.13929-1-simon.marchi@efficios.com
State New
Headers
Series [v2] gdbserver/linux: probe for libiconv in configure |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Simon Marchi March 1, 2024, 3:27 p.m. UTC
  Changes in v2:

 - Add dependencies on (in-tree) libiconv in top-level build system.
 - Minor rewording of comment to make it clear that libiconv is a hard
   requirement for Linux hosts and unused for other hosts.

Make gdbserver's build system locate libiconv when building for Linux.

Commit 07b3255c3bae ("Filter invalid encodings from Linux thread names")
make libiconv madantory for building gdbserver on Linux.

While trying to cross-compile gdb for xtensa-fsf-linux-uclibc (with a
toolchain generated with crosstool-ng), I got:

    /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:48:10: fatal error: iconv.h: No such file or directory
       48 | #include <iconv.h>
          |          ^~~~~~~~~

I downloaded GNU libiconv, built it for that host, and installed it in
an arbitrary directory.  I had to modify the gdbserver build system to
locate libiconv and use it, the result is this patch.

I eventually found that crosstool-ng has an config option to make
uclibc provide an implementation of iconv, which is of course much
easier.  But given that this patch is now written, I think it would be
worth merging it, it could help some people who do not have iconv
built-in their libc in the future (and may not have the luxury of
rebuilding their libc like I do).

Using AM_ICONV in configure.ac adds these options for configure (the
same we have for gdb):

    --with-libiconv-prefix[=DIR]  search for libiconv in DIR/include and DIR/lib
    --without-libiconv-prefix     don't search for libiconv in includedir and libdir
    --with-libiconv-type=TYPE     type of library to search for (auto/static/shared)

It sets the `LIBICONV` variable with whatever is needed to link with
libiconv, and adds the necessary `-I` flag to `CPPFLAGS`.

To avoid unnecessarily linking against libiconv on hosts that don't need
it, set `MAYBE_LIBICONV` with the contents of `LIBICONV` only if the
host is Linux, and use `MAYBE_LIBICONV` in `Makefile.in`.

Since libiconv is a hard requirement for Linux hosts, error out if it is
not found.

The bits in acinclude.m4 are similar to what we have in
gdb/acinclude.m4.

Update the top-level build system to support building against an in-tree
libiconv (I did not test this part though).  Something tells me that the
all-gdbserver dependency on all-libiconv is unnecessary, since there is
already a dependency of configure-gdbserver on all-libiconv (and
all-gdbserver surely depends on configure-gdbserver).  I just copied
what's done for GDB though.

ChangeLog:

	* Makefile.def: Add configure-gdbserver and all-gdbserver
	dependencies on all-libiconv.
	* Makefile.in: Re-generate.

Change-Id: I90f8ef88dd4917df5a68b45550d93622fc9cfed4
---
 Makefile.def           |   2 +
 Makefile.in            |   2 +
 gdbserver/Makefile.in  |   4 +-
 gdbserver/acinclude.m4 |   7 +
 gdbserver/aclocal.m4   |   4 -
 gdbserver/config.in    |   3 +
 gdbserver/configure    | 352 +++++++++++++++++++++++++++++++++++++++++
 gdbserver/configure.ac |  16 ++
 8 files changed, 385 insertions(+), 5 deletions(-)


base-commit: 5764bd565a9ca570e330c44c32cdbf1dce212c02
  

Comments

Tom Tromey March 14, 2024, 3:44 p.m. UTC | #1
>>>>> Simon Marchi <simon.marchi@efficios.com> writes:

> Changes in v2:
>  - Add dependencies on (in-tree) libiconv in top-level build system.
>  - Minor rewording of comment to make it clear that libiconv is a hard
>    requirement for Linux hosts and unused for other hosts.

Looks good to me.
Approved-By: Tom Tromey <tom@tromey.com>

Tom
  
Simon Marchi March 14, 2024, 5:42 p.m. UTC | #2
On 2024-03-14 11:44, Tom Tromey wrote:
>>>>>> Simon Marchi <simon.marchi@efficios.com> writes:
> 
>> Changes in v2:
>>  - Add dependencies on (in-tree) libiconv in top-level build system.
>>  - Minor rewording of comment to make it clear that libiconv is a hard
>>    requirement for Linux hosts and unused for other hosts.
> 
> Looks good to me.
> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Tom

Thanks, pushed.

Simon
  

Patch

diff --git a/Makefile.def b/Makefile.def
index 3e00a729a0c4..6b143ab10c63 100644
--- a/Makefile.def
+++ b/Makefile.def
@@ -461,9 +461,11 @@  dependencies = { module=all-gdb; on=all-libbacktrace; };
 
 // Host modules specific to gdbserver.
 dependencies = { module=configure-gdbserver; on=all-gnulib; };
+dependencies = { module=configure-gdbserver; on=all-libiconv; };
 dependencies = { module=all-gdbserver; on=all-gdbsupport; };
 dependencies = { module=all-gdbserver; on=all-gnulib; };
 dependencies = { module=all-gdbserver; on=all-libiberty; };
+dependencies = { module=all-gdbserver; on=all-libiconv; };
 
 dependencies = { module=configure-libgui; on=configure-tcl; };
 dependencies = { module=configure-libgui; on=configure-tk; };
diff --git a/Makefile.in b/Makefile.in
index a1f64a2ab5aa..589588dc23b9 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -67904,7 +67904,9 @@  all-gdb: maybe-all-opcodes
 all-gdb: maybe-all-libdecnumber
 all-gdb: maybe-all-libctf
 all-gdb: maybe-all-libbacktrace
+configure-gdbserver: maybe-all-libiconv
 all-gdbserver: maybe-all-libiberty
+all-gdbserver: maybe-all-libiconv
 configure-gdbsupport: maybe-configure-gettext
 all-gdbsupport: maybe-all-gettext
 configure-gprof: maybe-configure-gettext
diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in
index d12f8746611b..c7120895a26d 100644
--- a/gdbserver/Makefile.in
+++ b/gdbserver/Makefile.in
@@ -152,6 +152,8 @@  PTHREAD_LIBS = @PTHREAD_LIBS@
 
 WIN32APILIBS = @WIN32APILIBS@
 
+MAYBE_LIBICONV = @MAYBE_LIBICONV@
+
 # INTERNAL_CFLAGS is the aggregate of all other *CFLAGS macros.
 INTERNAL_CFLAGS_BASE = ${GLOBAL_CFLAGS} \
 	${PROFILE_CFLAGS} ${INCLUDE_CFLAGS} ${CPPFLAGS} $(PTHREAD_CFLAGS)
@@ -354,7 +356,7 @@  gdbserver$(EXEEXT): $(sort $(OBS)) ${CDEPS} $(LIBGNU) $(LIBIBERTY) \
 		$(CXXFLAGS) \
 		-o gdbserver$(EXEEXT) $(OBS) $(GDBSUPPORT) $(LIBGNU) \
 		$(LIBGNU_EXTRA_LIBS) $(LIBIBERTY) $(INTL) \
-		$(GDBSERVER_LIBS) $(XM_CLIBS) $(WIN32APILIBS)
+		$(GDBSERVER_LIBS) $(XM_CLIBS) $(WIN32APILIBS) $(MAYBE_LIBICONV)
 
 gdbreplay$(EXEEXT): $(sort $(GDBREPLAY_OBS)) $(LIBGNU) $(LIBIBERTY) \
 		$(INTL_DEPS) $(GDBSUPPORT)
diff --git a/gdbserver/acinclude.m4 b/gdbserver/acinclude.m4
index db69b7d243b1..5a00c6e9878b 100644
--- a/gdbserver/acinclude.m4
+++ b/gdbserver/acinclude.m4
@@ -18,6 +18,13 @@  dnl anything else in gdbserver.
 m4_include(../config/codeset.m4)
 m4_include(../gdbsupport/common.m4)
 
+dnl For AM_ICONV.  We need to explicitly include these other files before
+dnl iconv.m4 to avoid warnings.
+m4_include([../config/lib-ld.m4])
+m4_include([../config/lib-prefix.m4])
+m4_include([../config/lib-link.m4])
+m4_include([../config/iconv.m4])
+
 dnl For libiberty_INIT.
 m4_include(../gdbsupport/libiberty.m4)
 
diff --git a/gdbserver/aclocal.m4 b/gdbserver/aclocal.m4
index db9266266bed..fe94a5bffc05 100644
--- a/gdbserver/aclocal.m4
+++ b/gdbserver/aclocal.m4
@@ -204,12 +204,8 @@  m4_include([../config/ax_pthread.m4])
 m4_include([../config/depstand.m4])
 m4_include([../config/gettext-sister.m4])
 m4_include([../config/gettext.m4])
-m4_include([../config/iconv.m4])
 m4_include([../config/intlmacosx.m4])
 m4_include([../config/lead-dot.m4])
-m4_include([../config/lib-ld.m4])
-m4_include([../config/lib-link.m4])
-m4_include([../config/lib-prefix.m4])
 m4_include([../config/nls.m4])
 m4_include([../config/override.m4])
 m4_include([../config/po.m4])
diff --git a/gdbserver/config.in b/gdbserver/config.in
index ca4b9f4d6f20..47e1e722fe0a 100644
--- a/gdbserver/config.in
+++ b/gdbserver/config.in
@@ -402,6 +402,9 @@ 
 /* Define to 1 if you have the <ws2tcpip.h> header file. */
 #undef HAVE_WS2TCPIP_H
 
+/* Define as const if the declaration of iconv() needs const. */
+#undef ICONV_CONST
+
 /* Define to the address where bug reports for this package should be sent. */
 #undef PACKAGE_BUGREPORT
 
diff --git a/gdbserver/configure b/gdbserver/configure
index 0942ef173d45..b85db9cd49df 100755
--- a/gdbserver/configure
+++ b/gdbserver/configure
@@ -623,6 +623,7 @@  ac_header_list=
 gt_needs=
 ac_subst_vars='LTLIBOBJS
 LIBOBJS
+MAYBE_LIBICONV
 GNULIB_STDINT_H
 extra_libraries
 IPA_DEPFILES
@@ -14736,6 +14737,357 @@  if test x"$STDINT_H" != x; then
 fi
 
 
+# Check for libiconv.  It is a requirement for Linux hosts, and others hosts
+# don't use it at all.  Define MAYBE_LIBICONV only if the host is Linux.
+
+
+
+
+
+          am_save_CPPFLAGS="$CPPFLAGS"
+
+  for element in $INCICONV; do
+    haveit=
+    for x in $CPPFLAGS; do
+
+  acl_save_prefix="$prefix"
+  prefix="$acl_final_prefix"
+  acl_save_exec_prefix="$exec_prefix"
+  exec_prefix="$acl_final_exec_prefix"
+  eval x=\"$x\"
+  exec_prefix="$acl_save_exec_prefix"
+  prefix="$acl_save_prefix"
+
+      if test "X$x" = "X$element"; then
+        haveit=yes
+        break
+      fi
+    done
+    if test -z "$haveit"; then
+      CPPFLAGS="${CPPFLAGS}${CPPFLAGS:+ }$element"
+    fi
+  done
+
+
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for iconv" >&5
+$as_echo_n "checking for iconv... " >&6; }
+if ${am_cv_func_iconv+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+
+    am_cv_func_iconv="no, consider installing GNU libiconv"
+    am_cv_lib_iconv=no
+    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+#include <stdlib.h>
+#include <iconv.h>
+
+int
+main ()
+{
+iconv_t cd = iconv_open("","");
+           iconv(cd,NULL,NULL,NULL,NULL);
+           iconv_close(cd);
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  am_cv_func_iconv=yes
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+    if test "$am_cv_func_iconv" != yes; then
+      am_save_LIBS="$LIBS"
+      LIBS="$LIBS $LIBICONV"
+      cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+#include <stdlib.h>
+#include <iconv.h>
+
+int
+main ()
+{
+iconv_t cd = iconv_open("","");
+             iconv(cd,NULL,NULL,NULL,NULL);
+             iconv_close(cd);
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  am_cv_lib_iconv=yes
+        am_cv_func_iconv=yes
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+      LIBS="$am_save_LIBS"
+    fi
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $am_cv_func_iconv" >&5
+$as_echo "$am_cv_func_iconv" >&6; }
+  if test "$am_cv_func_iconv" = yes; then
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for working iconv" >&5
+$as_echo_n "checking for working iconv... " >&6; }
+if ${am_cv_func_iconv_works+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+
+                  am_save_LIBS="$LIBS"
+      if test $am_cv_lib_iconv = yes; then
+        LIBS="$LIBS $LIBICONV"
+      fi
+      am_cv_func_iconv_works=no
+      for ac_iconv_const in '' 'const'; do
+        if test "$cross_compiling" = yes; then :
+  case "$host_os" in
+             aix* | hpux*) am_cv_func_iconv_works="guessing no" ;;
+             *)            am_cv_func_iconv_works="guessing yes" ;;
+           esac
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+#include <iconv.h>
+#include <string.h>
+
+#ifndef ICONV_CONST
+# define ICONV_CONST $ac_iconv_const
+#endif
+
+int
+main ()
+{
+int result = 0;
+  /* Test against AIX 5.1...7.2 bug: Failures are not distinguishable from
+     successful returns.  This is even documented in
+     <https://www.ibm.com/support/knowledgecenter/ssw_aix_72/i_bostechref/iconv.html> */
+  {
+    iconv_t cd_utf8_to_88591 = iconv_open ("ISO8859-1", "UTF-8");
+    if (cd_utf8_to_88591 != (iconv_t)(-1))
+      {
+        static ICONV_CONST char input[] = "\342\202\254"; /* EURO SIGN */
+        char buf[10];
+        ICONV_CONST char *inptr = input;
+        size_t inbytesleft = strlen (input);
+        char *outptr = buf;
+        size_t outbytesleft = sizeof (buf);
+        size_t res = iconv (cd_utf8_to_88591,
+                            &inptr, &inbytesleft,
+                            &outptr, &outbytesleft);
+        if (res == 0)
+          result |= 1;
+        iconv_close (cd_utf8_to_88591);
+      }
+  }
+  /* Test against Solaris 10 bug: Failures are not distinguishable from
+     successful returns.  */
+  {
+    iconv_t cd_ascii_to_88591 = iconv_open ("ISO8859-1", "646");
+    if (cd_ascii_to_88591 != (iconv_t)(-1))
+      {
+        static ICONV_CONST char input[] = "\263";
+        char buf[10];
+        ICONV_CONST char *inptr = input;
+        size_t inbytesleft = strlen (input);
+        char *outptr = buf;
+        size_t outbytesleft = sizeof (buf);
+        size_t res = iconv (cd_ascii_to_88591,
+                            &inptr, &inbytesleft,
+                            &outptr, &outbytesleft);
+        if (res == 0)
+          result |= 2;
+        iconv_close (cd_ascii_to_88591);
+      }
+  }
+  /* Test against AIX 6.1..7.1 bug: Buffer overrun.  */
+  {
+    iconv_t cd_88591_to_utf8 = iconv_open ("UTF-8", "ISO-8859-1");
+    if (cd_88591_to_utf8 != (iconv_t)(-1))
+      {
+        static ICONV_CONST char input[] = "\304";
+        static char buf[2] = { (char)0xDE, (char)0xAD };
+        ICONV_CONST char *inptr = input;
+        size_t inbytesleft = 1;
+        char *outptr = buf;
+        size_t outbytesleft = 1;
+        size_t res = iconv (cd_88591_to_utf8,
+                            &inptr, &inbytesleft,
+                            &outptr, &outbytesleft);
+        if (res != (size_t)(-1) || outptr - buf > 1 || buf[1] != (char)0xAD)
+          result |= 4;
+        iconv_close (cd_88591_to_utf8);
+      }
+  }
+#if 0 /* This bug could be worked around by the caller.  */
+  /* Test against HP-UX 11.11 bug: Positive return value instead of 0.  */
+  {
+    iconv_t cd_88591_to_utf8 = iconv_open ("utf8", "iso88591");
+    if (cd_88591_to_utf8 != (iconv_t)(-1))
+      {
+        static ICONV_CONST char input[] = "\304rger mit b\366sen B\374bchen ohne Augenma\337";
+        char buf[50];
+        ICONV_CONST char *inptr = input;
+        size_t inbytesleft = strlen (input);
+        char *outptr = buf;
+        size_t outbytesleft = sizeof (buf);
+        size_t res = iconv (cd_88591_to_utf8,
+                            &inptr, &inbytesleft,
+                            &outptr, &outbytesleft);
+        if ((int)res > 0)
+          result |= 8;
+        iconv_close (cd_88591_to_utf8);
+      }
+  }
+#endif
+  /* Test against HP-UX 11.11 bug: No converter from EUC-JP to UTF-8 is
+     provided.  */
+  {
+    /* Try standardized names.  */
+    iconv_t cd1 = iconv_open ("UTF-8", "EUC-JP");
+    /* Try IRIX, OSF/1 names.  */
+    iconv_t cd2 = iconv_open ("UTF-8", "eucJP");
+    /* Try AIX names.  */
+    iconv_t cd3 = iconv_open ("UTF-8", "IBM-eucJP");
+    /* Try HP-UX names.  */
+    iconv_t cd4 = iconv_open ("utf8", "eucJP");
+    if (cd1 == (iconv_t)(-1) && cd2 == (iconv_t)(-1)
+        && cd3 == (iconv_t)(-1) && cd4 == (iconv_t)(-1))
+      result |= 16;
+    if (cd1 != (iconv_t)(-1))
+      iconv_close (cd1);
+    if (cd2 != (iconv_t)(-1))
+      iconv_close (cd2);
+    if (cd3 != (iconv_t)(-1))
+      iconv_close (cd3);
+    if (cd4 != (iconv_t)(-1))
+      iconv_close (cd4);
+  }
+  return result;
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_run "$LINENO"; then :
+  am_cv_func_iconv_works=yes
+fi
+rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
+  conftest.$ac_objext conftest.beam conftest.$ac_ext
+fi
+
+        test "$am_cv_func_iconv_works" = no || break
+      done
+      LIBS="$am_save_LIBS"
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $am_cv_func_iconv_works" >&5
+$as_echo "$am_cv_func_iconv_works" >&6; }
+    case "$am_cv_func_iconv_works" in
+      *no) am_func_iconv=no am_cv_lib_iconv=no ;;
+      *)   am_func_iconv=yes ;;
+    esac
+  else
+    am_func_iconv=no am_cv_lib_iconv=no
+  fi
+  if test "$am_func_iconv" = yes; then
+
+$as_echo "#define HAVE_ICONV 1" >>confdefs.h
+
+  fi
+  if test "$am_cv_lib_iconv" = yes; then
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking how to link with libiconv" >&5
+$as_echo_n "checking how to link with libiconv... " >&6; }
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: $LIBICONV" >&5
+$as_echo "$LIBICONV" >&6; }
+  else
+            CPPFLAGS="$am_save_CPPFLAGS"
+    LIBICONV=
+    LTLIBICONV=
+  fi
+
+
+
+  if test "$am_cv_func_iconv" = yes; then
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether iconv is compatible with its POSIX signature" >&5
+$as_echo_n "checking whether iconv is compatible with its POSIX signature... " >&6; }
+if ${gl_cv_iconv_nonconst+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+#include <stdlib.h>
+#include <iconv.h>
+extern
+#ifdef __cplusplus
+"C"
+#endif
+size_t iconv (iconv_t cd, char * *inbuf, size_t *inbytesleft, char * *outbuf, size_t *outbytesleft);
+
+int
+main ()
+{
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+  gl_cv_iconv_nonconst=yes
+else
+  gl_cv_iconv_nonconst=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gl_cv_iconv_nonconst" >&5
+$as_echo "$gl_cv_iconv_nonconst" >&6; }
+  else
+            gl_cv_iconv_nonconst=yes
+  fi
+  if test $gl_cv_iconv_nonconst = yes; then
+    iconv_arg1=""
+  else
+    iconv_arg1="const"
+  fi
+
+cat >>confdefs.h <<_ACEOF
+#define ICONV_CONST $iconv_arg1
+_ACEOF
+
+
+
+      if test "$am_func_iconv" = yes; then
+    if test -n "$LIBICONV"; then
+      am_cv_func_iconv_summary='yes, in libiconv'
+    else
+      am_cv_func_iconv_summary='yes, in libc'
+    fi
+  else
+    if test "$am_cv_func_iconv" = yes; then
+      am_cv_func_iconv_summary='not working, consider installing GNU libiconv'
+    else
+      am_cv_func_iconv_summary='no, consider installing GNU libiconv'
+    fi
+  fi
+
+MAYBE_LIBICONV=
+case "$host" in
+  *linux*)
+    if test "$am_cv_func_iconv" != yes; then
+      as_fn_error $? "could not find libiconv (required for host $host)" "$LINENO" 5
+    fi
+
+    MAYBE_LIBICONV="$LIBICONV"
+    ;;
+esac
+
+
+
 ac_config_files="$ac_config_files Makefile"
 
 
diff --git a/gdbserver/configure.ac b/gdbserver/configure.ac
index 280043dd7fc3..ee0de9decbde 100644
--- a/gdbserver/configure.ac
+++ b/gdbserver/configure.ac
@@ -456,6 +456,22 @@  if test x"$STDINT_H" != x; then
 fi
 AC_SUBST(GNULIB_STDINT_H)
 
+# Check for libiconv.  It is a requirement for Linux hosts, and others hosts
+# don't use it at all.  Define MAYBE_LIBICONV only if the host is Linux.
+AM_ICONV
+MAYBE_LIBICONV=
+case "$host" in
+  *linux*)
+    if test "$am_cv_func_iconv" != yes; then
+      AC_MSG_ERROR([could not find libiconv (required for host $host)])
+    fi
+
+    MAYBE_LIBICONV="$LIBICONV"
+    ;;
+esac
+
+AC_SUBST(MAYBE_LIBICONV)
+
 AC_CONFIG_FILES([Makefile])
 
 AC_OUTPUT