Re: test-memchr crash

[Eric Blake]

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

According to Bruno Haible on 6/12/2009 6:33 AM:
> Looks pretty much like one of the glibc memchr bugs that were recently fixed:
>   <http://sourceware.org/bugzilla/show_bug.cgi?id=10162>
> 
> So, it was a good move to un-obsolete the 'memchr' module. But now we also 
> need
> to extend the m4/memchr.m4 test.

I've had this sitting around for a while [1], so now that we have
confirmed this is a problem in a wild, I'm committing it.  Simon, can you
please confirm that this fixes the failure?

[1]http://lists.gnu.org/archive/html/bug-gnulib/2009-05/msg00253.html

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

iEYEARECAAYFAkozIwQACgkQ84KuGfSFAYCKxQCgv81KXCPcRSmIb3L0lU/U4GjX
pigAoM88JsEAXr7BFEoBbEPZSJOMxdaM
=jWoA
-----END PGP SIGNATURE-----

>From 7cf183c33ef0c792b5c93dbb4ac5b7858ee03986 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 8 Jun 2009 06:29:17 -0600
Subject: [PATCH] memchr: detect broken x86_64 and alpha implementations

* modules/memchr-tests (Depends-on): Move mmap detection...
* modules/memchr (Depends-on): ...here.
(configure.ac): Set indicator.
* lib/string.in.h (memchr): Declare replacement.
* modules/string (Makefile.am): Trigger replacement.
* m4/string_h.m4 (gl_HEADER_STRING_H_DEFAULTS): Likewise.
* m4/memchr.m4 (gl_FUNC_MEMCHR): Use mmap to detect platform
bugs.
* doc/posix-functions/memchr.texi (memchr): Document the bug.
* modules/getpagesize (License): Relax license.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                       |   14 ++++++++
 doc/posix-functions/memchr.texi |    4 ++
 lib/string.in.h                 |   19 ++++++++++-
 m4/memchr.m4                    |   69 ++++++++++++++++++++++++++++++++++++++-
 m4/string_h.m4                  |    6 ++-
 modules/getpagesize             |    2 +-
 modules/memchr                  |    5 +++
 modules/memchr-tests            |    6 ---
 modules/string                  |    2 +
 9 files changed, 116 insertions(+), 11 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c065f8b..32d866d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2009-06-12  Eric Blake  <address@hidden>
+
+       memchr: detect broken x86_64 and alpha implementations
+       * modules/memchr-tests (Depends-on): Move mmap detection...
+       * modules/memchr (Depends-on): ...here.
+       (configure.ac): Set indicator.
+       * lib/string.in.h (memchr): Declare replacement.
+       * modules/string (Makefile.am): Trigger replacement.
+       * m4/string_h.m4 (gl_HEADER_STRING_H_DEFAULTS): Likewise.
+       * m4/memchr.m4 (gl_FUNC_MEMCHR): Use mmap to detect platform
+       bugs.
+       * doc/posix-functions/memchr.texi (memchr): Document the bug.
+       * modules/getpagesize (License): Relax license.
+
 2009-06-11  Bruno Haible  <address@hidden>

        * lib/idpriv.h: Add more references.
diff --git a/doc/posix-functions/memchr.texi b/doc/posix-functions/memchr.texi
index f8e33a5..62d2043 100644
--- a/doc/posix-functions/memchr.texi
+++ b/doc/posix-functions/memchr.texi
@@ -10,6 +10,10 @@ Portability problems fixed by Gnulib:
 @itemize
 @item
 This function is missing on some older platforms.
+
address@hidden
+This function dereferences too much memory on some platforms:
+glibc 2.10 on x86_64 or Alpha
 @end itemize

 Portability problems not fixed by Gnulib:
diff --git a/lib/string.in.h b/lib/string.in.h
index 7d508cc..c913cce 100644
--- a/lib/string.in.h
+++ b/lib/string.in.h
@@ -1,6 +1,6 @@
 /* A GNU-like <string.h>.

-   Copyright (C) 1995-1996, 2001-2008 Free Software Foundation, Inc.
+   Copyright (C) 1995-1996, 2001-2009 Free Software Foundation, Inc.

    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -49,6 +49,23 @@ extern "C" {
 #endif


+/* Return the first instance of C within N bytes of S, or NULL.  */
+#if @GNULIB_MEMCHR@
+# if @REPLACE_MEMCHR@
+#  define memchr rpl_memchr
+# endif
+# if @REPLACE_MEMCHR@
+extern void *memchr (void const *__s, int __c, size_t __n)
+  __attribute__ ((__pure__));
+# endif
+#elif defined GNULIB_POSIXCHECK
+# undef memchr
+# define memchr(s,c,n) \
+    (GL_LINK_WARNING ("memchr has platform-specific bugs - " \
+                      "use gnulib module memchr for portability" ), \
+     memchr (s, c, n))
+#endif
+
 /* Return the first occurrence of NEEDLE in HAYSTACK.  */
 #if @GNULIB_MEMMEM@
 # if @REPLACE_MEMMEM@
