Skip to content

Commit

Permalink
multipath-tools: Makefile.inc: set _FILE_OFFSET_BITS=64
Browse files Browse the repository at this point in the history
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@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
  • Loading branch information
mwilck committed Feb 12, 2024
1 parent 821c3ba commit 7b217f8
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 41 deletions.
1 change: 1 addition & 0 deletions Makefile.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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)\" \
Expand Down
5 changes: 4 additions & 1 deletion tests/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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) $(OPTFLAGS) $(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
Expand Down
4 changes: 2 additions & 2 deletions tests/alias.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "test-log.h"
#include <errno.h>
#include <string.h>

#include "wrap64.h"
#include "globals.c"
#include "../libmultipath/alias.c"

Expand Down Expand Up @@ -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;
}
Expand Down
7 changes: 4 additions & 3 deletions tests/directio.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <setjmp.h>
#include <stdlib.h>
#include <cmocka.h>
#include "wrap64.h"
#include "globals.c"
#include "../libmultipath/checkers/directio.c"

Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions tests/dmevents.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
57 changes: 29 additions & 28 deletions tests/sysfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "test-log.h"
#include "sysfs.h"
#include "util.h"
#include "wrap64.h"

#define TEST_FD 123

Expand All @@ -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;

Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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);
Expand Down
9 changes: 6 additions & 3 deletions tests/test-lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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])
Expand Down
48 changes: 48 additions & 0 deletions tests/wrap64.h
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 7b217f8

Please sign in to comment.