Bug 25128 - Handle decl-only classes that differ only in size

Because DWARF sometimes emit decl-only classes (real one, with no
members) with a size property, and the rest of the time, would emit
the same decl-only class without a size property, comparing the two
might yield some false positives.

This patch handles those beasts when comparing classes.

	* include/abg-comp-filter.h (is_decl_only_class_with_size_change):
	Declare an overload.
	* include/abg-fwd.h (look_through_decl_only_class): Declare an
	overload.
	* src/abg-comp-filter.cc (is_decl_only_class_with_size_change):
	Define an overload that takes class_or_union& type. Re-write the
	previous overload in terms of this new one.
	* src/abg-ir.cc (look_through_decl_only_class): Define a new
	overload that takes a class_or_union&.  Rewrite the previous
	overload in terms of this one.
	(equals): In the overload for class_or_union&, use
	is_decl_only_class_with_size_change to detect cases of decl-only
	classes that differ only by their size attribute and avoid
	comparing them.
	* tests/data/test-annotate/test21-pr19092.so.abi: Adjust.
	* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
	* tests/data/test-diff-filter/test41-report-0.txt: Likewise.

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

diff --git a/include/abg-comp-filter.h b/include/abg-comp-filter.h
index ca69346..752047b 100644 (file)
--- a/include/abg-comp-filter.h
+++ b/include/abg-comp-filter.h
@@ -54,6 +54,10 @@ bool
 has_virtual_mem_fn_change(const function_decl_diff* diff);
 
 bool
+is_decl_only_class_with_size_change(const class_or_union& first,
+                                   const class_or_union& second);
+
+bool
 is_decl_only_class_with_size_change(const class_or_union_sptr& first,
                                    const class_or_union_sptr& second);
 
@@ -63,6 +67,7 @@ is_decl_only_class_with_size_change(const diff *diff);
 bool
 has_class_decl_only_def_change(const class_or_union_sptr& first,
                               const class_or_union_sptr& second);
+
 bool
 has_class_decl_only_def_change(const diff *diff);
 

diff --git a/include/abg-fwd.h b/include/abg-fwd.h
index 24cfe94..823f0b9 100644 (file)
--- a/include/abg-fwd.h
+++ b/include/abg-fwd.h
@@ -511,6 +511,9 @@ method_type*
 is_method_type(type_or_decl_base*);
 
 class_or_union_sptr
+look_through_decl_only_class(const class_or_union&);
+
+class_or_union_sptr
 look_through_decl_only_class(class_or_union_sptr);
 
 var_decl*

diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc
index 3f94e5e..8ca40d6 100644 (file)
--- a/src/abg-comp-filter.cc
+++ b/src/abg-comp-filter.cc
@@ -838,6 +838,36 @@ base_classes_added_or_removed(const diff* diff)
 /// @return true if the two classes are decl-only and differ in their
 /// size.
 bool