diff --git a/m4/memchr.m4 b/m4/memchr.m4
index 53c5380..bcfe869 100644
--- a/m4/memchr.m4
+++ b/m4/memchr.m4
@@ -1,4 +1,4 @@
-# memchr.m4 serial 5
+# memchr.m4 serial 6
 dnl Copyright (C) 2002, 2003, 2004, 2009 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -6,9 +6,76 @@ dnl with or without modifications, as long as this notice is 
preserved.

 AC_DEFUN([gl_FUNC_MEMCHR],
 [
+  dnl Check for prerequisites for memory fence checks.
+  gl_FUNC_MMAP_ANON
+  AC_CHECK_HEADERS_ONCE([sys/mman.h])
+  AC_CHECK_FUNCS_ONCE([mprotect])
+
+  dnl These days, we assume memchr is present.  But just in case...
+  AC_REQUIRE([gl_HEADER_STRING_H_DEFAULTS])
   AC_REPLACE_FUNCS([memchr])
   if test $ac_cv_func_memchr = no; then
     gl_PREREQ_MEMCHR
+    REPLACE_MEMCHR=1
+  fi
+
+  if test $ac_cv_func_memchr = yes; then
+    # Detect platform-specific bugs in some versions of glibc:
+    # memchr should not dereference anything with length 0
+    #   http://bugzilla.redhat.com/499689
+    # memchr should not dereference overestimated length after a match
+    #   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=521737
+    # Assume that memchr works on platforms that lack mprotect.
+    AC_CACHE_CHECK([whether memchr works], [gl_cv_func_memchr_works],
+      [AC_RUN_IFELSE([AC_LANG_PROGRAM([[
+#include <string.h>
+#if HAVE_SYS_MMAN_H
+# include <fcntl.h>
+# include <unistd.h>
+# include <sys/types.h>
+# include <sys/mman.h>
+# ifndef MAP_FILE
+#  define MAP_FILE 0
+# endif
+#endif
+]], [[
+  char *fence = NULL;
+#if HAVE_SYS_MMAN_H && HAVE_MPROTECT
+# if HAVE_MAP_ANONYMOUS
+  const int flags = MAP_ANONYMOUS | MAP_PRIVATE;
+  const int fd = -1;
+# else /* !HAVE_MAP_ANONYMOUS */
+  const int flags = MAP_FILE | MAP_PRIVATE;
+  int fd = open ("/dev/zero", O_RDONLY, 0666);
+  if (fd >= 0)
+# endif
+    {
+      int pagesize = getpagesize ();
+      char *two_pages =
+       (char *) mmap (NULL, 2 * pagesize, PROT_READ | PROT_WRITE,
+                      flags, fd, 0);
+      if (two_pages != (char *)(-1)
+         && mprotect (two_pages + pagesize, pagesize, PROT_NONE) == 0)
+       fence = two_pages + pagesize;
+    }
+#endif
+  if (fence)
+    {
+      if (memchr (fence, 0, 0))
+        return 1;
+      strcpy (fence - 9, "12345678");
+      if (memchr (fence - 9, 0, 79) != fence - 1)
+        return 2;
+    }
+  return 0;
+]])], [gl_cv_func_memchr_works=yes], [gl_cv_func_memchr_works=no],
+      [dnl Be pessimistic for now.
+       gl_cv_func_memchr_works="guessing no"])])
+    if test "$gl_cv_func_memchr_works" != yes; then
+      gl_PREREQ_MEMCHR
+      REPLACE_MEMCHR=1
+      AC_LIBOBJ([memchr])
+    fi
   fi
 ])

diff --git a/m4/string_h.m4 b/m4/string_h.m4
index 2d5553c..11f09c8 100644
--- a/m4/string_h.m4
+++ b/m4/string_h.m4
@@ -1,11 +1,11 @@
 # Configure a GNU-like replacement for <string.h>.

-# Copyright (C) 2007, 2008 Free Software Foundation, Inc.
+# Copyright (C) 2007, 2008, 2009 Free Software Foundation, Inc.
 # This file is free software; the Free Software Foundation
 # gives unlimited permission to copy and/or distribute it,
 # with or without modifications, as long as this notice is preserved.

-# serial 6
+# serial 7

 # Written by Paul Eggert.

