From f8341632fe3b06fd113b4a4da0a20838e4eaba0f Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 25 Feb 2025 11:42:52 +0000 Subject: [PATCH] tests: Only change the behaviour of getpwuid() inside each test function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `gutils-user-database` test uses `LD_PRELOAD` to load `getpwuid-preload.so`, which contains overrides for `getpwuid()`, `getpwnam_r()` and `getpwuid_r()`. These overrides clear the `pw_name` field to `NULL` to test handling of a weird error case from an old systemd implementation of those functions (see !2244). When running the `G_DISABLE_ASSERT` CI job, this test was crashing without any debugging output. The `G_DISABLE_ASSERT` CI job runs its tests under `--setup thorough`, which was the culprit here: the function overrides in `getpwuid-preload.so` were being loaded (via `LD_PRELOAD`) into the `bash` process which runs `thorough-test-wrapper.sh` and causing it to crash. Interestingly, this couldn’t be reproduced locally with a comparable version of bash. Perhaps it depends on the CI machine’s environment, triggering bash to make a `getpwuid()` (or similar) call which it wouldn’t normally do locally. So, slightly change `getpwuid-preload.so` to additionally link to `libglib-2.0.so` and query `g_test_get_path()` to work out whether it’s being called inside a unit test. If not, don’t modify the return value. Signed-off-by: Philip Withnall Fixes: #3272 --- glib/tests/getpwuid-preload.c | 20 +++++++++++++++++--- glib/tests/meson.build | 2 +- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/glib/tests/getpwuid-preload.c b/glib/tests/getpwuid-preload.c index 0c005a8b4..5da4fa8b7 100644 --- a/glib/tests/getpwuid-preload.c +++ b/glib/tests/getpwuid-preload.c @@ -21,6 +21,7 @@ */ #include +#include #include #include #include @@ -58,13 +59,24 @@ #undef getpwuid +/* Only modify the result if running inside a GLib test. Otherwise, we risk + * breaking test harness code, or test wrapper processes (as this binary is + * loaded via LD_PRELOAD, it will affect all wrapper processes). */ +static inline int +should_modify_result (void) +{ + const char *path = g_test_get_path (); + return (path != NULL && *path != '\0'); +} + static struct passwd my_pw; DEFINE_WRAPPER (struct passwd *, getpwuid, (uid_t uid)) { struct passwd *pw = GET_REAL (getpwuid) (uid); my_pw = *pw; - my_pw.pw_name = NULL; + if (should_modify_result ()) + my_pw.pw_name = NULL; return &my_pw; } @@ -75,7 +87,8 @@ DEFINE_WRAPPER (int, getpwnam_r, (const char *name, struct passwd **result)) { int code = GET_REAL (getpwnam_r) (name, pwd, buf, buflen, result); - pwd->pw_name = NULL; + if (should_modify_result ()) + pwd->pw_name = NULL; return code; } @@ -86,6 +99,7 @@ DEFINE_WRAPPER (int, getpwuid_r, (uid_t uid, struct passwd **result)) { int code = GET_REAL (getpwuid_r) (uid, pwd, buf, buflen, result); - pwd->pw_name = NULL; + if (should_modify_result ()) + pwd->pw_name = NULL; return code; } diff --git a/glib/tests/meson.build b/glib/tests/meson.build index b662c3713..b101b061e 100644 --- a/glib/tests/meson.build +++ b/glib/tests/meson.build @@ -261,7 +261,7 @@ else getpwuid_preload = shared_library('getpwuid-preload', 'getpwuid-preload.c', name_prefix : '', - dependencies: libdl_dep, + dependencies: [libdl_dep, libglib_dep], install_dir : installed_tests_execdir, install_tag : 'tests', install: installed_tests_enabled)