Commit Graph

2263 Commits

Author SHA1 Message Date
Dodji Seketeli
2f92777dc8 Consider the implicit 'this' parameter when comparing methods
Since 2013 the implicit 'this' parameter has been excluded from the
function parameters taken into account while comparing class member
functions.  This was an early measure to avoid infinite recursion that
would then occur when comparing classes (and thus their member
functions that are referenced in their vtable).  But since then, we've
built descent infrastructure to prevent this kind of recursion in a
more generic manner.

This patch thus removes that restriction and should therefore lift the
concerns expressed in the bug
https://sourceware.org/bugzilla/show_bug.cgi?id=26672.

Namely, changes to (data members of) a class should now be detected
when comparing member functions of that class.

With this change, the reference output of several comparison
regression tests changed because, obviously, some impacted member
functions are now reported along with detecting changes in data
membrers of classes.  The patch thus adjusts those reference ouputs.

The patch also adjust the behaviour of the predicate:
  "accessed_through = pointer|reference|reference-or-pointer"
The idea is to make the predicate work on qualified version of a type.

	* include/abg-ir.h (function_type::get_first_parm): Declare new
	accessor.
	* src/abg-ir.cc (function_type::get_first_parm): Define new
	accessor.
	(equals): In the overload for function_type,
	always take the implicit "this" parameter into account in
	parameter comparisons.
	(function_type::get_first_non_implicit_parm): Adjust comment.
	* src/abg-comp-filter.cc (function_name_changed_but_not_symbol):
	Avoid potential NULL pointer dereferencing.
	* src/abg-comparison.cc
	(function_type_diff::ensure_lookup_tables_populated): Always take
	the changes to the implicit 'this' parameter into account in the
	function type diff.
	(compute_diff): In the overload for function_type, Always compare
	the implicit 'this' parameter when comparing function parameters.
	* src/abg-default-reporter.cc (default_reporter::report): Refer to
	"implicit parameter" when reporting changes on parameters
	artificially generated by the compiler.
	* src/abg-suppression.cc (type_suppression::suppresses_diff): Make
	the 'access_through' predicate work on a qualified version of type
	'S', even if it was meant to work on type 'S'.  This allows it to
	work on 'const S', especially when S is accessed through 'pointer
	to const S', which happens when we consider the implicit 'this'
	parameter of a const member function.
	* tests/data/test-abicompat/test5-fn-changed-report-0.txt: Adjust.
	* tests/data/test-abicompat/test5-fn-changed-report-1.txt: Likewise.
	* tests/data/test-abidiff-exit/test1-voffset-change-report0.txt:
	Likewise.
	* tests/data/test-abidiff/test-PR18791-report0.txt: Likewise.
	* tests/data/test-abidiff/test-struct1-report.txt: Likewise.
	* tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1-report-0.txt:
	Likewise.
	* tests/data/test-diff-dwarf/test0-report.txt: Likewise.
	* tests/data/test-diff-dwarf/test28-vtable-changes-report-0.txt: Likewise.
	* tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt: Likewise.
	* tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt: Likewise.
	* tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt: Likewise.
	* tests/data/test-diff-dwarf/test36-ppc64-aliases-report-0.txt: Likewise.
	* tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt: Likewise.
	* tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt: Likewise.
	* tests/data/test-diff-dwarf/test5-report.txt: Likewise.
	* tests/data/test-diff-dwarf/test8-report.txt: Likewise.
	* tests/data/test-diff-filter/test0-report.txt: Likewise.
	* tests/data/test-diff-filter/test01-report.txt: Likewise.
	* tests/data/test-diff-filter/test10-report.txt: Likewise.
	* tests/data/test-diff-filter/test13-report.txt: Likewise.
	* tests/data/test-diff-filter/test2-report.txt: Likewise.
	* tests/data/test-diff-filter/test28-redundant-and-filtered-children-nodes-report-0.txt:
	Likewise.
	* tests/data/test-diff-filter/test28-redundant-and-filtered-children-nodes-report-1.txt:
	Likewise.
	* tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.txt:
	Likewise.
	* tests/data/test-diff-filter/test30-pr18904-rvalueref-report1.txt:
	Likewise.
	* tests/data/test-diff-filter/test30-pr18904-rvalueref-report2.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/test35-pr18754-no-added-syms-report-0.txt:
	Likewise.
	* tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-1.txt:
	Likewise.
	* tests/data/test-diff-filter/test4-report.txt: Likewise.
	* tests/data/test-diff-filter/test41-report-0.txt: Likewise.
	* tests/data/test-diff-filter/test9-report.txt: Likewise.
	* tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-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-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/test24-soname-report-0.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-1.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-10.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-11.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-12.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-13.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-14.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-15.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-16.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-2.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-3.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-4.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-5.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-6.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-7.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-8.txt: Likewise.
	* tests/data/test-diff-suppr/test24-soname-report-9.txt: Likewise.
	* tests/data/test-diff-suppr/test31-report-1.txt: Likewise.
	* tests/data/test-diff-suppr/test33-report-0.txt: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-10-09 11:05:23 +02:00
Giuliano Procida
b96040e5b3 Fix two wrongs in test suppression regex
The suppression specification in test38-char-class-in-ini.abignore was
introduced in commit 1478d9cc1c.

Unfortunately it contains two errors. One causes the file name not to
match as the string is the full path, not the base name. The other is
a typo that causes the file name match not to even be attempted. The
two mistakes cancel in the test, but result in a suppression
specification that is broader than intended.

	* tests/data/test-diff-suppr/test38-char-class-in-ini.abignore:
	Don't anchor regex match to beginning of file name.
	Change "filename_regexp" to "file_name_regexp".

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-10-01 16:42:27 +02:00
Giuliano Procida
1ab36e02e5 Add missing newlines to end of test files.
Various test files were missing terminal newlines.

	* tests/data/test-diff-suppr/test0-type-suppr-2.suppr: Add
	final new line.
	* tests/data/test-diff-suppr/test22-suppr-removed-var-sym-4.suppr:
	Likewise.
	* tests/data/test-diff-suppr/test23-alias-filter-0.suppr:
	Likewise.
	* tests/data/test-diff-suppr/test23-alias-filter-4.suppr:
	Likewise.
	* tests/data/test-diff-suppr/test28-add-aliased-function-1.suppr:
	Likewise.
	* tests/data/test-diff-suppr/test28-add-aliased-function-2.suppr:
	Likewise.
	* tests/data/test-diff-suppr/test28-add-aliased-function-3.suppr:
	Likewise.
	* tests/data/test-diff-suppr/test28-add-aliased-function-4.suppr:
	Likewise.
	* tests/data/test-diff-suppr/test41-enumerator-changes-0.suppr:
	Likewise.
	* tests/data/test-diff-suppr/test7-var-suppr-7.suppr:
	Likewise.
	* tests/data/test-ini/test01-equal-in-property-string.abignore:
	Likewise.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-10-01 16:11:29 +02:00
Giuliano Procida
ebaf3305bc abg-corpus.cc: report architecture discrepancies
If there ever is a discrepancy between the architectures of the
corpuses of a corpus group, libabigail will just abort with an
assertion, if enabled. However, this is a case of invalid input and
the cause should be reported to the user.

	* src/abg-corpus.cc (corpus_group::add_corpus): Report
	architecture discrepancies.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-09-22 14:46:07 +02:00
Dodji Seketeli
59610d5572 Bug 26568 - Union should support more than one anonymous member
When building a union type we try to ensure that each member is
present only once.  This is because the code to build the union is
also used to actually update a partially constructed union.  To do so,
before adding a member to the union, the member is looked up (among
the current members) by name to see if it's already present or not.

But then for anonymous members, the name of the member is empty.
After the first anonymous member is added to the union, subsequent
look-ups with an empty name all succeed.  So no more than one
anonymous member is added to the union.  Oops.

A way to fix this is to perform the lookup by taking into account the
type of the anonymous data member, not its (empty) name.  We already
do this for anonymous data members of classes/structs.

This patch thus uses that type-based anonymous data member lookup for
unions.

But then now that unions can have several anonymous members, another
issue was uncovered.

Array types whose elements are of anonymous type can be wrongly
considered different because of canonicalization issues.

Let's suppose we have these two arrays whose internal pretty
representation are:

    "__anonymous_struct_1__ foo[5]"

and

   "__anonymous_struct_2__ foo[5]"

These are arrays of 5 elements of type anonymous struct.  Suppose the
anonymous structs "__anonymous_struct_1__" and
"__anonymous_struct_2__" are structurally equivalent.  Because the
internal names of these array element types are different, the
internal pretty representations of the arrays are different.  And thus
the canonical types of the two arrays are different.  And that's
wrong.  In this particular case, they should have the same canonical
type and thus be considered equivalent.

This patch thus teaches 'get_type_name' to make the internal type
name of anonymous types of a given kind be the same.  Thus, making all
arrays of 5 anonymous struct have the same pretty representation:

    "__anonymous_struct__ foo[5]"

This gives the type canonicalizer a chance to detect that those arrays
having the same canonical type.

