From patchwork Mon Feb 27 18:57:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 65691 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6B5AE3858C2B for ; Mon, 27 Feb 2023 18:57:50 +0000 (GMT) X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by sourceware.org (Postfix) with ESMTPS id 2FFBD3858D39 for ; Mon, 27 Feb 2023 18:57:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2FFBD3858D39 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 69C9A60004; Mon, 27 Feb 2023 18:57:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=seketeli.org; s=gm1; t=1677524263; 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: in-reply-to:in-reply-to:references:references; bh=jxCJSAFzEQkn5It9yBP1yPwAuD397pFHTMQ9p4WLbuM=; b=i6/qWwIw//67vc1Tixixujh3YoGPHBjWKPrJctjmEEqtz2nPyCNLbxC4SPz9Sl4PqmR3Rd JHacDq7b7JZlc2B5lbPr8WZWZsnq0SIfe+GDsSV4Osjv3GqjNG4bHeOklqXv6tAqhj9IhC uL/xTpcyrbbVLEaYkmFT1ZijQ/4w+NNt0UpRDsKSOD6NSpR2UfhqgbomKsdrAOKpZjZohA BmvyHtQiEr5fWXZV82Yw/IAUK6+DA5sx+LNMmpiT+zAeyM+YOIShZtgj4Zf0kiCZpeHShd K+n79iEtIVx2nTFY2AJFoa/aE6m9XpA0J5+lTThS8GvJjwiAgtxNQK010pG4iQ== Received: by localhost (Postfix, from userid 1000) id 9C987581C79; Mon, 27 Feb 2023 19:57:42 +0100 (CET) From: Dodji Seketeli To: "Guillermo E. Martinez via Libabigail" Cc: "Guillermo E. Martinez" Subject: [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> X-Operating-System: Fedora 38 X-URL: http://www.seketeli.net/~dodji Date: Mon, 27 Feb 2023 19:57:42 +0100 In-Reply-To: <87sferrlb7.fsf@seketeli.org> (Dodji Seketeli's message of "Mon, 27 Feb 2023 19:54:20 +0100") Message-ID: <87o7pfrl5l.fsf_-_@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 X-Spam-Status: No, score=-10.2 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, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, 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 X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" Hello Guillermo, Here is the amended patch that I am proposing to address the issue you reported. I try to fix the problem of tools_utils::file_is_kernel_package first, then I added your CTF-specific fixes on top. Can you please tell me what you think about this approach? Thanks. 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 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 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); +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 = *path; - ctfa_file = ctfa_dirname + "/vmlinux.ctfa"; - if (file_exists(ctfa_file)) - return true; - } + if (tools_utils::find_file_under_dir(*path, "vmlinux.ctfa", ctfa_file)) + return true; return false; } @@ -428,10 +424,10 @@ public: && corpus_group()) { tools_utils::base_name(corpus_path(), dict_name); - - if (dict_name != "vmlinux") - // remove .ko suffix - dict_name.erase(dict_name.length() - 3, 3); + // remove .* suffix + std::size_t pos = dict_name.find("."); + if (pos != string::npos) + dict_name.erase(pos); 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, // 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; return false; @@ -1764,34 +1764,64 @@ get_rpm_arch(const string& str, string& arch) /// 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 = false; - string package_name; if (file_type == FILE_TYPE_RPM) { - if (!get_rpm_name(file_name, package_name)) - return false; - result = (package_name == "kernel"); + if (rpm_contains_file(file_path, "vmlinuz")) + result = true; } else if (file_type == FILE_TYPE_DEB) { - if (!get_deb_name(file_name, package_name)) - return false; - result = (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 = (string_begins_with(package_name, "linux-image")); } return result; } +/// 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 = 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 string& root, vector di_roots; di_roots.push_back(&di_root_ptr); +#ifdef WITH_CTF + shared_ptr di_root_ctf; + if (requested_fe_kind & corpus::CTF_ORIGIN) + { + di_root_ctf = make_path_absolute(root.c_str()); + char *di_root_ctf_ptr = di_root_ctf.get(); + di_roots.push_back(&di_root_ctf_ptr); + } +#endif + abigail::elf_based_reader_sptr reader = 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, options &opts) if (pkg.type() != abigail::tools_utils::FILE_TYPE_RPM) return false; - string pkg_name = pkg.base_name(); - bool is_linux_kernel_package = file_is_kernel_package(pkg_name, pkg.type()); + bool is_linux_kernel_package = file_is_kernel_package(pkg.path(), + pkg.type()); if (!is_linux_kernel_package) return false; @@ -2036,7 +2036,7 @@ maybe_handle_kabi_whitelist_pkg(const package& pkg, options &opts) return false; string rpm_arch; - if (!get_rpm_arch(pkg_name, rpm_arch)) + if (!get_rpm_arch(pkg.base_name(), rpm_arch)) return false; string kabi_wl_path = kabi_wl_pkg->extracted_dir_path(); @@ -2412,8 +2412,7 @@ create_maps_of_package_content(package& package, options& 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 = package.base_name(); - bool is_linux_kernel_package = file_is_kernel_package(pkg_name, + bool is_linux_kernel_package = file_is_kernel_package(package.path(), package.type()); if (is_linux_kernel_package) { @@ -3219,7 +3218,7 @@ compare_prepared_package(package& first_package, package& second_package, { abidiff_status status = abigail::tools_utils::ABIDIFF_OK; - 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 = false; @@ -3737,14 +3736,14 @@ main(int argc, char* argv[]) | abigail::tools_utils::ABIDIFF_ERROR); } - 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) - != file_is_kernel_package(second_package->base_name(), + != file_is_kernel_package(second_package->path(), abigail::tools_utils::FILE_TYPE_RPM)) { emit_prefix("abipkgdiff", cerr)