Search code examples
cmallocvalgrind

How can I resolve a sysmalloc assertion caused by an "Invalid write" and an "Invalid read" (according to Valgrind)?


How can I resolve this sysmalloc assertion:

main: malloc.c:2542: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)' failed.

Am I making some obvious memory related errors in the 2 functions Valgrind refers to (i.e. set_slice_array() and print_int())?

After doing some research, it seems that the error I'm seeing potentially could be caused by me overwriting some record (that malloc needs) on the heap of its past allocations. But I'm still learning the ropes of dynamic memory allocation in C, so I'm not sure how to resolve the issue(s).

Running my program through Valgrind, gives me the following info (and two hints about an "Invalid write" and an "Invalid read"):

==2328== **Invalid write of size 2**
==2328==    at 0x48452E3: memmove (in /nix/store/7p3q3hn63ya4c46swsxlbmy03sbxz3d9-valgrind-3.16.1/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2328==    by 0x4019FB: set_slice_array (main.c:44)
==2328==    by 0x4019FB: extend_slice (main.c:97)
==2328==    by 0x4019FB: join_slices (main.c:107)
==2328==    by 0x4019FB: main (main.c:124)
==2328==  Address 0x4a14698 is 0 bytes after a block of size 40 alloc'd
==2328==    at 0x4840B68: realloc (in /nix/store/7p3q3hn63ya4c46swsxlbmy03sbxz3d9-valgrind-3.16.1/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2328==    by 0x40189D: grow_dynamic_array (main.c:29)
==2328==    by 0x40189D: set_slice_array (main.c:41)
==2328==    by 0x40189D: new_slice_from_array (main.c:59)
==2328==    by 0x40189D: main (main.c:118)
==2328== 
==2328== **Invalid read of size 4**
==2328==    at 0x401B59: print_int (main.c:65)
==2328==    by 0x401B59: print_slice (main.c:71)
==2328==    by 0x401B59: debug_slice (main.c:86)
==2328==    by 0x401B59: main (main.c:125)
==2328==  Address 0x4a14698 is 0 bytes after a block of size 40 alloc'd
==2328==    at 0x4840B68: realloc (in /nix/store/7p3q3hn63ya4c46swsxlbmy03sbxz3d9-valgrind-3.16.1/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2328==    by 0x40189D: grow_dynamic_array (main.c:29)
==2328==    by 0x40189D: set_slice_array (main.c:41)
==2328==    by 0x40189D: new_slice_from_array (main.c:59)
==2328==    by 0x40189D: main (main.c:118)
==2328== 
==2328== 
==2328== HEAP SUMMARY:
==2328==     in use at exit: 168 bytes in 5 blocks
==2328==   total heap usage: 9 allocs, 4 frees, 1,252 bytes allocated
==2328== 
==2328== LEAK SUMMARY:
==2328==    definitely lost: 88 bytes in 3 blocks
==2328==    indirectly lost: 80 bytes in 2 blocks
==2328==      possibly lost: 0 bytes in 0 blocks
==2328==    still reachable: 0 bytes in 0 blocks
==2328==         suppressed: 0 bytes in 0 blocks

Referring to the code below (in main.c) where I'm trying to implement Go's "slice" data structure in C:

    #include <stdio.h>
    #include <stdlib.h>
    
    typedef char byte; 
    
    typedef struct dynamic_array {
        size_t capacity;
        size_t item_size;
        byte *data;
    } dynamic_array;
    
    typedef struct slice {
        size_t start;
        size_t length;
        void (*print_func)(void *);
        dynamic_array *array;
    } slice;
    
    dynamic_array *new_dynamic_array(capacity, item_size) {
        dynamic_array *this = malloc(sizeof(dynamic_array));
        this->capacity = capacity;
        this->item_size = item_size;
        this->data = malloc(capacity * item_size * sizeof(byte));
        return this;
    }
    
    void grow_dynamic_array(dynamic_array *da, size_t length_needed) {
        da->capacity = length_needed;
        da->data = realloc(da->data, length_needed * sizeof(byte) * da->item_size);
    }
    
    void *get_slice_ptr(slice this) {
        return (this.array->data + (this.start * this.array->item_size));
    }
    
    slice set_slice_array(slice this, void *array, size_t capacity) {
        dynamic_array *da = this.array;
        size_t capacity_required = capacity;
    
        while (capacity_required >= da->capacity) { 
            grow_dynamic_array(da, 2 * capacity_required);
        }
        
        memcpy(get_slice_ptr(this), array, da->item_size * capacity); // Culprit?
        this.length += capacity;
        return this;
    }
    
    slice new_slice(size_t capacity, size_t item_size, void (*print_func_ptr)(void *)) {
        slice this;
        this.start = this.length = 0;
        this.print_func = print_func_ptr;
        this.array = new_dynamic_array(capacity, item_size);
        return this;
    }
    
    slice new_slice_from_array(void *array, size_t capacity, size_t item_size, void (*print_func_ptr)(void *)) {
        slice this = new_slice(capacity, item_size, print_func_ptr);
        this = set_slice_array(this, array, capacity);
        return this;
    }
    
    void print_int(void *data_item) {
        int value;
        memcpy(&value, data_item, sizeof(int));
        printf("[%d] ", value);
    }
    
    void print_slice(slice this) {
        for (size_t i = this.start; i < this.start + this.length; ++i) {
            this.print_func(this.array->data + (i * this.array->item_size));
        }
        printf("\n");
    }
    
    void debug_slice(slice this) {
        dynamic_array da = *this.array;
        printf("Length: %ld\nStart: %ld\nArray: %p\nCapacity: %ld\nItem_size: %ld\nData: %p\n",
            this.length,
            this.start,
            (void *) this.array,
            da.capacity,
            da.item_size,
            (void *) da.data
        );
        print_slice(this);
    }
    
    slice sub_slice(slice source, size_t sub_start, size_t sub_length) {
        slice portion = source;
        portion.start = source.start + sub_start;
        portion.length = sub_length;
        return portion;
    }
    
    slice extend_slice(slice this, const void *item, size_t count) {
        slice extension = set_slice_array(sub_slice(this, this.length, 0), item, count);
        this.length = this.length + extension.length; 
        return this;
    }
    
    slice append_slice(slice this, const void *item) {
        return extend_slice(this, item, 1);
    }
    
    slice join_slices(slice base, slice extension) {
        return extend_slice(base, get_slice_ptr(extension), extension.length);
    }
    
    int main(void) {
        int test_array_ints[] = { 1, 2, 3, 4, 5 };
    
        // First slice & debug print
        slice test_slice1 = new_slice_from_array(&test_array_ints, 5, sizeof(int), print_int);
        debug_slice(test_slice1);
    
        printf("\n"); // Second slice & debug print
        slice test_slice2 = new_slice_from_array(&test_array_ints, 5, sizeof(int), print_int);
        int to_add = 6;
        test_slice2 = append_slice(test_slice2, &to_add); // One culprit?
        debug_slice(test_slice2);
        
        printf("\n"); // Merge the 2 slices and then debug print
        test_slice2 = join_slices(test_slice2, test_slice1);
        debug_slice(test_slice2);
    
        printf("\n"); // Create a third slice (which triggers the assert)
        slice test_slice3 = new_slice_from_array(&test_array_ints, 5, sizeof(int), print_int); 
    
        return 0;    
    }

Any help would be much appreciated!


Solution

  • size_t capacity_required = capacity;
    memcpy(get_slice_ptr(this),
    

    Well, decide - either capacity_required is the capacity including this.start, or capacity_required is the whole capacity and you are copying to this.array->data.

     size_t capacity_required = capacity + this.start;
     memcpy(get_slice_ptr(this),
    

    or

     size_t capacity_required = capacity;
     this.start = 0;
     memcpy(this.array->data,
    

    Depending on what set_slice_array should exactly do. Should it reallocate and replace elements in the underlying storage, or should it "set" the slice to a region and re-construct the slice? I do not know, there are no comments.

    The loop while (capacity_required >= da->capacity) is odd, that's an if. I would move that if inside grow_dynamic_array anyway.