public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
* [PATCH, applied] Canonicalize DIEs w/o assuming ODR & handle typedefs transparently
@ 2022-06-20 15:35 Dodji Seketeli
  0 siblings, 0 replies; only message in thread
From: Dodji Seketeli @ 2022-06-20 15:35 UTC (permalink / raw)
  To: libabigail

Hello,

When canonicalizing DIEs, if two type DIEs have the same textual
representation and we are looking at a C++ DIE, then we assume the two
DIEs are equivalent because of the "One Definition Rule" of C++.

There are binaries however where there seem to be type DIEs that are
slightly different and yet with the same textual representation.  It
seems that this leads to some heisenbugs depending on which of the two
DIEs is "kept".  That order would then depend on whatever the linker's
output is at a particular moment when linking the binary.  Those
slight differences (i.e, modulo typedefs, for instance) are a pain to
reproduce across binaries that are re-linked.  So I am removing this
optimization.  As it doesn't seem to be so useful anymore, given all
the areas of the pipeline that go improved recently, speed-wise.

After doing that, it appeared the comparison engine was seeing more
DIE kinds.  So rather than aborting when seeing an unexpected DIE
kind, we now assume the two DIEs we are seeing are different.  They
are thus not canonicalized.  That's not a problem because the IR level
type canonicalizer will pick that up and fully canonicalize the type
at that level.

So as that is in place, it appeared we were doing canonical DIE
propagation even in cases where we should not.  Namely, when a subset
of the stack of aggregates being compared depends on a recursive (or
redundant) sub-type, no canonical type DIE propagation should be
performed.  The patch thus detects that state of things and disables
the canonical type DIE propagation in that case.

Once this change in place, as the DIE canonicalizer started to look
into more types, the IR started to get more types that were deemed
equal to their decl-only counterpart before, and so were represented
those decl-only counterpart by virtue of the ODR.

That uncovered the long standing issue described below.

Canonicalizing typedefs is done today by considering that two typedefs
that have different names are different even if they have the same
underlying types.

That means we can have two types T and T' (in a binary) that are
equivalent modulo a sub-type that is a typedef with different names in
T and T', yet with equivalent underlying types.  Today, libabigail
would consider T and T' different, even though they are equivalent
from an ABI standpoint.

This can lead to spurious changes that we have to handle later by
adding passes that would suppress those spurious changes.

This patch thus changes the structural comparison for typedefs by not
taking the typedef name into account anymore.  Also, typedefs don't
carry canonical types anymore.  That way, to compare typedefs,
libabigail always look into their underlying types.

