malloc align memory C

Question

I have to recode malloc, free and realloc for a school project.

I done every function but when I try to do ls -lRa /home or cat or gcc I got a segmentation fault after a while of execution... I tried to search the error with valgrind but I don't understand the errors it gives me.

At school someone told me that I have to align my pointers on 16 bytes to execute ls, etc. So I modified the size of the blocks that I allocate with sbrk to only get sizes multiple of 16 and my structure for holding the blocks malloced is 32 bytes. I verified with small hand made programs and all the pointers returned by my malloc are aligned on 16 bytes with this method. Maybe there is a better way to align my pointers?

Here is the error:

/home/abinder/.cache/mozilla/firefox/wv0yeusd.default/cache2/doomed:
total 8
drwxr-xr-x. 2 abinder abinder 4096 11 févr. 17:08 .
drwx------. 4 abinder abinder 4096 11 févr. 17:06 ..

/home/abinder/.cache/mozilla/firefox/wv0yeusd.default/cache2/entries:
==3813== Invalid read of size 1
==3813==    at 0x550AF0D: strcoll_l (in /usr/lib64/libc-2.25.so)
==3813==    by 0x1171EB: ??? (in /usr/bin/ls)
==3813==    by 0x1171B0: ??? (in /usr/bin/ls)
==3813==    by 0x1171B0: ??? (in /usr/bin/ls)
==3813==    by 0x1172C0: ??? (in /usr/bin/ls)
==3813==    by 0x1172C0: ??? (in /usr/bin/ls)
==3813==    by 0x1172AF: ??? (in /usr/bin/ls)
==3813==    by 0x1172AF: ??? (in /usr/bin/ls)
==3813==    by 0x1171B0: ??? (in /usr/bin/ls)
==3813==    by 0x10DB5B: ??? (in /usr/bin/ls)
==3813==    by 0x1122CA: ??? (in /usr/bin/ls)
==3813==    by 0x10C53C: ??? (in /usr/bin/ls)
==3813==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==3813==
==3813==
==3813== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==3813==  Access not within mapped region at address 0x0
==3813==    at 0x550AF0D: strcoll_l (in /usr/lib64/libc-2.25.so)
==3813==    by 0x1171EB: ??? (in /usr/bin/ls)
==3813==    by 0x1171B0: ??? (in /usr/bin/ls)
==3813==    by 0x1171B0: ??? (in /usr/bin/ls)
==3813==    by 0x1172C0: ??? (in /usr/bin/ls)
==3813==    by 0x1172C0: ??? (in /usr/bin/ls)
==3813==    by 0x1172AF: ??? (in /usr/bin/ls)
==3813==    by 0x1172AF: ??? (in /usr/bin/ls)
==3813==    by 0x1171B0: ??? (in /usr/bin/ls)
==3813==    by 0x10DB5B: ??? (in /usr/bin/ls)
==3813==    by 0x1122CA: ??? (in /usr/bin/ls)
==3813==    by 0x10C53C: ??? (in /usr/bin/ls)
==3813==  If you believe this happened as a result of a stack
==3813==  overflow in your program's main thread (unlikely but
==3813==  possible), you can try to increase the size of the
==3813==  main thread stack using the --main-stacksize= flag.
==3813==  The main thread stack size used in this run was 8388608.
==3813==
==3813== Process terminating with default action of signal 11 (SIGSEGV)
==3813==  General Protection Fault
==3813==    at 0x55C016C: _dl_catch_error (in /usr/lib64/libc-2.25.so)
==3813==    by 0x55BF8E6: __libc_dlclose (in /usr/lib64/libc-2.25.so)
==3813==    by 0x55EB5E4: free_mem (in /usr/lib64/libc-2.25.so)
==3813==    by 0x55EB1E1: __libc_freeres (in /usr/lib64/libc-2.25.so)
==3813==    by 0x4A296DB: _vgnU_freeres (vg_preloaded.c:77)
==3813==
==3813== HEAP SUMMARY:
==3813==     in use at exit: 1,313,866 bytes in 5,125 blocks
==3813==   total heap usage: 5,125 allocs, 0 frees, 1,313,866 bytes allocated
==3813==
==3813== LEAK SUMMARY:
==3813==    definitely lost: 1,307,936 bytes in 5,110 blocks
==3813==    indirectly lost: 0 bytes in 0 blocks
==3813==      possibly lost: 2,560 bytes in 10 blocks
==3813==    still reachable: 3,370 bytes in 5 blocks
==3813==         suppressed: 0 bytes in 0 blocks
==3813== Rerun with --leak-check=full to see details of leaked memory
==3813==
==3813== For counts of detected and suppressed errors, rerun with: -v
==3813== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Segmentation fault (core dumped)

