[08/11] Support declaration-only enums in DWARF reader.

  This patch adds declaration-only handling enums to the DWARF reader,
except for support in get_opaque_version_of_type.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-dwarf-reader.cc | 238 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 237 insertions(+), 1 deletion(-)
  

Giuliano Procida <gprocida@google.com> a écrit:

> This patch adds declaration-only handling enums to the DWARF reader,
> except for support in get_opaque_version_of_type.

I have amended this patch to fuse it with this one: 
    "[PATCH 09/11] Support constructing opaque types for enums"
, add a ChangeLog part to its commit log and make some ancillary changes
like ensuring that the underlying type of the enum is also flagged
decl-only when the enum is decl-only.  This helps to filter out spurious
changes decl-only-ness changes to to underlying types of enums.

I am adding the resulting patch below, but it can also be retrived from
the branch dodji/incomp-enums browsable from
https://sourceware.org/git/?p=libabigail.git;a=shortlog;h=refs/heads/dodji/incomp-enums.

Cheers,

From 2351d1ee8b3597980ecd43e1f677ace4d28eae6c Mon Sep 17 00:00:00 2001
From: Dodji Seketeli <dodji@redhat.com>
Date: Wed, 10 Jun 2020 12:59:38 +0100
Subject: [PATCH 2/4] Support declaration-only enums in DWARF reader.

This patch adds declaration-only handling enums to the DWARF reader.

	* src/abg-dwarf-reader.cc (string_enums_map): Define new
	convenience typedef.
	(read_context::decl_only_enums_map_): Define new data member.
	(read_context::{declaration_only_enums,
	is_decl_only_enum_scheduled_for_resolution,
	resolve_declaration_only_enums}): Define new member functions.
	(build_internal_underlying_enum_type_name)
	(build_enum_underlying_type): Factorize these functions out of ...
	(build_enum_type): ... here.  Detect a decl-only enum and flag it
	as such.  If the enum type is decl-only, then set its underlying
	type as decl-only as well.
	(build_enum_underlying_type): Mark the underlying type as
	artificial.
	(get_opaque_version_of_type): Make this handle enums as well.  So
	make its return type be type_or_decl_base_sptr, rather than just
	class_or_union_sptr as it used to be.
	(read_debug_info_into_corpus): Add logging to trace decl-only
	enums resolution.
	(build_ir_node_from_die): Detect when a suppression specification
	makes an enum opaque.  In that case, get an opaque version of the
	enum type by invoking get_opaque_version_of_type.  Note that
	get_opaque_version_of_type doesn't support returning opaque
	-- i.e, decl-only enum types -- yet, but this is going to be
	handled in a subsequent patch.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-dwarf-reader.cc | 379 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 348 insertions(+), 31 deletions(-)

diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index afe574d..dd9921a 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -198,6 +198,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.
 ///
@@ -434,12 +438,15 @@ static string
 build_internal_anonymous_die_name(const string &base_name,
 				  size_t anonymous_type_index);
 
-
 static string
 get_internal_anonymous_die_name(Dwarf_Die *die,
 				size_t anonymous_type_index);
 
 static string
+build_internal_underlying_enum_type_name(const string &base_name,
+					 bool is_anonymous);
+
+static string
 die_qualified_type_name(const read_context& ctxt,
 			const Dwarf_Die* die,
 			size_t where);
@@ -2233,6 +2240,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_;
@@ -4442,6 +4450,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
@@ -10268,6 +10473,29 @@ build_internal_anonymous_die_name(const string &base_name,
   return name;
 }
 
+/// Build the internal name of the underlying type of an enum.
+///
+/// @param base_name the (unqualified) name of the enum the underlying
+/// type is destined to.
+///
+/// @param is_anonymous true if the underlying type of the enum is to
+/// be anonymous.
+static string
+build_internal_underlying_enum_type_name(const string &base_name,
+					 bool is_anonymous = true)
+{
+  std::ostringstream o;
+
+  if (is_anonymous)
+    o << "unnamed-enum";
+  else
+    o << "enum-" << base_name;
+
+  o << "-underlying-type";
+
+  return o.str();
+}
+
 /// Build a full internal anonymous type name.
 ///
 /// @param die the DIE representing the anonymous type to consider.
@@ -12775,6 +13003,40 @@ build_type_decl(read_context& ctxt, Dwarf_Die* die, size_t where_offset)
   return result;
 }
 
+/// Construct the type that is to be used as the underlying type of an
+/// enum.
+///
+/// @param ctxt the read context to use.
+///
+/// @param enum_name the name of the enum that this type is going to
+/// be the underlying type of.
+///
+/// @param enum_size the size of the enum.
+///
+/// @param is_anonymous whether the underlying type is anonymous or
+/// not. By default, this should be set to true as before c++11 (and
+/// in C), it's almost the case.
+static type_decl_sptr
+build_enum_underlying_type(read_context& ctxt,
+			   string enum_name,
+			   uint64_t enum_size,
+			   bool is_anonymous = true)
+{
+  string underlying_type_name =
+    build_internal_underlying_enum_type_name(enum_name, is_anonymous);
+
+  type_decl_sptr result(new type_decl(ctxt.env(), underlying_type_name,
+				      enum_size, enum_size, location()));
+  result->set_is_anonymous(is_anonymous);
+  result->set_is_artificial(true);
+  translation_unit_sptr tu = ctxt.cur_transl_unit();
+  decl_base_sptr d = add_decl_to_scope(result, tu->get_global_scope().get());
+  result = dynamic_pointer_cast<type_decl>(d);
+  ABG_ASSERT(result);
+  canonicalize(result);
+  return result;
+}
+
 /// Build an enum_type_decl from a DW_TAG_enumeration_type DIE.
 ///
 /// @param ctxt the read context to use.
