On Mon, Apr 28, 2025 at 03:20:56PM +0200, Michal Privoznik via Devel wrote: > From: Michal Privoznik <mpriv...@redhat.com> > > Currently, if we want to mock a function the noinline attribute > is appended after the function (via G_NO_INLINE macro). This used > to work for non pure functions. But there are some trivial > functions (for instance virQEMUCapsProbeHVF()) that are pure, > i.e. have no side effect, and while their call from other parts > of the code is not optimized out, their call from within the same > compilation unit (qemu_capabilities.c) is optimized out. > > This is because inlining and semantic interposition are two > different things. Even GCC's documentation for noinline attribute > [1] states that clearly: > > This function attribute prevents a function from being > considered for inlining. It also disables some other > interprocedural optimizations; it’s preferable to use the more > comprehensive noipa attribute instead if that is your goal. > > Even if a function is declared with the noinline attribute, > there are optimizations other than inlining that can cause > calls to be optimized away if it does not have side effects, > although the function call is live. > > Unfortunately, despite attempts [2] Clang still does not support > the attribute and thus we have to rely on noinline + > -fsemantic-interposition combo. > > 1: > https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noinline-function-attribute > 2: https://reviews.llvm.org/D101011 > > Signed-off-by: Michal Privoznik <mpriv...@redhat.com> > --- > docs/coding-style.rst | 2 +- > scripts/cocci-macro-file.h | 1 + > src/internal.h | 21 +++++++++++++++++++++ > 3 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/docs/coding-style.rst b/docs/coding-style.rst > index fe5fe9a906..81382f11d4 100644 > --- a/docs/coding-style.rst > +++ b/docs/coding-style.rst > @@ -633,7 +633,7 @@ analysis tools understand the code better: > ``G_GNUC_FALLTHROUGH`` > allow code reuse by multiple switch cases > > -``G_NO_INLINE`` > +``ATTRIBUTE_NOIPA`` > the function is mocked in the test suite > > ``G_GNUC_NORETURN`` > diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h > index c3112663d1..b26018c20b 100644 > --- a/scripts/cocci-macro-file.h > +++ b/scripts/cocci-macro-file.h > @@ -21,6 +21,7 @@ > > #pragma once > > +#define ATTRIBUTE_NOIPA > #define ATTRIBUTE_NONNULL(x) > #define ATTRIBUTE_PACKED > > diff --git a/src/internal.h b/src/internal.h > index 20aa9b1d41..b96661249e 100644 > --- a/src/internal.h > +++ b/src/internal.h > @@ -177,6 +177,27 @@ > # endif > #endif > > +/** > + * > + * ATTRIBUTE_NOIPA > + * > + * Force compiler to disable interprocedural optimizations between the > + * function with this attribute and its callers. This implies noinline > + * attribute and some others and allows us to mock functions even if > + * they are pure. > + * > + * WARNING: on compilers which don't support the attribute (clang) this > + * is silently declared as noinline which in combination with > + * -fsemantic-interposition option does roughly the same. > + */ > +#ifndef ATTRIBUTE_NOIPA > +# if defined(__has_attribute) && __has_attribute(noipa) > +# define ATTRIBUTE_NOIPA __attribute__((noipa)) > +# else > +# define ATTRIBUTE_NOIPA G_NO_INLINE
Hmm, that's kinda misleading. How about we detach our naming from the impl choice ? ATTRIBUTE_MOCKABLE ? 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 :|
