Reputation: 2051
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
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
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