Program crashes at realloc on direct run, but runs fine in debug mode

Question

I'm trying to implement binary heap with dynamically allocating and free memory as and when new nodes are inserted or deleted. So, whenever insert/delete node is called, I'm using realloc to increase/decrease the memory. Program runs fine on Debug mode, but when i run it directly it crashes (possibly at realloc)

My reasoning is due to the fact that if I remove the realloc inside the delete function (this means i will never free already allocated memory), the program runs fine on direct run. What could be the issue in the code?

P.S: I'm using Eclipse CDT along with Cygwin on Windows 10

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <stdbool.h>

typedef struct heap
{
    uint32_t size;
    int32_t* heaparray;
}heap;


void insert_max(heap** h1, int32_t value)
{
    uint32_t hole;
    heap* h = *h1;

    if(h->size == 0)
    {
        h->size = 2;
        h->heaparray = (int32_t *)(malloc(sizeof(int32_t) * h->size));
        h->heaparray[0] = 0;
        h->heaparray[1] = value;
        return;
    }

    hole = h->size++;
    h->heaparray[0] = value;
    h->heaparray = (int32_t *)(realloc(h->heaparray , sizeof(int32_t) * h->size));

    //sift up
    for(; value > h->heaparray[hole/2]; hole /= 2)
    {
        h->heaparray[hole] = h->heaparray[hole/2];
    }
    h->heaparray[hole] = value;
}

void printheap(heap* h)
{
    uint32_t index;
    printf("\nHeap: ");
    for(index = 1; index < h->size; index++)
    {
        printf("%3d\t", h->heaparray[index]);
    }
}

void siftDown_max(heap** h1, uint32_t index)
{
    uint32_t rightIndex, leftIndex, maxIndex, temp;
    heap* h = *h1;
    leftIndex = (2 * index);
    rightIndex = (2 * index) + 1;
    if(rightIndex >= h->size)
    {
        if(leftIndex >= h->size)
            return;
        else
        {
            maxIndex = leftIndex;
        }
    }
    else
    {
        if(h->heaparray[rightIndex] >= h->heaparray[leftIndex])
        {
            maxIndex = rightIndex;
        }
        else
        {
            maxIndex = leftIndex;
        }
    }
    if(h->heaparray[index] < h->heaparray[maxIndex])
    {
        temp = h->heaparray[index];
        h->heaparray[index] = h->heaparray[maxIndex];
        h->heaparray[maxIndex] = temp;
        siftDown_max(h1, maxIndex);
    }
}

void siftDown_min(heap** h1, uint32_t index)
{
    uint32_t rightIndex, leftIndex, minIndex, temp;
    heap* h = *h1;
    leftIndex = 2 * index;
    rightIndex = (2 * index) + 1;

    if(rightIndex >= h->size)
    {
        if(leftIndex >= h->size)
        {
            return;
        }
        else
        {
            minIndex = leftIndex;
        }
    }
    else
    {
        if(h->heaparray[leftIndex] <= h->heaparray[rightIndex])
        {
            minIndex = leftIndex;
        }
        else
        {
            minIndex = rightIndex;
        }
    }

    if(h->heaparray[index] > h->heaparray[minIndex])
    {
        temp = h->heaparray[minIndex];
        h->heaparray[minIndex] = h->heaparray[index];
        h->heaparray[index] = temp;
        siftDown_min(h1, minIndex);
    }
}

void Delete(heap** h1, bool maxflag)
{
    uint32_t hole = 0;
    heap* h = *h1;
    if(h->size == 1)
    {
        h = NULL;
        return;
    }
    else
    {
        hole = --h->size;
        h->heaparray[1] = h->heaparray[hole];
        h->heaparray = (int32_t *)(realloc(h->heaparray , sizeof(int32_t) * h->size));
        if(maxflag)
        {
            siftDown_max(h1, 1);
        }
        else
        {
            siftDown_min(h1, 1);
        }
    }
}

void insert_min(heap** h1, int32_t value)
{
    uint32_t hole_index = 0;
    heap* local_heap = *h1;
    if (local_heap->size == 0)
    {
        local_heap->size = 2;
        local_heap->heaparray = (int32_t*)malloc(sizeof(int32_t) * local_heap->size);
        local_heap->heaparray[0] = 0;
        local_heap->heaparray[1] = value;
        return;
    }
    hole_index = local_heap->size++;
    local_heap->heaparray[0] = value;

    for(; value < local_heap->heaparray[hole_index/2]; hole_index /= 2)
    {
        local_heap->heaparray[hole_index] = local_heap->heaparray[hole_index / 2];
    }

    local_heap->heaparray[hole_index] = value;

}