+is_decl_only_class_with_size_change(const class_or_union& first,
+                                   const class_or_union& second)
+{
+  if (first.get_qualified_name() != second.get_qualified_name())
+    return false;
+
+  if (!first.get_is_declaration_only() || !second.get_is_declaration_only())
+    return false;
+
+  bool f_is_empty = first.get_data_members().empty();
+  bool s_is_empty = second.get_data_members().empty();
+
+  return f_is_empty && s_is_empty;
+}
+
+/// Test if two classes that are decl-only (have the decl-only flag
+/// and carry no data members) but are different just by their size.
+///
+/// In some weird DWARF representation, it happens that a decl-only
+/// class (with no data member) actually carries a non-zero size.
+/// That shouldn't happen, but hey, we need to deal with real life.
+/// So we need to detect that case first.
+///
+/// @param first the first class or union to consider.
+///
+/// @param seconf the second class or union to consider.
+///
+/// @return true if the two classes are decl-only and differ in their
+/// size.
+bool
 is_decl_only_class_with_size_change(const class_or_union_sptr& first,
                                    const class_or_union_sptr& second)
 {
@@ -849,16 +879,7 @@ is_decl_only_class_with_size_change(const class_or_union_sptr& first,
   class_or_union_sptr s =
     look_through_decl_only_class(second);
 
-  if (f->get_qualified_name() != s->get_qualified_name())
-    return false;
-
-  if (!f->get_is_declaration_only() || !s->get_is_declaration_only())
-    return false;
-
-  bool f_is_empty = f->get_data_members().empty();
-  bool s_is_empty = s->get_data_members().empty();
-
-  return f_is_empty && s_is_empty;
+  return is_decl_only_class_with_size_change(*f, *s);
 }
 
 /// Test if a diff node is for two classes that are decl-only (have

diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index ebe2e3f..a6ae7ae 100644 (file)
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -46,6 +46,7 @@ ABG_END_EXPORT_DECLARATIONS
 // </headers defining libabigail's API>
 
 #include "abg-tools-utils.h"
+#include "abg-comp-filter.h"
 #include "abg-ir-priv.h"
 
 namespace
@@ -7844,12 +7845,16 @@ is_method_type(type_or_decl_base* t)
 /// If a class (or union) is a decl-only class, get its definition.
 /// Otherwise, just return the initial class.
 ///
-/// @param klass the class (or union) to consider.
+/// @param the_klass the class (or union) to consider.
 ///
 /// @return either the definition of the class, or the class itself.
 class_or_union_sptr
-look_through_decl_only_class(class_or_union_sptr klass)
+look_through_decl_only_class(const class_or_union& the_class)
 {
+  class_or_union_sptr klass;
+  if (the_class.get_is_declaration_only())
+    klass = the_class.get_definition_of_declaration();
+
   if (!klass)
     return klass;
 
@@ -7862,6 +7867,25 @@ look_through_decl_only_class(class_or_union_sptr klass)
   return klass;
 }
 
+/// If a class (or union) is a decl-only class, get its definition.
+/// Otherwise, just return the initial class.
+///
+/// @param klass the class (or union) to consider.
+///
+/// @return either the definition of the class, or the class itself.
+class_or_union_sptr
+look_through_decl_only_class(class_or_union_sptr klass)
+{
+  if (!klass)
+    return klass;
+
+  class_or_union_sptr result = look_through_decl_only_class(*klass);
+  if (!result)
+    result = klass;
+
+  return result;
+}
+
 /// Tests if a declaration is a variable declaration.
 ///
 /// @param decl the decl to test.
@@ -18637,6 +18661,17 @@ equals(const class_or_union& l, const class_or_union& r, change_kind* k)
 
       if (!def1 || !def2)
        {
+         if (!l.get_is_anonymous()
+             && !r.get_is_anonymous()
+             && l_is_decl_only && r_is_decl_only
+             && comparison::filtering::is_decl_only_class_with_size_change(l, r))
+           // The two decl-only classes differ from their size.  A
+           // true decl-only class should not have a size property to
+           // begin with.  This comes from a DWARF oddity and can
+           // results in a false positive, so let's not consider that
+           // change.
+           return true;
+
          if (l.get_environment()->decl_only_class_equals_definition()
              && !l.get_is_anonymous()
              && !r.get_is_anonymous())

diff --git a/tests/data/test-diff-filter/test41-report-0.txt b/tests/data/test-diff-filter/test41-report-0.txt
index edeb24e..3a87adb 100644 (file)
--- a/tests/data/test-diff-filter/test41-report-0.txt
+++ b/tests/data/test-diff-filter/test41-report-0.txt
@@ -48,10 +48,6 @@ Variable symbols changes summary: 0 Removed, 0 Added variable symbol not referen
               class std::tr1::__shared_ptr<abigail::ir::type_base, __gnu_cxx::_Lock_policy::_S_atomic> at shared_ptr.h:539:1
 
   [C]'method bool abigail::xml_writer::write_context::type_ptr_cmp::operator()(const abigail::ir::type_base*, const abigail::ir::type_base*) const' at abg-writer.cc:359:1 has some indirect sub-type changes:
-    parameter 1 of type 'const abigail::ir::type_base*' has sub-type changes:
-      in pointed to type 'const abigail::ir::type_base':
-        in unqualified underlying type 'class abigail::ir::type_base':
-          type size changed from 0 to 384 (in bits)