Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Nasty bug, please help, unexplained assignment to a region of memory

Reply
Thread Tools

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

 
 
Zygmunt Krynicki
Guest
Posts: n/a
 
      09-19-2003
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;
}



 
Reply With Quote
 
 
 
 
Eric Sosman
Guest
Posts: n/a
 
      09-19-2003
Zygmunt Krynicki wrote:
>
> 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)'.

--
http://www.velocityreviews.com/forums/(E-Mail Removed)
 
Reply With Quote
 
 
 
 
Zygmunt Krynicki
Guest
Posts: n/a
 
      09-19-2003
On Fri, 19 Sep 2003 16:07:59 -0400, Eric Sosman wrote:

>> if (context->capacity < id) {

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


True, thank you.

>
>> 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 ...).


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.


 
Reply With Quote
 
Bertrand Mollinier Toublet
Guest
Posts: n/a
 
      09-19-2003
Zygmunt Krynicki wrote:
> 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

 
Reply With Quote
 
Arthur J. O'Dwyer
Guest
Posts: n/a
 
      09-19-2003

On Fri, 19 Sep 2003, Bertrand Mollinier Toublet wrote:
>
> Zygmunt Krynicki wrote:
> >
> > 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);


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;
> }


> > /*
> > 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;


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

 
Reply With Quote
 
Zygmunt Krynicki
Guest
Posts: n/a
 
      09-19-2003
On Fri, 19 Sep 2003 17:12:39 -0400, Arthur J. O'Dwyer wrote:

>
> On Fri, 19 Sep 2003, Bertrand Mollinier Toublet wrote:


> 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]

> -Arthur



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

Z.

 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
A nasty memory alignment bug.. PCool C++ 7 10-08-2009 07:16 AM
UK only, Yorkshire region - PC problems, onsite support for Yorkshire region. Need PC upgrading or a new one cheap? Computek Ltd Computer Support 7 02-22-2005 02:38 AM
Convert Region 2 DVD to Region 1? Nsagio Computer Support 4 08-30-2004 02:18 PM
Converting Region 1 to Region 2 Coop DVD Video 0 09-27-2003 10:19 PM
Buffy Region 1 vs. Region 2/4 Anthrax442 DVD Video 1 09-14-2003 04:05 PM



Advertisments