On Wed, 29 May 2024 at 14:32, Thomas Huth <th...@redhat.com> wrote:
>
> Casting function pointers from one type to another causes undefined
> behavior errors when compiling with -fsanitize=undefined with Clang v18:
>
> $ QTEST_QEMU_BINARY=./qemu-system-mips64 tests/qtest/netdev-socket
> TAP version 13
> # random seed: R02S4424f4f460de783fdd3d72c5571d3adc
> 1..10
> # Start of mips64 tests
> # Start of netdev tests
> # Start of stream tests
> # starting QEMU: exec ./qemu-system-mips64 -qtest
> unix:/tmp/qtest-1213196.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-1213196.qmp,id=char0 -mon chardev=char0,mode=control
> -display none -audio none -nodefaults -M none -netdev
> stream,id=st0,addr.type=fd,addr.str=3 -accel qtest
> ../io/task.c:78:13: runtime error: call to function qapi_free_SocketAddress
> through pointer to incorrect function type 'void (*)(void *)'
> /tmp/qemu-sanitize/qapi/qapi-types-sockets.c:170: note:
> qapi_free_SocketAddress defined here
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../io/task.c:78:13
It's a pity the sanitizer error doesn't tell you the actual
function type as well as the incorrect one it got cast to
(especially since in this case the function and its declaration
are both in generated code in the build tree not conveniently
findable with 'git grep'.)
In this case the function being called is:
void qapi_free_SocketAddress(SocketAddress *obj)
and it's cast to a GDestroyNotify, which is
typedef void (*GDestroyNotify) (gpointer data);
(and gpointer is void*)
and although you can pass a foo* to a function expecting void*,
you can't treat a pointer to a function taking foo* as if it was
a pointer to a function taking void*, just in case the compiler
needs to do some clever trickery with the pointer value.
So the wrapper function looks like it doesn't do anything,
but it's doing the permitted implicit-cast of the argument.
> Add a wrapper function to avoid the problem.
>
> Signed-off-by: Thomas Huth <th...@redhat.com>
> ---
> io/channel-socket.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/io/channel-socket.c b/io/channel-socket.c
> index 3a899b0608..aa2a1c8586 100644
> --- a/io/channel-socket.c
> +++ b/io/channel-socket.c
> @@ -193,6 +193,10 @@ static void qio_channel_socket_connect_worker(QIOTask
> *task,
> qio_task_set_error(task, err);
> }
>
> +static void qio_qapi_free_SocketAddress(gpointer sa)
> +{
> + qapi_free_SocketAddress(sa);
> +}
>
> void qio_channel_socket_connect_async(QIOChannelSocket *ioc,
> SocketAddress *addr,
> @@ -213,7 +217,7 @@ void qio_channel_socket_connect_async(QIOChannelSocket
> *ioc,
> qio_task_run_in_thread(task,
> qio_channel_socket_connect_worker,
> addrCopy,
> - (GDestroyNotify)qapi_free_SocketAddress,
> + qio_qapi_free_SocketAddress,
> context);
> }
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
thanks
-- PMM
