This is the mail archive of the libc-hacker@sourceware.org mailing list for the glibc project.

Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] malloc_set_size fixes


Hi!

The way emacs uses malloc_[gs]et_state unfortunately makes some pieces of
malloc's internals part of glibc ABI, we need to ensure that emacs dumped
with older glibc will work even against a newer glibc.

In the last 30 days there were 3 disruptive changes:
1) the 2007-04-30 BZ#4349 change - if emacs is dumped against older
   glibc than that, {fd,bk}_nextsize fields are uninitialized, but when
   run against more recent glibc, it expects they are initialized
   and crashes otherwise.  Fixed below in malloc_set_state, by putting
   in that case all bins into unsorted bins and clearing those fields
   - malloc will later on when moving from unsorted to sorted bins
   recompute those fields.
2) I turned on MALLOC_DEBUG in my tree when testing the {fd,bk}_nextsize
   changes and accidentally committed it together with arena.c changes
   on May, 7th.  This in itself just means glibc 2.6 malloc is slower
   than it could.
3) unfortunately 2007-05-13 changes added a field in the middle
   of struct malloc_save_state, without bumping version.  Although
   that was added only if MALLOC_DEBUG was defined, together with
   2) this means glibc 2.6 as was released last week is malloc_set_state
   incompatible with other glibcs and there is no way to work around
   that (other than quessing from the values of the structure).

The following patch fixes 1), reverts both 2) and 3) and removes
a bogus assert mp_.n_mmaps <= mp_.n_mmaps_max.  IMHO that's the best
fix, mp_.n_mmaps_max can be changed to any value any time through
mallopt and so it is possible it is smaller than n_mmaps.  Also,
ialloc clears mp_.n_mmaps_max temporarily, so it likely is smaller
than mp_.n_mmaps in some cases.
n_mmaps_cmax field just basically contains MAX(mp_.n_mmaps, mp_.n_mmaps_max)
and comparing that to mp_.n_mmaps in an assert doesn't make much sense
to me (and furthermore wastes a few bytes in mp_ and causes ABI
nightmare in malloc_*et_state).
If this is committed, it means that running emacs dumped with pre-2007-04-30
glibc against stock glibc 2.6 will not work and similarly running emacs
dumped with glibc after this patch won't work against glibc 2.6.
Running emacs dumped with glibc < 2.6 or glibc 2.6.1+/2.7+ will work
against glibc != 2.6.  Therefore, once this gets tested, I think it would
be a good idea to document this and release glibc 2.6.1 with these changes
soon.

So far I tried to run glibc 2.5 dumped emacs against libc with this
patch (both -DMALLOC_DEBUG and without compiled libc) and it worked
just fine.

2007-05-21  Jakub Jelinek  <jakub@redhat.com>

	* malloc/hooks.c (MALLOC_STATE_VERSION): Bump.
	(public_sET_STATe): If ms->version < 3, put all chunks into
	unsorted chunks and clear {fd,bk}_nextsize fields of largebin
	chunks.
	* malloc/malloc.c (do_check_malloc_state): Don't assert
	n_mmaps is not greater than n_mmaps_max.

	* malloc/malloc.c [MALLOC_DEBUG]: Revert 2007-05-13 changes.
	* malloc/hooks.c: Likewise.
	* malloc/arena.c: Likewise.
	* malloc/Makefile (CFLAGS-malloc.c): Revert accidental
	2007-05-07 commit.

--- libc/malloc/arena.c.jj	2007-05-20 20:01:52.000000000 +0200
+++ libc/malloc/arena.c	2007-05-21 11:18:13.000000000 +0200
@@ -370,9 +370,6 @@ ptmalloc_init_minimal (void)
   mp_.top_pad        = DEFAULT_TOP_PAD;
 #endif
   mp_.n_mmaps_max    = DEFAULT_MMAP_MAX;
-#if MALLOC_DEBUG
-  mp_.n_mmaps_cmax   = DEFAULT_MMAP_MAX;
-#endif
   mp_.mmap_threshold = DEFAULT_MMAP_THRESHOLD;
   mp_.trim_threshold = DEFAULT_TRIM_THRESHOLD;
   mp_.pagesize       = malloc_getpagesize;
