haneefmubarak
haneefmubarak

Reputation: 2051

Segmentation Fault in call to free()

I have a test case that keeps throwing a segmentation fault. When I used gdb to try and find where it was segfaulting, I found that it failed in a call to free() in the code being tested.

Now the thing is, the parameter being passed to free() was allocated using malloc() and wasn't adjusted after that (AFAICT).

Here's the relevant code:

structure.h:

#pragma once

//===   Includes

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

#include "kv/backend.h"
#include "xm.h"
#include "constants.h"

//===   Defines

#define STRUCTURE_TYPE_NULL (0x00)  // invalid - we should never see this
#define STRUCTURE_TYPE_SUB  (0x01)  // substructure
#define STRUCTURE_TYPE_I64  (0x02)  // 64b signed integer
#define STRUCTURE_TYPE_U64  (0x03)  // 64b unsigned integer
#define STRUCTURE_TYPE_H64  (0x04)  // 64b translate to hex (ie: 0x'...')
#define STRUCTURE_TYPE_F64  (0x05)  // 64b IEEE 754 double float
#define STRUCTURE_TYPE_BLOB (0x06)  // variable length binary blob
#define STRUCTURE_TYPE_STRING   (0x07)  // variable length null terminated string
#define STRUCTURE_TYPE_TIME (0x08)  // 64b packed YMDhms time
#define STRUCTURE_TYPE_UNIXTIME (0x09)  // 64b signed unix time

//===   Structures

typedef struct {
    int64_t     year    :35;
    uint64_t    month   :4;
    uint64_t    day :5;
    uint64_t    hour    :5;
    uint64_t    minute  :6;
    uint64_t    second  :6;
} structure_time;

typedef struct structure_struct {
    uint8_t *key;
    union {
        int64_t         i64;
        uint64_t        u64;
        uint64_t        h64;
        double          f64;
        uint8_t         *blob;
        uint8_t         *string;
        structure_time      time;
        int64_t         unixtime;
        struct structure_struct *children;
    };
    union {
        uint16_t    len;    // blob + string (includes NULL terminator)
        uint16_t    count;  // children
    };
    uint8_t     type;
} structure;

//===   Special

static inline int cmp_structure (const void *arg1, const void *arg2) {
    const structure *a = arg1;
    const structure *b = arg2;
    return strcmp (a->key, b->key); // compare keys
}

#define SORT_NAME structure
#define SORT_TYPE structure
#define SORT_CMP(x, y)  cmp_structure (&x, &y)
#include "../deps/sort/sort.h"

//===   Functions

static inline void StructureSortChildren (structure *s) {
    structure_tim_sort (s->children, s->count);
    return;
}

int StructureAddChildren    (structure *parent, const structure *children, int count);
void StructureFree      (structure *s);

structure.c:

#include "structure.h"

int StructureAddChildren (structure *parent, const structure *children, int count) {
    if (parent->type != STRUCTURE_TYPE_SUB)
        return 1;   // yeah lets not cause a memory issue

    // realloc () may lose data
    structure *tmp = malloc ((parent->count + count) * sizeof (structure *));
    if (!tmp)
        return -1;  // memory failure

    // copy over the old children
    memcpy (tmp, parent->children, parent->count * sizeof (structure));
    if (parent->count > 0)
        free (parent->children);        // segfault occurs here
    parent->children = tmp;

    // copy over the new children
    memcpy (&parent->children[parent->count], children, count * sizeof (structure));
    parent->count += count;

    // resort the array to allow bsearch() to find members
    structure_tim_sort (parent->children, parent->count);

    return 0;
}

test_structure.c:

#include "test.h"
#include "../structure.h"

const char *key[4] = { "head", "number", "integer", "string" };
const char *text = "text";

