Why does this C code generate a segmentation fault on macOS, but not on other systems?

Question

While trying to implement a doubly linked list in C, I noticed that the following snippet would throw a segmentation fault on macOS 10.11 El Capitan. However, when testing in Linux or Haiku, it would run happily, generating the expected results.

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

typedef struct node_structure {
    int data;
    struct node_structure *prev;
    struct node_structure *next;
} *node;

node createNode(int value) {
    node newNode = (node) malloc(sizeof(node));
    if (newNode != NULL) {
        newNode->data = value;
        newNode->prev = NULL;
        newNode->next = NULL;
    }
    return newNode;
}

void displayLinkedList(node linked_list) {
    node cursor = linked_list;
    while (cursor != NULL) {
        printf("DATA: %d \tTHIS:%p \tPREV:%p \tNEXT:%p\n", cursor->data, (void*)cursor, (void *)cursor->prev, (void *)cursor->next);
        cursor=cursor->next;
    }
}

int insertAtHead(node *head, int value) {
    node newHead = createNode(value);
    if(newHead != NULL) {
        (*head)->prev = newHead;
        newHead->next = *head;
        *head = newHead;
        return 0;
    }
    else return 1;
}

int main() {
    printf("\nCreating a single element linked list.\n");
    node head = createNode(10);
    displayLinkedList(head);

    printf("\nInserting 10 elements at head.\n");
    for(int i = 0; i < 10; i++) { 
        insertAtHead(&head, 8); 
    }
    displayLinkedList(head);
    return 0;
}

This is the console output:

$ gcc --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 8.0.0 (clang-800.0.42.1)
Target: x86_64-apple-darwin15.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

$ gcc -Wall -pedantic 04_doubly_linked_lists__debugging.c

$ ./a.out

Creating a single element linked list.
DATA: 10        THIS:0x7fd19a403390     PREV:0x0        NEXT:0x0

Inserting 10 elements at head.
DATA: 8         THIS:0x7fd19a403430     PREV:0x0        NEXT:0x7fd19a403420
DATA: 8         THIS:0x7fd19a403420     PREV:0x7fd19a403430     NEXT:0x7fd100000008
Segmentation fault: 11

As you can see, in the last iteration before the crash, the next pointer seems to be overwritten by the value what goes into the data field of the struct (in this example, an integer with value 8).

What makes this especially weird is that the same code runs without any trouble in other operating systems, completing the 10 elements insertion loop and the correct display of all elements and respective memory addresses to the screen.

Am I doing anything wrong here?

Answer 1

You've declared node to be a pointer type, so malloc(sizeof node) allocates enough memory for a pointer, not enough for the structure. If it ever worked, it was purely accidental.

A good habit to get into when allocating pointers is to always use the form:

fooptr = malloc(sizeof *fooptr);

so that you can see at a glance that you are allocating the size of the thing your pointer points to, and you can even change the type without changing that code.

Answer 2

The problem is this:

node newNode = (node) malloc(sizeof(node));

If you don't want to modify anything else, you could correct it with this:

node newNode = (node) malloc(sizeof(*node));

However, there are a few things I'd like to address in your code. First, don't cast malloc as it is completely unnecessary, unless you for some reason are using a C++ compiler.

Second, it's much better to put the variable rather than the type as argument to malloc, because it avoids code duplication. In this case it would also have solved your bug. With these two things, you could write like this:

node newNode = malloc(sizeof(*newNode));

Thirdly, there's absolutely no reason at all to use different names for node_structure and node. Write like this instead:

typedef struct node {
    int data;
    struct node *prev;
    struct node *next;
} *node;

Fourthly, you can use typedefs to hide structures and pointers with descent (some people argue about hiding pointers this way) validity when you are creating the interface to a library, but do not use them in the code actually manipulating them. Your create should look like this:

struct node *createNode(int value) {
    struct node *newNode = malloc(sizeof(*newNode));
    // Same as before in the rest

What makes this especially weird is that the same code runs without any trouble in other operating systems

That's not weird. It's an almost 100% certain sign that your code has undefined behavior.