--- libc/malloc/malloc.c.jj	2007-05-20 20:01:52.000000000 +0200
+++ libc/malloc/malloc.c	2007-05-21 11:19:32.000000000 +0200
@@ -2358,9 +2358,6 @@ struct malloc_par {
   /* Memory map support */
   int              n_mmaps;
   int              n_mmaps_max;
-#if MALLOC_DEBUG
-  int              n_mmaps_cmax;
-#endif
   int              max_n_mmaps;
   /* the mmap_threshold is dynamic, until the user sets
      it manually, at which point we need to disable any
@@ -2876,8 +2873,6 @@ static void do_check_malloc_state(mstate
   assert(total <= (unsigned long)(mp_.max_total_mem));
   assert(mp_.n_mmaps >= 0);
 #endif
-  assert(mp_.n_mmaps <= mp_.n_mmaps_cmax);
-  assert(mp_.n_mmaps_max <= mp_.n_mmaps_cmax);
   assert(mp_.n_mmaps <= mp_.max_n_mmaps);
 
   assert((unsigned long)(av->system_mem) <=
@@ -3475,13 +3470,6 @@ munmap_chunk(p) mchunkptr p;
     }
 
   mp_.n_mmaps--;
-#if MALLOC_DEBUG
-  if (mp_.n_mmaps_cmax > mp_.n_mmaps_max)
-    {
-      assert (mp_.n_mmaps_cmax == mp_.n_mmaps + 1);
-      mp_.n_mmaps_cmax = mp_.n_mmaps;
-    }
-#endif
   mp_.mmapped_mem -= total_size;
 
   int ret __attribute__ ((unused)) = munmap((char *)block, total_size);
@@ -5397,9 +5385,6 @@ mstate av; size_t n_elements; size_t* si
   mp_.n_mmaps_max = 0;
   mem = _int_malloc(av, size);
   mp_.n_mmaps_max = mmx;   /* reset mmap */
-#if MALLOC_DEBUG
-  mp_.n_mmaps_cmax = mmx;
-#endif
   if (mem == 0)
     return 0;
 
@@ -5725,17 +5710,8 @@ int mALLOPt(param_number, value) int par
       res = 0;
     else
 #endif
-      {
-#if MALLOC_DEBUG
-	if (mp_.n_mmaps <= value)
-	  mp_.n_mmaps_cmax = value;
-	else
-	  mp_.n_mmaps_cmax = mp_.n_mmaps;
-#endif
-
-	mp_.n_mmaps_max = value;
-	mp_.no_dyn_threshold = 1;
-      }
+      mp_.n_mmaps_max = value;
+      mp_.no_dyn_threshold = 1;
     break;
 
   case M_CHECK_ACTION:
--- libc/malloc/hooks.c.jj	2007-05-20 20:01:52.000000000 +0200
+++ libc/malloc/hooks.c	2007-05-21 11:18:13.000000000 +0200
@@ -496,7 +496,7 @@ free_starter(mem, caller) Void_t* mem; c
    then the hooks are reset to 0.  */
 
 #define MALLOC_STATE_MAGIC   0x444c4541l
-#define MALLOC_STATE_VERSION (0*0x100l + 2l) /* major*0x100 + minor */
+#define MALLOC_STATE_VERSION (0*0x100l + 3l) /* major*0x100 + minor */
 
 struct malloc_save_state {
   long          magic;
@@ -507,9 +507,6 @@ struct malloc_save_state {
   unsigned long trim_threshold;
   unsigned long top_pad;
   unsigned int  n_mmaps_max;
-#if MALLOC_DEBUG
-  unsigned int  n_mmaps_cmax;
-#endif
   unsigned long mmap_threshold;
   int           check_action;
   unsigned long max_sbrked_mem;
@@ -553,9 +550,6 @@ public_gET_STATe(void)
   ms->trim_threshold = mp_.trim_threshold;
   ms->top_pad = mp_.top_pad;
   ms->n_mmaps_max = mp_.n_mmaps_max;
-#if MALLOC_DEBUG
-  ms->n_mmaps_cmax = mp_.n_mmaps_cmax;
-#endif
   ms->mmap_threshold = mp_.mmap_threshold;
   ms->check_action = check_action;
   ms->max_sbrked_mem = main_arena.max_system_mem;
@@ -601,8 +595,9 @@ public_sET_STATe(Void_t* msptr)
       assert(ms->av[2*i+3] == 0);
       first(b) = last(b) = b;
     } else {
-      if(i<NSMALLBINS || (largebin_index(chunksize(ms->av[2*i+2]))==i &&
-			  largebin_index(chunksize(ms->av[2*i+3]))==i)) {
+      if(ms->version >= 3 &&
+	 (i<NSMALLBINS || (largebin_index(chunksize(ms->av[2*i+2]))==i &&
+			   largebin_index(chunksize(ms->av[2*i+3]))==i))) {
 	first(b) = ms->av[2*i+2];
 	last(b) = ms->av[2*i+3];
 	/* Make sure the links to the bins within the heap are correct.  */
@@ -622,14 +617,22 @@ public_sET_STATe(Void_t* msptr)
       }
     }
   }
+  if (ms->version < 3) {
+    /* Clear fd_nextsize and bk_nextsize fields.  */
+    b = unsorted_chunks(&main_arena)->fd;
+    while (b != unsorted_chunks(&main_arena)) {
+      if (!in_smallbin_range(chunksize(b))) {
+	b->fd_nextsize = NULL;
+	b->bk_nextsize = NULL;
+      }
+      b = b->fd;
+    }
+  }
   mp_.sbrk_base = ms->sbrk_base;
   main_arena.system_mem = ms->sbrked_mem_bytes;
   mp_.trim_threshold = ms->trim_threshold;
   mp_.top_pad = ms->top_pad;
   mp_.n_mmaps_max = ms->n_mmaps_max;
-#if MALLOC_DEBUG
-  mp_.n_mmaps_cmax = ms->n_mmaps_cmax;
-#endif
   mp_.mmap_threshold = ms->mmap_threshold;
   check_action = ms->check_action;
   main_arena.max_system_mem = ms->max_sbrked_mem;
--- libc/malloc/Makefile.jj	2007-04-30 11:51:54.000000000 +0200
+++ libc/malloc/Makefile	2007-05-21 11:20:41.000000000 +0200
@@ -104,7 +104,6 @@ $(objpfx)memusagestat: $(memusagestat-mo
 include ../Rules
 
 CFLAGS-mcheck-init.c = $(PIC-ccflag)
-CFLAGS-malloc.c += -DMALLOC_DEBUG
 
 $(objpfx)libmcheck.a: $(objpfx)mcheck-init.o
 	-rm -f $@

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]