Re: Broken autoconf mmap test (was Re: 1.7] BUG - GREP slows to a crawl with large number of matches on a single file)

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

[please limit replies about the patch itself to autoconf-patches]

According to Corinna Vinschen on 11/9/2009 7:05 AM:
> This part of the testcase
> 
>   data2 = (char *) malloc (2 * pagesize);
>   if (!data2)
>     return 1;
>   data2 += (pagesize - ((long int) data2 & (pagesize - 1))) & (pagesize - 1);
>   if (data2 != mmap (data2, pagesize, PROT_READ | PROT_WRITE,
>                        MAP_PRIVATE | MAP_FIXED, fd, 0L))
>     return 1;
> 
> is bad.  The chance that the address of data2 is not usable for mmap on
> Windows/Cygwin is 100%.  The problem here is that the generic HAVE_MMAP
> test tests one certain feature, which is not usable on Windows, and which
> is non-portable.

MAP_FIXED appears to be more portable when the fixed address was obtained
from a previous mmap call.  Therefore, this patch fixes the macro as well
as making diagnosing configure failures more accurately pinpoint why they
are declaring failure.  I don't have access to HP-UX 11, which is another
platform where AC_FUNC_MMAP was failing; I would appreciate if someone
else could see if this makes a difference there.  But I have verified that
this now sets HAVE_MMAP for cygwin 1.5.x and cygwin 1.7 where the old
version failed, and that it does not change behavior on Linux or OpenBSD.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkr484kACgkQ84KuGfSFAYDCIgCbBl/eHS9C9acPwXp5Krk7KAeF
zAIAoMBEbnQm5tLpRDkCFWhEXNieL5cf
=3fYB
-----END PGP SIGNATURE-----

>From fb1f28a2ff2c688e63dc97ece7fde86e16864491 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 9 Nov 2009 21:45:00 -0700
Subject: [PATCH] Fix AC_FUNC_MMAP for cygwin.

* lib/autoconf/functions.m4 (AC_FUNC_MMAP): Make the test more
portable: Actually check for <sys/param.h>, and only use MAP_FIXED
on an address previously returned from mmap.
* THANKS: Update.
Reported by Corinna Vinschen.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                 |    9 +++++++
 NEWS                      |    3 ++
 lib/autoconf/functions.m4 |   55 ++++++++++++++++++++++++++------------------
 3 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4d028c0..77e9d4e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2009-11-09  Eric Blake  <address@hidden>
+
+       Fix AC_FUNC_MMAP for cygwin.
+       * lib/autoconf/functions.m4 (AC_FUNC_MMAP): Make the test more
+       portable: Actually check for <sys/param.h>, and only use MAP_FIXED
+       on an address previously returned from mmap.
+       * THANKS: Update.
+       Reported by Corinna Vinschen.
+
 2009-11-04  Eric Blake  <address@hidden>

        Redocument AS_DIRNAME, even with its flaws.
diff --git a/NEWS b/NEWS
index 9e7e64c..86a0c3f 100644
--- a/NEWS
+++ b/NEWS
@@ -29,6 +29,9 @@ GNU Autoconf NEWS - User visible changes.
    longer mistakenly select a 32-bit type on some compilers (bug present
    since macros were introduced in 2.59c).

+** The AC_FUNC_MMAP macro has been fixed to be portable to systems like
+   Cygwin (bug present since macro was introduced in 2.0).
+
 ** The following documented autotest macros are new:
    AT_CHECK_EUNIT

