Destructors `msg_fddata_release` and `str_release` being called via `void (*)(void *)` is an undefined behaviour.

[shamefulCake1]

As far as I understand, calling a function via incompatible function pointers (even if the argument types are compatible) is undefined behaviour.

A related (but for c++) question discussed here: https://stackoverflow.com/questions/43220926/call-to-function-unknown-through-pointer-to-incorrect-function-type

Would it be possible to fix this somehow?

libslack/msg.c:357:4: runtime error: call to function msg_fddata_release through pointer to incorrect function type 'void (*)(void *)'
~/repos/daemon/libslack/msg.c:514: note: msg_fddata_release defined here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libslack/msg.c:357:4 
libslack/list.c:340:4: runtime error: call to function str_release through pointer to incorrect function type 'void (*)(void *)'
~/repos/daemon/libslack/str.c:874: note: str_release defined here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libslack/list.c:340:4 

[raforg]

Hi, Sorry but I don't think it's fixable. That's how to do generics in C (i.e. rely on the the ability of void * to act as a placeholder for any other pointer). I know of no other way to do it. But the good news is that, even though it's undefined behaviour according to UBSAN, it has never not worked on any operating system I have access to in 25 years. In fact, the only way I can think of for it to not work is if some function pointers were 64-bit while other function pointers were 128-bit. Hopefully, that will never happen. But even if it does happen, whoever tries to use the differently-sized function pointer should find out pretty quickly that it doesn't work and find their own solution (like maybe a wrapper function with the right-sized function pointer).

[shamefulCake1]

It's there no way to use some clever type-casting, or doing a union of different types?

[raforg]

Not a union (but keep reading for the good news). A union would require defining a union in the library that contains every function pointer type that will ever be used by any client of the library. That's not possible to do without requiring every user of the library to modify the union in the library whenever they want to use a new function with the library. And there is already clever type casting but it seems that casting function pointer types is undefined (who knew?).

Actually, I suspect it could be fixed by changing the signatures of msg_fddata_release and str_release to take a void * parameter, but I'm not willing to do that. That would take away type checking from those functions when they are used explicitly and not through a generic function pointer.

Actually, it could probably also be fixed by defining an additional wrapper function for each release function that is used generically. The wrapper function could have the correct signature to match the generic function pointer type, and the body of the wrapper function could call the underlying function and cast its own void * argument to the appropriate underlying type. That way, an undefined function pointer cast is replaced with a defined pointer cast.

Until it's actually broken, I'd rather not introduce extra function calls. Although it might be safe to assume that they would be inlined.

But that does sound like a good solution. The underlying release functions in the library stay as they are, and when some client code wants to use them with code expecting a generic function pointer, that client code can add a wrapper function for that purpose.

Actually the wrapper functions should be added to the library (at least for all the release functions which are most likely to need to be used in a generic context). And the wrapper functions probably need to be in the library itself if there is to be any chance of them being inlined.

Thanks for not giving up! I will fix it. I recently added the ability to use ubsan analysis and I'm in the middle of a large increase in library test coverage, so presumably I'll see this in normal testing and will resolve it. But that work was interrupted quite some time ago and other things got in the way. But thanks for making me think more about this (although I don't think it's important, but maybe it will be when we have 128-bit computers). But I admit that it does feel good when there is no undefined behaviour. When I added ubsan analysis to another program of mine, it was great that the only undefined behaviour was a test that deliberately instructed the program to do arithmetic overflow.