These two changes while being seemingly unrelated need to be bundled
together to fix a number of issues in the existing test reference
outputs because fixing the first one is needed to uncover the later
issue.

	* src/abg-dwarf-reader.cc (add_or_update_union_type): Don't use
	the empty name of anonymous members in the lookup to ensure that
	all data members are unique.  Rather, use the whole anonymous
	member itself for the lookup, just like is done to handle
	anonymous data member in classes/structs.
	* src/abg-reader.cc (build_union_decl): Likewise.
	* src/abg-ir.cc (get_generic_anonymous_internal_type_name): Define
	new static function.
	(get_type_name): For internal purposes, make the type name of all
	anonymous types of a given kind to be the same.  This allows the
	internal representation of anonymous types which are based on type
	names to all be the same, so that they can be compared among
	themselves during type canonicalization.
	* tests/data/test-read-dwarf/test-PR26568-{1,2}.c: Source code of
	binary test input.
	* tests/data/test-read-dwarf/test-PR26568-{1,2}.o: New binary test input.
	* tests/data/test-read-dwarf/test-PR26568-{1,2}.o.abi: New
	reference test ouput.
	* tests/data/Makefile.am: Add the new test material above to
	source distribution.
	* tests/test-read-dwarf.cc (in_out_specs): Add the new binary test
	input above to this test harness.
	* tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0.abi: Adjust.
	* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
	* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-09-18 16:32:49 +02:00
Dodji Seketeli
ee2b54ddd9 Make abidiff and abidw support several --headers-dir{1,2} options
When handling a binary with abidiff or abidw it can be useful to
provide several different header files directories, for the cases
where the header files of the binary are scathered in several
different directories.

It thus becomes possible to invoke abidiff like this:

    abidiff --headers-dir1 first-header-dir1   \
	    --headers-dir1 second-header-dir1  \
	    --headers-dir2 first-header-dir2   \
	    --headers-dir2 second-header-dir2  \
	    binary1 binary2

This patch adds support for that.  It also modifies the
tests/test-abidiff-exit.cc test harness to make it take header
directories.  With that modification done, a new test is added in that
harness to exercise this new feature.

This should close the feature request over at
https://sourceware.org/bugzilla/show_bug.cgi?id=26565.

	* doc/manuals/abidiff.rst: Update documentation for the
	--headers-dir{1,2} options.
	* doc/manuals/abidw.rst: Likewise for the --header-dir option.
	* include/abg-tools-utils.h (gen_suppr_spec_from_headers): Add new
	overload that takes a vector of headers root dirs.
	* src/abg-tools-utils.cc (gen_suppr_spec_from_headers_root_dir):
	Define new function.
	(gen_suppr_spec_from_headers): Define a new overload that takes a
	vector of head_root_dir strings; it uses the new
	gen_suppr_spec_from_headers function.  Use the new overload in the
	previous one that takes just one head_root_dir string.
	* tools/abidiff.cc (options::headers_dirs{1,2}): Rename
	option::headers_dir{1,2} into this one and make it be a vector of
	strings rather than just a string.
	(parse_command_line): Support several --headers-dir{1,2} on the
	command line.
	(set_diff_context_from_opts, set_suppressions): Adjust.
	* tools/abidw.cc (options::headers_dirs): Renamed
	options::headers_dir into this and make it be a vector of strings
	rather than just a string.
	(parse_command_line): Support several --headers-dir on the command
	line.
	(set_suppressions): Adjust.
	* tests/data/test-abidiff-exit/test-headers-dirs/headers-a/header-a-v{0,1}.h:
	Header files of new binary test input.
	* tests/data/test-abidiff-exit/test-headers-dirs/headers-b/header-b-v{0,1}.h:
	Likewise.
	* tests/data/test-abidiff-exit/test-headers-dirs/test-headers-dir-v{0,1}.c:
	Source code of new binary test input.
	* tests/data/test-abidiff-exit/test-headers-dirs/test-headers-dir-report-{1,2}.txt:
	Reference output of new binary test input.
	* tests/data/test-abidiff-exit/test-headers-dirs/test-headers-dir-v{0,1}.o:
	New binary test input.
	* tests/data/Makefile.am: Add the new files above to source
	distribution.
	* tests/test-abidiff-exit.cc (InOutSpec::in_elfv{0,1}_path): Add
	new data members.
	(in_out_specs): Adjust the content of this array as its type
	changed.  Also, add two new entries to run the test over the new
	binary test inputs above.
	(do_prefix_strings): Define new static function.
	(main): Use it the new do_prefix_strings here.  Make abidiff
	use the --header-dir{1,2} option whenever header directories are
	specified in an entry of the in_out_specs array.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-09-18 10:27:18 +02:00
Dodji Seketeli
3d7916caa4 Bug 26309 - Wrong leaf reporting of changes to typedef underlying type
In leaf mode, libabigail fails to report changes to the underlying
type of a typedef.

At its core, this is due to the fact that changes to the underlying
type of a typedef are not considered local.  As the leaf reporter only
reports local changes (as opposed to non-local changes which are
changes to sub-types) it doesn't detect those non-local typedef
changes.

To handle this, this patch makes changes to the underlying type of a
typedef be considered as local changes.  This is like what we already
do for pointer and qualified types.

Now that we have another set of changes to report in the leaf
reporter, we need to handle how to propagate the category and
redundancy status of those changes.  The patch does this too.

Also, just like we do pointer and qualified type changes, the patch
avoids marking the diff node carrying the typedef change as being a
leaf change.  That way, only existing leaf changes carrying that
typedef diff node will be reported.  For instance, a function whose
parameter has a typedef change will be reported because that change to
the function is considered a leaf change.  Otherwise, reporting the
typedef (or the pointer or qualified) type change on its own is not
useful unless it impacts those leaf changes that we deem useful.

The patch adds the example given in problem report to the testsuite.

 	* src/abg-ir.cc (equals): In the overload for typedef_decls,
	report changes to the underlying type as being local of kind
	LOCAL_TYPE_CHANGE_KIND.
	* src/abg-comparison.cc
	(leaf_diff_node_marker_visitor::visit_begin): Do not mark typedef
	diff node as leaf node.
	(suppression_categorization_visitor::visit_end): Propagate the
	'suppressed' category of the underlying type to the parent typedef
	unless the later has a local non-type change.
	(redundancy_marking_visitor::visit_end): Likewise for the
	'redundant' category.
	* include/abg-reporter.h (report_non_type_typedef_changes): Rename ...
	* src/abg-default-reporter.cc (report_non_type_typedef_changes):
	... report_local_typedef_changes into this.
	* src/abg-leaf-reporter.cc (leaf_reporter::report): Make the leaf
	reporter invoke the reporting method of the default reporter for
	typedefs as all typedef changes are now local.
	* tests/data/test-diff-filter/test-PR26309-report-0.txt: Add new
	test reference output.
	* tests/data/test-diff-filter/test-PR26309-v{0,1}.o: Add new test
	binary input.
	* tests/data/test-diff-filter/test-PR26309-v{0,1}.c: Add source
	code for new test binary input.
	* tests/data/Makefile.am: Add the new text material above to
	source distribution.
	* tests/test-diff-filter.cc (in_out_specs): Add the new test input
	above to this test harness.
	* 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.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-09-15 08:51:58 +02:00
Dodji Seketeli
a434894e4d Fix thinko in get_vmlinux_path_from_kernel_dist
* src/abg-tools-utils.cc (get_vmlinux_path_from_kernel_dist): Fix
	thinko.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-09-14 15:51:25 +02:00
Dodji Seketeli
5cf2473d3c writer: Avoid using dynamic hashing in type maps
When using type maps the hash used for a given type T is usually the
pointer value of the canonical type of T.

In the rare cases where T doesn't have a canonical type, we were using
a 'dynamic hash' computed by recursively walking the components of T
and progressively building a hash that way.

The dynamic hashing code hasn't been updated in a while and would need
some overhaul to support hashing with schemes like MD5 or maybe sha
even.  It might be useful for various use cases that have been
proposed by some users over the years but nobody was motivated enough
to implement it.

In the mean time, rather than trying to come up with a fully beefed up
dynamic hashing code, we'd rather just return a constant number for
non canonicalized types.  In practise that amounts to forcing the code
of the maps to always use structural comparison for those non
canonicalized types.

Note that the amount of non-canonicalized types should be fairly
small.  For now, the only non-canonicalized types should be
declaration-only types and those are quite fast to compare anyway.

This patch thus introduces a new hashing scheme for maps in the
writer which just uses a numerical constant as the hash for
non-canonicalized types.

	* include/abg-fwd.h (hash_as_canonical_type_or_constant): Declare ...
	* src/abg-ir.cc (hash_as_canonical_type_or_constant): ... new
	function.
	* src/abg-writer.cc (type_hasher::operator()): Use the new
	hash_as_canonical_type_or_constant.
	* tests/data/test-read-dwarf/test16-pr18904.so.abi: Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-09-10 15:50:10 +02:00
Dodji Seketeli
005ab5c9bd Use flat representation to canonicalize anonymous classes and unions
During the canonicalization of a type T, the algorithm uses the
internal pretty representation of T to limit the number of types to
compare T to.  That internal pretty representation is based on the
type name.

