Why does this code segfault on one machine but run fine on another?

Question

This code segfaults on line 97 (according to gdb) on one machine (Linode) yet runs just fine on a different machine (personal) and I haven't really been able to figure out why. I tried ensuring that the heap was extended properly via sbrk but that still didn't seem to fix the issue. If anyone wouldn't mind explaining what I did wrong, I'd really appreciate it.

`

#define _DEFAULT_SOURCE
#define _BSD_SOURCE

#include <stdio.h>
#include <math.h>

typedef struct block        // This is a node in a linked list representing various sections of memory
{
    int size;               // size of the block of allocated memory
    int free;               // whether the block is free and can be reallocated
    struct block* next;     // pointer to the next block in the linked list
    char end[1];            // end represents the end of the header block struct
} block_t;

#define STRUCT_SIZE sizeof(block_t)

// --- Global variables
block_t* head = NULL;                           // Pointer to head of the linked list
block_t* lastVisited = NULL;                    // Pointer to the last visited node

void* brkPoint = NULL;                         // This is a pointer to the empty space on heap

// findBlock: Iterates through all blocks of memory until it is able to return a block able to contain a node of size size.
//      headptr: Head of the linked list of blocks
//      size: Size of the memory section being claimed
block_t* findBlock(block_t* headptr, unsigned int size) {
    block_t* ptr = headptr;

    while (ptr != NULL) {
        if (ptr->size >= (size + STRUCT_SIZE) && ptr->free == 1) {
            return ptr;
        }
        lastVisited = ptr;
        ptr = ptr->next;
    }
    return ptr;
}


// splitBlock: Given a block ptr, split it into two blocks of size of size and ptr->size - size
//      ptr: Pointer to the block being split
//      size: Size of the first one of the two blocks
void splitBlock(block_t* ptr, unsigned int size) {
    block_t* newBlock;

    newBlock = ptr->end + size;
    newBlock->size = ptr->size - size - STRUCT_SIZE;
    newBlock->free = 1;
    newBlock->next = ptr->next;

    ptr->size = size;
    ptr->free = 0;
    ptr->next = newBlock;
}


// Increase amount of memory the program uses from the heap
//      lastVisitedPtr: Pointer to the beginning of free heap (end of the program heap)
//      size: The amount that you want to increase
block_t* increaseAllocation(block_t* lastVisitedPtr, unsigned int size) {
    brkPoint = sbrk(0);
    block_t* curBreak = brkPoint;       //Current breakpoint of the heap

    if (sbrk(size + STRUCT_SIZE) == (void*)-1) {
        return NULL;
    }

    curBreak->size = (size + STRUCT_SIZE) - STRUCT_SIZE;
    curBreak->free = 0;
    curBreak->next = NULL;
    lastVisitedPtr->next = curBreak;

    if (curBreak->size > size)
        splitBlock(curBreak, size);

    return curBreak;
}

// malloc: A custom implementation of malloc, behaves exactly as expected
//      _size: the amount of memory to be allocated into a block
//      returns void*, a pointer to the block
void* mymalloc(size_t _size) {
    void* brkPoint1;                        // New heap breakpoint
    unsigned int size = _size;
    int memoryNeed =  size + STRUCT_SIZE;   // Total size needed, including metadata
    block_t* ptr;                           // ptr to new block

    brkPoint = sbrk(0);                     // Set breakpoint to heap

    if (head == NULL) {                     // If being run for the first time
        if (sbrk(memoryNeed) == (void*)-1) { // If you cannot allocate enough memory, return null
            return NULL;
        }

        brkPoint1 = sbrk(0);                // Set new breakpoint to heap
        head = brkPoint;                    // Head is at heap
        head->size = memoryNeed - STRUCT_SIZE;
        head->free = 0;                     // Head is no longer free
        head->next = NULL;                  // No next

        ptr = head;                         // Return pointer is head 

        printf("Malloc %zu bytes\n", size);
        return ptr->end;                    // Return end of the metadata pointer (AKA beginning of allocated memory)
    }

    else {                                  // Head exists
        block_t* freeBlock = NULL;          
        freeBlock = findBlock(head, size);  // Find a block that can fit size

        if (freeBlock == NULL) {             
            freeBlock = increaseAllocation(lastVisited, size);  // Increase heap and create new block
            if (freeBlock == NULL) {
                return NULL;
            }
            printf("Malloc %zu bytes\n", size);
            return freeBlock->end;
        }

        else {                              // Free block with size > _size exists, claim it
            if (freeBlock->size > size) {   // If block's size is > size, split it down to size
                splitBlock(freeBlock, size);
            }
        }
        printf("Malloc %zu bytes\n", size);
        return freeBlock->end;
    }
}

// myfree: Sets block referenced by pointer to be free and merges consecutive blocks
void myfree(void* ptr) {

    block_t* toFree;
    toFree = ptr - STRUCT_SIZE;

    if (toFree >= head && toFree <= brkPoint) {
        toFree->free = 1;
        printf("Freed %zu bytes\n", toFree->size);
    }
}


#define ARRAY_ELEMENTS 1024

int main() {

  // Allocate some data
  int *data = (int *) mymalloc(ARRAY_ELEMENTS * sizeof(int));

  // Do something with the data
  int i = 0;
  for (i = 0; i < ARRAY_ELEMENTS; i++) {
    data[i] = i;
  }

  // Free the data
  myfree(data);

  return 0;
}

`

As mentioned above, I tried debugging with gdb and expanding the heap with sbrk, but that didn't fix the issue. I have no idea why it would run fine on my personal machine but not on a machine hosted elsewhere. Thanks a lot for checking this out

Answer 1

There is a ton of warnings which you should fix.

This one in particular is likely to cause crashes:

t.c:61:16: warning: implicit declaration of function ‘sbrk’.

Why is this likely to cause a crash?

Without a prototype, the C compiler is required (by the standard) to assume that the function returns an int.

On 32-bit platforms, this typically doesn't cause a problem because sizeof(int) == sizeof(void*) == 4.

But on 64-bit platforms sizeof(int) == 4 and sizeof(void*) == 8. Thus assigning void *p = sbrk(0); without a prototype may result in the pointer having only the low 4 bytes of the returned address; and that is likely to produce a crash when that pointer is dereferenced.

When I add missing #include <unistd.h> (where the prototype for sbrk is), the crash goes away.

---

In general you should always compile with -Wall -Wextra and fix resulting warnings. The compiler will often tell you about bugs, and save you a lot of debugging time.