[applied] xml reader: Fix recursive qualified & reference type definition

Message ID 87cztf3wxz.fsf@redhat.com
State Committed
Headers
Series [applied] xml reader: Fix recursive qualified & reference type definition |

Commit Message

Dodji Seketeli May 25, 2021, 10:54 a.m. UTC
  Hello,

This is a followup patch for the fix for
https://bugzilla.redhat.com/show_bug.cgi?id=1944088, which was in the
patch:

    commit 51ae965305f9eaa554d5d2111fa24eeb07d35244
    Author: Dodji Seketeli <dodji@redhat.com>
    Date:   Fri May 21 23:55:44 2021 +0200

	abixml reader: Fix recursive type definition handling

After that patch, I noticed that qualified and reference types also
need to be able to handle the case where their underlying/pointed-to
type recursively refers to the type being created.  Just like typedef
and pointer types in that patch.

This patch thus adjusts build_qualified_type_decl and
build_reference_type_def to support that.  It also adjusts the
qualified_type_def and reference_type_def classes to support being
created without underlying/pointed-to type initially.

	* include/abg-ir.h (qualified_type_def::qualified_type_def):
	Declare a constructor with no underlying type.
	(reference_type_def::reference_type_def): Declare a constructor
	with no pointed-to type.
	(reference_type_def::set_pointed_to_type): Declare new method.
	* src/abg-ir.cc (qualified_type_def::priv::priv): Define a
	constructor that takes no underlying type.
	(qualified_type_def::build_name): Make this work even on
	incomplete types with no underlying type.  In that case, this
	behaves like if the underlying type is "void".
	(qualified_type_def::qualified_type_def): Define a constructor
	that takes no underlying type.
	(qualified_type_def::get_size_in_bits): Make this work on
	incomplete types with no underlying type.
	(qualified_type_def::set_underlying_type): Adjust to properly
	update this type when a new underlying type is set.  Particularly,
	its name and the lookup maps from the type scope.
	(reference_type_def::reference_type_def): Define a constructor
	that takes no pointed-to type.
	(reference_type_def::set_pointed_to_type): Define new function.
	* src/abg-reader.cc (build_qualified_type_decl): Construct the
	qualified type early before we try to construct its underlying
	type.  Associate this incomplete type with the type-id.  Then try
	to construct the underlying type.  During its construction, if
	this incomplete qualified type is needed due to recursion then it
	can be used, leading to just one qualified type being used as it
	should be.
	(build_reference_type_def): Likewise for building incomplete
	reference type first before its pointed-to type.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>

Applied to master.

---
 include/abg-ir.h  |   8 +++
 src/abg-ir.cc     | 154 +++++++++++++++++++++++++++++++++++++++++++---
 src/abg-reader.cc | 136 +++++++++++++++++-----------------------
 3 files changed, 211 insertions(+), 87 deletions(-)
  

Patch

diff --git a/include/abg-ir.h b/include/abg-ir.h
index 2ffa5c3c..e0638087 100644
--- a/include/abg-ir.h
+++ b/include/abg-ir.h
@@ -2165,6 +2165,8 @@  public:
 
   qualified_type_def(type_base_sptr type, CV quals, const location& locus);
 
+  qualified_type_def(environment* env, CV quals, const location& locus);
+
   virtual size_t
   get_size_in_bits() const;
 
@@ -2328,6 +2330,12 @@  public:
 		     bool lvalue, size_t size_in_bits,
 		     size_t alignment_in_bits, const location& locus);
 
+  reference_type_def(const environment* env, bool lvalue, size_t size_in_bits,
+		     size_t alignment_in_bits, const location& locus);
+
+  void
+  set_pointed_to_type(type_base_sptr& pointed_to_type);
+
   virtual bool
   operator==(const decl_base&) const;
 
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 764c12f2..5963a7cd 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -14313,6 +14313,10 @@  class qualified_type_def::priv
     : cv_quals_(quals),
       underlying_type_(t)
   {}
+
+    priv(qualified_type_def::CV quals)
+    : cv_quals_(quals)
+  {}
 };// end class qualified_type_def::priv
 
 /// Build the name of the current instance of qualified type.
@@ -14328,11 +14332,17 @@  class qualified_type_def::priv
 string
 qualified_type_def::build_name(bool fully_qualified, bool internal) const
 {
-  ABG_ASSERT(get_underlying_type());
+  type_base_sptr t = get_underlying_type();
+  if (!t)
+    // The qualified type might temporarily have no underlying type,
+    // especially during the construction of the type, while the
+    // underlying type is not yet constructed.  In that case, let's do
+    // like if the underlying type is the 'void' type.
+    t = get_environment()->get_void_type();
 
-  return get_name_of_qualified_type(get_underlying_type(),
-				    get_cv_quals(),
-				    fully_qualified, internal);
+  return get_name_of_qualified_type(t, get_cv_quals(),
+				    fully_qualified,
+				    internal);
 }
 
 /// This function is automatically invoked whenever an instance of
