From 9ab1c0eb13f5eacad108341e0616a82e4f36bcec Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Thu, 30 Jan 2025 13:32:37 +0000 Subject: [PATCH 01/15] utils: Factor out _srt_string_ends_with_len() If the length is already known, this avoids recalculating it (and if the suffix contains embedded NULs, this makes it possible to check for). Signed-off-by: Simon McVittie <smcv@collabora.com> --- steam-runtime-tools/utils-internal.h | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/steam-runtime-tools/utils-internal.h b/steam-runtime-tools/utils-internal.h index 732a9837d..a7d8e098c 100644 --- a/steam-runtime-tools/utils-internal.h +++ b/steam-runtime-tools/utils-internal.h @@ -604,6 +604,23 @@ gboolean _srt_string_read_fd_until_eof (GString *buf, int fd, GError **error); +/* + * _srt_string_ends_with_len: + * @str: A #GString + * @suffix: A suffix + * @len: Length of suffix + * + * Returns: %TRUE if @str ends with @suffix + */ +static inline gboolean +_srt_string_ends_with_len (const GString *str, + const char *suffix, + gsize len) +{ + return (str->len >= len + && memcmp (str->str + str->len - len, suffix, len) == 0); +} + /* * _srt_string_ends_with: * @str: A #GString @@ -615,10 +632,7 @@ static inline gboolean _srt_string_ends_with (const GString *str, const char *suffix) { - size_t len = strlen (suffix); - - return (str->len >= len - && strcmp (str->str + str->len - len, suffix) == 0); + return _srt_string_ends_with_len (str, suffix, strlen (suffix)); } gboolean _srt_is_identifier (const char *name); -- GitLab From a5f7ba333bf4bb7390d02ea6238e7bd5e8125d9d Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Thu, 30 Jan 2025 13:47:07 +0000 Subject: [PATCH 02/15] tests: Add some missing coverage We never explicitly tested the case where _srt_string_ends_with() is looking for a suffix longer than the string itself. Signed-off-by: Simon McVittie <smcv@collabora.com> --- tests/utils.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/utils.c b/tests/utils.c index a70beed33..270daf4fc 100644 --- a/tests/utils.c +++ b/tests/utils.c @@ -1578,6 +1578,7 @@ static const struct { { "", 0, "", TRUE }, { "bar", -1, "bar", TRUE }, + { "bar", -1, "bard", FALSE }, { "foobar", -1, "bar", TRUE }, { "foobar", -1, "BAR", FALSE }, { "foo\0bar", 7, "ar", TRUE }, -- GitLab From d47e2e0cf2c3dc028663f331baeeb9f043d6d703 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Thu, 30 Jan 2025 13:04:43 +0000 Subject: [PATCH 03/15] tests: Add missing test coverage for what happens without PvPerArchDirs If we are unable to discover `$LIB` and `$PLATFORM`, or if they do not fit any of the patterns we expect, then we might have to skip reorganising the LD_PRELOAD and LD_AUDIT modules. Test this, to make sure refactoring does not make it regress. Signed-off-by: Simon McVittie <smcv@collabora.com> --- tests/pressure-vessel/adverb-preload.c | 244 +++++++++++++++++++------ 1 file changed, 188 insertions(+), 56 deletions(-) diff --git a/tests/pressure-vessel/adverb-preload.c b/tests/pressure-vessel/adverb-preload.c index aea1c3cd7..fbe595a5d 100644 --- a/tests/pressure-vessel/adverb-preload.c +++ b/tests/pressure-vessel/adverb-preload.c @@ -22,34 +22,50 @@ typedef struct typedef struct { - int unused; + gboolean can_discover_platform; } Config; +static const Config default_config = { + .can_discover_platform = TRUE, +}; + static void setup (Fixture *f, gconstpointer context) { - G_GNUC_UNUSED const Config *config = context; + const Config *config = context; g_autoptr(GError) local_error = NULL; gsize i; + if (config == NULL) + config = &default_config; + f->old_fds = tests_check_fd_leaks_enter (); f->bwrap = flatpak_bwrap_new (flatpak_bwrap_empty_env); - f->lib_temp_dirs = pv_per_arch_dirs_new (&local_error); - if (f->lib_temp_dirs == NULL) + if (config->can_discover_platform) { - g_test_skip (local_error->message); - return; - } + f->lib_temp_dirs = pv_per_arch_dirs_new (&local_error); + + if (f->lib_temp_dirs == NULL) + { + g_test_skip (local_error->message); + return; + } - g_test_message ("Cross-platform module prefix: %s", - f->lib_temp_dirs->libdl_token_path); + g_test_message ("Cross-platform module prefix: %s", + f->lib_temp_dirs->libdl_token_path); - for (i = 0; i < PV_N_SUPPORTED_ARCHITECTURES; i++) - g_test_message ("Concrete path for %s architecture: %s", - pv_multiarch_tuples[i], - f->lib_temp_dirs->abi_paths[i]); + for (i = 0; i < PV_N_SUPPORTED_ARCHITECTURES; i++) + g_test_message ("Concrete path for %s architecture: %s", + pv_multiarch_tuples[i], + f->lib_temp_dirs->abi_paths[i]); + + } + else + { + g_test_message ("Pretending we cannot use $LIB/$PLATFORM"); + } } static void @@ -85,9 +101,6 @@ test_basic (Fixture *f, gboolean ret; gsize i; - if (f->lib_temp_dirs == NULL) - return; - ret = pv_adverb_set_up_preload_modules (f->bwrap, f->lib_temp_dirs, modules, @@ -101,20 +114,35 @@ test_basic (Fixture *f, i = 0; g_string_assign (expected, "LD_AUDIT="); - g_string_append_printf (expected, "%s/libaudit.so", - f->lib_temp_dirs->libdl_token_path); + + if (f->lib_temp_dirs != NULL) + g_string_append_printf (expected, "%s/libaudit.so", + f->lib_temp_dirs->libdl_token_path); + else + g_string_append (expected, "/opt/libaudit.so"); + g_assert_cmpstr (f->bwrap->envp[i], ==, expected->str); i++; /* Order is preserved, independent of whether an ABI is specified */ g_string_assign (expected, "LD_PRELOAD="); - g_string_append_printf (expected, "%s/libpreload.so", - f->lib_temp_dirs->libdl_token_path); + + if (f->lib_temp_dirs != NULL) + g_string_append_printf (expected, "%s/libpreload.so", + f->lib_temp_dirs->libdl_token_path); + else + g_string_append (expected, "/opt/libpreload.so"); + g_string_append_c (expected, ':'); g_string_append (expected, "/opt/unspecified.so"); g_string_append_c (expected, ':'); - g_string_append_printf (expected, "%s/libpreload2.so", - f->lib_temp_dirs->libdl_token_path); + + if (f->lib_temp_dirs != NULL) + g_string_append_printf (expected, "%s/libpreload2.so", + f->lib_temp_dirs->libdl_token_path); + else + g_string_append (expected, "/opt/libpreload2.so"); + g_string_append_c (expected, ':'); g_string_append (expected, "/opt/unspecified2.so"); g_assert_cmpstr (f->bwrap->envp[i], ==, expected->str); @@ -126,6 +154,9 @@ test_basic (Fixture *f, { g_autofree gchar *target = NULL; + if (f->lib_temp_dirs == NULL) + continue; + /* Empty module entries are ignored */ if (modules[i].name[0] == '\0') continue; @@ -172,9 +203,6 @@ test_biarch (Fixture *f, gboolean ret; gsize i; - if (f->lib_temp_dirs == NULL) - return; - ret = pv_adverb_set_up_preload_modules (f->bwrap, f->lib_temp_dirs, modules, @@ -194,8 +222,19 @@ test_biarch (Fixture *f, g_string_assign (expected, "LD_PRELOAD="); g_string_append (expected, "/opt/libpreload.so"); g_string_append_c (expected, ':'); - g_string_append_printf (expected, "%s/libpreload.so", - f->lib_temp_dirs->libdl_token_path); + + if (f->lib_temp_dirs != NULL) + { + g_string_append_printf (expected, "%s/libpreload.so", + f->lib_temp_dirs->libdl_token_path); + } + else + { + g_string_append (expected, "/opt/lib0/libpreload.so"); + g_string_append_c (expected, ':'); + g_string_append (expected, "/opt/lib1/libpreload.so"); + } + g_assert_cmpstr (f->bwrap->envp[i], ==, expected->str); i++; @@ -205,6 +244,9 @@ test_biarch (Fixture *f, { g_autofree gchar *target = NULL; + if (f->lib_temp_dirs == NULL) + continue; + g_string_assign (path, f->lib_temp_dirs->abi_paths[i]); g_string_append_c (path, G_DIR_SEPARATOR); g_string_append (path, "libpreload.so"); @@ -253,9 +295,6 @@ test_gameoverlayrenderer (Fixture *f, G_STATIC_ASSERT (PV_N_SUPPORTED_ARCHITECTURES == 2); - if (f->lib_temp_dirs == NULL) - return; - ret = pv_adverb_set_up_preload_modules (f->bwrap, f->lib_temp_dirs, modules, @@ -271,8 +310,19 @@ test_gameoverlayrenderer (Fixture *f, g_string_assign (expected, "LD_PRELOAD="); g_string_append (expected, "/opt/steam/some-other-abi/gameoverlayrenderer.so"); g_string_append_c (expected, ':'); - g_string_append_printf (expected, "%s/gameoverlayrenderer.so", - f->lib_temp_dirs->libdl_token_path); + + if (f->lib_temp_dirs != NULL) + { + g_string_append_printf (expected, "%s/gameoverlayrenderer.so", + f->lib_temp_dirs->libdl_token_path); + } + else + { + g_string_append (expected, "/opt/steam/ubuntu12_32/gameoverlayrenderer.so"); + g_string_append_c (expected, ':'); + g_string_append (expected, "/opt/steam/ubuntu12_64/gameoverlayrenderer.so"); + } + g_string_append_c (expected, ':'); g_string_append (expected, "/opt/steam/some-other-abi/gameoverlayrenderer.so"); g_assert_cmpstr (f->bwrap->envp[i], ==, expected->str); @@ -284,6 +334,9 @@ test_gameoverlayrenderer (Fixture *f, { g_autofree gchar *target = NULL; + if (f->lib_temp_dirs == NULL) + continue; + g_string_assign (path, f->lib_temp_dirs->abi_paths[i]); g_string_append_c (path, G_DIR_SEPARATOR); g_string_append (path, "gameoverlayrenderer.so"); @@ -347,9 +400,6 @@ test_repetition (Fixture *f, gboolean ret; gsize i; - if (f->lib_temp_dirs == NULL) - return; - ret = pv_adverb_set_up_preload_modules (f->bwrap, f->lib_temp_dirs, modules, @@ -363,30 +413,95 @@ test_repetition (Fixture *f, i = 0; g_string_assign (expected, "LD_PRELOAD="); - g_string_append_printf (expected, "%s/libfirst.so", - f->lib_temp_dirs->libdl_token_path); + + if (f->lib_temp_dirs != NULL) + g_string_append_printf (expected, "%s/libfirst.so", + f->lib_temp_dirs->libdl_token_path); + else + g_string_append (expected, "/opt/lib0/libfirst.so"); + g_string_append_c (expected, ':'); - g_string_append_printf (expected, "%s/same-basename.so", - f->lib_temp_dirs->libdl_token_path); + + if (f->lib_temp_dirs != NULL) + g_string_append_printf (expected, "%s/same-basename.so", + f->lib_temp_dirs->libdl_token_path); + else + g_string_append (expected, "/opt/lib0/one/same-basename.so"); + g_string_append_c (expected, ':'); /* We don't do the per-architecture split if there's a basename * collision */ g_string_append (expected, "/opt/lib0/two/same-basename.so"); g_string_append_c (expected, ':'); - g_string_append_printf (expected, "%s/libpreload.so", - f->lib_temp_dirs->libdl_token_path); + + if (f->lib_temp_dirs != NULL) + { + g_string_append_printf (expected, "%s/libpreload.so", + f->lib_temp_dirs->libdl_token_path); + } + else + { + g_string_append (expected, "/opt/lib0/libpreload.so"); +#if PV_N_SUPPORTED_ARCHITECTURES > 1 + g_string_append_c (expected, ':'); + g_string_append (expected, "/opt/lib1/libpreload.so"); +#endif + } + #if defined(__x86_64__) || defined(__i386__) g_string_append_c (expected, ':'); - g_string_append_printf (expected, "%s/gameoverlayrenderer.so", - f->lib_temp_dirs->libdl_token_path); + + if (f->lib_temp_dirs != NULL) + { + g_string_append_printf (expected, "%s/gameoverlayrenderer.so", + f->lib_temp_dirs->libdl_token_path); + } + else + { + g_string_append (expected, "/opt/steam/ubuntu12_32/gameoverlayrenderer.so"); + g_string_append_c (expected, ':'); + g_string_append (expected, "/opt/steam/ubuntu12_64/gameoverlayrenderer.so"); + } #endif + g_string_append_c (expected, ':'); - g_string_append_printf (expected, "%s/libmiddle.so", - f->lib_temp_dirs->libdl_token_path); + + if (f->lib_temp_dirs != NULL) + g_string_append_printf (expected, "%s/libmiddle.so", + f->lib_temp_dirs->libdl_token_path); + else + g_string_append (expected, "/opt/lib0/libmiddle.so"); + g_string_append_c (expected, ':'); - /* The duplicates don't appear in the search path a second time */ - g_string_append_printf (expected, "%s/liblast.so", - f->lib_temp_dirs->libdl_token_path); + + if (f->lib_temp_dirs != NULL) + { + /* If we are able to split up the modules by architecture, + * then the duplicates don't appear in the search path a second time */ + g_string_append_printf (expected, "%s/liblast.so", + f->lib_temp_dirs->libdl_token_path); + } + else + { + /* If we were unable to split up the modules by architecture, + * we change as little as possible, so in this case we do not + * deduplicate */ + g_string_append (expected, "/opt/lib0/libpreload.so"); + g_string_append_c (expected, ':'); +#if PV_N_SUPPORTED_ARCHITECTURES > 1 + g_string_append (expected, "/opt/lib1/libpreload.so"); + g_string_append_c (expected, ':'); +#endif +#if defined(__x86_64__) || defined(__i386__) + g_string_append (expected, "/opt/steam/ubuntu12_32/gameoverlayrenderer.so"); + g_string_append_c (expected, ':'); + g_string_append (expected, "/opt/steam/ubuntu12_64/gameoverlayrenderer.so"); + g_string_append_c (expected, ':'); +#endif + + g_string_append (expected, "/opt/lib0/liblast.so"); + } + g_assert_cmpstr (f->bwrap->envp[i], ==, expected->str); i++; @@ -398,6 +513,9 @@ test_repetition (Fixture *f, { gsize j; + if (f->lib_temp_dirs == NULL) + continue; + for (j = 0; j < G_N_ELEMENTS (modules); j++) { g_autofree gchar *target = NULL; @@ -434,6 +552,9 @@ test_repetition (Fixture *f, { g_autofree gchar *target = NULL; + if (f->lib_temp_dirs == NULL) + continue; + g_string_assign (path, f->lib_temp_dirs->abi_paths[i]); g_string_append_c (path, G_DIR_SEPARATOR); g_string_append (path, "gameoverlayrenderer.so"); @@ -450,6 +571,10 @@ test_repetition (Fixture *f, #endif } +static const Config cannot_discover_platform = { + .can_discover_platform = FALSE, +}; + int main (int argc, char **argv) @@ -462,14 +587,21 @@ main (int argc, g_setenv ("PRESSURE_VESSEL_TEST_STANDARDIZE_PLATFORM", "1", TRUE); _srt_tests_init (&argc, &argv, NULL); - g_test_add ("/basic", Fixture, NULL, - setup, test_basic, teardown); - g_test_add ("/biarch", Fixture, NULL, - setup, test_biarch, teardown); - g_test_add ("/gameoverlayrenderer", Fixture, NULL, - setup, test_gameoverlayrenderer, teardown); - g_test_add ("/repetition", Fixture, NULL, - setup, test_repetition, teardown); + +#define add_test(name, function) \ + do { \ + g_test_add (name, \ + Fixture, NULL, \ + setup, function, teardown); \ + g_test_add (name "/cannot-discover-platform", \ + Fixture, &cannot_discover_platform, \ + setup, function, teardown); \ + } while (0) + + add_test ("/basic", test_basic); + add_test ("/biarch", test_biarch); + add_test ("/gameoverlayrenderer", test_gameoverlayrenderer); + add_test ("/repetition", test_repetition); return g_test_run (); } -- GitLab From c08dc25004398c575f57e6f5dad400c6f878826d Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Fri, 31 Jan 2025 11:48:35 +0000 Subject: [PATCH 04/15] tests: Ensure we have better coverage if built with --werror Release builds are not done with --werror, so we will gracefully skip various parts of the build if we are running on a particularly strange platform (where /dev/pts isn't a directory, /dev/stderr isn't a symlink, or we are unable to expand `${LIB}` and `${PLATFORM}`). But for our official CI and individual developers' builds, we expect the build to be done on a platform that makes sense, so we can be more strict. Signed-off-by: Simon McVittie <smcv@collabora.com> --- config.h.in | 1 + meson.build | 7 +++++++ tests/pressure-vessel/adverb-preload.c | 7 +++++++ tests/pressure-vessel/adverb-sdl.c | 7 +++++++ tests/utils.c | 10 ++++++++++ 5 files changed, 32 insertions(+) diff --git a/config.h.in b/config.h.in index 27b06c306..016f10f0b 100644 --- a/config.h.in +++ b/config.h.in @@ -7,6 +7,7 @@ #define _SRT_SONAME "libsteam-runtime-tools-@api_major@.so.@abi_major" #define _GNU_SOURCE 1 #mesondefine _SRT_MULTIARCH +#mesondefine _SRT_TESTS_STRICT #mesondefine VERSION /* Allow using stuff from Flatpak with minimal modifications */ diff --git a/meson.build b/meson.build index fcf4b0e60..346cc565f 100644 --- a/meson.build +++ b/meson.build @@ -262,10 +262,17 @@ elif host_machine.cpu_family() == 'x86' multiarch = 'i386-linux-gnu' elif host_machine.cpu_family() == 'aarch64' and host_machine.endian() == 'little' multiarch = 'aarch64-linux-gnu' +elif get_option('werror') + # Some of our test coverage is skipped if we don't know the multiarch tuple + error('-Dmultiarch_tuple is required when building with --werror on an unknown CPU') else multiarch = '' endif +if get_option('werror') + conf_data.set('_SRT_TESTS_STRICT', true) +endif + if multiarch != '' conf_data.set_quoted('_SRT_MULTIARCH', multiarch) endif diff --git a/tests/pressure-vessel/adverb-preload.c b/tests/pressure-vessel/adverb-preload.c index fbe595a5d..6078455a0 100644 --- a/tests/pressure-vessel/adverb-preload.c +++ b/tests/pressure-vessel/adverb-preload.c @@ -46,6 +46,13 @@ setup (Fixture *f, if (config->can_discover_platform) { f->lib_temp_dirs = pv_per_arch_dirs_new (&local_error); +#ifdef _SRT_TESTS_STRICT + /* We allow this to fail because it might fail on particularly strange + * OS configurations, but for platforms we actively support, + * we expect it to work */ + g_assert_no_error (local_error); + g_assert_nonnull (f->lib_temp_dirs); +#endif if (f->lib_temp_dirs == NULL) { diff --git a/tests/pressure-vessel/adverb-sdl.c b/tests/pressure-vessel/adverb-sdl.c index d6fde6ef8..207f3c4b8 100644 --- a/tests/pressure-vessel/adverb-sdl.c +++ b/tests/pressure-vessel/adverb-sdl.c @@ -43,6 +43,13 @@ setup (Fixture *f, f->old_fds = tests_check_fd_leaks_enter (); f->bwrap = flatpak_bwrap_new (flatpak_bwrap_empty_env); f->lib_temp_dirs = pv_per_arch_dirs_new (&f->lib_temp_dirs_error); +#ifdef _SRT_TESTS_STRICT + /* We allow this to fail because it might fail on particularly strange + * OS configurations, but for platforms we actively support, + * we expect it to work */ + g_assert_no_error (f->lib_temp_dirs_error); + g_assert_nonnull (f->lib_temp_dirs); +#endif if (f->lib_temp_dirs != NULL) { diff --git a/tests/utils.c b/tests/utils.c index 270daf4fc..8fbb2e913 100644 --- a/tests/utils.c +++ b/tests/utils.c @@ -1434,6 +1434,11 @@ test_recursive_list (Fixture *f, } else { +#ifdef _SRT_TESTS_STRICT + /* We expect this to exist in our official CI, + * so make the test fail if it doesn't. */ + g_warning ("/dev/pts doesn't exist or isn't a directory"); +#endif /* This could conceivably be false in some containers. * Mark the test as skipped but intentionally don't early-return * here: we can still check for /dev/stderr. */ @@ -1450,6 +1455,11 @@ test_recursive_list (Fixture *f, } else { +#ifdef _SRT_TESTS_STRICT + /* We expect this to exist in our official CI, + * so make the test fail if it doesn't. */ + g_warning ("/dev/stderr isn't a symlink"); +#endif /* This could conceivably be false in some containers. * Again, intentionally not early-returning here. */ g_test_skip ("/dev/stderr isn't a symlink"); -- GitLab From 21f1fddaeda9f59777c674aee26210de0a3ad91a Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Fri, 31 Jan 2025 11:49:38 +0000 Subject: [PATCH 05/15] tests: When building with --werror, make assertions about /etc/os-release We can expect and assume that on our official CI or on developer machines, we'll be running in a reasonable environment. Signed-off-by: Simon McVittie <smcv@collabora.com> --- tests/system-info.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/system-info.c b/tests/system-info.c index 0dac96383..1c5a8e21b 100644 --- a/tests/system-info.c +++ b/tests/system-info.c @@ -2119,6 +2119,28 @@ os_no_os_release (Fixture *f, g_free (sysroot); } +#ifdef _SRT_TESTS_STRICT +static void +os_real (Fixture *f, + gconstpointer context) +{ + g_autoptr(SrtSystemInfo) info = NULL; + gchar *s; + + info = srt_system_info_new (NULL); + check_os_release (info); + + /* We expect the ID and NAME to be present on any reasonable OS. */ + s = srt_system_info_dup_os_id (info); + g_assert_cmpstr (s, !=, NULL); + g_free (s); + + s = srt_system_info_dup_os_name (info); + g_assert_cmpstr (s, !=, NULL); + g_free (s); +} +#endif + static void overrides (Fixture *f, gconstpointer context) @@ -4505,6 +4527,10 @@ main (int argc, setup, os_invalid_os_release, teardown); g_test_add ("/system-info/os/no-os-release", Fixture, NULL, setup, os_no_os_release, teardown); +#ifdef _SRT_TESTS_STRICT + g_test_add ("/system-info/os/real", Fixture, NULL, + setup, os_real, teardown); +#endif g_test_add ("/system-info/libs/overrides", Fixture, NULL, setup, overrides, teardown); -- GitLab From bcdfdab897ba2a800f943ef095ac5d0243d3ae36 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Tue, 28 Jan 2025 20:36:50 +0000 Subject: [PATCH 06/15] tests: Future-proof PvAdverbPreloadModule initializers This will let the test continue to pass without code changes when we add new struct members, as long as zero/NULL is a valid default value for those members, reducing the diffstat for refactoring in this area. Signed-off-by: Simon McVittie <smcv@collabora.com> --- tests/pressure-vessel/adverb-preload.c | 174 +++++++++++++++++++------ 1 file changed, 135 insertions(+), 39 deletions(-) diff --git a/tests/pressure-vessel/adverb-preload.c b/tests/pressure-vessel/adverb-preload.c index 6078455a0..a50fc2a01 100644 --- a/tests/pressure-vessel/adverb-preload.c +++ b/tests/pressure-vessel/adverb-preload.c @@ -92,15 +92,41 @@ test_basic (Fixture *f, { static const PvAdverbPreloadModule modules[] = { - { (char *) "", PV_PRELOAD_VARIABLE_INDEX_LD_AUDIT, 0 }, - { (char *) "/opt/libaudit.so", PV_PRELOAD_VARIABLE_INDEX_LD_AUDIT, 0 }, - { (char *) "", PV_PRELOAD_VARIABLE_INDEX_LD_AUDIT, PV_UNSPECIFIED_ABI }, - { (char *) "/opt/libpreload.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, 0 }, - { (char *) "/opt/unspecified.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, - PV_UNSPECIFIED_ABI }, - { (char *) "/opt/libpreload2.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, 0 }, - { (char *) "/opt/unspecified2.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, - PV_UNSPECIFIED_ABI }, + { + .name = (char *) "", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_AUDIT, + .abi_index = 0, + }, + { + .name = (char *) "/opt/libaudit.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_AUDIT, + .abi_index = 0, + }, + { + .name = (char *) "", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_AUDIT, + .abi_index = PV_UNSPECIFIED_ABI, + }, + { + .name = (char *) "/opt/libpreload.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = 0, + }, + { + .name = (char *) "/opt/unspecified.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = PV_UNSPECIFIED_ABI, + }, + { + .name = (char *) "/opt/libpreload2.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = 0, + }, + { + .name = (char *) "/opt/unspecified2.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = PV_UNSPECIFIED_ABI, + }, }; g_autoptr(GError) local_error = NULL; g_autoptr(GString) expected = g_string_new (""); @@ -198,11 +224,23 @@ test_biarch (Fixture *f, #if PV_N_SUPPORTED_ARCHITECTURES >= 2 static const PvAdverbPreloadModule modules[] = { - { (char *) "/opt/libpreload.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, PV_UNSPECIFIED_ABI }, + { + .name = (char *) "/opt/libpreload.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = PV_UNSPECIFIED_ABI, + }, /* In practice x86_64-linux-gnu */ - { (char *) "/opt/lib0/libpreload.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, 0 }, + { + .name = (char *) "/opt/lib0/libpreload.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = 0, + }, /* In practice i386-linux-gnu */ - { (char *) "/opt/lib1/libpreload.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, 1 }, + { + .name = (char *) "/opt/lib1/libpreload.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = 1, + }, }; g_autoptr(GError) local_error = NULL; g_autoptr(GString) expected = g_string_new (""); @@ -285,14 +323,26 @@ test_gameoverlayrenderer (Fixture *f, #if defined(__x86_64__) || defined(__i386__) static const PvAdverbPreloadModule modules[] = { - { (char *) "/opt/steam/some-other-abi/gameoverlayrenderer.so", - PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, PV_UNSPECIFIED_ABI }, - { (char *) "/opt/steam/ubuntu12_32/gameoverlayrenderer.so", - PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, PV_UNSPECIFIED_ABI }, - { (char *) "/opt/steam/ubuntu12_64/gameoverlayrenderer.so", - PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, PV_UNSPECIFIED_ABI }, - { (char *) "/opt/steam/some-other-abi/gameoverlayrenderer.so", - PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, PV_UNSPECIFIED_ABI }, + { + .name = (char *) "/opt/steam/some-other-abi/gameoverlayrenderer.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = PV_UNSPECIFIED_ABI, + }, + { + .name = (char *) "/opt/steam/ubuntu12_32/gameoverlayrenderer.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = PV_UNSPECIFIED_ABI, + }, + { + .name = (char *) "/opt/steam/ubuntu12_64/gameoverlayrenderer.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = PV_UNSPECIFIED_ABI, + }, + { + .name = (char *) "/opt/steam/some-other-abi/gameoverlayrenderer.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = PV_UNSPECIFIED_ABI, + }, }; g_autoptr(GError) local_error = NULL; g_autoptr(GString) expected = g_string_new (""); @@ -373,33 +423,79 @@ test_repetition (Fixture *f, { static const PvAdverbPreloadModule modules[] = { - { (char *) "/opt/lib0/libfirst.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, 0 }, - { (char *) "/opt/lib0/one/same-basename.so", - PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, 0 }, - { (char *) "/opt/lib0/two/same-basename.so", - PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, 0 }, - { (char *) "/opt/lib0/libpreload.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, 0 }, + { + .name = (char *) "/opt/lib0/libfirst.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = 0, + }, + { + .name = (char *) "/opt/lib0/one/same-basename.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = 0, + }, + { + .name = (char *) "/opt/lib0/two/same-basename.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = 0, + }, + { + .name = (char *) "/opt/lib0/libpreload.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = 0, + }, #if PV_N_SUPPORTED_ARCHITECTURES > 1 - { (char *) "/opt/lib1/libpreload.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, 1 }, + { + .name = (char *) "/opt/lib1/libpreload.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = 1, + }, #endif #if defined(__x86_64__) || defined(__i386__) - { (char *) "/opt/steam/ubuntu12_32/gameoverlayrenderer.so", - PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, PV_UNSPECIFIED_ABI }, - { (char *) "/opt/steam/ubuntu12_64/gameoverlayrenderer.so", - PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, PV_UNSPECIFIED_ABI }, + { + .name = (char *) "/opt/steam/ubuntu12_32/gameoverlayrenderer.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = PV_UNSPECIFIED_ABI, + }, + { + .name = (char *) "/opt/steam/ubuntu12_64/gameoverlayrenderer.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = PV_UNSPECIFIED_ABI, + }, #endif - { (char *) "/opt/lib0/libmiddle.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, 0 }, - { (char *) "/opt/lib0/libpreload.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, 0 }, + { + .name = (char *) "/opt/lib0/libmiddle.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = 0, + }, + { + .name = (char *) "/opt/lib0/libpreload.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = 0, + }, #if PV_N_SUPPORTED_ARCHITECTURES > 1 - { (char *) "/opt/lib1/libpreload.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, 1 }, + { + .name = (char *) "/opt/lib1/libpreload.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = 1, + }, #endif #if defined(__x86_64__) || defined(__i386__) - { (char *) "/opt/steam/ubuntu12_32/gameoverlayrenderer.so", - PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, PV_UNSPECIFIED_ABI }, - { (char *) "/opt/steam/ubuntu12_64/gameoverlayrenderer.so", - PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, PV_UNSPECIFIED_ABI }, + { + .name = (char *) "/opt/steam/ubuntu12_32/gameoverlayrenderer.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = PV_UNSPECIFIED_ABI, + }, + { + .name = (char *) "/opt/steam/ubuntu12_64/gameoverlayrenderer.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = PV_UNSPECIFIED_ABI, + }, #endif - { (char *) "/opt/lib0/liblast.so", PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, 0 }, + { + .name = (char *) "/opt/lib0/liblast.so", + .index_in_preload_variables = PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + .abi_index = 0, + }, }; g_autoptr(GError) local_error = NULL; g_autoptr(GString) expected = g_string_new (""); -- GitLab From 0f9b4eefbb5f57f29bf1c1cce7ca5bde60cd7749 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Tue, 28 Jan 2025 18:26:22 +0000 Subject: [PATCH 07/15] pv-wrap: Remove redundant reinitialization of SrtSystemInfo Instantiating this can't fail, and if it somehow does, trying it again is unlikely to help. Signed-off-by: Simon McVittie <smcv@collabora.com> --- pressure-vessel/wrap-setup.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/pressure-vessel/wrap-setup.c b/pressure-vessel/wrap-setup.c index 2dc46ccf3..a185fd1ac 100644 --- a/pressure-vessel/wrap-setup.c +++ b/pressure-vessel/wrap-setup.c @@ -712,9 +712,6 @@ append_preload_per_architecture (GPtrArray *argv, g_autoptr(SrtSystemInfo) system_info = srt_system_info_new (NULL); gsize i; - if (system_info == NULL) - system_info = srt_system_info_new (NULL); - for (i = 0; i < PV_N_SUPPORTED_ARCHITECTURES; i++) { g_autoptr(GString) mock_path = NULL; -- GitLab From 575afde01a1280200721cb566aacc2db71adacb4 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Fri, 31 Jan 2025 12:40:01 +0000 Subject: [PATCH 08/15] build-aux: Add convenience files to cross-compile for arm64 This is a useful way to check that non-x86 builds aren't regressing, without actually needing a non-x86 device. Not all tests will pass in this configuration because qemu-user is not a perfect emulation of a real aarch64 machine (for example PR_GET_CHILD_SUBREAPER is unimplemented), but it's better than nothing. In particular, the adverb-preload and wrap-setup tests pass, and those are the ones I'm altering for steamrt/tasks#595. Signed-off-by: Simon McVittie <smcv@collabora.com> --- build-aux/many-builds.py | 13 +++++++++++++ build-aux/meson/arm64.txt | 41 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 build-aux/meson/arm64.txt diff --git a/build-aux/many-builds.py b/build-aux/many-builds.py index 9b72d979c..abbd5449a 100755 --- a/build-aux/many-builds.py +++ b/build-aux/many-builds.py @@ -53,6 +53,8 @@ _build/ coverage/ Build for host system with coverage doc/ Build for host system with gtk-doc and pandoc host-no-asan/ No AddressSanitizer, for use with valgrind + arm64/ Build for host system for arm64, as an + example of a non-x86 platform i386/ Build for host system for i386 """ @@ -298,6 +300,17 @@ class Environment: ] + args, ) + self.setup_one( + 'arm64', + dev_build + [ + '-Dintrospection=disabled', + '-Dmultiarch_tuple=aarch64-linux-gnu', + '--cross-file=build-aux/meson/arm64.txt', + ] + args, + # Host system doesn't necessarily have an arm64 toolchain + check=False, + ) + self.setup_one( 'i386', asan_dev_build + [ diff --git a/build-aux/meson/arm64.txt b/build-aux/meson/arm64.txt new file mode 100644 index 000000000..afa1aed2f --- /dev/null +++ b/build-aux/meson/arm64.txt @@ -0,0 +1,41 @@ +# Copyright 2022-2025 Collabora Ltd. +# SPDX-License-Identifier: MIT +# +# Loosely based on the output of: +# /usr/share/meson/debcrossgen --arch arm64 + +[constants] +DEB_HOST_GNU_TYPE = 'aarch64-linux-gnu' +DEB_HOST_MULTIARCH = 'aarch64-linux-gnu' + +[binaries] +c = DEB_HOST_GNU_TYPE + '-gcc' +cpp = DEB_HOST_GNU_TYPE + '-g++' +ar = DEB_HOST_GNU_TYPE + '-ar' +strip = DEB_HOST_GNU_TYPE + '-strip' +objcopy = DEB_HOST_GNU_TYPE + '-objcopy' +ld = DEB_HOST_GNU_TYPE + '-ld' +pkgconfig = DEB_HOST_GNU_TYPE + '-pkg-config' +pkg-config = DEB_HOST_GNU_TYPE + '-pkg-config' +cmake = 'cmake' +cups-config = 'cups-config' +sdl2-config = 'sdl2-config' + +[built-in options] +libdir = 'lib' / DEB_HOST_MULTIARCH + +[properties] +# This assumes that if we will run tests, we have qemu-static installed +needs_exe_wrapper = false + +[cmake] +CMAKE_C_COMPILER = DEB_HOST_GNU_TYPE + '-gcc' +CMAKE_CXX_COMPILER = DEB_HOST_GNU_TYPE + '-g++' +CMAKE_SYSTEM_NAME = 'Linux' +CMAKE_SYSTEM_PROCESSOR = 'aarch64' + +[host_machine] +system = 'linux' +cpu_family = 'aarch64' +cpu = 'arm64' +endian = 'little' -- GitLab From 4524f7a603c404f351e5a5d3b14f523b0fba4ee5 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Fri, 31 Jan 2025 12:52:33 +0000 Subject: [PATCH 09/15] pressure-vessel: Avoid looping through zero-length array of emulator hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If compiled for a non-x86 architecture, we don't expect to be emulated by some foreign architecture with a mechanism like FEX or qemu-user, so PV_N_SUPPORTED_ARCHITECTURES_AS_EMULATOR_HOST is zero. In this case don't try to allocate and loop over an array of length 0. This also avoids gcc warnings with -Wtype-limits, "comparison of unsigned expression in ‘< 0’ is always false". Signed-off-by: Simon McVittie <smcv@collabora.com> --- pressure-vessel/runtime.c | 12 ++++++++++++ tests/pressure-vessel/wrap-setup.c | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/pressure-vessel/runtime.c b/pressure-vessel/runtime.c index ef0f58819..408194767 100644 --- a/pressure-vessel/runtime.c +++ b/pressure-vessel/runtime.c @@ -96,7 +96,9 @@ struct _PvRuntime PvGraphicsProvider *interpreter_host_provider; EnumerationThread indep_thread; EnumerationThread host_thread; +#if PV_N_SUPPORTED_ARCHITECTURES_AS_EMULATOR_HOST > 0 EnumerationThread *arch_host_threads; +#endif EnumerationThread *arch_threads; SrtDirentCompareFunc arbitrary_dirent_order; GCompareFunc arbitrary_str_order; @@ -1519,6 +1521,7 @@ pv_runtime_initable_init (GInitable *initable, self->interpreter_host_provider, "real-host"); +#if PV_N_SUPPORTED_ARCHITECTURES_AS_EMULATOR_HOST > 0 self->arch_host_threads = g_new0 (EnumerationThread, PV_N_SUPPORTED_ARCHITECTURES_AS_EMULATOR_HOST); @@ -1527,6 +1530,7 @@ pv_runtime_initable_init (GInitable *initable, &pv_multiarch_as_emulator_details[i], self->flags, self->interpreter_host_provider); +#endif } self->arch_threads = g_new0 (EnumerationThread, @@ -1857,8 +1861,10 @@ pv_runtime_dispose (GObject *object) g_clear_object (&self->host_root); enumeration_thread_clear (&self->indep_thread); enumeration_thread_clear (&self->host_thread); +#if PV_N_SUPPORTED_ARCHITECTURES_AS_EMULATOR_HOST > 0 enumeration_threads_clear (&self->arch_host_threads, PV_N_SUPPORTED_ARCHITECTURES_AS_EMULATOR_HOST); +#endif enumeration_threads_clear (&self->arch_threads, PV_N_SUPPORTED_ARCHITECTURES); @@ -7101,6 +7107,7 @@ collect_dri_drivers (PvRuntime *self, return TRUE; } +#if PV_N_SUPPORTED_ARCHITECTURES_AS_EMULATOR_HOST > 0 /* * @search_path: (inout): */ @@ -7174,6 +7181,7 @@ pv_append_host_dri_library_paths (PvRuntime *self, while (_srt_hash_table_iter_next (&iter, &drivers_path, NULL)) pv_search_path_append (search_path, drivers_path); } +#endif /* * Returns: (element-type IcdDetails): @@ -7495,12 +7503,14 @@ pv_runtime_use_provider_graphics_stack (PvRuntime *self, if (self->flags & PV_RUNTIME_FLAGS_SINGLE_THREAD) { system_info = pv_graphics_provider_create_system_info (self->provider); + if (self->interpreter_host_provider != NULL) host_system_info = pv_graphics_provider_create_system_info (self->interpreter_host_provider); } else { system_info = g_object_ref (enumeration_thread_join (&self->indep_thread)); + if (self->interpreter_host_provider != NULL) host_system_info = g_object_ref (enumeration_thread_join (&self->host_thread)); } @@ -7781,6 +7791,7 @@ pv_runtime_use_provider_graphics_stack (PvRuntime *self, { g_assert (pv_multiarch_as_emulator_tuples[PV_N_SUPPORTED_ARCHITECTURES_AS_EMULATOR_HOST] == NULL); +#if PV_N_SUPPORTED_ARCHITECTURES_AS_EMULATOR_HOST > 0 for (i = 0; i < PV_N_SUPPORTED_ARCHITECTURES_AS_EMULATOR_HOST; i++) { SrtSystemInfo *arch_system_info; @@ -7795,6 +7806,7 @@ pv_runtime_use_provider_graphics_stack (PvRuntime *self, pv_multiarch_as_emulator_tuples[i], dri_path); } +#endif } part_timer = _srt_profiling_start ("Finishing graphics stack capture"); diff --git a/tests/pressure-vessel/wrap-setup.c b/tests/pressure-vessel/wrap-setup.c index d5cf0330e..04b52a989 100644 --- a/tests/pressure-vessel/wrap-setup.c +++ b/tests/pressure-vessel/wrap-setup.c @@ -1911,6 +1911,7 @@ test_supported_archs (Fixture *f, /* The array of tuples has one extra element, to make it a GStrv */ g_assert_cmpstr (pv_multiarch_tuples[i], ==, NULL); +#if PV_N_SUPPORTED_ARCHITECTURES_AS_EMULATOR_HOST > 0 /* Emulator host details and tuples are also in the same order */ for (i = 0; i < PV_N_SUPPORTED_ARCHITECTURES_AS_EMULATOR_HOST; i++) { @@ -1918,6 +1919,9 @@ test_supported_archs (Fixture *f, g_assert_cmpstr (pv_multiarch_as_emulator_tuples[i], ==, details->tuple); } +#else + i = 0; +#endif /* Array of tuples has one extra element, again */ g_assert_cmpstr (pv_multiarch_as_emulator_tuples[i], ==, NULL); -- GitLab From 0103ed2d0f04d19488a38be93eca49322aa5b777 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Fri, 31 Jan 2025 13:25:22 +0000 Subject: [PATCH 10/15] tests: Combine expected test results for x86_64 and non-x86 There are three differences between our handling of x86 and non-x86 here: 1. On x86, x86_64 is always treated as the primary architecture, with i386 as secondary (even if pressure-vessel itself is compiled as an i386 binary). On other platforms, the native platform is primary. 2. On x86, i386 is available as a secondary architecture. On other platforms, there its no secondary architecture. 3. We have different mock expansions for `${LIB}`, `${PLATFORM}` and the multiarch tuple on x86 and non-x86. By abstracting the "primary architecture", we can avoid having lines of test code that are only run on non-x86 platforms, which in practice means they are not routinely run at all. This makes them less likely to regress. Signed-off-by: Simon McVittie <smcv@collabora.com> --- tests/pressure-vessel/wrap-setup.c | 64 +++++++++++++++++++----------- 1 file changed, 40 insertions(+), 24 deletions(-) diff --git a/tests/pressure-vessel/wrap-setup.c b/tests/pressure-vessel/wrap-setup.c index 04b52a989..d22205181 100644 --- a/tests/pressure-vessel/wrap-setup.c +++ b/tests/pressure-vessel/wrap-setup.c @@ -58,6 +58,24 @@ #define MOCK_LIB_64 "lib/" SRT_ABI_X86_64 #define MOCK_LIB_GENERIC "lib/" MOCK_ABI +#if defined(__i386__) || defined(__x86_64__) + /* On x86, we treat x86_64 as the primary architecture. + * This means it's the first one whenever we have a list of + * per-architecture things, and if we pretend to only support + * one architecture for test coverage purposes, that architecture + * will be x86_64. */ +# define PRIMARY_ABI SRT_ABI_X86_64 +# define PRIMARY_PLATFORM MOCK_PLATFORM_64 +# define PRIMARY_LIB MOCK_LIB_64 +#else + /* On non-x86, the mock implementation of srt_system_info_check_library() + * uses these expansions for ${PLATFORM} and ${LIB} instead of the + * real ones. */ +# define PRIMARY_ABI MOCK_ABI +# define PRIMARY_PLATFORM MOCK_PLATFORM_GENERIC +# define PRIMARY_LIB MOCK_LIB_GENERIC +#endif + typedef struct { TestsOpenFdSet old_fds; @@ -242,20 +260,23 @@ setup_ld_preload (Fixture *f, "steam/lib/gameoverlayrenderer.so", "usr/lib/libpreloadU.so", "usr/local/lib/libgtk3-nocsd.so.0", + "opt/" PRIMARY_LIB "/libpreloadL.so", + "platform/plat-" PRIMARY_PLATFORM "/libpreloadP.so", + }; #if defined(__i386__) || defined(__x86_64__) + static const char * const touch_i386[] = + { "opt/" MOCK_LIB_32 "/libpreloadL.so", - "opt/" MOCK_LIB_64 "/libpreloadL.so", "platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so", - "platform/plat-" MOCK_PLATFORM_64 "/libpreloadP.so", "in-root-plat-" MOCK_PLATFORM_32 "-only-32-bit.so", -#else - "opt/" MOCK_LIB_GENERIC "/libpreloadL.so", - "platform/plat-" MOCK_PLATFORM_GENERIC "/libpreloadP.so", -#endif }; +#endif setup (f, context); fixture_populate_dir (f, f->mock_host->fd, touch, G_N_ELEMENTS (touch)); +#if defined(__i386__) || defined(__x86_64__) + fixture_populate_dir (f, f->mock_host->fd, touch_i386, G_N_ELEMENTS (touch_i386)); +#endif f->env = g_environ_setenv (f->env, "STEAM_COMPAT_CLIENT_INSTALL_PATH", "/steam", TRUE); } @@ -1608,14 +1629,13 @@ populate_ld_preload (Fixture *f, static const char * const expected_preload_paths[] = { "/app/lib/libpreloadA.so", + "/platform/plat-" PRIMARY_PLATFORM "/libpreloadP.so:abi=" PRIMARY_ABI, #if defined(__i386__) || defined(__x86_64__) - "/platform/plat-" MOCK_PLATFORM_64 "/libpreloadP.so:abi=" SRT_ABI_X86_64, "/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so:abi=" SRT_ABI_I386, - "/opt/" MOCK_LIB_64 "/libpreloadL.so:abi=" SRT_ABI_X86_64, +#endif + "/opt/" PRIMARY_LIB "/libpreloadL.so:abi=" PRIMARY_ABI, +#if defined(__i386__) || defined(__x86_64__) "/opt/" MOCK_LIB_32 "/libpreloadL.so:abi=" SRT_ABI_I386, -#else - "/platform/plat-" MOCK_PLATFORM_GENERIC "/libpreloadP.so:abi=" MOCK_ABI, - "/opt/" MOCK_LIB_GENERIC "/libpreloadL.so:abi=" MOCK_ABI, #endif "/lib/libpreload-rootfs.so", "/usr/lib/libpreloadU.so", @@ -1632,11 +1652,9 @@ static const char * const expected_preload_paths[] = /* Our mock implementation of pv_runtime_has_library() behaves as though * libfakeroot is not in the runtime or graphics stack provider, only * the current namespace */ + "/path/to/" PRIMARY_LIB "/libfakeroot.so:abi=" PRIMARY_ABI, #if defined(__i386__) || defined(__x86_64__) - "/path/to/" MOCK_LIB_64 "/libfakeroot.so:abi=" SRT_ABI_X86_64, "/path/to/" MOCK_LIB_32 "/libfakeroot.so:abi=" SRT_ABI_I386, -#else - "/path/to/" MOCK_LIB_GENERIC "/libfakeroot.so:abi=" MOCK_ABI, #endif /* Our mock implementation of pv_runtime_has_library() behaves as though * libpthread.so.0 *is* in the runtime, as we would expect */ @@ -1688,14 +1706,13 @@ test_remap_ld_preload (Fixture *f, g_assert_false (flatpak_exports_path_is_visible (exports, "/opt")); g_assert_false (flatpak_exports_path_is_visible (exports, "/opt/lib")); g_assert_false (flatpak_exports_path_is_visible (exports, "/platform")); + + g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" PRIMARY_LIB "/libpreloadL.so")); + g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" PRIMARY_PLATFORM "/libpreloadP.so")); + #if defined(__i386__) || defined(__x86_64__) g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_32 "/libpreloadL.so")); - g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_64 "/libpreloadL.so")); g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so")); - g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_64 "/libpreloadP.so")); -#else - g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_GENERIC "/libpreloadL.so")); - g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_GENERIC "/libpreloadP.so")); #endif /* FlatpakExports never exports /lib as /lib */ @@ -1809,14 +1826,13 @@ test_remap_ld_preload_no_runtime (Fixture *f, g_assert_false (flatpak_exports_path_is_visible (exports, "/opt")); g_assert_false (flatpak_exports_path_is_visible (exports, "/opt/lib")); g_assert_false (flatpak_exports_path_is_visible (exports, "/platform")); + + g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" PRIMARY_LIB "/libpreloadL.so")); + g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" PRIMARY_PLATFORM "/libpreloadP.so")); + #if defined(__i386__) || defined(__x86_64__) g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_32 "/libpreloadL.so")); - g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_64 "/libpreloadL.so")); g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so")); - g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_64 "/libpreloadP.so")); -#else - g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_GENERIC "/libpreloadL.so")); - g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_GENERIC "/libpreloadP.so")); #endif /* FlatpakExports never exports /lib as /lib */ -- GitLab From be7d6382b13d4310efdcea170f8c91e1609a143e Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Fri, 31 Jan 2025 13:27:05 +0000 Subject: [PATCH 11/15] pv-wrap: Document PV_APPEND_PRELOAD_FLAGS_IN_UNIT_TESTS Fixes: 08751fff "tests: Assert that modules with ${LIB} or ${PLATFORM} are handled" Signed-off-by: Simon McVittie <smcv@collabora.com> --- pressure-vessel/wrap-setup.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pressure-vessel/wrap-setup.h b/pressure-vessel/wrap-setup.h index a690e3cbf..ceb71fc69 100644 --- a/pressure-vessel/wrap-setup.h +++ b/pressure-vessel/wrap-setup.h @@ -66,6 +66,8 @@ void pv_wrap_move_into_scope (const char *steam_app_id); * @PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX: The game will be run in * a Flatpak subsandbox * @PV_APPEND_PRELOAD_FLAGS_REMOVE_GAME_OVERLAY: Disable the Steam Overlay + * @PV_APPEND_PRELOAD_FLAGS_IN_UNIT_TESTS: Normalize $LIB and $PLATFORM, + * for unit testing * @PV_APPEND_PRELOAD_FLAGS_NONE: None of the above * * Flags affecting the behaviour of pv_wrap_append_preload(). -- GitLab From de82a412b3dd1eb8d5fd9e9d7542d82fe8907ec0 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Fri, 31 Jan 2025 14:23:35 +0000 Subject: [PATCH 12/15] pv-wrap: Have test coverage for the single-architecture case Until now, the case where only one architecture is supported has only been tested if pressure-vessel was built for a non-x86 architecture such as ARM, which is rarely (if ever) done. We can get coverage for this case by making pv-wrap behave as if only the first architecture was supported, as a runtime rather than compile-time decision: this means that an ordinary x86_64 build can exercise both the x86 and non-x86 code paths. This is important because steamrt/tasks#595 will add new code that differs according to whether there is only a single architecture, or more than one. In the test, this means we need to be able to mark which of the expected paths are expected on i386 only, and which ones are expected in general. Instead of the verbosity that would result from turning all of our test data into structs, I've done this with an ad-hoc mini-language: the ones that are only expected on i386 are prefixed with `i386:`. We also might conceivably want to make use of something similar to PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE in production in future, if we start providing legacy-free containers that only support running x86_64 games. Signed-off-by: Simon McVittie <smcv@collabora.com> --- pressure-vessel/wrap-setup.c | 9 +- pressure-vessel/wrap-setup.h | 3 + tests/pressure-vessel/wrap-setup.c | 157 +++++++++++++++++++++-------- 3 files changed, 125 insertions(+), 44 deletions(-) diff --git a/pressure-vessel/wrap-setup.c b/pressure-vessel/wrap-setup.c index a185fd1ac..31bf7b83c 100644 --- a/pressure-vessel/wrap-setup.c +++ b/pressure-vessel/wrap-setup.c @@ -710,9 +710,16 @@ append_preload_per_architecture (GPtrArray *argv, FlatpakExports *exports) { g_autoptr(SrtSystemInfo) system_info = srt_system_info_new (NULL); + gsize n_supported_architectures = PV_N_SUPPORTED_ARCHITECTURES; gsize i; - for (i = 0; i < PV_N_SUPPORTED_ARCHITECTURES; i++) + if (flags & PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE) + { + G_STATIC_ASSERT (PV_N_SUPPORTED_ARCHITECTURES >= 1); + n_supported_architectures = 1; + } + + for (i = 0; i < n_supported_architectures; i++) { g_autoptr(GString) mock_path = NULL; g_autoptr(SrtLibrary) details = NULL; diff --git a/pressure-vessel/wrap-setup.h b/pressure-vessel/wrap-setup.h index ceb71fc69..f0308e8fb 100644 --- a/pressure-vessel/wrap-setup.h +++ b/pressure-vessel/wrap-setup.h @@ -68,6 +68,8 @@ void pv_wrap_move_into_scope (const char *steam_app_id); * @PV_APPEND_PRELOAD_FLAGS_REMOVE_GAME_OVERLAY: Disable the Steam Overlay * @PV_APPEND_PRELOAD_FLAGS_IN_UNIT_TESTS: Normalize $LIB and $PLATFORM, * for unit testing + * @PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE: Behave as though there is + * only one architecture supported, for test coverage * @PV_APPEND_PRELOAD_FLAGS_NONE: None of the above * * Flags affecting the behaviour of pv_wrap_append_preload(). @@ -77,6 +79,7 @@ typedef enum PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX = (1 << 0), PV_APPEND_PRELOAD_FLAGS_REMOVE_GAME_OVERLAY = (1 << 1), PV_APPEND_PRELOAD_FLAGS_IN_UNIT_TESTS = (1 << 2), + PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE = (1 << 3), PV_APPEND_PRELOAD_FLAGS_NONE = 0 } PvAppendPreloadFlags; diff --git a/tests/pressure-vessel/wrap-setup.c b/tests/pressure-vessel/wrap-setup.c index d22205181..885cea99b 100644 --- a/tests/pressure-vessel/wrap-setup.c +++ b/tests/pressure-vessel/wrap-setup.c @@ -94,6 +94,7 @@ typedef struct typedef struct { PvRuntimeFlags runtime_flags; + PvAppendPreloadFlags preload_flags; } Config; static const Config default_config = {}; @@ -106,6 +107,10 @@ static const Config interpreter_root_config = .runtime_flags = (PV_RUNTIME_FLAGS_COPY_RUNTIME | PV_RUNTIME_FLAGS_INTERPRETER_ROOT), }; +static const Config one_arch_config = +{ + .preload_flags = PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE, +}; static int open_or_die (const char *path, @@ -250,6 +255,7 @@ static void setup_ld_preload (Fixture *f, gconstpointer context) { + const Config *config = context; static const char * const touch[] = { "app/lib/libpreloadA.so", @@ -274,9 +280,14 @@ setup_ld_preload (Fixture *f, setup (f, context); fixture_populate_dir (f, f->mock_host->fd, touch, G_N_ELEMENTS (touch)); + + if (!(config->preload_flags & PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE)) + { #if defined(__i386__) || defined(__x86_64__) - fixture_populate_dir (f, f->mock_host->fd, touch_i386, G_N_ELEMENTS (touch_i386)); + fixture_populate_dir (f, f->mock_host->fd, touch_i386, G_N_ELEMENTS (touch_i386)); #endif + } + f->env = g_environ_setenv (f->env, "STEAM_COMPAT_CLIENT_INSTALL_PATH", "/steam", TRUE); } @@ -1630,22 +1641,16 @@ static const char * const expected_preload_paths[] = { "/app/lib/libpreloadA.so", "/platform/plat-" PRIMARY_PLATFORM "/libpreloadP.so:abi=" PRIMARY_ABI, -#if defined(__i386__) || defined(__x86_64__) - "/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so:abi=" SRT_ABI_I386, -#endif + "i386:/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so:abi=" SRT_ABI_I386, "/opt/" PRIMARY_LIB "/libpreloadL.so:abi=" PRIMARY_ABI, -#if defined(__i386__) || defined(__x86_64__) - "/opt/" MOCK_LIB_32 "/libpreloadL.so:abi=" SRT_ABI_I386, -#endif + "i386:/opt/" MOCK_LIB_32 "/libpreloadL.so:abi=" SRT_ABI_I386, "/lib/libpreload-rootfs.so", "/usr/lib/libpreloadU.so", "/home/me/libpreloadH.so", "/steam/lib/gameoverlayrenderer.so", "/overlay/libs/${ORIGIN}/../lib/libpreloadO.so", "/future/libs-$FUTURE/libpreloadF.so", -#if defined(__i386__) || defined(__x86_64__) - "/in-root-plat-i686-only-32-bit.so:abi=" SRT_ABI_I386, -#endif + "i386:/in-root-plat-i686-only-32-bit.so:abi=" SRT_ABI_I386, "/in-root-${FUTURE}.so", "./${RELATIVE}.so", "./relative.so", @@ -1653,41 +1658,77 @@ static const char * const expected_preload_paths[] = * libfakeroot is not in the runtime or graphics stack provider, only * the current namespace */ "/path/to/" PRIMARY_LIB "/libfakeroot.so:abi=" PRIMARY_ABI, -#if defined(__i386__) || defined(__x86_64__) - "/path/to/" MOCK_LIB_32 "/libfakeroot.so:abi=" SRT_ABI_I386, -#endif + "i386:/path/to/" MOCK_LIB_32 "/libfakeroot.so:abi=" SRT_ABI_I386, /* Our mock implementation of pv_runtime_has_library() behaves as though * libpthread.so.0 *is* in the runtime, as we would expect */ "libpthread.so.0", }; +static GPtrArray * +filter_expected_paths (const Config *config) +{ + g_autoptr(GPtrArray) filtered = g_ptr_array_new_with_free_func (NULL); + gsize i; + + /* Some of the expected paths are only expected to appear on i386. + * Filter the list accordingly. */ + for (i = 0; i < G_N_ELEMENTS (expected_preload_paths); i++) + { + const char *path = expected_preload_paths[i]; + + if (g_str_has_prefix (path, "i386:")) + { +#if defined(__i386__) || defined(__x86_64__) + if (!(config->preload_flags & PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE)) + g_ptr_array_add (filtered, (char *) (path + strlen ("i386:"))); +#endif + } + else + { + g_ptr_array_add (filtered, (char *) path); + } + } + + return g_steal_pointer (&filtered); +} + static void test_remap_ld_preload (Fixture *f, gconstpointer context) { + const Config *config = context; g_autoptr(FlatpakExports) exports = fixture_create_exports (f); g_autoptr(GPtrArray) argv = g_ptr_array_new_with_free_func (g_free); + g_autoptr(GPtrArray) filtered = filter_expected_paths (config); g_autoptr(PvRuntime) runtime = fixture_create_runtime (f, PV_RUNTIME_FLAGS_NONE); gsize i; + gboolean expect_i386 = FALSE; + +#if defined(__i386__) || defined(__x86_64__) + if (!(config->preload_flags & PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE)) + expect_i386 = TRUE; +#endif - populate_ld_preload (f, argv, PV_APPEND_PRELOAD_FLAGS_NONE, runtime, exports); + populate_ld_preload (f, argv, config->preload_flags, runtime, exports); - g_assert_cmpuint (argv->len, ==, G_N_ELEMENTS (expected_preload_paths)); + g_assert_cmpuint (argv->len, ==, filtered->len); for (i = 0; i < argv->len; i++) { + char *expected = g_ptr_array_index (filtered, i); char *argument = g_ptr_array_index (argv, i); + g_assert_true (g_str_has_prefix (argument, "--ld-preload=")); argument += strlen("--ld-preload="); - if (g_str_has_prefix (expected_preload_paths[i], "/lib/") - || g_str_has_prefix (expected_preload_paths[i], "/usr/lib/")) + if (g_str_has_prefix (expected, "/lib/") + || g_str_has_prefix (expected, "/usr/lib/")) { g_assert_true (g_str_has_prefix (argument, "/run/host/")); argument += strlen("/run/host"); } - g_assert_cmpstr (argument, ==, expected_preload_paths[i]); + g_assert_cmpstr (argument, ==, expected); } /* FlatpakExports never exports /app */ @@ -1710,10 +1751,8 @@ test_remap_ld_preload (Fixture *f, g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" PRIMARY_LIB "/libpreloadL.so")); g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" PRIMARY_PLATFORM "/libpreloadP.so")); -#if defined(__i386__) || defined(__x86_64__) - g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_32 "/libpreloadL.so")); - g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so")); -#endif + g_assert_cmpint (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_32 "/libpreloadL.so"), ==, expect_i386); + g_assert_cmpint (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so"), ==, expect_i386); /* FlatpakExports never exports /lib as /lib */ g_assert_false (flatpak_exports_path_is_visible (exports, "/lib")); @@ -1747,30 +1786,36 @@ static void test_remap_ld_preload_flatpak (Fixture *f, gconstpointer context) { + const Config *config = context; g_autoptr(GPtrArray) argv = g_ptr_array_new_with_free_func (g_free); + g_autoptr(GPtrArray) filtered = filter_expected_paths (config); g_autoptr(PvRuntime) runtime = fixture_create_runtime (f, PV_RUNTIME_FLAGS_FLATPAK_SUBSANDBOX); gsize i; - populate_ld_preload (f, argv, PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX, + populate_ld_preload (f, argv, + (config->preload_flags + | PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX), runtime, NULL); - g_assert_cmpuint (argv->len, ==, G_N_ELEMENTS (expected_preload_paths)); + g_assert_cmpuint (argv->len, ==, filtered->len); for (i = 0; i < argv->len; i++) { + char *expected = g_ptr_array_index (filtered, i); char *argument = g_ptr_array_index (argv, i); + g_assert_true (g_str_has_prefix (argument, "--ld-preload=")); argument += strlen("--ld-preload="); - if (g_str_has_prefix (expected_preload_paths[i], "/app/") - || g_str_has_prefix (expected_preload_paths[i], "/lib/") - || g_str_has_prefix (expected_preload_paths[i], "/usr/lib/")) + if (g_str_has_prefix (expected, "/app/") + || g_str_has_prefix (expected, "/lib/") + || g_str_has_prefix (expected, "/usr/lib/")) { g_assert_true (g_str_has_prefix (argument, "/run/parent/")); argument += strlen("/run/parent"); } - g_assert_cmpstr (argument, ==, expected_preload_paths[i]); + g_assert_cmpstr (argument, ==, expected); } } @@ -1783,31 +1828,44 @@ static void test_remap_ld_preload_no_runtime (Fixture *f, gconstpointer context) { + const Config *config = context; g_autoptr(GPtrArray) argv = g_ptr_array_new_with_free_func (g_free); + g_autoptr(GPtrArray) filtered = filter_expected_paths (config); g_autoptr(FlatpakExports) exports = fixture_create_exports (f); gsize i, j; + gboolean expect_i386 = FALSE; - populate_ld_preload (f, argv, PV_APPEND_PRELOAD_FLAGS_REMOVE_GAME_OVERLAY, +#if defined(__i386__) || defined(__x86_64__) + if (!(config->preload_flags & PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE)) + expect_i386 = TRUE; +#endif + + populate_ld_preload (f, argv, + (config->preload_flags + | PV_APPEND_PRELOAD_FLAGS_REMOVE_GAME_OVERLAY), NULL, exports); - g_assert_cmpuint (argv->len, ==, G_N_ELEMENTS (expected_preload_paths) - 1); + g_assert_cmpuint (argv->len, ==, filtered->len - 1); for (i = 0, j = 0; i < argv->len; i++, j++) { + char *expected = g_ptr_array_index (filtered, j); char *argument = g_ptr_array_index (argv, i); + g_assert_true (g_str_has_prefix (argument, "--ld-preload=")); argument += strlen("--ld-preload="); /* /steam/lib/gameoverlayrenderer.so is missing because we used the * REMOVE_GAME_OVERLAY flag */ - if (g_str_has_suffix (expected_preload_paths[j], "/gameoverlayrenderer.so")) + if (g_str_has_suffix (expected, "/gameoverlayrenderer.so")) { /* We expect to skip only one element */ g_assert_cmpint (i, ==, j); j++; + expected = g_ptr_array_index (filtered, j); } - g_assert_cmpstr (argument, ==, expected_preload_paths[j]); + g_assert_cmpstr (argument, ==, expected); } /* FlatpakExports never exports /app */ @@ -1830,10 +1888,8 @@ test_remap_ld_preload_no_runtime (Fixture *f, g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" PRIMARY_LIB "/libpreloadL.so")); g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" PRIMARY_PLATFORM "/libpreloadP.so")); -#if defined(__i386__) || defined(__x86_64__) - g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_32 "/libpreloadL.so")); - g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so")); -#endif + g_assert_cmpint (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_32 "/libpreloadL.so"), ==, expect_i386); + g_assert_cmpint (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so"), ==, expect_i386); /* FlatpakExports never exports /lib as /lib */ g_assert_false (flatpak_exports_path_is_visible (exports, "/lib")); @@ -1862,21 +1918,27 @@ static void test_remap_ld_preload_flatpak_no_runtime (Fixture *f, gconstpointer context) { + const Config *config = context; g_autoptr(GPtrArray) argv = g_ptr_array_new_with_free_func (g_free); + g_autoptr(GPtrArray) filtered = filter_expected_paths (config); gsize i; - populate_ld_preload (f, argv, PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX, + populate_ld_preload (f, argv, + (config->preload_flags + | PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX), NULL, NULL); - g_assert_cmpuint (argv->len, ==, G_N_ELEMENTS (expected_preload_paths)); + g_assert_cmpuint (argv->len, ==, filtered->len); for (i = 0; i < argv->len; i++) { + char *expected = g_ptr_array_index (filtered, i); char *argument = g_ptr_array_index (argv, i); + g_assert_true (g_str_has_prefix (argument, "--ld-preload=")); argument += strlen("--ld-preload="); - g_assert_cmpstr (argument, ==, expected_preload_paths[i]); + g_assert_cmpstr (argument, ==, expected); } } @@ -2248,13 +2310,13 @@ main (int argc, g_test_add ("/options/true", Fixture, NULL, setup, test_options_true, teardown); g_test_add ("/passwd", Fixture, NULL, setup, test_passwd, teardown); - g_test_add ("/remap-ld-preload", Fixture, NULL, + g_test_add ("/remap-ld-preload", Fixture, &default_config, setup_ld_preload, test_remap_ld_preload, teardown); - g_test_add ("/remap-ld-preload-flatpak", Fixture, NULL, + g_test_add ("/remap-ld-preload-flatpak", Fixture, &default_config, setup_ld_preload, test_remap_ld_preload_flatpak, teardown); - g_test_add ("/remap-ld-preload-no-runtime", Fixture, NULL, + g_test_add ("/remap-ld-preload-no-runtime", Fixture, &default_config, setup_ld_preload, test_remap_ld_preload_no_runtime, teardown); - g_test_add ("/remap-ld-preload-flatpak-no-runtime", Fixture, NULL, + g_test_add ("/remap-ld-preload-flatpak-no-runtime", Fixture, &default_config, setup_ld_preload, test_remap_ld_preload_flatpak_no_runtime, teardown); g_test_add ("/supported-archs", Fixture, NULL, setup, test_supported_archs, teardown); @@ -2263,5 +2325,14 @@ main (int argc, g_test_add ("/use-host-os", Fixture, NULL, setup, test_use_host_os, teardown); + g_test_add ("/one-arch/remap-ld-preload", Fixture, &one_arch_config, + setup_ld_preload, test_remap_ld_preload, teardown); + g_test_add ("/one-arch/remap-ld-preload-flatpak", Fixture, &one_arch_config, + setup_ld_preload, test_remap_ld_preload_flatpak, teardown); + g_test_add ("/one-arch/remap-ld-preload-no-runtime", Fixture, &one_arch_config, + setup_ld_preload, test_remap_ld_preload_no_runtime, teardown); + g_test_add ("/one-arch/remap-ld-preload-flatpak-no-runtime", Fixture, &one_arch_config, + setup_ld_preload, test_remap_ld_preload_flatpak_no_runtime, teardown); + return g_test_run (); } -- GitLab From d33604f369acae03924e18efc5b913a8cd3dc063 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Fri, 31 Jan 2025 14:45:17 +0000 Subject: [PATCH 13/15] pv-wrap: Use a more realistic multiarch tuple for unit tests All of our supported architectures define _SRT_MULTIARCH from the build system, and pressure-vessel can't be compiled for other architectures without defining _SRT_MULTIARCH, so we don't need to override this to a mock string; we can leave it set to the real multiarch tuple of the architecture. This avoids setting up inconsistencies between this code path, and other parts of pressure-vessel that do not force a mock architecture. Signed-off-by: Simon McVittie <smcv@collabora.com> --- pressure-vessel/wrap-setup.c | 1 - tests/pressure-vessel/wrap-setup.c | 6 ++---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/pressure-vessel/wrap-setup.c b/pressure-vessel/wrap-setup.c index 31bf7b83c..fbaef9c99 100644 --- a/pressure-vessel/wrap-setup.c +++ b/pressure-vessel/wrap-setup.c @@ -748,7 +748,6 @@ append_preload_per_architecture (GPtrArray *argv, /* As a mock ${PLATFORM}, use the first one listed. */ platform = pv_multiarch_details[i].platforms[0]; #else - multiarch_tuple = "mock-multiarch-tuple"; platform = "mock"; #endif /* As a mock ${LIB}, behave like Debian or the fdo SDK. */ diff --git a/tests/pressure-vessel/wrap-setup.c b/tests/pressure-vessel/wrap-setup.c index 885cea99b..a550e05ae 100644 --- a/tests/pressure-vessel/wrap-setup.c +++ b/tests/pressure-vessel/wrap-setup.c @@ -43,8 +43,6 @@ #include "wrap-setup.h" #include "utils.h" -#define MOCK_ABI "mock-multiarch-tuple" - /* These match the first entry in PvMultiArchdetails.platforms, * which is the easiest realistic thing for a mock implementation of * srt_system_info_check_library() to use. */ @@ -56,7 +54,7 @@ * a mock implementation of srt_system_info_check_library() to use. */ #define MOCK_LIB_32 "lib/" SRT_ABI_I386 #define MOCK_LIB_64 "lib/" SRT_ABI_X86_64 -#define MOCK_LIB_GENERIC "lib/" MOCK_ABI +#define MOCK_LIB_GENERIC "lib/" _SRT_MULTIARCH #if defined(__i386__) || defined(__x86_64__) /* On x86, we treat x86_64 as the primary architecture. @@ -71,7 +69,7 @@ /* On non-x86, the mock implementation of srt_system_info_check_library() * uses these expansions for ${PLATFORM} and ${LIB} instead of the * real ones. */ -# define PRIMARY_ABI MOCK_ABI +# define PRIMARY_ABI _SRT_MULTIARCH # define PRIMARY_PLATFORM MOCK_PLATFORM_GENERIC # define PRIMARY_LIB MOCK_LIB_GENERIC #endif -- GitLab From 68d78d8abeb7f8bd4384f5ef93c71372fd707b26 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Thu, 30 Jan 2025 19:08:25 +0000 Subject: [PATCH 14/15] PvAdverbPreloadModule: Add initializer and g_auto implementation This is a useful thing to have if we might ever want a PvAdverbPreloadModule on the stack. Signed-off-by: Simon McVittie <smcv@collabora.com> --- pressure-vessel/adverb-preload.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pressure-vessel/adverb-preload.h b/pressure-vessel/adverb-preload.h index 531661781..3f6ca1f59 100644 --- a/pressure-vessel/adverb-preload.h +++ b/pressure-vessel/adverb-preload.h @@ -28,14 +28,25 @@ typedef struct gsize abi_index; } PvAdverbPreloadModule; +#define PV_ADVERB_PRELOAD_MODULE_INIT \ +{ \ + .name = NULL, \ + .index_in_preload_variables = 0, \ + .abi_index = PV_UNSPECIFIED_ABI, \ +} + static inline void pv_adverb_preload_module_clear (gpointer p) { PvAdverbPreloadModule *self = p; + PvAdverbPreloadModule blank = PV_ADVERB_PRELOAD_MODULE_INIT; g_free (self->name); + *self = blank; } +G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC (PvAdverbPreloadModule, pv_adverb_preload_module_clear) + gboolean pv_adverb_set_up_preload_modules (FlatpakBwrap *wrapped_command, PvPerArchDirs *lib_temp_dirs, const PvAdverbPreloadModule *preload_modules, -- GitLab From 4d7d964d7143c185276be5a8e7bbeff5eea77bc8 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Tue, 28 Jan 2025 16:23:34 +0000 Subject: [PATCH 15/15] pv-adverb, pv-wrap: Centralize more knowledge about LD_AUDIT and LD_PRELOAD Previously we had this in two places, but we can share code between pv-adverb and pv-wrap to make this easier to understand. Signed-off-by: Simon McVittie <smcv@collabora.com> --- pressure-vessel/adverb-preload.c | 25 ++++++++++++++++++------- pressure-vessel/adverb-preload.h | 9 +++++++++ pressure-vessel/meson.build | 4 ++-- pressure-vessel/wrap-context.c | 28 ++++++++++++---------------- pressure-vessel/wrap-context.h | 9 ++------- pressure-vessel/wrap-setup.c | 17 ++++++++++------- pressure-vessel/wrap-setup.h | 4 ++-- pressure-vessel/wrap.c | 15 ++------------- tests/pressure-vessel/wrap-setup.c | 17 ++++++++--------- 9 files changed, 65 insertions(+), 63 deletions(-) diff --git a/pressure-vessel/adverb-preload.c b/pressure-vessel/adverb-preload.c index 415b4ee8e..2685f6fbf 100644 --- a/pressure-vessel/adverb-preload.c +++ b/pressure-vessel/adverb-preload.c @@ -9,11 +9,22 @@ #include "supported-architectures.h" #include "utils.h" -/* Indexed by PreloadVariableIndex */ -static const char *preload_variables[] = +const PvPreloadVariable pv_preload_variables[] = { - "LD_AUDIT", - "LD_PRELOAD", + [PV_PRELOAD_VARIABLE_INDEX_LD_AUDIT] = { + .variable = "LD_AUDIT", + .adverb_option = "--ld-audit", + /* "The items in the list are colon-separated, and there is no support + * for escaping the separator." —ld.so(8) */ + .separators = ":", + }, + [PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD] = { + .variable = "LD_PRELOAD", + .adverb_option = "--ld-preload", + /* "The items of the list can be separated by spaces or colons, and + * there is no support for escaping either separator." —ld.so(8) */ + .separators = ": ", + }, }; static gpointer @@ -45,7 +56,7 @@ pv_adverb_set_up_preload_modules (FlatpakBwrap *wrapped_command, gsize n_preload_modules, GError **error) { - GPtrArray *preload_search_paths[G_N_ELEMENTS (preload_variables)] = { NULL }; + GPtrArray *preload_search_paths[G_N_ELEMENTS (pv_preload_variables)] = { NULL }; gsize i; /* Iterate through all modules, populating preload_search_paths */ @@ -155,11 +166,11 @@ pv_adverb_set_up_preload_modules (FlatpakBwrap *wrapped_command, /* Serialize search_paths[PRELOAD_VARIABLE_INDEX_LD_AUDIT] into * LD_AUDIT, etc. */ - for (i = 0; i < G_N_ELEMENTS (preload_variables); i++) + for (i = 0; i < G_N_ELEMENTS (pv_preload_variables); i++) { GPtrArray *search_path = preload_search_paths[i]; g_autoptr(GString) buffer = g_string_new (""); - const char *variable = preload_variables[i]; + const char *variable = pv_preload_variables[i].variable; if (search_path != NULL) g_ptr_array_foreach (search_path, (GFunc) append_to_search_path, buffer); diff --git a/pressure-vessel/adverb-preload.h b/pressure-vessel/adverb-preload.h index 3f6ca1f59..bd9c578dc 100644 --- a/pressure-vessel/adverb-preload.h +++ b/pressure-vessel/adverb-preload.h @@ -18,6 +18,15 @@ typedef enum PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, } PvPreloadVariableIndex; +typedef struct +{ + const char *variable; + const char *adverb_option; + const char *separators; +} PvPreloadVariable; + +extern const PvPreloadVariable pv_preload_variables[2]; + #define PV_UNSPECIFIED_ABI (G_MAXSIZE) typedef struct diff --git a/pressure-vessel/meson.build b/pressure-vessel/meson.build index 5bb38e9cf..59bfe47c7 100644 --- a/pressure-vessel/meson.build +++ b/pressure-vessel/meson.build @@ -71,6 +71,8 @@ enums = gnome.mkenums_simple( pressure_vessel_utils = static_library( 'pressure-vessel-utils', sources : [ + 'adverb-preload.c', + 'adverb-preload.h', 'flatpak-bwrap.c', 'flatpak-bwrap-private.h', 'flatpak-utils-base.c', @@ -106,8 +108,6 @@ pressure_vessel_utils_dep = declare_dependency( pressure_vessel_adverb_lib = static_library( 'pressure-vessel-adverb-lib', sources : [ - 'adverb-preload.c', - 'adverb-preload.h', 'adverb-sdl.c', 'adverb-sdl.h', 'per-arch-dirs.c', diff --git a/pressure-vessel/wrap-context.c b/pressure-vessel/wrap-context.c index 3b2621576..dd711208e 100644 --- a/pressure-vessel/wrap-context.c +++ b/pressure-vessel/wrap-context.c @@ -192,17 +192,17 @@ opt_ignored_cb (const gchar *option_name, static gboolean opt_ld_something (PvWrapOptions *self, - PreloadVariableIndex which, + PvPreloadVariableIndex which, const char *value, - const char *separators, + gboolean split, GError **error) { g_auto(GStrv) tokens = NULL; size_t i; - if (separators != NULL) + if (split) { - tokens = g_strsplit_set (value, separators, 0); + tokens = g_strsplit_set (value, pv_preload_variables[which].separators, 0); } else { @@ -230,8 +230,8 @@ opt_ld_audit_cb (const gchar *option_name, gpointer data, GError **error) { - return opt_ld_something (data, PRELOAD_VARIABLE_INDEX_LD_AUDIT, - value, NULL, error); + return opt_ld_something (data, PV_PRELOAD_VARIABLE_INDEX_LD_AUDIT, + value, FALSE, error); } static gboolean @@ -240,10 +240,8 @@ opt_ld_audits_cb (const gchar *option_name, gpointer data, GError **error) { - /* "The items in the list are colon-separated, and there is no support - * for escaping the separator." —ld.so(8) */ - return opt_ld_something (data, PRELOAD_VARIABLE_INDEX_LD_AUDIT, - value, ":", error); + return opt_ld_something (data, PV_PRELOAD_VARIABLE_INDEX_LD_AUDIT, + value, TRUE, error); } static gboolean @@ -252,8 +250,8 @@ opt_ld_preload_cb (const gchar *option_name, gpointer data, GError **error) { - return opt_ld_something (data, PRELOAD_VARIABLE_INDEX_LD_PRELOAD, - value, NULL, error); + return opt_ld_something (data, PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + value, FALSE, error); } static gboolean @@ -262,10 +260,8 @@ opt_ld_preloads_cb (const gchar *option_name, gpointer data, GError **error) { - /* "The items of the list can be separated by spaces or colons, and - * there is no support for escaping either separator." —ld.so(8) */ - return opt_ld_something (data, PRELOAD_VARIABLE_INDEX_LD_PRELOAD, - value, ": ", error); + return opt_ld_something (data, PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, + value, TRUE, error); } static gboolean diff --git a/pressure-vessel/wrap-context.h b/pressure-vessel/wrap-context.h index 56d5bcfd7..17d2db2c2 100644 --- a/pressure-vessel/wrap-context.h +++ b/pressure-vessel/wrap-context.h @@ -11,6 +11,7 @@ #include "steam-runtime-tools/resolve-in-sysroot-internal.h" #include "steam-runtime-tools/utils-internal.h" +#include "pressure-vessel/adverb-preload.h" #include "pressure-vessel/flatpak-exports-private.h" #include "pressure-vessel/wrap-interactive.h" @@ -39,15 +40,9 @@ typedef enum TRISTATE_MAYBE } Tristate; -typedef enum -{ - PRELOAD_VARIABLE_INDEX_LD_AUDIT, - PRELOAD_VARIABLE_INDEX_LD_PRELOAD, -} PreloadVariableIndex; - typedef struct { - PreloadVariableIndex which; + PvPreloadVariableIndex which; gchar *preload; } WrapPreloadModule; diff --git a/pressure-vessel/wrap-setup.c b/pressure-vessel/wrap-setup.c index fbaef9c99..7e31e55cf 100644 --- a/pressure-vessel/wrap-setup.c +++ b/pressure-vessel/wrap-setup.c @@ -33,6 +33,7 @@ #include <string.h> +#include "adverb-preload.h" #include "bwrap.h" #include "exports.h" #include "flatpak-run-dbus-private.h" @@ -869,10 +870,7 @@ append_preload_basename (GPtrArray *argv, /** * pv_wrap_append_preload: * @argv: (element-type filename): Array of command-line options to populate - * @variable: (type filename): Environment variable from which this - * preload module was taken, either `LD_AUDIT` or `LD_PRELOAD` - * @option: (type filename): Command-line option to add to @argv, - * either `--ld-audit` or `--ld-preload` + * @which: Either `LD_AUDIT` or `LD_PRELOAD` * @preload: (type filename): Path of preloadable module in current * namespace, possibly including special ld.so tokens such as `$LIB`, * or basename of a preloadable module to be found in the standard @@ -889,8 +887,7 @@ append_preload_basename (GPtrArray *argv, */ void pv_wrap_append_preload (GPtrArray *argv, - const char *variable, - const char *option, + PvPreloadVariableIndex which, const char *preload, GStrv env, PvAppendPreloadFlags flags, @@ -899,11 +896,17 @@ pv_wrap_append_preload (GPtrArray *argv, { SrtLoadableKind kind; SrtLoadableFlags loadable_flags; + const char *variable; + const char *option; g_return_if_fail (argv != NULL); - g_return_if_fail (option != NULL); g_return_if_fail (preload != NULL); g_return_if_fail (runtime == NULL || PV_IS_RUNTIME (runtime)); + g_return_if_fail (which >= 0); + g_return_if_fail (which < G_N_ELEMENTS (pv_preload_variables)); + + variable = pv_preload_variables[which].variable; + option = pv_preload_variables[which].adverb_option; if (strstr (preload, "gtk3-nocsd") != NULL) { diff --git a/pressure-vessel/wrap-setup.h b/pressure-vessel/wrap-setup.h index f0308e8fb..936314a4d 100644 --- a/pressure-vessel/wrap-setup.h +++ b/pressure-vessel/wrap-setup.h @@ -26,6 +26,7 @@ #include "steam-runtime-tools/env-overlay-internal.h" #include "steam-runtime-tools/utils-internal.h" +#include "adverb-preload.h" #include "bwrap.h" #include "flatpak-bwrap-private.h" #include "flatpak-exports-private.h" @@ -84,8 +85,7 @@ typedef enum } PvAppendPreloadFlags; void pv_wrap_append_preload (GPtrArray *argv, - const char *variable, - const char *option, + PvPreloadVariableIndex which, const char *preload, GStrv env, PvAppendPreloadFlags flags, diff --git a/pressure-vessel/wrap.c b/pressure-vessel/wrap.c index 00aacc9a1..eddd59e00 100644 --- a/pressure-vessel/wrap.c +++ b/pressure-vessel/wrap.c @@ -66,16 +66,6 @@ static const GDebugKey pv_debug_keys[] = { "container", PV_WRAP_LOG_FLAGS_CONTAINER }, }; -static const struct -{ - const char *variable; - const char *adverb_option; -} preload_options[] = -{ - [PRELOAD_VARIABLE_INDEX_LD_AUDIT] = { "LD_AUDIT", "--ld-audit" }, - [PRELOAD_VARIABLE_INDEX_LD_PRELOAD] = { "LD_PRELOAD", "--ld-preload" }, -}; - #define usage_error(...) _srt_log_failure (__VA_ARGS__) int @@ -851,10 +841,9 @@ main (int argc, j); g_assert (module->which >= 0); - g_assert (module->which < G_N_ELEMENTS (preload_options)); + g_assert (module->which < G_N_ELEMENTS (pv_preload_variables)); pv_wrap_append_preload (adverb_preload_argv, - preload_options[module->which].variable, - preload_options[module->which].adverb_option, + module->which, module->preload, environ, append_preload_flags, diff --git a/tests/pressure-vessel/wrap-setup.c b/tests/pressure-vessel/wrap-setup.c index a550e05ae..c238b8af0 100644 --- a/tests/pressure-vessel/wrap-setup.c +++ b/tests/pressure-vessel/wrap-setup.c @@ -979,37 +979,37 @@ test_options_true (Fixture *f, g_assert_cmpuint (options->preload_modules->len, >, i); module = &g_array_index (options->preload_modules, WrapPreloadModule, i++); - g_assert_cmpint (module->which, ==, PRELOAD_VARIABLE_INDEX_LD_AUDIT); + g_assert_cmpint (module->which, ==, PV_PRELOAD_VARIABLE_INDEX_LD_AUDIT); g_assert_cmpstr (module->preload, ==, "libaudit.so"); g_assert_cmpuint (options->preload_modules->len, >, i); module = &g_array_index (options->preload_modules, WrapPreloadModule, i++); - g_assert_cmpint (module->which, ==, PRELOAD_VARIABLE_INDEX_LD_AUDIT); + g_assert_cmpint (module->which, ==, PV_PRELOAD_VARIABLE_INDEX_LD_AUDIT); g_assert_cmpstr (module->preload, ==, "libaudit1.so"); g_assert_cmpuint (options->preload_modules->len, >, i); module = &g_array_index (options->preload_modules, WrapPreloadModule, i++); - g_assert_cmpint (module->which, ==, PRELOAD_VARIABLE_INDEX_LD_AUDIT); + g_assert_cmpint (module->which, ==, PV_PRELOAD_VARIABLE_INDEX_LD_AUDIT); g_assert_cmpstr (module->preload, ==, "libaudit2.so"); g_assert_cmpuint (options->preload_modules->len, >, i); module = &g_array_index (options->preload_modules, WrapPreloadModule, i++); - g_assert_cmpint (module->which, ==, PRELOAD_VARIABLE_INDEX_LD_PRELOAD); + g_assert_cmpint (module->which, ==, PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD); g_assert_cmpstr (module->preload, ==, "libpreload.so"); g_assert_cmpuint (options->preload_modules->len, >, i); module = &g_array_index (options->preload_modules, WrapPreloadModule, i++); - g_assert_cmpint (module->which, ==, PRELOAD_VARIABLE_INDEX_LD_PRELOAD); + g_assert_cmpint (module->which, ==, PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD); g_assert_cmpstr (module->preload, ==, "libpreload1.so"); g_assert_cmpuint (options->preload_modules->len, >, i); module = &g_array_index (options->preload_modules, WrapPreloadModule, i++); - g_assert_cmpint (module->which, ==, PRELOAD_VARIABLE_INDEX_LD_PRELOAD); + g_assert_cmpint (module->which, ==, PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD); g_assert_cmpstr (module->preload, ==, "libpreload2.so"); g_assert_cmpuint (options->preload_modules->len, >, i); module = &g_array_index (options->preload_modules, WrapPreloadModule, i++); - g_assert_cmpint (module->which, ==, PRELOAD_VARIABLE_INDEX_LD_PRELOAD); + g_assert_cmpint (module->which, ==, PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD); g_assert_cmpstr (module->preload, ==, "libpreload3.so"); g_assert_cmpuint (i, ==, options->preload_modules->len); @@ -1612,8 +1612,7 @@ populate_ld_preload (Fixture *f, } pv_wrap_append_preload (argv, - "LD_PRELOAD", - "--ld-preload", + PV_PRELOAD_VARIABLE_INDEX_LD_PRELOAD, preloads[i].string, f->env, flags | PV_APPEND_PRELOAD_FLAGS_IN_UNIT_TESTS, -- GitLab