Re: [PATCH 3/3] configure: Enable -Wthread-safety if present

[Kevin Wolf <kwolf@redhat.com>]

Am 17.01.2023 um 16:01 hat Daniel P. Berrangé geschrieben:
> On Tue, Jan 17, 2023 at 03:41:29PM +0100, Emanuele Giuseppe Esposito wrote:
> > 
> > 
> > Am 17/01/2023 um 15:02 schrieb Daniel P. Berrangé:
> > > On Tue, Jan 17, 2023 at 08:52:03AM -0500, Emanuele Giuseppe Esposito wrote:
> > >> From: Kevin Wolf <kwolf@redhat.com>
> > >>
> > >> This enables clang's thread safety analysis (TSA), which we'll use to
> > >> statically check the block graph locking.
> > >>
> > >> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > >> Message-Id: <20221207131838.239125-9-kwolf@redhat.com>
> > >> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> > >> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > >> ---
> > >>  configure | 1 +
> > >>  1 file changed, 1 insertion(+)
> > >>
> > >> diff --git a/configure b/configure
> > >> index 2281892657..14668e6269 100755
> > >> --- a/configure
> > >> +++ b/configure
> > >> @@ -1183,6 +1183,7 @@ add_to warn_flags -Wnested-externs
> > >>  add_to warn_flags -Wendif-labels
> > >>  add_to warn_flags -Wexpansion-to-defined
> > >>  add_to warn_flags -Wimplicit-fallthrough=2
> > >> +add_to warn_flags -Wthread-safety
> > > 
> > > Does this thread safety analysis have any kind of measurable
> > > impact on compilation speed ?
> > > 
> > > Our CI jobs are quite sensitive to any increase in build
> > > time.
> > 
> > From a quick run in my machine (Dell PowerEdge R440 with Intel(R)
> > Xeon(R) Gold 5120 CPU @ 2.20GHz, 28 cpus):
> > 
> > without clang:
> > real    2m46.729s\x0f
> > \x03user    19m24.122s\x0f
> > \x03sys     2m58.643s\x0f
> > 
> > with clang:
> > real    2m45.204s
> > user    19m52.096s
> > sys     2m9.036s
> > 
> > So there should be no significative impact.
> > 
> > I also forgot to mention that this serie fixes the CI failure seen in:
> > 
> > https://gitlab.com/qemu-project/qemu/-/jobs/3479763741
> > https://gitlab.com/qemu-project/qemu/-/jobs/3479763746
> 
> Odd, that job already has  -Wthread-safety included in CFLAGS, which
> would seem to make this patch redundant, but I don't see where
> -Wthread-safety came from in that pipeline 

This patch is what was already in the branch in which the above CI tests
ran. I dropped it from the pull request because of these build failures
on FreeBSD. (Their libc includes TSA annotations, which means that QEMU
functions have to be annotated as well when they call pthread locking
functions, before we can enable -Wthread-safety.)

So patches 1 and 2 fix the failure that you would otherwise see with
this patch.

Kevin