@@ -14371,6 +14381,32 @@  qualified_type_def::qualified_type_def(type_base_sptr		type,
   set_name(name);
 }
 
+/// Constructor of the qualified_type_def
+///
+/// @param env the environment of the type.
+///
+/// @param quals a bitfield representing the const/volatile qualifiers
+///
+/// @param locus the location of the qualified type definition
+qualified_type_def::qualified_type_def(environment* env,
+				       CV quals,
+				       const location& locus)
+  : type_or_decl_base(env,
+		      QUALIFIED_TYPE
+		      | ABSTRACT_TYPE_BASE
+		      | ABSTRACT_DECL_BASE),
+    type_base(env, /*size_in_bits=*/0,
+	      /*alignment_in_bits=*/0),
+    decl_base(env, "", locus, ""),
+    priv_(new priv(quals))
+{
+  runtime_type_instance(this);
+  // We don't yet have an underlying type.  So for naming purpose,
+  // let's temporarily pretend the underlying type is 'void'.
+  interned_string name = env->intern("void");
+  set_name(name);
+}
+
 /// Get the size of the qualified type def.
 ///
 /// This is an overload for type_base::get_size_in_bits().
@@ -14379,9 +14415,16 @@  qualified_type_def::qualified_type_def(type_base_sptr		type,
 size_t
 qualified_type_def::get_size_in_bits() const
 {
-  size_t s = get_underlying_type()->get_size_in_bits();
-  if (s != type_base::get_size_in_bits())
-    const_cast<qualified_type_def*>(this)->set_size_in_bits(s);
+  size_t s = 0;
+  if (type_base_sptr ut = get_underlying_type())
+    {
+      // We do have the underlying type properly set, so let's make
+      // the size of the qualified type match the size of its
+      // underlying type.
+      s = ut->get_size_in_bits();
+      if (s != type_base::get_size_in_bits())
+	const_cast<qualified_type_def*>(this)->set_size_in_bits(s);
+    }
   return type_base::get_size_in_bits();
 }
 
@@ -14631,7 +14674,25 @@  qualified_type_def::get_underlying_type() const
 /// @param t the new underlying type.
 void
 qualified_type_def::set_underlying_type(const type_base_sptr& t)
-{priv_->underlying_type_ = t;}
+{
+  ABG_ASSERT(t);
+  priv_->underlying_type_ = t;
+  // Now we need to update other properties that depend on the new underlying type.
+  set_size_in_bits(t->get_size_in_bits());
+  set_alignment_in_bits(t->get_alignment_in_bits());
+  interned_string name = get_environment()->intern(build_name(false));
+  set_name(name);
+  if (scope_decl* s = get_scope())
+      {
+	// Now that the name has been updated, we need to update the
+	// lookup maps accordingly.
+	scope_decl::declarations::iterator i;
+	if (s->find_iterator_for_member(this, i))
+	  maybe_update_types_lookup_map(*i);
+	else
+	  ABG_ASSERT_NOT_REACHED;
+      }
+}
 
 /// Non-member equality operator for @ref qualified_type_def
 ///
@@ -15108,6 +15169,18 @@  void
 reference_type_def::on_canonical_type_set()
 {clear_qualified_name();}
 
+/// Constructor of the reference_type_def type.
+///
+/// @param pointed_to the pointed to type.
+///
+/// @param lvalue wether the reference is an lvalue reference.  If
+/// false, the reference is an rvalue one.
+///
+/// @param size_in_bits the size of the type, in bits.
+///
+/// @param align_in_bits the alignment of the type, in bits.
+///
+/// @param locus the source location of the type.
 reference_type_def::reference_type_def(const type_base_sptr	pointed_to,
 				       bool			lvalue,
 				       size_t			size_in_bits,
@@ -15148,6 +15221,71 @@  reference_type_def::reference_type_def(const type_base_sptr	pointed_to,
     {}
 }
 
+/// Constructor of the reference_type_def type.
+///
+/// This one creates a type that has no pointed-to type, temporarily.
+/// This is useful for cases where the underlying type is not yet
+/// available.  It can be set later using
+/// reference_type_def::set_pointed_to_type().
+///
+/// @param env the environment of the type.
+///
+/// @param lvalue wether the reference is an lvalue reference.  If
+/// false, the reference is an rvalue one.
+///
+/// @param size_in_bits the size of the type, in bits.
+///
+/// @param align_in_bits the alignment of the type, in bits.
+///
+/// @param locus the source location of the type.
+reference_type_def::reference_type_def(const environment* env, bool lvalue,
+				       size_t size_in_bits,
+				       size_t alignment_in_bits,
+				       const location& locus)
+  : type_or_decl_base(env,
+		      REFERENCE_TYPE
+		      | ABSTRACT_TYPE_BASE
+		      | ABSTRACT_DECL_BASE),
+    type_base(env, size_in_bits, alignment_in_bits),
+    decl_base(env, "", locus, ""),
+    is_lvalue_(lvalue)
+{
+  runtime_type_instance(this);
+  string name = "void&";
+  if (!is_lvalue())
+    name += "&";
+  ABG_ASSERT(env);
+  set_name(env->intern(name));
+  pointed_to_type_ = type_base_wptr(env->get_void_type());
+}
+
+/// Setter of the pointed_to type of the current reference type.
+///
+/// @param pointed_to the new pointed to type.
+void
+reference_type_def::set_pointed_to_type(type_base_sptr& pointed_to_type)
+{
+  ABG_ASSERT(pointed_to_type);
+  pointed_to_type_ = pointed_to_type;
+
+  decl_base_sptr pto;
+  try
+    {pto = dynamic_pointer_cast<decl_base>(pointed_to_type);}
+  catch (...)
+    {}
+
+  if (pto)
+    {
+      set_visibility(pto->get_visibility());
+      string name = string(pto->get_name()) + "&";
+      if (!is_lvalue())
+	name += "&";
+      environment* env = pto->get_environment();
+      ABG_ASSERT(env && env == get_environment());
+      set_name(env->intern(name));
+    }
+}
+
 /// Compares two instances of @ref reference_type_def.
 ///
 /// If the two intances are different, set a bitfield to give some
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 4e554d6d..34136140 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -3688,29 +3688,17 @@  build_qualified_type_decl(read_context&	ctxt,
       return result;
     }
 
-  string type_id;
-  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "type-id"))
-    type_id = CHAR_STR(s);
-
-  shared_ptr<type_base> underlying_type =
-    ctxt.build_or_get_type_decl(type_id, true);
-  ABG_ASSERT(underlying_type);
-
-  // maybe building the underlying type triggered building this one in
-  // the mean time ...
-  if (decl_base_sptr d = ctxt.get_decl_for_xml_node(node))
-    {
-      qualified_type_def_sptr result =
-	dynamic_pointer_cast<qualified_type_def>(d);
-      ABG_ASSERT(result);
-      return result;
-    }
-
   string id;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE (node, "id"))
     id = CHAR_STR(s);
 
