public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
From: Dodji Seketeli <dodji@redhat.com>
To: libabigail@sourceware.org
Cc: dodji@redhat.com
Subject: [PATCH 4/4, applied] ir, writer: Go back to canonicalizing typedefs in the IR
Date: Tue, 20 Sep 2022 13:37:05 +0200	[thread overview]
Message-ID: <878rme5lb2.fsf@redhat.com> (raw)
In-Reply-To: <87tu525ngh.fsf@seketeli.org> (Dodji Seketeli's message of "Tue, 20 Sep 2022 12:50:38 +0200")

[-- Attachment #1: Type: text/plain, Size: 15969 bytes --]

Hello,

Now that DIE and IR canonicalizing has progressed in precision,
especially due to fixing the canonical type propagation it seems like
we can go back to canonicalizing typedefs again.  This makes the
writer not needing to handle non-canonicalized types because only a
short number and types of types are still non-canonicalized.

Then I ran the test suite and hell broke lose.  Things would work on a
given platform but won't on others, etc.

So a great deal of the patch actually fixes issues uncovered by the
fact that we are back to canonicalizing typedefs.

The symptoms of many of the issues are related to emitting abixml in a
stable manner across toolchains.  But in reality those issues are
real, deeper ones.

This patch is thus ALSO an attempt at fixing all the issues at once
because they can't be separated.  Either the abixml is stable and
"make check" passes on all platforms, or it is not and testing fails
on some platforms.

I believe that at the core of the problems lies the observation that a
given typedef (of a given name) could appear several times (the number
of times not being quite constant) in a given class or namespace.
That would lead to some instabilities in the sorting and ordering of
type-ids depending on the toolchain being used, for instance.

To fix this add_or_update_class_type in the DWARF reader is taught to
avoid adding a typedef to a class if a member type of the same name
already exists at that class scope.  This handles the fact that a
given class can be built piece-wise in the DWARF.  Also, when handling
a typedef type, build_ir_node_from_die is taught to avoid creating a
typedef IR for a given scope (namespace, union or class) if said scope
already contains a type of the same name.  To be able to do that, the
handling of member types is moved from the ir::class_or_union type to
the ir::scope_decl type.  That way, all scopes (not just classes or
unions) can handle looking up for the (member) types they contain.

Another issue that was uncovered is that when emitting decl-only class
named A (in a namespace for instance) there can be some instability as
to which decl-only class A is emitted.  This is due to the fact that
libabigail considers all decl-only classes named A to be equal.  The
problem however is that a decl-only class A might have member types.
And a decl-only A declared in a translation unit somewhere might have
some member types that are different from the same decl-only A
declared in another translation unit.  This doesn't necessarily
violate the ODR, but rather, can be the result of compiler
optimization.  For instance, in a given translation unit, only a given
member type of the decl-only A is used by the code, whereas in another
one, another member type of the same decl-only A is used.  So only
partial views of A are emitted in the translation unit depending on
what is used.  Anyway, to handle that case, when comes the time to
emit the decl-only A in the ABIXML writer, write_class_decl now emits
all the member types of all the instances A known to the system.  This
hopefully removes the instability I was seeing.  To do that,
maybe_update_types_lookup_map<class_decl> has been taught to also keep
track of decl-only classes.  A new lookup_decl_only_class_types has
been defined to gather all the instances of a given decl-only class
known to the system.

Last but not least, the topological type sorting facilities carried by
the types {decl,type}_topo_comp has been fixed to ensure a more stable
sorting and also to ensure that "abilint foo.abi" emits the decls and
types in the same order as the one defined in foo.abi.  This fixes
another abixml instability I was seeing in the test suite across
different toolchains.  While doing that, I noticed that the ABIXML
reader was forgetting to properly mark the corpus as originating from
ABIXML.  So I fixed that too.

	* include/abg-fwd.h (lookup_decl_only_class_types): Declare new
	function.
	* src/abg-ir-priv.h (class_or_union::priv::member_types_): Move
	this data member into scope_decl::priv.
	(class_or_union::priv::priv): Do not take member types.
	* include/abg-ir.h (sort_type): Likewise.
	(namespaces_type): Add new typedefs.
	(scope_decl::insert_member_decl): Make this be non-virtual.
	(scope_decl::{insert_member_type, add_member_type,
	remove_member_type, get_member_types, get_sorted_member_types,
	find_member_type}): Move these methods here from ...
	(class_or_union::{insert_member_type, add_member_type,
	remove_member_type, get_member_types, get_sorted_member_types,
	find_member_type}): ... here.
	(class_or_union::insert_member_decl): Make this be non-virtual.
	(class_decl::insert_member_decl): Make this be non-virtual.
	(class_or_union::get_sorted_member_types): Declare ...
	* src/abg-dwarf-reader.cc (add_or_update_class_type): Do not add a
	member type to the class type being built if it already exists
	there.
	(build_ir_node_from_die): If a scope (namespace, union or class)
	already has a typedef, do not create a new one to add it there
	again.
	* src/abg-ir.cc (equals): In the overload for typedefs take into
	account the name of typedefs again.
	(lookup_decl_only_class_types): Define new function.
	(compare_using_locations): Define new static function that has
	been factorized out of ...
	(decl_topo_comp::operator()): ... here.  Also, two decls originate
	from an abixml corpus, compare them only using their artificial
	locations, meaning make them keep the same order as the order of
	the original abixml file.
	(type_top_comp::operator()): Likewise.
	(sort_types): Define new function.
	(scope_decl::priv::member_types_): Move this here from
	class_or_union::priv.
	(scope_decl::priv::sorted_member_types_): Define new data member.
	(scope_decl::{get_member_types, find_member_type,
	insert_member_type, add_member_type, remove_member_type}): Move
	these methods here from class_or_union.
	(scope_decl::get_sorted_member_types): Define new method.
	(is_non_canonicalized_type): Canonicalize typedefs.
	(lookup_type_in_map): Allow this to look through decl-only types
	to get their definition.  Otherwise, if only a decl-only was
	found, return it.
	(maybe_update_types_lookup_map<class_decl>): Allow adding
	decl-only class types to the map of classes held per TU and per
	corpus.
	(class_or_union::{class_or_union, add_member_decl,
	has_no_member}): Adjust.
	(class_or_union::{insert_member_type, add_member_type,
	add_member_type, remove_member_type, get_member_types,
	get_sorted_member_types, find_member_type}): Move these methods to
	scope_decl.
	({class_decl, class_or_union}::insert_member_decl): Remove the
	"before" parameter as it was not used anymore due to the re-use of
	the scope_decl::add_member that handles member types.
	* src/abg-reader.cc (read_corpus_group_from_input)
	(create_native_xml_read_context): Set the corpus origin.
	* src/abg-writer.cc (write_context::{m_nc_type_id_map,
	m_emitted_non_canonicalized_type_set,
	m_referenced_non_canonicalized_types_set}): Remove these maps that
	hold stuff for non-canonicalized types.
	(write_context::{type_has_existing_id, get_id_for_type,
	clear_type_id_map, has_non_emitted_referenced_types,
	record_type_as_referenced, type_is_referenced,
	record_type_as_emitted, type_is_emitted, clear_referenced_types,
	write_referenced_types, write_canonical_type_ids}): Adjust these
	as the maps for non-canonicalized types are gone.
	(write_context::{get_referenced_non_canonicalized_types,
	get_emitted_non_canonicalized_type_set}): Remove these methods.
	(write_decl_in_scope)
	(write_class_decl_opening_tag, write_union_decl_opening_tag)
	(write_union_decl): Adjust comments.
	(write_class_decl): Sort member types.  Also, When emitting a
	decl-only class, get all of the decl-only classes of the same name
	declared in several other TUs and emit all their member types into
	this decl-only class just once.  This reduces redundancies and
	sorting instabilities because for libabigail, all decl-only
	classes of a given name are equal, even if they have member types.
	* 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: Likewise.
	* 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-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/PR24690/PR24690-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-read-dwarf/PR22015-libboost_iostreams.so.abi:
	Likewise.
	* tests/data/test-read-dwarf/PR22122-libftdc.so.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/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/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>
---
 include/abg-fwd.h                             |     5 +
 include/abg-ir.h                              |    58 +-
 src/abg-dwarf-reader.cc                       |    17 +-
 src/abg-ir-priv.h                             |     7 +-
 src/abg-ir.cc                                 |   420 +-
 src/abg-reader.cc                             |     3 +
 src/abg-writer.cc                             |   166 +-
 .../test-abidiff/test-PR18791-report0.txt     |    13 +
 tests/data/test-annotate/libtest23.so.abi     |   724 +-
 .../test-annotate/libtest24-drop-fns-2.so.abi |   804 +-
 .../test-annotate/libtest24-drop-fns.so.abi   |   804 +-
 .../test-anonymous-members-0.o.abi            |    96 +-
 tests/data/test-annotate/test0.abi            |     4 +-
 tests/data/test-annotate/test1.abi            |    32 +-
 .../data/test-annotate/test13-pr18894.so.abi  |  2322 +-
 .../data/test-annotate/test14-pr18893.so.abi  | 18402 ++---
 .../data/test-annotate/test15-pr18892.so.abi  | 64330 ++++++++--------
 .../data/test-annotate/test17-pr19027.so.abi  | 36303 +++++----
 ...st18-pr19037-libvtkRenderingLIC-6.1.so.abi | 18052 +++--
 ...19-pr19023-libtcmalloc_and_profiler.so.abi | 49963 ++++++------
 tests/data/test-annotate/test2.so.abi         |    38 +-
 ...st20-pr19025-libvtkParallelCore-6.1.so.abi | 28890 +++----
 .../data/test-annotate/test21-pr19092.so.abi  |  7608 +-
 .../test42-PR21296-clanggcc-report0.txt       |     6 +
 .../test31-pr18535-libstdc++-report-0.txt     |    31 +-
 .../test31-pr18535-libstdc++-report-1.txt     |    31 +-
 .../data/test-diff-filter/test41-report-0.txt |    30 +-
 .../PR24690/PR24690-report-0.txt              |     2 +-
 ...l7.x86_64-0.12.8-1.el7.x86_64-report-2.txt |    31 +-
 .../PR22015-libboost_iostreams.so.abi         |  2858 +-
 .../test-read-dwarf/PR22122-libftdc.so.abi    | 16990 +++-
 .../data/test-read-dwarf/PR25007-sdhci.ko.abi | 14620 ++--
 .../PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi |   642 +-
 .../test-read-dwarf/PR26261/PR26261-exe.abi   |    10 +-
 .../test-read-dwarf/PR27700/test-PR27700.abi  |     2 +-
 tests/data/test-read-dwarf/libtest23.so.abi   |   626 +-
 .../libtest24-drop-fns-2.so.abi               |   702 +-
 .../test-read-dwarf/libtest24-drop-fns.so.abi |   702 +-
 .../data/test-read-dwarf/test-PR26568-1.o.abi |    16 +-
 .../data/test-read-dwarf/test-PR26568-2.o.abi |    22 +-
 .../test-read-dwarf/test-libaaudio.so.abi     |   238 +-
 .../test-read-dwarf/test-libandroid.so.abi    | 56370 +++++++-------
 tests/data/test-read-dwarf/test0.abi          |     2 +-
 tests/data/test-read-dwarf/test0.hash.abi     |     2 +-
 tests/data/test-read-dwarf/test1.abi          |    26 +-
 tests/data/test-read-dwarf/test1.hash.abi     |     6 +-
 .../test-read-dwarf/test10-pr18818-gcc.so.abi |  7990 +-
 .../test-read-dwarf/test11-pr18828.so.abi     | 23566 +++---
 .../test-read-dwarf/test12-pr18844.so.abi     | 33361 ++++----
 .../test-read-dwarf/test13-pr18894.so.abi     |  2090 +-
 .../test-read-dwarf/test14-pr18893.so.abi     | 15144 ++--
 .../test-read-dwarf/test15-pr18892.so.abi     | 26815 ++++---
 .../test-read-dwarf/test16-pr18904.so.abi     | 40951 +++++-----
 .../test-read-dwarf/test17-pr19027.so.abi     | 24080 +++---
 ...st18-pr19037-libvtkRenderingLIC-6.1.so.abi | 14733 ++--
 ...19-pr19023-libtcmalloc_and_profiler.so.abi | 23754 +++---
 tests/data/test-read-dwarf/test2.so.abi       |    34 +-
 tests/data/test-read-dwarf/test2.so.hash.abi  |     4 +-
 ...st20-pr19025-libvtkParallelCore-6.1.so.abi | 21807 +++---
 .../test-read-dwarf/test21-pr19092.so.abi     |  6494 +-
 .../test22-pr19097-libstdc++.so.6.0.17.so.abi | 59148 +++++++-------
 .../test9-pr18818-clang.so.abi                |  4896 +-
 tests/data/test-read-write/test18.xml         |     2 +-
 .../test28-without-std-fns-ref.xml            |   860 +-
 .../test28-without-std-vars-ref.xml           |   780 +-
 65 files changed, 318361 insertions(+), 311174 deletions(-)

The patch is too big for the list so I am attaching it here.


[-- Attachment #2: 0004-ir-writer-Go-back-to-canonicalizing-typedefs-in-the-.patch.gz --]
[-- Type: application/gzip, Size: 4002840 bytes --]

[-- Attachment #3: Type: text/plain, Size: 14 bytes --]


-- 
		Dodji


      parent reply	other threads:[~2022-09-20 11:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-20 10:50 [PATCH 0/4, applied] Speed up type DIEs canonicalization Dodji Seketeli
2022-09-20 11:27 ` [PATCH 1/4, applied] dwarf-reader: Revamp the canonical type DIE propagation algorithm Dodji Seketeli
2022-09-20 11:29 ` [PATCH 2/4, applied] Allow restricting analyzed decls to exported symbols Dodji Seketeli
2022-09-20 11:30 ` [PATCH 3/4] Fix IR comparison result caching and canonical type propagation tracking Dodji Seketeli
2022-09-20 11:37 ` Dodji Seketeli [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878rme5lb2.fsf@redhat.com \
    --to=dodji@redhat.com \
    --cc=libabigail@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).