For anonymous types, the type name is not unique; it's constructed
just for internal purposes.  So using that in the pretty
representation might negatively impact the accuracy of the
canonicalization; it might make it so that two types might wrongly be
considered canonicaly different.

To fix that, this change makes the internal pretty representation of
anonymous classes (and unions) use their flat representation.

For the record, the flat representation of an anonymous struct with a
an integer and a char data members is the string:

            'struct {int i; char c;}'

	* src/abg-ir.cc ({class, union}_decl::get_pretty_representation):
	Use the flat representation of the class or union even for
	internal purposes.
	* tests/data/test-annotate/libtest23.so.abi: Adjust.
	* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt: Likewise.
	* tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.txt: Likewise.
	* tests/data/test-diff-filter/test30-pr18904-rvalueref-report1.txt: Likewise.
	* tests/data/test-diff-filter/test30-pr18904-rvalueref-report2.txt: Likewise.
	* tests/data/test-diff-filter/test35-pr18754-no-added-syms-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/PR25007-sdhci.ko.abi: Likewise.
	* tests/data/test-read-dwarf/libtest23.so.abi: Likewise.
	* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-09-10 15:49:11 +02:00
Giuliano Procida
0bd4b93517 DWARF: track chained DIE declaration-only status
Bug 26297: Possible misinterpretation of DW_AT_declaration via DW_AT_specification

A DIE representing a definition can refer to the DIE that represents
the matching declaration by using the DW_AT_specification attribute.

A similar situation exists for a cloned entity (like a cloned variable
or function) where the DW_AT_abstract_origin points to the original
entity.

Usually, to get the union of the attributes for a given definition
DIE, we also need to gather the attributes carried by the declaration
(or original) DIE accompanying the current definition DIE.

For the "is_declaration" attribute however, we should only look at the
current (definition) DIE at hand.  Said otherwise, we should not
follow declaration/origin link when we want to know if a given entity
is declaration-only.

Thus, this commit causes the DWARF reader to only consider a DIE to be
"declaration only" if all DIEs in the chain leading to it have the
DW_AT_declaration attribute set.

It is a follow-up commit to a change making die_is_declaration_only
examine just the immediate DIE.

The responsibility of tracking the cumulative declaration-only status
of DIEs falls on build_ir_node_from_die which is the function that
makes recursive calls to itself on encountering a DW_AT_specification
or DW_AT_abstract_origin link.

Various other functions that would have previously called
die_is_declaration_only are modified so that get the cumulative value
for this flag, rather than just examing the DIE they are given.

This change eliminates a lot of spurious declaration-only types in ABI
output and may also prevent the same, particularly when anonymous,
from confusing libabigail's type equality and canonicalisation logic.

	* src/abg-dwarf-reader.cc (add_or_update_class_type): Add an
	is_declaration_only argument. Use this in favour of the
	die_is_declaration_only helper function.
	(add_or_update_union_type): Ditto.
	(function_is_suppressed): Ditto.
	(build_or_get_fn_decl_if_not_suppressed): Ditto.
	(build_enum_type): Ditto.
	(build_ir_node_from_die): To the main overload, add
	is_declaration_only argument and default this to true.
	Update this to false if the given DIE is not declaration
	only and pass this on in recusrive calls and calls to
	build_enum_type, add_or_update_union_type,
	add_or_update_class_type and
	build_or_get_fn_decl_if_not_suppressed.
	* tests/data/test-annotate/test17-pr19027.so.abi: Update
	test. This is mostly the removal of is-declaration-only
	attributes, removal of unreachable parts of the type graph and
	type id renumbering.
	* 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/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Likewise.
	* tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1-report-0.txt:
	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/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Likewise.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-08-06 18:35:27 +02:00
Giuliano Procida
386fede9ce DWARF: look up DW_AT_declaration non-recursively
Bug 26297: Possible misinterpretation of DW_AT_declaration via DW_AT_specification

The DWARF attribute DW_AT_declaration indicates that a DIE is
"declaration only". DWARF DIEs can be linked with DW_AT_specification
and DW_AT_abstract_origin attributes, effectively combining them. A
lone DW_AT_declaration in a chain of such DIEs should not render the
whole chain declaration only.

The function die_is_declaration_only currently traverses such links in
search of the attribute which precludes being able to check for the
attribute at each DIE in the chain and some DIEs are mistakenly
treated as declaration-only by the DWARF reader.

This commit changes die_is_declaration_only to examine the given DIE
only. It extends the die_flag_attribute function so that it can
perform a direct as well as a recursive attribute search. The function
die_die_attribute's 'look_thru_abstract_origin' argument is renamed to
'recursively' to match.

A following commit will change the DWARF reader to ensure it takes
note of the declaration-only status of all DIEs in a chain.

	* src/abg-dwarf-reader.cc (die_die_attribute): Rename
	'look_thru_abstract_origin' argument to 'recursively' and
	mention DW_AT_specification in its doc comment. Remove stale
	comment for non-existent argument. Simplify code with the help
	of the ternary operator.  (die_flag_attribute): Add
	recursively argument, defaulted to true. If this is false,
	look for attribute using dwarf_attr rather than
	dwarf_attr_integrate.  (die_is_declaration_only): Call
	die_flag_attribute specifying non-recursive attribute search.
	* tests/data/test-annotate/test15-pr18892.so.abi: Update
	tests. This is mostly the removal of unreachable parts of the type
	graph and type id renumbering.
	* 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-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1-report-0.txt:
	Likewise.
	* tests/data/test-read-dwarf/test15-pr18892.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/test22-pr19097-libstdc++.so.6.0.17.so.abi:
	Likewise.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-08-06 18:35:26 +02:00
Giuliano Procida
f18a192aa6 Improve documentation of abidiff --type-id-style
* tools/abidw.cc (display_usage): In documentation of
	"--type-id-style" option, add a missing closing ')', spell
	"type id" without a '-', split overly long string over two
	lines, use "<...>" to indicate mandatory argument and improve
	description of formats.
	* doc/manuals/abidw.rst: In documentation of "--type-id-style"
	option, use "<...>" to indicate mandatory argument.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-08-04 18:41:01 +02:00
Giuliano Procida
dfd410c585 Fix maybe_report_data_members_replaced_by_anon_dm
The function maybe_report_data_members_replaced_by_anon_dm has a bug
and some minor issues. This commit tidies these up.

The function came to my attention when a broken equality test
triggered an infinite loop.

The issues were:

- dms_replaced_by_same_anon_dm declared outside loop, gets reused
- self-comparison of first decl, potential infinite loop
- anonymous_data_member, assigned but not used
- two iterators i, j used, when one would suffice

The first issue results in incorrect diff reports if data members
in a structure are replaced by more than one anonymous data member.
This commit adds additional test cases for this, following the pattern
used for the existing PR25661 ones.

The second issue only affects behaviour if equality is defined
inconsistently with object identity.

	* src/abg-reporter-priv.cc
	(maybe_report_data_members_replaced_by_anon_dm): Move
	declarations of anonymous_data_member and
	dms_replaced_by_same_anon_dm into inner loop. Use
	anonymous_data_member for testing and reporting, allowing
	iterators i and j to be replaced by just iterator i. Push
	first decl onto dms_replaced_by_same_anon_dm unconditionally
	and move control flow logic into loop condition.
	* tests/data/Makefile.am: Add new test cases.
	* tests/data/test-diff-filter/test-PR25661-7-report-1.txt: New
	test case file.
	* tests/data/test-diff-filter/test-PR25661-7-report-2.txt:
	Likewise.
	* tests/data/test-diff-filter/test-PR25661-7-report-3.txt:
	Likewise.
	* tests/data/test-diff-filter/test-PR25661-7-report-4.txt:
	Likewise.
	* tests/data/test-diff-filter/test-PR25661-7-v0.c: Likewise.
	* tests/data/test-diff-filter/test-PR25661-7-v0.o: Likewise.
	* tests/data/test-diff-filter/test-PR25661-7-v1.c: Likewise.
	* tests/data/test-diff-filter/test-PR25661-7-v1.o: Likewise.
	* tests/test-diff-filter.cc: Call new test cases.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-08-04 18:27:22 +02:00
Giuliano Procida
d5576ba30a Fix decl_base comparison function
When two decl_base ojects are compared, there are both fast and slow
paths to the name comparison. The latter is roughly equivalent to
comparing names after applying the regex
s/(__anonymous_(?:struct|union|enum)__)\d+/\1/g to the names before
comparing them while the former is a straight string comparison with
some tweaks for detecting anonymous types.

The slow path is taken care of by the helper function
tools_utils::decl_names_equal but unfortunately, there is a missing
negation of the returned bool. This commit fixes this and updates the
few affected tests.

Rather than just adding a '!', this commit replaces the negative
decls_are_different with a positive decls_are_same. I spent far too
long staring at the code before I spotted the mistake and having
positively-named things improves readability.

