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@xxxxxxxxxx> > --- > 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
