From 3ea4c43add4f6038830ae0a6e42821481f7ce9a0 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Thu, 25 Feb 2021 09:24:41 +0200 Subject: [PATCH] Cleanup usage of malloc_usable_size. (#8554) * Add better control of malloc_usable_size() usage. * Use malloc_usable_size on alpine libc daily job. * Add no-malloc-usable-size daily jobs. * Fix zmalloc(0) when HAVE_MALLOC_SIZE is undefined. In order to align with the jemalloc behavior, this should never return NULL or OOM panic. --- .github/workflows/daily.yml | 37 ++++++++++++++++++++++++++++++++++++- src/zmalloc.c | 10 ++++++++-- src/zmalloc.h | 11 +++++++++-- 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/.github/workflows/daily.yml b/.github/workflows/daily.yml index 9b9d15988..59d236d9a 100644 --- a/.github/workflows/daily.yml +++ b/.github/workflows/daily.yml @@ -48,6 +48,25 @@ jobs: - name: cluster tests run: ./runtest-cluster + test-ubuntu-no-malloc-usable-size: + runs-on: ubuntu-latest + if: github.repository == 'redis/redis' + timeout-minutes: 14400 + steps: + - uses: actions/checkout@v2 + - name: make + run: make MALLOC=libc CFLAGS=-DNO_MALLOC_USABLE_SIZE + - name: test + run: | + sudo apt-get install tcl8.6 + ./runtest --accurate --verbose --dump-logs + - name: module api test + run: ./runtest-moduleapi --verbose + - name: sentinel tests + run: ./runtest-sentinel + - name: cluster tests + run: ./runtest-cluster + test-ubuntu-32bit: runs-on: ubuntu-latest if: github.repository == 'redis/redis' @@ -132,6 +151,22 @@ jobs: - name: module api test run: ./runtest-moduleapi --valgrind --no-latency --verbose --clients 1 + test-valgrind-no-malloc-usable-size: + runs-on: ubuntu-latest + if: github.repository == 'redis/redis' + timeout-minutes: 14400 + steps: + - uses: actions/checkout@v2 + - name: make + run: make valgrind CFLAGS="-DNO_MALLOC_USABLE_SIZE" + - name: test + run: | + sudo apt-get update + sudo apt-get install tcl8.6 valgrind -y + ./runtest --valgrind --verbose --clients 1 --dump-logs + - name: module api test + run: ./runtest-moduleapi --valgrind --no-latency --verbose --clients 1 + test-centos7-jemalloc: runs-on: ubuntu-latest if: github.repository == 'redis/redis' @@ -250,7 +285,7 @@ jobs: - name: make run: | apk add build-base - make REDIS_CFLAGS='-Werror' USE_JEMALLOC=no + make REDIS_CFLAGS='-Werror' USE_JEMALLOC=no CFLAGS=-DUSE_MALLOC_USABLE_SIZE - name: test run: | apk add tcl procps diff --git a/src/zmalloc.c b/src/zmalloc.c index fbac09616..1dc662e57 100644 --- a/src/zmalloc.c +++ b/src/zmalloc.c @@ -60,6 +60,11 @@ void zlibc_free(void *ptr) { #define ASSERT_NO_SIZE_OVERFLOW(sz) assert((sz) + PREFIX_SIZE > (sz)) #endif +/* When using the libc allocator, use a minimum allocation size to match the + * jemalloc behavior that doesn't return NULL in this case. + */ +#define MALLOC_MIN_SIZE(x) ((x) > 0 ? (x) : sizeof(long)) + /* Explicitly override malloc/free etc when using tcmalloc. */ #if defined(USE_TCMALLOC) #define malloc(size) tc_malloc(size) @@ -93,7 +98,7 @@ static void (*zmalloc_oom_handler)(size_t) = zmalloc_default_oom; * '*usable' is set to the usable size if non NULL. */ void *ztrymalloc_usable(size_t size, size_t *usable) { ASSERT_NO_SIZE_OVERFLOW(size); - void *ptr = malloc(size+PREFIX_SIZE); + void *ptr = malloc(MALLOC_MIN_SIZE(size)+PREFIX_SIZE); if (!ptr) return NULL; #ifdef HAVE_MALLOC_SIZE @@ -153,7 +158,7 @@ void zfree_no_tcache(void *ptr) { * '*usable' is set to the usable size if non NULL. */ void *ztrycalloc_usable(size_t size, size_t *usable) { ASSERT_NO_SIZE_OVERFLOW(size); - void *ptr = calloc(1, size+PREFIX_SIZE); + void *ptr = calloc(1, MALLOC_MIN_SIZE(size)+PREFIX_SIZE); if (ptr == NULL) return NULL; #ifdef HAVE_MALLOC_SIZE @@ -675,6 +680,7 @@ int zmalloc_test(int argc, char **argv) { UNUSED(argc); UNUSED(argv); + printf("Malloc prefix size: %d\n", (int) PREFIX_SIZE); printf("Initial used memory: %zu\n", zmalloc_used_memory()); ptr = zmalloc(123); printf("Allocated 123 bytes; used: %zu\n", zmalloc_used_memory()); diff --git a/src/zmalloc.h b/src/zmalloc.h index 6f3f6354a..d44c7b389 100644 --- a/src/zmalloc.h +++ b/src/zmalloc.h @@ -62,11 +62,18 @@ #endif /* On native libc implementations, we should still do our best to provide a - * HAVE_MALLOC_SIZE capability. + * HAVE_MALLOC_SIZE capability. This can be set explicitly as well: + * + * NO_MALLOC_USABLE_SIZE disables it on all platforms, even if they are + * known to support it. + * USE_MALLOC_USABLE_SIZE forces use of malloc_usable_size() regardless + * of platform. */ #ifndef ZMALLOC_LIB #define ZMALLOC_LIB "libc" -#if defined(__GLIBC__) || defined(__FreeBSD__) +#if !defined(NO_MALLOC_USABLE_SIZE) && \ + (defined(__GLIBC__) || defined(__FreeBSD__) || \ + defined(USE_MALLOC_USABLE_SIZE)) #include #define HAVE_MALLOC_SIZE 1 #define zmalloc_size(p) malloc_usable_size(p)