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

Discussion in 'C Programming' started by Zygmunt Krynicki, Sep 19, 2003.

  1. 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;
    }
    Zygmunt Krynicki, Sep 19, 2003
    #1
    1. Advertising

  2. Zygmunt Krynicki

    Eric Sosman Guest

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

    --
    Eric Sosman, Sep 19, 2003
    #2
    1. Advertising

  3. 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.
    Zygmunt Krynicki, Sep 19, 2003
    #3
  4. Re: Nasty bug, please help, unexplained assignment to a region ofmemory

    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
    Bertrand Mollinier Toublet, Sep 19, 2003
    #4
  5. 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
    Arthur J. O'Dwyer, Sep 19, 2003
    #5
  6. 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.
    Zygmunt Krynicki, Sep 19, 2003
    #6
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Dejan Vesic
    Replies:
    1
    Views:
    656
    Chee Seong Ong
    Jul 3, 2003
  2. Larry Barowski
    Replies:
    2
    Views:
    689
    Tor Iver Wilhelmsen
    Sep 11, 2004
  3. bugbear
    Replies:
    1
    Views:
    445
    bugbear
    Jul 13, 2005
  4. SAL

    #Region #End Region issue

    SAL, Aug 29, 2008, in forum: ASP .Net
    Replies:
    1
    Views:
    343
    Alexey Smirnov
    Aug 29, 2008
  5. PCool
    Replies:
    7
    Views:
    641
    gwowen
    Oct 8, 2009
Loading...

Share This Page