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


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

Re: [PATCH] malloc: simplify and fix calloc


On Tue, Jan 26, 2016 at 12:25 AM, Joern Engel <joern@purestorage.com> wrote:
> Calloc was essentially a copy of malloc that tried to skip the memset if
> the memory was just returned from mmap() and is already cleared.  It was
> also buggy and a suitable test could trigger a segfault.

Would it be possible to add such a test to the testsuite?

> New code only does the overflow check, malloc and memset.  Runtime cost
> is lower than maintenance cost of already-buggy code.
>
> JIRA: PURE-27597
> JIRA: PURE-36718
> ---
>  tpc/malloc2.13/malloc.c | 106 +++---------------------------------------------
>  1 file changed, 5 insertions(+), 101 deletions(-)
>
> diff --git a/tpc/malloc2.13/malloc.c b/tpc/malloc2.13/malloc.c
> index d9fecfe3f921..190c1d24b082 100644
> --- a/tpc/malloc2.13/malloc.c
> +++ b/tpc/malloc2.13/malloc.c
> @@ -3506,13 +3506,8 @@ Void_t *public_pVALLOc(size_t bytes)
>
>  Void_t *public_cALLOc(size_t n, size_t elem_size)
>  {
> -       struct malloc_state *av;
> -       mchunkptr oldtop, p;
> -       INTERNAL_SIZE_T bytes, sz, csz, oldtopsize;
> +       INTERNAL_SIZE_T bytes;
>         Void_t *mem;
> -       unsigned long clearsize;
> -       unsigned long nclears;
> -       INTERNAL_SIZE_T *d;
>
>         /* size_t is unsigned so the behavior on overflow is defined.  */
>         bytes = n * elem_size;
> @@ -3525,101 +3520,10 @@ Void_t *public_cALLOc(size_t n, size_t elem_size)
>                 }
>         }
>
> -       __malloc_ptr_t(*hook) __MALLOC_PMT((size_t, __const __malloc_ptr_t)) = force_reg(dlmalloc_hook);
> -       if (hook != NULL) {
> -               sz = bytes;
> -               mem = (*hook) (sz, RETURN_ADDRESS(0));
> -               if (mem == 0)
> -                       return 0;
> -#ifdef HAVE_MEMCPY
> -               return memset(mem, 0, sz);
> -#else
> -               while (sz > 0)
> -                       ((char *)mem)[--sz] = 0;        /* rather inefficient */
> -               return mem;
> -#endif
> -       }
> -
> -       sz = bytes;
> -
> -       av = arena_get(sz);
> -       if (!av)
> -               return 0;
> -
> -       /* Check if we hand out the top chunk, in which case there may be no
> -          need to clear. */
> -#if MORECORE_CLEARS
> -       oldtop = top(av);
> -       oldtopsize = chunksize(top(av));
> -#if MORECORE_CLEARS < 2
> -       /* Only newly allocated memory is guaranteed to be cleared.  */
> -       if (av == &main_arena && oldtopsize < mp_.sbrk_base + av->max_system_mem - (char *)oldtop)
> -               oldtopsize = (mp_.sbrk_base + av->max_system_mem - (char *)oldtop);
> -#endif
> -       if (av != &main_arena) {
> -               heap_info *heap = heap_for_ptr(oldtop);
> -               if (oldtopsize < (char *)heap + heap->mprotect_size - (char *)oldtop)
> -                       oldtopsize = (char *)heap + heap->mprotect_size - (char *)oldtop;
> -       }
> -#endif
> -       mem = _int_malloc(av, sz);
> -       if (mem == 0) {
> -               av = get_backup_arena(av, bytes);
> -               mem = _int_malloc(&main_arena, sz);
> -       }
> -       arena_unlock(av);
> -
> -       assert(!mem || chunk_is_mmapped(mem2chunk(mem)) || av == arena_for_chunk(mem2chunk(mem)));
> -       if (mem == 0)
> -               return 0;
> -       p = mem2chunk(mem);
> -
> -       /* Two optional cases in which clearing not necessary */
> -       if (chunk_is_mmapped(p)) {
> -               if (perturb_byte)
> -                       MALLOC_ZERO(mem, sz);
> -               return mem;
> -       }
> -
> -       csz = chunksize(p);
> -
> -#if MORECORE_CLEARS
> -       if (perturb_byte == 0 && (p == oldtop && csz > oldtopsize)) {
> -               /* clear only the bytes from non-freshly-sbrked memory */
> -               csz = oldtopsize;
> -       }
> -#endif
> -
> -       /* Unroll clear of <= 36 bytes (72 if 8byte sizes).  We know that
> -          contents have an odd number of INTERNAL_SIZE_T-sized words;
> -          minimally 3.  */
> -       d = (INTERNAL_SIZE_T *) mem;
> -       clearsize = csz - SIZE_SZ;
> -       nclears = clearsize / sizeof(INTERNAL_SIZE_T);
> -       assert(nclears >= 3);
> -
> -       if (nclears > 9)
> -               MALLOC_ZERO(d, clearsize);
> -
> -       else {
> -               *(d + 0) = 0;
> -               *(d + 1) = 0;
> -               *(d + 2) = 0;
> -               if (nclears > 4) {
> -                       *(d + 3) = 0;
> -                       *(d + 4) = 0;
> -                       if (nclears > 6) {
> -                               *(d + 5) = 0;
> -                               *(d + 6) = 0;
> -                               if (nclears > 8) {
> -                                       *(d + 7) = 0;
> -                                       *(d + 8) = 0;
> -                               }
> -                       }
> -               }
> -       }
> -
> -       return mem;
> +       mem = public_mALLOc(bytes);
> +       if (!mem)
> +               return NULL;
> +       return memset(mem, 0, bytes);
>  }
>
>
> --
> 2.7.0.rc3
>


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