int main(void)
{

    int hy = 0;
    heap *newheap = (heap *)(malloc(sizeof(heap)));
    newheap->size = 0;
    insert_max(&newheap, 5);
    insert_max(&newheap, 3);
    insert_max(&newheap, 8);
    insert_max(&newheap, 2);
    insert_max(&newheap, 10);
    insert_max(&newheap, 13);
    insert_max(&newheap, 7);
    insert_max(&newheap, 26);
    insert_max(&newheap, 11);
    printheap(newheap);
    Delete(&newheap, true);
    printheap(newheap);
    Delete(&newheap, true);
    printheap(newheap);
    Delete(&newheap, true);
    printheap(newheap);
    Delete(&newheap, true);
    printheap(newheap);
    Delete(&newheap, true);
    printheap(newheap);
    Delete(&newheap, true);
    printheap(newheap);
    Delete(&newheap, true);
    printheap(newheap);
    Delete(&newheap, true);
    printheap(newheap);
    Delete(&newheap, true);
    printheap(newheap);
    insert_max(&newheap, 134);
    printheap(newheap);

    heap *minheap = (heap *)(malloc(sizeof(heap)));
    minheap->size = 0;
    insert_min(&minheap, 5);
    printheap(minheap);
    insert_min(&minheap, 3);
    printheap(minheap);
    insert_min(&minheap, 8);
    printheap(minheap);
    insert_min(&minheap, 2);
    printheap(minheap);
    insert_min(&minheap, 10);
    printheap(minheap);
    insert_min(&minheap, 13);
    printheap(minheap);
    insert_min(&minheap, 7);
    printheap(minheap);
    insert_min(&minheap, 26);
    printheap(minheap);
    insert_min(&minheap, 11);
    printheap(minheap);
    Delete(&minheap, false);
    printheap(minheap);
    Delete(&minheap, false);
    printheap(minheap);
    Delete(&minheap, false);
    printheap(minheap);
    Delete(&minheap, false);
    printheap(minheap);
    Delete(&minheap, false);
    printheap(minheap);
    Delete(&minheap, false);
    printheap(minheap);
    Delete(&minheap, false);
    printheap(minheap);
    Delete(&minheap, false);
    printheap(minheap);
    Delete(&minheap, false);
    printheap(minheap);
    insert_min(&minheap, 138);
    printheap(minheap);
    hy = 3;

    return EXIT_SUCCESS;
}

Answer 1

I've made a Minimal, Complete, and Verifiable example for you, then it was easy to spot a serious bug.

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <stdbool.h>

typedef struct heap
{
    uint32_t size;
    int32_t* heaparray;
    // START DEBUG CODE    
    uint32_t debug_allocated_size;   // contains the actual allocated size
    // END DEBUG CODE
}heap;


void insert_min(heap** h1, int32_t value)
{
    uint32_t hole_index = 0;
    heap* local_heap = *h1;
    if (local_heap->size == 0)
    {
        local_heap->size = 2;
        local_heap->heaparray = (int32_t*)malloc(sizeof(int32_t) * local_heap->size);

        // START DEBUG CODE
        local_heap->debug_allocated_size = local_heap->size;
        // END DEBUG CODE

        local_heap->heaparray[0] = 0;
        local_heap->heaparray[1] = value;
        return;
    }
    hole_index = local_heap->size++;
    local_heap->heaparray[0] = value;

    for(; value < local_heap->heaparray[hole_index/2]; hole_index /= 2)
    {
      // START DEBUG CODE
      if (local_heap->debug_allocated_size >= hole_index)
      {
        // if hole_index is larger than the actuallly allocated size there is a problem...
        printf("argh: buffer overflow\n");
        exit(1);
      }
      // END DEBUG CODE

      local_heap->heaparray[hole_index] = local_heap->heaparray[hole_index / 2];
    }

    local_heap->heaparray[hole_index] = value;
}


int main(void)
{
    heap *minheap = (heap *)(malloc(sizeof(heap)));
    minheap->size = 0;
    insert_min(&minheap, 5);
    insert_min(&minheap, 3);
    insert_min(&minheap, 8);
    insert_min(&minheap, 2);

    return EXIT_SUCCESS;
}

Look at the comments I've added. This should help you to correct the bug.

Disclaimer: there may be more bugs in other parts of your code.

Anticipating your next question: Why did my code work fine un debug mode?

Answer: your program exhibited "undefined behaviour". As soon as you've overwritten memory that doesn't belong to you, you enter the realm of "undefine behaviour" and from then on anything can happen.

Answer 2

You have a latent bug in your use of realloc:

h->heaparray = (int32_t *)(realloc(h->heaparray , sizeof(int32_t) * h->size));

If realloc fails for any reason, it will return NULL. When that happens, your program is going to crash horribly. realloc is just an ugly function that you should be very careful about using.

I don't have the solution to your problem. I do, however, have some general advice about building heaps in particular, and resizeable collection data structures in general.

By reallocating at every insertion and deletion, you've made a heap that has O(n) insert and O(n) removal. You might as well use an unordered array, because the benefit of the heap structure is being overshadowed by the cost of reallocating and copying the memory every time.

If you want to use a dynamic array, you should start with some minimum size, say 16 items, and keep track of the free space in your array. When you reallocate, increase by more than 1. Probably your best bet is to double the size of the array. That way, you amortize the cost of the reallocations. Your insert and removal become O(log n) rather than O(n).

The key is that your heap structure needs a count field that keeps track of the current number of items in the heap:

typedef struct heap
{
    uint32_t size;  /* size of the heap array */
    uint32_t count; /* number of items currently in the heap */
    int32_t* heaparray;
}heap;

So when you insert, you check to see if count == size. If it does, then reallocate to double the size. Always use count (rather than size, as in your current code) in your calculations when you're inserting and removing.

When removing items, only reallocate if size > count*2. That way, you minimize calls to realloc. You might also want a trimToCount function that you can call if you want to minimize the amount of space the heap takes.

Also, reconsider your choice of a 1-based heap. C arrays are 0-based, so it makes sense for the root of the heap to be at index 0. It's trivial to adjust the parent and child calculations so that they work with the 0-based heap. For more on the reasoning here, see https://stackoverflow.com/a/49806133/56778 and http://blog.mischel.com/2016/09/19/but-thats-the-way-weve-always-done-it/.