On 03/06/2024 17.52, Daniel P. Berrangé wrote:
On Mon, Jun 03, 2024 at 04:12:34PM +0100, Peter Maydell wrote:
On Mon, 3 Jun 2024 at 15:58, Peter Maydell <peter.mayd...@linaro.org> wrote:

On Mon, 3 Jun 2024 at 15:49, Daniel P. Berrangé <berra...@redhat.com> wrote:
We can't rely on the sanitizers to catch all cases where we're casting
functions, as we don't have good enough code coverage in tests to
identify all places that way.

Unless there's a warning flag we can use to get diagnosis of this
problem at compile time and then fix all reported issues, I won't have
any confidence in our ability to remove -fsanitize-cfi-icall-generalize-pointers
for CFI.

You might think that -Wcast-function-type would detect them at compile
time, but that has two loopholes:
  1. void(*) (void)  matches everything
  2. any parameter of pointer type matches any other pointer type

It seems to me rather inconsistent that the sanitizers do
not match up with the warning semantics here. I think I
would vote for raising that with the compiler folks --
either the sanitizer should be made looser so it matches
the -Wcast-function-type semantics, or else a new tighter
warning option that matches the sanitizer should be
provided. Ideally both, I think. But it's definitely silly
to have a runtime check that flags up things that the
compiler perfectly well could detect at compile time but
is choosing not to.

Slightly further investigation suggests clang 16 and later
have -Wcast-function-type-strict for the "report all the
same casts that the sanitizer does". gcc doesn't I think have
that yet. I didn't spot any option in the sanitizer to
loosen the set of things it considers mismatched function
pointers.

I just tried that with

CC=clang ./configure --target-list=x86_64-softmmu 
--extra-cflags="-Wcast-function-type-strict"  --disable-werror

and it rather shows the futility of the task, picking one reoprted
warning that is repeated over & over in differnt contexts:

In file included from qapi/qapi-types-block-core.c:15:
qapi/qapi-types-block-core.h:3620:1: warning: cast from 'void 
(*)(DummyBlockCoreForceArrays *)' (aka 'void (*)(struct 
DummyBlockCoreForceArrays *)') to 'void (*)(void)' converts to incompatible 
function type [-Wcast-function-type-strict]
  3620 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(DummyBlockCoreForceArrays, 
qapi_free_DummyBlockCoreForceArrays)
       | 
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/gmacros.h:1372:3: note: expanded from macro 
'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
  1372 |   _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func)
       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/gmacros.h:1364:57: note: expanded from macro 
'_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS'
  1364 |     { if (*_q) g_queue_free_full (*_q, (GDestroyNotify) 
(void(*)(void)) cleanup); }                             \
       |                                                         
^~~~~~~~~~~~~~~~~~~~~~~


IOW, the GLib headers themselves have bad casts in macros which we
rely on.  So we'll never be cast safe until GLib changes its own
code...if it ever does.

Ok, thanks for checking! So the ultimate answer to the problem (at least right now) is: Let's use -fno-sanitize=function in QEMU.

 Thomas



Reply via email to