From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::223]) by sourceware.org (Postfix) with ESMTPS id 4AB113858D33 for ; Tue, 28 Feb 2023 11:48:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4AB113858D33 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=seketeli.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=seketeli.org Received: (Authenticated sender: dodji@seketeli.org) by mail.gandi.net (Postfix) with ESMTPSA id 78D4960007; Tue, 28 Feb 2023 11:48:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=seketeli.org; s=gm1; t=1677584916; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5IKNvQ1ofvN/WDcIG0JBSRDujFXuTySNRHVSYOCaxm4=; b=dSjxdzmTELiV5h8okIUIJy4FKeeFJNSZh18xCVldju+O4KdymVDKs2Ao58S9w/hNxMMWho ehT3punyhlwBVPBfM1mzWp3cLYIEHSaTGUvUx582LpSoInh5yGVM4KH96v5/STcan3Xafh VebGRIa+RIY6dUVHbhi/GXCkH9PfbADWSFidgmhK/uiWjzH5AgSY6wJP8MuwHwIeWI0wvg vkh5WMFD1kABVFZFjhGJDsA98qa7VkIYWazTkpN/RE9VeEFuNNUYoRlNb0wE0WauSgjYY8 pj3XsqjcLT583XDbmoIKEOBtd6nxEl5rFDC6e39jHgSCVzoV297zmWw/QpHqkA== Received: by localhost (Postfix, from userid 1000) id CE80F581C79; Tue, 28 Feb 2023 12:48:34 +0100 (CET) From: Dodji Seketeli To: "Guillermo E. Martinez" Cc: "Guillermo E. Martinez via Libabigail" Subject: Re: [PATCH, RFC] abipkgdiff: Fix kernel package detection when comparing Organization: Me, myself and I References: <20230217005743.2607886-1-guillermo.e.martinez@oracle.com> <87sferrlb7.fsf@seketeli.org> <87o7pfrl5l.fsf_-_@seketeli.org> <20230227235021.2qwxmmxx3eek4fqz@kamehouse> X-Operating-System: Fedora 38 X-URL: http://www.seketeli.net/~dodji Date: Tue, 28 Feb 2023 12:48:34 +0100 In-Reply-To: <20230227235021.2qwxmmxx3eek4fqz@kamehouse> (Guillermo E. Martinez's message of "Mon, 27 Feb 2023 17:50:21 -0600") Message-ID: <87bklerox9.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,JMQ_SPF_NEUTRAL,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hello Guillermo, "Guillermo E. Martinez" a =C3=A9crit: > I'm agree with your approach!. Thanks! I have thus applied the patch to the master branch. I am putting below the exact patch that was applied. Cheers, >From ca4f4d894c8fe4ce9eb4599c8b4f02694bdde587 Mon Sep 17 00:00:00 2001 From: "Guillermo E. Martinez" Date: Mon, 27 Feb 2023 19:45:11 +0100 Subject: [PATCH] abipkgdiff: Fix kernel package detection when comparing kA= BIs Using abipkgdiff to analyze kABIs from Oracle Linux packages with CTF debug format, abipkgdiff is not able to identify kernel packages because the naming of OL kernel packages differs from the naming used on other RPM-based distributions. As abipkgdiff fails to see that it's looking at a Linux kernel package, the binaries are analyzed as user space binaries and that is not what we want. This patch addresses the issue by looking for the "vmlinuz" binary inside the package to determine that it's a kernel package. In other words, tools_utils::file_is_kernel_package is changed to look for "vmlinuz" inside the package, rather than look for a particular pattern in the package name of the package. Additionally, when the kernel package contains CTF debug information, the `vmlinux.ctfa' file is not necessarily shipped in the debuginfo package. This patch thus adjusts the search path of that file in that case. * include/abg-tools-utils.h (rpm_contains_file): Declare new function. * src/abg-ctf-reader.cc (ctf::reader::find_ctfa_file): Use `find_file_under_dir' utility function to locate `vmlinux.ctfa' file. (ctf::reader::process_ctf_archive): Adjust dictionary name according to module name, removing characters after dot. * src/abg-tools-utils.cc (file_has_ctf_debug_info): Use `find_file_under_dir' utility function to locate `vmlinux.ctfa' file. (rpm_contains_file): Define new function. (file_is_kernel_package): Use the new `rpm_contains_file' to look for the `vmlinuz' file inside the RPM package. For Debian packages however, we don't keep looking at the naming pattern as we don't yet have a deb_contains_file function. Also, this function now takes the full path to the RPM. (build_corpus_group_from_kernel_dist_under): for CTF, add the `root' directory of the extracted package to the set of directories under which we should look for debug info. * tools/abipkgdiff.cc (maybe_handle_kabi_whitelist_pkg) (create_maps_of_package_content, compare_prepared_package, main): Adjust call to file_is_kernel_package as it now takes the full path to the package. Signed-off-by: Guillermo E. Martinez Signed-off-by: Dodji Seketeli --- include/abg-tools-utils.h | 4 +++ src/abg-ctf-reader.cc | 16 ++++------- src/abg-tools-utils.cc | 60 ++++++++++++++++++++++++++++++++------- tools/abipkgdiff.cc | 19 ++++++------- 4 files changed, 69 insertions(+), 30 deletions(-) diff --git a/include/abg-tools-utils.h b/include/abg-tools-utils.h index 70745b0f..43bf6a3e 100644 --- a/include/abg-tools-utils.h +++ b/include/abg-tools-utils.h @@ -300,6 +300,10 @@ bool file_is_kernel_package(const string& file_path, file_type file_type); =20 +bool +rpm_contains_file(const string& rpm_path, + const string& file_name); + bool file_is_kernel_debuginfo_package(const string& file_path, file_type file_type); diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc index 54257d11..7db8dbc6 100644 --- a/src/abg-ctf-reader.cc +++ b/src/abg-ctf-reader.cc @@ -336,12 +336,8 @@ public: // for vmlinux.ctfa should be provided with --debug-info-dir // option. for (const auto& path : debug_info_root_paths()) - { - ctfa_dirname =3D *path; - ctfa_file =3D ctfa_dirname + "/vmlinux.ctfa"; - if (file_exists(ctfa_file)) - return true; - } + if (tools_utils::find_file_under_dir(*path, "vmlinux.ctfa", ctfa_fil= e)) + return true; =20 return false; } @@ -428,10 +424,10 @@ public: && corpus_group()) { tools_utils::base_name(corpus_path(), dict_name); - - if (dict_name !=3D "vmlinux") - // remove .ko suffix - dict_name.erase(dict_name.length() - 3, 3); + // remove .* suffix + std::size_t pos =3D dict_name.find("."); + if (pos !=3D string::npos) + dict_name.erase(pos); =20 std::replace(dict_name.begin(), dict_name.end(), '-', '_'); } diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc index 81f9aa75..85719cdc 100644 --- a/src/abg-tools-utils.cc +++ b/src/abg-tools-utils.cc @@ -501,7 +501,7 @@ file_has_ctf_debug_info(const string& elf_file_path, =20 // vmlinux.ctfa could be provided with --debug-info-dir for (const auto& path : debug_info_root_paths) - if (dir_contains_ctf_archive(*path, vmlinux)) + if (find_file_under_dir(*path, "vmlinux.ctfa", vmlinux)) return true; =20 return false; @@ -1764,34 +1764,64 @@ get_rpm_arch(const string& str, string& arch) =20 /// Tests if a given file name designates a kernel package. /// -/// @param file_name the file name to consider. +/// @param file_path the path to the file to consider. /// /// @param file_type the type of the file @p file_name. /// /// @return true iff @p file_name of kind @p file_type designates a /// kernel package. bool -file_is_kernel_package(const string& file_name, file_type file_type) +file_is_kernel_package(const string& file_path, file_type file_type) { bool result =3D false; - string package_name; =20 if (file_type =3D=3D FILE_TYPE_RPM) { - if (!get_rpm_name(file_name, package_name)) - return false; - result =3D (package_name =3D=3D "kernel"); + if (rpm_contains_file(file_path, "vmlinuz")) + result =3D true; } else if (file_type =3D=3D FILE_TYPE_DEB) { - if (!get_deb_name(file_name, package_name)) - return false; - result =3D (string_begins_with(package_name, "linux-image")); + string file_name; + base_name(file_path, file_name); + string package_name; + if (get_deb_name(file_name, package_name)) + result =3D (string_begins_with(package_name, "linux-image")); } =20 return result; } =20 +/// Test if an RPM package contains a given file. +/// +/// @param rpm_path the path to the RPM package. +/// +/// @param file_name the file name to test the presence for in the +/// rpm. +/// +/// @return true iff the file named @file_name is present in the RPM. +bool +rpm_contains_file(const string& rpm_path, const string& file_name) +{ + vector query_output; + // We don't check the return value of this command because on some + // system, the command can issue errors but still emit a valid + // output. We'll rather rely on the fact that the command emits a + // valid output or not. + execute_command_and_get_output("rpm -qlp " + + rpm_path + " 2> /dev/null", + query_output); + + for (auto& line : query_output) + { + line =3D trim_white_space(line); + if (string_ends_with(line, file_name)) + return true; + } + + return false; +} + /// Tests if a given file name designates a kernel debuginfo package. /// /// @param file_name the file name to consider. @@ -2812,6 +2842,16 @@ build_corpus_group_from_kernel_dist_under(const stri= ng& root, vector di_roots; di_roots.push_back(&di_root_ptr); =20 +#ifdef WITH_CTF + shared_ptr di_root_ctf; + if (requested_fe_kind & corpus::CTF_ORIGIN) + { + di_root_ctf =3D make_path_absolute(root.c_str()); + char *di_root_ctf_ptr =3D di_root_ctf.get(); + di_roots.push_back(&di_root_ctf_ptr); + } +#endif + abigail::elf_based_reader_sptr reader =3D create_best_elf_based_reader(vmlinux, di_roots, diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc index a4b4f1a1..c2fc09ca 100644 --- a/tools/abipkgdiff.cc +++ b/tools/abipkgdiff.cc @@ -2022,8 +2022,8 @@ maybe_handle_kabi_whitelist_pkg(const package& pkg, o= ptions &opts) if (pkg.type() !=3D abigail::tools_utils::FILE_TYPE_RPM) return false; =20 - string pkg_name =3D pkg.base_name(); - bool is_linux_kernel_package =3D file_is_kernel_package(pkg_name, pkg.ty= pe()); + bool is_linux_kernel_package =3D file_is_kernel_package(pkg.path(), + pkg.type()); =20 if (!is_linux_kernel_package) return false; @@ -2036,7 +2036,7 @@ maybe_handle_kabi_whitelist_pkg(const package& pkg, o= ptions &opts) return false; =20 string rpm_arch; - if (!get_rpm_arch(pkg_name, rpm_arch)) + if (!get_rpm_arch(pkg.base_name(), rpm_arch)) return false; =20 string kabi_wl_path =3D kabi_wl_pkg->extracted_dir_path(); @@ -2412,8 +2412,7 @@ create_maps_of_package_content(package& package, opti= ons& opts) // if package is linux kernel package and its associated debug // info package looks like a kernel debuginfo package, then try to // go find the vmlinux file in that debug info file. - string pkg_name =3D package.base_name(); - bool is_linux_kernel_package =3D file_is_kernel_package(pkg_name, + bool is_linux_kernel_package =3D file_is_kernel_package(package.path(), package.type()); if (is_linux_kernel_package) { @@ -3219,7 +3218,7 @@ compare_prepared_package(package& first_package, pack= age& second_package, { abidiff_status status =3D abigail::tools_utils::ABIDIFF_OK; =20 - if (abigail::tools_utils::file_is_kernel_package(first_package.base_name= (), + if (abigail::tools_utils::file_is_kernel_package(first_package.path(), first_package.type())) { opts.show_symbols_not_referenced_by_debug_info =3D false; @@ -3737,14 +3736,14 @@ main(int argc, char* argv[]) | abigail::tools_utils::ABIDIFF_ERROR); } =20 - if (file_is_kernel_package(first_package->base_name(), + if (file_is_kernel_package(first_package->path(), abigail::tools_utils::FILE_TYPE_RPM) - || file_is_kernel_package(second_package->base_name(), + || file_is_kernel_package(second_package->path(), abigail::tools_utils::FILE_TYPE_RPM)) { - if (file_is_kernel_package(first_package->base_name(), + if (file_is_kernel_package(first_package->path(), abigail::tools_utils::FILE_TYPE_RPM) - !=3D file_is_kernel_package(second_package->base_name(), + !=3D file_is_kernel_package(second_package->path(), abigail::tools_utils::FILE_TYPE_RPM)) { emit_prefix("abipkgdiff", cerr) --=20 2.39.2 --=20 Dodji