To adapt the precision of the abixml output, the patch now uses a
special set (to track emitted non-canonicalized types) and a special
map (to track type ID for non-canonicalized types) as those can't be
put in the classical maps that track types using their canonical
types.

	* include/abg-fwd.h (peel_typedef_pointer_or_reference_type):
	Declare new function.
	* src/abg-dwarf-reader.cc (read_context::{compute_canonical_die,
	get_canonical_die, get_or_compute_canonical_die}): Don't
	special-case ODR-relevant cases.
	(is_canon_type_to_be_propagated_tag): Rename
	is_canonicalizeable_type_tag into this.
	(erase_offset_pair): Make this return a bool, not void.
	(have_offset_pair_in_common, try_canonical_die_comparison)
	(notify_die_comparison_failed): Define new static functions.
	(ABG_RETURN, ABG_RETURN_FALSE): Define new macros.
	(compare_dies): Handle DW_TAG_{class,unspecified}_type DIES.  Also
	if a comparing a DIE kind is not supported, assume the comparison
	yields "false" for the purpose of canonicalization.  Add a new
	parameter for redundant aggregates being compared.  Use the
	redundant aggregate set to detect if a redundant aggregate has
	been detected and is still being "explored"; if that's the case,
	then canonical DIE propagation is de-activated.  This might incur
	a performance hit.  Use the new ABG_RETURN and ABG_RETURN_FALSE
	macros.  Use the new try_canonical_die_comparison to compare
	canonical DIEs rather than doing it by hand.
	(build_typedef_type): Don't try to rely on canonical typedefs
	DIEs.
	* src/abg-ir.cc
	(type_topo_comp::has_artificial_or_natural_location): Define new
	method.
	(type_topo_comp::operator(const type_base*, const type_base*)): In
	this topological comparison operator of types, when two types have
	the same textual representation, if they don't have any location,
	if they are pointer/reference/typedef types, use the textual
	representation of their ultimate underlying type for sorting
	order.
	(peel_typedef_pointer_or_reference_type): Define new function.
	(compare_types_during_canonicalization): Perform the structural
	canonical comparison first.
	(equals): In the overload for array_type_def::subrange_type&,
	remove unused code. In the overload for typedef_decl& don't
	compare typedef names.  In the overload for class_or_union, make
	the RETURN macro *NOT* propagate canonical type because this equal
	function is used as a subroutine by overloads for class and union.
	That means that the class_or_union equal function can return true,
	and still, the caller (overload for class_decl or union_decl) can
	later return false; in that case, we don't want canonical type to
	be propagated.  In the overload for class_decl::base_spec, use the
	ABG_RETURN macro when returning comparing decl-bases.  In the
	overload for class_decl, use ABG_RETURN when returning the result
	of comparing class_or_union artifacts.
	(maybe_propagate_canonical_type):  If we are using canonical type
	comparison, then do not propagate the type, if
	WITH_DEBUG_TYPE_CANONICALIZATION is defined.
	(is_non_canonicalized_type): Add typedefs to the set of
	non-canonicalized types.
	* src/abg-writer.cc (struct non_canonicalized_type_hash, struct
	non_canonicalized_type_equal): Define new functors.
	(typedef nc_type_ptr_set_type): Define a new non-canonicalized set
	type.
	(typedef nc_type_ptr_istr_map_type): Define a new map of
	non-canonicalized types/interned string map.
	(write_context::{m_nc_type_id_map,
	m_emitted_non_canonicalized_type_set,
	m_referenced_non_canonicalized_types_set}): New data members.
	(write_context::type_has_existing_id): Look for non-canonicalized
	types in the m_nc_type_id_map map.
	(write_context::get_id_for_type): Use m_nc_type_id_map for
	non-canonicalized types.
	(write_context::clear_type_id_map): Clear m_nc_type_id_map too.
	(write_context::get_referenced_non_canonicalized_types): Renamed
	write_context::get_referenced_non_canonical_types into this.  Make
	it return the new
	write_context::m_referenced_non_canonicalized_types_set.
	(write_context::has_non_emitted_referenced_types): Just use
	write_context::type_is_emitted as it knows where to look up for
	non-canonicalized types.
	(write_context::record_type_as_referenced): Use
	m_referenced_non_canonicalized_types_set to store referenced
	non-canonicalized types.  The other types are stored in
	m_emitted_set as previously.
	(write_context::type_is_emitted): Look into
	m_emitted_non_canonicalized_type_set for non-canonicalized types.
	Other types are still in m_emitted_type_set.
	(write_context::{record_decl_only_type_as_emitted,
	decl_only_type_is_emitted, get_emitted_decl_only_types_set}):
	Remove methods.
	(write_context::get_emitted_non_canonicalized_type_set): Define
	new method.
	(write_context::clear_referenced_types): Clear
	m_referenced_non_canonicalized_types_set too, as
	m_referenced_non_canonical_types_set is no more.
	(write_context::referenced_type_should_be_emitted): Use only
	write_context::type_is_emitted as it knows how to check it all
	now.
	(write_referenced_types): Use the more compact ranged-base for
	loops.
	(write_translation_unit): Write the non-canonicalized types that
	are not yet emitted.
	(write_class_decl, write_union_decl): Adjust recording type as
	emitted.
	(write_canonical_type_ids): Adjust.
	* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
	* tests/data/test-annotate/libtest23.so.abi: Likewise.
	* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Likewise.
	* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
	* tests/data/test-annotate/test-anonymous-members-0.o.abi: Likewise.
	* tests/data/test-annotate/test0.abi: Likewise.
	* tests/data/test-annotate/test1.abi: Likewise.
	* tests/data/test-annotate/test13-pr18894.so.abi: Likewise.
	* tests/data/test-annotate/test14-pr18893.so.abi: Adjust.
	* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
	* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
	* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise.
	* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise.
	* tests/data/test-annotate/test2.so.abi: Likewise.
	* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise.
	* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
	* tests/data/test-annotate/test3.so.abi: Likewise.
	* tests/data/test-annotate/test5.o.abi: Likewise.
	* tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0.abi:
	Likewise.
	* tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1.abi:
	Likewise.
	* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt: Likewise.
	* tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt: Likewise.
	* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt: Likewise.
	* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt: Likewise.
	* tests/data/test-diff-filter/test41-report-0.txt: Likewise.
	* tests/data/test-diff-pkg/libICE-1.0.6-1.el6.x86_64.rpm--libICE-1.0.9-2.el7.x86_64:
	Likewise.
	* tests/data/test-diff-pkg/nmap-7.70-5.el8_testjcc.x86_64-self-check-report-0.txt:
	Likewise
	* tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.txt:
	Likewise.
	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-0.txt:
	Likewise.
	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
	Likewise.
	* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt:
	Likewise.
	* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt:
	Likewise.
	* tests/data/test-diff-suppr/test39-opaque-type-report-0.txt:
	Likewise.
	* tests/data/test-read-ctf/test-ambiguous-struct-A.o.hash.abi:
	Likewise.
	* tests/data/test-read-ctf/test-conflicting-type-syms-a.o.hash.abi:
	Likewise.
	* tests/data/test-read-ctf/test-conflicting-type-syms-b.o.hash.abi:
	Likewise.
	* tests/data/test-read-ctf/test-functions-declaration.abi:
	Likewise.
	* tests/data/test-read-ctf/test1.so.abi: Likewise.
	* tests/data/test-read-ctf/test1.so.hash.abi: Likewise.
	* tests/data/test-read-ctf/test2.so.abi: Likewise.
	* tests/data/test-read-ctf/test2.so.hash.abi: Likewise.
	* tests/data/test-read-ctf/test5.o.abi: Likewise.
	* tests/data/test-read-ctf/test7.o.abi: Likewise.
	* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
	Likewise.
	* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
	* tests/data/test-read-dwarf/PR24378-fn-is-not-scope.abi:
	Likewise.
	* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.
	* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
	Likewise.
	* tests/data/test-read-dwarf/PR26261/PR26261-exe.abi: Likewise.
	* tests/data/test-read-dwarf/PR27700/test-PR27700.abi: Likewise.
	* tests/data/test-read-dwarf/libtest23.so.abi: Likewise.
	* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi:
	Likewise.
	* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Likewise.
	* tests/data/test-read-dwarf/test-PR26568-1.o.abi: Likewise.
	* tests/data/test-read-dwarf/test-PR26568-2.o.abi: Likewise.
	* tests/data/test-read-dwarf/test-libaaudio.so.abi: Likewise.
	* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
	* tests/data/test-read-dwarf/test-suppressed-alias.o.abi:
	Likewise.
	* tests/data/test-read-dwarf/test0.abi: Likewise.
	* tests/data/test-read-dwarf/test0.hash.abi: Likewise.
	* tests/data/test-read-dwarf/test1.abi: Likewise.
	* tests/data/test-read-dwarf/test1.hash.abi: Likewise.
	* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
	* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
	* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
	* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
	* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
	* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
	* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
	* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
	* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
	Likewise.
	* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
	Likewise.
	* tests/data/test-read-dwarf/test2.so.abi: Likewise.
	* tests/data/test-read-dwarf/test2.so.hash.abi: Likewise.
	* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Likewise.
	* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
	* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
	Likewise.
	* tests/data/test-read-dwarf/test3-alias-1.so.hash.abi: Likewise.
	* tests/data/test-read-dwarf/test3-alias-2.so.hash.abi: Likewise.
	* tests/data/test-read-dwarf/test3-alias-3.so.hash.abi: Likewise.
	* tests/data/test-read-dwarf/test3-alias-4.so.hash.abi: Likewise.
	* tests/data/test-read-dwarf/test3.so.abi: Likewise.
	* tests/data/test-read-dwarf/test3.so.hash.abi: Likewise.
	* tests/data/test-read-dwarf/test5.o.abi: Likewise.
	* tests/data/test-read-dwarf/test5.o.hash.abi: Likewise.
	* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
	* tests/data/test-read-write/test18.xml: Likewise.
	* tests/data/test-read-write/test28-without-std-fns-ref.xml:
	Likewise.
	* tests/data/test-read-write/test28-without-std-vars-ref.xml:
	Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Applied to master.
