From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 10DCE3858D20 for ; Thu, 29 Dec 2022 11:44:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 10DCE3858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1672314298; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type; bh=0GmqrIKkN4jKj12plY4At2lI5Rr/AYrhilHa32Hsy/I=; b=P/r8MJ8edUti8yKWVBxyuU4E0I9ABLQzcBx2nmMY8139hudXVXezoa3ZCnVXjwUy7uwb4K Zo5g3F+DKFtFWt7Jz41GIeXFy+fhgVEpU0miB0yucZmrWv/IyMj/gy1mLhHVRKPYiK59o0 vxjBaYGiWViqx0IZdTIM4INgyJNFkDI= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-427-E7lOHpqqOfSdmMySIuDCdA-1; Thu, 29 Dec 2022 06:44:57 -0500 X-MC-Unique: E7lOHpqqOfSdmMySIuDCdA-1 Received: by mail-qk1-f200.google.com with SMTP id v7-20020a05620a0f0700b006faffce43b2so12665213qkl.9 for ; Thu, 29 Dec 2022 03:44:57 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=0GmqrIKkN4jKj12plY4At2lI5Rr/AYrhilHa32Hsy/I=; b=2ZsOG94wvegikCGyocCwhzeZcc/YnXj/PyJq3go3ImJA7vVfOengZHo1QrJY0mscmu eQhOWiKxLpeJhTxSlHtmEWcx66uoiYJLMzYgd7tqszjFCELOWi0xEyfrHZnDDjLLjpwL pqnr+j0b4jVpYE/6OcGaonZjKo9vttwBOjZjZh7/BWnCW3F8SSLo61C2FLWNW72olcaV p3oCoex7y4yztndAZMdPjjydUCLeuld7HVXQDhx2r/qci9M29HNFFwlxZWNpuf9//scC IaifmwhoTgJXgKiW4XqmzbJD5+GOWKuVXiWh2j5wErNM6M83gPm3uW39tZUYZIvL/uK/ PrlA== X-Gm-Message-State: AFqh2kpbr2Vpr/tX0URnVwWiznsmny7M+ECWK8PeO6bD4UJNXAIEWnQp K70LbmWuFIgjcZ0xU5cLwkpdVM1RO5vf/qbkT+ju+sGTzRVcN09fjZIGoMF/UTW2hJ2T3wsYn8I twvshV1ymRa9Mz0diACO9Bg6gQ88WO5gphw3hVAReY7QCQJWEA+NWyxanQI/FhNb5Tewi X-Received: by 2002:a05:622a:250d:b0:3ab:5f5f:b7c4 with SMTP id cm13-20020a05622a250d00b003ab5f5fb7c4mr41274585qtb.24.1672314296373; Thu, 29 Dec 2022 03:44:56 -0800 (PST) X-Google-Smtp-Source: AMrXdXsYWJiNFVHPGJKKKNceFFaPPNzTuSedh/2hyOWGJ5jf674X3/f8pQxlzHrEsO1CtwuOFJ/sUA== X-Received: by 2002:a05:622a:250d:b0:3ab:5f5f:b7c4 with SMTP id cm13-20020a05622a250d00b003ab5f5fb7c4mr41274548qtb.24.1672314295624; Thu, 29 Dec 2022 03:44:55 -0800 (PST) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id g1-20020a05620a40c100b006f87d28ea3asm13019637qko.54.2022.12.29.03.44.54 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Dec 2022 03:44:55 -0800 (PST) Received: by localhost (Postfix, from userid 1000) id 74A7AA2E53; Thu, 29 Dec 2022 12:44:51 +0100 (CET) From: Dodji Seketeli To: libabigail@sourceware.org Subject: [PATCH, applied] ir: Add sanity checking to canonical type propagation confirmation Organization: Red Hat / France X-Operating-System: CentOS Stream release 9 X-URL: http://www.redhat.com Date: Thu, 29 Dec 2022 12:44:51 +0100 Message-ID: <874jtebfik.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 Content-Type: text/plain X-Spam-Status: No, score=-11.6 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 List-Id: Hello, To understand the problem reported at https://sourceware.org/bugzilla/show_bug.cgi?id=29934 where a type was left non-canonicalized when analysing the binary /usr/lib64/dovecot/libdovecot-sieve.so.0.0.0 from https://vault.centos.org/7.6.1810/os/x86_64/Packages/dovecot-2.2.36-3.el7.x86_64.rpm and http://debuginfo.centos.org/7/x86_64/dovecot-debuginfo-2.2.36-3.el7.x86_64.rpm, I had to add some sanity checking code to ensure that types that have seen their propagated canonical cleared during the canonicalization process are fully canonicalized at the end of the canonicalization process. In order to performg that sanity checking this patch tracks the set of types which propagated canonical type has been cleared during the canonicalization of a particular type. When a type with such a cleared propagated canonical type is finally canonicalized, it is removed from the set of tracked types. At the end of the canonicalization process, the set of tracked types must be empty. This sanity check is compiled in only if the WITH_DEBUG_CT_PROPAGATION preprocessor macro is defined. That macro is defined if the --enable-debug-ct-propagation configure switch is used. * configure.ac: Add a new --enable-debug-ct-propagation configure flag that defines the WITH_DEBUG_CT_PROPAGATION preprocessor macro. * src/abg-ir-priv.h (environment::priv::types_with_cleared_propagated_ct_): Define new data member for tracking types with cleared propagated canonical type. (environment::priv::types_with_cleared_propagated_ct): Add getter and setter for the new data member above. (environment::priv::{record_type_with_cleared_propagated_canonical_type, erase_type_with_cleared_propagated_canonical_type}): Add book-keeping functions for the set of types with cleared propagated canonical type. (type_base::priv::clear_propagated_canonical_type): Make this return true if the propagated canonical type is cleared. (environment::priv::clear_propagated_canonical_type): Define a new function that takes a type_base* and clears its propagated canonical type. This also adds the type to the set of tracked types returned by environment::priv::types_with_cleared_propagated_ct(). (environment::priv::{cancel_ct_propagation_for_types_dependant_on, cancel_ct_propagation}): Call the new environment::priv::clear_propagated_canonical_type() rather than calling the now low-level type_base::priv::clear_propagated_canonical_type(). (environment::priv::propagate_ct): Remove the type which just gained a propagated canonical type from the set of tracked types returned by environment::priv::types_with_cleared_propagated_ct. (canonicalize_types): Define new function that canonicalizes all the types of the system (passed in parameter) and performs sanity checking to make sure all types with cleared propagated canonical types have been canonicalized. * include/abg-ir.h (string_type_base_sptr_map_type): Define new typedef for an unordered_map. * src/abg-ir.cc (canonicalize): Remove the type which has just been canonicalized from the set of tracked types returned by environment::priv::types_with_cleared_propagated_ct. * src/abg-ctf-reader.cc (reader::types_map): Use the new string_type_base_sptr_map_type typedef for the type of this map. (reader::canonicalize_all_types): Use the new function abigail::ir::canonicalize_types to canonicalize the types of the system and perform necessary sanity checking. * src/abg-dwarf-reader.cc (reader::canonicalize_types_scheduled): Likewise. Signed-off-by: Dodji Seketeli --- configure.ac | 16 +++++ include/abg-ir.h | 4 ++ src/abg-ctf-reader.cc | 8 ++- src/abg-dwarf-reader.cc | 35 ++------- src/abg-ir-priv.h | 155 ++++++++++++++++++++++++++++++++++++++-- src/abg-ir.cc | 65 ++++++++++------- 6 files changed, 218 insertions(+), 65 deletions(-) diff --git a/configure.ac b/configure.ac index df933ec0..6c7856cd 100644 --- a/configure.ac +++ b/configure.ac @@ -116,6 +116,12 @@ AC_ARG_ENABLE(debug-type-canonicalization, ENABLE_DEBUG_TYPE_CANONICALIZATION=$enableval, ENABLE_DEBUG_TYPE_CANONICALIZATION=no) +AC_ARG_ENABLE(debug-ct-propagation, + AS_HELP_STRING([--enable-debug-ct-propagation=yes|no], + [enable debugging of canonical type propagation (default is no)]), + ENABLE_DEBUG_CT_PROPAGATION=$enableval, + ENABLE_DEBUG_CT_PROPAGATION=no) + AC_ARG_ENABLE(show-type-use-in-abilint, AS_HELP_STRING([--enable-show-type-use-in-abilint=yes|no], ['enable abilint --show-type-use'(default is no)]), @@ -434,6 +440,15 @@ fi AM_CONDITIONAL(ENABLE_DEBUG_TYPE_CANONICALIZATION, test x$ENABLE_DEBUG_TYPE_CANONICALIZATION = xyes) +if test x$ENABLE_DEBUG_CT_PROPAGATION = xyes; then + AC_DEFINE([WITH_DEBUG_CT_PROPAGATION], + 1, + [compile support of debugging canonical type propagation]) + AC_MSG_NOTICE([support of debugging canonical type propagation is enabled]) +else + AC_MSG_NOTICE([support of debugging canonical type propagation is disabled]) +fi + dnl Check for the dpkg program if test x$ENABLE_DEB = xauto -o x$ENABLE_DEB = xyes; then AC_CHECK_PROG(HAS_DPKG, dpkg, yes, no) @@ -1063,6 +1078,7 @@ AC_MSG_NOTICE([ Enable abilint --show-type-use : ${ENABLE_SHOW_TYPE_USE_IN_ABILINT} Enable self comparison debugging : ${ENABLE_DEBUG_SELF_COMPARISON} Enable type canonicalization debugging : ${ENABLE_DEBUG_TYPE_CANONICALIZATION} + Enable propagated canonical type debugging : ${ENABLE_DEBUG_CT_PROPAGATION} Enable deb support in abipkgdiff : ${ENABLE_DEB} Enable GNU tar archive support in abipkgdiff : ${ENABLE_TAR} Enable bash completion : ${ENABLE_BASH_COMPLETION} diff --git a/include/abg-ir.h b/include/abg-ir.h index 498e1a2f..399409f5 100644 --- a/include/abg-ir.h +++ b/include/abg-ir.h @@ -537,6 +537,10 @@ typedef unordered_set string_type_base_wptr_map_type; +/// A convenience typedef for a map which key is a string and which +/// value is a @ref type_base_sptr. +typedef unordered_map string_type_base_sptr_map_type; + /// A convenience typedef for a map which key is an @ref /// interned_string and which value is a @ref type_base_wptr. typedef unordered_map diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc index e0b8bfb1..ee690a32 100644 --- a/src/abg-ctf-reader.cc +++ b/src/abg-ctf-reader.cc @@ -131,7 +131,7 @@ class reader : public elf_based_reader /// A map associating CTF type ids with libabigail IR types. This /// is used to reuse already generated types. - unordered_map types_map; + string_type_base_sptr_map_type types_map; /// A set associating unknown CTF type ids std::set unknown_types_set; @@ -203,8 +203,10 @@ public: void canonicalize_all_types(void) { - for (auto t = types_map.begin(); t != types_map.end(); t++) - canonicalize (t->second); + canonicalize_types + (types_map.begin(), types_map.end(), + [](const string_type_base_sptr_map_type::const_iterator& i) + {return i->second;}); } /// Constructor. diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc index 668f970d..2aeda8cf 100644 --- a/src/abg-dwarf-reader.cc +++ b/src/abg-dwarf-reader.cc @@ -4591,37 +4591,10 @@ public: } if (!types_to_canonicalize().empty()) - { - tools_utils::timer single_type_cn_timer; - size_t total = types_to_canonicalize().size(); - if (do_log()) - cerr << total << " Types to canonicalize\n"; - size_t i = 1; - for (vector::const_iterator it = - types_to_canonicalize().begin(); - it != types_to_canonicalize().end(); - ++it, ++i) - { - if (do_log()) - { - cerr << "canonicalizing type " - << get_pretty_representation(*it, false) - << " [" << i << "/" << total << "]"; - if (corpus_sptr c = corpus()) - cerr << "@" << c->get_path(); - cerr << " ..."; - single_type_cn_timer.start(); - } - canonicalize(*it); - if (do_log()) - { - single_type_cn_timer.stop(); - cerr << "DONE:" - << single_type_cn_timer - << "\n"; - } - } - } + canonicalize_types(types_to_canonicalize().begin(), + types_to_canonicalize().end(), + [](const vector::const_iterator& i) + {return *i;}); if (do_log()) { diff --git a/src/abg-ir-priv.h b/src/abg-ir-priv.h index b216c957..18ed3666 100644 --- a/src/abg-ir-priv.h +++ b/src/abg-ir-priv.h @@ -337,7 +337,7 @@ struct type_base::priv /// If the current canonical type was set as the result of the /// "canonical type propagation optimization", then clear it. - void + bool clear_propagated_canonical_type() { if (canonical_type_propagated_ && !propagated_canonical_type_confirmed_) @@ -345,7 +345,9 @@ struct type_base::priv canonical_type.reset(); naked_canonical_type = nullptr; set_canonical_type_propagated(false); + return true; } + return false; } }; // end struct type_base::priv @@ -458,6 +460,14 @@ struct environment::priv // must be cleared. pointer_set types_with_non_confirmed_propagated_ct_; pointer_set recursive_types_; +#ifdef WITH_DEBUG_CT_PROPAGATION + // Set of types which propagated canonical type has been cleared + // during the "canonical type propagation optimization" phase. Those + // types are tracked in this set to ensure that they are later + // canonicalized. This means that at the end of the + // canonicalization process, this set must be empty. + mutable pointer_set types_with_cleared_propagated_ct_; +#endif #ifdef WITH_DEBUG_SELF_COMPARISON // This is used for debugging purposes. // When abidw is used with the option --debug-abidiff, some @@ -794,6 +804,11 @@ struct environment::priv dest.priv_->canonical_type = canonical; dest.priv_->naked_canonical_type = canonical.get(); dest.priv_->set_canonical_type_propagated(true); +#ifdef WITH_DEBUG_CT_PROPAGATION + // If dest was previously a type which propagated canonical type + // has been cleared, let the book-keeping system know. + erase_type_with_cleared_propagated_canonical_type(&dest); +#endif return true; } @@ -874,6 +889,56 @@ struct environment::priv types_with_non_confirmed_propagated_ct_.clear(); } +#ifdef WITH_DEBUG_CT_PROPAGATION + /// Getter for the set of types which propagated canonical type has + /// been cleared during the "canonical type propagation + /// optimization" phase. Those types are tracked in this set to + /// ensure that they are later canonicalized. This means that at + /// the end of the canonicalization process, this set must be empty. + /// + /// @return the set of types which propagated canonical type has + /// been cleared. + const pointer_set& + types_with_cleared_propagated_ct() const + {return types_with_cleared_propagated_ct_;} + + /// Getter for the set of types which propagated canonical type has + /// been cleared during the "canonical type propagation + /// optimization" phase. Those types are tracked in this set to + /// ensure that they are later canonicalized. This means that at + /// the end of the canonicalization process, this set must be empty. + /// + /// @return the set of types which propagated canonical type has + /// been cleared. + pointer_set& + types_with_cleared_propagated_ct() + {return types_with_cleared_propagated_ct_;} + + /// Record a type which propagated canonical type has been cleared + /// during the "canonical type propagation optimization phase". + /// + /// @param t the type to record. + void + record_type_with_cleared_propagated_canonical_type(const type_base* t) + { + uintptr_t ptr = reinterpret_cast(t); + types_with_cleared_propagated_ct_.insert(ptr); + } + + /// Erase a type (which propagated canonical type has been cleared + /// during the "canonical type propagation optimization phase") from + /// the set of types that have been recorded by the invocation of + /// record_type_with_cleared_propagated_canonical_type() + /// + /// @param t the type to erase from the set. + void + erase_type_with_cleared_propagated_canonical_type(const type_base* t) + { + uintptr_t ptr = reinterpret_cast(t); + types_with_cleared_propagated_ct_.erase(ptr); + } +#endif //WITH_DEBUG_CT_PROPAGATION + /// Collect the types that depends on a given "target" type. /// /// Walk a set of types and if they depend directly or indirectly on @@ -941,7 +1006,7 @@ struct environment::priv type_base_sptr canonical = t->priv_->canonical_type.lock(); if (canonical) { - t->priv_->clear_propagated_canonical_type(); + clear_propagated_canonical_type(t); t->priv_->set_does_not_depend_on_recursive_type(); } } @@ -980,9 +1045,7 @@ struct environment::priv { // This cannot carry any tentative canonical type at this // point. - if (t->priv_->canonical_type_propagated() - && !t->priv_->propagated_canonical_type_confirmed()) - t->priv_->clear_propagated_canonical_type(); + 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(); @@ -990,6 +1053,30 @@ struct environment::priv } } + /// Clear the propagated canonical type of a given type. + /// + /// This function also updates the book-keeping of the set of types + /// which propagated canonical types have been cleared. + /// + /// Please note that at the end of the canonicalization of all the + /// types in the system, all the types which propagated canonical + /// type has been cleared must be canonicalized. + /// + /// @param t the type to + void + clear_propagated_canonical_type(const type_base *t) + { + if (t->priv_->clear_propagated_canonical_type()) + { +#ifdef WITH_DEBUG_CT_PROPAGATION + // let the book-keeping system know that t has its propagated + // canonical type cleared. + record_type_with_cleared_propagated_canonical_type(t) +#endif + ; + } + } + /// Add a given type to the set of types that have been /// non-confirmed subjects of the canonical type propagation /// optimization. @@ -1092,6 +1179,64 @@ struct environment::priv #endif };// end struct environment::priv +/// Compute the canonical type for all the IR types of the system. +/// +/// After invoking this function, the time it takes to compare two +/// types of the IR is equivalent to the time it takes to compare +/// their pointer value. That is faster than performing a structural +/// (A.K.A. member-wise) comparison. +/// +/// Note that this function performs some sanity checks after* the +/// canonicalization process. It ensures that at the end of the +/// canonicalization process, all types have been canonicalized. This +/// is important because the canonicalization algorithm sometimes +/// clears some canonical types after having speculatively set them +/// for performance purposes. At the end of the process however, all +/// types must be canonicalized, and this function detects violations +/// of that assertion. +/// +/// @tparam input_iterator the type of the input iterator of the @p +/// beging and @p end. +/// +/// @tparam deref_lambda a lambda function which takes in parameter +/// the input iterator of type @p input_iterator and dereferences it +/// to return the type to canonicalize. +/// +/// @param begin an iterator pointing to the first type of the set of types +/// to canonicalize. +/// +/// @param end an iterator pointing to the end (after the last type) of +/// the set of types to canonicalize. +/// +/// @param deref a lambda function that knows how to dereference the +/// iterator @p begin to return the type to canonicalize. +template +void +canonicalize_types(const input_iterator& begin, + const input_iterator& end, + deref_lambda deref) +{ + if (begin == end) + return; + + // First, let's compute the canonical type of this type. + for (auto t = begin; t != end; ++t) + canonicalize(deref(t)); + +#ifdef WITH_DEBUG_CT_PROPAGATION + // Then now, make sure that all types -- which propagated canonical + // type has been cleared -- have been canonicalized. In other + // words, the set of types which have been recorded because their + // propagated canonical type has been cleared must be empty. + const environment& env = deref(begin)->get_environment(); + pointer_set to_canonicalize = + env.priv_->types_with_cleared_propagated_ct(); + + ABG_ASSERT(to_canonicalize.empty()); +#endif // WITH_DEBUG_CT_PROPAGATION +} + // struct class_or_union::priv { diff --git a/src/abg-ir.cc b/src/abg-ir.cc index f32e8d1f..46ac81a2 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -14564,37 +14564,50 @@ canonicalize(type_base_sptr t) } if (canonical) - if (decl_base_sptr d = is_decl_slow(canonical)) - { - scope_decl *scope = d->get_scope(); - // Add the canonical type to the set of canonical types - // belonging to its scope. - if (scope) - { - if (is_type(scope)) - // The scope in question is itself a type (e.g, a class - // or union). Let's call that type ST. We want to add - // 'canonical' to the set of canonical types belonging - // to ST. - if (type_base_sptr c = is_type(scope)->get_canonical_type()) - // We want to add 'canonical' to set of canonical - // types belonging to the canonical type of ST. That - // way, just looking at the canonical type of ST is - // enough to get the types that belong to the scope of - // the class of equivalence of ST. - scope = is_scope_decl(is_decl(c)).get(); - scope->get_canonical_types().insert(canonical); - } - // else, if the type doesn't have a scope, it's not meant to be - // emitted. This can be the case for the result of the - // function strip_typedef, for instance. - } + { + if (decl_base_sptr d = is_decl_slow(canonical)) + { + scope_decl *scope = d->get_scope(); + // Add the canonical type to the set of canonical types + // belonging to its scope. + if (scope) + { + if (is_type(scope)) + // The scope in question is itself a type (e.g, a class + // or union). Let's call that type ST. We want to add + // 'canonical' to the set of canonical types belonging + // to ST. + if (type_base_sptr c = is_type(scope)->get_canonical_type()) + // We want to add 'canonical' to set of canonical + // types belonging to the canonical type of ST. That + // way, just looking at the canonical type of ST is + // enough to get the types that belong to the scope of + // the class of equivalence of ST. + scope = is_scope_decl(is_decl(c)).get(); + scope->get_canonical_types().insert(canonical); + } + // else, if the type doesn't have a scope, it's not meant to be + // emitted. This can be the case for the result of the + // function strip_typedef, for instance. + } + +#ifdef WITH_DEBUG_CT_PROPAGATION + // Update the book-keeping of the set of the types which + // propagated canonical type has been cleared. + // + // If this type 't' which has just been canonicalized was + // previously in the set of types which propagated canonical + // type has been cleared, then remove it from that set because + // its canonical type is now computed and definitely set. + const environment& env = t->get_environment(); + env.priv_->erase_type_with_cleared_propagated_canonical_type(t.get()); +#endif + } t->on_canonical_type_set(); return canonical; } - /// Set the definition of this declaration-only @ref decl_base. /// /// @param d the new definition to set. -- 2.31.1 -- Dodji