The same helper function is also called by has_harmless_name_change
and that should be reviewed as well.

	* src/abg-ir.cc (equals): In the decl_base overload, note that
	the value returned by decl_names_equal should be negated and
	replace decls_are_different with decls_are_same, negating all
	occurrences.
	* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
	Update tests, removing some spurious anonymous union name change.
	* tests/data/test-diff-filter/test33-report-0.txt: Diff now
	completely empty.
	* tests/data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64-multiple-sym-vers-report-0.txt:
	3 functions previously considered to have harmless changes are
	now deemed to have no changes.
	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
	1 struct RedStore data member previously considered to have
	harmless changes is now deemed to have no changes.
	* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi:
	One instance of an anonymous struct removed and a typedef
	repointed at another existing instance; many type ids
	renumbered.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-08-04 17:07:24 +02:00
Giuliano Procida
2862147c0a Make decl_names_equal more accurate
This utility function compares qualified names component-wise with
special handling of components that are anonymous names.

The code is incorrect for some cases when there are different numbers
of components on each side. Also, the control flow in the loop body is
more complex than it needs to be.

This commit simplifies the control flow, fixes the comparison bugs and
adds some extra tests to cover these cases.

	* src/abg-tools-utils.cc (decl_names_equal): Move {l,r}_pos2
	declarations into the loop and make {l,r}_length const. Avoid
	chance of arithmetic on string::npos values. Rework
	logic so there is a single test for "names compare equal" and
	a single test for different numbers of name components.
	* tests/test-tools-utils.cc (main): Add nine more tests.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-08-04 14:51:13 +02:00
Dodji Seketeli
817a2755bc Bug 26261 - Fix logic for canonicalizing DW_TAG_subroutine_type DIEs
In the function compare_dies, the handling of DW_TAG_subroutine_type
was unfinished.

This DIE represents function types pointed-to by function pointer.  It
doesn't represent the type of a "real" function type like the
abigail::ir::function_type would.  The represent more an interface to
a function, which a pointer can point to or which can be used to issue
a call.

So intended idea is that compare_dies compares two DIEs of the
DW_TAG_subroutine_type and DW_TAG_subprogram kind (the later
represents real function definitions) among other kinds, structurally,
but by trying to optimize for speed, for the purpose of canonicalizing
DIEs even before type canonicalization happens at the libabigail IR
level.  This is critical to save space (and time) by doing DIE
de-duplication on huge binaries.

This patch finishes to implement the comparison for
DW_TAG_subroutine_type and comes with a carefully crafted test case
that hits that code path.

	* src/abg-dwarf-reader.cc (compare_dies): Get out early if we are
	are in the middle of a potential recursive comparison of function
	types.  Likewise if we detect that the two function types have
	different textual representations, linkage names, or have a the
	same textual representation, linkage names and are defined in the
	same translation unit.
	* tests/data/test-read-dwarf/PR26261/PR26261-exe: New test binary
	input file.
	* tests/data/test-read-dwarf/PR26261/PR26261-exe.abi: New
	reference test output file.
	* tests/data/test-read-dwarf/PR26261/PR26261-main.c: Source code
	of the binary above.
	* tests/data/test-read-dwarf/PR26261/PR26261-obj{a,b}.{c,h}:
	Likewise.
	* tests/data/test-read-dwarf/PR26261/Makefile: Makefile to
	build the exe out of the source files.
	* tests/data/Makefile.am: Add the new test input files to source
	distribution.
	* tests/test-read-dwarf.cc (in_out_spec): Add the binary test
	input above to the test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-08-03 11:41:08 +02:00
Dodji Seketeli
63cfd6142c dwarf-reader: re-indent a block of code
While looking at something else, I noticed that a block of code in
compare_dies wasn't properly indented.  Fixed thus.

	* src/abg-dwarf-reader.cc (compare_dies): Properly indent a
	sub-block of the big switch case statement in there.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-07-30 17:09:11 +02:00
Giuliano Procida
fd8640dc09 abg-ir.cc: Remove duplicated line of code
The commit

dbef379a Support incomplete enums in core and diff code.

added a duplicated line of code in error. This commit removes it.

	* src/abg-ir.cc (decl_base::set_definition_of_declaration):
	Remove duplicated assignment statement.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-07-28 18:52:57 +02:00
Giuliano Procida
ca956fb48e abg-writer.cc: Fix indentation of XML output
The XML writer produces valid XML. However, it differs in a few
respects from that obtained with xmllint --format.

- there is no XML declaration at the start
- attributes use single quotes rather than double quotes
- indentation is mostly 2 spaces but this is broken in places

This commit fixes the last of these issues as it actually causes
readability issues when examining diffs. It also does this for every
test XML file, whether used as input, compared against output or not
used at all, to match what xmllint --format would do.

	* src/abg-writer.cc (write_canonical_types_of_scope): Do not
	add additional indentation. (write_translation_unit): Pass
	additional indentation to write_canonical_types_of_scope.
	(write_class_decl): Ensure optional annotations of base
	classes have the same indentation as the base classes
	themselves.
	* tests/data/test-annotate/libtest23.so.abi: Fix indentation.
	* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Ditto.
	* tests/data/test-annotate/libtest24-drop-fns.so.abi: Ditto.
	* tests/data/test-annotate/test-anonymous-members-0.o.abi:
	Ditto.
	* tests/data/test-annotate/test1.abi: Ditto.
	* tests/data/test-annotate/test14-pr18893.so.abi: Ditto.
	* tests/data/test-annotate/test15-pr18892.so.abi: Ditto.
	* tests/data/test-annotate/test17-pr19027.so.abi: Ditto.
	* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
	Ditto.
	* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
	Ditto.
	* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Ditto.
	* tests/data/test-annotate/test21-pr19092.so.abi: Ditto.
	* tests/data/test-diff-suppr/test45-abi-wl.xml: Ditto.
	* tests/data/test-diff-suppr/test45-abi.xml: Ditto.
	* tests/data/test-diff-suppr/test46-PR25128-base.xml: Ditto.
	* tests/data/test-diff-suppr/test46-PR25128-new.xml: Ditto.
	* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Ditto.
	* tests/data/test-read-dwarf/libtest23.so.abi: Ditto.
	* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Ditto.
	* tests/data/test-read-dwarf/test1.abi: Ditto.
	* tests/data/test-read-dwarf/test1.hash.abi: Ditto.
	* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Ditto.
	* tests/data/test-read-dwarf/test11-pr18828.so.abi: Ditto.
	* tests/data/test-read-dwarf/test12-pr18844.so.abi: Ditto.
	* tests/data/test-read-dwarf/test14-pr18893.so.abi: Ditto.
	* tests/data/test-read-dwarf/test15-pr18892.so.abi: Ditto.
	* tests/data/test-read-dwarf/test16-pr18904.so.abi: Ditto.
	* tests/data/test-read-dwarf/test17-pr19027.so.abi: Ditto.
	* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/test21-pr19092.so.abi: Ditto.
	* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Ditto.
	* tests/data/test-read-write/test10.xml: Ditto.
	* tests/data/test-read-write/test15.xml: Ditto.
	* tests/data/test-read-write/test21.xml: Ditto.
	* tests/data/test-read-write/test28-without-std-fns-ref.xml:
	Ditto.
	* tests/data/test-read-write/test28-without-std-vars-ref.xml:
	Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-07-28 17:07:03 +02:00
Giuliano Procida
6579ea4d54 Remove ABI XML test data file blank lines
Following a change which eliminated the output of extra blank lines in
XML output, it now also makes sense to remove such blank lines from
saved generated XML files.

This commit does this.

	* tests/data/test-abidiff/test-PR18166-libtirpc.so.abi: Remove
	blank lines.
	* tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0.abi:
	Ditto.
	* tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1.abi:
	Ditto.
	* tests/data/test-diff-suppr/test45-abi-wl.xml: Ditto.
	* tests/data/test-diff-suppr/test45-abi.xml: Ditto.
	* tests/data/test-diff-suppr/test46-PR25128-base.xml: Ditto.
	* tests/data/test-diff-suppr/test46-PR25128-new.xml: Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-07-28 16:21:34 +02:00
Giuliano Procida
2f83b47ba4 abg-ir.cc: Fix incorrect pop of compared types.
In one case there is an attempt to "pop" an in-progress type
comparison which hasn't actually happened.

This commit fixes this.

	* src/abg-ir.cc (equals): In the class_or_union overload,
	replace one instance of RETURN(false) with return false.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-07-28 15:31:52 +02:00
Giuliano Procida
7401923193 Enable Clang's -Werror-overloaded-virtual.
Ths warning is no longer triggered and can be reenabled.

	* configure.ac: Remove the special clause that disabled
	-Werror-overloaded-virtual for Clang builds.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-07-27 18:26:16 +02:00
Giuliano Procida
23f11d70de Fix inheritance of scope_decl::insert_member_decl
The classes class_decl, class_or_union and scope_decl derive from each
other. The method insert_member_decl is declared virtual and defined
in each of these. Unfortunately, it has different argument types in
the base scope_decl class.

Most calls to insert_member_decl are at a statically known class, but
in insert_decl_into_scope the method is called via a scope_decl
pointer. There is the possibility that this could be a type derived
from scope_decl rather than scope_decl itself, in which case the base
method would be called, not as intended.

