Bug 18818 - abidw aborts on a class with a non-complete base class

On some binaries with debug info emitted by "Ubuntu clang version
3.6.0-2ubuntu1" and "GNU C++ 4.9.2" (as the value of the
DW_AT_producer property), it seems some classes can have a base class
that is not complete.  E.g, the debug info (that I have extracted
using the command eu-readelf --debug-dump=info
<the-binary-attached-to-the-bug>) has these relevant pieces:

    [...]

     [  5ff7]        class_type
		     containing_type      (ref4) [  7485]
		     name                 (strp) "system_error"
		     byte_size            (data1) 40
		     decl_file            (data1) 46
		     decl_line            (data1) 22
     [  6003]          inheritance
		       type                 (ref4) [  7480]
    [...]

Here, we are looking at the type system_error (actually
boost::system::system_error) that inherits the type which DIE is
referred to as offset '7480'.

Then the definition of the DIE at offset 7480 is:

    [...]

     [  7480]      class_type
		   name                 (strp) "runtime_error"
		   declaration          (flag_present)
     [  7485]      class_type
		   name                 (strp) "exception"
		   declaration          (flag_present)
    [...]

You can see that the type "runtime_error" (actually
std::runtime_error) has the flag DW_AT_declaration set, marking it as
a declaration (with no definition yet).  And no other DIE in the same
translation unit
(src/third_party/boost-1.56.0/libs/filesystem/src/codecvt_error_category.cpp)
or in the same DSO provides the definition for that declaration.

I believe this is ill-formed.  A base class should be defined and have
a layout completed expressed and accessible from the translation unit
it's used in.

The patch I am proposing detects that the base class is still
incomplete when we finish loading the current binary.  In that case,
the base class is made complete with a size of 1.  Meaning it's an
empty class (with no data member and no base class).  This works as a
viable work-around *if* the producer only omitted definitions for
empty classes.  We'll need to fix the producers eventually.

	* src/abg-dwarf-reader.cc
	(read_context::decl_only_classes_to_force_defined_map_): New data
	member.
	(read_context::declaration_only_classes_to_force_defined): New
	accessors.
	(read_context::schedule_declaration_only_class_for_forced_resolution):
	New member function.
	(build_class_type_and_add_to_ir): If a base class is a
	declaration-only class then mark it as needing to be force-defined
	*if* it's still not defined at the end of the abi corpus loading.
	(read_context::resolve_declaration_only_classes): If
	declaration-only classes that need to force-defined are present
	and not defined (when we reach the end of the ABI corpus) then
	force-define them as empty classes.
	* tests/data/test-read-dwarf/test10-pr18818-gcc.so: New test
	binary input file.  This comes from a user binary submitted to bug
	https://sourceware.org/bugzilla/show_bug.cgi?id=18818.  The
	original URL to the binary is
	https://sourceware.org/bugzilla/attachment.cgi?id=8518.
	* tests/data/test-read-dwarf/test9-pr18818-clang.so: New binary
	input file.  This comes from the same bug report as above.  The
	original URL to the binary is
	https://sourceware.org/bugzilla/attachment.cgi?id=8511.
	* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: New
	reference output file.
	* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
	* tests/data/Makefile.am: Add the new files above to the source
	distribution.
	* tests/test-read-dwarf.cc (in_out_specs): Add the test inputs
	above the set of tests input this harness has to run over.

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

src/abg-dwarf-reader.cc

@@ -1744,6 +1744,16 @@ 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_;
@@ -2205,7 +2215,49 @@ public:
   declaration_only_classes() const
   {return decl_only_classes_map_;}
 
-    /// Getter for the map of declaration-only classes that are to be
+  /// 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.
   ///
@@ -2237,6 +2289,29 @@ 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.
   ///
@@ -2304,7 +2379,32 @@ public:
     for (vector<string>::iterator i = resolved_classes.begin();
 	 i != resolved_classes.end();
 	 ++i)
-      declaration_only_classes().erase(*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)
+      {
+	for (classes_type::iterator j = i->second.begin();
+		 j != i->second.end();
+		 ++j)
+	  {
+	    assert ((*j)->get_is_declaration_only()
+		    && (*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();
   }
 
   /// Return a reference to the vector containing the offsets of the
@@ -6424,7 +6524,14 @@ 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));
+		{
+		  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);
+		}
 	      result->add_base_specifier(base);
 	    }
 	  // Handle data members.

tests/data/Makefile.am

@@ -250,6 +250,10 @@ test-read-dwarf/test7.so.abi		\
 test-read-dwarf/test8-qualified-this-pointer.cc		\
 test-read-dwarf/test8-qualified-this-pointer.so		\
 test-read-dwarf/test8-qualified-this-pointer.so.abi		\
+test-read-dwarf/test9-pr18818-clang.so  \
+test-read-dwarf/test10-pr18818-gcc.so \
+test-read-dwarf/test9-pr18818-clang.so.abi  \
+test-read-dwarf/test10-pr18818-gcc.so.abi \
 \
 test-diff-filter/test0-v0.cc		\
 test-diff-filter/test0-v1.cc		\

tests/test-read-dwarf.cc

@@ -95,6 +95,16 @@ InOutSpec in_out_specs[] =
     "data/test-read-dwarf/test8-qualified-this-pointer.so.abi",
     "output/test-read-dwarf/test8-qualified-this-pointer.so.abi"
   },
+  {
+    "data/test-read-dwarf/test9-pr18818-clang.so",
+    "data/test-read-dwarf/test9-pr18818-clang.so.abi",
+    "output/test-read-dwarf/test9-pr18818-clang.so.abi",
+  },
+  {
+    "data/test-read-dwarf/test10-pr18818-gcc.so",
+    "data/test-read-dwarf/test10-pr18818-gcc.so.abi",
+    "output/test-read-dwarf/test10-pr18818-gcc.so.abi",
+  },
   // This should be the last entry.
   {NULL, NULL, NULL}
 };