0

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?

6
  • structure *tmp = malloc ((parent->count + count) * sizeof (structure *)); should probably be structure *tmp = malloc ((parent->count + count) * sizeof (structure));, since you're allocating space for structures rather than pointers to them. Commented Jul 15, 2015 at 3:33
  • Very minor nit: string literals separated only by whitespace are concatenated even without line-splicing - you don't need the backslashes. Commented Jul 15, 2015 at 3:35
  • @Dmitri I can't believe I missed that - that was it! Please put that in an answer so I can mark it correct. Commented Jul 15, 2015 at 3:35
  • @mlp noted. I will remember and use that in the future - thanks for the heads up! Commented Jul 15, 2015 at 3:36
  • Bigger nit: arguments passed to printf() must match the format string or you reap Undefined Behaviour. It seems unlikely that uint16_t (s->len and s->count) is the same as int (%i) on a platform that also supports int64_t. And you should use an unsigned format for an unsigned argument, too. Commented Jul 15, 2015 at 3:39

2 Answers 2

4

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)); 
Sign up to request clarification or add additional context in comments.

Comments

2

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 

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.