Fix handling of class declaration during DWARF reading

It appears now that forcing unresolved class declarations to be
declared is not a good idea.  It's better to just leave them as is,
and they'll have a hash value of zero.  We were forcing them to be
defined (with a size of 1) because they were used as base classes.  It
appears that GCC and Clang (at least) allow base classes to be
non-complete, in case the base class has a vtable; in that case, the
full debug info of the base class would be emitted in another DSO,
where the vtable is emitted, making the base class be complete from a
debug info standpoint.  So it's better for us to be in par with that
vision.

Furthermore, one of the reasons why they were not resolved, most of
the time, was that the resolution code was buggy; and that has been
fixed in a patch applied very recently.

So this patch removes the forcing code.

The patch also fixes the handling of class declaration during the
parsing.  Basically, bugs in some versions of Clang are so that we
cannot completely trust the DW_AT_declaration property on a class.
What we do is that when we see that property, we flag the class as
being a declaration.  But then if there is a DW_AT_byte_size property,
the class is considered as being defined.  We were being over-zealous
in considering the class as being defined, because having a member
function was enough; this patch now only considers the presence of a
*virtual* member functions, data members, base classes or a
DW_AT_byte_size as being conditions for being defined.

	* src/abg-dwarf-reader.cc (read_context::decl_only_classes_map_):
	Remove this data member.
	(read_context::{declaration_only_classes_to_force_defined,
	schedule_declaration_only_class_for_forced_resolution}): Remove
	these member functions.
	(read_context::resolve_declaration_only_classes): Do not force
	resolution of class declaration.
	(build_class_type_and_add_to_ir): Do not schedule classes for
	forced-resolution when they are used as base classes.  The
	presence of a member function is not enough to make the class be
	defined.  It needs to be a virtual member function.

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

diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 73dbd9bcea83b74066fedf07c91279dc3195767c..503442819c27a05a9038d5a689a05044da0da427 100644 (file)
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -1747,16 +1747,6 @@ class read_context
   vector<Dwarf_Off>            types_to_canonicalize_;
   vector<Dwarf_Off>            alt_types_to_canonicalize_;
   string_classes_map           decl_only_classes_map_;
-  // Decl-only classes that are expected to be complete.  This is to
-  // implement a work-around for DWARF producers that are over-eager
-  // to mark certain classes as being declarations, forget to provide
-  // their definitions and use the declaration-only classes in
-  // contexts that require them to be defined; for instance, using
-  // those decl-only classes as base class.  The idea of the
-  // work-around is to detect those decl-only classes that are
-  // expected to be complete and mark them as complete, with a size of
-  // 1, if their size is still zero.
-  string_classes_map           decl_only_classes_to_force_defined_map_;
   die_tu_map_type              die_tu_map_;
   corpus_sptr                  cur_corpus_;
   translation_unit_sptr        cur_tu_;
@@ -2233,48 +2223,6 @@ public:
   declaration_only_classes() const
   {return decl_only_classes_map_;}
 
-  /// Getter for the map of declaration-only classes that *REALLY* are
-  /// to be resolved to their definition classes (because they are
-  /// actually used in context that require a complete type) by the
-  /// end of the corpus loading.  If they are are not resolved at the
-  /// end of corpus loading then they need to be "forced" to be
-  /// complete by considering that they are a type of size 1 and with
-  /// no data member and no base class.
-  ///
-  /// This is to implement a work-around for DWARF producers that
-  /// forget to provide definitions for classes that are actually
-  /// empty (no data member and no base class), and more importantly,
-  /// are used in contexts that require complete types like a base
-  /// class.
-  ///
-  /// @return a map of string -> vector of classes where the key is
-  /// the fully qualified name of the class and the value is the
-  /// vector of declaration-only class.
-  const string_classes_map&
-  declaration_only_classes_to_force_defined() const
-  {return decl_only_classes_to_force_defined_map_;}
-
-  /// Getter for the map of declaration-only classes that *REALLY* are
-  /// to be resolved to their definition classes (because they are
-  /// actually used in context that require a complete type) by the
-  /// end of the corpus loading.  If they are are not resolved at the
-  /// end of corpus loading then they need to be "forced" to be
-  /// complete by considering that they are a type of size 1 and with
-  /// no data member and no base class.
-  ///
-  /// This is to implement a work-around for DWARF producers that
-  /// forget to provide definitions for classes that are actually
-  /// empty (no data member and no base class), and more importantly,
-  /// are used in contexts that require complete types like a base
-  /// class.
-  ///
-  /// @return a map of string -> vector of classes where the key is
-  /// the fully qualified name of the class and the value is the
-  /// vector of declaration-only class.
-  string_classes_map&
-  declaration_only_classes_to_force_defined()
-  {return decl_only_classes_to_force_defined_map_;}
-
   /// Getter for the map of declaration-only classes that are to be
   /// resolved to their definition classes by the end of the corpus
   /// loading.
