Nasty bug, please help, unexplained assignment to a region of memory

Z

Zygmunt Krynicki

I've spent the whole day at the debugger narrowing the problem to a piece
of code (it's reproducable). I would please ask for some help from someone
who is better at the debugger than I am. It might be some trivial error
overseen due to long times at work but I'm really giving up this time:

To anyone concerned about standarts: this code uses a gnu excension
that may be removed without any difference, namely:
__attribute__((noreturn))

Also I am aware that leading underscores are reserved for the
implementation and thus should not be used in normal programs. Somehow
though I cannot resist in using them frome time to time ;-)

In any case, thank you for spending your time reading this.

(just one function, see below for the rest)
void
__register_exception (
struct exception_context *context,
exception_ctor ctor,
exception_dtor dtor,
size_t size,
size_t id,
char *name
)
{
struct exception *exception;
if (context->capacity < id) {
context->capacity = id + 1;
context->exception = realloc (context->exception, context->capacity * sizeof (struct exception));
}
/*
the following line seems to be causing the problem
after it gets executed another region of memory (definitly not the one
pointed by exception) is assigned with some 'garbage' that gets produced
out of the value od id. SIGSEV follows shortly.
*/
exception = context->exception + id;
exception->context = context;
exception->ctor = ctor;
exception->dtor = dtor;
exception->size = size;
exception->id = id;
exception->name = name;
}

The full [reduced] code is over 350 lines long but maybe someone wouldn't
mind to have a look; here it goes:

/* exception.h */

#ifndef FCQP_EXCEPTION_H
#define FCQP_EXCEPTION_H

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

/* exception constructor */
typedef void (*exception_ctor)(void *ptr, va_list ap);

/* exception destructor */
typedef void (*exception_dtor)(void *ptr);

/* forward declaration of exception context */
struct exception_context;

struct exception
{
/* context pointer */
struct exception_context *context;
/* constructor, destructor */
exception_ctor ctor;
exception_dtor dtor;
/* data size */
size_t size;
/* id of this exception */
size_t id;
/* name */
char *name;
};

struct exception_context
{
/* array of exceptions */
struct exception *exception;
/* size of the array */
size_t capacity;
};

struct jp_list {
jmp_buf buf;
struct jp_list *prev;
};

struct exception_global_data
{
/* stack, see below */
char *base;
char *stack;
/* uncought exception handler */
void (*uncought_handler)(void);
/* jump point list */
struct jp_list *jp_list;
};

/* global variable */
extern struct exception_global_data __exception_global_data;

/* shortcuts */
#define __egd_s __exception_global_data.stack
#define __egd_b __exception_global_data.base
#define __egd_uh __exception_global_data.uncought_handler
#define __egd_jp __exception_global_data.jp_list

/* ------------------------------------------------------------ */
/* public functions */

/* initialize global exception data, assign stack space */
void
initialize_exceptions (void *stack, size_t size);

/* set uncought exception handler */
void
set_uncought_exception_handler (void (*handler)(void));

/* get data owned by the current exception */
void *
get_exception_data(void);

/* ------------------------------------------------------------ */
/* internal functions */

/* default uncought exception handler */
void
__def_uncought_exception_handler(void);

/* register exception in a given context */
void
__register_exception (struct exception_context *context, exception_ctor ctor,
exception_dtor dtor, size_t size, size_t id, char *name);

/* throw an exception */
void
__throw (struct exception *exception, ...) __attribute__((noreturn));

/* rethrow current exception */
void
__rethrow (void) __attribute__((noreturn));

/* clean up after the last exception */
int
__cleanup (void);

/* ------------------------------------------------------------ */

/* exception context macros */
#define EXCEPTION_CONTEXT(context) struct exception_context context##_exception_context; void context##_init_context(void)

#define CONTEXT_CTOR(context) void context##_init_context(void)

#define init_context(context) context##_init_context()

/* helper statement, sets target context of following register_exception's */
#define target_context(context) struct exception_context *__target_context = & context##_exception_context;