diff --git a/lib/autoconf/functions.m4 b/lib/autoconf/functions.m4
index 946a646..6b6e7fc 100644
--- a/lib/autoconf/functions.m4
+++ b/lib/autoconf/functions.m4
@@ -1186,9 +1186,9 @@ AU_ALIAS([AM_FUNC_MKTIME], [AC_FUNC_MKTIME])
 # ------------
 AN_FUNCTION([mmap], [AC_FUNC_MMAP])
 AC_DEFUN([AC_FUNC_MMAP],
-[AC_CHECK_HEADERS(stdlib.h unistd.h)
-AC_CHECK_FUNCS(getpagesize)
-AC_CACHE_CHECK(for working mmap, ac_cv_func_mmap_fixed_mapped,
+[AC_CHECK_HEADERS_ONCE([stdlib.h unistd.h sys/param.h])
+AC_CHECK_FUNCS([getpagesize])
+AC_CACHE_CHECK([for working mmap], [ac_cv_func_mmap_fixed_mapped],
 [AC_RUN_IFELSE([AC_LANG_SOURCE([AC_INCLUDES_DEFAULT]
 [[/* malloc might have been renamed as rpl_malloc. */
 #undef malloc
@@ -1224,11 +1224,6 @@ char *malloc ();

 /* This mess was copied from the GNU getpagesize.h.  */
 #ifndef HAVE_GETPAGESIZE
-/* Assume that all systems that can run configure have sys/param.h.  */
-# ifndef HAVE_SYS_PARAM_H
-#  define HAVE_SYS_PARAM_H 1
-# endif
-
 # ifdef _SC_PAGESIZE
 #  define getpagesize() sysconf(_SC_PAGESIZE)
 # else /* no _SC_PAGESIZE */
@@ -1264,7 +1259,7 @@ main ()
 {
   char *data, *data2, *data3;
   int i, pagesize;
-  int fd;
+  int fd, fd2;

   pagesize = getpagesize ();

@@ -1277,27 +1272,41 @@ main ()
   umask (0);
   fd = creat ("conftest.mmap", 0600);
   if (fd < 0)
-    return 1;
+    return 2;
   if (write (fd, data, pagesize) != pagesize)
-    return 1;
+    return 3;
   close (fd);

+  /* Next, check that the tail of a page is zero-filled.  File must have
+     non-zero length, otherwise we risk SIGBUS for entire page.  */
+  fd2 = open ("conftest.txt", O_RDWR | O_CREAT | O_TRUNC, 0600);
+  if (fd2 < 0)
+    return 4;
+  data2 = "";
+  if (write (fd2, data2, 1) != 1)
+    return 5;
+  data2 = mmap (0, pagesize, PROT_READ | PROT_WRITE, MAP_SHARED, fd2, 0L);
+  if (data2 == MAP_FAILED)
+    return 6;
+  for (i = 0; i < pagesize; ++i)
+    if (*(data2 + i))
+      return 7;
+  close (fd2);
+  if (munmap (data2, pagesize))
+    return 8;
+
   /* Next, try to mmap the file at a fixed address which already has
      something else allocated at it.  If we can, also make sure that
      we see the same garbage.  */
   fd = open ("conftest.mmap", O_RDWR);
   if (fd < 0)
-    return 1;
-  data2 = (char *) malloc (2 * pagesize);
-  if (!data2)
-    return 1;
-  data2 += (pagesize - ((long int) data2 & (pagesize - 1))) & (pagesize - 1);
+    return 9;
   if (data2 != mmap (data2, pagesize, PROT_READ | PROT_WRITE,
                     MAP_PRIVATE | MAP_FIXED, fd, 0L))
-    return 1;
+    return 10;
   for (i = 0; i < pagesize; ++i)
     if (*(data + i) != *(data2 + i))
-      return 1;
+      return 11;

   /* Finally, make sure that changes to the mapped area do not
      percolate back to the file as seen by read().  (This is a bug on
@@ -1306,12 +1315,12 @@ main ()
     *(data2 + i) = *(data2 + i) + 1;
   data3 = (char *) malloc (pagesize);
   if (!data3)
-    return 1;
+    return 12;
   if (read (fd, data3, pagesize) != pagesize)
-    return 1;
+    return 13;
   for (i = 0; i < pagesize; ++i)
     if (*(data + i) != *(data3 + i))
-      return 1;
+      return 14;
   close (fd);
   return 0;
 }]])],
@@ -1319,10 +1328,10 @@ main ()
               [ac_cv_func_mmap_fixed_mapped=no],
               [ac_cv_func_mmap_fixed_mapped=no])])
 if test $ac_cv_func_mmap_fixed_mapped = yes; then
-  AC_DEFINE(HAVE_MMAP, 1,
+  AC_DEFINE([HAVE_MMAP], [1],
            [Define to 1 if you have a working `mmap' system call.])
 fi
-rm -f conftest.mmap
+rm -f conftest.mmap conftest.txt
 ])# AC_FUNC_MMAP


-- 
1.6.5.rc1