Re: [Bug-gnulib] Re: fnmatch.c includes alloca.h conditionally?

[Derek Robert Price]

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

My apologies for the broad CC, but I discovered potential Automake
and/or Autoconf issues while researching this problem.  Go ahead and
scan for "------>Automake/Autoconf Related Issues:" if you are reading
for one of those lists.  :^)

Derek Robert Price wrote:

> Derek Robert Price wrote:
>
> >Hey all,
>
> >I noticed that the fnmatch.c file is including alloca.h conditionally,
> >based on HAVE_ALLOCA_H, as is regex.c.  Shouldn't they both be able to
> >assume that alloca.h can be included, as vasnprintf.c does, since both
> >the fnmatch and regex modules are listed as being dependent on the
> >alloca module?
>
> >Derek
>
> Similarly, vasnprintf.c & regex.c are switching on HAVE_ALLOCA, which
> they should also be able to assume.  In fact, on systems where
> lib/alloca.c needs to be compiled, I think that HAVE_ALLOCA often
> isn't being defined and thus the function is compiled and not used by
> vasnprintf or regex.
>
> fnmatch.c appears to define HAVE_ALLOCA as well, then not use it.
> This can probably be removed.


Anyhow, this is causing problems on our Windows build.

Very obviously, fnmatch.c duplicates some code in "alloca.h":

#ifdef __GNUC__
# define alloca __builtin_alloca
# define HAVE_ALLOCA 1
#else
# if defined HAVE_ALLOCA_H || defined _LIBC
#  include <alloca.h>
# else
#  ifdef _AIX
 #  pragma alloca
#  else
#   ifndef alloca
char *alloca ();
#   endif
#  endif
# endif
#endif

I worked around this on Windows by simply defining HAVE_ALLOCA_H in
config.h, which caused the version in lib to be included, but this is
obviously not the correct fix, since Windows _does not_ actually have
an alloc.h header.  This usage discrepancy is even more obvious when
it is noted that the GNULIB alloca_.h appears to be assuming that it
will always be included by including <alloca.h> itself when
HAVE_ALLOCA_H is defined:

# ifdef __GNUC__
#  ifndef alloca
#   define alloca __builtin_alloca
#  endif
# else
#  ifdef _MSC_VER
#   include <malloc.h>
#   define alloca _alloca
#  else
#   if HAVE_ALLOCA_H
#    include <alloca.h>
#   else
#    ifdef _AIX
 #    pragma alloca
#    else
#     ifdef __hpux /* This section must match that of bison generated
files. */
#      ifdef __cplusplus
extern "C" void *alloca (unsigned int);
#      else /* not __cplusplus */
extern void *alloca ();
#      endif /* not __cplusplus */
#     else /* not __hpux */
#      ifndef alloca
extern char *alloca ();
#      endif
#     endif /* __hpux */
#    endif
#   endif
#  endif
# endif

I would submit a patch (my paperwork is done), but I'm not sure myself
whether it would be better to alter the alloca module to assume that
its header is always included and let it do the switching on
HAVE_ALLOCA_H or to include the GNULIB alloca.h conditionally based on
HAVE_ALLOCA_H and remove its on switch on HAVE_ALLOCA_H.  It seems to
me the first option would be less code to duplicate in programs and
modules dependent on the alloca module, however.

- ------> Automake/Autoconf Related Issues:

On further inspection of the Autoconf docs, it appears that configure
does not AC_LIBOBJ(alloca) when it finds it missing.  Apparently it
puts it someplace different:

     Check how to get `alloca'.  Tries to get a builtin version by
     checking for `alloca.h' or the predefined C preprocessor macros
     `__GNUC__' and `_AIX'

     If those attempts fail, it looks for the function in the standard C
     library.  If any of those methods succeed, it defines
     `HAVE_ALLOCA'.  Otherwise, it sets the output variable `ALLOCA' to
     `alloca.o' and defines `C_ALLOCA' (so programs can periodically
     call `alloca(0)' to garbage collect).  This variable is separate
     from `LIBOBJS' so multiple programs can share the value of
     `ALLOCA' without needing to create an actual library, in case only
     some of them use the code in `LIBOBJS'.

