Hello, There are several self comparison issues uncovered by comparing the file test-PR27995.abi (provided in the bug report) against itself. This patch address them all as well as the regressions induced on some of the test suite and then and updates the other reference test suite output that need it. In the equals overload for decl_base, we compare the non-internal versions of qualified decl names. For var_decls of anonymous class or union types, the non-internal version is the flat-representation of the type. Thus a benign change in a data member name of the anonymous type might cause the equals function to consider the var_decls to be wrongly different. The internal version of the qualified decl name should return a name that is stable for types, irrespective of these benign variations. The patch thus makes the equals overload for decl_base to compare internal versions of qualified decl names instead. The patch ensures that enum_type_decl::get_pretty_representation return and internal pretty representation that is "stable" for anonymous types. Basically, all anonymous enums will have the same of name that looks like "__anonymous_enum__". This is to ensure two things: first, that all anonymous enums are compared against each other during type canonicalization, ensuring that when two anonymous enums are canonically different, it really is because of changes in their enumerators or basic type, not because of anything having to do with their artificial names. Second, that in the equals overload for decl_base, their internal qualified name always compare equal. This nullifies the risk of having anonymous types compare different because of their (non existent) name. This is because libabigail's dwarf reader assigns artificial unique names to anonymous types, so we don't want to use these during actual type comparison. We do something similar for class_decl::get_pretty_representation and union_decl::get_pretty_representation where the pretty internal representation for class/union decl would now be __anonymous_{struct,union}__. The patch scouts the uses of get_pretty_representation() to make sure to use avoid using the internal-form of the pretty representations when it's not warranted. It also updates the doxygen comments of the overloads of that function. In the abixml reader, we were wrongly canonicalizing array types early, even before they were fully created. The was leading to spurious type chances down the road. The patch also fixes the caching of the name of function types by making it consistent with caching of the names of the other types of the system. The idea is that we don't cache the name of a function type until it's canonicalize. This is because the type might be edited during its pre-canonicalization life time; and that editing might change its name. However once the type is canonicalized, it becomes immutable. At that point we can cache its name, for performance purposes. Note that we need to do that both for the "internal version" of the type name (used for canonilization purposes) and the "non-internal version" one, which is used for other purposes. This caching scheme wasn't respected for function types, so we were caching a potentially wrong name for the type after its canonicalization. Last but not least, there is a problem that makes canonical type comparison different from structural type comparison. Let's consider these two declarations: typedef int FirstInt; typedef int SecondInt; Now, consider these two pointer types: FirstInt* and SecondInt*; These two pointer types are canonically different because they have different type names. This is because during type canonicalization, types with the same "pretty representation" are compared against each other. So types with different type names will certainly have different pretty representations and won't be compared; they are thus going to have different canonical types. However, FirstInt* and SecondInt* do compare equal, structurally, because the equals overload for pointer_type_def compares the pointed-to types of pointers by peeling off typedefs. So, here, as both pointed-to types are 'int' when the typedefs are peeled off, the two pointers structurally compare equal. This discrepancy between structural and canonical equality introduces subtle and spurious type changes depending on the order in which types are canonicalized. For instance: struct {FirstInt* m0;}; /* First type. */ struct {SecondInt* m0;}; /* Second type. */ If FirstInt* and SecondInt* are canonicalized before their containing anonymous types, then the two anonymous types will compare different (because FirstInt* and SecondInt* compare different) and have different canonical types. If, however, the anonymous types are canonicalized before FirstInt* and SecondInt*, then will compare equal because FirstInt* and SecondInt* are structurally equivalent. FirstInt* and SecondInt* will be canonicalized latter and have different canonical types (because they have different type names) despite being structurally equivalent. The change in the order of canonicalization can happen when canonicalizing types from a corpus coming from DWARF as opposed to canonicalizing types from a corpus coming from abixml. The patch fixes this discrepancy by not peeling off typedefs from the pointed-to types when comparing pointers. Note that this makes us regress on bug https://sourceware.org/bugzilla/show_bug.cgi?id=27236, where the typedef peeling was introduced. In hindsight, introducing that typedef peeling was a mistake. I'll try to address that bug again in a subsequent patch. * doc/manuals/abidiff.rst: Add documentation for the --debug option. * src/abg-ir.cc (equals): In the overload for decl_base consider the internal version of qualified decl name. In the overload for pointer_type_def do not peel typedefs off from the compared pointed-to types. In the overload for typedef_decl compare the typedef as a decl as well. In the overload for var_decl, compare variables that have the same ELF symbols without taking into account their qualified name, rather than their name. Stop comparing data member without considering their names. In the overload for class_or_union, when a decl-only class that is ODR-relevant is compared against another type, assume that equality if names are equal. This is useful in environments where some TUs are ODR-relevant and others aren't. (*::get_pretty_representation): Update doxygen comments. (enum_type_decl::get_pretty_representation): Return an internal pretty representation that is stable across all anonymous enums. (var_decl::get_anon_dm_reliable_name): Use the non-internal pretty representation for anonymous data members. (function_type::priv::temp_internal_cached_name_): New data member. (function_type::get_cached_name): Cache the internal name after the function type is canonicalized. Make sure internal name and non-internal name are cached separately. (class_or_union::find_anonymous_data_member): Look for the anonymous data member by looking at its non-internal name. ({class, union}_decl::get_pretty_representation): Use something like "class __anonymous_{union,struct}__" for all anonymous classes, so that they can all be compared against each other during type canonicalization. (type_has_sub_type_changes): Use non-internal pretty representation. (hash_type_or_decl, function_decl_is_less_than:): Use internal pretty representation for comparison here. * src/abg-reader.cc (read_context::maybe_canonicalize_type): Don't early canonicalize array types. * src/abg-writer.cc (annotate): Use non-internal pretty representation. * tests/data/test-diff-filter/test-PR27995-report-0.txt: New reference report. * tests/data/test-diff-filter/test-PR27995.abi: New test input abixml file. * tests/data/Makefile.am: Add test-PR27995.abi, test-PR27995-report-0.txt to the source distribution. * tests/data/test-annotate/libtest23.so.abi: Adjust. * tests/data/test-diff-dwarf/test6-report.txt: Adjust. * tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt: Adjust. * tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt: Adjust. * tests/data/test-diff-filter/test41-report-0.txt: Adjust. * tests/data/test-diff-filter/test43-decl-only-def-change-leaf-report-0.txt: Adjust. * tests/data/test-diff-filter/test8-report.txt: Adjust. * tests/data/test-diff-pkg/libICE-1.0.6-1.el6.x86_64.rpm--libICE-1.0.9-2.el7.x86_64.rpm-report-0.txt: Adjust. * tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-0.txt: Adjust. * tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt: Adjust. * tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt: Adjust. * tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt: Adjust. * tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt: Adjust. * tests/data/test-diff-suppr/test39-opaque-type-report-0.txt: Adjust. * tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: Adjust. * tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust. * tests/data/test-read-dwarf/libtest23.so.abi: Adjust. * tests/data/test-read-dwarf/test-libandroid.so.abi: Adjust. * tests/data/test-read-dwarf/test11-pr18828.so.abi: Adjust. * tests/data/test-read-dwarf/test12-pr18844.so.abi: Adjust. * tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust. * tests/test-diff-filter.cc (in_out_specs): Add the test-PR27995.abi to the test harness. * tools/abidiff.cc (options::do_debug): New data member. (options::options): Initialize it. (parse_command_line): Parse --debug. (main): Activate self comparison debug if the user provided --debug. Signed-off-by: Dodji Seketeli Applied to master. --- doc/manuals/abidiff.rst | 9 + src/abg-ir.cc | 332 +- src/abg-reader.cc | 1 + src/abg-writer.cc | 2 +- tests/data/Makefile.am | 2 + tests/data/test-annotate/libtest23.so.abi | 24 +- tests/data/test-diff-dwarf/test6-report.txt | 12 + .../test-PR27995-report-0.txt | 0 tests/data/test-diff-filter/test-PR27995.abi | 290129 +++++++++++++++ .../test31-pr18535-libstdc++-report-0.txt | 59 +- .../test31-pr18535-libstdc++-report-1.txt | 59 +- .../data/test-diff-filter/test41-report-0.txt | 2 +- ...t43-decl-only-def-change-leaf-report-0.txt | 5 - tests/data/test-diff-filter/test8-report.txt | 3 + ...libICE-1.0.9-2.el7.x86_64.rpm-report-0.txt | 2 +- ...l7.x86_64-0.12.8-1.el7.x86_64-report-0.txt | 10 +- ...l7.x86_64-0.12.8-1.el7.x86_64-report-2.txt | 17 +- ...l7.x86_64-0.12.8-1.el7.x86_64-report-3.txt | 4 +- ...bb-4.3-3.20141204.fc23.x86_64-report-0.txt | 2 +- ...bb-4.3-3.20141204.fc23.x86_64-report-1.txt | 2 +- .../test39-opaque-type-report-0.txt | 9 +- .../PR22015-libboost_iostreams.so.abi | 2 +- .../test-read-dwarf/PR22122-libftdc.so.abi | 206 +- tests/data/test-read-dwarf/libtest23.so.abi | 22 +- .../test-read-dwarf/test-libandroid.so.abi | 266 +- .../test-read-dwarf/test11-pr18828.so.abi | 250 +- .../test-read-dwarf/test12-pr18844.so.abi | 2 +- .../test9-pr18818-clang.so.abi | 2 +- tests/test-diff-filter.cc | 9 +- tools/abidiff.cc | 19 + 30 files changed, 290891 insertions(+), 572 deletions(-) create mode 100644 tests/data/test-diff-filter/test-PR27995-report-0.txt create mode 100644 tests/data/test-diff-filter/test-PR27995.abi The patch is big so I am attaching it gziped.