If you want to test my code:

here is my struct for the linked list:

typedef struct s_memory {
    size_t             size;
    unsigned short int free;
    struct s_memory    *next;
    void               *data;
} memory_t;

here are my malloc, free and realloc functions:

#define align4(x)  (((((x) - 1) >> 4) << 4) + 16)

memory_t *list = NULL;

void show_alloc_mem(void) {
    show_list(list);
    fprintf(stdout, "\n");
}

void free(void *ptr) {
    unsigned char *ptr_casted = (unsigned char *)ptr;
    memory_t *to_free = ptr - sizeof(memory_t);
    memory_t *buff = list;

    if (ptr == NULL || ptr >= sbrk(0) || to_free->data != ptr)
        return;
    to_free->free = 1;
    if (buff != NULL && buff->free == 1) {
        while (buff != NULL && buff->free != 0) {
            list = list->next;
            brk(buff);
            buff = list;
        }
    } else
        *(ptr_casted) = (unsigned char)0;
}

void *malloc(size_t size) {
    memory_t *freed;
    void *newMem;

    size = align4(size);
    freed = search_freed(&list, size);
    if (freed == NULL) {
        newMem = add_in_memorylist(&list, size);
        if (newMem == NULL) {
            fprintf(stderr, "no more ram to alloc %ld !\n", size);
            return NULL;
        } else {
            return newMem;
        }
    } else
        resize_in_memory(freed, size);

    return ((void *)(unsigned char *)freed + sizeof(memory_t));
}

void *cpy_mem(memory_t *to_realloc, size_t size) {
    long double *new;
    long double *to_real_cast = (long double *)to_realloc;

    if (((void *)(unsigned char *)to_realloc +
            sizeof(memory_t) + to_realloc->size) == sbrk(0)) {
        new = sbrk(align4(size - to_realloc->size));
        if (new == (void *)-1)
            return NULL;
        to_realloc->size += align4(size - to_realloc->size);
        return ((void *)(unsigned char *)to_realloc + sizeof(memory_t));
    }
    new = malloc(align4(size));
    if (new == NULL)
        return NULL;
    for (size_t i = 0; i < to_realloc->size / 16 && i < size / 16; i++) {
        new[i] = to_real_cast[(sizeof(memory_t) / 16) + i];
    }
    free((void *)(unsigned char *)to_realloc + sizeof(memory_t));
    return new;
}

void *realloc(void *ptr, size_t size) {
    memory_t *to_realloc = ptr - sizeof(memory_t);

    if (ptr == NULL && size > 0)
        return malloc(align4(size));
    else if (size <= 0) {
        free(ptr);
        return NULL;
    } else
        return cpy_mem(to_realloc, size);
}

and here are the functions to handle the linked list:

void resize_in_memory(memory_t *freed, size_t size) {
    memory_t *new_struct;
    unsigned char *new_struct_cast;

    freed->free = 0;
    if (freed->size >= size + sizeof(memory_t) + 16) {
        new_struct = ((void *)(unsigned char *)freed + sizeof(memory_t) + size);
        new_struct_cast = (unsigned char *)new_struct;
        new_struct->data = new_struct_cast + sizeof(memory_t);
        new_struct->size = freed->size - sizeof(memory_t) - size;
        new_struct->free = 1;
        new_struct->next = freed->next;
        freed->next = new_struct;
        freed->size = size;
    }
}

void *add_in_memorylist(memory_t **list, size_t size) {
    memory_t *new_struct = NULL;
    void *data_space = NULL;

    new_struct = sbrk(sizeof(memory_t));
    data_space = sbrk(size);
    if (new_struct == (void *)-1 || data_space == (void *)-1)
        return NULL;
    new_struct->data = data_space;
    new_struct->size = sbrk(0) - data_space;
    new_struct->free = 0;
    new_struct->next = *list;
    *list = new_struct;
    return data_space;
}

