On Sat, Feb 10, 2024 at 12:27:35AM +0100, Martin Wilck wrote:
> Set _FILE_OFFSET_BITS=64 in order to avoid EOVERFLOW error for getdents()
> syscalls on some file systems (in particular, ext4) in emultated 32bit
> environments on 64 bit kernel, e.g. when running in qemu. This error occured
> in arm/v7 CI runs on Ubuntu runners on GitHub, which can be enabled after
> applying this change. See
> e.g. https://sourceware.org/bugzilla/show_bug.cgi?id=23960.
>
> This change requires fixes for the unit tests: If -D_FILE_OFFSET_BITS=64 is
> set, glibc headers replace some functions with their 64bit equivalents. open()
> is replaced by open64(), etc. cmocka wrappers for these functions must have
> correct name: defining __wrap_open() has no effect for calls to open64(). Unit
> tests using the wrongly named wrapper functions will fail in non-obvious ways.
>
> Use CPP macros to substitute the correct name for affected functions. Also,
> the Makefile rule that builds the -Wl,wrap= options must parse the C
> preprocessor
> output to generate suitable command line parameters for the linker.
>
> Signed-off-by: Martin Wilck <[email protected]>
Reviewed-by: Benjamin Marzinski <[email protected]>
> ---
> Makefile.inc | 1 +
> tests/Makefile | 5 ++++-
> tests/alias.c | 4 ++--
> tests/directio.c | 7 +++---
> tests/dmevents.c | 8 +++----
> tests/sysfs.c | 57 ++++++++++++++++++++++++------------------------
> tests/test-lib.c | 9 +++++---
> tests/wrap64.h | 48 ++++++++++++++++++++++++++++++++++++++++
> 8 files changed, 98 insertions(+), 41 deletions(-)
> create mode 100644 tests/wrap64.h
>
> diff --git a/Makefile.inc b/Makefile.inc
> index a0e36c4..5668e63 100644
> --- a/Makefile.inc
> +++ b/Makefile.inc
> @@ -100,6 +100,7 @@ WARNFLAGS := -Werror -Wall -Wextra -Wformat=2
> $(WFORMATOVERFLOW) -Werror=implici
> -Werror=implicit-function-declaration -Werror=format-security
> \
> $(WNOCLOBBERED) -Werror=cast-qual
> $(ERROR_DISCARDED_QUALIFIERS) $(W_URCU_TYPE_LIMITS)
> CPPFLAGS := $(FORTIFY_OPT) $(CPPFLAGS) $(D_URCU_VERSION) \
> + -D_FILE_OFFSET_BITS=64 \
> -DBIN_DIR=\"$(bindir)\" -DMULTIPATH_DIR=\"$(plugindir)\" \
> -DRUNTIME_DIR=\"$(runtimedir)\"
> -DCONFIG_DIR=\"$(configdir)\" \
> -DDEFAULT_CONFIGFILE=\"$(configfile)\"
> -DSTATE_DIR=\"$(statedir)\" \
> diff --git a/tests/Makefile b/tests/Makefile
> index 7dac8a8..7dde8bd 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -105,8 +105,11 @@ include $(wildcard $(OBJS:.o=.d))
> dep_clean:
> $(Q)$(RM) $(OBJS:.o=.d)
>
> +# Parse the C code for __wrap_xyz() functions and generate linker options
> from them.
> +# See comment in wrap64.h
> %.o.wrap: %.c
> - @sed -n 's/^.*__wrap_\([a-zA-Z0-9_]*\).*$$/-Wl,--wrap=\1/p' $< | \
> + $(Q)$(CC) $(CPPFLAGS) $($*-test_FLAGS) -E $< | \
> + sed -n 's/^.*__wrap_\([a-zA-Z0-9_]*\).*$$/-Wl,--wrap=\1/p' | \
> sort -u | tr '\n' ' ' >$@
>
> # Pass the original values of CFLAGS etc. to the sub-make, which will include
> diff --git a/tests/alias.c b/tests/alias.c
> index c2ae915..95ce994 100644
> --- a/tests/alias.c
> +++ b/tests/alias.c
> @@ -9,7 +9,7 @@
> #include "test-log.h"
> #include <errno.h>
> #include <string.h>
> -
> +#include "wrap64.h"
> #include "globals.c"
> #include "../libmultipath/alias.c"
>
> @@ -71,7 +71,7 @@ int __wrap_rename(const char *old, const char *new)
> return __set_errno(mock_type(int));
> }
>
> -int __wrap_mkstemp(char *template)
> +int WRAP_FUNC(mkstemp)(char *template)
> {
> return 10;
> }
> diff --git a/tests/directio.c b/tests/directio.c
> index 5201d21..cbedcc9 100644
> --- a/tests/directio.c
> +++ b/tests/directio.c
> @@ -24,6 +24,7 @@
> #include <setjmp.h>
> #include <stdlib.h>
> #include <cmocka.h>
> +#include "wrap64.h"
> #include "globals.c"
> #include "../libmultipath/checkers/directio.c"
>
> @@ -66,12 +67,12 @@ int __wrap_ioctl(int fd, ioctl_request_t request, void
> *argp)
> #endif
> }
>
> -int __real_fcntl(int fd, int cmd, long arg);
> +int REAL_FCNTL (int fd, int cmd, long arg);
>
> -int __wrap_fcntl(int fd, int cmd, long arg)
> +int WRAP_FCNTL (int fd, int cmd, long arg)
> {
> #ifdef DIO_TEST_DEV
> - return __real_fcntl(fd, cmd, arg);
> + return REAL_FCNTL(fd, cmd, arg);
> #else
> assert_int_equal(fd, test_fd);
> assert_int_equal(cmd, F_GETFL);
> diff --git a/tests/dmevents.c b/tests/dmevents.c
> index f08e649..540bf0d 100644
> --- a/tests/dmevents.c
> +++ b/tests/dmevents.c
> @@ -29,7 +29,7 @@
> #include <fcntl.h>
> #include "structs.h"
> #include "structs_vec.h"
> -
> +#include "wrap64.h"
> #include "globals.c"
> /* I have to do this to get at the static variables */
> #include "../multipathd/dmevents.c"
> @@ -207,7 +207,7 @@ static int teardown(void **state)
> return 0;
> }
>
> -int __wrap_open(const char *pathname, int flags)
> +int WRAP_FUNC(open)(const char *pathname, int flags)
> {
> assert_ptr_equal(pathname, "/dev/mapper/control");
> assert_int_equal(flags, O_RDWR);
> @@ -389,7 +389,7 @@ static void test_init_waiter_bad1(void **state)
> struct test_data *datap = (struct test_data *)(*state);
> if (datap == NULL)
> skip();
> - will_return(__wrap_open, -1);
> + wrap_will_return(WRAP_FUNC(open), -1);
> assert_int_equal(init_dmevent_waiter(&datap->vecs), -1);
> assert_ptr_equal(waiter, NULL);
> }
> @@ -400,7 +400,7 @@ static void test_init_waiter_good0(void **state)
> struct test_data *datap = (struct test_data *)(*state);
> if (datap == NULL)
> skip();
> - will_return(__wrap_open, 2);
> + wrap_will_return(WRAP_FUNC(open), 2);
> assert_int_equal(init_dmevent_waiter(&datap->vecs), 0);
> assert_ptr_not_equal(waiter, NULL);
> }
> diff --git a/tests/sysfs.c b/tests/sysfs.c
> index 6e07efd..fc256d8 100644
> --- a/tests/sysfs.c
> +++ b/tests/sysfs.c
> @@ -18,6 +18,7 @@
> #include "test-log.h"
> #include "sysfs.h"
> #include "util.h"
> +#include "wrap64.h"
>
> #define TEST_FD 123
>
> @@ -28,7 +29,7 @@ char *__wrap_udev_device_get_syspath(struct udev_device *ud)
> return val;
> }
>
> -int __wrap_open(const char *pathname, int flags)
> +int WRAP_FUNC(open)(const char *pathname, int flags)
> {
> int ret;
>
> @@ -166,10 +167,10 @@ static void test_sagv_open_fail(void **state)
>
> will_return(__wrap_udev_device_get_syspath, "/foo");
> expect_condlog(4, "open '/foo/bar'");
> - expect_string(__wrap_open, pathname, "/foo/bar");
> - expect_value(__wrap_open, flags, O_RDONLY);
> + expect_string(WRAP_FUNC(open), pathname, "/foo/bar");
> + expect_value(WRAP_FUNC(open), flags, O_RDONLY);
> errno = ENOENT;
> - will_return(__wrap_open, -1);
> + wrap_will_return(WRAP_FUNC(open), -1);
> expect_condlog(3, "__sysfs_attr_get_value: attribute '/foo/bar' cannot
> be opened");
> assert_int_equal(sysfs_attr_get_value((void *)state, "bar",
> buf, sizeof(buf)), -ENOENT);
> @@ -181,9 +182,9 @@ static void test_sagv_read_fail(void **state)
>
> will_return(__wrap_udev_device_get_syspath, "/foo");
> expect_condlog(4, "open '/foo/bar'");
> - expect_string(__wrap_open, pathname, "/foo/bar");
> - expect_value(__wrap_open, flags, O_RDONLY);
> - will_return(__wrap_open, TEST_FD);
> + expect_string(WRAP_FUNC(open), pathname, "/foo/bar");
> + expect_value(WRAP_FUNC(open), flags, O_RDONLY);
> + wrap_will_return(WRAP_FUNC(open), TEST_FD);
> expect_value(__wrap_read, fd, TEST_FD);
> expect_value(__wrap_read, count, sizeof(buf));
> errno = EISDIR;
> @@ -196,9 +197,9 @@ static void test_sagv_read_fail(void **state)
>
> will_return(__wrap_udev_device_get_syspath, "/foo");
> expect_condlog(4, "open '/foo/baz'");
> - expect_string(__wrap_open, pathname, "/foo/baz");
> - expect_value(__wrap_open, flags, O_RDONLY);
> - will_return(__wrap_open, TEST_FD);
> + expect_string(WRAP_FUNC(open), pathname, "/foo/baz");
> + expect_value(WRAP_FUNC(open), flags, O_RDONLY);
> + wrap_will_return(WRAP_FUNC(open), TEST_FD);
> expect_value(__wrap_read, fd, TEST_FD);
> expect_value(__wrap_read, count, sizeof(buf));
> errno = EPERM;
> @@ -222,9 +223,9 @@ static void _test_sagv_read(void **state, unsigned int
> bufsz)
> memset(buf, '.', sizeof(buf));
> will_return(__wrap_udev_device_get_syspath, "/foo");
> expect_condlog(4, "open '/foo/bar'");
> - expect_string(__wrap_open, pathname, "/foo/bar");
> - expect_value(__wrap_open, flags, O_RDONLY);
> - will_return(__wrap_open, TEST_FD);
> + expect_string(WRAP_FUNC(open), pathname, "/foo/bar");
> + expect_value(WRAP_FUNC(open), flags, O_RDONLY);
> + wrap_will_return(WRAP_FUNC(open), TEST_FD);
> expect_value(__wrap_read, fd, TEST_FD);
> expect_value(__wrap_read, count, bufsz);
> will_return(__wrap_read, sizeof(input) - 1);
> @@ -249,9 +250,9 @@ static void _test_sagv_read(void **state, unsigned int
> bufsz)
> memset(buf, '.', sizeof(buf));
> will_return(__wrap_udev_device_get_syspath, "/foo");
> expect_condlog(4, "open '/foo/baz'");
> - expect_string(__wrap_open, pathname, "/foo/baz");
> - expect_value(__wrap_open, flags, O_RDONLY);
> - will_return(__wrap_open, TEST_FD);
> + expect_string(WRAP_FUNC(open), pathname, "/foo/baz");
> + expect_value(WRAP_FUNC(open), flags, O_RDONLY);
> + wrap_will_return(WRAP_FUNC(open), TEST_FD);
> expect_value(__wrap_read, fd, TEST_FD);
> expect_value(__wrap_read, count, bufsz);
> will_return(__wrap_read, sizeof(input) - 1);
> @@ -300,9 +301,9 @@ static void _test_sagv_read_zeroes(void **state, unsigned
> int bufsz)
> memset(buf, '.', sizeof(buf));
> will_return(__wrap_udev_device_get_syspath, "/foo");
> expect_condlog(4, "open '/foo/bar'");
> - expect_string(__wrap_open, pathname, "/foo/bar");
> - expect_value(__wrap_open, flags, O_RDONLY);
> - will_return(__wrap_open, TEST_FD);
> + expect_string(WRAP_FUNC(open), pathname, "/foo/bar");
> + expect_value(WRAP_FUNC(open), flags, O_RDONLY);
> + wrap_will_return(WRAP_FUNC(open), TEST_FD);
> expect_value(__wrap_read, fd, TEST_FD);
> expect_value(__wrap_read, count, bufsz);
> will_return(__wrap_read, sizeof(input) - 1);
> @@ -385,10 +386,10 @@ static void test_sasv_open_fail(void **state)
>
> will_return(__wrap_udev_device_get_syspath, "/foo");
> expect_condlog(4, "open '/foo/bar'");
> - expect_string(__wrap_open, pathname, "/foo/bar");
> - expect_value(__wrap_open, flags, O_WRONLY);
> + expect_string(WRAP_FUNC(open), pathname, "/foo/bar");
> + expect_value(WRAP_FUNC(open), flags, O_WRONLY);
> errno = EPERM;
> - will_return(__wrap_open, -1);
> + wrap_will_return(WRAP_FUNC(open), -1);
> expect_condlog(3, "sysfs_attr_set_value: attribute '/foo/bar' cannot be
> opened");
> assert_int_equal(sysfs_attr_set_value((void *)state, "bar",
> buf, sizeof(buf)), -EPERM);
> @@ -400,9 +401,9 @@ static void test_sasv_write_fail(void **state)
>
> will_return(__wrap_udev_device_get_syspath, "/foo");
> expect_condlog(4, "open '/foo/bar'");
> - expect_string(__wrap_open, pathname, "/foo/bar");
> - expect_value(__wrap_open, flags, O_WRONLY);
> - will_return(__wrap_open, TEST_FD);
> + expect_string(WRAP_FUNC(open), pathname, "/foo/bar");
> + expect_value(WRAP_FUNC(open), flags, O_WRONLY);
> + wrap_will_return(WRAP_FUNC(open), TEST_FD);
> expect_value(__wrap_write, fd, TEST_FD);
> expect_value(__wrap_write, count, sizeof(buf));
> errno = EISDIR;
> @@ -421,9 +422,9 @@ static void _test_sasv_write(void **state, unsigned int
> n_written)
> assert_in_range(n_written, 0, sizeof(buf));
> will_return(__wrap_udev_device_get_syspath, "/foo");
> expect_condlog(4, "open '/foo/bar'");
> - expect_string(__wrap_open, pathname, "/foo/bar");
> - expect_value(__wrap_open, flags, O_WRONLY);
> - will_return(__wrap_open, TEST_FD);
> + expect_string(WRAP_FUNC(open), pathname, "/foo/bar");
> + expect_value(WRAP_FUNC(open), flags, O_WRONLY);
> + wrap_will_return(WRAP_FUNC(open), TEST_FD);
> expect_value(__wrap_write, fd, TEST_FD);
> expect_value(__wrap_write, count, sizeof(buf));
> will_return(__wrap_write, n_written);
> diff --git a/tests/test-lib.c b/tests/test-lib.c
> index f75ea31..665d438 100644
> --- a/tests/test-lib.c
> +++ b/tests/test-lib.c
> @@ -16,6 +16,7 @@
> #include "propsel.h"
> #include "unaligned.h"
> #include "test-lib.h"
> +#include "wrap64.h"
>
> const int default_mask =
> (DI_SYSFS|DI_BLACKLIST|DI_WWID|DI_CHECKER|DI_PRIO|DI_SERIAL);
> const char default_devnode[] = "sdxTEST";
> @@ -38,16 +39,18 @@ const char default_wwid_1[] = "TEST-WWID-1";
> * resolved by the assembler before the linking stage.
> */
>
> -int __real_open(const char *path, int flags, int mode);
> +
> +int REAL_FUNC(open)(const char *path, int flags, int mode);
>
> static const char _mocked_filename[] = "mocked_path";
> -int __wrap_open(const char *path, int flags, int mode)
> +
> +int WRAP_FUNC(open)(const char *path, int flags, int mode)
> {
> condlog(4, "%s: %s", __func__, path);
>
> if (!strcmp(path, _mocked_filename))
> return 111;
> - return __real_open(path, flags, mode);
> + return REAL_FUNC(open)(path, flags, mode);
> }
>
> int __wrap_libmp_get_version(int which, unsigned int version[3])
> diff --git a/tests/wrap64.h b/tests/wrap64.h
> new file mode 100644
> index 0000000..8c91d27
> --- /dev/null
> +++ b/tests/wrap64.h
> @@ -0,0 +1,48 @@
> +#ifndef _WRAP64_H
> +#define _WRAP64_H 1
> +#include "util.h"
> +
> +/*
> + * Make cmocka work with _FILE_OFFSET_BITS == 64
> + *
> + * If -D_FILE_OFFSET_BITS=64 is set, glibc headers replace some functions
> with
> + * their 64bit equivalents. "open()" is replaced by "open64()", etc. cmocka
> + * wrappers for these functions must have correct name __wrap_open() doesn't
> + * work if the function called is not open() but open64(). Consequently, unit
> + * tests using such wrappers will fail.
> + * Use some CPP trickery to insert the correct name. The Makefile rule that
> + * creates the .wrap file must parse the C preprocessor output to generate
> the
> + * correct -Wl,wrap= option.
> + */
> +
> +/* Without this indirection, WRAP_FUNC(x) would be expanded to
> __wrap_WRAP_NAME(x) */
> +#define CONCAT2_(x, y) x ## y
> +#define CONCAT2(x, y) CONCAT2_(x, y)
> +
> +#if defined(__GLIBC__) && _FILE_OFFSET_BITS == 64
> +#define WRAP_NAME(x) x ## 64
> +#else
> +#define WRAP_NAME(x) x
> +#endif
> +#define WRAP_FUNC(x) CONCAT2(__wrap_, WRAP_NAME(x))
> +#define REAL_FUNC(x) CONCAT2(__real_, WRAP_NAME(x))
> +
> +/*
> + * fcntl() needs special treatment; fcntl64() has been introduced in 2.28.
> + * https://savannah.gnu.org/forum/forum.php?forum_id=9205
> + */
> +#if defined(__GLIBC__) && __GLIBC_PREREQ(2, 28)
> +#define WRAP_FCNTL_NAME WRAP_NAME(fcntl)
> +#else
> +#define WRAP_FCNTL_NAME fcntl
> +#endif
> +#define WRAP_FCNTL CONCAT2(__wrap_, WRAP_FCNTL_NAME)
> +#define REAL_FCNTL CONCAT2(__real_, WRAP_FCNTL_NAME)
> +
> +/*
> + * will_return() is itself a macro that uses CPP "stringizing". We need a
> + * macro indirection to make sure the *value* of WRAP_FUNC() is stringized
> + * (see https://gcc.gnu.org/onlinedocs/cpp/Stringizing.html).
> + */
> +#define wrap_will_return(x, y) will_return(x, y)
> +#endif
> --
> 2.43.0