-  string const_str;
+  ABG_ASSERT(!id.empty());
+
+  location loc;
+  read_location(ctxt, node, loc);
+
+  qualified_type_def::CV cv = qualified_type_def::CV_NONE;
+    string const_str;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "const"))
     const_str = CHAR_STR(s);
   bool const_cv = const_str == "yes";
@@ -3725,7 +3713,6 @@  build_qualified_type_decl(read_context&	ctxt,
     restrict_str = CHAR_STR(s);
   bool restrict_cv = restrict_str == "yes";
 
-  qualified_type_def::CV cv = qualified_type_def::CV_NONE;
   if (const_cv)
     cv = cv | qualified_type_def::CV_CONST;
   if (volatile_cv)
@@ -3733,31 +3720,31 @@  build_qualified_type_decl(read_context&	ctxt,
   if (restrict_cv)
     cv = cv | qualified_type_def::CV_RESTRICT;
 
-  location loc;
-  read_location(ctxt, node, loc);
-
-  ABG_ASSERT(!id.empty());
-
+  // Create the qualified type /before/ the underlying type.  After
+  // the creation, the type is 'keyed' using
+  // ctxt.push_and_key_type_decl.  This means that the type can be
+  // retrieved from its type ID.  This is so that if the underlying
+  // type indirectly uses this qualified type (via recursion) then
+  // that is made possible.
+  //
+  // The underlying type will later be set after it's created.
   qualified_type_def_sptr decl;
+  decl.reset(new qualified_type_def(ctxt.get_environment(), cv, loc));
+  maybe_set_artificial_location(ctxt, node, decl);
+  ctxt.push_and_key_type_decl(decl, id, add_to_current_scope);
+  ctxt.map_xml_node_to_decl(node, decl);
 
-  if (type_base_sptr d = ctxt.get_type_decl(id))
-    {
-      qualified_type_def_sptr ty = is_qualified_type(d);
-      ABG_ASSERT(ty);
-      string pr1 = get_pretty_representation(ty->get_underlying_type()),
-	pr2 = get_pretty_representation(underlying_type);
-      return ty;
-    }
+  string type_id;
+  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "type-id"))
+    type_id = CHAR_STR(s);
+  ABG_ASSERT(!type_id.empty());
 
