immutable: Implement on native Windows

[Bruno Haible]

The 'immutable' module, so far, worked only on platforms with mmap() and
mprotect(). With this patch, it also works on native Windows.


2021-01-17  Bruno Haible  

immutable: Implement on native Windows.
* lib/immutable.h (IMMUTABLE_EFFECTIVE): Set to 1 on native Windows.
* lib/immutable.c: Include .
(CreateFileMapping): New macro.
(init_pagesize, init_mmap_file, alloc_pages, free_pages): Add
implementation for native Windows.

diff --git a/lib/immutable.c b/lib/immutable.c
index 089ad17..35f7397 100644
--- a/lib/immutable.c
+++ b/lib/immutable.c
@@ -37,27 +37,42 @@
 
 # include 
 
+# if defined _WIN32 && !defined __CYGWIN__
+
+/* Declare VirtualAlloc(), GetSystemInfo.  */
+#  define WIN32_LEAN_AND_MEAN
+#  define WIN32_EXTRA_LEAN
+#  include 
+
+/* Don't assume that UNICODE is not defined.  */
+#  undef CreateFileMapping
+#  define CreateFileMapping CreateFileMappingA
+
+# else
+
 /* Declare getpagesize().  */
-# include 
+#  include 
 /* On HP-UX, getpagesize exists, but it is not declared in  even if
the compiler options -D_HPUX_SOURCE -D_XOPEN_SOURCE=600 are used.  */
-# ifdef __hpux
+#  ifdef __hpux
 extern
-#  ifdef __cplusplus
+#   ifdef __cplusplus
"C"
-#  endif
+#   endif
int getpagesize (void);
-# endif
+#  endif
 
 /* Declare mmap(), mprotect().  */
-# include 
-# include 
+#  include 
+#  include 
 
 /* Declare open().  */
-# include 
-# include 
+#  include 
+#  include 
 
-# include "glthread/lock.h"
+#  include "glthread/lock.h"
+
+# endif
 
 
 /* = Back end of the malloc implementation = */
@@ -71,10 +86,28 @@ static void
 init_pagesize (void)
 {
   /* Simultaneous execution of this initialization in multiple threads is OK. 
*/
+# if defined _WIN32 && !defined __CYGWIN__
+  /* GetSystemInfo
+ 
+   */
+  SYSTEM_INFO info;
+  GetSystemInfo (&info);
+  pagesize = info.dwPageSize;
+# else
   pagesize = getpagesize ();
+# endif
 }
 
 
+# if defined _WIN32 && !defined __CYGWIN__
+
+static inline void
+init_mmap_file (void)
+{
+}
+
+# else
+
 /* Variables needed for obtaining memory pages via mmap().  */
 static int file_fd;
 static long file_length;
@@ -109,6 +142,8 @@ init_mmap_file (void)
   gl_once (for_mmap_once, do_init_mmap_file);
 }
 
+# endif
+
 
 /* Size of the (page-aligned) header that links the writable mapping
and the read-only mapping together.  */
@@ -121,6 +156,44 @@ init_mmap_file (void)
 static uintptr_t
 alloc_pages (size_t size)
 {
+# if defined _WIN32 && !defined __CYGWIN__
+  /* Allocate pages from the system paging file.
+ CreateFileMapping
+ 

  */
+  HANDLE h =
+CreateFileMapping (INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE | SEC_COMMIT,
+   size >> 16 >> 16, size & 0xU, NULL);
+  if (h == NULL)
+{
+  fprintf (stderr,
+   "glimm: Cannot allocate file mapping. GetLastError() = 
0x%08X\n",
+   (unsigned int) GetLastError ());
+  return 0;
+}
+  /* MapViewOfFile
+ 

  */
+  char *mem_w = (char *) MapViewOfFile (h, FILE_MAP_WRITE, 0, 0, size);
+  char *mem_r = (char *) MapViewOfFile (h, FILE_MAP_READ,  0, 0, size);
+  if (mem_w == NULL || mem_r == NULL)
+{
+  /* UnmapViewOfFile
+ 

  */
+  if (mem_w != NULL)
+UnmapViewOfFile (mem_w);
+  if (mem_r != NULL)
+UnmapViewOfFile (mem_r);
+  return 0;
+}
+  /* It is OK to call CloseHandle before UnmapViewOfFile.  The file mapping
+ object gets really closed only once all its views are unmapped.  */
+  if (!CloseHandle (h))
+{
+  UnmapViewOfFile (mem_w);
+  UnmapViewOfFile (mem_r);
+  CloseHandle (h);
+  return 0;
+}
+# else
   /* Extend the file by size/pagesize pages.  */
   long new_file_length = file_length + size;
   if (ftruncate (file_fd, new_file_length) < 0)
@@ -142,6 +215,7 @@ alloc_pages (size_t size)
   return 0;
 }
   file_length = new_file_length;
