Somewhat worrying casting of function pointers

[ghost]

Hi,

While porting the latest version of Chipmunk to D, I've been hitting segfaults. The bottom-line issue was that a function taking one parameter was being casted to one taking two parameters:

cpBodyActivate(cpBody *body);
cpSpaceEachBody(space, (cpSpaceBodyIteratorFunc)cpBodyActivate, NULL);

cpSpaceEachBody ends up calling cpBodyActivate with:

cpBodyActivate(body, NULL);

Because of the C calling convention body ends up being pushed on the stack last (after NULL), and hence cpBodyActivate has no problem working with this. Also, the caller (not the callee) cleans up the stack, so there's no stack corruption.

But when I've ported this code over to D, which uses its own calling convention, cpBodyActivate would receive NULL rather than the value of body. This would then lead to a crash when body was accessed.

Anyway, I'm just wondering whether this is something you're aware of and are just taking advantage of the way the C calling convention works, or if it's an oversight and would maybe prefer to use safer code as a workaround?

Cheers.

[ghost]

There's also the following case where a bool return function is casted to a void return one:

cpHashSetEach(tree->leaves, (cpHashSetIteratorFunc)LeafUpdate, tree);

The two functions are typed as:

typedef void (*cpHashSetIteratorFunc)(void *elt, void *data);
static cpBool LeafUpdate(Node *leaf, cpBBTree *tree)

Function return values are returned in the EAX register (edit: In the C calling convention), but if the compiler sees the function is typed as a void return when calling it, it might assume EAX is free to use for something other than a return value and could potentially use it for something else (e.g. scratch space for another stack variable). Then LeafUpdate above ends up writing to EAX and it might potentially overwrite data.

I'm just hypothesizing here though, but I think these casts are really unsafe.

[slembcke]

Hey Andrej.

So those are definitely bugs. I don't use undefined behavior like that on purpose. Every ABI is different enough that it's hard to say that the calling conventions are safe or not. Good catch! Fixing them now.