multi inheriting a struct from struct and template struct, ordering matters when accessing base non-template struct data

Question

#include <stdlib.h>
#include <string>
#include <atomic>

struct base_C_event {
    const char* ev;
    const char* da;
};

template <class T>
struct ref_counter {

    private:
        std::atomic<std::ptrdiff_t> _counter;
};

struct derived_event : ref_counter<derived_event>, base_C_event {

    derived_event() : event_type(), event_data() {
        ev = event_type.c_str();
        da = event_data.c_str();
    }

    std::string event_type;
    std::string event_data;
};

struct derived_event2 : base_C_event, ref_counter<derived_event2> {
    derived_event2() : event_type(), event_data() {
        ev = event_type.c_str();
        da = event_data.c_str();
    }

    std::string event_type;
    std::string event_data;
};

struct some_cool_event {
    int type;
    void* payload;
};

void OnEvent(const some_cool_event* event) {
    auto e = static_cast<base_C_event*>(event->payload); //...and then shows itself here
    printf("%s - %s\n", e->ev, e->da);
}

int main() {
    derived_event evt;
    evt.event_type = "type";
    evt.event_data = "Hello World";

    derived_event2 evt2;
    evt2.event_type = "hi";
    evt2.event_data = "there";

    some_cool_event my_event;
    my_event.type = 1;
    my_event.payload = &evt; //Problem starts here...
    OnEvent(&my_event);

    my_event.type = 2;
    my_event.payload = &evt2;
    OnEvent(&my_event);

    return 0;
}

output: (compiled with g++)

(null) - type

type - Hello World

now, in my real environment (XCode) the ordering of inheritance for derived_event causes a BADACCESS exception; with g++ it just produces (null) as shown in the output.

however, the ordering for derived_event2 works just fine.

The way i understand the standard, the order of multiple inheritance effects the order of constructors and destructors, and also the layout of the memory. Can anyone explain what is happening here?

EDIT: I have actually figured this out. The line that sets the event object to the void* payload, and then the ensuing static_cast<> back to the base type... seems to invalidate the first pointer (ev) because the struct becomes just a memory layout at that point, so the pointers are getting set to the first two pointer size chunks... in this case std::atomic<std::ptrdiff_t> and then the base_C_event. so the cast is grabbing the data for the std::atomic and using that as the pointer address for ev, and what was originally ev in the derived object is now what da points at.

I unfortunately in my real scenario can't use composition for the base_C_event in my derived_event and send that. that's why the refcounting is there, so i have to send the derived object so that later on in a callback i can decrement the refcount.

Is there a way to prevent this from happening?

Answer 1

If you cast a pointer to void * then always do the exact inverse cast when casting back to actual type.

So if you have :

D *d = new D;
void *v = d;  // Here D* is casted to void *

When you get back the pointer, use the inverse cast. The following example are correct:

D *d2 = static_cast<D *>(v);
A *a2 = static_cast<D *>(v);
B<D> *b2 = static_cast<D *>(v);

Even better, if you can, try to avoid using void *. It can easily lead to hard-to-find bugs and this is even worst when using multiple inheritance.

If you have to use void *, then try do to it as locally as possible in the code so that ideally the conversion is done exactly at one place in the code for each direction.

class VoidMember
{
public:
    void set(D *d) { v = d; }
    D *get() { return static_cast<D *>(v);

private:
    // In reality, you would not store a void like that but assume this is stored in 
    // a library / API that use `void *`
    void *v;
};

While casting to other types might sometime works, it should be avoided as it make the code more fragile if code is refactored at some point like reordering base classes.

Answer 2

Hm, I think I see where the problem lies:

struct D : B<D>, A { };

This way you inherit both a B<D> and a A instance. Effectively, this ressembles something like this:

struct D
{
    B<D> implicitly_inherited_B_D;
    A    implicitly_inherited_A;
};

You now do the following:

D* d    = new D();
void* v = d;
A* a    = static_cast<A*>(v);

Problem is: v now points to the D instance, which shares its address with the inherited B<D> instance. But you cast the pointer back to A*, however, Ds A has an offset. So what you do corresponds to:

D* d    = new D();
void* v = &d->implicitly_inherited_B_D;
A* a    = static_cast<A*>(v);
// or equivalent:
A* aa   = reinterpret_cast<A*>(&d->implicitly_inherited_B_D);

This is bound to fail...

If you want to cast back to A*, you need to make sure that your pointer actually points to the inherited A within D - which is quite easy:

D* d    = new D();
void* v = static_cast<A*>(d);
// now this will work fine (v points to D's A part):
A* a    = static_cast<A*>(v);
D* dd   = static_cast<D*>(a); // even this one, original object was constructed as D

For comparison:

D* d  = new D();
A* a  = d;
D* ds = static_cast<D*>(a);
D* dr = reinterpret_cast<D*>(a); // actually undefined behaviour!!!

std::cout << d << std::endl << a << std::endl << ds << std::endl << dr << std::endl;

Assuming address of d is 0x10001000 and A within D has an offset of 8(sizeof(B<D> + possibly fill bytes for alignment), you'd see an output like this:

10001000
10001008
10001000
10001008

Note that the last line originates from the D* pointer received via reinterpret_cast!

Final note: Be aware that members can be rearranged - members declared first preceding members declared afterwards only is guaranteed for members within the same accessibility class (public/protected/private), between these sections, compiler is allowed to re-arrange. So in general you only can be safe if you go back from void* the same way you used for getting there:

void* v = d; // -> need to go back via static_cast<D*>!

A* a    = static_cast<A*>(v);    // requires v = static_cast<A*>(d);
B<D>* d = static_cast<B<D>*>(v); // requires v = static_cast<B<D>*>(d);

Anything else is undefined behaviour (be aware that the matter gets even worse as soon as virtual classes are involved, as then additionally there are the vtable pointers...).