+# endif
 
   /* Link the two memory areas together.  */
   ((intptr_t *) mem_w)[0] = mem_r - mem_w;
@@ -158,10 +232,17 @@ free_pages (uintptr_t pages, size_t size)
 abort ();
   char *mem_w = (char *) pages;
   char *mem_r = mem_w + ((intptr_t *) mem_w)[0];
+# if defined _WIN32 && !defined __CYGWIN__
+  if (!UnmapViewOfFile (mem_w))
+abort ();
+  if (!UnmapViewOfFile (mem_r))
+abort ();
+# else
   if (munmap (mem_w, size) < 0)
 abort ();
   if (munmap (mem_r, size) < 0)
 abort ();
+# endif
 }
 
 /* Cygwin defines PAGESIZE in .  */
diff --git a/lib/immutable.h b/lib/immutable.h
index 75a2f5a..5f9f435 100644
-

Re: canonicalize test failures on Cygwin

[Ken Brown]

On 1/17/2021 11:04 AM, Bruno Haible wrote:

Hi Paul,

Some of the new tests that you added to test-canonicalize.c and
test-canonicalize-lgpl.c on 2020-12-24 fail on Cygwin 2.9:

   /* Check that a non-directory symlink with trailing slash yields NULL,
  and likewise for other troublesome suffixes.  */
   {
 char const *const file_name[]
   = {
  BASE "/huk/",
  BASE "/huk/.",
  BASE "/huk/./",
  BASE "/huk/./.",
  BASE "/huk/x",
  BASE "/huk/..",
  BASE "/huk/../",
  BASE "/huk/../.",
  BASE "/huk/../x",
  BASE "/huk/./..",
  BASE "/huk/././../x",
 };
 for (int i = 0; i < sizeof file_name / sizeof *file_name; i++)
   {
 ...

The tests in the 'for' loop fail for i = 5 ... 10. That is, something
with the ".." handling is not working as expected.


I took a quick look, and it appears that this is a Cygwin bug in which 
realpath() fails with ENOENT instead of ENOTDIR.  (I've only checked this for 
the first failure, but I suspect it's the same for all of them.)  I'll submit a 
patch to Cygwin to fix this, probably tomorrow.


Ken

Re: canonicalize test failures on Cygwin

[Paul Eggert]

On 1/17/21 8:04 AM, Bruno Haible wrote:

The tests in the 'for' loop fail for i = 5 ... 10. That is, something
with the ".." handling is not working as expected.

Do you have access to a Cygwin machine for investigation?


Unfortunately not; I'm really not a Cygwin guy.

OK with me if we simply disable those tests on Cygwin.

Re: gnulib-tests/test-suite.log fails with musl libc 1.2.x

[Bruno Haible]

Hi,

Natanael Copa wrote in
 :
> 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
>From b67dcfce4b8d5daf32340616b8bc1b106323c14a Mon Sep 17 00:00:00 2001
From: Bruno Haible 
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  
 
+	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  
+
 	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 cano

canonicalize test failures on Cygwin

[Bruno Haible]

Hi Paul,

Some of the new tests that you added to test-canonicalize.c and
test-canonicalize-lgpl.c on 2020-12-24 fail on Cygwin 2.9:

  /* Check that a non-directory symlink with trailing slash yields NULL,
 and likewise for other troublesome suffixes.  */
  {
char const *const file_name[]
  = {
 BASE "/huk/",
 BASE "/huk/.",
 BASE "/huk/./",
 BASE "/huk/./.",
 BASE "/huk/x",
 BASE "/huk/..",
 BASE "/huk/../",
 BASE "/huk/../.",
 BASE "/huk/../x",
 BASE "/huk/./..",
 BASE "/huk/././../x",
};
for (int i = 0; i < sizeof file_name / sizeof *file_name; i++)
  {
...

The tests in the 'for' loop fail for i = 5 ... 10. That is, something
with the ".." handling is not working as expected.

Do you have access to a Cygwin machine for investigation?

Bruno

Re: different CFLAGS for gnulib code?

[Pádraig Brady]

On 15/01/2021 08:55, Bruno Haible wrote:

I always thought that GNU package maintainers want their entire package to
be compiled with the same CFLAGS and CPPFLAGS. Would compiling the gnulib
part with options for fewer warnings be OK with you?

Paul, Pádraig, Jim, Paul, Akim, Simon, all: what's your opinion?


Re coreutils we already have different warnings,
specifically a subset of warnings are used with gnulib.

gnulib is necessarily more general/portable than a particular project,
and so could not be as generally stringent in its warnings I think.
Also a new project has more flexibility to enable a stricter set
from the start, than a mature code base like gnulib.

cheers,
Pádraig

argp tests: Avoid test failures on Alpine Linux

[Bruno Haible]

On Alpine Linux 3.12, 3.13, I see these test failures:

FAIL: test-argp-2.sh
FAIL: test-argp-version-etc-1.sh

They are caused by a 'diff' program that is not POSIX compliant: it produces
unified diffs always and does not accept a '-c' nor a '-u' option.

  diff: unrecognized option: c
  BusyBox v1.32.1 () multi-call binary.

  Usage: diff [-abBdiNqrTstw] [-L LABEL] [-S FILE] [-U LINES] FILE1 FILE2

This patch works around it, by using the 'compare' shell function from init.sh.


2021-01-17  Bruno Haible  

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'.
* tests/test-argp-version-etc-1.sh: Likewise.

diff --git a/tests/test-argp-2.sh b/tests/test-argp-2.sh
index 9388c01..4a33fac 100755
--- a/tests/test-argp-2.sh
+++ b/tests/test-argp-2.sh
@@ -16,46 +16,48 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see .  */
 
-TMP=argp.$$
+. "${srcdir=.}/init.sh"; path_prepend_ .
 
-unset ARGP_HELP_FMT
 ERR=0
 
+unset ARGP_HELP_FMT
+
 func_compare() {
 # If argp was compiled without base_name, it will display full program name.
 # If run on mingw, it will display the program name with a .exe suffix.
   sed '1{
  s,: [^ ]*/test-argp,: test-argp,
  s,: test-argp\.exe,: test-argp,
-}' | LC_ALL=C tr -d '\r' | diff -c $TMP -
+}' | LC_ALL=C tr -d '\r' > out
+  compare expected out
 }
 
 
 # Test --usage output
-cat > $TMP < expected < $TMP < expected <$TMP ]*>.$/\1<>./' | func_compare || ERR=1
 
 
 # Test ambiguous option handling
 
-${CHECKER} ./test-argp${EXEEXT} --optio 2>/dev/null && ERR=1
+${CHECKER} test-argp${EXEEXT} --optio 2>/dev/null && ERR=1
 
 
 # Run built-in tests
-${CHECKER} ./test-argp${EXEEXT} || ERR=1
-
-rm $TMP
+${CHECKER} test-argp${EXEEXT} || ERR=1
 
-exit $ERR
+Exit $ERR
diff --git a/tests/test-argp-version-etc-1.sh b/tests/test-argp-version-etc-1.sh
index 10b51d7..3ac4236 100755
--- a/tests/test-argp-version-etc-1.sh
+++ b/tests/test-argp-version-etc-1.sh
@@ -16,12 +16,14 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see .
 
-TMP=ave-expected.tmp
+. "${srcdir=.}/init.sh"; path_prepend_ .
+
+ERR=0
+
 LC_ALL=C
 export LC_ALL
-ERR=0
 
-cat > $TMP < expected .
@@ -31,13 +33,12 @@ There is NO WARRANTY, to the extent permitted by law.
 Written by Sergey Poznyakoff.
 EOT
 
-${CHECKER} ./test-argp-version-etc${EXEEXT} --version |
+${CHECKER} test-argp-version-etc${EXEEXT} --version |
  sed '1s/test-argp-version-etc (.*) .*/test-argp-version-etc (PROJECT) VERSION/
   /^Packaged by/d
   2,3 s/Copyright (C) [0-9]\{4,4\}/COPYRIGHT/' |
- tr -d '\015' |
- diff -c $TMP - || ERR=1
+ tr -d '\015' > output
 
-rm $TMP
+compare expected output || ERR=1
 
-exit $ERR
+Exit $ERR

get-rusage-data tests: Avoid test failure on musl libc

[Bruno Haible]

On Alpine Linux 3.12, 3.13, I see this test failure:

FAIL: test-get-rusage-data
==

../../gltests/test-get-rusage-data.c:60: assertion 'value3 > value1' failed
Aborted
FAIL test-get-rusage-data (exit status: 134)

This patch fixes it.


2021-01-17  Bruno Haible  

get-rusage-data tests: Avoid test failure on musl libc.
* modules/get-rusage-data-tests (Files): Add m4/musl.m4.
(configure.ac): Invoke gl_MUSL_LIBC.
* tests/test-get-rusage-data.c (main): Treat musl libc like glibc.

diff --git a/modules/get-rusage-data-tests b/modules/get-rusage-data-tests
index 0b55794..109cce5 100644
--- a/modules/get-rusage-data-tests
+++ b/modules/get-rusage-data-tests
@@ -1,10 +1,12 @@
 Files:
 tests/test-get-rusage-data.c
 tests/macros.h
+m4/musl.m4
 
 Depends-on:
 
 configure.ac:
+gl_MUSL_LIBC
 
 Makefile.am:
 TESTS += test-get-rusage-data
diff --git a/tests/test-get-rusage-data.c b/tests/test-get-rusage-data.c
index 229fd8c..c626540 100644
--- a/tests/test-get-rusage-data.c
+++ b/tests/test-get-rusage-data.c
@@ -55,7 +55,7 @@ main ()
   ASSERT (value2 >= value1);
   ASSERT (value3 >= value2);
 
-#if !(__GLIBC__ == 2 || (defined __APPLE__ && defined __MACH__) || defined 
__FreeBSD__ || defined __DragonFly__ || defined __OpenBSD__ || defined _WIN32 
|| defined __CYGWIN__)
+#if !(__GLIBC__ == 2 || MUSL_LIBC || (defined __APPLE__ && defined __MACH__) 
|| defined __FreeBSD__ || defined __DragonFly__ || defined __OpenBSD__ || 
defined _WIN32 || defined __CYGWIN__)
   /* Allocating 2.5 MB of memory should increase the data segment size.  */
   ASSERT (value3 > value1);
 #endif

