From: Dodji Seketeli Date: Fri, 2 Mar 2018 14:47:15 +0000 (+0100) Subject: Bug 22913 - Correctly de-duplicate pointers to anonymous structs inside a given X-Git-Tag: libabigail-1.2~6 X-Git-Url: https://sourceware.org/git/?a=commitdiff_plain;h=30314e3a62d15055a4fb484c690840318efcf946;p=libabigail.git 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 --- diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc index 5b1f932a..c7df7230 100644 --- 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 132c0b76..4a1eca5f 100644 --- 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 index 00000000..cd4f7290 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 index 00000000..15d3959c 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 index 00000000..3bc46b9b --- /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 index 00000000..1eabc45b --- /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 index 00000000..2f1c383a --- /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 ebe0c64f..56b442c6 100644 --- 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} };