Bug 22913 - Correctly de-duplicate pointers to anonymous structs inside a given

During type DIE canonicalization, libabigail performs an optimization
while comparing two types defined in the same translation unit.  That
is, inside a given translation unit two pointers that point to a type
named T (that is, two T*) are considered equal.  They are considered
equal without having to structurally compare the two types named T.

This generally makes sense, because if two types of the same kind,
defined in the same translation unit, have the same name then we can
safely conclude that they are actually the same type.  Unless the two
T are anonymous structs.

If the two T are anonymous structs defined in the same translation
unit, we really need to compare them structurally to know if they are
equal or not.

This is what this patch does.

	* src/abg-dwarf-reader.cc
	(pointer_or_qual_die_of_anonymous_class_type)
	(die_is_qualified_type): Define new functions.
	(compare_dies): If pointers, reference or qualified type have an
	anonymous struct as their underlying type, then we need to
	structurally compare the underlying anonymous struct.
	* tests/data/test-diff-dwarf/libtest43-PR22913-v{0,1}.so: New
	binary test input files.
	* tests/data/test-diff-dwarf/test43-PR22913-report-0.txt: New
	reference output of the comparison of the two binaries above.
	* tests/data/test-diff-dwarf/test43-PR22913-v{0,1}.c: Source code
	of the binaries above.
	* tests/test-diff-dwarf.cc (in_out_specs): Make the test harness
	compare the two binaries above.
	* tests/data/Makefile.am: Add the new test files above to the
	source distribution.

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

diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 5b1f932ad0ca8072274459ee001b87c96f95d6ff..c7df7230345e8e39ac0003266933a75d3de99637 100644 (file)
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -323,6 +323,9 @@ die_is_void_type(Dwarf_Die* die);
 static bool
 die_is_pointer_type(Dwarf_Die* die);
 
+static bool
+pointer_or_qual_die_of_anonymous_class_type(Dwarf_Die* die);
+
 static bool
 die_is_reference_type(Dwarf_Die* die);
 
@@ -332,6 +335,9 @@ die_is_pointer_or_reference_type(Dwarf_Die* die);
 static bool
 die_is_class_type(Dwarf_Die* die);
 
+static bool
+die_is_qualified_type(Dwarf_Die* die);
+
 static bool
 die_has_object_pointer(Dwarf_Die* die,
                       Dwarf_Die& object_pointer);
@@ -8618,6 +8624,32 @@ die_is_pointer_type(Dwarf_Die* die)
   return false;
 }
 
+/// Test if a DIE is for a pointer, reference or qualified type to
+/// anonymous class or struct.
+///
+/// @param die the DIE to consider.
+///
+/// @return true iff @p is for a pointer, reference or qualified type
+/// to anonymous class or struct.
+static bool
+pointer_or_qual_die_of_anonymous_class_type(Dwarf_Die* die)
+{
+  if (!die_is_pointer_or_reference_type(die)
+      && !die_is_qualified_type(die))
+    return false;
+
+  Dwarf_Die underlying_type_die;
+  if (!die_die_attribute(die, DW_AT_type, underlying_type_die))
+    return false;
+
+  if (!die_is_class_type(&underlying_type_die))
+    return false;
+
+  string name = die_name(&underlying_type_die);
+
+  return name.empty();
+}
+
 /// Test if a DIE represents a reference type.
 ///
 /// @param die the die to consider.
@@ -8681,6 +8713,23 @@ die_is_class_type(Dwarf_Die* die)
   return false;
 }
 
+/// Test if a DIE is for a qualified type.
+///
+/// @param die the DIE to consider.
+///
+/// @return true iff @p die is for a qualified type.
+static bool
+die_is_qualified_type(Dwarf_Die* die)
+{
+    int tag = dwarf_tag(die);
+    if (tag == DW_TAG_const_type
+       || tag == DW_TAG_volatile_type
+       || tag == DW_TAG_restrict_type)
+      return true;
+
+    return false;
+}
+
 /// Test if a DIE for a function pointer or member function has an
 /// DW_AT_object_pointer attribute.
 ///