@@ -32,6 +32,7 @@ AC_DEFUN([gl_STRING_MODULE_INDICATOR],

 AC_DEFUN([gl_HEADER_STRING_H_DEFAULTS],
 [
+  GNULIB_MEMCHR=0;      AC_SUBST([GNULIB_MEMCHR])
   GNULIB_MEMMEM=0;      AC_SUBST([GNULIB_MEMMEM])
   GNULIB_MEMPCPY=0;     AC_SUBST([GNULIB_MEMPCPY])
   GNULIB_MEMRCHR=0;     AC_SUBST([GNULIB_MEMRCHR])
@@ -83,6 +84,7 @@ AC_DEFUN([gl_HEADER_STRING_H_DEFAULTS],
   HAVE_DECL_STRERROR=1;                AC_SUBST([HAVE_DECL_STRERROR])
   HAVE_DECL_STRSIGNAL=1;       AC_SUBST([HAVE_DECL_STRSIGNAL])
   HAVE_STRVERSCMP=1;           AC_SUBST([HAVE_STRVERSCMP])
+  REPLACE_MEMCHR=0;            AC_SUBST([REPLACE_MEMCHR])
   REPLACE_MEMMEM=0;            AC_SUBST([REPLACE_MEMMEM])
   REPLACE_STRDUP=0;            AC_SUBST([REPLACE_STRDUP])
   REPLACE_STRSTR=0;            AC_SUBST([REPLACE_STRSTR])
diff --git a/modules/getpagesize b/modules/getpagesize
index 9cc4f37..6ad1620 100644
--- a/modules/getpagesize
+++ b/modules/getpagesize
@@ -18,7 +18,7 @@ Include:
 <unistd.h>

 License:
-LGPL
+LGPLv2+

 Maintainer:
 Jim Meyering
diff --git a/modules/memchr b/modules/memchr
index da7e7a2..a77bec5 100644
--- a/modules/memchr
+++ b/modules/memchr
@@ -4,11 +4,16 @@ memchr() function: scan memory for a byte.
 Files:
 lib/memchr.c
 m4/memchr.m4
+m4/mmap-anon.m4

 Depends-on:
+extensions
+getpagesize
+string

 configure.ac:
 gl_FUNC_MEMCHR
+gl_STRING_MODULE_INDICATOR([memchr])

 Makefile.am:

diff --git a/modules/memchr-tests b/modules/memchr-tests
index c2c9e2b..32cdd6d 100644
--- a/modules/memchr-tests
+++ b/modules/memchr-tests
@@ -1,16 +1,10 @@
 Files:
 tests/test-memchr.c
 tests/zerosize-ptr.h
-m4/mmap-anon.m4

 Depends-on:
-extensions
-getpagesize

 configure.ac:
-gl_FUNC_MMAP_ANON
-AC_CHECK_HEADERS_ONCE([sys/mman.h])
-AC_CHECK_FUNCS_ONCE([mprotect])

 Makefile.am:
 TESTS += test-memchr
diff --git a/modules/string b/modules/string
index 6283f9e..b2c802c 100644
--- a/modules/string
+++ b/modules/string
@@ -38,6 +38,7 @@ string.h: string.in.h
              -e 's|@''GNULIB_MBSSPN''@|$(GNULIB_MBSSPN)|g' \
              -e 's|@''GNULIB_MBSSEP''@|$(GNULIB_MBSSEP)|g' \
              -e 's|@''GNULIB_MBSTOK_R''@|$(GNULIB_MBSTOK_R)|g' \
+             -e 's|@''GNULIB_MEMCHR''@|$(GNULIB_MEMCHR)|g' \
              -e 's|@''GNULIB_MEMMEM''@|$(GNULIB_MEMMEM)|g' \
              -e 's|@''GNULIB_MEMPCPY''@|$(GNULIB_MEMPCPY)|g' \
              -e 's|@''GNULIB_MEMRCHR''@|$(GNULIB_MEMRCHR)|g' \
@@ -74,6 +75,7 @@ string.h: string.in.h
              -e 's|@''HAVE_DECL_STRERROR''@|$(HAVE_DECL_STRERROR)|g' \
              -e 's|@''HAVE_DECL_STRSIGNAL''@|$(HAVE_DECL_STRSIGNAL)|g' \
              -e 's|@''HAVE_STRVERSCMP''@|$(HAVE_STRVERSCMP)|g' \
+             -e 's|@''REPLACE_MEMCHR''@|$(REPLACE_MEMCHR)|g' \
              -e 's|@''REPLACE_MEMMEM''@|$(REPLACE_MEMMEM)|g' \
              -e 's|@''REPLACE_STRCASESTR''@|$(REPLACE_STRCASESTR)|g' \
              -e 's|@''REPLACE_STRDUP''@|$(REPLACE_STRDUP)|g' \
-- 
1.6.3.rc3.2.g4b51