From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd42.google.com (mail-io1-xd42.google.com [IPv6:2607:f8b0:4864:20::d42]) by sourceware.org (Postfix) with ESMTPS id 39DBA385E029 for ; Thu, 26 Mar 2020 12:02:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 39DBA385E029 Received: by mail-io1-xd42.google.com with SMTP id q128so5696035iof.9 for ; Thu, 26 Mar 2020 05:02:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=htX18wMCSdZLdN/AMoyVLpJKO2JGmIar87G0virDoPg=; b=lH4qNTMADLicnP7MyfbJQNomSsXUDyU215tXWKYfc+x8rpfmzn0Ae4NZ+J8uReHxOh Sq2ydSh5439fYwwbVn/ZoGsT7QcmNLVkY15ZtT/Os9KaLoJq5PymbYqqjeUZ9TkI1qnF 0cIEOmTjjMoc7EzPzW4NdJcvYOB/v3w8Zd6eZzp7BJjDmvVdUulKzoOmjVR8kHY06s0P vrmLok4pUvi73cV6yOhCJUkErphzaP4RjyxiP3vWoszgCg1QiNOuilFZ+CKmhZWWLfHu EayWwJzil9ho9mwU5SmmFytEduJZkN3usm9dTKpFFmLXkH9k4z6anOrapFiNdzGSIJOG 7q1A== X-Gm-Message-State: ANhLgQ2KrG95caN4rhTog2ZlGPk3FhOxhygqKDFN+nDxWXjJsA9iLAPA fT0rUEBuLuIRkZ9k2FVOwjRJQb8qAZA9oQtALB+YbA== X-Google-Smtp-Source: ADFU+vtkpF6WAsjx6QvWMq568iZS3kNj+eeXgzXBVgMj3cXOJjJPUORx0+NXMsLFQM6LrSExpy+C4GABxnSwQVLflBI= X-Received: by 2002:a6b:8f11:: with SMTP id r17mr6993444iod.92.1585224118968; Thu, 26 Mar 2020 05:01:58 -0700 (PDT) MIME-Version: 1.0 References: <20200324171344.258865-1-gprocida@google.com> <20200326103717.GC153882@google.com> In-Reply-To: <20200326103717.GC153882@google.com> From: Giuliano Procida Date: Thu, 26 Mar 2020 12:01:40 +0000 Message-ID: Subject: Re: [RFC] Fix has_net_changes for leaf-changes-only mode. To: Matthias Maennich Cc: libabigail@sourceware.org, Dodji Seketeli , kernel-team@android.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-37.9 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libabigail mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Mar 2020 12:02:01 -0000 Hi Matthias. On Thu, 26 Mar 2020 at 10:37, Matthias Maennich wrote: > > On Tue, Mar 24, 2020 at 05:13:44PM +0000, Giuliano Procida wrote: > >* Which other functions need the same treatment? > >* Should something else change instead? > >* What about tracking of changed types (inclusion of these in summary > > stats and exit code for leaf mode)? > > > >This patch breaks tests and is not suitable for applying in its > >current form. > > > >The issue we saw is that anonymous struct name changes triggered > >hundreds of diffs (mostly but not all filtered out in non-leaf mode > >which may be its own bug). > > > > * src/abg-comparison.cc (corpus_diff::has_net_changes): In > > leaf-changes-only mode, check leaf changes to functions and > > variables to avoid returning non-zero exit code with empty > > output. > > > > That patch looks generally good to me. Do you have an idea how to > address the test breakages? Here's an example (the same without --impacted-interfaces): $ build/tools/abidiff --impacted-interfaces --leaf-changes-only tests/data/test-abidiff-exit/test-leaf2-v?.o Leaf changes summary: 1 artifact changed Changed leaf types summary: 1 leaf type changed Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable 'struct ops at test-leaf2-v0.cc:1:1' changed: type size hasn't changed there are data member changes: type 'void (int)*' of 'ops::munge' changed: pointer type changed from: 'void (int)*' to: 'char (long int, bool)*' one impacted interface: function void register_ops(const ops&) $ echo $? 0 Exit code 0 is wrong. We should be counting and reporting changed types and using the count as part of the exit code. > >Signed-off-by: Giuliano Procida > >--- > > src/abg-comparison.cc | 33 +++++++++++++++++++-------------- > > 1 file changed, 19 insertions(+), 14 deletions(-) > > > >diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc > >index 46bf9e30..7756c12b 100644 > >--- a/src/abg-comparison.cc > >+++ b/src/abg-comparison.cc > >@@ -10602,21 +10602,26 @@ corpus_diff::has_net_changes() const > > const diff_stats& stats = const_cast(this)-> > > apply_filters_and_suppressions_before_reporting(); > > > >+ bool leaf = context()->show_leaf_changes_only(); > > const I think it's OK as-is. There are no instances of local "const bool" in the code. This bool is only used in the next expression. > Cheers, > Matthias Regards, Giuliano. > > return (architecture_changed() > >- || soname_changed() > >- || stats.net_num_func_changed() > >- || stats.net_num_vars_changed() > >- || stats.net_num_func_added() > >- || stats.net_num_added_func_syms() > >- || stats.net_num_func_removed() > >- || stats.net_num_removed_func_syms() > >- || stats.net_num_vars_added() > >- || stats.net_num_added_var_syms() > >- || stats.net_num_vars_removed() > >- || stats.net_num_removed_var_syms() > >- || stats.net_num_added_unreachable_types() > >- || stats.net_num_removed_unreachable_types() > >- || stats.net_num_changed_unreachable_types()); > >+ || soname_changed() > >+ || (leaf > >+ ? stats.net_num_leaf_func_changes() > >+ : stats.net_num_func_changed()) > >+ || (leaf > >+ ? stats.net_num_leaf_var_changes() > >+ : stats.net_num_vars_changed()) > >+ || stats.net_num_func_added() > >+ || stats.net_num_added_func_syms() > >+ || stats.net_num_func_removed() > >+ || stats.net_num_removed_func_syms() > >+ || stats.net_num_vars_added() > >+ || stats.net_num_added_var_syms() > >+ || stats.net_num_vars_removed() > >+ || stats.net_num_removed_var_syms() > >+ || stats.net_num_added_unreachable_types() > >+ || stats.net_num_removed_unreachable_types() > >+ || stats.net_num_changed_unreachable_types()); > > } > > > > /// Apply the different filters that are registered to be applied to > >-- > >2.25.1.696.g5e7596f4ac-goog > >