From patchwork Wed Jun 10 11:59:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 39552 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 515F3388C019; Wed, 10 Jun 2020 11:59:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 515F3388C019 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1591790399; bh=bIUGi5q0CDSaZMZltJ81lv3s0hPE0lH1E8XZymIPlXM=; h=Date:In-Reply-To:References:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Help:List-Subscribe:From:Reply-To:Cc:From; b=HiOVz+id/QLIZLZy6JPcoe/mapUi8hiOBVPCczXSITKE1LnkuqXbndCr7X5M79LTN yP7aZW/S/kvm2Lo772xFdLtnVwFim7hNzhai2D1JgLK+VQXy001DyRiJS4i5COlpH6 k5np2pX0FevgSVqkRIKNDHbE5p54t5JsAHj2glNE= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from mail-qt1-x84a.google.com (mail-qt1-x84a.google.com [IPv6:2607:f8b0:4864:20::84a]) by sourceware.org (Postfix) with ESMTPS id CE34D388C00E for ; Wed, 10 Jun 2020 11:59:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CE34D388C00E Received: by mail-qt1-x84a.google.com with SMTP id u26so1591452qtj.21 for ; Wed, 10 Jun 2020 04:59:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=bIUGi5q0CDSaZMZltJ81lv3s0hPE0lH1E8XZymIPlXM=; b=KStc+B9CqbF7W97anhL22MrwB6saXembKeVf7CLK9m6UGVEfwZaH2BoXc9xVYOAR/0 erbU1T24sC6KGyht2cyf8XoO1f0hez31b5MSwyvpDlahNUTcMDbLfw/AFPBJ/Lbx9jB2 bMg6inikzZwPh8zlwebnYbUhpGUFAqi1pe6GvRf6EuA8rsUcdx6FKmFnx3scRpcmeH4a umCBRukQ15QomJhpIPYZw1CAJqdD7rl87Cajiai6lH5Ok8N1YMZ804MEBZq4WeGk+ptQ cZO1GhHJLD3r7aWY7pWd3b8y3rM4upj8hHmB6NJQA36yBaj/7hnI7gefH59hkE3T7zNP iJww== X-Gm-Message-State: AOAM531NPI2RB77UdlXCzaSHfxnc+GdWzi+hxbmw6SVo+SfyPn/pqVPG S8MWie6Ws3lR4dXak3L0npMTRHmAFv5FRE8mGzffiyz09/v3RP25/OwIdIezbGsUR9roOrRzVl2 1cHFGiGCi6SoJEGI59Qu+QX77v3JzTq1BOOu5R1pJLO3G6qkHgREsnrVPAjBsAX00uoefz+Q= X-Google-Smtp-Source: ABdhPJzoT1j3MeAnhUxqqj3x1plAE0TtRPkqigIv5sqMbF7ycBQDsOxVMoQAxHM8DRTLcfp0p+R+4CV0X9Z7+A== X-Received: by 2002:a05:6214:10e1:: with SMTP id q1mr2594585qvt.78.1591790396226; Wed, 10 Jun 2020 04:59:56 -0700 (PDT) Date: Wed, 10 Jun 2020 12:59:35 +0100 In-Reply-To: <20200610115940.26035-1-gprocida@google.com> Message-Id: <20200610115940.26035-7-gprocida@google.com> Mime-Version: 1.0 References: <20200610115940.26035-1-gprocida@google.com> X-Mailer: git-send-email 2.27.0.278.ge193c7cf3a9-goog Subject: [PATCH 06/11] Support incomplete enums in core and diff code. To: libabigail@sourceware.org X-Spam-Status: No, score=-23.5 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_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: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-Patchwork-Original-From: Giuliano Procida via Libabigail From: Giuliano Procida Reply-To: Giuliano Procida Cc: kernel-team@android.com, gprocida@google.com Errors-To: libabigail-bounces@sourceware.org Sender: "Libabigail" This is an almost blind attempt at addition of support for incomplete, also known as forward-declared, enum types. I've not made any attempt to refactor or share logic with the struct/union code and I've probably got other things wrong. This patch: - adds enums_type and enum_type_decl_wptr typedefs - adds declaration-only machinery to enum_type_decl - adds is_compatible_with_enum_type, look_through_decl_only_class, there_is_a_decl_only_enum and lookup_enum_type helpers - adds has_enum_decl_only_def_change functions - inserts has_enum_decl_only_def_change wherever has_class_decl_only_def_change is present - adds reporting of enum declaration/definition diffs - doesn't address the DWARF reader, XML writer or XML reader Signed-off-by: Giuliano Procida Signed-off-by: Giuliano Procida Signed-off-by: Dodji Seketeli Signed-off-by: Giuliano Procida Signed-off-by: Dodji Seketeli --- include/abg-comp-filter.h | 7 ++ include/abg-fwd.h | 24 +++++ include/abg-ir.h | 15 +++ src/abg-comp-filter.cc | 74 +++++++++++++- src/abg-comparison.cc | 4 +- src/abg-default-reporter.cc | 19 ++++ src/abg-ir.cc | 196 +++++++++++++++++++++++++++++++++++- 7 files changed, 333 insertions(+), 6 deletions(-) diff --git a/include/abg-comp-filter.h b/include/abg-comp-filter.h index 81130033..3b845088 100644 --- a/include/abg-comp-filter.h +++ b/include/abg-comp-filter.h @@ -68,9 +68,16 @@ bool has_class_decl_only_def_change(const class_or_union_sptr& first, const class_or_union_sptr& second); +bool +has_enum_decl_only_def_change(const enum_type_decl_sptr& first, + const enum_type_decl_sptr& second); + bool has_class_decl_only_def_change(const diff *diff); +bool +has_enum_decl_only_def_change(const diff *diff); + bool has_basic_type_name_change(const diff *); diff --git a/include/abg-fwd.h b/include/abg-fwd.h index 999b071b..73d809ab 100644 --- a/include/abg-fwd.h +++ b/include/abg-fwd.h @@ -155,6 +155,12 @@ class enum_type_decl; /// Convenience typedef for shared pointer to a @ref enum_type_decl. typedef shared_ptr enum_type_decl_sptr; +/// Convenience typedef for a vector of @ref enum_type_decl_sptr +typedef vector enums_type; + +/// Convenience typedef for a weak pointer to a @ref enum_type_decl. +typedef weak_ptr enum_type_decl_wptr; + class class_or_union; typedef shared_ptr class_or_union_sptr; @@ -423,6 +429,12 @@ is_typedef(const type_base*); typedef_decl* is_typedef(type_base*); +enum_type_decl_sptr +is_compatible_with_enum_type(const type_base_sptr&); + +enum_type_decl_sptr +is_compatible_with_enum_type(const decl_base_sptr&); + enum_type_decl_sptr is_enum_type(const type_or_decl_base_sptr&); @@ -513,6 +525,12 @@ look_through_decl_only_class(const class_or_union&); class_or_union_sptr look_through_decl_only_class(class_or_union_sptr); +enum_type_decl_sptr +look_through_decl_only_enum(const enum_type_decl&); + +enum_type_decl_sptr +look_through_decl_only_enum(enum_type_decl_sptr); + var_decl* is_var_decl(const type_or_decl_base*); @@ -1085,6 +1103,12 @@ lookup_enum_type(const string&, const corpus&); enum_type_decl_sptr lookup_enum_type(const interned_string&, const corpus&); +const type_base_wptrs_type* +lookup_enum_types(const interned_string&, const corpus&); + +const type_base_wptrs_type* +lookup_enum_types(const string&, const corpus&); + enum_type_decl_sptr lookup_enum_type_per_location(const interned_string&, const corpus&); diff --git a/include/abg-ir.h b/include/abg-ir.h index ebcc4b81..2114e27b 100644 --- a/include/abg-ir.h +++ b/include/abg-ir.h @@ -2497,6 +2497,21 @@ public: enumerators& get_enumerators(); + bool + get_is_declaration_only() const; + + void + set_is_declaration_only(bool f); + + void + set_definition_of_declaration(enum_type_decl_sptr); + + const enum_type_decl_sptr + get_definition_of_declaration() const; + + const enum_type_decl* + get_naked_definition_of_declaration() const; + virtual string get_pretty_representation(bool internal = false, bool qualified_name = true) const; diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc index 81548427..70858f27 100644 --- a/src/abg-comp-filter.cc +++ b/src/abg-comp-filter.cc @@ -118,6 +118,25 @@ there_is_a_decl_only_class(const class_decl_sptr& class1, return false; } +/// Test if there is a enum that is declaration-only among the two +/// enums in parameter. +/// +/// @param enum1 the first enum to consider. +/// +/// @param enum2 the second enum to consider. +/// +/// @return true if either enums are declaration-only, false +/// otherwise. +static bool +there_is_a_decl_only_enum(const enum_type_decl_sptr& enum1, + const enum_type_decl_sptr& enum2) +{ + if ((enum1 && enum1->get_is_declaration_only()) + || (enum2 && enum2->get_is_declaration_only())) + return true; + return false; +} + /// Test if the diff involves a declaration-only class. /// /// @param diff the class diff to consider. @@ -146,7 +165,9 @@ type_size_changed(const type_base_sptr f, const type_base_sptr s) || f->get_size_in_bits() == 0 || s->get_size_in_bits() == 0 || there_is_a_decl_only_class(is_compatible_with_class_type(f), - is_compatible_with_class_type(s))) + is_compatible_with_class_type(s)) + || there_is_a_decl_only_enum(is_compatible_with_enum_type(f), + is_compatible_with_enum_type(s))) return false; return f->get_size_in_bits() != s->get_size_in_bits(); @@ -956,6 +977,31 @@ has_class_decl_only_def_change(const class_or_union_sptr& first, return (f->get_is_declaration_only() != s->get_is_declaration_only()); } +/// Test if two @ref enum_sptr are different just by the +/// fact that one is decl-only and the other one is defined. +/// +/// @param first the first enum to consider. +/// +/// @param second the second enum to consider. +/// +/// @return true iff the two arguments are different just by the fact +/// that one is decl-only and the other one is defined. +bool +has_enum_decl_only_def_change(const enum_type_decl_sptr& first, + const enum_type_decl_sptr& second) +{ + if (!first || !second) + return false; + + enum_type_decl_sptr f = look_through_decl_only_enum(first); + enum_type_decl_sptr s = look_through_decl_only_enum(second); + + if (f->get_qualified_name() != s->get_qualified_name()) + return false; + + return (f->get_is_declaration_only() != s->get_is_declaration_only()); +} + /// Test if a class_or_union_diff carries a change in which the two /// classes are different by the fact that one is a decl-only and the /// other one is defined. @@ -980,6 +1026,28 @@ has_class_decl_only_def_change(const diff *diff) return has_class_decl_only_def_change(f, s); } +/// Test if a enum_diff carries a change in which the two enums are +/// different by the fact that one is a decl-only and the other one is +/// defined. +/// +/// @param diff the diff node to consider. +/// +/// @return true if the enum_diff carries a change in which the two +/// enums are different by the fact that one is a decl-only and the +/// other one is defined. +bool +has_enum_decl_only_def_change(const diff *diff) +{ + const enum_diff *d = dynamic_cast(diff); + if (!d) + return false; + + enum_type_decl_sptr f = look_through_decl_only_enum(d->first_enum()); + enum_type_decl_sptr s = look_through_decl_only_enum(d->second_enum()); + + return has_enum_decl_only_def_change(f, s); +} + /// Test if a diff node carries a basic type name change. /// /// @param d the diff node to consider. @@ -1517,7 +1585,8 @@ categorize_harmless_diff_node(diff *d, bool pre) decl_base_sptr f = is_decl(d->first_subject()), s = is_decl(d->second_subject()); - if (has_class_decl_only_def_change(d)) + if (has_class_decl_only_def_change(d) + || has_enum_decl_only_def_change(d)) category |= TYPE_DECL_ONLY_DEF_CHANGE_CATEGORY; if (access_changed(f, s)) @@ -1608,6 +1677,7 @@ categorize_harmful_diff_node(diff *d, bool pre) // // TODO: be more specific -- not all size changes are harmful. if (!has_class_decl_only_def_change(d) + && !has_enum_decl_only_def_change(d) && (type_size_changed(f, s) || data_member_offset_changed(f, s) || non_static_data_member_type_size_changed(f, s) diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc index f77799f2..21cab0e7 100644 --- a/src/abg-comparison.cc +++ b/src/abg-comparison.cc @@ -10962,7 +10962,9 @@ struct leaf_diff_node_marker_visitor : public diff_node_visitor // with the is_declaration flag set) that carries a non-zero // size! And of course at some point that non-zero size // changes. We need to be able to detect that. - && !filtering::is_decl_only_class_with_size_change(d)) + && !filtering::is_decl_only_class_with_size_change(d) + // Don't show decl-only-ness changes of enums. + && !filtering::has_enum_decl_only_def_change(d)) { diff_context_sptr ctxt = d->context(); const corpus_diff *corpus_diff_node = ctxt->get_corpus_diff().get(); diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc index 2acb6954..cbf8c2bc 100644 --- a/src/abg-default-reporter.cc +++ b/src/abg-default-reporter.cc @@ -99,6 +99,25 @@ default_reporter::report(const enum_diff& d, ostream& out, enum_type_decl_sptr first = d.first_enum(), second = d.second_enum(); const diff_context_sptr& ctxt = d.context(); + + // Report enum decl-only <-> definition changes. + if (ctxt->get_allowed_category() & TYPE_DECL_ONLY_DEF_CHANGE_CATEGORY) + if (filtering::has_enum_decl_only_def_change(first, second)) + { + string was = + first->get_is_declaration_only() + ? " was a declaration-only enum type" + : " was a defined enum type"; + + string is_now = + second->get_is_declaration_only() + ? " and is now a declaration-only enum type" + : " and is now a defined enum type"; + + out << indent << "enum type " << name << was << is_now << "\n"; + return; + } + report_name_size_and_alignment_changes(first, second, ctxt, out, indent); maybe_report_diff_for_member(first, second, ctxt, out, indent); diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 5cc39f59..9f4890e8 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -7924,6 +7924,38 @@ typedef_decl* is_typedef(type_base* t) {return dynamic_cast(t);} +/// Test if a type is a enum. This function looks through typedefs. +/// +/// @parm t the type to consider. +/// +/// @return the enum_decl if @p t is a enum_decl or null otherwise. +enum_type_decl_sptr +is_compatible_with_enum_type(const type_base_sptr& t) +{ + if (!t) + return enum_type_decl_sptr(); + + // Normally we should strip typedefs entirely, but this is + // potentially costly, especially on binaries with huge changesets + // like the Linux Kernel. So we just get the leaf types for now. + // + // Maybe there should be an option by which users accepts to pay the + // CPU usage toll in exchange for finer filtering? + + // type_base_sptr ty = strip_typedef(t); + type_base_sptr ty = peel_typedef_type(t);; + return is_enum_type(ty); +} + +/// Test if a type is a enum. This function looks through typedefs. +/// +/// @parm t the type to consider. +/// +/// @return the enum_decl if @p t is a enum_decl or null otherwise. +enum_type_decl_sptr +is_compatible_with_enum_type(const decl_base_sptr& t) +{return is_compatible_with_enum_type(is_type(t));} + /// Test if a decl is an enum_type_decl /// /// @param d the decl to test for. @@ -8307,6 +8339,50 @@ look_through_decl_only_class(class_or_union_sptr klass) return result; } +/// If a enum is a decl-only enum, get its definition. +/// Otherwise, just return the initial enum. +/// +/// @param the_enum the enum to consider. +/// +/// @return either the definition of the enum, or the enum itself. +enum_type_decl_sptr +look_through_decl_only_enum(const enum_type_decl& the_enum) +{ + enum_type_decl_sptr enom; + if (the_enum.get_is_declaration_only()) + enom = the_enum.get_definition_of_declaration(); + + if (!enom) + return enom; + + while (enom + && enom->get_is_declaration_only() + && enom->get_definition_of_declaration()) + enom = enom->get_definition_of_declaration(); + + ABG_ASSERT(enom); + return enom; +} + +/// If a enum is a decl-only enum, get its definition. +/// Otherwise, just return the initial enum. +/// +/// @param enom the enum to consider. +/// +/// @return either the definition of the enum, or the enum itself. +enum_type_decl_sptr +look_through_decl_only_enum(enum_type_decl_sptr enom) +{ + if (!enom) + return enom; + + enum_type_decl_sptr result = look_through_decl_only_enum(*enom); + if (!result) + result = enom; + + return result; +} + /// Tests if a declaration is a variable declaration. /// /// @param decl the decl to test. @@ -10165,6 +10241,37 @@ lookup_enum_type(const interned_string& qualified_name, const corpus& corp) return result; } +/// Look into a given corpus to find the enum type*s* that have a +/// given qualified name. +/// +/// @param qualified_name the qualified name of the type to look for. +/// +/// @param corp the corpus to look into. +/// +/// @return the vector of enum types that which name is @p qualified_name. +const type_base_wptrs_type * +lookup_enum_types(const interned_string& qualified_name, const corpus& corp) +{ + const istring_type_base_wptrs_map_type& m = corp.get_types().enum_types(); + + return lookup_types_in_map(qualified_name, m); +} + +/// Look into a given corpus to find the enum type*s* that have a +/// given qualified name. +/// +/// @param qualified_name the qualified name of the type to look for. +/// +/// @param corp the corpus to look into. +/// +/// @return the vector of enum types that which name is @p qualified_name. +const type_base_wptrs_type* +lookup_enum_types(const string& qualified_name, const corpus& corp) +{ + interned_string s = corp.get_environment()->intern(qualified_name); + return lookup_enum_types(s, corp); +} + /// Look up an @ref enum_type_decl from a given corpus, by its location. /// /// @param loc the location to consider. @@ -14974,17 +15081,26 @@ class enum_type_decl::priv { type_base_sptr underlying_type_; enumerators enumerators_; + decl_base_sptr declaration_; + enum_type_decl_wptr definition_of_declaration_; + enum_type_decl* naked_definition_of_declaration_; + bool is_declaration_only_; friend class enum_type_decl; - priv(); - public: + priv() + : naked_definition_of_declaration_(), + is_declaration_only_(true) + {} + priv(type_base_sptr underlying_type, enumerators& enumerators) : underlying_type_(underlying_type), - enumerators_(enumerators) + enumerators_(enumerators), + naked_definition_of_declaration_(), + is_declaration_only_(false) {} }; // end class enum_type_decl::priv @@ -15040,6 +15156,80 @@ enum_type_decl::enumerators& enum_type_decl::get_enumerators() {return priv_->enumerators_;} +/// Set the definition of this declaration-only @ref enum_type_decl. +/// +/// @param d the new definition to set. +void +enum_type_decl::set_definition_of_declaration(enum_type_decl_sptr d) +{ + ABG_ASSERT(get_is_declaration_only()); + priv_->definition_of_declaration_ = d; + if (d->get_canonical_type()) + type_base::priv_->canonical_type = d->get_canonical_type(); + + priv_->naked_definition_of_declaration_ = d.get(); +} + +/// If this @ref enum_type_decl_sptr is declaration-only, get its +/// definition, if any. +/// +/// @return the definition of this decl-only enum. +const enum_type_decl_sptr +enum_type_decl::get_definition_of_declaration() const +{ + if (priv_->definition_of_declaration_.expired()) + return enum_type_decl_sptr(); + return enum_type_decl_sptr(priv_->definition_of_declaration_); +} + +/// If this @ref enum_type_decl is declaration-only, get its +/// definition, if any. +/// +/// Note that this function doesn't return a smart pointer, but rather +/// the underlying pointer managed by the smart pointer. So it's as +/// fast as possible. This getter is to be used in code paths that are +/// proven to be performance hot spots; especially, when comparing +/// sensitive types like enums. Those are compared extremely frequently +/// and thus, their access to the definition of declaration must be +/// fast. +/// +/// @return the definition of the enum. +const enum_type_decl* +enum_type_decl::get_naked_definition_of_declaration() const +{return priv_->naked_definition_of_declaration_;} + +/// Test if a @ref enum_type_decl is a declaration-only @ref +/// enum_type_decl. +/// +/// @return true iff the current @ref enum_type_decl is a +/// declaration-only @ref enum_type_decl. +bool +enum_type_decl::get_is_declaration_only() const +{return priv_->is_declaration_only_;} + +/// Set a flag saying if the @ref enum_type_decl is a declaration-only +/// @ref enum_type_decl. +/// +/// @param f true if the @ref enum_type_decl is a decalaration-only +/// @ref enum_type_decl. +void +enum_type_decl::set_is_declaration_only(bool f) +{ + bool update_types_lookup_map = !f && priv_->is_declaration_only_; + + priv_->is_declaration_only_ = f; + + if (update_types_lookup_map) + if (scope_decl* s = get_scope()) + { + scope_decl::declarations::iterator i; + if (s->find_iterator_for_member(this, i)) + maybe_update_types_lookup_map(*i); + else + ABG_ASSERT_NOT_REACHED; + } +} + /// Get the pretty representation of the current instance of @ref /// enum_type_decl. ///