immutable, get-rusage-data: Fix autoconf warning

[Bruno Haible]

While creating a testdir for the modules 'immutable', 'get-rusage-data', I get
this warning:

configure.ac:672: warning: AC_REQUIRE: `gl_FUNC_MMAP_ANON' was expanded before 
it was required
configure.ac:672: 
https://www.gnu.org/software/autoconf/manual/autoconf.html#Expanded-Before-Required
glm4/mprotect.m4:14: gl_FUNC_MPROTECT_WORKS is expanded from...
glm4/immutable.m4:7: gl_IMMUTABLE is expanded from...
configure.ac:35: gl_INIT is expanded from...
configure.ac:672: the top level

The cause is that gl_FUNC_MMAP_ANON is invoked in some places and required in
other places. This patch fixes it.


2021-01-17  Bruno Haible  

immutable, get-rusage-data: Fix autoconf warning.
* m4/mmap-anon.m4 (gl_FUNC_MMAP_ANON): Define through AC_DEFUN_ONCE.

diff --git a/m4/mmap-anon.m4 b/m4/mmap-anon.m4
index 5a9f968..e47aa2d 100644
--- a/m4/mmap-anon.m4
+++ b/m4/mmap-anon.m4
@@ -1,4 +1,4 @@
-# mmap-anon.m4 serial 11
+# mmap-anon.m4 serial 12
 dnl Copyright (C) 2005, 2007, 2009-2021 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -13,7 +13,7 @@ dnl with or without modifications, as long as this notice is 
preserved.
 # - On IRIX, neither exists, and a file descriptor opened to /dev/zero must be
 #   used.
 
-AC_DEFUN([gl_FUNC_MMAP_ANON],
+AC_DEFUN_ONCE([gl_FUNC_MMAP_ANON],
 [
   dnl Persuade glibc  to define MAP_ANONYMOUS.
   AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS])

vma-iter: Port to 64-bit Haiku

[Bruno Haible]

On Haiku/x86_64 the call to get_next_area_info emits a warning, because
the signature of this function has changed. This patch fixes it.


2021-01-16  Bruno Haible  

vma-iter: Port to 64-bit Haiku.
* lib/vma-iter.c (vma_iterate): Adapt to changed signature of
get_next_area_info.

diff --git a/lib/vma-iter.c b/lib/vma-iter.c
index f3bfec4..809828b 100644
--- a/lib/vma-iter.c
+++ b/lib/vma-iter.c
@@ -1459,7 +1459,7 @@ vma_iterate (vma_iterate_callback_fn callback, void *data)
   /* Use the BeOS specific API.  */
 
   area_info info;
-  int32 cookie;
+  ssize_t cookie;
 
   cookie = 0;
   while (get_next_area_info (0, &cookie, &info) == B_OK)