From: Josef 'Jeff' Sipek Date: Wed, 5 Jul 2017 13:08:41 +0000 (+0300) Subject: lib: remove support for Boehm GC X-Git-Tag: 2.3.0.rc1~1292 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=cbe4d8212ec8b9014ae7ae892e90abb98a15d797;p=thirdparty%2Fdovecot%2Fcore.git lib: remove support for Boehm GC There were several issues with our usage of Boehm GC: - it didn't help with memory fragmentation - it is not very efficient - it was rarely enabled & used Proper use of a GC would involve radical changes to how we write code and cause portability issues. We can get most of the benefits of a GC with data stacks and alloc-only memory pools. --- diff --git a/configure.ac b/configure.ac index 6ba95ce585..6676f10986 100644 --- a/configure.ac +++ b/configure.ac @@ -267,11 +267,6 @@ if test "$systemdsystemunitdir" != ""; then fi AM_CONDITIONAL(HAVE_SYSTEMD, test "$systemdsystemunitdir" != "") -AC_ARG_WITH(gc, -AS_HELP_STRING([--with-gc], [Use Boehm garbage collector]), - TEST_WITH(gc, $withval), - want_gc=no) - DC_DOVECOT_MODULEDIR AC_ARG_WITH(docs, @@ -525,8 +520,6 @@ DOVECOT_MNTCTL DOVECOT_SSL -DOVECOT_GC - dnl ** dnl ** userdb and passdb checks diff --git a/doc/securecoding.txt b/doc/securecoding.txt index e3e3306223..72489ae427 100644 --- a/doc/securecoding.txt +++ b/doc/securecoding.txt @@ -57,9 +57,6 @@ Accessing freed memory is the most difficult problem to solve with C code. Only real solution is to use garbage collector, but it's not possible to write a portable GC without radical changes in how you write code. -I've added support for Boehm GC, but it doesn't seem to be working very -well currently. In any case I'd rather not make it required. - There are a few ways to avoid most free() calls however: data stack and memory pools. diff --git a/m4/gc.m4 b/m4/gc.m4 deleted file mode 100644 index 473993ced4..0000000000 --- a/m4/gc.m4 +++ /dev/null @@ -1,17 +0,0 @@ -dnl ** -dnl ** Garbage Collector -dnl ** - -AC_DEFUN([DOVECOT_GC], [ - if test $want_gc != no; then - AC_CHECK_LIB(gc, GC_malloc, [ - AC_CHECK_HEADERS(gc/gc.h gc.h) - AC_DEFINE(USE_GC,, [Define if you want to use Boehm GC]) - LIBS="$LIBS -lgc" - ], [ - if test $want_gc = yes; then - AC_ERROR([Can't build with GC: libgc not found]) - fi - ]) - fi -]) diff --git a/src/lib/data-stack.c b/src/lib/data-stack.c index 83d64020a1..c4c8e3be02 100644 --- a/src/lib/data-stack.c +++ b/src/lib/data-stack.c @@ -6,12 +6,6 @@ #include "data-stack.h" -#ifdef HAVE_GC_GC_H -# include -#elif defined (HAVE_GC_H) -# include -#endif - /* Initial stack size - this should be kept in a size that doesn't exceed in a normal use to avoid extra malloc()ing. */ #ifdef DEBUG @@ -152,11 +146,7 @@ data_stack_frame_t t_push(const char *marker) frame_pos = 0; if (unused_frame_blocks == NULL) { /* allocate new block */ -#ifndef USE_GC frame_block = calloc(sizeof(*frame_block), 1); -#else - frame_block = GC_malloc(sizeof(*frame_block)); -#endif if (frame_block == NULL) { i_fatal_status(FATAL_OUTOFMEM, "t_push(): Out of memory"); @@ -220,15 +210,11 @@ static void free_blocks(struct stack_block *block) memset(STACK_BLOCK_DATA(block), CLEAR_CHR, block->size); if (unused_block == NULL || block->size > unused_block->size) { -#ifndef USE_GC free(unused_block); -#endif unused_block = block; } else { -#ifndef USE_GC if (block != &outofmem_area.block) free(block); -#endif } block = next; @@ -348,11 +334,7 @@ static struct stack_block *mem_block_alloc(size_t min_size) /* nearest_power() returns 2^n values, so alloc_size can't be anywhere close to SIZE_MAX */ -#ifndef USE_GC block = malloc(SIZEOF_MEMBLOCK + alloc_size); -#else - block = GC_malloc(SIZEOF_MEMBLOCK + alloc_size); -#endif if (unlikely(block == NULL)) { if (outofmem) { if (min_size > outofmem_area.block.left) @@ -622,7 +604,6 @@ void data_stack_deinit(void) frame_pos != BLOCK_FRAME_COUNT-1) i_panic("Missing t_pop() call"); -#ifndef USE_GC while (unused_frame_blocks != NULL) { struct stack_frame_block *frame_block = unused_frame_blocks; unused_frame_blocks = unused_frame_blocks->prev; @@ -632,7 +613,6 @@ void data_stack_deinit(void) free(current_block); free(unused_block); -#endif unused_frame_blocks = NULL; current_block = NULL; unused_block = NULL; diff --git a/src/lib/mempool-alloconly.c b/src/lib/mempool-alloconly.c index 5b0a5e9f55..66ba6bfcc2 100644 --- a/src/lib/mempool-alloconly.c +++ b/src/lib/mempool-alloconly.c @@ -6,12 +6,6 @@ #include "mempool.h" -#ifdef HAVE_GC_GC_H -# include -#elif defined (HAVE_GC_H) -# include -#endif - #define MAX_ALLOC_SIZE SSIZE_T_MAX struct alloconly_pool { @@ -189,9 +183,7 @@ static void pool_alloconly_destroy(struct alloconly_pool *apool) } #endif -#ifndef USE_GC free(block); -#endif } static const char *pool_alloconly_get_name(pool_t pool ATTR_UNUSED) @@ -250,11 +242,7 @@ static void block_alloc(struct alloconly_pool *apool, size_t size) #endif } -#ifndef USE_GC block = calloc(size, 1); -#else - block = GC_malloc(size); -#endif if (unlikely(block == NULL)) { i_fatal_status(FATAL_OUTOFMEM, "block_alloc(%"PRIuSIZE_T "): Out of memory", size); @@ -386,9 +374,7 @@ static void pool_alloconly_clear(pool_t pool) SIZEOF_POOLBLOCK + block->size); } #endif -#ifndef USE_GC free(block); -#endif } /* clear the first block */ diff --git a/src/lib/mempool-system.c b/src/lib/mempool-system.c index f3e646d846..6cf4719373 100644 --- a/src/lib/mempool-system.c +++ b/src/lib/mempool-system.c @@ -14,12 +14,6 @@ # include /* Linux */ #endif -#ifdef HAVE_GC_GC_H -# include -#elif defined (HAVE_GC_H) -# include -#endif - #define CLEAR_CHR 0xde static const char *pool_system_get_name(pool_t pool); @@ -78,11 +72,7 @@ static void *pool_system_malloc(pool_t pool ATTR_UNUSED, size_t size) if (unlikely(size == 0 || size > SSIZE_T_MAX)) i_panic("Trying to allocate %"PRIuSIZE_T" bytes", size); -#ifndef USE_GC mem = calloc(size, 1); -#else - mem = GC_malloc(size); -#endif if (unlikely(mem == NULL)) { i_fatal_status(FATAL_OUTOFMEM, "pool_system_malloc(%"PRIuSIZE_T "): Out of memory", size); @@ -99,12 +89,10 @@ void pool_system_free(pool_t pool ATTR_UNUSED, void *mem ATTR_UNUSED) #ifdef DEBUG int old_errno = errno; #endif -#if !defined(USE_GC) && defined(HAVE_MALLOC_USABLE_SIZE) && defined(DEBUG) +#if defined(HAVE_MALLOC_USABLE_SIZE) && defined(DEBUG) safe_memset(mem, CLEAR_CHR, malloc_usable_size(mem)); #endif -#ifndef USE_GC free(mem); -#endif #ifdef DEBUG /* we rely on errno not changing. it shouldn't. */ i_assert(errno == old_errno); @@ -121,16 +109,12 @@ static void *pool_system_realloc(pool_t pool ATTR_UNUSED, void *mem, i_assert(old_size == 0); return pool_system_malloc(pool, new_size); } -#if !defined(USE_GC) && defined(HAVE_MALLOC_USABLE_SIZE) +#if defined(HAVE_MALLOC_USABLE_SIZE) i_assert(old_size == (size_t)-1 || mem == NULL || old_size <= malloc_usable_size(mem)); #endif -#ifndef USE_GC mem = realloc(mem, new_size); -#else - mem = GC_realloc(mem, new_size); -#endif if (unlikely(mem == NULL)) { i_fatal_status(FATAL_OUTOFMEM, "pool_system_realloc(%"PRIuSIZE_T "): Out of memory", new_size);