This sounds somewhat backwards to me, but it implies, looking at
m4/alloca.m4 that AC_LIBOBJ(alloca) is never actually called and
alloca.o never built on systems where configure fails to find it.  Is
this an error in the Autoconf docs or in the module?  The comments in
Autoconf's functions.m4 imply that Automake picks up the slack:

# _AC_LIBOBJ_ALLOCA
# -----------------
# Set up the LIBOBJ replacement of `alloca'.  Well, not exactly
# AC_LIBOBJ since we actually set the output variable `ALLOCA'.
# Nevertheless, for Automake, AC_LIBSOURCES it.

Unfortunately, I can find no evidence in my Automake 1.7.9 generated
Makefile.in that $(ALLOCA) is ever referenced.

I've attached a GNULIB patch to remove the switches on HAVE_ALLOCA
from the allocsa, vasnprintf, and xallocsa modules.  Since these
modules are all dependent on the alloca module, this should be able to
be assumed.  Applying this patch may, however, due to the issues noted
above, cause these modules to begin breaking on systems without
alloca, but this is the fault of the GNULIB alloca module for one or
more of the reasons pointed out above, not the allocsa, vasnprintf, or
xallocsa modules as they will exist after this patch.

The patch also removes a "#define HAVE_ALLOCA 1" from lib/fnmatch.c
that is never referenced.

Derek
- --
                *8^)

Email: derek@ximbiot.com

Get CVS support at <http://ximbiot.com>!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFAg+j0LD1OTBfyMaQRAnMnAKDW88PuL9Q0tMUmmtAR0o7S8bNPwQCfTt8G
x9sco55ut3LwiQ4PjqcWFpI=
=ltPH
-----END PGP SIGNATURE-----

Index: ChangeLog
===================================================================
RCS file: /cvsroot/gnulib/gnulib/ChangeLog,v
retrieving revision 1.145
diff -u -p -r1.145 ChangeLog
--- ChangeLog   18 Apr 2004 18:12:50 -0000      1.145
+++ ChangeLog   19 Apr 2004 14:54:19 -0000
@@ -1,3 +1,10 @@
+2004-04-19  Derek Price  <derek@ximbiot.com>
+
+       Assume existance of alloca().
+       * allocsa.c, allocsa.h, vasnprintf.c, xallocsa.h: Don't switch on
+       HAVE_ALLOCA - assume it.
+       * fnmatch.c: Don't define HAVE_ALLOCA - it is not used.
+
 2004-04-18  Jim Meyering  <jim@meyering.net>
 
        Change jm_ to gl_ in AC_DEFINE'd names.
Index: lib/allocsa.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/allocsa.c,v
retrieving revision 1.1
diff -u -p -r1.1 allocsa.c
--- lib/allocsa.c       20 Jan 2004 14:03:28 -0000      1.1
+++ lib/allocsa.c       19 Apr 2004 14:54:20 -0000
@@ -28,8 +28,6 @@
    mallocsa() and freesa() in the other case are not critical, because they
    are only invoked for big memory sizes.  */
 
-#if HAVE_ALLOCA
-
 /* Store the mallocsa() results in a hash table.  This is needed to reliably
    distinguish a mallocsa() result and an alloca() result.
 
@@ -61,12 +59,9 @@ typedef int verify1[2 * (HEADER_SIZE == 
 #define HASH_TABLE_SIZE 257
 static void * mallocsa_results[HASH_TABLE_SIZE];
 
-#endif
-
 void *
 mallocsa (size_t n)
 {
-#if HAVE_ALLOCA
   /* Allocate one more word, that serves as an indicator for malloc()ed
      memory, so that freesa() of an alloca() result is fast.  */
   size_t nplus = n + HEADER_SIZE;
@@ -94,16 +89,8 @@ mallocsa (size_t n)
     }
   /* Out of memory.  */
   return NULL;
-#else
-# if !MALLOC_0_IS_NONNULL
-  if (n == 0)
-    n = 1;
-# endif
-  return malloc (n);
-#endif
 }
 
-#if HAVE_ALLOCA
 void
 freesa (void *p)
 {
@@ -136,4 +123,3 @@ freesa (void *p)
       /* At this point, we know it was not a mallocsa() result.  */
     }
 }