memory_t *merge_memory(memory_t *buff) {
    size_t size = 0;
    int ok = 0;
    memory_t *ptr = buff;

    while (ok == 0) {
        if (ptr->next == NULL || ptr->next->free == 0) {
            size += ptr->size;
            ok = 1;
        } else
            size += ptr->size + sizeof(memory_t);

        if (ptr->next != NULL)
            ptr = ptr->next;
    }
    if (ptr != buff)
        ptr->size = size;
    return ptr;
}

memory_t *search_freed(memory_t **list, size_t size) {
    memory_t *buff = *list;
    memory_t *ret = NULL;

    while (buff != NULL && ret == NULL) {
        if (buff->free == 0 && buff->next != NULL && buff->next->free == 1)
            buff->next = merge_memory(buff->next);
        if (buff->size >= size && buff->free == 1)
            ret = buff;
        buff = buff->next;
    }
    return ret;
}

PS: I am not asking you to give me a working code from what I gave you but to tell me what I did wrong and eventually where in my code I made mistakes.

Note that malloc will allocate a "memory block that is suitably aligned for any object type." If you need specific allignment requirements (that are different from 4 or 8 bytes on a 32 or 64 bit platform) you should first of all check if your operating system have such functions (it usually have). — Some programmer dude
– Some programmer dude, Commented Feb 11, 2018 at 21:23
what do you mean with "I done every function but when i try to do "ls -lRa /home" or "cat" or "gcc" I got a segmentation fault after a while of execution.." — RedArrow
– RedArrow, Commented Feb 11, 2018 at 22:33
As sizeof(memory_t) may not be a multiple of 16, how does code insure malloc() results are 16-aligned? — chux
– chux, Commented Feb 12, 2018 at 1:52
I mean that i wrote my malloc, realloc, free functions and I tested them. And the problem comes when i want to execute commands using my own functions and not the system's ones — Adrien Binder
– Adrien Binder, Commented Feb 12, 2018 at 10:26
Actually my struct "memory_t" is of size 32 and I transform every size passed to my malloc function to a multiple of 16 so I think that all the block from my malloc() are 16-aligned — Adrien Binder
– Adrien Binder, Commented Feb 12, 2018 at 10:31

chqrlie · Accepted Answer · 2019-07-07 09:41:17Z

Here are some remarks:

The macro to round the size up to a multiple of 16 is usually written without shifts:

    #define align4(x)  (((x) + 15) & ~(size_t)15)

Bear in mind that both versions convert extremely large sizes to 0, which may lead to successful allocations with an incorrect size.

Regarding the linked list structure:

the member data seems redundant since it always points to the byte that follows the structure. You could instead use this space to make the list doubly linked to simplify free block coalescing in free and remove the need for merge_memory.

In function free():

missing cast: memory_t *to_free = ptr - sizeof(memory_t);
calling brk(buff); in free() seems incorrect. You release memory back to the system that might not be the last free block.
what is the purpose of *(ptr_casted) = (unsigned char)0;?

In function malloc():

((void *)(unsigned char *)freed + sizeof(memory_t)); is incorrect because the parentheses are misplaced: you add the size to a void * pointer. The expression could be simplified as (void *)(freed + 1); or simply freed + 1.

in function realloc():

void * arithmetic again: memory_t *to_realloc = ptr - sizeof(memory_t); Use this instead: memory_t *to_realloc = (memory_t *)ptr - 1;
because of the overflow problem, you should not write return malloc(align4(size)); but simply return malloc(size);

In function add_in_memorylist:

you request 2 different blocks for the linked list srtucture and the data block. This leads to an important overhead and is inconsistent with the method used in free to locate the list link. You should instead request a block of size size + sizeof(memory_t), rounded up to a multiple of the page size, a multiple of 4K for this matter.
similarly, you keep calling sbrk(0). You should instead use a global variable to keep track of the arena size and only make system calls to extend it (or possibly shrink it, but that's more difficult).

Also note that you should redefine calloc() to ensure that the version from the C library does not get linked as this version might be incompatible with your redefined version of malloc().

Collectives™ on Stack Overflow

malloc align memory C

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related