From patchwork Wed Apr 5 16:04:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 67396 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 33C5A3858D38 for ; Wed, 5 Apr 2023 16:04:50 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 33C5A3858D38 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1680710690; bh=Fw5s9o8+Rf7YvA+EBkbF5ZR4PVvD4P0Z5S9P2Gv5CBo=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Help: List-Subscribe:From:Reply-To:From; b=DiuZfDfF7K5MCiEhu+zA0YJGc8yLqZND/M1T5kOaEW3fx/SXasSu3GlrW1ykBPqt4 8dtsHDRWkV1uuux27mJAMylGWm9D6Y83lgYhxA98skSi0tMV2CAHxXao/aEUbLUTVo RkoVicFni83X/4SjPy1yCi+dBWr0tRdUD0tlaPMw= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 5888E3858D20 for ; Wed, 5 Apr 2023 16:04:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5888E3858D20 Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-572-WcAbQmB_PH6HMG0mUXMA9Q-1; Wed, 05 Apr 2023 12:04:35 -0400 X-MC-Unique: WcAbQmB_PH6HMG0mUXMA9Q-1 Received: by mail-qk1-f198.google.com with SMTP id x80-20020a376353000000b0074681bc7f42so16420047qkb.8 for ; Wed, 05 Apr 2023 09:04:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680710674; h=mime-version:user-agent:message-id:date:organization:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Fw5s9o8+Rf7YvA+EBkbF5ZR4PVvD4P0Z5S9P2Gv5CBo=; b=iou0yCS5x2Mt4HcB8EWUtesZPjSI/LORTyVaq8XXUTmx9CNLfkpStczY7Xb9Xv5pwn y3iVmfjFKwpqcIKM2Aim59ES7qLZCksrEjyKGsZvgOvhIdlClD8nJfgPfiXWrw6eE06T zRSJdsrt+JLT8kOk0PgDMNJkeCWkZruu3bun25YYPpwUbm9fhplKkN1Vr2x+jByBuOJG 25V6ihQ/hfzHl8N4/pkoct8udf6nofsAWNKNxFaWuE45DbM+/IjVqdEWhZmdYibczsNs Vl01YQSLAkey2/OuJj04q043TYpOLV7cPdEoIjrzgZUJvn9KGMVab+nv4WjE4Em6XWMr D7tA== X-Gm-Message-State: AAQBX9eaE227KXwCbu718XUAJnTEjFFP90DYmHHvX+N5RosUxDz1zB/p xDIIeUY1Q7mpkI6iHi3ThKbfVxHrHxfeLUyZSy4ACP7ziEtonf9g2rC8rTjFzxhSEioCBXabY+S qkn8MuJyk1RUruTyYlcxnK/xJht8+rVKxH1CPPsEzaE+3teObWN6SKl6c2uVs8J9TFSIVBSB03j Z7 X-Received: by 2002:ac8:4e42:0:b0:3bf:d9ee:882d with SMTP id e2-20020ac84e42000000b003bfd9ee882dmr6492140qtw.40.1680710673495; Wed, 05 Apr 2023 09:04:33 -0700 (PDT) X-Google-Smtp-Source: AKy350bpuQADJIn6WYW6DtDK1+MoIB7YNUO7HgldPEEAZSM82H5kZbArriUWOgX7SbTPOM9L9RyJ+g== X-Received: by 2002:ac8:4e42:0:b0:3bf:d9ee:882d with SMTP id e2-20020ac84e42000000b003bfd9ee882dmr6492045qtw.40.1680710672737; Wed, 05 Apr 2023 09:04:32 -0700 (PDT) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id t7-20020a37aa07000000b0074a0a47a1f3sm4572508qke.5.2023.04.05.09.04.31 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Apr 2023 09:04:32 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id 28BD6581C79; Wed, 5 Apr 2023 18:04:29 +0200 (CEST) To: libabigail@sourceware.org Subject: [PATCH, applied] Bug rhbz#2182807 -- abipkgdiff crashes on missing debuginfo package Organization: Red Hat / France X-Operating-System: Fedora 38 X-URL: http://www.redhat.com Date: Wed, 05 Apr 2023 18:04:29 +0200 Message-ID: <87ttxul3hu.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, 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: , X-Patchwork-Original-From: Dodji Seketeli via Libabigail From: Dodji Seketeli Reply-To: Dodji Seketeli Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" Hello, When abipkgdiff is called with a debug info package that references an alternate debug info file that is not found -- because debug info package is missing from the command line -- the program aborts. This is because the libabigail library is further invoked by the tool with debuginfo in an inconsistent state (missing alternate debug info). Note however that abipkgdiff only emits an explanatory message when invoked with the --verbose option. This patch teaches abipkgdiff to emit explanatory messages when an alternate debug info file is not found. The message suggests that the user adds the missing RPM package (which contains the alternate missing debuginfo file) to the command line using the --d1/--d2 switches. * src/abg-fe-iface.cc (status_to_diagnostic_string): Remove the newline from the end of the returned diagnostic string. * tools/abipkgdiff.cc (get_pretty_printed_list_of_packages) (emit_alt_debug_info_not_found_error): Define new static functions. (compare, compare_to_self): Add an ostream& parameter as a destination of diagnostic messages. If the abigail::fe_iface::STATUS_ALT_DEBUG_INFO_NOT_FOUND bit is set in the status code, emit the explanatory message to output stream associated to the current comparison task. Remove the previous handling of this case. (compare_task::maybe_emit_pretty_error_message_to_output): Define new member function to get the output stream associated to the current task massage it and stick to result into the pretty output string to be emitted to the user in the end, namely the compare_task::pretty_output string data member. ({compares, self_compare}_task::perform): Adjust this to call the new maybe_emit_pretty_error_message_to_output in case of error. * tests/data/test-diff-pkg/libxfce4ui-devel-4.12.1-8.fc27.ppc64-self-report-0.txt: Adjust. * tests/data/test-diff-pkg/test-dbus-glib-0.80-3.fc12.x86_64-report-0.txt: Likewise. Signed-off-by: Dodji Seketeli --- src/abg-fe-iface.cc | 6 +- ...evel-4.12.1-8.fc27.ppc64-self-report-0.txt | 11 +- ...-dbus-glib-0.80-3.fc12.x86_64-report-0.txt | 2 + tools/abipkgdiff.cc | 288 ++++++++++-------- 4 files changed, 173 insertions(+), 134 deletions(-) diff --git a/src/abg-fe-iface.cc b/src/abg-fe-iface.cc index bc272425..6ec50bfd 100644 --- a/src/abg-fe-iface.cc +++ b/src/abg-fe-iface.cc @@ -397,13 +397,13 @@ status_to_diagnostic_string(fe_iface::status s) std::string str; if (s & fe_iface::STATUS_DEBUG_INFO_NOT_FOUND) - str += "could not find debug info\n"; + str += "could not find debug info"; if (s & fe_iface::STATUS_ALT_DEBUG_INFO_NOT_FOUND) - str += "could not find alternate debug info\n"; + str += "could not find alternate debug info"; if (s & fe_iface::STATUS_NO_SYMBOLS_FOUND) - str += "could not load ELF symbols\n"; + str += "could not load ELF symbols"; return str; } diff --git a/tests/data/test-diff-pkg/libxfce4ui-devel-4.12.1-8.fc27.ppc64-self-report-0.txt b/tests/data/test-diff-pkg/libxfce4ui-devel-4.12.1-8.fc27.ppc64-self-report-0.txt index 61169111..0188bc9b 100644 --- a/tests/data/test-diff-pkg/libxfce4ui-devel-4.12.1-8.fc27.ppc64-self-report-0.txt +++ b/tests/data/test-diff-pkg/libxfce4ui-devel-4.12.1-8.fc27.ppc64-self-report-0.txt @@ -1,4 +1,7 @@ -abipkgdiff: Could not find alternate debug info file: ../../../../.dwz/libxfce4ui-4.12.1-8.fc27.ppc64 -==== Error happened during processing of 'libxfce4uiglade.so' ==== -could not find alternate debug info -==== End of error for 'libxfce4uiglade.so' ==== +abipkgdiff: ==== Error happened during processing of 'libxfce4uiglade.so' ==== +abipkgdiff: could not find alternate debug info: +abipkgdiff: While reading elf file 'libxfce4uiglade.so', could not find alternate debug info in provided debug info package(s) '/home/dodji/git/libabigail/fixes/tests/data/test-diff-pkg/libxfce4ui-devel-debuginfo-4.12.1-8.fc27.ppc64.rpm' +abipkgdiff: The alternate debug info file being looked for is: ../../../../.dwz/libxfce4ui-4.12.1-8.fc27.ppc64 +abipkgdiff: You must provide the additional debug info package that contains that alternate debug info file, using an additional --d1/--d2 switch +abipkgdiff: ==== End of error for 'libxfce4uiglade.so' ==== + diff --git a/tests/data/test-diff-pkg/test-dbus-glib-0.80-3.fc12.x86_64-report-0.txt b/tests/data/test-diff-pkg/test-dbus-glib-0.80-3.fc12.x86_64-report-0.txt index e69de29b..5cb62b12 100644 --- a/tests/data/test-diff-pkg/test-dbus-glib-0.80-3.fc12.x86_64-report-0.txt +++ b/tests/data/test-diff-pkg/test-dbus-glib-0.80-3.fc12.x86_64-report-0.txt @@ -0,0 +1,2 @@ +No ABI change detected +No ABI change detected diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc index 2765284e..d10706a9 100644 --- a/tools/abipkgdiff.cc +++ b/tools/abipkgdiff.cc @@ -281,6 +281,9 @@ get_interesting_files_under_dir(const string dir, options& opts, vector& interesting_files); +static string +get_pretty_printed_list_of_packages(const vector& packages); + /// Abstract ELF files from the packages which ABIs ought to be /// compared class elf_file @@ -1302,6 +1305,68 @@ set_generic_options(abigail::elf_based_reader& rdr, const options& opts) opts.assume_odr_for_cplusplus; } +/// Emit an error message on standard error about alternate debug info +/// not being found. +/// +/// @param reader the ELF based reader being used. +/// +/// @param elf_file the ELF file being looked at. +/// +/// @param opts the options passed to the tool. +/// +/// @param is_old_package if this is true, then we are looking at the +/// first (the old) package of the comparison. Otherwise, we are +/// looking at the second (the newest) package of the comparison. +static void +emit_alt_debug_info_not_found_error(abigail::elf_based_reader& reader, + const elf_file& elf_file, + const options& opts, + ostream& out, + bool is_old_package) +{ + ABG_ASSERT(is_old_package + ? !opts.debug_packages1.empty() + : !opts.debug_packages2.empty()); + + string filename; + tools_utils::base_name(elf_file.path, filename); + emit_prefix("abipkgdiff", out) + << "While reading elf file '" + << filename + << "', could not find alternate debug info in provided " + "debug info package(s) " + << get_pretty_printed_list_of_packages(is_old_package + ? opts.debug_packages1 + : opts.debug_packages2) + << "\n"; + + string alt_di_path; +#ifdef WITH_CTF + if (opts.use_ctf) + ; + else +#endif +#ifdef WITH_BTF + if (opts.use_btf) + ; + else +#endif + reader.refers_to_alt_debug_info(alt_di_path); + if (!alt_di_path.empty()) + { + emit_prefix("abipkgdiff", out) + << "The alternate debug info file being looked for is: " + << alt_di_path << "\n"; + } + else + emit_prefix("abipkgdiff", out) << "\n"; + + emit_prefix("abipkgdiff", out) + << "You must provide the additional " + << "debug info package that contains that alternate " + << "debug info file, using an additional --d1/--d2 switch\n"; +} + /// Compare the ABI two elf files, using their associated debug info. /// /// The result of the comparison is emitted to standard output. @@ -1341,6 +1406,7 @@ compare(const elf_file& elf1, abigail::ir::environment& env, corpus_diff_sptr& diff, diff_context_sptr& ctxt, + ostream& out, abigail::fe_iface::status* detailed_error_status = 0) { char *di_dir1 = (char*) debug_dir1.c_str(), @@ -1437,6 +1503,15 @@ compare(const elf_file& elf1, bail_out = true; } + if (c1_status & abigail::fe_iface::STATUS_ALT_DEBUG_INFO_NOT_FOUND) + { + emit_alt_debug_info_not_found_error(*reader, elf1, opts, out, + /*is_old_package=*/true); + if (detailed_error_status) + *detailed_error_status = c1_status; + bail_out = true; + } + if (opts.fail_if_no_debug_info) { bool debug_info_error = false; @@ -1457,36 +1532,6 @@ compare(const elf_file& elf1, debug_info_error = true; } - if (c1_status & abigail::fe_iface::STATUS_ALT_DEBUG_INFO_NOT_FOUND) - { - if (opts.verbose) - emit_prefix("abipkgdiff", cerr) - << "while reading file" << elf1.path << "\n"; - - emit_prefix("abipkgdiff", cerr) - << "Could not find alternate debug info file"; - string alt_di_path; -#ifdef WITH_CTF - if (opts.use_ctf) - ; - else -#endif -#ifdef WITH_BTF - if (opts.use_btf) - ; - else -#endif - reader->refers_to_alt_debug_info(alt_di_path); - if (!alt_di_path.empty()) - cerr << ": " << alt_di_path << "\n"; - else - cerr << "\n"; - - if (detailed_error_status) - *detailed_error_status = c1_status; - debug_info_error = true; - } - if (debug_info_error) bail_out = true; } @@ -1547,6 +1592,15 @@ compare(const elf_file& elf1, bail_out = true; } + if (c2_status & abigail::fe_iface::STATUS_ALT_DEBUG_INFO_NOT_FOUND) + { + emit_alt_debug_info_not_found_error(*reader, elf2, opts, out, + /*is_old_package=*/false); + if (detailed_error_status) + *detailed_error_status = c2_status; + bail_out = true; + } + if (opts.fail_if_no_debug_info) { bool debug_info_error = false; @@ -1567,36 +1621,6 @@ compare(const elf_file& elf1, debug_info_error = true; } - if (c2_status & abigail::fe_iface::STATUS_ALT_DEBUG_INFO_NOT_FOUND) - { - if (opts.verbose) - emit_prefix("abipkgdiff", cerr) - << "while reading file" << elf2.path << "\n"; - - emit_prefix("abipkgdiff", cerr) - << "Could not find alternate debug info file"; - string alt_di_path; -#ifdef WITH_CTF - if (opts.use_ctf) - ; - else -#endif -#ifdef WITH_BTF - if (opts.use_btf) - ; - else -#endif - reader->refers_to_alt_debug_info(alt_di_path); - if (!alt_di_path.empty()) - cerr << ": " << alt_di_path << "\n"; - else - cerr << "\n"; - - if (detailed_error_status) - *detailed_error_status = c2_status; - debug_info_error = true; - } - if (debug_info_error) bail_out = true; } @@ -1661,6 +1685,7 @@ compare_to_self(const elf_file& elf, abigail::ir::environment& env, corpus_diff_sptr& diff, diff_context_sptr& ctxt, + ostream& out, abigail::fe_iface::status* detailed_error_status = 0) { char *di_dir = (char*) debug_dir.c_str(); @@ -1718,15 +1743,10 @@ compare_to_self(const elf_file& elf, } else if (c_status & abigail::fe_iface::STATUS_ALT_DEBUG_INFO_NOT_FOUND) { - if (opts.verbose) - emit_prefix("abipkgdiff", cerr) - << "Could not read find alternate DWARF debug info for '" - << elf.path - << "'. You might have forgotten to provide a debug info package\n"; - + emit_alt_debug_info_not_found_error(*reader, elf, opts, out, + /*is_old_package=*/true); if (detailed_error_status) *detailed_error_status = c_status; - return abigail::tools_utils::ABIDIFF_ERROR; } @@ -2160,28 +2180,10 @@ public: status(abigail::tools_utils::ABIDIFF_OK) {} - /// The job performed by the task. - /// - /// This compares two ELF files, gets the resulting test report and - /// stores it in an output stream. - virtual void - perform() + void + maybe_emit_pretty_error_message_to_output(const corpus_diff_sptr& diff, + abigail::fe_iface::status detailed_status) { - abigail::ir::environment env; - diff_context_sptr ctxt; - corpus_diff_sptr diff; - - abigail::fe_iface::status detailed_status = - abigail::fe_iface::STATUS_UNKNOWN; - - if (args->opts.exported_interfaces_only.has_value()) - env.analyze_exported_interfaces_only - (*args->opts.exported_interfaces_only); - - status |= compare(args->elf1, args->debug_dir1, args->private_types_suppr1, - args->elf2, args->debug_dir2, args->private_types_suppr2, - args->opts, env, diff, ctxt, &detailed_status); - // If there is an ABI change, tell the user about it. if ((status & abigail::tools_utils::ABIDIFF_ABI_CHANGE) ||( diff && diff->has_net_changes())) @@ -2198,7 +2200,10 @@ public: else { if (args->opts.show_identical_binaries) - out << "No ABI change detected\n"; + { + out << "No ABI change detected\n"; + pretty_output += out.str(); + } } // If an error happened while comparing the two binaries, tell the @@ -2212,13 +2217,47 @@ public: "Unknown error. Please run the tool again with --verbose\n"; string name = args->elf1.name; - pretty_output += - "==== Error happened during processing of '" + name + "' ====\n"; - pretty_output += diagnostic; - pretty_output += - "==== End of error for '" + name + "' ====\n"; + std::stringstream o; + emit_prefix("abipkgdiff", o) + << "==== Error happened during processing of '" + << name + << "' ====\n"; + emit_prefix("abipkgdiff", o) + << diagnostic + << ":\n" + << out.str(); + emit_prefix("abipkgdiff", o) + << "==== End of error for '" + << name + << "' ====\n\n"; + pretty_output += o.str(); } } + + /// The job performed by the task. + /// + /// This compares two ELF files, gets the resulting test report and + /// stores it in an output stream. + virtual void + perform() + { + abigail::ir::environment env; + diff_context_sptr ctxt; + corpus_diff_sptr diff; + + abigail::fe_iface::status detailed_status = + abigail::fe_iface::STATUS_UNKNOWN; + + if (args->opts.exported_interfaces_only.has_value()) + env.analyze_exported_interfaces_only + (*args->opts.exported_interfaces_only); + + status |= compare(args->elf1, args->debug_dir1, args->private_types_suppr1, + args->elf2, args->debug_dir2, args->private_types_suppr2, + args->opts, env, diff, ctxt, out, &detailed_status); + + maybe_emit_pretty_error_message_to_output(diff, detailed_status); + } }; // end class compare_task /// Convenience typedef for a shared_ptr of @ref compare_task. @@ -2252,44 +2291,14 @@ public: abigail::fe_iface::STATUS_UNKNOWN; status |= compare_to_self(args->elf1, args->debug_dir1, - args->opts, env, diff, ctxt, + args->opts, env, diff, ctxt, out, &detailed_status); string name = args->elf1.name; if (status == abigail::tools_utils::ABIDIFF_OK) pretty_output += "==== SELF CHECK SUCCEEDED for '"+ name + "' ====\n"; - else if ((status & abigail::tools_utils::ABIDIFF_ABI_CHANGE) - ||( diff && diff->has_net_changes())) - { - // There is an ABI change, tell the user about it. - diff->report(out, /*indent=*/" "); - - pretty_output += - string("======== comparing'") + name + - "' to itself wrongly yielded result: ===========\n" - + out.str() - + "===SELF CHECK FAILED for '"+ name + "'\n"; - } - - // If an error happened while comparing the two binaries, tell the - // user about it. - if (status & abigail::tools_utils::ABIDIFF_ERROR) - { - string diagnostic = - abigail::status_to_diagnostic_string(detailed_status); - - if (diagnostic.empty()) - diagnostic = - "Unknown error. Please run the tool again with --verbose\n"; - - string name = args->elf1.name; - pretty_output += - "==== Error happened during self check of '" + name + "' ====\n"; - pretty_output += diagnostic; - pretty_output += - "==== SELF CHECK FAILED for '" + name + "' ====\n"; - - } + else + maybe_emit_pretty_error_message_to_output(diff, detailed_status); } }; // end class self_compare @@ -2396,6 +2405,31 @@ get_interesting_files_under_dir(const string dir, return is_ok; } +/// Return a string representing a list of packages that can be +/// printed out to the user. +/// +/// @param packages a vector of package names +/// +/// @return a string representing the list of packages @p packages. +static string +get_pretty_printed_list_of_packages(const vector& packages) +{ + if (packages.empty()) + return string(); + + bool need_comma = false; + std::stringstream o; + for (auto p : packages) + { + if (need_comma) + o << ", "; + else + need_comma = true; + o << "'" << p << "'"; + } + return o.str(); +} + /// Create maps of the content of a given package. /// /// The maps contain relevant metadata about the content of the