This commit adjusts the type of the member argument to
scope_decl::insert_member_decl to match the other two classes and
eliminates the last trigger of Clang's -Werror-overloaded-virtual.

	* include/abg-ir.h (scope_decl::insert_member_decl): Change
	type of member argument from const decl_base_sptr& to plain
	decl_base_sptr.
	* src/abg-ir.cc (scope_decl::insert_member_decl): Likewise.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-07-27 18:04:05 +02:00
Dodji Seketeli
38b1476765 Pimpl-ify traversable_base and remove its unused traverse method
The traverse method is defined in the traversable_base class but is
never used as the synonymous methods in the derived classes have
different argument types and so hide it.

It's never used because it's mostly intended as documentation for what
the implementations of this interface should look like.  Namely they
should define a traversable method and its parameter type should
derive from the parameter type of traversable_base::traverse.

But apparently, clang's -Werror-overloaded-virtual is not happy about
this.  It flags it as an error.  It's not.  But hey, let's work-around
it then.

So this patch just comments that method out and document its intent.

To make the change somewhat useful, this patch pimpl-ifies this
abg-traverse.h header file to get us one step closer to some a{b,p}i
stability.  The definitions are moved into abg-traverse.cc.

	* include/abg-traverse.h (traversable_base::priv): Declare new type.
	(traverse_base::priv_sptr): Add pointer to private data
	member.
	(traverse_base::visiting_): Move this data member definition into
	traverse_base::priv.
	(traverse_base::{visiting, traverse_base, ~traverse_base}): Move
	definitions out-of-oline.
	(traverse_base::traverse): Comment out.
	* src/abg-traverse.cc (struct traversable_base::priv): Define new
	type.
	(traversable_base::{traversable_base, ~traversable_base, traverse,
	visiting}): Move these previous inline definitions here.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-07-27 17:50:50 +02:00
Giuliano Procida
93fae877d8 Simplify peel_typedef_pointer_or_reference_type
The second argument to this function controls whether CV qualifiers
should be stripped as well as the elements mentioned in the function
name. While it defaults to true, it is now always passed in as false.

The function contains incomplete code for peeling array types. This is
not needed or indeed wanted by its current callers.

This commits removes the peel_qual_type argument and removes the
associated behaviour. It removes the array peeling code.

There are no functional changes apart from no longer performing early
canonicalisation of certain array types.

I looked at the history of this function to see where the behaviours
originated.

bd161caa Make type_has_non_canonicalized_subtype() tighter

The function is added to help make more types, self-referential ones,
candidates for early canonicalisation.

5822798d Bug 18894 - Fix representation of enumerators in abixml format

This undid the previous change but the function remained.

c20c8c79 Fix infinite loop in peel_typedef_pointer_or_reference_type

As it says, fixing the overload that is currently in use.

6e36a438 Late canonicalize all types that reference classes when reading DWARF

This reintroduced the use of the function to control canonicalisation
by the detection of class types. It also added array peeling to the
function, but in a broken fashion as it would only work for certain
combinations of pointers, references or typedefs referring to arrays.

e9bdb488 Bug 19025 - abixml writer forgets to emit some member types

This added a use of the function in a map key comparison function.

8cc382c8 Fix emitting of referenced type in abixml writer

This undid the previous change.

1bee40c0 Do not forget to peel qualified type off when peeling types

This made the function remove CV qualifiers unconditionally.

e73901a5 Do not mark "distinct" diff nodes as being redundant

This made behaviour to remove CV qualifiers optional and newly added
is_mostly_distinct_diff disabled it.

5d6af8d5 Delay canonicalization for array and qualified types

This change switches maybe_canonicalize_type to not request CV
qualifer peeling from peel_typedef_pointer_or_reference_type.

It partially resolves the array type issue as they are separately
checked for. Presumably they shouldn't be peeled, but still are under
some circumstances.

The tests here could be subject to further refinement. Many types have
delayed canonicalisation already.

9cf76b11 abg-ir.cc: Improve types_have_similar_structure.

This change replaced the use of the function with a more delicate
matched peeling process for pointer and reference types plus
peel_qualified_or_typedef_type. It obsoleted the behaviour where CV
qualifiers were stripped.

	* include/abg-fwd.h (peel_qualified_or_typedef_type): Remove
	second argument in declarations of both overloads.
	* src/abg-comp-filter.cc (is_mostly_distinct_diff): Remove
	second argument to peel_qualified_or_typedef_type.
	* src/abg-dwarf-reader.cc (maybe_canonicalize_type): Likewise.
	* src/abg-ir.cc (peel_qualified_or_typedef_type): In both
	overloads, remove second argument peel_qual_type, simplify
	code with the assumption it was always false and remove
	incomplete array type peeling logic. In type_base_sptr
	overload, remove stray space.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-07-27 15:26:27 +02:00
Giuliano Procida
3b93574beb Remove unused is_reference_or_pointer_diff.
The function is_reference_or_pointer_diff was added in commit

85929105 Fix redundancy marking for change of types used directly

and was updated to peel typedefs as a first step in

ef9d20c9 Fix redundancy detection through fn ptr and typedef paths

which, however, also made it obsolete.

This commit removes the function's declaration and definition.
There are no functional changes.

	* include/abg-comparison.h (is_reference_or_pointer_diff):
	Drop function declaration.
	* src/abg-comparison.cc (is_reference_or_pointer_diff): Drop
	function definition.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-07-27 15:26:04 +02:00
Giuliano Procida
12baf89751 abg-comparison.h: Remove stray declaration
The functon is_reference_or_pointer_diff_to_non_basic_distinct_types
was declared in

ef9d20c9 Fix redundancy detection through fn ptr and typedef paths

but never defined. This commit removes it. There are no functional
changes.

	* include/abg-comparison
	(is_reference_or_pointer_diff_to_non_basic_distinct_types):
	Remove stray declaration.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-07-27 15:26:04 +02:00
Giuliano Procida
0fd2fb93eb abg-ir.cc: Refactor operator== methods with helper function
Many of the operator== definitions in this source file follow the same
pattern:

- First, canonical comparison is attempted if canonical types are
  present.
- Otherwise, the comparison is performed structurally using the
  'equals' function.

This commit refactors the common logic into a templated helper
function named "try_canonical_compare".

There are no behavioural changes.

	* src/abg-ir.cc (try_canonical_compare): New template function.
	(type_decl::operator==): Use it here.
	(scope_type_decl::operator==): Likewise.
	(qualified_type_def::operator==): Likewise.
	(pointer_type_def::operator==): Likewise.
	(reference_type_def::operator==): Likewise.
	(array_type_def::subrange_type::operator==): Likewise.
	(array_type_def::operator==): Likewise.
	(enum_type_decl::operator==): Likewise.
	(typedef_decl::operator==): Likewise.
	(function_type::operator==): Likewise.
	(class_or_union::operator==): Likewise.
	(class_decl::operator==): Likewise.
	(union_decl::operator==): Likewise.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-07-27 15:26:03 +02:00
Giuliano Procida
dfd8314159 abg-comparison.cc: Tidy some corpus_diff code
This commit fixes up some code whitespace for style and consitency,
renames a poorly-named variable and fixes a comment typo.

There are no behavioural changes.

	* src/abg-comparison.cc (corpus_diff::priv::emit_diff_stats):
	Adjust code whitespace; rename the second instance of
	total_nb_variable_changes to
	total_nb_unreachable_type_changes.
	(corpus_diff::has_incompatible_changes): Fix comment typo.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-07-27 11:59:07 +02:00
Giuliano Procida
be0effa6fd Fix --type-id-style hash for empty internal names.
libabigail's intern_string class treats the empty string specially. It
is not safe to call the raw method without checking for a empty
pointer. It is safe to convert to std::string.

This commit changes the XML writer to convert interned strings to
std::strings before computing their hashes.

	* src/abg-writer.cc (write_context::get_id_for_type): When
	hashing internal type names, convert to std::string rather
	than using the raw method directly as this will avoid a null
	pointer dereference in the case of an empty string; tabify
	code indentation.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-07-27 11:23:26 +02:00
Giuliano Procida
96ce0785fd abg-ir.cc: Tidy some operator== definitions
Many of the operator== definitions in this source file follow the same
pattern:

- the address of the argument is dynamic_cast to type of 'this'
- naked canonical type pointers are compared, if both present
- the types are compared structurally with 'equals'

In a couple of cases extra work is done to fetch the canonical type
of the definition of a declaration.

This commit adjusts a few cases so they more closely follow the common
form. This is to make the next refactoring trivial.

There are no behavioural changes.

	* src/abg-irc.cc (scope_type_decl::operator==): Compare naked
	canonical type pointers instead of the shared pointers.
	(qualified_type_def::operator==): Remove excess blank line.
	(function_type::operator==): Do dynamic_cast and check of
	argument before comparing naked canonical type pointers.
	(class_or_union::operator==): Eliminate temporary reference.
	(class_decl::operator==): Likewise.
	(union_decl::operator==): Likewise.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-07-27 09:33:44 +02:00
Giuliano Procida
697347e402 Fix corpus_diff::has_net_changes for --leaf-changes-only mode
This function was not aware of --leaf-changes-only mode.

    - Stats counters for changed variables and types have
      different names in the different modes.
    - Net leaf type changes were not included in leaf mode.

