From f84101202a99c25e9d6c44703ba77004009c97b2 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Tue, 23 Mar 2021 15:14:48 +0000 Subject: [PATCH] library: Use line-based output from inspect-library This means we can cope with filenames in their filesystem encoding, which are arbitrary bytestrings (not necessarily UTF-8). Signed-off-by: Simon McVittie <smcv@collabora.com> --- steam-runtime-tools/library.c | 181 ++++++++++++--------- tests/i386-mock-debian-inspect-library.c | 15 +- tests/i386-mock-fedora-inspect-library.c | 41 +++-- tests/mock-abi-inspect-library.c | 22 +-- tests/mock-ubuntu-64-bit-inspect-library.c | 18 +- tests/x86_64-mock-debian-inspect-library.c | 15 +- tests/x86_64-mock-fedora-inspect-library.c | 15 +- 7 files changed, 170 insertions(+), 137 deletions(-) diff --git a/steam-runtime-tools/library.c b/steam-runtime-tools/library.c index 240548b40..bf89f0b13 100644 --- a/steam-runtime-tools/library.c +++ b/steam-runtime-tools/library.c @@ -196,7 +196,8 @@ srt_library_set_property (GObject *object, if (tmp != NULL && tmp[0] == '\0') tmp = NULL; - self->messages = g_strdup (tmp); + if (tmp != NULL) + self->messages = g_utf8_make_valid (tmp, -1); break; case PROP_MISSING_SYMBOLS: @@ -380,7 +381,8 @@ srt_library_class_init (SrtLibraryClass *cls) * * Return the absolute path of @self. * - * Returns: A string like `/usr/lib/libz.so.1`, which is valid as long as + * Returns: (type filename) (transfer none): A string + * like `/usr/lib/libz.so.1`, which is valid as long as * @self is not destroyed. */ const char * @@ -397,8 +399,8 @@ srt_library_get_absolute_path (SrtLibrary *self) * Return the diagnostic messages produced while checking this library, * if any. * - * Returns: (nullable) (transfer none): A string, which must not be freed, - * or %NULL if there were no diagnostic messages. + * Returns: (nullable) (transfer none) (type utf8): A string, which must + * not be freed, or %NULL if there were no diagnostic messages. */ const char * srt_library_get_messages (SrtLibrary *self) @@ -413,8 +415,8 @@ srt_library_get_messages (SrtLibrary *self) * * Return the name that was requested to be loaded when checking @self. * - * Returns: A string like `libz.so.1`, which is valid as long as @self - * is not destroyed. + * Returns: (type filename) (transfer none): A string like `libz.so.1`, + * which is valid as long as @self is not destroyed. */ const char * srt_library_get_requested_name (SrtLibrary *self) @@ -431,8 +433,8 @@ srt_library_get_requested_name (SrtLibrary *self) * See also srt_library_get_real_soname(), which might be what you * thought this did. * - * Returns: A string like `libz.so.1`, which is valid as long as @self - * is not destroyed. + * Returns: (type filename) (transfer none): A string like `libz.so.1`, + * which is valid as long as @self is not destroyed. */ const char * srt_library_get_soname (SrtLibrary *self) @@ -448,7 +450,8 @@ srt_library_get_soname (SrtLibrary *self) * This is often the same as srt_library_get_requested_name(), * but can differ when compatibility aliases are involved. * - * Returns: A string like `libz.so.1`, which is valid as long as @self + * Returns: (type filename) (transfer none): A string like `libz.so.1`, + * which is valid as long as @self * is not destroyed, or %NULL if the SONAME could not be determined. */ const char * @@ -545,7 +548,7 @@ int srt_library_get_terminating_signal (SrtLibrary *self) * Return the symbols that were expected to be provided by @self but * were not found. * - * Returns: (array zero-terminated=1) (element-type utf8): The symbols + * Returns: (array zero-terminated=1) (element-type filename): The symbols * that were missing from @self, as a %NULL-terminated array. The * pointer remains valid until @self is destroyed. */ @@ -568,7 +571,7 @@ srt_library_get_missing_symbols (SrtLibrary *self) * `curl_getenv@CURL_OPENSSL_4` (as seen in Debian 10) or an unversioned * curl_getenv, then this list would contain `curl_getenv@CURL_OPENSSL_3` * - * Returns: (array zero-terminated=1) (element-type utf8): The symbols + * Returns: (array zero-terminated=1) (element-type filename): The symbols * were available with a different version from @self, as a %NULL-terminated * array. The pointer remains valid until @self is destroyed. */ @@ -638,21 +641,20 @@ _srt_check_library_presence (const char *helpers_path, int wait_status = -1; int exit_status = -1; int terminating_signal = 0; - g_autoptr(JsonNode) node = NULL; - JsonObject *json; - JsonArray *missing_array = NULL; - JsonArray *misversioned_array = NULL; - JsonArray *dependencies_array = NULL; GError *error = NULL; - GStrv missing_symbols = NULL; - GStrv misversioned_symbols = NULL; - GStrv dependencies = NULL; + g_autoptr(GPtrArray) missing_symbols = NULL; + g_autoptr(GPtrArray) misversioned_symbols = NULL; + g_autoptr(GPtrArray) dependencies = NULL; SrtLibraryIssues issues = SRT_LIBRARY_ISSUES_NONE; GStrv my_environ = NULL; const gchar *ld_preload; gchar *filtered_preload = NULL; SrtHelperFlags flags = SRT_HELPER_FLAGS_TIME_OUT; GString *log_args = NULL; + gchar *next_line; + const char * const *missing_symbols_strv = NULL; + const char * const *misversioned_symbols_strv = NULL; + const char * const *dependencies_strv = NULL; g_return_val_if_fail (requested_name != NULL, SRT_LIBRARY_ISSUES_UNKNOWN); g_return_val_if_fail (multiarch != NULL, SRT_LIBRARY_ISSUES_UNKNOWN); @@ -676,6 +678,8 @@ _srt_check_library_presence (const char *helpers_path, goto out; } + g_ptr_array_add (argv, g_strdup ("--line-based")); + log_args = g_string_new (""); for (guint i = 0; i < argv->len; ++i) @@ -756,89 +760,116 @@ _srt_check_library_presence (const char *helpers_path, exit_status = 0; } - node = json_from_string (output, &error); - if (node == NULL) - { - g_debug ("The helper output is not a valid JSON: %s", error == NULL ? "" : error->message); - issues |= SRT_LIBRARY_ISSUES_CANNOT_LOAD; - goto out; - } + missing_symbols = g_ptr_array_new_with_free_func (g_free); + misversioned_symbols = g_ptr_array_new_with_free_func (g_free); + dependencies = g_ptr_array_new_with_free_func (g_free); - json = json_node_get_object (node); - if (!json_object_has_member (json, requested_name)) + next_line = output; + + while (next_line != NULL) { - g_debug ("The helper output is missing the expected SONAME %s", - requested_name); - issues |= SRT_LIBRARY_ISSUES_CANNOT_LOAD; - goto out; - } - json = json_object_get_object_member (json, requested_name); + char *line = next_line; + char *equals; + g_autofree gchar *decoded = NULL; + + if (*next_line == '\0') + break; - absolute_path = g_strdup (json_object_get_string_member_with_default (json, "path", NULL)); - real_soname = g_strdup (json_object_get_string_member_with_default (json, "SONAME", NULL)); + next_line = strchr (line, '\n'); - if (json_object_has_member (json, "missing_symbols")) - missing_array = json_object_get_array_member (json, "missing_symbols"); - if (missing_array != NULL) - { - if (json_array_get_length (missing_array) > 0) + if (next_line != NULL) { - issues |= SRT_LIBRARY_ISSUES_MISSING_SYMBOLS; - missing_symbols = g_new0 (gchar *, json_array_get_length (missing_array) + 1); - for (guint i = 0; i < json_array_get_length (missing_array); i++) - { - missing_symbols[i] = g_strdup (json_array_get_string_element (missing_array, i)); - } - missing_symbols[json_array_get_length (missing_array)] = NULL; + *next_line = '\0'; + next_line++; } - } - if (json_object_has_member (json, "misversioned_symbols")) - misversioned_array = json_object_get_array_member (json, "misversioned_symbols"); - if (misversioned_array != NULL) - { - if (json_array_get_length (misversioned_array) > 0) + equals = strchr (line, '='); + + if (equals == NULL) + { + g_warning ("Unexpected line in inspect-library output: %s", line); + continue; + } + + decoded = g_strcompress (equals + 1); + + if (g_str_has_prefix (line, "requested=")) { - issues |= SRT_LIBRARY_ISSUES_MISVERSIONED_SYMBOLS; - misversioned_symbols = g_new0 (gchar *, json_array_get_length (misversioned_array) + 1); - for (guint i = 0; i < json_array_get_length (misversioned_array); i++) + if (strcmp (requested_name, decoded) != 0) { - misversioned_symbols[i] = g_strdup (json_array_get_string_element (misversioned_array, i)); + g_warning ("Unexpected inspect-library output: " + "asked for \"%s\", but got \"%s\"?", + requested_name, decoded); + /* might as well continue to process it, though... */ } - misversioned_symbols[json_array_get_length (misversioned_array)] = NULL; } - } - - if (json_object_has_member (json, "dependencies")) - dependencies_array = json_object_get_array_member (json, "dependencies"); - if (dependencies_array != NULL) - { - dependencies = g_new0 (gchar *, json_array_get_length (dependencies_array) + 1); - for (guint i = 0; i < json_array_get_length (dependencies_array); i++) + else if (g_str_has_prefix (line, "soname=")) + { + if (real_soname == NULL) + real_soname = g_steal_pointer (&decoded); + else + g_warning ("More than one SONAME in inspect-library output"); + } + else if (g_str_has_prefix (line, "path=")) + { + if (absolute_path == NULL) + absolute_path = g_steal_pointer (&decoded); + else + g_warning ("More than one path in inspect-library output"); + } + else if (g_str_has_prefix (line, "missing_symbol=")) + { + g_ptr_array_add (missing_symbols, g_steal_pointer (&decoded)); + } + else if (g_str_has_prefix (line, "misversioned_symbol=")) + { + g_ptr_array_add (misversioned_symbols, g_steal_pointer (&decoded)); + } + else if (g_str_has_prefix (line, "dependency=")) { - dependencies[i] = g_strdup (json_array_get_string_element (dependencies_array, i)); + g_ptr_array_add (dependencies, g_steal_pointer (&decoded)); + } + else + { + g_debug ("Unknown line in inspect-library output: %s", line); } - dependencies[json_array_get_length (dependencies_array)] = NULL; } out: + if (missing_symbols != NULL && missing_symbols->len > 0) + { + issues |= SRT_LIBRARY_ISSUES_MISSING_SYMBOLS; + g_ptr_array_add (missing_symbols, NULL); + missing_symbols_strv = (const char * const *) missing_symbols->pdata; + } + + if (misversioned_symbols != NULL && misversioned_symbols->len > 0) + { + issues |= SRT_LIBRARY_ISSUES_MISVERSIONED_SYMBOLS; + g_ptr_array_add (misversioned_symbols, NULL); + misversioned_symbols_strv = (const char * const *) misversioned_symbols->pdata; + } + + if (dependencies != NULL && dependencies->len > 0) + { + g_ptr_array_add (dependencies, NULL); + dependencies_strv = (const char * const *) dependencies->pdata; + } + if (more_details_out != NULL) *more_details_out = _srt_library_new (multiarch, absolute_path, requested_name, issues, child_stderr, - (const char **) missing_symbols, - (const char **) misversioned_symbols, - (const char **) dependencies, + missing_symbols_strv, + misversioned_symbols_strv, + dependencies_strv, real_soname, exit_status, terminating_signal); g_strfreev (my_environ); - g_strfreev (missing_symbols); - g_strfreev (misversioned_symbols); - g_strfreev (dependencies); g_clear_pointer (&argv, g_ptr_array_unref); g_free (absolute_path); g_free (child_stderr); diff --git a/tests/i386-mock-debian-inspect-library.c b/tests/i386-mock-debian-inspect-library.c index 524bc6123..c72dd5561 100644 --- a/tests/i386-mock-debian-inspect-library.c +++ b/tests/i386-mock-debian-inspect-library.c @@ -25,22 +25,23 @@ #include <stdio.h> #include <stdlib.h> +#include <string.h> #include <glib.h> int main (int argc, char **argv) { - g_return_val_if_fail (argc == 2, EXIT_FAILURE); + g_return_val_if_fail (argc == 3, EXIT_FAILURE); + g_return_val_if_fail (strcmp (argv[1], "--line-based") == 0, EXIT_FAILURE); gchar **envp = g_get_environ (); - gchar *path = g_build_filename (g_environ_getenv (envp, "SRT_TEST_SYSROOT"), "usr", "lib", "i386-linux-gnu", argv[1], NULL); + gchar *path = g_build_filename (g_environ_getenv (envp, "SRT_TEST_SYSROOT"), "usr", "lib", "i386-linux-gnu", argv[2], NULL); - /* Return a JSON like if we found the given soname in a canonical Debian style, i386 lib folder */ - printf ("{\n\t\"%s\": {\n" - "\t\t\"path\": \"%s\"\n" - "\t}\n" - "}\n", argv[1], path); + /* Return as though we found the given soname in a canonical Debian style, + * i386 lib directory */ + printf ("requested=%s\n", argv[2]); + printf ("path=%s\n", path); g_free (path); g_strfreev (envp); return 0; diff --git a/tests/i386-mock-fedora-inspect-library.c b/tests/i386-mock-fedora-inspect-library.c index 42fe31c8c..34f514ac0 100644 --- a/tests/i386-mock-fedora-inspect-library.c +++ b/tests/i386-mock-fedora-inspect-library.c @@ -25,31 +25,31 @@ #include <stdio.h> #include <stdlib.h> +#include <string.h> #include <glib.h> int main (int argc, char **argv) { - g_return_val_if_fail (argc == 2, EXIT_FAILURE); + g_return_val_if_fail (argc == 3, EXIT_FAILURE); + g_return_val_if_fail (strcmp (argv[1], "--line-based") == 0, EXIT_FAILURE); - /* If the first argument is an absolute path we assume it is a library loader. + /* If the argument is an absolute path we assume it is a library loader. * Because the loaders are mock objects we just check if they are located in * the expected locations. */ - if (g_str_has_prefix (argv[1], "/")) + if (g_str_has_prefix (argv[2], "/")) { - if (g_strstr_len (argv[1], -1, "/lib/i386-linux-gnu/") != NULL || - g_strstr_len (argv[1], -1, "/lib32/dri/") != NULL || - g_strstr_len (argv[1], -1, "/lib/dri/") != NULL || - g_strstr_len (argv[1], -1, "/lib/vdpau/") != NULL || - g_strstr_len (argv[1], -1, "/another_custom_path/") != NULL || - g_strstr_len (argv[1], -1, "/custom_path32/") != NULL || - g_strstr_len (argv[1], -1, "/custom_path32_2/") != NULL) + if (g_strstr_len (argv[2], -1, "/lib/i386-linux-gnu/") != NULL || + g_strstr_len (argv[2], -1, "/lib32/dri/") != NULL || + g_strstr_len (argv[2], -1, "/lib/dri/") != NULL || + g_strstr_len (argv[2], -1, "/lib/vdpau/") != NULL || + g_strstr_len (argv[2], -1, "/another_custom_path/") != NULL || + g_strstr_len (argv[2], -1, "/custom_path32/") != NULL || + g_strstr_len (argv[2], -1, "/custom_path32_2/") != NULL) { - printf ("{\n\t\"%s\": {\n" - "\t\t\"path\": \"%s\"\n" - "\t}\n" - "}\n", argv[1], argv[1]); + printf ("requested=%s\n", argv[2]); + printf ("path=%s\n", argv[2]); return EXIT_SUCCESS; } else @@ -59,17 +59,16 @@ main (int argc, } /* If the argument is a 64bit directory, we return an exit failure */ - if (g_strstr_len (argv[1], -1, "/custom_path64/") != NULL) + if (g_strstr_len (argv[2], -1, "/custom_path64/") != NULL) return EXIT_FAILURE; gchar **envp = g_get_environ (); - gchar *path = g_build_filename (g_environ_getenv (envp, "SRT_TEST_SYSROOT"), "usr", "lib", argv[1], NULL); + gchar *path = g_build_filename (g_environ_getenv (envp, "SRT_TEST_SYSROOT"), "usr", "lib", argv[2], NULL); - /* Return a JSON like if we found the given soname in a canonical Fedora style, 32bit lib folder */ - printf ("{\n\t\"%s\": {\n" - "\t\t\"path\": \"%s\"\n" - "\t}\n" - "}\n", argv[1], path); + /* Return as though we found the given soname in a canonical Fedora style, + * 32-bit lib directory */ + printf ("requested=%s\n", argv[2]); + printf ("path=%s\n", path); g_free (path); g_strfreev (envp); return EXIT_SUCCESS; diff --git a/tests/mock-abi-inspect-library.c b/tests/mock-abi-inspect-library.c index 804553f76..c14afefaa 100644 --- a/tests/mock-abi-inspect-library.c +++ b/tests/mock-abi-inspect-library.c @@ -25,13 +25,15 @@ #include <stdio.h> #include <stdlib.h> +#include <string.h> #include <glib.h> int main (int argc, char **argv) { - g_return_val_if_fail (argc == 2, EXIT_FAILURE); + g_return_val_if_fail (argc == 3, EXIT_FAILURE); + g_return_val_if_fail (strcmp (argv[1], "--line-based") == 0, EXIT_FAILURE); #if defined(MOCK_ARCHITECTURE_x86_64) const gchar *multiarch = "x86_64-mock-abi"; @@ -50,23 +52,23 @@ main (int argc, gchar *path = NULL; gchar **envp = g_get_environ (); - if (argv[1][0] == '/') + if (argv[2][0] == '/') { /* This is a very naive check to simulate the exit error that occurrs * when we request a library that is of the wrong ELF class. */ - if (g_strstr_len (argv[1], -1, wrong_abi) != NULL) + if (g_strstr_len (argv[2], -1, wrong_abi) != NULL) goto out; - if (g_strstr_len (argv[1], -1, wrong_lib_dir) != NULL) + if (g_strstr_len (argv[2], -1, wrong_lib_dir) != NULL) goto out; /* If the path is already absolute, just prepend the sysroot */ - path = g_build_filename (g_environ_getenv (envp, "SRT_TEST_SYSROOT"), argv[1], NULL); + path = g_build_filename (g_environ_getenv (envp, "SRT_TEST_SYSROOT"), argv[2], NULL); } else { path = g_build_filename (g_environ_getenv (envp, "SRT_TEST_SYSROOT"), "usr", - "lib", multiarch, argv[1], NULL); + "lib", multiarch, argv[2], NULL); } /* When loading a library by its absolute or relative path, glib expands @@ -77,11 +79,9 @@ main (int argc, g_free (path); path = g_strjoinv (lib_dir, split); - /* Return a JSON like if we found the given soname in a mock-abi lib folder */ - printf ("{\n\t\"%s\": {\n" - "\t\t\"path\": \"%s\"\n" - "\t}\n" - "}\n", argv[1], path); + /* Return as if we found the given soname in a mock-abi lib directory */ + printf ("requested=%s\n", argv[2]); + printf ("path=%s\n", path); ret = EXIT_SUCCESS; diff --git a/tests/mock-ubuntu-64-bit-inspect-library.c b/tests/mock-ubuntu-64-bit-inspect-library.c index 4e7db2cee..c166f9480 100644 --- a/tests/mock-ubuntu-64-bit-inspect-library.c +++ b/tests/mock-ubuntu-64-bit-inspect-library.c @@ -25,31 +25,31 @@ #include <stdio.h> #include <stdlib.h> +#include <string.h> #include <glib.h> int main (int argc, char **argv) { - g_return_val_if_fail (argc == 2, EXIT_FAILURE); + g_return_val_if_fail (argc == 3, EXIT_FAILURE); + g_return_val_if_fail (strcmp (argv[1], "--line-based") == 0, EXIT_FAILURE); gchar **envp = g_get_environ (); gchar *path = NULL; /* If we need to locate "libGL.so.1" we return a canonical Ubuntu 16.04 style 64 bit folder, under * the "mesa" subfolder */ - if (g_strcmp0 (argv[1], "libGL.so.1") == 0) + if (g_strcmp0 (argv[2], "libGL.so.1") == 0) path = g_build_filename (g_environ_getenv (envp, "SRT_TEST_SYSROOT"), "usr", "lib", - "mock-ubuntu-64-bit", "mesa", argv[1], NULL); + "mock-ubuntu-64-bit", "mesa", argv[2], NULL); else path = g_build_filename (g_environ_getenv (envp, "SRT_TEST_SYSROOT"), "usr", "lib", - "mock-ubuntu-64-bit", argv[1], NULL); + "mock-ubuntu-64-bit", argv[2], NULL); - /* Return a JSON like if we found the given soname */ - printf ("{\n\t\"%s\": {\n" - "\t\t\"path\": \"%s\"\n" - "\t}\n" - "}\n", argv[1], path); + /* Return as if we found the given SONAME */ + printf ("requested=%s\n", argv[2]); + printf ("path=%s\n", path); g_free (path); g_strfreev (envp); return 0; diff --git a/tests/x86_64-mock-debian-inspect-library.c b/tests/x86_64-mock-debian-inspect-library.c index a1be89194..82aeaf80c 100644 --- a/tests/x86_64-mock-debian-inspect-library.c +++ b/tests/x86_64-mock-debian-inspect-library.c @@ -25,22 +25,23 @@ #include <stdio.h> #include <stdlib.h> +#include <string.h> #include <glib.h> int main (int argc, char **argv) { - g_return_val_if_fail (argc == 2, EXIT_FAILURE); + g_return_val_if_fail (argc == 3, EXIT_FAILURE); + g_return_val_if_fail (strcmp (argv[1], "--line-based") == 0, EXIT_FAILURE); gchar **envp = g_get_environ (); - gchar *path = g_build_filename (g_environ_getenv (envp, "SRT_TEST_SYSROOT"), "usr", "lib", "x86_64-linux-gnu", argv[1], NULL); + gchar *path = g_build_filename (g_environ_getenv (envp, "SRT_TEST_SYSROOT"), "usr", "lib", "x86_64-linux-gnu", argv[2], NULL); - /* Return a JSON like if we found the given soname in a canonical Debian style, x86_64 lib folder */ - printf ("{\n\t\"%s\": {\n" - "\t\t\"path\": \"%s\"\n" - "\t}\n" - "}\n", argv[1], path); + /* Return as though we found the given soname in a canonical Debian style, + * x86_64 lib directory */ + printf ("requested=%s\n", argv[2]); + printf ("path=%s\n", path); g_free (path); g_strfreev (envp); return 0; diff --git a/tests/x86_64-mock-fedora-inspect-library.c b/tests/x86_64-mock-fedora-inspect-library.c index 87fbb04aa..9e2ea43d4 100644 --- a/tests/x86_64-mock-fedora-inspect-library.c +++ b/tests/x86_64-mock-fedora-inspect-library.c @@ -25,22 +25,23 @@ #include <stdio.h> #include <stdlib.h> +#include <string.h> #include <glib.h> int main (int argc, char **argv) { - g_return_val_if_fail (argc == 2, EXIT_FAILURE); + g_return_val_if_fail (argc == 3, EXIT_FAILURE); + g_return_val_if_fail (strcmp (argv[1], "--line-based") == 0, EXIT_FAILURE); gchar **envp = g_get_environ (); - gchar *path = g_build_filename (g_environ_getenv (envp, "SRT_TEST_SYSROOT"), "usr", "lib64", argv[1], NULL); + gchar *path = g_build_filename (g_environ_getenv (envp, "SRT_TEST_SYSROOT"), "usr", "lib64", argv[2], NULL); - /* Return a JSON like if we found the given soname in a canonical Fedora style, 64bit lib folder */ - printf ("{\n\t\"%s\": {\n" - "\t\t\"path\": \"%s\"\n" - "\t}\n" - "}\n", argv[1], path); + /* Return as though we found the given soname in a canonical Fedora style, + * x86_64 lib directory */ + printf ("requested=%s\n", argv[2]); + printf ("path=%s\n", path); g_free (path); g_strfreev (envp); return 0; -- GitLab