[libvirt] [PATCH] tests: fix mocking of stat() / lstat() functions

[Daniel P. Berrangé]

On Mon, Apr 01, 2019 at 06:03:29PM +0100, Daniel P. Berrangé wrote:
> Quite a few of the tests have a need to mock the stat() / lstat()
> functions and they are taking somewhat different & inconsistent
> approaches none of which are actually fully correct. This is shown
> by fact that 'make check' fails on 32-bit hosts. Investigation
> revealed that the code was calling into the native C library impl,
> not getting intercepted by our mocks.
> 
> The POSIX stat() function might resolve to any number of different
> symbols in the C library.
> 
> The may be an additional stat64() function exposed by the headers
> too.
> 
> On 64-bit hosts the stat & stat64 functions are identical, always
> refering to the 64-bit ABI.
> 
> On 32-bit hosts they refer to the 32-bit & 64-bit ABIs respectively.
> 
> Libvirt uses _FILE_OFFSET_BITS=64 on 32-bit hosts, which causes the
> C library to transparently rewrite stat() calls to be stat64() calls.
> Libvirt will never see the 32-bit ABI from the traditional stat()
> call. We cannot assume this rewriting is done using a macro. It might
> be, but on GLibC it is done with a magic __asm__ statement to apply
> the rewrite at link time instead of at preprocessing.
> 
> In GLibC there may be two additional functions exposed by the headers,
> __xstat() and __xstat64(). When these exist, stat() and stat64() are
> transparently rewritten to call __xstat() and __xstat64() respectively.
> The former symbols will not actally exist in the library at all, only
> the header. The leading "__" indicates the symbols are a private impl
> detail of the C library that applications should not care about.
> Unfortunately, because we are trying to mock replace the C library,
> we need to know about this internal impl detail.
> 
> With all this in mind the list of functions we have to mock will depend
> on several factors
> 
>  - If _FILE_OFFSET_BITS is set, then we are on a 32-bit host, and we
>    only need to mock stat64 and __xstat64. The other stat / __xstat
>    functions exist, but we'll never call them so they can be ignored
>    for mocking.
> 
>  - If _FILE_OFFSET_BITS is not set, then we are on a 64-bit host and
>    we should mock stat, stat64, __xstat & __xstat64. Either may be
>    called by app code.
> 
>  - If __xstat & __xstat64 exist, then stat & stat64 will not exist
>    as symbols in the library, so the latter should not be mocked.
> 
> The same all applies to lstat()
> 
> These rules are complex enough that we don't want to duplicate them
> across every mock file, so this centralizes all the logic in a helper
> file virmockstathelper.c that should be #included when needed. The
> code merely need to provide a filename rewriting callback called
> virMockStatRedirect(). Optionally VIR_MOCK_STAT_HOOK can be defined
> as a macro if further processing is needed inline.
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  build-aux/mock-noinline.pl |   3 +
>  cfg.mk                     |   4 +-
>  tests/qemusecuritymock.c   | 131 +++++-----------
>  tests/vircgroupmock.c      | 146 +++--------------
>  tests/virfilewrapper.c     |  85 ++--------
>  tests/virmock.h            |  11 --
>  tests/virmockstathelpers.c | 310 +++++++++++++++++++++++++++++++++++++
>  tests/virpcimock.c         |  93 +----------
>  tests/virtestmock.c        | 140 +----------------
>  9 files changed, 396 insertions(+), 527 deletions(-)
>  create mode 100644 tests/virmockstathelpers.c
> 
> NB I have tested on Fedora 32-bit and 64-bit but have not yet
> tested on other platforms, so they may break. This is to be
> investigated before pushing so we don't break other platforms
> more than they are already broken wrt mocking :-)

It passes on FreeBSD 11, but fails on RHEL-7, so need a v2 of this.


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