For some inputs, this resulted in abidiff producing an empty report
but returning a non-zero exit status in --leaf-changes-only mode.

For other inputs the combination of both issues still resulted in the
correct return code. This included the following test-abidiff-exit
test cases:

    - test-leaf-peeling
    - test-leaf2
    - test-no-stray-comma

This patch makes corpus_diff::has_net_changes mirror emit_diff_stats,
modulo flags like --non-reachable-types which if absent can still
result in discrepancies between output and return code.

To achieve this in a more maintainable way, the patch introduces a new interface
reporter_base::diff_has_net_changes.  That interface is implemented by
all current reporters.  Each reporter focuses on its own
particularities to provide the required behavious. Then
corpus_diff:has_net_changes just has to invoke
reporter_base::diff_has_net_changes on the reporter that is currently
in used.

The tests below verify that the exit code is zero when all the changes
between the test files are suppressed.

	* include/abg-reporter.h ({reporter_base, default_reporter,
	leaf_reporter}::diff_has_net_changes): Add new virtual function.
	This breaks binary compatibility but should conserve source
	compatibility.
	* src/abg-default-reporter.cc
	(default_reporter::diff_has_net_changes): Define new member
	function.
	* src/abg-leaf-reporter.cc (leaf_reporter::diff_has_net_changes):
	Likewise.
	* src/abg-comparison.cc (corpus_diff::has_net_changes): Invoke
	reporter_base::diff_has_net_changes on the current reporter,
	rather than trying to handle all the different kinds of reporters
	here.
	(corpus_diff::priv::apply_filters_and_compute_diff_stats): Add a
	TODO to possibly delegate the implementation of this function to
	the reporters.
	* tests/data/Makefile.am: Add new test case files.
	* tests/data/test-abidiff-exit/test-net-change-report0.txt:
	Normal mode, nothing suppressed.
	* tests/data/test-abidiff-exit/test-net-change-report1.txt:
	Normal mode, everything suppressed.
	* tests/data/test-abidiff-exit/test-net-change-report2.txt:
	Leaf mode, nothing suppressed.
	* tests/data/test-abidiff-exit/test-net-change-report3.txt:
	Leaf mode, everything suppressions.
	* tests/data/test-abidiff-exit/test-net-change-v0.c: Test file
	* tests/data/test-abidiff-exit/test-net-change-v0.o: Test file
	* tests/data/test-abidiff-exit/test-net-change-v1.c: Test file
	* tests/data/test-abidiff-exit/test-net-change-v1.o: Test file
	* tests/data/test-abidiff-exit/test-net-change.abignore: This
	suppresses changes for all variables, functions and types in
	the test files, except for the 'victim' function.
	* tests/test-abidiff-exit.cc: Run new test cases.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-07-21 08:53:19 +02:00
Giuliano Procida
e0d29a919d reporter: Fix report whitespace typos.
The patch fixes two minor formatting typos.

	* src/abg-reporter-priv.cc (represent): Add missing space to
	string split across two lines in certain anonymous data member
	diffs.
	* src/abg-default-reporter.cc (report): In the array_diff
	overload, eliminate trailing space at end of line.
	* tests/data/test-diff-dwarf/test10-report.txt: Delete
	trailing whitespace.
	* tests/data/test-diff-filter/test24-compatible-vars-report-1.txt:
	Ditto.
	* 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:
	Ditto.
	* tests/data/test-diff-pkg/libcdio-0.94-1.fc26.x86_64--libcdio-0.94-2.fc26.x86_64-report.1.txt:
	Ditto.
	* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt:
	Ditto.
	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
	Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-07-10 15:09:33 +02:00
Giuliano Procida
f0bf868022 abg-writer.cc: Clean up new line emission.
The XML writer has a few different styles of new line handling in
different places. Some functions are responsible for line termination,
others are not and there is bespoke logic and state variables in a few
places.

Extra or missing newlines should have no impact on the semantics of
any given ABI file but they do affect textual diffs and diff
statistics.

By insisting the XML emitted should have exactly one XML tag (or
comment) per line, we can simplify the code and make it more
composable.

This commit does this, yielding a modest reduction in code size and
eliminating all blank lines in XML output (7127 blank lines in current
tests). The commit also fixes some code whitespace.

	* src/abg-writer.cc (annotate): In the
	function_decl::parameter_sptr overload, fix code whitespace.
	(write_decl_in_scope): Remove wrote_context state variable and
	associated logic; emit new line unconditionally after end of
	XML tags and nowhere else.
	(write_canonical_types_of_scope): Emit new line after end of
	XML comment and nowhere else.
	(write_translation_unit): Emit new line after end of XML tags
	and nowhere else.
	(write_type_decl): Likewise.
	(write_namespace_decl): Likewise.
	(write_qualified_type_def): Emit new line after end of XML tag.
	(write_pointer_type_def): Likewise.
	(write_reference_type_def): Likewise.
	(write_array_type_def): Emit new line after end of XML tags
	and nowhere else.
	(write_enum_type_decl): Emit new line after end of XML tag.
	(write_elf_symbol): Likewise.
	(write_elf_symbols_table): Emit no new lines.
	(write_elf_needed): Emit new line unconditionally after end of
	XML tags.
	(write_typedef_decl): Emit new line after end of XML tag.
	(write_var_decl): Emit new line after end of XML tag.
	(write_function_decl): Likewise.
	(write_function_type): Fold two output statements into
	one; emit new line after end of XML tag.
	(write_class_decl_opening_tag): Emit new line unconditionally
	after end of XML tags and simplify empty element tag logic.
	(write_union_decl_opening_tag): Likewise.
	(write_class_decl): Emit new line after end of XML tag and
	nowhere else.
	(write_union_decl): Likewise.
	(write_member_type_opening_tag): Emit new line after end of
	XML tag.
	(write_member_type): Emit new lines only after XML tags.
	(write_type_tparameter): Emit new line after XML tag.
	(write_non_type_tparameter): Likewise.
	(write_template_tparameter): Emit new line after XML tag and
	nowhere else.
	(write_type_composition): Likewise.
	(write_template_parameters): Emit no new lines.
	(write_function_tdecl): Emit new line after XML tag and
	nowhere else.
	(write_class_tdecl): Likewise.
	(write_corpus): Emit new lines only after XML tags.
	(dump): In the decl_base_sptr overload, don't emit final new
	line as this is now done by write_decl. In the var_decl_sptr
	overload, don't emit final new line (mistakenly done to cerr
	instead of o) as this is now done by write_var_decl. In the
	translation_unit overload, don't emit final new line as this
	doubles that emitted by write_translation_unit.
	* tests/data/test-annotate/libtest23.so.abi: Delete all blank
	lines.
	* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Ditto.
	* tests/data/test-annotate/libtest24-drop-fns.so.abi: Ditto.
	* tests/data/test-annotate/test-anonymous-members-0.o.abi:
	Ditto.
	* tests/data/test-annotate/test1.abi: Ditto.
	* tests/data/test-annotate/test13-pr18894.so.abi: Ditto.
	* tests/data/test-annotate/test14-pr18893.so.abi: Ditto.
	* tests/data/test-annotate/test15-pr18892.so.abi: Ditto.
	* tests/data/test-annotate/test17-pr19027.so.abi: Ditto.
	* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
	Ditto.
	* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
	Ditto.
	* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Ditto.
	* tests/data/test-annotate/test21-pr19092.so.abi: Ditto.
	* tests/data/test-annotate/test7.so.abi: Ditto.
	* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Ditto.
	* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Ditto.
	* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
	Ditto.
	* tests/data/test-read-dwarf/libtest23.so.abi: Ditto.
	* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Ditto.
	* tests/data/test-read-dwarf/test1.abi: Ditto.
	* tests/data/test-read-dwarf/test1.hash.abi: Ditto.
	* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Ditto.
	* tests/data/test-read-dwarf/test11-pr18828.so.abi: Ditto.
	* tests/data/test-read-dwarf/test12-pr18844.so.abi: Ditto.
	* tests/data/test-read-dwarf/test13-pr18894.so.abi: Ditto.
	* tests/data/test-read-dwarf/test14-pr18893.so.abi: Ditto.
	* tests/data/test-read-dwarf/test15-pr18892.so.abi: Ditto.
	* tests/data/test-read-dwarf/test16-pr18904.so.abi: Ditto.
	* tests/data/test-read-dwarf/test17-pr19027.so.abi: Ditto.
	* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/test21-pr19092.so.abi: Ditto.
	* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/test7.so.abi: Ditto.
	* tests/data/test-read-dwarf/test7.so.hash.abi: Ditto.
	* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi:
	Ditto.
	* tests/data/test-read-write/test10.xml: Ditto.
	* tests/data/test-read-write/test15.xml: Ditto.
	* tests/data/test-read-write/test21.xml: Ditto.
	* tests/data/test-read-write/test25.xml: Ditto.
	* tests/data/test-read-write/test28-without-std-fns-ref.xml:
	Ditto.
	* tests/data/test-read-write/test28-without-std-vars-ref.xml:
	Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-07-09 19:10:12 +02:00