@@ -12803,6 +13065,7 @@ build_enum_type(read_context&	ctxt,
   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 is_anonymous = false;
   // If the enum is anonymous, let's give it a name.
@@ -12860,15 +13123,6 @@ build_enum_type(read_context&	ctxt,
 
   // for now we consider that underlying types of enums are all anonymous
   bool enum_underlying_type_is_anonymous= true;
-  string underlying_type_name;
-  if (enum_underlying_type_is_anonymous)
-    {
-      underlying_type_name = "unnamed-enum";
-      enum_underlying_type_is_anonymous = true;
-    }
-  else
-    underlying_type_name = string("enum-") + name;
-  underlying_type_name += "-underlying-type";
 
   enum_type_decl::enumerators enms;
   Dwarf_Die child;
@@ -12893,20 +13147,19 @@ build_enum_type(read_context&	ctxt,
   // underlying type, so let's create an artificial one here, which
   // sole purpose is to be passed to the constructor of the
   // enum_type_decl type.
-  type_decl_sptr t(new type_decl(ctxt.env(), underlying_type_name,
-				 size, size, location()));
-  t->set_is_anonymous(enum_underlying_type_is_anonymous);
-  translation_unit_sptr tu = ctxt.cur_transl_unit();
-  decl_base_sptr d =
-    add_decl_to_scope(t, tu->get_global_scope().get());
-  canonicalize(t);
+  type_decl_sptr t =
+    build_enum_underlying_type(ctxt, name, size,
+			       enum_underlying_type_is_anonymous);
+  t->set_is_declaration_only(is_declaration_only);
 
-  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(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;
 }
 
@@ -15147,7 +15400,8 @@ type_is_suppressed(const read_context& ctxt,
 /// a private type.
 ///
 /// The opaque version version of the type is just a declared-only
-/// version of the type (class or union type) denoted by @p type_die.
+/// version of the type (class, union or enum type) denoted by @p
+/// type_die.
 ///
 /// @param ctxt the read context in use.
 ///
@@ -15162,13 +15416,13 @@ type_is_suppressed(const read_context& ctxt,
 ///
 /// @return the opaque version of the type denoted by @p type_die or
 /// nil if no opaque version was found.
-static class_or_union_sptr
+static type_or_decl_base_sptr
 get_opaque_version_of_type(read_context	&ctxt,
 			   scope_decl		*scope,
 			   Dwarf_Die		*type_die,
 			   size_t		where_offset)
 {
-  class_or_union_sptr result;
+  type_or_decl_base_sptr result;
 
   if (type_die == 0)
     return result;
@@ -15176,7 +15430,14 @@ get_opaque_version_of_type(read_context	&ctxt,
   unsigned tag = dwarf_tag(type_die);
   if (tag != DW_TAG_class_type
       && tag != DW_TAG_structure_type
-      && tag != DW_TAG_union_type)
+      && tag != DW_TAG_union_type
+      && tag != DW_TAG_enumeration_type)
+    return result;
+
+  if (tag == DW_TAG_union_type)
+    // TODO: also handle declaration-only unions.  To do that, we mostly
+    // need to adapt add_or_update_union_type to make it schedule
+    // declaration-only unions for resolution too.
     return result;
 
   string type_name, linkage_name;
@@ -15187,17 +15448,19 @@ get_opaque_version_of_type(read_context	&ctxt,
 
   string qualified_name = build_qualified_name(scope, type_name);
 
+  //
   // TODO: also handle declaration-only unions.  To do that, we mostly
   // need to adapt add_or_update_union_type to make it schedule
   // declaration-only unions for resolution too.
-  string_classes_map::const_iterator i =
-    ctxt.declaration_only_classes().find(qualified_name);
-  if (i != ctxt.declaration_only_classes().end())
-    result = i->second.back();
-
-  if (!result)
+  //
+  if (tag == DW_TAG_structure_type || tag == DW_TAG_class_type)
     {
-      if (tag == DW_TAG_class_type || tag == DW_TAG_structure_type)
+      string_classes_map::const_iterator i =
+	ctxt.declaration_only_classes().find(qualified_name);
+      if (i != ctxt.declaration_only_classes().end())
+	result = i->second.back();
+
+      if (!result)
 	{
 	  // So we didn't find any pre-existing forward-declared-only
 	  // class for the class definition that we could return as an
@@ -15217,6 +15480,31 @@ get_opaque_version_of_type(read_context	&ctxt,
 	}
     }
 
+  if (tag == DW_TAG_enumeration_type)
+    {
+      string_enums_map::const_iterator i =
+	ctxt.declaration_only_enums().find(qualified_name);
+      if (i != ctxt.declaration_only_enums().end())
+	result = i->second.back();
+
+      if (!result)
+	{
+	  uint64_t size = 0;
+	  if (die_unsigned_constant_attribute(type_die, DW_AT_byte_size, size))
+	    size *= 8;
+	  type_decl_sptr underlying_type =
+	    build_enum_underlying_type(ctxt, type_name, size,
+				       /*anonymous=*/true);
+	  enum_type_decl::enumerators enumeratorz;
+	  enum_type_decl_sptr enum_type (new enum_type_decl(type_name,
+							    type_location,
+							    underlying_type,
+							    enumeratorz,
+							    linkage_name));
+	  result = enum_type;
+	}
+    }
+
   return result;
 }
 
@@ -15622,6 +15910,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())
+      {
 	cerr << "fixing up functions with linkage name but "
 	     << "no advertised underlying symbols ....";
 	t.start();
@@ -16050,7 +16356,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);