Re: [PATCH] io/channel-socket: Fix -fsanitize=undefined problem with latest Clang

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Jun 03, 2024 at 04:12:34PM +0100, Peter Maydell wrote:
> On Mon, 3 Jun 2024 at 15:58, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Mon, 3 Jun 2024 at 15:49, Daniel P. Berrangé <berrange@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.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|