-  decl.reset(new qualified_type_def(underlying_type, cv, loc));
-   maybe_set_artificial_location(ctxt, node, decl);
-  if (ctxt.push_and_key_type_decl(decl, id, add_to_current_scope))
-    {
-      ctxt.map_xml_node_to_decl(node, decl);
-      return decl;
-    }
+  shared_ptr<type_base> underlying_type =
+    ctxt.build_or_get_type_decl(type_id, true);
+  ABG_ASSERT(underlying_type);
+  decl->set_underlying_type(underlying_type);
 
-  return shared_ptr<qualified_type_def>((qualified_type_def*)0);
+  return decl;
 }
 
 /// Build a pointer_type_def from a 'pointer-type-def' xml node.
@@ -3865,33 +3852,6 @@  build_reference_type_def(read_context&		ctxt,
       return result;
     }
 
-  string kind;
-  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "kind"))
-    kind = CHAR_STR(s); // this should be either "lvalue" or "rvalue".
-  bool is_lvalue = kind == "lvalue";
-
-  string type_id;
-  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "type-id"))
-    type_id = CHAR_STR(s);
-
-  shared_ptr<type_base> pointed_to_type = ctxt.build_or_get_type_decl(type_id,
-								      true);
-  ABG_ASSERT(pointed_to_type);
-
-  // maybe building the underlying type triggered building this one in
-  // the mean time ...
-  if (decl_base_sptr d = ctxt.get_decl_for_xml_node(node))
-    {
-      reference_type_def_sptr result =
-	dynamic_pointer_cast<reference_type_def>(d);
-      ABG_ASSERT(result);
-      return result;
-    }
-
-  size_t size_in_bits = ctxt.get_translation_unit()->get_address_size();
-  size_t alignment_in_bits = 0;
-  read_size_and_alignment(node, size_in_bits, alignment_in_bits);
-
   string id;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "id"))
     id = CHAR_STR(s);
@@ -3901,26 +3861,44 @@  build_reference_type_def(read_context&		ctxt,
     {
       reference_type_def_sptr ty = is_reference_type(d);
       ABG_ASSERT(ty);
-      ABG_ASSERT(ctxt.types_equal(pointed_to_type, ty->get_pointed_to_type()));
       return ty;
     }
 
   location loc;
   read_location(ctxt, node, loc);
+  string kind;
+  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "kind"))
+    kind = CHAR_STR(s); // this should be either "lvalue" or "rvalue".
+  bool is_lvalue = kind == "lvalue";
+
+  size_t size_in_bits = ctxt.get_translation_unit()->get_address_size();
+  size_t alignment_in_bits = 0;
+  read_size_and_alignment(node, size_in_bits, alignment_in_bits);
+
+  string type_id;
+  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "type-id"))
+    type_id = CHAR_STR(s);
+  ABG_ASSERT(!type_id.empty());
 
-  shared_ptr<reference_type_def> t(new reference_type_def(pointed_to_type,
-							  is_lvalue,
-							  size_in_bits,
-							  alignment_in_bits,
-							  loc));
+  // Create the reference type /before/ the pointed-to type.  After
+  // the creation, the type is 'keyed' using
+  // ctxt.push_and_key_type_decl.  This means that the type can be
+  // retrieved from its type ID.  This is so that if the pointed-to
+  // type indirectly uses this reference type (via recursion) then
+  // that is made possible.
+  reference_type_def_sptr t(new reference_type_def(ctxt.get_environment(),
+						   is_lvalue, size_in_bits,
+						   alignment_in_bits, loc));
   maybe_set_artificial_location(ctxt, node, t);
   if (ctxt.push_and_key_type_decl(t, id, add_to_current_scope))
-    {
-      ctxt.map_xml_node_to_decl(node, t);
-      return t;
-    }
+    ctxt.map_xml_node_to_decl(node, t);
 
-  return nil;
+  type_base_sptr pointed_to_type =
+    ctxt.build_or_get_type_decl(type_id,/*add_to_current_scope=*/ true);
+  ABG_ASSERT(pointed_to_type);
+  t->set_pointed_to_type(pointed_to_type);
+
+  return t;
 }
 
 /// Build a function_type from a pointer to 'function-type'