-#endif
Index: lib/allocsa.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/allocsa.h,v
retrieving revision 1.1
diff -u -p -r1.1 allocsa.h
--- lib/allocsa.h       20 Jan 2004 14:03:28 -0000      1.1
+++ lib/allocsa.h       19 Apr 2004 14:54:20 -0000
@@ -32,37 +32,24 @@
      - in inline functions - the allocation may actually last until the
        calling function returns.
 */
-#if HAVE_ALLOCA
 /* The OS usually guarantees only one guard page at the bottom of the stack,
    and a page size can be as small as 4096 bytes.  So we cannot safely
    allocate anything larger than 4096 bytes.  Also care for the possibility
    of a few compiler-allocated temporary stack slots.
    This must be a macro, not an inline function.  */
 # define safe_alloca(N) ((N) < 4032 ? alloca (N) : NULL)
-#else
-# define safe_alloca(N) ((N), NULL)
-#endif
 
 /* allocsa(N) is a safe variant of alloca(N).  It allocates N bytes of
    memory allocated on the stack, that must be freed using freesa() before
    the function returns.  Upon failure, it returns NULL.  */
-#if HAVE_ALLOCA
 # define allocsa(N) \
   ((N) < 4032 - sa_increment                                       \
    ? (void *) ((char *) alloca ((N) + sa_increment) + sa_increment) \
    : mallocsa (N))
-#else
-# define allocsa(N) \
-  mallocsa (N)
-#endif
 extern void * mallocsa (size_t n);
 
 /* Free a block of memory allocated through allocsa().  */
-#if HAVE_ALLOCA
 extern void freesa (void *p);
-#else
-# define freesa free
-#endif
 
 /* Maybe we should also define a variant
     nallocsa (size_t n, size_t s) - behaves like allocsa (n * s)
Index: lib/fnmatch.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/fnmatch.c,v
retrieving revision 1.26
diff -u -p -r1.26 fnmatch.c
--- lib/fnmatch.c       14 Jan 2004 22:15:48 -0000      1.26
+++ lib/fnmatch.c       19 Apr 2004 14:54:20 -0000
@@ -26,7 +26,6 @@
 
 #ifdef __GNUC__
 # define alloca __builtin_alloca
-# define HAVE_ALLOCA 1
 #else
 # if defined HAVE_ALLOCA_H || defined _LIBC
 #  include <alloca.h>
Index: lib/vasnprintf.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/vasnprintf.c,v
retrieving revision 1.12
diff -u -p -r1.12 vasnprintf.c
--- lib/vasnprintf.c    25 Nov 2003 11:18:46 -0000      1.12
+++ lib/vasnprintf.c    19 Apr 2004 14:54:20 -0000
@@ -145,14 +145,12 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
        sprintf or snprintf.  */
     buf_neededlength =
       xsum4 (7, d.max_width_length, d.max_precision_length, 6);
-#if HAVE_ALLOCA
     if (buf_neededlength < 4000 / sizeof (CHAR_T))
       {
        buf = (CHAR_T *) alloca (buf_neededlength * sizeof (CHAR_T));
        buf_malloced = NULL;
       }
     else
-#endif
       {
        size_t buf_memsize = xtimes (buf_neededlength, sizeof (CHAR_T));
        if (size_overflow_p (buf_memsize))
Index: lib/xallocsa.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/xallocsa.h,v
retrieving revision 1.1
diff -u -p -r1.1 xallocsa.h
--- lib/xallocsa.h      20 Jan 2004 14:07:25 -0000      1.1
+++ lib/xallocsa.h      19 Apr 2004 14:54:20 -0000
@@ -24,16 +24,11 @@
 /* xallocsa(N) is a checking safe variant of alloca(N).  It allocates N bytes
    of memory allocated on the stack, that must be freed using freesa() before
    the function returns.  Upon failure, it exits with an error message.  */
-#if HAVE_ALLOCA
 # define xallocsa(N) \
   ((N) < 4032 - sa_increment                                       \
    ? (void *) ((char *) alloca ((N) + sa_increment) + sa_increment) \
    : xmallocsa (N))
 extern void * xmallocsa (size_t n);
-#else
-# define xallocsa(N) \
-  xmalloc (N)
-#endif
 
 /* Maybe we should also define a variant
     xnallocsa (size_t n, size_t s) - behaves like xallocsa (n * s)