/* exception registration macros */
#define register_exception(name) __register_exception (__target_context, (exception_ctor) name##_ctor, (exception_dtor) name##_dtor, sizeof (struct name##_exception), name##_exception_id, #name)

/* exception definition */
#define EXCEPTION_DATA(name) struct name##_exception
#define EXCEPTION_CTOR(name) void name##_ctor(struct name##_exception *EXCEPTION, va_list ARG)
#define EXCEPTION_DTOR(name) void name##_dtor(struct name##_exception *EXCEPTION)

/* exception declaration */
#define EXCEPTION_PROTOTYPE(name, value) enum name##_EXCEPTION_ID { name##_exception_id = value };\
EXCEPTION_DATA(name);\
EXCEPTION_CTOR(name);\
EXCEPTION_DTOR(name);\
EXCEPTION_DATA(name)

/* throwing exceptions */
#define throw(context, name, ...) __throw (&context##_exception_context.exception[name##_exception_id], ## __VA_ARGS__)

/* rethrowing exceptions (from catch blocks) */
#define rethrow do { __propagate = 1; break; } while (0)

/* the try block */
#define try do {\
struct jp_list __jp;\
int __id;\
int __propagate = 0;\
struct exception_context *__context = NULL;\
__jp.prev = __egd_jp;\
__egd_jp = & __jp;\
if ((__id = setjmp (__jp.buf))) {\
__context = (*(struct exception**)__egd_s)->context;\
__propagate = 1;\
} else {
/* specific context selector */
#define context(context) }\
if (__context == & context##_exception_context )\
switch (__id) {\
case 0: {\
/* default context selector */
#define known_context }\
switch (__id) {\
case 0: {\
/* the catch block */
#define catch(name) } break;\
case name##_exception_id: __propagate = 0; {\
/* the catch_as block */
#define catch_as(name, var) } break;\
case name##_exception_id: __propagate = 0; {\
struct name##_exception * var = get_exception_data();
/* the intercept block = catch + rethrow */
#define intercept(name) } break;\
case name##_exception_id:\
/* the intercept_as block */
#define intercept_as(name, var) } break;\
case name##_exception_id: {\
struct name##_exception * var = get_exception_data();
/* the catch_rest block, acts like catch (...) in C++ */
#define catch_rest } break;\
default: __propagate = 0; {\
/* just a little help */
#define done }
/* finally block, code cleanup goes here */
#define finally } {
/* untry block, rethrows or cleanups after exception */
#define untry }\
if (__propagate) __rethrow();\
if (__id) __cleanup(); else __egd_jp = __egd_jp->prev;\
} while (0)

#endif


/* exception.c */

#include "exception.h"

struct exception_global_data __exception_global_data = { 0 };

/* ------------------------------------------------------------ */
/* public functions */

void
initialize_exceptions (void *stack, size_t size)
{
__egd_b = __egd_s = stack + size;
__egd_uh = __def_uncought_exception_handler;
__egd_jp = NULL;
}

void
set_uncought_exception_handler (void (*handler)(void))
{
__egd_uh = handler;
}

void *get_exception_data(void)
{
return __egd_s == __egd_b ? NULL : __egd_s + sizeof (void*);
}

/* ------------------------------------------------------------ */
/* internal functions */

void
__def_uncought_exception_handler(void)
{
do {
fprintf (stderr, "Uncought exception: %s\n",
(*(struct exception**)(__egd_s))->name
);
} while (__cleanup());
abort ();
}

/* register exception in a given context */
void
__register_exception (struct exception_context *context, exception_ctor ctor, exception_dtor dtor, size_t size, size_t id, char *name)
{
struct exception *exception;
if (context->capacity < id) {
context->capacity = id + 1;
context->exception = realloc (context->exception, context->capacity * sizeof (struct exception));
}
exception = context->exception + id;
exception->context = context;
exception->ctor = ctor;
exception->dtor = dtor;
exception->size = size;
exception->id = id;
exception->name = name;
}

void __throw (struct exception *exception, ...)
{
struct jp_list *jp;
va_list ap;
printf ("Constructor: %p\n", exception->ctor);
if (__egd_jp == NULL)
__egd_uh();
/* get jump coordinates */
jp = __egd_jp;
__egd_jp = __egd_jp->prev;
/* make room for data */
__egd_s -= exception->size;
/* call constructor */
va_start (ap, exception);
exception->ctor (__egd_s, ap);
va_end (ap);
/* store exception pointer */
__egd_s -= sizeof (void*);
*(void**)__egd_s = exception;
/* jump */
longjmp (jp->buf, exception->id);
}

void __rethrow (void)
{
struct jp_list *jp;
if (__egd_jp == NULL)
__egd_uh();
/* get jump coordinates */
jp = __egd_jp;
__egd_jp = __egd_jp->prev;
/* jump */
longjmp (jp->buf, ((struct exception*)(__egd_s))->id);
}

int __cleanup(void)
{
struct exception *exception;
if (__egd_s < __egd_b) {
/* pop exception */
exception = *(struct exception **)__egd_s;
__egd_s += sizeof (void*);
/* call destructor */
exception->dtor (__egd_s);
/* pop exception data */
__egd_s += exception->size;
}
return __egd_s < __egd_b;
}

/* err.c <- this is the minimal reproduction of the problem that I've
come up with */

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

#include "exception.h"

EXCEPTION_CONTEXT(other_lib);
/* '1' has something to do with why segfaults occur */
EXCEPTION_PROTOTYPE(froz, 1) { }; /* exception's data was here - snipped */

EXCEPTION_CONTEXT(my_lib);
/* note the exact same id of froz and foo (they will be registered in different context so that's ok */
EXCEPTION_PROTOTYPE(foo, 1) { }; /* as above */
EXCEPTION_PROTOTYPE(bar, 2) { };

EXCEPTION_CTOR(foo) { }
EXCEPTION_DTOR(foo) { }
EXCEPTION_CTOR(bar) { }
EXCEPTION_DTOR(bar) { }

/* initialization macros */
CONTEXT_CTOR(my_lib)
{
target_context (my_lib);
register_exception (foo);
register_exception (bar);
}

EXCEPTION_CTOR(froz) { }
EXCEPTION_DTOR(froz) { }

/* initialization macros */
CONTEXT_CTOR(other_lib)
{
target_context (other_lib);
register_exception (froz);
}

int main(void)
{
char stack[1024];
initialize_exceptions (stack, sizeof(stack));
init_context(my_lib);
init_context(other_lib);
try {
throw (my_lib, bar);
} context (my_lib) {
catch (bar) {
printf ("cought bar\n");
} done;
} untry;
return EXIT_SUCCESS;
}
 
E

Eric Sosman

Zygmunt said:
I've spent the whole day at the debugger narrowing the problem to a piece
of code (it's reproducable). [...]
struct exception *exception;
if (context->capacity < id) {

The operator should be `<=', not `<'.
context->capacity = id + 1;
context->exception = realloc (context->exception, context->capacity * sizeof (struct exception));

Dangerous on two counts: First, if realloc() returns NULL
you've lost your only pointer to what `context->exception'
formerly pointed to. Second, if realloc() returns NULL you'll
go right ahead and try to dereference it (which will probably
produce a crash, so maybe the memory leak won't last for
very long ...).

Also, `sizeof *context->exception' is recommended as better
practice than `sizeof(struct exception)'.
 
Z

Zygmunt Krynicki

The operator should be `<=', not `<'.

True, thank you.
Dangerous on two counts: First, if realloc() returns NULL
you've lost your only pointer to what `context->exception'
formerly pointed to. Second, if realloc() returns NULL you'll
go right ahead and try to dereference it (which will probably
produce a crash, so maybe the memory leak won't last for
very long ...).

Not important in this case yet very true. realloc was really something
like my_realloc that detects memory allocation failures and kills the
application. I'm not entirely sure that's a very good solution but I
didn't seem to have found a better one.
Also, `sizeof *context->exception' is recommended as better
practice than `sizeof(struct exception)'.

I'm very new to this use of the sizeof operator, a week before I thought
it could olny be used like sizeof(type).

Z.
 
B

Bertrand Mollinier Toublet

Zygmunt said:
In any case, thank you for spending your time reading this.

(just one function, see below for the rest)
void
__register_exception (
struct exception_context *context,
exception_ctor ctor,
exception_dtor dtor,
size_t size,
size_t id,
char *name
)
{
struct exception *exception;
if (context->capacity < id) {
context->capacity = id + 1;
context->exception = realloc (context->exception, context->capacity * sizeof (struct exception));

That's a mistake. realloc may fail. If it does, you need to check for
it. The clc approved way to go about it is:
void *tmp;
tmp = realloc(
context->exception,
context->capacity * sizeof *exception);
if (NULL == tmp)
{
/* deal with the memory shortage... */
}
else
{
context->exception = tmp;
}
}
/*
the following line seems to be causing the problem
after it gets executed another region of memory (definitly not the one
pointed by exception) is assigned with some 'garbage' that gets produced
out of the value od id. SIGSEV follows shortly.
*/
exception = context->exception + id;
exception->context = context;
exception->ctor = ctor;
exception->dtor = dtor;
exception->size = size;
exception->id = id;
exception->name = name;
}

The full [reduced] code is over 350 lines long but maybe someone wouldn't
mind to have a look; here it goes:
I would, though...

--
Bertrand Mollinier Toublet
"In regard to Ducatis vs. women, it has been said: 'One is a sexy thing
that you've just got to ride, even if it breaks down a lot, costs a lot
of money, and will probably try to kill you'. However, nowadays I can't
seem to remember which one is which." -- Peer Landa
 
A

Arthur J. O'Dwyer

That's a mistake. realloc may fail. If it does, you need to check for
it. The clc approved way to go about it is:
void *tmp;
tmp = realloc(
context->exception,
context->capacity * sizeof *exception);

Whoops! You almost certainly really meant

context->capacity * sizeof *context->exception);

While your code does the same thing as the OP's (since '*exception'
is of type 'struct exception'), I'm pretty sure the OP meant to
compute the size of a single element of the array to whose first
member 'context->exception' points.
If 'context->exception' is a pointer to void, then of course
I'm wrong and you're closer to being right. :) But we see from
other parts of the code that 'context->exception' is not a pointer
to void, since it can be an operand to the binary + operator.
if (NULL == tmp)
{
/* deal with the memory shortage... */
}
else
{
context->exception = tmp;
}

My best guess is that realloc is returning NULL, and then you're trying
to immediately add 'id' to NULL and dereference the resulting value.
Since that's invalid, the program crashes.

Are you *sure* your 'realloc' function can *never* return NULL?

HTH,
-Arthur
 
Z

Zygmunt Krynicki

My best guess is that realloc is returning NULL, and then you're trying
to immediately add 'id' to NULL and dereference the resulting value.
Since that's invalid, the program crashes.

Accualy the very first answer was correct, it was a problem of off_by_one
memory alocation (I wanted <= instead of <) and now everything works fine.
I found another very serious error and fixed it, if anyone is interested
in the final version feel free to send me an email.

void __rethrow (void)
{
[snip first few lines]
longjmp (jp->buf, (*(struct exception**)(__egd_s))->id);
}

previously this was:

longjmp (jp->buf, (struct exception*)(__egd_s))->id);

which was obviously wrong, this has caused all rethrown exceptions
to loose their identity.

Are you *sure* your 'realloc' function can *never* return NULL?

[explained above, this was not a NULL pointer error]


I'd like to thank you all for your quick and insightful answers.
You've all helped me alot. :)

Z.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,755
Messages
2,569,536
Members
45,012
Latest member
RoxanneDzm

Latest Threads

Top