From patchwork Wed Apr 26 12:23:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 68303 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 EE1913858C53 for ; Wed, 26 Apr 2023 12:24:09 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EE1913858C53 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1682511850; bh=jjmRGNM3td6lZoDl0YCLd5HZlUD7rFMFAhO/aA6MTmA=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Help: List-Subscribe:From:Reply-To:From; b=IdpJF1VeO9CaPcbKvc2OMtuhA+eMh0Q3LyN89QUFwm+0V/WaTybo9YnB7b3QSdyKu pinIzdcasqrgLXJNtnTBcw4jRYrMn5AETJ4D8E68fUcGI6K03Zhmjxvg36PeF9IsHs O2cE8DYcxS6d8GGEg1wSkxom1oTm7P22u4xBphUs= 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.129.124]) by sourceware.org (Postfix) with ESMTPS id 0994B3858401 for ; Wed, 26 Apr 2023 12:24:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0994B3858401 Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-587-Le2hF9YFPEWXp8LdpCYPZw-1; Wed, 26 Apr 2023 08:24:03 -0400 X-MC-Unique: Le2hF9YFPEWXp8LdpCYPZw-1 Received: by mail-qv1-f70.google.com with SMTP id 6a1803df08f44-5ef4b68f47bso43316186d6.2 for ; Wed, 26 Apr 2023 05:24:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682511842; x=1685103842; 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=jjmRGNM3td6lZoDl0YCLd5HZlUD7rFMFAhO/aA6MTmA=; b=GmnC/dKqdzfQQXsfFo/8GZDK/MKl3Xc6o6rFoR0XuetqpviZLs0y2cdDpzyx3UUN1g +XO9zmjEtnRZIV2hKbXKKfM9i2rCAnVlFTpLIUMuCFXKjCTGbNZZlZyHloqqIuzMNYd7 vl6pkzbuL8xHg2tbLmrm1d9CYtjUwOj0cFngScP4vuAppxj11okOzRQNpYigyQNSWIgt M9RVaZkihoqZXholpRBJGjlH2XaFDyF/PDNXcqMAoEEYwEE8m0CARTiikcIsSt8zAQAK lqr1BSlhXCCL4Wzqt10CJcpE1LMxfSLwn1IuHZfomUTBsHzzfgsfaw+DpUNXMRV0O0u4 ns/Q== X-Gm-Message-State: AAQBX9e1gMZ+MW7L3rdwonNg0XK2JfQ73wjfJmtfw2fzdn7g3ng7GSnt FPAGUol0V2UKf8L3vxWwmjVlrkk6TpgmNg8C7TwuFC0S0P4j9EJbafoflwK7oPBgov5nFt2tKVh WM6cisplRFus3iApX51P9zgbAovhRf/WRVLs69C0XW8/oaJ5Xj0pBfPF86Zyu1Hrb+nSqDTt0Ch 1L X-Received: by 2002:a05:6214:1cc8:b0:5ac:58cc:69d1 with SMTP id g8-20020a0562141cc800b005ac58cc69d1mr30959901qvd.31.1682511842504; Wed, 26 Apr 2023 05:24:02 -0700 (PDT) X-Google-Smtp-Source: AKy350Z0vuQL8CrisDAbptOGO7WYIPzHcmU7gRnSOYymaTFzvhY/vKMvhcfhpnVR8QAFWtSwpao3sw== X-Received: by 2002:a05:6214:1cc8:b0:5ac:58cc:69d1 with SMTP id g8-20020a0562141cc800b005ac58cc69d1mr30959862qvd.31.1682511842029; Wed, 26 Apr 2023 05:24:02 -0700 (PDT) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id b20-20020a05620a271400b0074357fa9e15sm5055558qkp.42.2023.04.26.05.24.01 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Apr 2023 05:24:01 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id C8DBDB5078; Wed, 26 Apr 2023 14:23:59 +0200 (CEST) To: libabigail@sourceware.org Subject: [PATCH, applied] ir: fix canonical type propagation canceling error Organization: Red Hat / France X-Operating-System: CentOS Stream release 9 X-URL: http://www.redhat.com Date: Wed, 26 Apr 2023 14:23:59 +0200 Message-ID: <87wn1y7rxc.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.0 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, T_SCC_BODY_TEXT_LINE 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 (with the help of the 9 patches that precede it) fixes the self-comparison error that is seen when running the command line: $ fedabipkgdiff --self-compare -a --from fc37 bpftrace It turns out the root cause of the self-comparison error seen here is that sometimes, when the speculatively computed canonical type (that is called propagated canonical type) is known to be wrong, the code forgets to cancel that speculation. And that leads to wrong comparisons during subsequent type canonicalizations and later during type comparisons. This patch fixes the logic the of the cancellation of propagated canonical type that must happen when it is known that the propagated canonical type is invalid. * src/abg-ir-priv.h (environment::priv::{confirm_ct_propagation_for_types_dependant_on, confirm_ct_propagation_for_types_dependant_on}): Asserting that the type should be recursive is wrong because the recursive-ness flag is set to false upon confirmation of the canonical type propagation. So there can be some types that would make this assertion fail before we reach the end of the set of types to confirm the propagation for. (environment::priv::cancel_ct_propagation): Cancel the canonical type propagation for all types we are instructed to cancel, not just for types that are recursive or depends on recursive types. This is because the the recursive-ness flag is set to false upon cancellation of the canonical type propagation. So there can be some types that would make this condition fail before we reach the end of the set of types to cancel the propagation for. (environment::priv::cancel_all_non_confirmed_propagated_canonical_types): Define new member function. * src/abg-ir.cc (return_comparison_result): Confirm the speculative canonical type propagation result when we are done comparing the current type and the result of the comparison is true. Let's not try to be smart here. Just be safe. This optimization is fast enough as is. Otherwise, if the result of the comparison is false, then all the speculatively propagated canonical types of all the non-confirmed types should be canceled. All of them. Again, let's not try to be smart. This is smart & fast enough as is. And thing are going to be correct this way. Signed-off-by: Dodji Seketeli --- src/abg-ir-priv.h | 38 +++++++++++++++++++++++--------------- src/abg-ir.cc | 39 ++++++++++++++------------------------- 2 files changed, 37 insertions(+), 40 deletions(-) diff --git a/src/abg-ir-priv.h b/src/abg-ir-priv.h index bea7e2ec..67124805 100644 --- a/src/abg-ir-priv.h +++ b/src/abg-ir-priv.h @@ -827,8 +827,6 @@ struct environment::priv for (auto i : types_with_non_confirmed_propagated_ct_) { type_base *t = reinterpret_cast(i); - ABG_ASSERT(t->get_environment().priv_->is_recursive_type(t) - || t->priv_->depends_on_recursive_type()); t->priv_->set_does_not_depend_on_recursive_type(dependant_type); if (!t->priv_->depends_on_recursive_type()) { @@ -887,8 +885,6 @@ struct environment::priv for (auto i : types_with_non_confirmed_propagated_ct_) { type_base *t = reinterpret_cast(i); - ABG_ASSERT(t->get_environment().priv_->is_recursive_type(t) - || t->priv_->depends_on_recursive_type()); t->priv_->set_does_not_depend_on_recursive_type(); t->priv_->set_propagated_canonical_type_confirmed(true); #ifdef WITH_DEBUG_SELF_COMPARISON @@ -1049,17 +1045,13 @@ struct environment::priv const environment& env = t->get_environment(); env.priv_->cancel_ct_propagation_for_types_dependant_on(t); - if (t->priv_->depends_on_recursive_type() - || env.priv_->is_recursive_type(t)) - { - // This cannot carry any tentative canonical type at this - // point. - clear_propagated_canonical_type(t); - // Reset the marking of the type as it no longer carries a - // tentative canonical type that might be later cancelled. - t->priv_->set_does_not_depend_on_recursive_type(); - env.priv_->remove_from_types_with_non_confirmed_propagated_ct(t); - } + // This cannot carry any tentative canonical type at this + // point. + clear_propagated_canonical_type(t); + // Reset the marking of the type as it no longer carries a + // tentative canonical type that might be later canceled. + t->priv_->set_does_not_depend_on_recursive_type(); + env.priv_->remove_from_types_with_non_confirmed_propagated_ct(t); } /// Clear the propagated canonical type of a given type. @@ -1110,6 +1102,22 @@ struct environment::priv types_with_non_confirmed_propagated_ct_.erase(i); } + /// Cancel the propagated canonical types of all the types which + /// propagated canonical type have not yet been confirmed. + void + cancel_all_non_confirmed_propagated_canonical_types() + { + vector to_erase; + for (auto i : types_with_non_confirmed_propagated_ct_) + to_erase.push_back(i); + + for (auto i : to_erase) + { + type_base *t = reinterpret_cast(i); + cancel_ct_propagation(t); + } + } + #ifdef WITH_DEBUG_SELF_COMPARISON const unordered_map& diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 67f1a6bc..c899f92e 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -1041,40 +1041,29 @@ return_comparison_result(T& l, T& r, bool value, // eventually fails. env.priv_->add_to_types_with_non_confirmed_propagated_ct(is_type(&r)); } - else if (value == true - && (// The type is neither recursive nor dependant on a - // recursive type ... - (!env.priv_->is_recursive_type(&r) - && !is_type(&r)->priv_->depends_on_recursive_type() - && is_type(&r)->priv_->canonical_type_propagated() - && !is_type(&r)->priv_->propagated_canonical_type_confirmed()) - || - // ... or the comparison stack is empty, meaning, - // comparing r & l is completely done. - env.priv_->right_type_comp_operands_.empty())) + else if (value == true && env.priv_->right_type_comp_operands_.empty()) { - // Either: - // - // A/ 'r' is neither recursive nor dependant on a - // recursive type + // The type provided in the 'r' argument is the type that is + // being canonicalized; 'r' is not a mere subtype being + // compared, it's the whole type being canonicalized. And + // its canonicalization has just succeeded. // - // B/ Or the type provided in the 'r' argument is the type - // that is being canonicalized; 'r' is not a mere subtype - // being compared, it's the whole type being canonicalized. - // And its canonicalization has just succeeded. - // - // In both cases, let's confirm the canonical type resulting - // from the "canonical type propagation" optimization. + // Let's confirm the canonical type resulting from the + // "canonical type propagation" optimization. env.priv_->confirm_ct_propagation(&r); } + else if (value == true) + // In any other case, we are not sure if propagated types + // should be confirmed yet. So let's mark them as such. + env.priv_->add_to_types_with_non_confirmed_propagated_ct(is_type(&r)); else if (value == false) { // The comparison of the current sub-type failed. So all - // the types in - // env.prix_->types_with_non_confirmed_propagated_ct_ + // the with non-confirmed propagated types (those in + // env.prix_->types_with_non_confirmed_propagated_ct_) // should see their tentatively propagated canonical type // cancelled. - env.priv_->cancel_ct_propagation(&r); + env.priv_->cancel_all_non_confirmed_propagated_canonical_types(); } }