From f9f5ab57189b9c378d1893cf302865ac9b88bbbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Thu, 16 Mar 2023 12:50:56 +0000 Subject: [PATCH] meson: stop CLang doing inter-procedural analysis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 successfully 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 would 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. Out of those choices option (3) is the only viable option for CLang. We don't want todo it for GCC though as it is such a big hammer. Probably we should apply (2) for GCC, should we experiance a problem in future. Signed-off-by: Daniel P. Berrangé --- meson.build | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/meson.build b/meson.build index 319ed790f9..c35823a79a 100644 --- a/meson.build +++ b/meson.build @@ -404,6 +404,26 @@ cc_flags += [ '-Wwrite-strings', ] +if cc.get_id() == 'clang' + # Stop CLang from doing inter-procedural analysis of calls + # between functions in the same compilation unit. Such an + # optimization has been know to break the test suite by + # making assumptions that a return value is a constant. + # This makes it impossible to mock certain functions with + # replacement definitions via LD_PRELOAD that have different + # semantics. + # + # This is a bit of a big hammer, but alternatives don't work: + # + # - 'weak' attribute - weak symbols get dropped from + # when the .a libs are combined into the .so + # see commit 407a281a8e2b6c5078ba1148535663ea64fd9314 + # + # - 'noipa' attribute - only available with GCC currently + # https://reviews.llvm.org/D101011 + cc_flags += [ '-fsemantic-interposition' ] +endif + supported_cc_flags = [] if get_option('warning_level') == '2' supported_cc_flags = cc.get_supported_arguments(cc_flags)