Dodji Seketeli
3506ba5795 reader: Remove useless support for WIP types
In the abixml reader, WIP types are tracked to know if a type has been
fully constructed yet or not.  This information is later useful to
know if a given type should be canonicalized right away, or if its
canonicalization should be delayed until the entire abixml file has
been read.

Right now, with all the evolutions that happened in the abixml reader,
only scalar types are canonicalized right away.  All other types are
canonicalized late, meaning, after the entire abixml file is read.
This doesn't have any noticeable performance impact because the volume
of types coming from an abixml file is relatively small enough,
compared to what we can see in a DWARF/ELF binary due to type
duplication.

So the whole WIP tracking becomes is now pretty much useless, in
practise.  So this patch does away with it altogether.

	* src/abg-reader.cc (read_context::m_wip_types_map): Remove data
	member.
	(read_context::{clear_wip_classes_map, mark_type_as_wip,
	unmark_type_as_wip, is_wip_type}): Remove member functions.
	(read_context::maybe_canonicalize_type): Remove use of
	is_wip_type.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-07-09 14:44:57 +02:00
Giuliano Procida
8e8e0778f2 Use pointers not strings in type graph comparison.
During structural comparison of types there is the possibilitiy of
infinite recursion as types can have self-references and there can
be more elaborate mutual references between them.

The current comparison algorithm keeps track of currently seen (struct
and function) types by name. This causes earlier caching of names than
is needed and, more significantly, may result in types comparing equal
unexpectedly. This commit switches to storing their addresses instead.

This change affects some tests which show more diffs than previously.

	src/abg-ir.cc: (environment::priv): Change types of
	classes_being_compared_ and fn_types_being_compared_ to be
	simple sets of pointers.
	(function_type::priv::mark_as_being_compared): Just add
	address to set.
	(function_type::priv::unmark_as_being_compared): Just remove
	address from set.
	(function_type::priv::comparison_started): Just look up
	address in set.
	(class_or_union::priv::mark_as_being_compared): Just add
	address to set.
	(class_or_union::priv::unmark_as_being_compared): Just remove
	address from set.
	(class_or_union::priv::comparison_started): Just look up
	address in set.
	* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
	Update.
	* tests/data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64-multiple-sym-vers-report-0.txt:
	Update.
	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
	Update.
	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt:
	Update.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-07-09 10:02:53 +02:00
Giuliano Procida
b0f98180e0 Add tests for declaration-only enums.
* tests/data/test-abidiff-exit/test-decl-enum-report-2.txt: Add
	new test reference output.
	* tests/data/test-abidiff-exit/test-decl-enum-report-3.txt: Likewise.
	* tests/data/test-abidiff-exit/test-decl-enum-report.txt: Likewise.
	* tests/data/test-abidiff-exit/test-decl-enum-v{0,1}.c: Add source
	code for the binaries below.
	* tests/data/test-abidiff-exit/test-decl-enum-v{0,1}.o: Add new
	binary test inputs.
	* tests/data/Makefile.am: Add the new files above to source
	distribution.
	* tests/test-abidiff-exit.cc: Add the test inputs above to the
	test harness.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-07-08 16:59:56 +02:00
Giuliano Procida
6295ff70ef Add declaration-only enums to XML reader/writer.
* src/abg-reader.cc (build_enum_type_decl): Detect a
	declaration-only enum and flag it as such.
	(build_type_decl): Support reading the "is-declaration" attribute.
	(build_class_decl): Adjust.
	* src/abg-writer.cc (write_is_declaration_only): Renamed
	write_class_or_union_is_declaration_only into this.
	(write_enum_is_declaration_only): Remove.
	(write_type_decl, write_enum_type_decl)
	(write_class_decl_opening_tag, write_union_decl_opening_tag): Use
	write_is_declaration_only.
	* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.
	* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
	* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-07-08 16:59:56 +02:00
Dodji Seketeli
9a113ce771 Support declaration-only enums in DWARF reader.
This patch adds declaration-only handling enums to the DWARF reader.

	* src/abg-dwarf-reader.cc (string_enums_map): Define new
	convenience typedef.
	(read_context::decl_only_enums_map_): Define new data member.
	(read_context::{declaration_only_enums,
	is_decl_only_enum_scheduled_for_resolution,
	resolve_declaration_only_enums}): Define new member functions.
	(build_internal_underlying_enum_type_name)
	(build_enum_underlying_type): Factorize these functions out of ...
	(build_enum_type): ... here.  Detect a decl-only enum and flag it
	as such.  If the enum type is decl-only, then set its underlying
	type as decl-only as well.
	(build_enum_underlying_type): Mark the underlying type as
	artificial.
	(get_opaque_version_of_type): Make this handle enums as well.  So
	make its return type be type_or_decl_base_sptr, rather than just
	class_or_union_sptr as it used to be.
	(read_debug_info_into_corpus): Add logging to trace decl-only
	enums resolution.
	(build_ir_node_from_die): Detect when a suppression specification
	makes an enum opaque.  In that case, get an opaque version of the
	enum type by invoking get_opaque_version_of_type.  Note that
	get_opaque_version_of_type doesn't support returning opaque
	-- i.e, decl-only enum types -- yet, but this is going to be
	handled in a subsequent patch.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-07-08 16:59:56 +02:00
