Re: [PATCH v2 7/7] multipath-tools: Makefile.inc: set _FILE_OFFSET_BITS=64

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <mwilck@xxxxxxxx>
Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> ---
>  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





[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux