bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
* Re: gnulib-tests/test-suite.log fails with musl libc 1.2.x
       [not found] <20210113174527.1ceb35aa@ncopa-desktop.lan>
@ 2021-01-17 21:29 ` Bruno Haible
  0 siblings, 0 replies; only message in thread
From: Bruno Haible @ 2021-01-17 21:29 UTC (permalink / raw)
  To: bug-gnulib; +Cc: Natanael Copa

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

Hi,

Natanael Copa wrote in
 <https://lists.gnu.org/archive/html/bug-gettext/2021-01/msg00021.html>:
> This happens on Alpine linux (3.13.0_rc4) on aarch64, armv7, mips64,
> ppc64le, s390x, x86 and x86_64.
> 
> 
> The failure is:
> 
> test-canonicalize-lgpl.c:211: assertion 'strcmp (result1, "/") == 0' failed
> Aborted
> FAIL test-canonicalize-lgpl (exit status: 134)
> 
> Problem is that musl realpath("//", ...) returns "//", which is allowed
> in POSIX.
> 
> This fixes it:
> 
> diff --git a/gettext-tools/gnulib-tests/test-canonicalize-lgpl.c b/gettext-tools/gnulib-tests/test-canonicalize-lgpl.c
> index ff82981..17842e8 100644
> --- a/gettext-tools/gnulib-tests/test-canonicalize-lgpl.c
> +++ b/gettext-tools/gnulib-tests/test-canonicalize-lgpl.c
> @@ -208,8 +208,8 @@ main (void)
>  #ifndef __MVS__
>      if (SAME_INODE (st1, st2))
>        {
> -        ASSERT (strcmp (result1, "/") == 0);
> -        ASSERT (strcmp (result2, "/") == 0);
> +        ASSERT (strcmp (result1, "/") == 0 || strcmp (result1, "//") == 0);
> +        ASSERT (strcmp (result2, "/") == 0 || strcmp (result2, "//") == 0);
>        }
>      else
>  #endif

Thanks for the report.

This failure is not reproducible on Alpine Linux 3.12; it is apparently caused
by this commit in musl libc (first released in musl libc 1.2.2):
http://git.musl-libc.org/cgit/musl/commit/?id=29ff7599a448232f2527841c2362643d246cee36

I don't agree that the test suite should be loosened for this. It is true
that POSIX allows arbitrary things to happen for file names that start
with 2 slashes. AFAIU, this is meant as a compromise for Cygwin and/or z/OS
(cf. m4/double-slash-root.m4). Making // work differently than / on Linux
is just silly and pointless.

On OSes other than Cygwin, Windows, z/OS, many applications expect that
realpath() and canonicalize_file_name() return a *canonicalized* file name,
that is, that if the result of canonicalize_file_name() on two strings is
different, the files are different (except for hard links).

So, the right fix for Gnulib is to make realpath() and canonicalize_file_name()
work like glibc does, on all Linux platforms.

The attached patches
1) add more unit tests, to check also the case with up to 3 slashes,
2) override realpath() on musl libc systems.

I experimented with a simpler override like this:

char *
rpl_realpath (const char *name, char *resolved)
# undef realpath
{
  if (name == NULL)
    {
      errno = EINVAL;
      return NULL;
    }
  /* Combine multiple leading slashes to a single one.  */
  while (name[0] == '/' && name[1] == '/')
    name++;
  return realpath (name, resolved);
}

but it fixes only the case where the input file name starts with two
slashes; it does not fix the case where a symlink's value starts with
two slashes. So, we have to enable the full glibc-based realpath() as
override.

3) Avoid a link error in a testdir of all of gnulib on Solaris 10:

cc -O  -g  -L/home/haible/prefix-x86/lib -o test-canonicalize-lgpl test-canonicalize-lgpl.o libtests.a ../gllib/libgnu.a libtests.a ../gllib/libgnu.a libtests.a  -lm -lm -lm -lm -lm -lm -lm -lm -lm -lm -lm
Undefined                       first referenced
 symbol                             in file
libintl_dcngettext                  ../gllib/libgnu.a(openat-die.o)
libintl_gettext                     ../gllib/libgnu.a(openat-die.o)
libintl_dcgettext                   ../gllib/libgnu.a(openat-die.o)
ld: fatal: symbol referencing errors. No output written to test-canonicalize-lgpl

Bruno

[-- Attachment #2: 0001-canonicalize-lgpl-tests-Add-more-tests.patch --]
[-- Type: text/x-patch, Size: 6310 bytes --]

From b67dcfce4b8d5daf32340616b8bc1b106323c14a Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Sun, 17 Jan 2021 17:22:25 +0100
Subject: [PATCH 1/3] canonicalize[-lgpl] tests: Add more tests.

* tests/test-canonicalize.c (main): Add detailed tests for // handling.
* tests/test-canonicalize-lgpl.c (main): Likewise.
---
 ChangeLog                      |  6 ++++++
 tests/test-canonicalize-lgpl.c | 44 ++++++++++++++++++++++++++++++++++++++-
 tests/test-canonicalize.c      | 47 +++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 3c4a513..dad3119 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2021-01-17  Bruno Haible  <bruno@clisp.org>
 
+	canonicalize[-lgpl] tests: Add more tests.
+	* tests/test-canonicalize.c (main): Add detailed tests for // handling.
+	* tests/test-canonicalize-lgpl.c (main): Likewise.
+
+2021-01-17  Bruno Haible  <bruno@clisp.org>
+
 	argp tests: Avoid test failures on Alpine Linux.
 	* tests/test-argp-2.sh: Use the test framework (init.sh). Use the
 	'compare' function instead of 'diff -c'.
diff --git a/tests/test-canonicalize-lgpl.c b/tests/test-canonicalize-lgpl.c
index d86f726..c0a5a55 100644
--- a/tests/test-canonicalize-lgpl.c
+++ b/tests/test-canonicalize-lgpl.c
@@ -67,6 +67,48 @@ main (void)
     ASSERT (close (fd) == 0);
   }
 
+  /* Check // handling (the easy cases, without symlinks).
+     This // handling is not mandated by POSIX.  However, many applications
+     expect that canonicalize_file_name "canonicalizes" the file name,
+     that is, that different results of canonicalize_file_name correspond
+     to different files (except for hard links).  */
+  {
+    char *result0 = canonicalize_file_name ("/etc/passwd");
+    if (result0 != NULL) /* This file does not exist on native Windows.  */
+      {
+        char *result;
+
+        result = canonicalize_file_name ("/etc//passwd");
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+
+        result = canonicalize_file_name ("/etc///passwd");
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+
+        /* On Windows, the syntax //host/share/filename denotes a file
+           in a directory named 'share', exported from host 'host'.
+           See also m4/double-slash-root.m4.  */
+#if !(defined _WIN32 || defined __CYGWIN__)
+        result = canonicalize_file_name ("//etc/passwd");
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+
+        result = canonicalize_file_name ("//etc//passwd");
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+
+        result = canonicalize_file_name ("//etc///passwd");
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+#endif
+
+        result = canonicalize_file_name ("///etc/passwd");
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+
+        result = canonicalize_file_name ("///etc//passwd");
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+
+        result = canonicalize_file_name ("///etc///passwd");
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+      }
+  }
+
   /* Check for ., .., intermediate // handling, and for error cases.  */
   {
     char *result = canonicalize_file_name (BASE "//./..//" BASE "/tra");
@@ -209,7 +251,7 @@ main (void)
     ASSERT (errno == ELOOP);
   }
 
-  /* Check that leading // is honored correctly.  */
+  /* Check that leading // within symlinks is honored correctly.  */
   {
     struct stat st1;
     struct stat st2;
diff --git a/tests/test-canonicalize.c b/tests/test-canonicalize.c
index e85ca45..dbde6f2 100644
--- a/tests/test-canonicalize.c
+++ b/tests/test-canonicalize.c
@@ -58,6 +58,51 @@ main (void)
     ASSERT (close (fd) == 0);
   }
 
+  /* Check // handling (the easy cases, without symlinks).
+     This // handling is not mandated by POSIX.  However, many applications
+     expect that canonicalize_filename_mode "canonicalizes" the file name,
+     that is, that different results of canonicalize_filename_mode correspond
+     to different files (except for hard links).  */
+  {
+    char *result0 = canonicalize_file_name ("/etc/passwd");
+    if (result0 != NULL) /* This file does not exist on native Windows.  */
+      {
+        char *result;
+
+        result = canonicalize_filename_mode ("/etc/passwd", CAN_MISSING);
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+
+        result = canonicalize_filename_mode ("/etc//passwd", CAN_MISSING);
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+
+        result = canonicalize_filename_mode ("/etc///passwd", CAN_MISSING);
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+
+        /* On Windows, the syntax //host/share/filename denotes a file
+           in a directory named 'share', exported from host 'host'.
+           See also m4/double-slash-root.m4.  */
+#if !(defined _WIN32 || defined __CYGWIN__)
+        result = canonicalize_filename_mode ("//etc/passwd", CAN_MISSING);
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+
+        result = canonicalize_filename_mode ("//etc//passwd", CAN_MISSING);
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+
+        result = canonicalize_filename_mode ("//etc///passwd", CAN_MISSING);
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+#endif
+
+        result = canonicalize_filename_mode ("///etc/passwd", CAN_MISSING);
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+
+        result = canonicalize_filename_mode ("///etc//passwd", CAN_MISSING);
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+
+        result = canonicalize_filename_mode ("///etc///passwd", CAN_MISSING);
+        ASSERT (result != NULL && strcmp (result, result0) == 0);
+      }
+  }
+
   /* Check for ., .., intermediate // handling, and for error cases.  */
   {
     char *result1 = canonicalize_file_name (BASE "//./..//" BASE "/tra");
@@ -334,7 +379,7 @@ main (void)
     free (result2);
   }
 
-  /* Check that leading // is honored correctly.  */
+  /* Check that leading // within symlinks is honored correctly.  */
   {
     struct stat st1;
     struct stat st2;
-- 
2.7.4


[-- Attachment #3: 0002-canonicalize-lgpl-Work-around-handling-in-realpath-o.patch --]
[-- Type: text/x-patch, Size: 7252 bytes --]

From 6ee6c7189f4bafbb603ef043fdf44b84c3f7532a Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Sun, 17 Jan 2021 22:07:14 +0100
Subject: [PATCH 2/3] canonicalize-lgpl: Work around // handling in realpath()
 of musl 1.2.2.

Reported by Natanael Copa <ncopa@alpinelinux.org> in
<https://lists.gnu.org/archive/html/bug-gettext/2021-01/msg00021.html>.

* m4/canonicalize.m4 (gl_FUNC_REALPATH_WORKS): Add a test whether // is
the same as /, on Linux only.
* lib/canonicalize-lgpl.c: Correct indentation of preprocessor
directives.
* doc/posix-functions/realpath.texi: Mention the musl 1.2.2 bug.
---
 ChangeLog                         | 11 +++++++++++
 doc/posix-functions/realpath.texi |  4 ++--
 lib/canonicalize-lgpl.c           | 19 ++++++++++---------
 m4/canonicalize.m4                | 35 ++++++++++++++++++++++++++++-------
 4 files changed, 51 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index dad3119..3570066 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
 2021-01-17  Bruno Haible  <bruno@clisp.org>
 
+	canonicalize-lgpl: Work around // handling in realpath() of musl 1.2.2.
+	Reported by Natanael Copa <ncopa@alpinelinux.org> in
+	<https://lists.gnu.org/archive/html/bug-gettext/2021-01/msg00021.html>.
+	* m4/canonicalize.m4 (gl_FUNC_REALPATH_WORKS): Add a test whether // is
+	the same as /, on Linux only.
+	* lib/canonicalize-lgpl.c: Correct indentation of preprocessor
+	directives.
+	* doc/posix-functions/realpath.texi: Mention the musl 1.2.2 bug.
+
+2021-01-17  Bruno Haible  <bruno@clisp.org>
+
 	canonicalize[-lgpl] tests: Add more tests.
 	* tests/test-canonicalize.c (main): Add detailed tests for // handling.
 	* tests/test-canonicalize-lgpl.c (main): Likewise.
diff --git a/doc/posix-functions/realpath.texi b/doc/posix-functions/realpath.texi
index 02837e7..a5b7c1d 100644
--- a/doc/posix-functions/realpath.texi
+++ b/doc/posix-functions/realpath.texi
@@ -22,14 +22,14 @@ Solaris 10.
 @item
 This function fails to detect trailing slashes on non-directories on
 some platforms:
-glibc 2.3.5, Mac OS X 10.13.
+glibc 2.3.5, Mac OS X 10.13, OpenBSD 6.0.
 @item
 This function fails to recognize non-directories followed @samp{..} on
 some platforms:
 cygwin.
 @item
 This function misbehaves on consecutive slashes on some platforms:
-AIX 7.
+musl libc 1.2.2, AIX 7.
 @end itemize
 
 Portability problems not fixed by Gnulib:
diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c
index 698f9ed..bf63355 100644
--- a/lib/canonicalize-lgpl.c
+++ b/lib/canonicalize-lgpl.c
@@ -141,11 +141,11 @@ suffix_requires_dir_check (char const *end)
    macOS 10.13 <https://bugs.gnu.org/30350>, and should also work on
    platforms like AIX 7.2 that need at least "/.".  */
 
-#if defined _LIBC || defined LSTAT_FOLLOWS_SLASHED_SYMLINK
+# if defined _LIBC || defined LSTAT_FOLLOWS_SLASHED_SYMLINK
 static char const dir_suffix[] = "/";
-#else
+# else
 static char const dir_suffix[] = "/./";
-#endif
+# endif
 
 /* Return true if DIR is a searchable dir, false (setting errno) otherwise.
    DIREND points to the NUL byte at the end of the DIR string.
@@ -187,13 +187,13 @@ get_path_max (void)
    to pacify GCC is known; even an explicit #pragma does not pacify GCC.
    When the GCC bug is fixed this workaround should be limited to the
    broken GCC versions.  */
-#if __GNUC_PREREQ (10, 1)
-# if defined GCC_LINT || defined lint
+# if __GNUC_PREREQ (10, 1)
+#  if defined GCC_LINT || defined lint
 __attribute__ ((__noinline__))
-# elif __OPTIMIZE__ && !__NO_INLINE__
-#  define GCC_BOGUS_WRETURN_LOCAL_ADDR
+#  elif __OPTIMIZE__ && !__NO_INLINE__
+#   define GCC_BOGUS_WRETURN_LOCAL_ADDR
+#  endif
 # endif
-#endif
 static char *
 realpath_stk (const char *name, char *resolved,
               struct scratch_buffer *rname_buf)
@@ -439,7 +439,8 @@ __realpath (const char *name, char *resolved)
 }
 libc_hidden_def (__realpath)
 versioned_symbol (libc, __realpath, realpath, GLIBC_2_3);
-#endif /* !FUNC_REALPATH_WORKS || defined _LIBC */
+
+#endif /* defined _LIBC || !FUNC_REALPATH_WORKS */
 
 
 #if SHLIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_3)
diff --git a/m4/canonicalize.m4 b/m4/canonicalize.m4
index 475fa15..6821c70 100644
--- a/m4/canonicalize.m4
+++ b/m4/canonicalize.m4
@@ -1,4 +1,4 @@
-# canonicalize.m4 serial 35
+# canonicalize.m4 serial 36
 
 dnl Copyright (C) 2003-2007, 2009-2021 Free Software Foundation, Inc.
 
@@ -91,6 +91,7 @@ AC_DEFUN([gl_FUNC_REALPATH_WORKS],
         #include <string.h>
       ]], [[
         int result = 0;
+        /* This test fails on Solaris 10.  */
         {
           char *name = realpath ("conftest.a", NULL);
           if (!(name && *name == '/'))
@@ -103,12 +104,14 @@ AC_DEFUN([gl_FUNC_REALPATH_WORKS],
             result |= 2;
           free (name);
         }
+        /* This test fails on Mac OS X 10.13, OpenBSD 6.0.  */
         {
           char *name = realpath ("conftest.a/", NULL);
           if (name != NULL)
             result |= 4;
           free (name);
         }
+        /* This test fails on AIX 7, Solaris 10.  */
         {
           char *name1 = realpath (".", NULL);
           char *name2 = realpath ("conftest.d//./..", NULL);
@@ -117,16 +120,31 @@ AC_DEFUN([gl_FUNC_REALPATH_WORKS],
           free (name1);
           free (name2);
         }
+        #ifdef __linux__
+        /* On Linux, // is the same as /. See also double-slash-root.m4.
+           realpath() should respect this.
+           This test fails on musl libc 1.2.2.  */
+        {
+          char *name = realpath ("//", NULL);
+          if (! name || strcmp (name, "/"))
+            result |= 16;
+          free (name);
+        }
+        #endif
         return result;
       ]])
      ],
      [gl_cv_func_realpath_works=yes],
-     [gl_cv_func_realpath_works=no],
+     [case $? in
+        16) gl_cv_func_realpath_works=nearly ;;
+        *)  gl_cv_func_realpath_works=no ;;
+      esac
+     ],
      [case "$host_os" in
                        # Guess yes on glibc systems.
         *-gnu* | gnu*) gl_cv_func_realpath_works="guessing yes" ;;
-                       # Guess yes on musl systems.
-        *-musl*)       gl_cv_func_realpath_works="guessing yes" ;;
+                       # Guess 'nearly' on musl systems.
+        *-musl*)       gl_cv_func_realpath_works="guessing nearly" ;;
                        # Guess no on native Windows.
         mingw*)        gl_cv_func_realpath_works="guessing no" ;;
                        # If we don't know, obey --enable-cross-guesses.
@@ -137,9 +155,12 @@ AC_DEFUN([gl_FUNC_REALPATH_WORKS],
   ])
   case "$gl_cv_func_realpath_works" in
     *yes)
-      AC_DEFINE([FUNC_REALPATH_WORKS], [1], [Define to 1 if realpath()
-        can malloc memory, always gives an absolute path, and handles
-        trailing slash correctly.])
+      AC_DEFINE([FUNC_REALPATH_WORKS], [1],
+        [Define to 1 if realpath() can malloc memory, always gives an absolute path, and handles leading slashes and a trailing slash correctly.])
+      ;;
+    *nearly)
+      AC_DEFINE([FUNC_REALPATH_NEARLY_WORKS], [1],
+        [Define to 1 if realpath() can malloc memory, always gives an absolute path, and handles a trailing slash correctly.])
       ;;
   esac
 ])
-- 
2.7.4


[-- Attachment #4: 0003-canonicalize-lgpl-tests-Fix-link-error.patch --]
[-- Type: text/x-patch, Size: 1348 bytes --]

From 8491d944e92339aebf621563de10285076b96444 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Sun, 17 Jan 2021 22:13:25 +0100
Subject: [PATCH 3/3] canonicalize-lgpl tests: Fix link error.

* modules/canonicalize-lgpl-tests (Makefile.am): Link
test-canonicalize-lgpl with $(LIBINTL).
---
 ChangeLog                       | 6 ++++++
 modules/canonicalize-lgpl-tests | 1 +
 2 files changed, 7 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 3570066..4bce821 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2021-01-17  Bruno Haible  <bruno@clisp.org>
 
+	canonicalize-lgpl tests: Fix link error.
+	* modules/canonicalize-lgpl-tests (Makefile.am): Link
+	test-canonicalize-lgpl with $(LIBINTL).
+
+2021-01-17  Bruno Haible  <bruno@clisp.org>
+
 	canonicalize-lgpl: Work around // handling in realpath() of musl 1.2.2.
 	Reported by Natanael Copa <ncopa@alpinelinux.org> in
 	<https://lists.gnu.org/archive/html/bug-gettext/2021-01/msg00021.html>.
diff --git a/modules/canonicalize-lgpl-tests b/modules/canonicalize-lgpl-tests
index 22b6b7f..73a0e32 100644
--- a/modules/canonicalize-lgpl-tests
+++ b/modules/canonicalize-lgpl-tests
@@ -14,3 +14,4 @@ configure.ac:
 Makefile.am:
 TESTS += test-canonicalize-lgpl
 check_PROGRAMS += test-canonicalize-lgpl
+test_canonicalize_lgpl_LDADD = $(LDADD) $(LIBINTL)
-- 
2.7.4


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2021-01-17 21:29 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210113174527.1ceb35aa@ncopa-desktop.lan>
2021-01-17 21:29 ` gnulib-tests/test-suite.log fails with musl libc 1.2.x Bruno Haible

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