Giuliano Procida
dbef379a96 Support incomplete enums in core and diff code.
This is an initial implementation of the support for incomplete, also
known as forward-declared, enum types. I've not made any attempt to
refactor or share logic with the struct/union code.

	* include/abg-comp-filter.h (has_decl_only_def_change) : Declare
	New function.
	* src/abg-comp-filter.cc (there_is_a_decl_only_enum): Define new
	static function and ...
	(type_size_changed): ... use it here.
	(has_decl_only_def_change): Define new function and ...
	(categorize_harm{less, ful}_diff_node): ... use it here.
	* include/abg-fwd.h (enums_type, decl_base_wptr): Declare new
	typedefs.
	(look_through_decl_only_class): Declare new overload for
	class_or_union*.
	(is_compatible_with_enum_type, is_compatible_with_enum_type)
	(look_through_decl_only, lookup_enum_types, lookup_enum_types):
	Declare new functions.
	* include/abg-ir.h (decl_base::{get_is_declaration_only,
	set_is_declaration_only, set_definition_of_declaration,
	get_definition_of_declaration,
	get_naked_definition_of_declaration}): Declare new member
	functions.  They were moved here from the class_or_union class.
	(class_or_union::{get_earlier_declaration,
	set_earlier_declaration, get_definition_of_declaration,
	set_definition_of_declaration,
	get_naked_definition_of_declaration, get_is_declaration_only,
	set_is_declaration_only}): Remove these member functions.
	* src/abg-ir.cc (decl_base::priv::{declaration_,
	definition_of_declaration_, naked_definition_of_declaration_,
	is_declaration_only_}): Define data members.  Moved here from
	class_or_union.
	(decl_base::priv::priv): Adjust to initialize the new data
	members.
	(decl_base::{get_earlier_declaration, set_earlier_declaration,
	get_definition_of_declaration,
	get_naked_definition_of_declaration, get_is_declaration_only,
	set_is_declaration_only, set_definition_of_declaration}): Define
	member functions.
	(operator|): In the overload for (change_kind, change_kind),
	adjust the return type of the call to
	decl_base::get_definition_of_declaration.
	(look_through_decl_only): Define new function.
	(look_through_decl_only_class): Adjust.
	(look_through_decl_only_enum): Likewise.
	(maybe_update_types_lookup_map<class_decl>): Adjust return type of
	call to decl_base::get_definition_of_declaration.
	(types_defined_same_linux_kernel_corpus_public): Use
	look_through_decl_only_class rather than open coding it.
	(class_or_union::priv::{declaration_, definition_of_declaration_,
	naked_definition_of_declaration_, is_declaration_only_}): Remove
	these data members.  They are now carried by decl_base::priv.
	(class_or_union::{g,s}et_alignment_in_bits): Adjust.
	(class_or_union::{g,s}et_size_in_bits): Likewise.
	(class_or_union::operator==): Likewise.
	(equals): Adjust the overload for class_or_union.
	(is_compatible_with_enum_type)
	* src/abg-comparison.cc (try_to_diff<class_decl>): Adjust the
	return type of decl_base::get_definition_of_declaration.
	(leaf_diff_node_marker_visitor::visit_begin): Use
	filtering::has_decl_only_def_change rather than
	filtering::has_class_decl_only_def_change.  Decl-only changes to
	enums (or any other type really) will thus not be recorded as leaf
	changes.
	* src/abg-dwarf-reader.cc (get_scope_for_die): Adjust return type
	of decl_base::get_definition_of_declaration.
	* src/abg-default-reporter.cc (default_reporter::report): Report
	enum decl-only <-> definition changes.
	* src/abg-hash.cc (class_or_union:#️⃣:operator()): In the
	overload for class_or_union& adjust the return type for
	decl_base::get_definition_of_declaration.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-07-08 16:59:56 +02:00
Giuliano Procida
401e3cded9 abg-ir.cc: Remove unused re_canonicalize function.
The function re_canonicalize was added in commit:

  286cadf8 Bug 24430 - Fold away const for array types

but later rendered obsolete by commit:

  5d6af8d5 Delay canonicalization for array and qualified types

This commit removes the function and its associated declarations.

	* include/abg-fwd.h (re_canonicalize): Remove declaration of
	obsolete function.
	* include/abg-ir.h (class {decl_base, type_base}): Remove
	re_canonicalize friend declarations from these classes.
	* src/abg-ir.cc (re_canonicalize): Remove obsolete function.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-07-07 16:26:47 +02:00
Dodji Seketeli
346e88dd76 Bug 26135 - Wrong linkage name causes anonymous classes miscomparison
When comparing decls, the overload of the 'equals' function for
instances of decl_base compares their linkage names.  If they are
different, then the decls are generally considered different.

Class declarations (and definitions) also use the 'equals' function
referred to above.  So when two classes have different linkage names,
they are always considered different.

Now let's consider the case of an anonymous class.  It doesn't have
any user-provided name, by definition.  Libabigail does, however,
assigns it an internal name for various (internal) purposes.  That
internal name is generally ignored for the purpose of (anonymous) type
comparison.  So by design, two anonymous classes can have different
internal anonymous names and yet still happen to be equal.

The root issue in this problem report is that by default, the linkage
name of a class is set to its name.  And when that class is anonymous,
its internal name is used as its linkage name.  Oops.  That leads to
anonymous classes being wrongly considered different.

This patch fixes the issue by providing additional constructors for a
class type to avoid using the internal anonymous name as its linkage
name.

Note that the same issue is present for unions so the patch does the
a similar thing for union types.

Enums are properly handled so we don't need to do anything in that
regard.

For good measure, the patch also adds an assert to
type_base::get_canonical_types_for to ensure that anonymous class or
union types don't have linkage names for now.

	* include/abg-ir.h (class_decl::class_decl): Add two overloads
	that take the "is_anonymous" flag.
	(union_decl::union_decl): Likewise.
	* src/abg-ir.cc (class_decl::class_decl): Define two overloads
	that take the "is_anonymous" flag and set the linkage name
	accordingly.
	(union_decl::union_decl): Likewise.
	(type_base::get_canonical_type_for): Assert that an anonymous
	class or union can't have a linkage name for now.
	* src/abg-dwarf-reader.cc (add_or_update_class_type)
	(add_or_update_union_type): Use a new overload for the constuctor
	of {class, union}_decl and set the "is_anonymous" flag.  Don't use
	decl_base::set_is_anonymous anymore.
	* src/abg-reader.cc (build_class_decl, build_union_decl):
	Likewise.
	* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.
	* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
	* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-07-06 18:26:08 +02:00
Giuliano Procida
dd1c608120 Rename declaration-definition change category.
This patch renames CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY to
TYPE_DECL_ONLY_DEF_CHANGE_CATEGORY.

	* include/abg-comparison.h (TYPE_DECL_ONLY_DEF_CHANGE_CATEGORY):
	Rename CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY into this.
	(EVERYTHING_CATEGORY): In the value of this enumerator, rename
	CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY into
	TYPE_DECL_ONLY_DEF_CHANGE_CATEGORY.
	* src/abg-comp-filter.cc (categorize_harmless_diff_node):
	Likewise.
	* src/abg-comparison.cc (get_default_harmless_categories_bitmap):
	Likewise.
	(operator<<(ostream& o, diff_category c)): Likewise.
	* src/abg-default-reporter.cc (default_reporter::report): Likewise
	in the overload for class_or_union_diff.
	* src/abg-leaf-reporter.cc (leaf_reporter::report): Likewise in
	the overload for class_or_union_diff.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-06-29 18:06:21 +02:00
Giuliano Procida
2b2f073d7a Tidy build_enum_type state variables.
This patch brings the enum code closer to the class/union code, in the
hope that this will ease future code maintenance.

There are no behavioural changes.

	* src/abg-dwarf-reader.cc (build_enum_type): Rename local
	variable enum_is_anonymous to is_anonymous. Move initilisation
	of local variable is_artificial to location corresponding to
	that in add_or_update_class_type and add_or_update_union_type
	functions.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-06-29 11:06:14 +02:00
Giuliano Procida
56db287aba Refactor d.context() as ctxt in report(enum_diff).
* src/abg-default-reporter.cc (report): In the enum_diff
	overload, introduce the name ctxt to replace four occurrences
	of d.context().

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-06-29 10:30:21 +02:00
Giuliano Procida
df47240f7d Improve code comments and whitespace.
These are zero impact changes.

	* include/abg-fwd.h: Correct doc-comment reference to
	enum_type_decl.
	* src/abg-comp-filter.cc: Fix doc-comment syntax.
	* src/abg-comparison.cc (operator<<): In the diff_category
	overload, fix code indentation.
	* src/abg-default-reporter.cc (report): In the
	class_or_union_diff overload, adjust comment to reflect that
	the code is reporting changes between declaration-only and
	defined types, in either direction.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-06-25 17:49:57 +02:00
Giuliano Procida
5f1945b130 get_canonical_type_for: restore environment better
The method type_base::get_canonical_type_for contains some logic which
temporarily changes a couple of control flags in the type's
environment. It then restores these, but not consistently.

This patch ensures the flags are restored unconditionally.

	* src/abg-ir.cc (get_canonical_type_for): Ensure the
	do_on_the_fly_canonicalization and
	decl_only_class_equals_definition flags are restored
	unconditionally.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-06-24 07:31:17 +02:00
Dodji Seketeli
a5d02b95a6 Bug 26127 - abidw --annotate emits incomplete function types
When we get the qualified name of a pointer type, the result is cached
so that subsequent invocations of the getter yields a faster result.

When the pointed-to type is not yet fully constructed at the time of
the first invocation of the getter of the qualified name, what is
cached is the name of the pointer to a non-yet fully qualified type.

Then after the pointed-to type is fully constructed (and
canonicalized), the pointer type also becomes canonicalized (in that
order) and thus, the cache needs to be invalidated so that the
qualified name of the pointer to the fully qualified type is cached
again by a subsequent invocation of the getter.

The problem in this problem report is that the cache doesn't get
invalidated when the pointer type is canonicalized.

This patch fixes that.  A similar issue exists with reference and
qualified types so the patch addresses it for those types as well.

	* include/abg-ir.h (decl_base::clear_qualified_name): Declare new
	protected member function.
	({pointer_type_def, reference_type_def, qualified_type_def,
	function_type}::on_canonical_type_set): Declare virtual member
	functions.
	* src/abg-ir.cc (decl_base::clear_qualified_name): Define new
	protected member function.
	({pointer_type_def, reference_type_def, qualified_type_def,
	function_type}::on_canonical_type_set): Define virtual member
	functions.
	* tests/data/test-annotate/test17-pr19027.so.abi: Adjust.
	* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
	Likewise.
	* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-06-18 15:07:34 +02:00
Giuliano Procida
84fc161a0b Fix bug that suppressed DWARF read tests.
Commit 4252dfd6 added code to allow the ABI write/reread and compare
phases of the tests to be skipped in the case that no ABI files are
given for comparison.

Unfortunately, the new code skipped those phases unconditionally.

This patch changes the in_abi_path and out_abi_path values used in
in_out_specs used to trigger the early termination from "" to NULL and
updates the conditional logic checking them. Several subsequent
commits which affect ABI output were missing these changes to the test
data files.

This change fixes the following list of commits.

4252dfd6 dwarf-reader: handle symtab.section_header.sh_entsize == 0
4457c10e dwarf-reader: handle binaries with missing symtab
34e867e7 dwarf-reader: remove superfluous ABG_ASSERT
2d5389f2 Fix size calculations for multidimensional arrays.
246ca200 corpus/writer: sort emitted translation units by path name
e8bf5b80 Bug 25989 - type_topo_comp doesn't meet irreflexive requirements

Finally, this commit also corrects some bad code formatting.

	* tests/test-read-dwarf.cc (in_out_specs): Use NULL instead of
	empty ABI paths for test25, test26 and test27. (perform):
	Check members of spec, rather than locals with same name, when
	deciding to terminate testing early; fix some code whitespace.
	* tests/data/test-read-dwarf/test14-pr18893.so.abi: Update
	multidimensional array sizes.
	* tests/data/test-read-dwarf/test17-pr19027.so.abi: Ditto.
	* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/test7.so.abi: Ditto.
	* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
	Update following translation unit ordering change.
	* tests/data/test-read-dwarf/test13-pr18894.so.abi: Ditto.
	* tests/data/test-read-dwarf/test14-pr18893.so.abi: Ditto.
	* tests/data/test-read-dwarf/test15-pr18892.so.abi: Ditto.
	* tests/data/test-read-dwarf/test16-pr18904.so.abi: Ditto.
	* tests/data/test-read-dwarf/test17-pr19027.so.abi: Ditto.
	* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/test21-pr19092.so.abi: Ditto.
	* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
	Ditto.
	* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Update
	following code changes affecting ordering of some ABI
	elements.
	* tests/data/test-read-dwarf/test16-pr18904.so.abi

Reviewed-by: Matthias Maennich <maennich@google.com>
Tested-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-06-17 11:27:13 +02:00