void printstructure (const structure *s) {
    switch (s->type) {
        case STRUCTURE_TYPE_SUB: {
            printf ("Structure:\n"  \
                "\tType:\t%s\n" \
                "\tKey:\t%s\n"  \
                "\tnumber Count:\t%i\n\n",
                "(Sub)Structure", s->key, s->count);
            break;
        }

        case STRUCTURE_TYPE_STRING: {
//          assert (s->len == (strlen (s->string) + 1));    // NULL terminator
            printf ("Structure:\n"  \
                "\tType:\t%s\n" \
                "\tKey:\t%s\n"  \
                "\tValue:\t%s\n" \
                "\tLength:\t%i\n\n",
                "String", s->key, s->string, s->len);
            break;
        }

        case STRUCTURE_TYPE_I64: {
            printf ("Structure:\n"  \
                "\tType:\t%s\n" \
                "\tKey:\t%s\n"  \
                "\tValue:\t%i\n\n",
                "64-Bit Signed Integer", s->key, (int) s->i64);
            break;
        }

        case STRUCTURE_TYPE_F64: {
            printf ("Structure:\n"  \
                "\tType:\t%s\n" \
                "\tKey:\t%s\n"  \
                "\tValue:\t%f\n\n",
                "64-Bit FP Number", s->key, (float) s->f64);
            break;
        }
    }

    return;
}

void test (void) {
    structure *head, *number, *integer, *string;

    // basic allocations
    head = malloc (sizeof (structure));
    head->key = strdup (key[0]);
    head->type = STRUCTURE_TYPE_SUB;
    head->count = 0;

    number = malloc (sizeof (structure));
    number->key = strdup (key[1]);
    number->type = STRUCTURE_TYPE_F64;
    number->f64 = 3.1415;

    integer = malloc (sizeof (structure));
    integer->key = strdup (key[2]);
    integer->type = STRUCTURE_TYPE_I64;
    integer->i64 = -42;

    string = malloc (sizeof (structure));
    string->key = strdup (key[3]);
    string->type = STRUCTURE_TYPE_STRING;
    string->string = strdup (text);
    string->len = strlen (text) + 1;    // NULL terminator

    // lets see the current states
    printf ("\n---Structures Built---\n\n");
    printstructure (head);
    printstructure (number);
    printstructure (integer);
    printstructure (string);

    // lets put them together
    // head +>  number
    //  ->  integer
    //  ->  string
    StructureAddChildren (head, integer, 1);
    StructureAddChildren (head, string, 1);
    StructureAddChildren (head, number, 1);     // the SEGFAULT occurs on this call

    ...
}

int main (int argc, char *argv) {
    test ();
    return 0;
}

Now, the SEGFAULT occurs on the third invocation of StructureAddChildren(). Specifically, on the line

StructureAddChildren (head, number, 1); // the SEGFAULT occurs on this call

from test_structure.c, once StructureAddChildren is called, the SEGFAULT occurs on the line

free (parent->children); // segfault occurs here

from structure.c.

I can't imagine what is causing the SEGFAULT, since StructureAddChildren() is the only point at which memory is being allocated and nothing else messes with that pointer (writing to it).


So, overall, what is causing the SEGFAULT and how can I resolve it?

Upvotes: 0

Views: 705

Answers (2)

Dmitri
Dmitri

Reputation: 9375

When allocating memory for the new (and old) structures in StructureAddChildren(), you've calculated the new size using sizeof(structure *) when it should be sizeof(structure). As a result, insufficient space is allocated and later writes overrun the allocated space. The corrected line should be:

structure *tmp = malloc ((parent->count + count) * sizeof (structure));

Upvotes: 4

Eric Tsui
Eric Tsui

Reputation: 1924

The size of malloc in StructureAddChildren function is probably wrong, please try to use sizeof(structure) to replace original sizeof(structure*)

// realloc () may lose data
structure *tmp = malloc ((parent->count + count) * sizeof (structure));//!!!
if (!tmp)
    return -1;  // memory failure

Upvotes: 2

Related Questions