From: Giuliano Procida <gprocida@google.com>
To: libabigail@sourceware.org
Cc: dodji@seketeli.org, kernel-team@android.com, gprocida@google.com,
maennich@google.com
Subject: [RFC PATCH 1/2] Support declaration-only enums.
Date: Thu, 23 Apr 2020 14:03:55 +0100 [thread overview]
Message-ID: <20200423130356.93136-2-gprocida@google.com> (raw)
In-Reply-To: <20200423130356.93136-1-gprocida@google.com>
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.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
include/abg-comp-filter.h | 7 +
include/abg-comparison.h | 7 +
include/abg-fwd.h | 26 ++-
include/abg-ir.h | 23 +++
src/abg-comp-filter.cc | 78 ++++++++-
src/abg-comparison.cc | 21 ++-
src/abg-default-reporter.cc | 34 +++-
src/abg-dwarf-reader.cc | 308 +++++++++++++++++++++++++++++++++++-
src/abg-ir.cc | 235 ++++++++++++++++++++++++++-
9 files changed, 713 insertions(+), 26 deletions(-)
diff --git a/include/abg-comp-filter.h b/include/abg-comp-filter.h
index b8da3156..c486fec5 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-comparison.h b/include/abg-comparison.h
index 4f60ff99..bb654cef 100644
--- a/include/abg-comparison.h
+++ b/include/abg-comparison.h
@@ -430,6 +430,12 @@ enum diff_category
/// array type of a global variable, but the ELF size of the
/// variable didn't change.
BENIGN_INFINITE_ARRAY_CHANGE_CATEGORY = 1 << 19,
+
+ /// This means that a diff node in the sub-tree carries a enum type
+ /// that was declaration-only and that is now defined, or vice
+ /// versa.
+ ENUM_DECL_ONLY_DEF_CHANGE_CATEGORY = 1 << 20,
+
/// A special enumerator that is the logical 'or' all the
/// enumerators above.
///
@@ -456,6 +462,7 @@ enum diff_category
| VAR_TYPE_CV_CHANGE_CATEGORY
| VOID_PTR_TO_PTR_CHANGE_CATEGORY
| BENIGN_INFINITE_ARRAY_CHANGE_CATEGORY
+ | ENUM_DECL_ONLY_DEF_CHANGE_CATEGORY
}; // enum diff_category
diff_category
diff --git a/include/abg-fwd.h b/include/abg-fwd.h
index 1aab70a6..d1cf0322 100644
--- a/include/abg-fwd.h
+++ b/include/abg-fwd.h
@@ -154,9 +154,15 @@ typedef weak_ptr<typedef_decl> typedef_decl_wptr;
class enum_type_decl;
-/// Convenience typedef for shared pointer on enum_type_decl.
+/// Convenience typedef for shared pointer to a @ref enum_type_decl.
typedef shared_ptr<enum_type_decl> enum_type_decl_sptr;
+/// Convenience typedef for a vector of @ref enum_type_decl_sptr
+typedef vector<enum_type_decl_sptr> enums_type;
+
+/// Convenience typedef for a weak pointer to a @ref enum_type_decl.
+typedef weak_ptr<enum_type_decl> enum_type_decl_wptr;
+
class class_or_union;
typedef shared_ptr<class_or_union> class_or_union_sptr;
@@ -422,6 +428,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&);
@@ -512,6 +524,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*);
@@ -1071,6 +1089,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 fda10de5..88ce39e0 100644
--- a/include/abg-ir.h
+++ b/include/abg-ir.h
@@ -2465,6 +2465,8 @@ public:
const string& mangled_name = "",
visibility vis = VISIBILITY_DEFAULT);
+ enum_type_decl(const environment* env, const string& name);
+
type_base_sptr
get_underlying_type() const;
@@ -2474,6 +2476,27 @@ 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;
+
+ decl_base_sptr
+ get_earlier_declaration() const;
+
+ void
+ set_earlier_declaration(decl_base_sptr declaration);
+
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 75df901c..c8a031d3 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
+/// enumes in parameter.
+///
+/// @param enum1 the first enum to consider.
+///
+/// @param enum2 the second enum to consider.
+///
+/// @return true if either enumes 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();
@@ -937,13 +958,38 @@ 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.
///
/// @param diff the diff node to consider.
-////
-//// @return true if the class_or_union_diff carries a change in which
+///
+/// @return true if the 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.
bool
@@ -961,6 +1007,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
+/// enumes 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 enumes 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<const enum_diff*>(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.
@@ -1501,6 +1569,9 @@ categorize_harmless_diff_node(diff *d, bool pre)
if (has_class_decl_only_def_change(d))
category |= CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY;
+ if (has_enum_decl_only_def_change(d))
+ category |= ENUM_DECL_ONLY_DEF_CHANGE_CATEGORY;
+
if (access_changed(f, s))
category |= ACCESS_CHANGE_CATEGORY;
@@ -1586,6 +1657,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 46bf9e30..40d80073 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -2952,7 +2952,8 @@ get_default_harmless_categories_bitmap()
| abigail::comparison::FN_RETURN_TYPE_CV_CHANGE_CATEGORY
| abigail::comparison::VAR_TYPE_CV_CHANGE_CATEGORY
| abigail::comparison::VOID_PTR_TO_PTR_CHANGE_CATEGORY
- | abigail::comparison::BENIGN_INFINITE_ARRAY_CHANGE_CATEGORY);
+ | abigail::comparison::BENIGN_INFINITE_ARRAY_CHANGE_CATEGORY
+ | abigail::comparison::ENUM_DECL_ONLY_DEF_CHANGE_CATEGORY);
}
/// Getter of a bitmap made of the set of change categories that are
@@ -3113,7 +3114,7 @@ operator<<(ostream& o, diff_category c)
emitted_a_category |= true;
}
- if (c & FN_RETURN_TYPE_CV_CHANGE_CATEGORY)
+ if (c & FN_RETURN_TYPE_CV_CHANGE_CATEGORY)
{
if (emitted_a_category)
o << "|";
@@ -3121,7 +3122,7 @@ operator<<(ostream& o, diff_category c)
emitted_a_category |= true;
}
- if (c & VAR_TYPE_CV_CHANGE_CATEGORY)
+ if (c & VAR_TYPE_CV_CHANGE_CATEGORY)
{
if (emitted_a_category)
o << "|";
@@ -3129,7 +3130,7 @@ operator<<(ostream& o, diff_category c)
emitted_a_category |= true;
}
- if (c & VOID_PTR_TO_PTR_CHANGE_CATEGORY)
+ if (c & VOID_PTR_TO_PTR_CHANGE_CATEGORY)
{
if (emitted_a_category)
o << "|";
@@ -3137,7 +3138,7 @@ operator<<(ostream& o, diff_category c)
emitted_a_category |= true;
}
- if (c & BENIGN_INFINITE_ARRAY_CHANGE_CATEGORY)
+ if (c & BENIGN_INFINITE_ARRAY_CHANGE_CATEGORY)
{
if (emitted_a_category)
o << "|";
@@ -3145,6 +3146,14 @@ operator<<(ostream& o, diff_category c)
emitted_a_category |= true;
}
+ if (c & ENUM_DECL_ONLY_DEF_CHANGE_CATEGORY)
+ {
+ if (emitted_a_category)
+ o << "|";
+ o << "ENUM_DECL_ONLY_DEF_CHANGE_CATEGORY";
+ emitted_a_category |= true;
+ }
+
return o;
}
@@ -10707,6 +10716,8 @@ struct leaf_diff_node_marker_visitor : public diff_node_visitor
&& !is_anonymous_class_or_union_diff(d)
// Don't show decl-only-ness changes of classes either.
&& !filtering::has_class_decl_only_def_change(d)
+ // Same for enums.
+ && !filtering::has_enum_decl_only_def_change(d)
// Sometime, we can encounter artifacts of bogus DWARF that
// yield a diff node for a decl-only class (and empty class
// with the is_declaration flag set) that carries a non-zero
diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
index 04e2bb76..19dec4cb 100644
--- a/src/abg-default-reporter.cc
+++ b/src/abg-default-reporter.cc
@@ -94,13 +94,33 @@ default_reporter::report(const enum_diff& d, ostream& out,
d.second_subject(),
"enum type");
- string name = d.first_enum()->get_pretty_representation();
-
enum_type_decl_sptr first = d.first_enum(), second = d.second_enum();
- report_name_size_and_alignment_changes(first, second, d.context(),
+ const diff_context_sptr& ctxt = d.context();
+
+ string name = first->get_pretty_representation();
+
+ // Report enum decl-only <-> definition changes.
+ if (ctxt->get_allowed_category() & ENUM_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, d.context(), out, indent);
+ maybe_report_diff_for_member(first, second, ctxt, out, indent);
//underlying type
d.underlying_type_diff()->report(out, indent);
@@ -165,12 +185,12 @@ default_reporter::report(const enum_diff& d, ostream& out,
<< "' from value '"
<< i->first.get_value() << "' to '"
<< i->second.get_value() << "'";
- report_loc_info(second, *d.context(), out);
+ report_loc_info(second, *ctxt, out);
out << "\n";
}
}
- if (d.context()->show_leaf_changes_only())
+ if (ctxt->show_leaf_changes_only())
maybe_report_interfaces_impacted_by_diff(&d, out, indent);
d.reported_once(true);
@@ -844,7 +864,7 @@ default_reporter::report(const class_or_union_diff& d,
const diff_context_sptr& ctxt = d.context();
- // Report class decl-only -> definition change.
+ // Report class decl-only <-> definition change.
if (ctxt->get_allowed_category() & CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY)
if (filtering::has_class_decl_only_def_change(first, second))
{
diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 850281ad..5748460e 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -139,6 +139,11 @@ typedef unordered_map<Dwarf_Off, class_decl_sptr> die_class_map_type;
/// corresponding class_or_union_sptr.
typedef unordered_map<Dwarf_Off, class_or_union_sptr> die_class_or_union_map_type;
+/// Convenience typedef for a map which key is the offset of a dwarf
+/// die, (given by dwarf_dieoffset()) and which value is the
+/// corresponding enum_type_decl_sptr.
+typedef unordered_map<Dwarf_Off, enum_type_decl_sptr> die_enum_map_type;
+
/// Convenience typedef for a map which key the offset of a dwarf die
/// and which value is the corresponding function_decl.
typedef unordered_map<Dwarf_Off, function_decl_sptr> die_function_decl_map_type;
@@ -198,6 +203,10 @@ typedef unordered_map<Dwarf_Off, Dwarf_Off> offset_offset_map_type;
/// value is a vector of smart pointer to a class.
typedef unordered_map<string, classes_type> string_classes_map;
+/// Convenience typedef for a map which key is a string and which
+/// value is a vector of smart pointer to a enum.
+typedef unordered_map<string, enums_type> string_enums_map;
+
/// The abstraction of the place where a partial unit has been
/// imported. This is what the DW_TAG_imported_unit DIE expresses.
///
@@ -2273,6 +2282,9 @@ public:
die_class_or_union_map_type die_wip_classes_map_;
die_class_or_union_map_type alternate_die_wip_classes_map_;
die_class_or_union_map_type type_unit_die_wip_classes_map_;
+ die_enum_map_type die_wip_enums_map_;
+ die_enum_map_type alternate_die_wip_enums_map_;
+ die_enum_map_type type_unit_die_wip_enums_map_;
die_function_type_map_type die_wip_function_types_map_;
die_function_type_map_type alternate_die_wip_function_types_map_;
die_function_type_map_type type_unit_die_wip_function_types_map_;
@@ -2282,6 +2294,7 @@ public:
vector<Dwarf_Off> type_unit_types_to_canonicalize_;
vector<type_base_sptr> extra_types_to_canonicalize_;
string_classes_map decl_only_classes_map_;
+ string_enums_map decl_only_enums_map_;
die_tu_map_type die_tu_map_;
corpus_group_sptr cur_corpus_group_;
corpus_sptr cur_corpus_;
@@ -4340,6 +4353,44 @@ public:
return die_wip_classes_map_;
}
+ /// Getter of a map that associates a die that represents a
+ /// enum with the declaration of the enum, while the enum
+ /// is being constructed.
+ ///
+ /// @param source where the DIE is from.
+ ///
+ /// @return the map that associates a DIE to the enum that is being
+ /// built.
+ const die_enum_map_type&
+ die_wip_enums_map(die_source source) const
+ {return const_cast<read_context*>(this)->die_wip_enums_map(source);}
+
+ /// Getter of a map that associates a die that represents a
+ /// enum with the declaration of the enum, while the enum
+ /// is being constructed.
+ ///
+ /// @param source where the DIE comes from.
+ ///
+ /// @return the map that associates a DIE to the enum that is being
+ /// built.
+ die_enum_map_type&
+ die_wip_enums_map(die_source source)
+ {
+ switch (source)
+ {
+ case PRIMARY_DEBUG_INFO_DIE_SOURCE:
+ break;
+ case ALT_DEBUG_INFO_DIE_SOURCE:
+ return alternate_die_wip_enums_map_;
+ case TYPE_UNIT_DIE_SOURCE:
+ return type_unit_die_wip_enums_map_;
+ case NO_DEBUG_INFO_DIE_SOURCE:
+ case NUMBER_OF_DIE_SOURCES:
+ ABG_ASSERT_NOT_REACHED;
+ }
+ return die_wip_enums_map_;
+ }
+
/// Getter for a map that associates a die (that represents a
/// function type) whith a function type, while the function type is
/// being constructed (WIP == work in progress).
@@ -4620,6 +4671,203 @@ public:
}
}
+ /// Getter for the map of declaration-only enums that are to be
+ /// resolved to their definition enums by the end of the corpus
+ /// loading.
+ ///
+ /// @return a map of string -> vector of enums where the key is
+ /// the fully qualified name of the enum and the value is the
+ /// vector of declaration-only enum.
+ const string_enums_map&
+ declaration_only_enums() const
+ {return decl_only_enums_map_;}
+
+ /// Getter for the map of declaration-only enums that are to be
+ /// resolved to their definition enums by the end of the corpus
+ /// loading.
+ ///
+ /// @return a map of string -> vector of enums where the key is
+ /// the fully qualified name of the enum and the value is the
+ /// vector of declaration-only enum.
+ string_enums_map&
+ declaration_only_enums()
+ {return decl_only_enums_map_;}
+
+ /// If a given enum is a declaration-only enum then stash it on
+ /// the side so that at the end of the corpus reading we can resolve
+ /// it to its definition.
+ ///
+ /// @param enom the enum to consider.
+ void
+ maybe_schedule_declaration_only_enum_for_resolution(enum_type_decl_sptr& enom)
+ {
+ if (enom->get_is_declaration_only()
+ && enom->get_definition_of_declaration() == 0)
+ {
+ string qn = enom->get_qualified_name();
+ string_enums_map::iterator record =
+ declaration_only_enums().find(qn);
+ if (record == declaration_only_enums().end())
+ declaration_only_enums()[qn].push_back(enom);
+ else
+ record->second.push_back(enom);
+ }
+ }
+
+ /// Test if a given declaration-only enum has been scheduled for
+ /// resolution to a defined enum.
+ ///
+ /// @param enom the enum to consider for the test.
+ ///
+ /// @return true iff @p enom is a declaration-only enum and if
+ /// it's been scheduled for resolution to a defined enum.
+ bool
+ is_decl_only_enum_scheduled_for_resolution(enum_type_decl_sptr& enom)
+ {
+ if (enom->get_is_declaration_only())
+ return (declaration_only_enums().find(enom->get_qualified_name())
+ != declaration_only_enums().end());
+
+ return false;
+ }
+
+ /// Walk the declaration-only enums that have been found during
+ /// the building of the corpus and resolve them to their definitions.
+ void
+ resolve_declaration_only_enums()
+ {
+ vector<string> resolved_enums;
+
+ for (string_enums_map::iterator i =
+ declaration_only_enums().begin();
+ i != declaration_only_enums().end();
+ ++i)
+ {
+ bool to_resolve = false;
+ for (enums_type::iterator j = i->second.begin();
+ j != i->second.end();
+ ++j)
+ if ((*j)->get_is_declaration_only()
+ && ((*j)->get_definition_of_declaration() == 0))
+ to_resolve = true;
+
+ if (!to_resolve)
+ {
+ resolved_enums.push_back(i->first);
+ continue;
+ }
+
+ // Now, for each decl-only enum that have the current name
+ // 'i->first', let's try to poke at the fully defined enum
+ // that is defined in the same translation unit as the
+ // declaration.
+ //
+ // If we find one enum (defined in the TU of the declaration)
+ // that defines the declaration, then the declaration can be
+ // resolved to that enum.
+ //
+ // If no defining enum is found in the TU of the declaration,
+ // then there are possibly three cases to consider:
+ //
+ // 1/ There is exactly one enum that defines the
+ // declaration and that enum is defined in another TU. In
+ // this case, the declaration is resolved to that
+ // definition.
+ //
+ // 2/ There are more than one enum that define that
+ // declaration and none of them is defined in the TU of the
+ // declaration. In this case, the declaration is left
+ // unresolved.
+ //
+ // 3/ No enum defines the declaration. In this case, the
+ // declaration is left unresoved.
+
+ // So get the enums that might define the current
+ // declarations which name is i->first.
+ const type_base_wptrs_type *enums =
+ lookup_enum_types(i->first, *current_corpus());
+ if (!enums)
+ continue;
+
+ unordered_map<string, enum_type_decl_sptr> per_tu_enum_map;
+ for (type_base_wptrs_type::const_iterator c = enums->begin();
+ c != enums->end();
+ ++c)
+ {
+ enum_type_decl_sptr enom = is_enum_type(type_base_sptr(*c));
+ ABG_ASSERT(enom);
+
+ enom = is_enum_type(look_through_decl_only_enum(enom));
+ if (enom->get_is_declaration_only())
+ continue;
+
+ string tu_path = enom->get_translation_unit()->get_absolute_path();
+ if (tu_path.empty())
+ continue;
+
+ // Build a map that associates the translation unit path
+ // to the enum (that potentially defines the declarations
+ // that we consider) that are defined in that translation unit.
+ per_tu_enum_map[tu_path] = enom;
+ }
+
+ if (!per_tu_enum_map.empty())
+ {
+ // Walk the declarations to resolve and resolve them
+ // either to the definitions that are in the same TU as
+ // the declaration, or to the definition found elsewhere,
+ // if there is only one such definition.
+ for (enums_type::iterator j = i->second.begin();
+ j != i->second.end();
+ ++j)
+ {
+ if ((*j)->get_is_declaration_only()
+ && ((*j)->get_definition_of_declaration() == 0))
+ {
+ string tu_path =
+ (*j)->get_translation_unit()->get_absolute_path();
+ unordered_map<string, enum_type_decl_sptr>::const_iterator e =
+ per_tu_enum_map.find(tu_path);
+ if (e != per_tu_enum_map.end())
+ (*j)->set_definition_of_declaration(e->second);
+ else if (per_tu_enum_map.size() == 1)
+ (*j)->set_definition_of_declaration
+ (per_tu_enum_map.begin()->second);
+ }
+ }
+ resolved_enums.push_back(i->first);
+ }
+ }
+
+ size_t num_decl_only_enums = declaration_only_enums().size(),
+ num_resolved = resolved_enums.size();
+ if (show_stats())
+ cerr << "resolved " << num_resolved
+ << " enum declarations out of "
+ << num_decl_only_enums
+ << "\n";
+
+ for (vector<string>::const_iterator i = resolved_enums.begin();
+ i != resolved_enums.end();
+ ++i)
+ declaration_only_enums().erase(*i);
+
+ for (string_enums_map::iterator i = declaration_only_enums().begin();
+ i != declaration_only_enums().end();
+ ++i)
+ {
+ if (show_stats())
+ {
+ if (i == declaration_only_enums().begin())
+ cerr << "Here are the "
+ << num_decl_only_enums - num_resolved
+ << " unresolved enum declarations:\n";
+ else
+ cerr << " " << i->first << "\n";
+ }
+ }
+ }
+
/// Some functions described by DWARF may have their linkage name
/// set, but no link to their actual underlying elf symbol. When
/// these are virtual member functions, comparing the enclosing type
@@ -13386,18 +13634,32 @@ build_enum_type(read_context& ctxt,
if (tag != DW_TAG_enumeration_type)
return result;
+ die_source source;
+ ABG_ASSERT(ctxt.get_die_source(die, source));
+ {
+ die_enum_map_type::const_iterator i =
+ ctxt.die_wip_enums_map(source).find(dwarf_dieoffset(die));
+ if (i != ctxt.die_wip_enums_map(source).end())
+ {
+ enum_type_decl_sptr u = is_enum_type(i->second);
+ ABG_ASSERT(u);
+ return u;
+ }
+ }
+
string name, linkage_name;
location loc;
die_loc_and_name(ctxt, die, loc, name, linkage_name);
+ bool is_declaration_only = die_is_declaration_only(die);
- bool enum_is_anonymous = false;
+ bool is_anonymous = false;
// If the enum is anonymous, let's give it a name.
if (name.empty())
{
name = get_internal_anonymous_die_prefix_name(die);
ABG_ASSERT(!name.empty());
// But we remember that the type is anonymous.
- enum_is_anonymous = true;
+ is_anonymous = true;
if (size_t s = scope->get_num_anonymous_member_enums())
name = build_internal_anonymous_die_name(name, s);
@@ -13409,7 +13671,7 @@ build_enum_type(read_context& ctxt,
// representation (name) and location can be later detected as being
// for the same type.
- if (!enum_is_anonymous)
+ if (!is_anonymous)
{
if (use_odr)
{
@@ -13442,6 +13704,7 @@ build_enum_type(read_context& ctxt,
uint64_t size = 0;
if (die_unsigned_constant_attribute(die, DW_AT_byte_size, size))
size *= 8;
+ bool is_artificial = die_is_artificial(die);
// for now we consider that underlying types of enums are all anonymous
bool enum_underlying_type_is_anonymous= true;
@@ -13474,8 +13737,6 @@ build_enum_type(read_context& ctxt,
while (dwarf_siblingof(&child, &child) == 0);
}
- bool is_artificial = die_is_artificial(die);
-
// DWARF up to version 4 (at least) doesn't seem to carry the
// underlying type, so let's create an artificial one here, which
// sole purpose is to be passed to the constructor of the
@@ -13491,9 +13752,13 @@ build_enum_type(read_context& ctxt,
t = dynamic_pointer_cast<type_decl>(d);
ABG_ASSERT(t);
result.reset(new enum_type_decl(name, loc, t, enms, linkage_name));
- result->set_is_anonymous(enum_is_anonymous);
+ result->set_is_anonymous(is_anonymous);
+ result->set_is_declaration_only(is_declaration_only);
result->set_is_artificial(is_artificial);
ctxt.associate_die_to_type(die, result, where_offset);
+
+ ctxt.maybe_schedule_declaration_only_enum_for_resolution(result);
+
return result;
}
@@ -16226,6 +16491,24 @@ read_debug_info_into_corpus(read_context& ctxt)
}
}
+ {
+ tools_utils::timer t;
+ if (ctxt.do_log())
+ {
+ cerr << "resolving declaration only enums ...";
+ t.start();
+ }
+ ctxt.resolve_declaration_only_enums();
+ if (ctxt.do_log())
+ {
+ t.stop();
+ cerr << " DONE@" << ctxt.current_corpus()->get_path()
+ << ":"
+ << t
+ <<"\n";
+ }
+ }
+
{
tools_utils::timer t;
if (ctxt.do_log())
@@ -16660,7 +16943,18 @@ build_ir_node_from_die(read_context& ctxt,
case DW_TAG_enumeration_type:
{
- if (!type_is_suppressed(ctxt, scope, die))
+ bool type_is_private = false;
+ bool type_suppressed =
+ type_is_suppressed(ctxt, scope, die, type_is_private);
+ if (type_suppressed && type_is_private)
+ // The type is suppressed because it's private. If other
+ // non-suppressed and declaration-only instances of this
+ // type exist in the current corpus, then it means those
+ // non-suppressed instances are opaque versions of the
+ // suppressed private type. Lets return one of these opaque
+ // types then.
+ result = get_opaque_version_of_type(ctxt, scope, die, where_offset);
+ else if (!type_suppressed)
{
enum_type_decl_sptr e = build_enum_type(ctxt, die, scope,
where_offset);
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 27831352..bfdc5679 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -7674,6 +7674,38 @@ typedef_decl*
is_typedef(type_base* t)
{return dynamic_cast<typedef_decl*>(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.
@@ -8057,6 +8089,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.
@@ -9915,6 +9991,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.
@@ -14724,17 +14831,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
@@ -14775,6 +14891,25 @@ enum_type_decl::enum_type_decl(const string& name,
e->set_enum_type(this);
}
+/// Constructor for instances of enum_type_decl that represent a
+/// declaration without definition.
+///
+/// @param env the environment we are operating from.
+///
+/// @param name the name of the enum.
+enum_type_decl::enum_type_decl(const environment* env,
+ const string& name)
+ : type_or_decl_base(env,
+ ENUM_TYPE
+ | ABSTRACT_TYPE_BASE
+ | ABSTRACT_DECL_BASE),
+ type_base(env, 0, 0),
+ decl_base(env, name, location(), name),
+ priv_(new priv)
+{
+ runtime_type_instance(this);
+}
+
/// Return the underlying type of the enum.
type_base_sptr
enum_type_decl::get_underlying_type() const
@@ -14790,6 +14925,100 @@ 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_;}
+
+/// If this @ref enum_type_decl_sptr is a definition, get its earlier
+/// declaration.
+///
+/// @return the earlier declaration of the enum, if any.
+decl_base_sptr
+enum_type_decl::get_earlier_declaration() const
+{return priv_->declaration_;}
+
+/// set the earlier declaration of this @ref enum_type_decl definition.
+///
+/// @param declaration the earlier declaration to set. Note that it's
+/// set only if it's a pure declaration.
+void
+enum_type_decl::set_earlier_declaration(decl_base_sptr declaration)
+{
+ enum_type_decl_sptr cl = dynamic_pointer_cast<enum_type_decl>(declaration);
+ if (cl && cl->get_is_declaration_only())
+ priv_->declaration_ = 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.
///
--
2.26.1.301.g55bc3eb7cb9-goog
next prev parent reply other threads:[~2020-04-23 13:04 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-23 13:03 [RFC PATCH 0/2] Incomplete enum support Giuliano Procida
2020-04-23 13:03 ` Giuliano Procida [this message]
2020-05-12 14:02 ` [RFC PATCH 1/2] Support declaration-only enums Matthias Maennich
2020-05-15 14:42 ` Giuliano Procida
2020-05-22 10:03 ` Dodji Seketeli
2020-06-04 17:23 ` Giuliano Procida
2020-04-23 13:03 ` [RFC PATCH 2/2] Add tests for " Giuliano Procida
2020-05-12 14:05 ` Matthias Maennich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200423130356.93136-2-gprocida@google.com \
--to=gprocida@google.com \
--cc=dodji@seketeli.org \
--cc=kernel-team@android.com \
--cc=libabigail@sourceware.org \
--cc=maennich@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).