On Thu, Mar 16, 2023 at 13:05:43 +0000, Daniel P. Berrangé wrote: > The virNumaNodeIsAvailable function is stubbed out when building > without libnuma, such that it just returns a constant value. When > CLang is optimizing, it does inter-procedural analysis across > function calls. When it sees that the call to virNumaNodeIsAvailable > returns a fixed constant, it elides the conditional check for errors > in the callers such as virNumaNodesetIsAvailable. > > This is a valid optimization as the C standard declares that there > must only be one implementation of each function in a binary. This > is normally the case, but ELF allows for function overrides when > linking or at runtime with LD_PRELOAD, which is technically outside > the mandated C language behaviour. > > So while CLang's optimization works fine at runtime, it breaks in our > test suite which aims to mock the virNumaNodeIsAvailable function so > that it has specific semantics regardless of whether libnuma is built > or not. The return value check optimization though means our mock > override won't have the right effect. The mock will be invoked, but > its return value is not used. > > Potentially the same problem could be exhibited with GCC if certain > combinations of optimizations are enabled, though thus far we've > not seen it. > > To be robust on both CLang and GCC we need to make it more explicit > that we want to be able to replace functions and thus optimization > of calls must be limited. Currently we rely on 'noinline' which > does succesfully prevent inlining of the function, but it cannot > stop the eliding of checks based on the constant return value. > Thus we need a bigger hammer. > > There are a couple of options to disable this optimization: > > * Annotate a symbol as 'weak'. This is tells the compiler > that the symbol is intended to be overridable at linktime > or runtime, and thus it will avoid doing inter-procedural > analysis for optimizations. This was tried previously but > have to be reverted as it had unintended consequences > when linking .a files into our final .so, resulting in all > the weak symbol impls being lost. See commit > 407a281a8e2b6c5078ba1148535663ea64fd9314 > > * Annotate a symbol with 'noipa'. This tells the compiler > to avoid inter-procedural analysis for calls to just this > function. This wold be ideal match for our scenario, but > unfortunately it is only implemented for GCC currently: > > https://reviews.llvm.org/D101011 > > * The '-fsemantic-interposition' argument tells the optimizer > that any functions may be replaced with alternative > implementations that have different semantics. It thus > blocks any optimizations across function calls. This is > quite a harsh block on the optimizer, but it appears to be > the only one that is viable with CLang. > > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> > --- > meson.build | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) Repeating approval from the MR. Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