---
 include/abg-fwd.h                             |     4 +
 include/abg-ir.h                              |     4 +
 src/abg-dwarf-reader.cc                       |   837 +-
 src/abg-ir-priv.h                             |     4 +-
 src/abg-ir.cc                                 |   251 +-
 src/abg-writer.cc                             |   200 +-
 tests/Makefile.am                             |     2 +-
 .../test-abidiff/test-PR18791-report0.txt     |    21 -
 tests/data/test-annotate/libtest23.so.abi     |   687 +-
 .../test-annotate/libtest24-drop-fns-2.so.abi |   704 +-
 .../test-annotate/libtest24-drop-fns.so.abi   |   704 +-
 .../test-anonymous-members-0.o.abi            |     4 +-
 tests/data/test-annotate/test0.abi            |    12 +-
 tests/data/test-annotate/test1.abi            |     4 +-
 .../data/test-annotate/test13-pr18894.so.abi  |  2016 +-
 .../data/test-annotate/test14-pr18893.so.abi  | 28721 +++---
 .../data/test-annotate/test15-pr18892.so.abi  | 39180 +++++----
 .../data/test-annotate/test17-pr19027.so.abi  | 17889 ++--
 ...st18-pr19037-libvtkRenderingLIC-6.1.so.abi | 22616 +++--
 ...19-pr19023-libtcmalloc_and_profiler.so.abi | 51710 ++++++-----
 tests/data/test-annotate/test2.so.abi         |    20 +-
 ...st20-pr19025-libvtkParallelCore-6.1.so.abi | 46271 +++++-----
 .../data/test-annotate/test21-pr19092.so.abi  |  7163 +-
 tests/data/test-annotate/test3.so.abi         |     4 +-
 tests/data/test-annotate/test5.o.abi          |     4 +-
 .../PR25409-librte_bus_dpaa.so.20.0.abi       |  3696 +-
 .../test0-pr19026-libvtkIOSQL-6.1.so.1.abi    | 16627 ++--
 .../PR25058-liblttng-ctl-report-1.txt         |     7 +-
 .../test42-PR21296-clanggcc-report0.txt       |     6 -
 .../test31-pr18535-libstdc++-report-0.txt     |    33 +-
 .../test31-pr18535-libstdc++-report-1.txt     |    33 +-
 .../data/test-diff-filter/test41-report-0.txt |    55 +-
 ...libICE-1.0.9-2.el7.x86_64.rpm-report-0.txt |    19 +-
 .../nss-3.23.0-1.0.fc23.x86_64-report-0.txt   |     4 +-
 ...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 |  1040 +-
 ...bb-4.3-3.20141204.fc23.x86_64-report-0.txt |   164 +-
 ...bb-4.3-3.20141204.fc23.x86_64-report-1.txt |    21 +-
 .../test39-opaque-type-report-0.txt           |     9 +-
 .../test-ambiguous-struct-A.o.hash.abi        |     2 +-
 .../test-conflicting-type-syms-a.o.hash.abi   |     4 +-
 .../test-conflicting-type-syms-b.o.hash.abi   |     4 +-
 .../test-functions-declaration.abi            |     2 +-
 tests/data/test-read-ctf/test1.so.abi         |    10 +-
 tests/data/test-read-ctf/test1.so.hash.abi    |     4 +-
 tests/data/test-read-ctf/test2.so.abi         |    22 +-
 tests/data/test-read-ctf/test2.so.hash.abi    |     2 +-
 tests/data/test-read-ctf/test5.o.abi          |    19 +-
 tests/data/test-read-ctf/test7.o.abi          |    17 +-
 .../PR22015-libboost_iostreams.so.abi         |  3553 +-
 .../test-read-dwarf/PR22122-libftdc.so.abi    |  9880 +--
 .../PR24378-fn-is-not-scope.abi               |     4 +-
 .../data/test-read-dwarf/PR25007-sdhci.ko.abi |  7604 +-
 .../PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi |   605 +-
 .../test-read-dwarf/PR26261/PR26261-exe.abi   |    42 +-
 .../test-read-dwarf/PR27700/test-PR27700.abi  |     2 +-
 tests/data/test-read-dwarf/libtest23.so.abi   |   643 +-
 .../libtest24-drop-fns-2.so.abi               |   657 +-
 .../test-read-dwarf/libtest24-drop-fns.so.abi |   657 +-
 .../data/test-read-dwarf/test-PR26568-1.o.abi |     2 +-
 .../data/test-read-dwarf/test-PR26568-2.o.abi |     2 +-
 .../test-read-dwarf/test-libaaudio.so.abi     |   155 +-
 .../test-read-dwarf/test-libandroid.so.abi    | 42451 ++++-----
 .../test-suppressed-alias.o.abi               |     2 +-
 tests/data/test-read-dwarf/test0.abi          |     7 +-
 tests/data/test-read-dwarf/test0.hash.abi     |     5 +-
 tests/data/test-read-dwarf/test1.abi          |     2 +-
 tests/data/test-read-dwarf/test1.hash.abi     |     2 +-
 .../test-read-dwarf/test10-pr18818-gcc.so.abi |  6917 +-
 .../test-read-dwarf/test11-pr18828.so.abi     | 14496 ++--
 .../test-read-dwarf/test12-pr18844.so.abi     | 24534 +++---
 .../test-read-dwarf/test13-pr18894.so.abi     |  1841 +-
 .../test-read-dwarf/test14-pr18893.so.abi     | 18542 ++--
 .../test-read-dwarf/test15-pr18892.so.abi     | 25925 +++---
 .../test-read-dwarf/test16-pr18904.so.abi     | 29231 ++++---
 .../test-read-dwarf/test17-pr19027.so.abi     | 16158 ++--
 ...st18-pr19037-libvtkRenderingLIC-6.1.so.abi | 14159 +--
 ...19-pr19023-libtcmalloc_and_profiler.so.abi | 22190 +++--
 tests/data/test-read-dwarf/test2.so.abi       |    14 +-
 tests/data/test-read-dwarf/test2.so.hash.abi  |     6 +-
 ...st20-pr19025-libvtkParallelCore-6.1.so.abi | 26243 +++---
 .../test-read-dwarf/test21-pr19092.so.abi     |  6424 +-
 .../test22-pr19097-libstdc++.so.6.0.17.so.abi | 71862 ++++++++++------
 .../test-read-dwarf/test3-alias-1.so.hash.abi |     2 +-
 .../test-read-dwarf/test3-alias-2.so.hash.abi |     2 +-
 .../test-read-dwarf/test3-alias-3.so.hash.abi |     2 +-
 .../test-read-dwarf/test3-alias-4.so.hash.abi |     2 +-
 tests/data/test-read-dwarf/test3.so.abi       |     2 +-
 tests/data/test-read-dwarf/test3.so.hash.abi  |     2 +-
 tests/data/test-read-dwarf/test5.o.abi        |     2 +-
 tests/data/test-read-dwarf/test5.o.hash.abi   |     2 +-
 .../test9-pr18818-clang.so.abi                |  4619 +-
 tests/data/test-read-write/test18.xml         |     2 +-
 .../test28-without-std-fns-ref.xml            |   770 +-
 .../test28-without-std-vars-ref.xml           |   690 +-
 95 files changed, 336241 insertions(+), 255515 deletions(-)

The patch is too big for the mailing list, so you can read it online at
https://sourceware.org/git/?p=libabigail.git;a=commit;h=7ecef6361799326b99129a479b43b138f0b237ae.

Cheers,

-- 
		Dodji


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-06-20 15:35 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-20 15:35 [PATCH, applied] Canonicalize DIEs w/o assuming ODR & handle typedefs transparently Dodji Seketeli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).