[PATCH 0/2] Work around broken clang. Again.

[Daniel P. Berrangé]

On Tue, Mar 21, 2023 at 10:46:36AM +0000, Daniel P. Berrangé wrote:
> On Mon, Mar 20, 2023 at 03:17:24PM +0100, Michal Prívozník wrote:
> > On 3/16/23 15:51, Andrea Bolognani wrote:
> > > A thought about VIR_OPTNONE. It seems to me that we'd want to apply
> > > this to all the functions that currently are marked with G_NO_INLINE
> > > for mocking purposes. So, wouldn't it make sense to instead have a
> > > single VIR_MOCKABLE annotation that combines the two, and use that
> > > everywhere?
> > > 
> > 
> > The problem is, __attribute__((optnone)) works only when passed at
> > function definition, while noinline can be passed at function
> > definition.
> 
> Is that a problem in fact ? We only need to suppress inlining when
> the calling function is in the same compilation unit as the called
> function. Likewise for this return value optimization. So if 'noinline'
> annotation was in the definition, rather than the declaration, it
> would be fine for our purposes.
> 
> I think it would make sense to have a VIR_MOCKABLE annotation that
> covered 'noline', 'optnone' and 'noipa' attributes (the precise set
> varying according to what compiler we target).

Empirically 'optnone' does *NOT* fix the problem we face. The
optimizations about return value still get performed with 'optnone'
present :-(

After chasing links I found

  https://reviews.llvm.org/D28404#641077

where there is this nugget

  "optnone isn't *really* no optimizations: clearly this is true,
   but then neither is -O0. We run the always inliner, a couple
   of other passes, and we run several parts of the code generators
   optimizer. I understand why optnone deficiencies (ie, too many
   optimizations) might be frustrating, but having *more users*
   seems likely to make this *better*."

Much of that ticket is debate of whether 'optnone' should be thue
same as '-O0'

There is also a blog post

  http://maskray.me/blog/2021-05-09-fno-semantic-interposition

which confirms what we discovered

  "For ELF -fpic, Clang never suppresses interprocedural optimizations
   (including inlining) on default visibility external linkage
   definitions. So projects relying on blocked interprocedural
   optimizations have been broken for years. They only probably work
   recently by specifying -fsemantic-interposition."

By 'default visibility' they basically mean any symbol which is not
marked as 'weak', which is what we tried todo before and then reverted
with:

  commit 407a281a8e2b6c5078ba1148535663ea64fd9314
  Author: Daniel P. Berrangé <berrange at redhat.com>
  Date:   Wed Jul 12 11:07:17 2017 +0100

    Revert "Prevent more compiler optimization of mockable functions"
    
    This reverts commit e4b980c853d2114b25fa805a84ea288384416221.


Looking at the revert again though, I think we might not be out of
luck. We had to revert because when we put stuff into .a files it
got lost when linked to the final executable when it was marked
weak. I think we might be able to work around this using linker
flag -Wl,--whole-archive.

This wasn't practical in 2017 as we still used automake+libtool
and they had a habit of re-ordering such args. Now we're using
meson though, it has a 'link_whole' option integrated which
should do the right thing with -Wl,--whole-archive.

This might let us re-try the weak function approach.

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 :|