[PATCH] Fixed `scm_i_take_stringbufn ()'

[Ludovic Courtès]

Hi,

The patch below fixes `scm_i_take_stringbufn ()' so that it registers
the right amount of collectable memory.  Failing to do so, an underflow
was triggered in `decrease_mtrigger ()' (when decreasing
SCM_MALLOCATED), resulting in a weird behavior (typically: fast increase
in memory consumption quickly followed by the memory overflow error in
`increase_mtrigger ()').

The problem can be reproduced using the program found at the end of this
post.  It could hardly be turned into a test case, however.

Note: I'm not sure what to do with `assert ()'.  Perhaps we should
define our own macro somewhere?

Thanks,
Ludovic.


2006-02-13  Ludovic Courtès  <address@hidden>

        *strings.c (scm_i_take_stringbufn): Register LEN + 1 octets
        instead of LEN.  Without this, too much collectable memory gets
        unregistered, which results in an underflow of SCM_MALLOCATED in
        `decrease_mtrigger ()'.

        * gc-malloc.c (decrease_mtrigger): Make sure SIZE is lower than
        or equal to SCM_MALLOCATED.


--- orig/libguile/gc-malloc.c
+++ mod/libguile/gc-malloc.c
@@ -64,6 +64,10 @@
 #include <unistd.h>
 #endif
 
+#ifdef HAVE_ASSERT_H
+#include <assert.h>
+#endif
+
 /*
   INIT_MALLOC_LIMIT is the initial amount of malloc usage which will
   trigger a GC.
@@ -184,6 +188,14 @@
 decrease_mtrigger (size_t size, const char * what)
 {
   scm_i_pthread_mutex_lock (&scm_i_gc_admin_mutex);
+
+#ifdef HAVE_ASSERT_H
+  assert (size <= scm_mallocated);
+#else
+  if (size > scm_mallocated)
+    abort ();
+#endif
+
   scm_mallocated -= size;
   scm_gc_malloc_collected += size;
   scm_i_pthread_mutex_unlock (&scm_i_gc_admin_mutex);


--- orig/libguile/strings.c
+++ mod/libguile/strings.c
@@ -122,12 +122,12 @@
     }
 }
 
-/* Return a new stringbuf whose underlying storage consists of the LEN octets
-   pointed to by STR.  */
+/* Return a new stringbuf whose underlying storage consists of the LEN+1
+   octets pointed to by STR (the last octet is zero).  */
 SCM_C_INLINE SCM
 scm_i_take_stringbufn (char *str, size_t len)
 {
-  scm_gc_register_collectable_memory (str, len, "stringbuf");
+  scm_gc_register_collectable_memory (str, len + 1, "stringbuf");
 
   return scm_double_cell (STRINGBUF_TAG, (scm_t_bits) str,
                          (scm_t_bits) len, (scm_t_bits) 0);




/* Copyright (C) 1996,1997,2000,2001 Free Software Foundation, Inc.
 * 
 * This library is free software; you can redistribute it and/or
 * modify it under the terms of the GNU Lesser General Public
 * License as published by the Free Software Foundation; either
 * version 2.1 of the License, or (at your option) any later version.
 *
 * This library is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 * Lesser General Public License for more details.
 *
 * You should have received a copy of the GNU Lesser General Public
 * License along with this library; if not, write to the Free Software
 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 */

/* This is the 'main' function for the `guile' executable.  It is not
   included in libguile.a.

   Eventually, we hope this file will be automatically generated,
   based on the list of installed, statically linked libraries on the
   system.  For now, please don't put interesting code in here.  */

#if HAVE_CONFIG_H
#  include <config.h>
#endif

#include <libguile.h>

#ifdef HAVE_CONFIG_H
#include <libguile/scmconfig.h>
#endif

#include <string.h>
#include <stdio.h>
#include <signal.h>

static void
inner_main (void *closure SCM_UNUSED, int argc, char **argv)
{
  static unsigned long long i = 0;

  while (1)
    {
      SCM str;

      i++;
      if ((i && !(i % 1000)) || (i > 82100))
        /* At some point you will witness the underflow of
           SCM_MALLOCATED.  */
        printf ("i=%llu mallocated: %lu\n", i, scm_mallocated);

#if 0
      if (i == 82112)
        /* Stop just before the underflow to attach GDB.  */
        kill (getpid (), SIGTSTP);
#endif

      str = scm_take_locale_string (strdup ("csdfsdfsfsdfsldfkslkdfnlskdf"));
    }
}

int
main (int argc, char **argv)
{
  scm_boot_guile (argc, argv, inner_main, 0);
  return 0; /* never reached */
}