@@ -2307,29 +2255,6 @@ public:
       }
   }
 
-  /// If a given class is a declaration-only class that really needs
-  /// to be resolved because it's used in a context that requires a
-  /// complete class, then stash it on the side so that at the end of
-  /// the corpus reading we can force its resolution it to its
-  /// definition.
-  ///
-  /// @param klass the class to consider.
-  void
-  schedule_declaration_only_class_for_forced_resolution(class_decl_sptr& klass)
-  {
-    if (klass->get_is_declaration_only()
-       && klass->get_definition_of_declaration() == 0)
-      {
-       string qn = klass->get_qualified_name();
-       string_classes_map::iterator record =
-         declaration_only_classes_to_force_defined().find(qn);
-       if (record == declaration_only_classes_to_force_defined().end())
-         declaration_only_classes_to_force_defined()[qn].push_back(klass);
-       else
-         record->second.push_back(klass);
-      }
-  }
-
   /// Test if a given declaration-only class has been scheduled for
   /// resolution to a defined class.
   ///
@@ -2397,41 +2322,7 @@ public:
     for (vector<string>::iterator i = resolved_classes.begin();
         i != resolved_classes.end();
         ++i)
-      {
-       declaration_only_classes().erase(*i);
-       declaration_only_classes_to_force_defined().erase(*i);
-      }
-
-    // Now force-resolve the remaining decl-only classes that were not
-    // resolved, but that are used in contexts where they need to be
-    // complete.
-    for (string_classes_map::iterator i =
-          declaration_only_classes_to_force_defined().begin();
-        i != declaration_only_classes_to_force_defined().end();
-        ++i)
-      {
-       // So i->first is the name of the declaration-only type.  i->second
-       // is a vector of instances of the declaration-only type named
-       // i->first.  Each instance was referenced in a context that
-       // needs a complete type.
-       for (classes_type::iterator j = i->second.begin();
-                j != i->second.end();
-                ++j)
-         {
-           if ((*j)->get_is_declaration_only())
-             {
-               // This particular declaration-only type hasn't been
-               // resolved yet.  So force resolve it.
-               assert ((*j)->get_size_in_bits() == 0
-                       && ((*j)->get_definition_of_declaration() == 0));
-
-               (*j)->set_is_declaration_only(false);
-               (*j)->set_size_in_bits(1);
-             }
-         }
-       declaration_only_classes().erase(i->first);
-      }
-    declaration_only_classes_to_force_defined().clear();
+      declaration_only_classes().erase(*i);
   }
 
   /// Return a reference to the vector containing the offsets of the
@@ -6671,14 +6562,7 @@ build_class_type_and_add_to_ir(read_context&     ctxt,
                                               : -1,
                                               is_virt));
              if (b->get_is_declaration_only())
-               {
-                 assert(ctxt.is_decl_only_class_scheduled_for_resolution(b));
-                 // In case the declared class "b" is not defined
-                 // until the end of the current DSO (some DWARF
-                 // producers emit debug info like that, yes) make
-                 // sure to force it to be defined somehow.
-                 ctxt.schedule_declaration_only_class_for_forced_resolution(b);
-               }
+               assert(ctxt.is_decl_only_class_scheduled_for_resolution(b));
              result->add_base_specifier(base);
            }
          // Handle data members.
@@ -6738,8 +6622,6 @@ build_class_type_and_add_to_ir(read_context&      ctxt,
          // Handle member functions;
          else if (tag == DW_TAG_subprogram)
            {
-             result->set_is_declaration_only(false);
-
              decl_base_sptr r = build_ir_node_from_die(ctxt, &child,
                                                        is_in_alt_di,
                                                        result.get(),
@@ -6752,6 +6634,9 @@ build_class_type_and_add_to_ir(read_context&      ctxt,
              assert(f);
 
              finish_member_function_reading(&child, f, result);
+             if (is_member_function(f)
+                 && get_member_function_is_virtual(f))
+               result->set_is_declaration_only(false);
 
              ctxt.associate_die_to_decl(dwarf_dieoffset(&child),
                                         is_in_alt_di, f);