@@ -11096,14 +11145,21 @@ compare_dies(const read_context& ctxt, Dwarf_Die *l, Dwarf_Die *r,
        bool from_the_same_tu = false;
        if (!compare_as_type_dies(l, r))
          result = false;
-       else if (compare_dies_cu_decl_file(l, r, from_the_same_tu)
+       else if (!pointer_or_qual_die_of_anonymous_class_type(l)
+                && compare_dies_cu_decl_file(l, r, from_the_same_tu)
                 && from_the_same_tu)
          // These two typedefs, pointer, reference, or qualified
          // types have the same name and are defined in the same TU.
          // They thus ought to be the same.
+         //
+         // Note that pointers, reference or qualified types to
+         // anonymous types are not taking into account here because
+         // those always need to be structurally compared.
          result = true;
        else
          {
+           // No fancy optimization in this case.  We need to
+           // structurally compare the two DIEs.
            Dwarf_Die lu_type_die, ru_type_die;
            bool lu_is_void, ru_is_void;
 

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 132c0b76eeec798f1a080b2ab3b6b030ca9cb3f7..4a1eca5f3162a0dd0ca8218a20c4b2acef263c44 100644 (file)
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -308,6 +308,11 @@ test-diff-dwarf/test41-PR20476-hidden-report-0.txt \
 test-diff-dwarf/test42-PR21296-libgcc.so \
 test-diff-dwarf/test42-PR21296-libclang.so \
 test-diff-dwarf/test42-PR21296-clanggcc-report0.txt \
+test-diff-dwarf/libtest43-PR22913-v0.so \
+test-diff-dwarf/libtest43-PR22913-v1.so \
+test-diff-dwarf/test43-PR22913-report-0.txt \
+test-diff-dwarf/test43-PR22913-v0.c \
+test-diff-dwarf/test43-PR22913-v1.c \
 \
 test-read-dwarf/test0                  \
 test-read-dwarf/test0.abi                      \

diff --git a/tests/data/test-diff-dwarf/libtest43-PR22913-v0.so b/tests/data/test-diff-dwarf/libtest43-PR22913-v0.so
new file mode 100755 (executable)
index 0000000..cd4f729
Binary files /dev/null and b/tests/data/test-diff-dwarf/libtest43-PR22913-v0.so differ

diff --git a/tests/data/test-diff-dwarf/libtest43-PR22913-v1.so b/tests/data/test-diff-dwarf/libtest43-PR22913-v1.so
new file mode 100755 (executable)
index 0000000..15d3959
Binary files /dev/null and b/tests/data/test-diff-dwarf/libtest43-PR22913-v1.so differ

diff --git a/tests/data/test-diff-dwarf/test43-PR22913-report-0.txt b/tests/data/test-diff-dwarf/test43-PR22913-report-0.txt
new file mode 100644 (file)
index 0000000..3bc46b9
--- /dev/null
+++ b/tests/data/test-diff-dwarf/test43-PR22913-report-0.txt
@@ -0,0 +1,15 @@
+Functions changes summary: 0 Removed, 1 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+1 function with some indirect sub-type change:
+
+  [C]'function char f1(Struct1Ptr)' has some indirect sub-type changes:
+    parameter 1 of type 'typedef Struct1Ptr' has sub-type changes:
+      underlying type '__anonymous_struct__*' changed:
+        in pointed to type 'struct __anonymous_struct__':
+          type size changed from 16 to 8 bits
+          1 data member deletion:
+            'char __anonymous_struct__::m2', at offset 8 (in bits)
+
+
+

diff --git a/tests/data/test-diff-dwarf/test43-PR22913-v0.c b/tests/data/test-diff-dwarf/test43-PR22913-v0.c
new file mode 100644 (file)
index 0000000..1eabc45
--- /dev/null
+++ b/tests/data/test-diff-dwarf/test43-PR22913-v0.c
@@ -0,0 +1,18 @@
+typedef struct
+{
+  char m0;
+} * Struct0Ptr;
+
+char
+f0(Struct0Ptr s)
+{return s->m0;}
+
+typedef struct
+{
+  char m1;
+  char m2;
+} * Struct1Ptr;
+
+char
+f1(Struct1Ptr s)
+{return s->m1;}

diff --git a/tests/data/test-diff-dwarf/test43-PR22913-v1.c b/tests/data/test-diff-dwarf/test43-PR22913-v1.c
new file mode 100644 (file)
index 0000000..2f1c383
--- /dev/null
+++ b/tests/data/test-diff-dwarf/test43-PR22913-v1.c
@@ -0,0 +1,17 @@
+typedef struct
+{
+  char m0;
+} * Struct0Ptr;
+
+char
+f0(Struct0Ptr s)
+{return s->m0;}
+
+typedef struct
+{
+  char m1;
+} * Struct1Ptr;
+
+char
+f1(Struct1Ptr s)
+{return s->m1;}

diff --git a/tests/test-diff-dwarf.cc b/tests/test-diff-dwarf.cc
index ebe0c64f4e8cd22d66c9b98d6bbf611c6fffe3c8..56b442c603372e5f3567e484c446e4554bcf03e1 100644 (file)
--- a/tests/test-diff-dwarf.cc
+++ b/tests/test-diff-dwarf.cc
@@ -326,6 +326,12 @@ InOutSpec in_out_specs[] =
     "data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt",
     "output/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt"
   },
+  {
+    "data/test-diff-dwarf/libtest43-PR22913-v0.so",
+    "data/test-diff-dwarf/libtest43-PR22913-v1.so",
+    "data/test-diff-dwarf/test43-PR22913-report-0.txt",
+    "output/test-diff-dwarf/test43-PR22913-report-0.txt"
+  },
   // This should be the last entry
   {NULL, NULL, NULL, NULL}
 };