From patchwork Thu Mar 2 19:03: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: 65924 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 5F6F43858C54 for ; Thu, 2 Mar 2023 19:03:55 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5F6F43858C54 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1677783835; bh=p65+E2CTcxeBrkfep94xVnbiui173sxRzJMpX4Lo84I=; h=To:Cc:Subject:References:Date:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Help:List-Subscribe:From: Reply-To:From; b=FkNCl+F/V3c8h7At1f0QPy2I9OYkRp6g2MMqDjrKekQsB1kOFh9CqbBq4XUbqMb78 qXO1rwggJbFAZolj52Whqor7i/vEAAN0DsOEVtlavvwvzpAkkFvOg+Gz1iZUouc4hI Ob1qQ4LXF8Nw6tv2Nb/VSn5qtyl+1LdH0cUdoJD4= 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 59F623858D33 for ; Thu, 2 Mar 2023 19:03:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 59F623858D33 Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-410-pjfua6EaN5CP3GcOwFZy9Q-1; Thu, 02 Mar 2023 14:03:45 -0500 X-MC-Unique: pjfua6EaN5CP3GcOwFZy9Q-1 Received: by mail-qt1-f198.google.com with SMTP id k19-20020ac86053000000b003bd0d4e3a50so147104qtm.9 for ; Thu, 02 Mar 2023 11:03:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677783825; h=mime-version:user-agent:message-id:in-reply-to:date:references :organization:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=p65+E2CTcxeBrkfep94xVnbiui173sxRzJMpX4Lo84I=; b=bawXqQAWLoFx4DCv6LB3PS8pEE3B5lzT1pH7hBjYYjjqjhn3ZCcPIkPIhGn/sBdSTB 9fvkvyAaAIrhEWopGiQ/l5qXKDcNy0MGmMK+q1cG3embtkmjKCbAoEWaEFyOdmaW/26A Byp8OlEFE2wmKUEN7JOyfcKSItjXxHt0cgiNBNmAjSxra0GiQIdprlBxU5Nm5djfz4Lv GbVPYF9sryoZiGDA37tAYHKIb9dcwWPyn3ibw9ozAUdOckgGVK2+FNLr7xMCMTF/hw4D MWzQZCOwihDH2sJM16vZgkZ5paO6yYbFzGHDF2QhyLe2qJ8U5z6pYH3tyRicVfBlEW1K qo3g== X-Gm-Message-State: AO0yUKWQ0vmI4GWvkEB4MZysrPRorSdNor7yPjZJ6/2vRnJzc47ndjDE F695hIeL/4Ec8tdYT6ydQ2kouorgzJxJJBOF9/vCEbPk8r/p0gVQ0Rleb9cDbFEz9mTZEkJuErB Kk8NUDwprNB3es/UyHZws X-Received: by 2002:a05:622a:1a13:b0:3bf:c8f9:def4 with SMTP id f19-20020a05622a1a1300b003bfc8f9def4mr20698952qtb.59.1677783824908; Thu, 02 Mar 2023 11:03:44 -0800 (PST) X-Google-Smtp-Source: AK7set/zjJi2lu9PrsqCXfAWW7dvLrJQXciItpJacmixTSEbOGZ4ykkjJKVxj5lT1VdryCnyGjaGdA== X-Received: by 2002:a05:622a:1a13:b0:3bf:c8f9:def4 with SMTP id f19-20020a05622a1a1300b003bfc8f9def4mr20698910qtb.59.1677783824519; Thu, 02 Mar 2023 11:03:44 -0800 (PST) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id e8-20020ac85988000000b003b9bca1e093sm248010qte.27.2023.03.02.11.03.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Mar 2023 11:03:44 -0800 (PST) Received: by localhost (Postfix, from userid 1000) id 85189581C79; Thu, 2 Mar 2023 20:03:42 +0100 (CET) To: Dodji Seketeli Cc: libabigail@sourceware.org, ckalina@redhat.com Subject: [PATCH 11/13] comparison: Add a mode to not apply filters on interface sub-graphs Organization: Red Hat / France References: <877cvzrnws.fsf@redhat.com> <87356nrnmq.fsf@redhat.com> X-Operating-System: Fedora 38 X-URL: http://www.redhat.com Date: Thu, 02 Mar 2023 20:03:42 +0100 In-Reply-To: <87356nrnmq.fsf@redhat.com> (Dodji Seketeli's message of "Thu, 02 Mar 2023 19:53:17 +0100") Message-ID: <87lekfou0h.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, This patch allows to avoid applying filters on interface diff node sub-graphs because those filters are useful for interface impact analysis only, which is not needed in the leaf-node report, for instance. When using the leaf-node report, this capability speeds up corpus_diff::apply_filters_and_suppressions_before_reporting, hence the functions like corpus_diff::{has_incompatible_changes, has_net_subtype_changes} are sped up too. That patch thus adds a --no-change-categorization option to abidiff to avoid doing that change categorization (A.K.A applying filters). * doc/manuals/abidiff.rst: Document the new --no-change-categorization option. * doc/manuals/kmidiff.rst: Likewise. * include/abg-comparison.h (diff_context::perform_change_categorization): Declare new accessor member functions. * src/abg-comparison-priv.h (diff_context::priv::perform_change_categorization_): Add new data member. (diff_context::priv::priv): Initialize the new data member. * src/abg-comparison.cc (diff_context::perform_change_categorization): Define new accessor member functions. (corpus_diff::priv::apply_filters_and_compute_diff_stats): Don't apply filters on the diff node sub-graphs of interfaces when the user requested "no change categorization". * tools/abidiff.cc (options::perform_change_categorization): New data member. (options::options): Initialize the new data member. (display_usage): Add a help string for the new --no-change-categorization. (parse_command_line): Parse the new --no-change-categorization option. (set_diff_context_from_opts): Set the option on the diff context here. * tools/kmidiff.cc(options::perform_change_categorization): New data member. (options::options): Initialize the new data member. (display_usage): Add a help string for the new --no-change-categorization. (parse_command_line): Parse the new --no-change-categorization option. (set_diff_context_from_opts): Set the option on the diff context here. Signed-off-by: Dodji Seketeli --- doc/manuals/abidiff.rst | 12 +++ doc/manuals/kmidiff.rst | 11 +++ include/abg-comparison.h | 6 ++ src/abg-comparison-priv.h | 2 + src/abg-comparison.cc | 168 +++++++++++++++++++++----------------- tools/abidiff.cc | 8 ++ tools/kmidiff.cc | 8 ++ 7 files changed, 140 insertions(+), 75 deletions(-) diff --git a/doc/manuals/abidiff.rst b/doc/manuals/abidiff.rst index 2debc20d..d5dc9699 100644 --- a/doc/manuals/abidiff.rst +++ b/doc/manuals/abidiff.rst @@ -601,6 +601,18 @@ Options This option disables those optimizations. + * ``--no-change-categorization | -x`` + + This option disables the categorization of changes into harmless + and harmful changes. Note that this categorization is a + pre-requisite for the filtering of changes so this option disables + that filtering. The goal of this option is to speed-up the + execution of the program for cases where the graph of changes is + huge and where the user is just interested in looking at, for + instance, leaf node changes without caring about their possible + impact on interfaces. In that case, this option would be used + along with the ``--leaf-changes-only`` one. + * ``--ctf`` When comparing binaries, extract ABI information from `CTF`_ debug diff --git a/doc/manuals/kmidiff.rst b/doc/manuals/kmidiff.rst index 40358b92..0e0b33f1 100644 --- a/doc/manuals/kmidiff.rst +++ b/doc/manuals/kmidiff.rst @@ -178,6 +178,17 @@ Options the :ref:`default suppression specification files ` are loaded . + * ``--no-change-categorization | -x`` + + This option disables the categorization of changes into harmless + and harmful changes. Note that this categorization is a + pre-requisite for the filtering of changes so this option disables + that filtering. The goal of this option is to speed-up the + execution of the program for cases where the graph of changes is + huge and where the user is just interested in looking at, for + instance, leaf node changes without caring about their possible + impact on interfaces. + * ``--ctf`` Extract ABI information from `CTF`_ debug information, if present, diff --git a/include/abg-comparison.h b/include/abg-comparison.h index 506f2bb7..2addb7ac 100644 --- a/include/abg-comparison.h +++ b/include/abg-comparison.h @@ -761,6 +761,12 @@ public: void add_suppressions(const suppr::suppressions_type& supprs); + bool + perform_change_categorization() const; + + void + perform_change_categorization(bool); + void show_leaf_changes_only(bool f); diff --git a/src/abg-comparison-priv.h b/src/abg-comparison-priv.h index 48a01188..29d2d2ac 100644 --- a/src/abg-comparison-priv.h +++ b/src/abg-comparison-priv.h @@ -189,6 +189,7 @@ struct diff_context::priv corpus_diff_sptr corpus_diff_; ostream* default_output_stream_; ostream* error_output_stream_; + bool perform_change_categorization_; bool leaf_changes_only_; bool forbid_visiting_a_node_twice_; bool reset_visited_diffs_for_each_interface_; @@ -219,6 +220,7 @@ struct diff_context::priv reporter_(), default_output_stream_(), error_output_stream_(), + perform_change_categorization_(true), leaf_changes_only_(), forbid_visiting_a_node_twice_(true), reset_visited_diffs_for_each_interface_(), diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc index dc451868..886e48fd 100644 --- a/src/abg-comparison.cc +++ b/src/abg-comparison.cc @@ -1532,6 +1532,21 @@ diff_context::add_suppressions(const suppressions_type& supprs) supprs.begin(), supprs.end()); } +/// Test if it's requested to perform diff node categorization. +/// +/// @return true iff it's requested to perform diff node +/// categorization. +bool +diff_context::perform_change_categorization() const +{return priv_->perform_change_categorization_;} + +/// Request change categorization or not. +/// +/// @param f true iff change categorization is requested. +void +diff_context::perform_change_categorization(bool f) +{priv_->perform_change_categorization_ = f;} + /// Set the flag that indicates if the diff using this context should /// show only leaf changes or not. /// @@ -9989,93 +10004,96 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat) diff_context_sptr ctxt = get_context(); tools_utils::timer t; - if (get_context()->do_log()) - { - std::cerr << "in apply_filters_and_compute_diff_stats:" - << "applying filters to " - << changed_fns_.size() - << " changed fns ...\n"; - t.start(); - } - // Walk the changed function diff nodes to apply the categorization - // filters. - diff_sptr diff; - for (function_decl_diff_sptrs_type::const_iterator i = - changed_fns_.begin(); - i != changed_fns_.end(); - ++i) + if (ctxt->perform_change_categorization()) { - diff_sptr diff = *i; - ctxt->maybe_apply_filters(diff); - } + if (get_context()->do_log()) + { + std::cerr << "in apply_filters_and_compute_diff_stats:" + << "applying filters to " + << changed_fns_.size() + << " changed fns ...\n"; + t.start(); + } + // Walk the changed function diff nodes to apply the categorization + // filters. + diff_sptr diff; + for (function_decl_diff_sptrs_type::const_iterator i = + changed_fns_.begin(); + i != changed_fns_.end(); + ++i) + { + diff_sptr diff = *i; + ctxt->maybe_apply_filters(diff); + } - if (get_context()->do_log()) - { - t.stop(); - std::cerr << "in apply_filters_and_compute_diff_stats:" - << "filters to changed fn applied!:" << t << "\n"; + if (get_context()->do_log()) + { + t.stop(); + std::cerr << "in apply_filters_and_compute_diff_stats:" + << "filters to changed fn applied!:" << t << "\n"; - std::cerr << "in apply_filters_and_compute_diff_stats:" - << "applying filters to " - << sorted_changed_vars_.size() - << " changed vars ...\n"; - t.start(); - } + std::cerr << "in apply_filters_and_compute_diff_stats:" + << "applying filters to " + << sorted_changed_vars_.size() + << " changed vars ...\n"; + t.start(); + } - // Walk the changed variable diff nodes to apply the categorization - // filters. - for (var_diff_sptrs_type::const_iterator i = sorted_changed_vars_.begin(); - i != sorted_changed_vars_.end(); - ++i) - { - diff_sptr diff = *i; - ctxt->maybe_apply_filters(diff); - } + // Walk the changed variable diff nodes to apply the categorization + // filters. + for (var_diff_sptrs_type::const_iterator i = sorted_changed_vars_.begin(); + i != sorted_changed_vars_.end(); + ++i) + { + diff_sptr diff = *i; + ctxt->maybe_apply_filters(diff); + } - if (get_context()->do_log()) - { - t.stop(); - std::cerr << "in apply_filters_and_compute_diff_stats:" - << "filters to changed vars applied!:" << t << "\n"; + if (get_context()->do_log()) + { + t.stop(); + std::cerr << "in apply_filters_and_compute_diff_stats:" + << "filters to changed vars applied!:" << t << "\n"; - std::cerr << "in apply_filters_and_compute_diff_stats:" - << "applying filters to unreachable types ...\n"; - t.start(); - } + std::cerr << "in apply_filters_and_compute_diff_stats:" + << "applying filters to unreachable types ...\n"; + t.start(); + } - // walk the changed unreachable types to apply categorization - // filters - for (diff_sptrs_type::const_iterator i = - changed_unreachable_types_sorted().begin(); - i != changed_unreachable_types_sorted().end(); - ++i) - { - diff_sptr diff = *i; - ctxt->maybe_apply_filters(diff); - } + // walk the changed unreachable types to apply categorization + // filters + for (diff_sptrs_type::const_iterator i = + changed_unreachable_types_sorted().begin(); + i != changed_unreachable_types_sorted().end(); + ++i) + { + diff_sptr diff = *i; + ctxt->maybe_apply_filters(diff); + } - if (get_context()->do_log()) - { - t.stop(); - std::cerr << "in apply_filters_and_compute_diff_stats:" - << "filters to unreachable types applied!:" << t << "\n"; + if (get_context()->do_log()) + { + t.stop(); + std::cerr << "in apply_filters_and_compute_diff_stats:" + << "filters to unreachable types applied!:" << t << "\n"; - std::cerr << "in apply_filters_and_compute_diff_stats:" - << "categorizing redundant changed sub nodes ...\n"; - t.start(); - } + std::cerr << "in apply_filters_and_compute_diff_stats:" + << "categorizing redundant changed sub nodes ...\n"; + t.start(); + } - categorize_redundant_changed_sub_nodes(); + categorize_redundant_changed_sub_nodes(); - if (get_context()->do_log()) - { - t.stop(); - std::cerr << "in apply_filters_and_compute_diff_stats:" - << "redundant changed sub nodes categorized!:" << t << "\n"; + if (get_context()->do_log()) + { + t.stop(); + std::cerr << "in apply_filters_and_compute_diff_stats:" + << "redundant changed sub nodes categorized!:" << t << "\n"; - std::cerr << "in apply_filters_and_compute_diff_stats:" - << "count changed fns ...\n"; - t.start(); + std::cerr << "in apply_filters_and_compute_diff_stats:" + << "count changed fns ...\n"; + t.start(); + } } // Walk the changed function diff nodes to count the number of diff --git a/tools/abidiff.cc b/tools/abidiff.cc index a0a670cb..3613a4a3 100644 --- a/tools/abidiff.cc +++ b/tools/abidiff.cc @@ -114,6 +114,7 @@ struct options bool show_impacted_interfaces; bool assume_odr_for_cplusplus; bool leverage_dwarf_factorization; + bool perform_change_categorization; bool dump_diff_tree; bool show_stats; bool do_log; @@ -170,6 +171,7 @@ struct options show_impacted_interfaces(), assume_odr_for_cplusplus(true), leverage_dwarf_factorization(true), + perform_change_categorization(true), dump_diff_tree(), show_stats(), do_log() @@ -276,6 +278,8 @@ display_usage(const string& prog_name, ostream& out) << " --impacted-interfaces display interfaces impacted by leaf changes\n" << " --no-leverage-dwarf-factorization do not use DWZ optimisations to " "speed-up the analysis of the binary\n" + << " --no-change-categorization | -x don't perform categorization " + "of changes, for speed purposes\n" << " --no-assume-odr-for-cplusplus do not assume the ODR to speed-up the " "analysis of the binary\n" << " --dump-diff-tree emit a debug dump of the internal diff tree to " @@ -641,6 +645,9 @@ parse_command_line(int argc, char* argv[], options& opts) opts.show_impacted_interfaces = true; else if (!strcmp(argv[i], "--no-leverage-dwarf-factorization")) opts.leverage_dwarf_factorization = false; + else if (!strcmp(argv[i], "--no-change-categorization") + || !strcmp(argv[i], "-x")) + opts.perform_change_categorization = false; else if (!strcmp(argv[i], "--no-assume-odr-for-cplusplus")) opts.leverage_dwarf_factorization = false; else if (!strcmp(argv[i], "--dump-diff-tree")) @@ -752,6 +759,7 @@ set_diff_context_from_opts(diff_context_sptr ctxt, { ctxt->default_output_stream(&cout); ctxt->error_output_stream(&cerr); + ctxt->perform_change_categorization(opts.perform_change_categorization); ctxt->show_leaf_changes_only(opts.leaf_changes_only); ctxt->show_hex_values(opts.show_hexadecimal_values); ctxt->show_offsets_sizes_in_bits(opts.show_offsets_sizes_in_bits); diff --git a/tools/kmidiff.cc b/tools/kmidiff.cc index f00895a3..f07e4c59 100644 --- a/tools/kmidiff.cc +++ b/tools/kmidiff.cc @@ -56,6 +56,7 @@ struct options bool display_version; bool verbose; bool missing_operand; + bool perform_change_categorization; bool leaf_changes_only; bool show_hexadecimal_values; bool show_offsets_sizes_in_bits; @@ -84,6 +85,7 @@ struct options display_version(), verbose(), missing_operand(), + perform_change_categorization(true), leaf_changes_only(true), show_hexadecimal_values(true), show_offsets_sizes_in_bits(false), @@ -128,6 +130,8 @@ display_usage(const string& prog_name, ostream& out) #ifdef WITH_BTF << " --btf use BTF instead of DWARF in ELF files\n" #endif + << " --no-change-categorization | -x don't perform categorization " + "of changes, for speed purposes\n" << " --impacted-interfaces|-i show interfaces impacted by ABI changes\n" << " --full-impact|-f show the full impact of changes on top-most " "interfaces\n" @@ -274,6 +278,9 @@ parse_command_line(int argc, char* argv[], options& opts) else if (!strcmp(argv[i], "--btf")) opts.use_btf = true; #endif + else if (!strcmp(argv[i], "--no-change-categorization") + || !strcmp(argv[i], "-x")) + opts.perform_change_categorization = false; else if (!strcmp(argv[i], "--impacted-interfaces") || !strcmp(argv[i], "-i")) opts.show_impacted_interfaces = true; @@ -344,6 +351,7 @@ set_diff_context(diff_context_sptr ctxt, const options& opts) ctxt->show_linkage_names(false); ctxt->show_symbols_unreferenced_by_debug_info (true); + ctxt->perform_change_categorization(opts.perform_change_categorization); ctxt->show_leaf_changes_only(opts.leaf_changes_only); ctxt->show_impacted_interfaces(opts.show_impacted_interfaces); ctxt->show_hex_values(opts.show_hexadecimal_values);