Re: Preventing stack overflows with alloca.

[Kenichi Handa <handa@m17n.org>]

In article <m3smctxje6.fsf@kfs-l.imdomain.dk>, storm@cua.dk (Kim F. Storm) writes:

> Jan just installed a change to fns.c to use xmalloc instead of
> alloca in string_to_multibyte.

> Always using xmalloc seems a bit expensive, so I suggest
> only using it if the size is > MAX_ALLOCA (already defined
> in fns.c).

> But there are other risky uses of alloca, so I suggest the 
> patch below which takes care of the problems in fns.c.  

Oops, I have completely forgotten about this problem.
Actually I noticed it last year and sent the attached mail,
but haven't worked on it any further.  I remember Richard
agreed with having the macro SAFE_ALLOCA.  Of course we need
the pairing SAFE_FREE which frees `address' and calls
unbind_to if necessary.

---
Ken'ichi HANDA
handa@m17n.org

Date: Thu, 20 Nov 2003 22:01:20 +0900 (JST)
From: Kenichi Handa <handa@m17n.org>
To: emacs-devel@gnu.org
Subject: dangerous alloca calls
Sender: emacs-devel-bounces+handa=m17n.org@gnu.org

With CVS HEAD emacs on GNU/Linux,
   (mapcar 'car (make-list #x400000 nil))
causes segfault.

The attached patch fixes it.  But, I found such dangerous
alloca calls in several other places.

E.g. (format (make-string #x1000000 32))
     (apply 'car (make-list #x100000 nil))

It seems that it is better that we use a macro something
like this consitently.  What do you think?

#define SAFE_ALLOCA(address, size, unwind_function)		\
  do {								\
    if (size < MAX_ALLOCA)					\
      address = alloca (size);					\
    else							\
      {								\
	address = xmalloc (size);				\
	if (unwind_function)					\
	  record_unwind_protect (unwind_function, Qnil);	\
      }								\
  } while (0)

---
Ken'ichi HANDA
handa@m17n.org

*** fns.c.~1.346.~	Wed Nov 19 20:06:32 2003
--- fns.c	Thu Nov 20 21:26:50 2003
***************
*** 79,84 ****
--- 79,88 ----
  #ifndef HAVE_UNISTD_H
  extern long time ();
  #endif
+ 
+ /* Don't use alloca for bytes larger than this, lest we overflow
+    their stack.  */
+ #define MAX_ALLOCA 16*1024
  \f
  DEFUN ("identity", Fidentity, Sidentity, 1, 1, 0,
         doc: /* Return the argument unchanged.  */)
***************
*** 2937,2942 ****
--- 2941,2956 ----
    UNGCPRO;
  }
  
+ static Lisp_Object *mapcar_args;
+ 
+ static Lisp_Object
+ mapcar_unwind (arg)
+      Lisp_Object arg;
+ {
+   xfree (mapcar_args);
+   return Qnil;
+ }
+ 
  DEFUN ("mapconcat", Fmapconcat, Smapconcat, 3, 3, 0,
         doc: /* Apply FUNCTION to each element of SEQUENCE, and concat the results as strings.
  In between each pair of results, stick in SEPARATOR.  Thus, " " as
***************
*** 2951,2963 ****
    register Lisp_Object *args;
    register int i;
    struct gcpro gcpro1;
  
    len = Flength (sequence);
    leni = XINT (len);
    nargs = leni + leni - 1;
    if (nargs < 0) return build_string ("");
  
!   args = (Lisp_Object *) alloca (nargs * sizeof (Lisp_Object));
  
    GCPRO1 (separator);
    mapcar1 (leni, args, function, sequence);
--- 2965,2986 ----
    register Lisp_Object *args;
    register int i;
    struct gcpro gcpro1;
+   int nbytes;
+   int count = SPECPDL_INDEX ();
  
    len = Flength (sequence);
    leni = XINT (len);
    nargs = leni + leni - 1;
    if (nargs < 0) return build_string ("");
  
!   nbytes = nargs * sizeof (Lisp_Object);
!   if (nbytes < MAX_ALLOCA)
!     args = (Lisp_Object *) alloca (nbytes);
!   else
!     {
!       args = mapcar_args = (Lisp_Object *) xmalloc (nbytes);
!       record_unwind_protect (mapcar_unwind, Qnil);
!     }
  
    GCPRO1 (separator);
    mapcar1 (leni, args, function, sequence);
***************
*** 2969,2975 ****
    for (i = 1; i < nargs; i += 2)
      args[i] = separator;
  
!   return Fconcat (nargs, args);
  }
  
  DEFUN ("mapcar", Fmapcar, Smapcar, 2, 2, 0,
--- 2992,3001 ----
    for (i = 1; i < nargs; i += 2)
      args[i] = separator;
  
!   if (nbytes < MAX_ALLOCA)
!     return Fconcat (nargs, args);
!   else
!     return unbind_to (count, Fconcat (nargs, args));
  }
  
  DEFUN ("mapcar", Fmapcar, Smapcar, 2, 2, 0,
***************
*** 2982,2995 ****
    register Lisp_Object len;
    register int leni;
    register Lisp_Object *args;
  
    len = Flength (sequence);
    leni = XFASTINT (len);
!   args = (Lisp_Object *) alloca (leni * sizeof (Lisp_Object));
  
    mapcar1 (leni, args, function, sequence);
  
!   return Flist (leni, args);
  }
  
  DEFUN ("mapc", Fmapc, Smapc, 2, 2, 0,
--- 3008,3035 ----
    register Lisp_Object len;
    register int leni;
    register Lisp_Object *args;
+   int nbytes;
+   int count = SPECPDL_INDEX ();
  
    len = Flength (sequence);
    leni = XFASTINT (len);
!   if (leni < 0) return Qnil;
! 
!   nbytes = leni * sizeof (Lisp_Object);
!   if (nbytes < MAX_ALLOCA)
!     args = (Lisp_Object *) alloca (nbytes);
!   else
!     {
!       args = mapcar_args = (Lisp_Object *) xmalloc (nbytes);
!       record_unwind_protect (mapcar_unwind, Qnil);
!     }
  
    mapcar1 (leni, args, function, sequence);
  
!   if (nbytes < MAX_ALLOCA)
!     return Flist (leni, args);
!   else
!     return unbind_to (count, Flist (leni, args));
  }
  
  DEFUN ("mapc", Fmapc, Smapc, 2, 2, 0,
***************
*** 3603,3612 ****
        c = from[i++];			\
      }					\
    while (IS_BASE64_IGNORABLE (c))
- 
- /* Don't use alloca for regions larger than this, lest we overflow
-    their stack.  */
- #define MAX_ALLOCA 16*1024
  
  /* Table of characters coding the 64 values.  */
  static char base64_value_to_char[64] =
--- 3643,3648 ----


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel