Commit Graph

2093 Commits

Author SHA1 Message Date
Dodji Seketeli
5ae7de0cb6 reader: Don't lose anonymous-ness of decl-only classes
When reading an anonymous declaration-only class from ABIXML
libabigail forgets to set the is-anonymous class.  This leads to
spurious change reports when comparing a binary against its ABIXML
representation.  Fixed thus.

Note that this doesn't yet impact any regression test but is useful
for a coming patch that will make abidw --abidiff to emit an error for
all ABI changes, not just the hard incompatible ones.  Without this
change, that coming patch will make runtestreaddwarf to fail.

	* src/abg-reader.cc (build_class_decl): Set the is-anonymous flag
	when reading a decl-only class.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-27 13:53:16 +01:00
Dodji Seketeli
a07fce3669 ir: Introduce internal pretty representation for anonymous classes
There are two views for internal pretty representation of anonymous
classes.

1/ When we look at the anonymous class itself, we use its 'flat
representation' i.e:

    'class {int blah; char bleh;}'

2/ When we look at a pointer or a reference to the anonymous class we
use its generic anonymous internal name, i.e:

   '__anonymous_struct__*'

As a general rule, libabigail always use the keyword 'class' to prefix
the name of classes for internal purposes, independent from the fact
that the type is a struct or a class.  That is a pre-requisite to be
able to canonicalize classes and structs together.  In other words, if
a class and a struct are structurally equal, they are going to be
considered equivalent by the canonicalization process.

Currently however, in the view 1/ of the pretty representation of
anonymous classes, a struct and a class will have different
representations.  For instance, and empty anonymous struct would be
represented as 'struct {}', whereas an empty anonymous class would be
represented as 'class {}'.  This prevents these two be considered
equivalent by the canonicalization process.  This leads to spurious
change reports later down the road.

In the view 2/ we have a similar but different problem: the qualified
names of the anonymous classes are not taken into account when
representing pointer or references to said anonymous classes.  Only
their unqualified generic anonymous internal names are taken into
account in the representation.  This leads to pointers/references to
anonymous classes being wrongly considered equivalent even when they
belong to different namespaces.

This patch corrects the issues related to both views 1/ and 2/.  It
should make libabigail correctly consider some anonymous classes as
equivalent (view 1) and correctly consider pointers/references to
anonymous classes as different when they belong to different
namespaces (view 2).

A number of reference tests are adjusted accordingly.

	* include/abg-fwd.h (get_class_or_union_flat_representation):
	Introduce an "internal" parameter.
	* src/abg-ir.cc (get_class_or_union_flat_representation):
	Introduce an "internal" parameter.  In the flat representation of
	a class for internal purposes, always use the prefix "class" even
	if this is a struct.
	(get_type_name): To build an internal name for a
	reference or pointer to an anonymous type, consider the namespace
	name of said type.
	(equals): In the overload for decl_base, take the namespace name
	of anonymous decls into account when comparing them.
	({var_decl, union_decl}::get_pretty_representation): Adjust calls
	to get_class_or_union_flat_representation to pass a proper
	"internal" argument.
	* src/abg-default-reporter.cc (default_reporter::report): Adjust
	the call to get_class_or_union_flat_representation to pass an
	"internal" argument set to 'false'.
	* tests/data/test-annotate/libtest23.so.abi: Adjust.
	* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
	* tests/data/test-read-dwarf/libtest23.so.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/test9-pr18818-clang.so.abi: Likewise.
	* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-27 13:53:15 +01:00
Dodji Seketeli
35162bd4e3 writer: Emit definitions of declarations when they are present
Libabigail goes a long way to resolve declaration-only classes
to their definitions when it's possible.

The ABIXML writer however sometimes forgets to emit the definition of
such declarations that have been "resolved".

Later, when the binary is compared to its own ABIXML representation,
the reporting engine thus reports that the definition is lost.

This patch fixes that.

	* src/abg-writer.cc (write_class_decl, write_union_decl): Get the
	definition of the declaration if it exists and emit that.
	* tests/data/test-read-dwarf/test13-pr18894.so.abi: Adjust.
	* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
	* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
	* tests/data/test-annotate/test13-pr18894.so.abi: Likewise.
	* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
	* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-27 13:53:15 +01:00
Dodji Seketeli
b553802af8 Bug 26780 - Fix array subrange bounds (de)serialization
When serializing subrange bounds to ABIXML today the writer omits the
lower and upper bounds.  Only the length is emitted.  The reader thus
assumes that the lower bound of an array subrange is always 0.  In
Fortran however, that is not true, as the lower bound is 1.

This patch instructs the writer to emits the lower bound whenever it's
different from zero.  It also emits the upper bound in that case.  The
reader is updated accordingly to take the lower and upper bounds into
account whenever they are present.  It they are not, then the
lower bound is assumed to be zero and the upper bound is deduced from
the length, as was already the case until now.

	* src/abg-reader.cc (build_subrange_type): Read lower-bound
	attribute if present.  Then try to read upper-bound attribute as
	well.  If this is not an infinite subrange assert that the length
	must be equal to the difference between the bounds.
	* src/abg-writer.cc (write_array_subrange_type): Write the
	lower-bound if it's present and not zero.  In that case, write the
	upper-bound as well.
	* tests/data/test-diff-pkg/hdf5-1.10.6-2.fc33.x86_64.rpm: Add new
	binary test input.
	* tests/data/test-diff-pkg/hdf5-debuginfo-1.10.6-2.fc33.x86_64.rpm:
	Likewise.
	* tests/data/test-diff-pkg/hdf5-1.10.6-2.fc33.x86_64.self-check-report-0.txt:
	Add new reference output.
	* tests/data/Makefile.am: Add the new test material above to
	source distribution.
	* tests/test-diff-pkg.cc (in_out_specs): Add the binary test input
	to the set of --self-check tests.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-27 13:50:47 +01:00
Dodji Seketeli
7d5ef25b9c reader: Read array subrange length into an uint64_t
We read subrange length like an integer of type 'int" today.  We
should read it as an uint64_t.  Fixed thus.

	* src/abg-reader.cc (build_subrange_type): Change the type of
	length to uint64_t and read it using strtoull.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-27 13:09:56 +01:00
Dodji Seketeli
6ac6cfb3f6 abipkgdiff: Avoid uncertainty when sorting worker tasks
Worker tasks that have compared the binaries of two packages are
sorted according to the sizes of the binaries they compared.  The
tasks that compared bigger binaries come first.  So the output of
these tasks is always sorted the same.

When two tasks have sorted binaries of the same size, let's sort them
by taking into account the lexicographic order of the binaries names.

	* elf_size_is_greater: Take the name of the binaries into account
	when their size is equal.  Also, assert that all comparison tasks
	have compared binaries.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-25 13:17:21 +01:00
Dodji Seketeli
33223963bd tests/data/test-fedabipkgdiff: Update reference output
Instances of self_compare_task don't wrongly shadow task comparison
arguments anymore.  Instead, they properly use the task comparison
arguments of the parent class compare_task.

So in the sorting function elf_size_is_greater, we now always have
proper elf binaries size.  It follows that the sorting can now yield a
consistent result, always.

It thus appears that the reference output recorded for the
"self-comparison" test of runtestfedabipkgdiffpy3.sh was wrong.  This
patch updates it.

	* tests/data/test-fedabipkgdiff/test7-self-compare-from-fc23-dbus-glib-report-0.txt:
	Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-25 13:15:18 +01:00
Matthias Maennich
18b0693ca1 abipkgdiff: minor cleanups
The recent changes to abipkgdiff triggered some minor clang-tidy
diagnostics that I address in this patch.

 - removed unused using statements
 - cleaned up some mismatches between commented and actual parameter names
 - removed shadowing data members from self_compare_task and construct
   with base class constructor instead.

	* tools/abipkgdiff.cc: remove unused using statements
	(self_compare_task): remove shadowing members and delegate
	construction to base class.

Signed-off-by: Matthias Maennich <maennich@google.com>
2020-11-25 09:08:03 +01:00
Dodji Seketeli
c2ce1a38c6 dwarf-reader: support artificially generated translation units
When GCC 10 artificially generates a translation unit, the path/name
of the translation unit as given by the DW_AT_name attribute of the
translation unit DIE is the string "<artificial>".

Libabigail expects that translation units have unique file paths so
having several artificially generated translation units like this one
with the same name makes hell break loose down the road.

This patch suffixes the name of artificial DIE with their die offset
number to have a unique path name for artificially generated
translation units.

	* configure.ac: Detect if we are running on RPM >= 4.15.  If yes,
	then define the preprocessor macro RPM_4_15.  If that macro is
	defined then test-diff-pkg.cc can support RPMs from Fedora >= 31
	as those are compressed with zstd.  Earlier RPM versions don't
	support that compression scheme.
	* src/abg-dwarf-reader.cc (build_translation_unit_and_add_to_ir):
	Suffix the offset of the translation unit to its name when that
	name is "<artificial>".
	* tests/data/test-diff-pkg/mesa-libGLU-9.0.1-3.fc33.x86_64.rpm:
	New binary test input.
	* tests/data/test-diff-pkg/mesa-libGLU-debuginfo-9.0.1-3.fc33.x86_64.rpm: Likewise.
	* tests/data/test-diff-pkg/mesa-libGLU-9.0.1-3.fc33.x86_64.self-check-report-0.txt:
	New reference output for the binary test input above.
	* tests/data/Makefile.am: Add the new test inputs above to source
	distribution.
	* tests/test-diff-pkg.cc (in_out_specs): Add the binary test
	inputs above to source distribution if we are running on an RPM
	version >= 4.15.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-24 05:04:06 +01:00
Dodji Seketeli
ddead98a51 fedabipkgdiff: make --self-compare use abipkgdiff --self-check
Now that the abipkgdiff program supports the --self-check option to
make it compare the binaries in an RPM against their own ABIXML
representation, make the --self-compare option of fedabipkgdiff use
the abipkgdiff --self-check.

	* tools/fedabipkgdiff (abipkgdiff): If the user provides the
	--self-compare options, generate the abipkgdiff command by using
	the --self-check option.
	(run_abipkgdiff): Each return value of the abipkgidiff runs can be
	negative because they are unsigned values in essence, but as
	python doesn't seem to have a unsigned integer type.  So we need
	to consider the max of the absolute value of the return codes
	here.
	* tests/data/test-fedabipkgdiff/test7-self-compare-from-fc23-dbus-glib-report-0.txt:
	Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-23 17:50:06 +01:00
Dodji Seketeli
edfe5a1dec abipkgdiff: Add a new --self-check option
After the report of
https://sourceware.org/bugzilla/show_bug.cgi?id=26769 it became
apparent that we want to be able compare a binary against its ABIXML
representation, including for cases where the binaries are embedded in
RPM packages.

This patches thus introduces the --self-check option to abipkgdiff so
that it can be invoked like this:

    $ abipkgdiff --self-check --d1 libstdc++-debuginfo-10.2.1-8.fc34.x86_64.rpm --d1 gcc-debuginfo-10.2.1-8.fc34.x86_64.rpm libstdc++-10.2.1-8.fc34.x86_64.rpm

    ==== SELF CHECK SUCCEEDED for 'libstdc++.so.6.0.28'====
    $

With this option, libabigail compares each binary in the RPM against
its own ABIXML representation.

This should hopefully help to write regression tests which have as
sole inputs the links to download the RPMs.  It's also useful to ease
the process of reproducing the issue raised.

This option can now be used, for instance, by the libabigail-selfcheck
program over at https://pagure.io/libabigail-selfcheck.

	* tools/abipkgdiff.cc (options::self_check): Define new data
	member.
	(options::options): Initialize it.
	(display_usage): Add help string for the --self-check option.
	(parse_command): Parse the new --self-check option.
	(extract_deb): Add missing newline.
	(compare): Remove useless white space.
	(compare_to_self, self_compare_prepared_userspace_package)
	(self_compare_prepared_package, compare_to_self): Add new static
	functions.
	(class self_compare_task): Add new class.
	(prepare_package): Add a new overload that takes just one
	parameter.
	(elf_size_is_greater): Don't crash if the args are empty.
	(main): If the --self-check option is given, make sure we have
	just one package in argument.  Use the new compare_to_self
	function to handle the --self-check option.
	* doc/manuals/abipkgdiff.rst: Add documentation for the new
	--self-check option.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-23 17:50:05 +01:00
Dodji Seketeli
cac59a176a Bug 26769 - Fix missing types in abixml output
The symptom of the issue at hand is that sometimes there can be types
missing from the abixml output.  This happens when analysing some C++
code bases.

The core of the issue is the following.  Support we have a type
"struct S" defined somewhere as:

    struct S // #0
    {
      int  dm1;
      char dm2;
    };

    S s;

Suppose that in another translation unit, we have the class 'S' being
extended to add a member type to it:

    struct S // #1
    {
      typedef int dm1_type;
    };

    typedef S::dm1_type Integer;
    Integer something;

When emitting the abixml for the codebase, the definition of the
typedef S::dm1_type can be missing.

Note that in location #1, struct S is considered declaration-only.
It's definition is in another translation unit, in location #0.

So the abixml writer emits the 'struct S' defined in location #0, but
forgets to emit the 'struct S' in #1, which is indirectly used for the
sole purpose of using its member type S::dm1_type.

This patch emits the S::dm1_type type that is mistakenly forgotten
today.

Now that the "struct S" of #1 is also emitted, a tangent problem is
uncovered: S in #0 can be wrongly thought to be equivalent to S in #1,
for ABI purposes
This is because of an ODR-based optimization that is used for C++.
That is, the two struct S can be wrongly considered equivalent just
because they have the same name.  Note that ODR means "One Definition Rule[1]"

This patch removes the ODR-based optimization and thus fixes many of
the issues uncovered by the previous changes.

The patch also uncovered that some non-static variables were sometimes wrongly
being added to the set of exported variables, while libabigail reads
corpora from abixml.  The patch fixes this as well.

[1]: One Definition Rule: https://en.wikipedia.org/wiki/One_Definition_Rule

	* include/abg-corpus.h (corpus::{record_canonical_type,
	lookup_canonical_type}): Remove function declarations.
	* src/abg-corpus-priv.h (corpus::priv::canonical_types_): Remove
	data member.
	* src/abg-corpus.cc (corpus::{record_canonical_type,
	lookup_canonical_type}): Remove functions.
	* src/abg-ir.cc (type_eligible_for_odr_based_comparison): Remove
	static function.
	(type_base::get_canonical_type_for): Don't perform the ODR-based
	optimization for C++ anymore.
	* src/abg-reader.cc
	(read_context&::maybe_add_var_to_exported_decls): Don't add a
	variable that hasn't been added to its scope.  Otherwise, it means
	we added a variable that wasn't yet properly constructed.  Also
	add a new overload for var_decl_sptr&.
	(build_var_decl): Do not add the var to its the set of exported
	declaration before we are sure it has been fully constructed and
	added to the scope it belongs.
	(build_class_decl): Only add *static* data members to the list of
	exported declarations.
	(handle_var_decl): A var decl seen here is a global variable
	declaration.  Add it to the list of exported declarations.
	* src/abg-writer.cc (write_context::decl_only_type_is_emitted):
	Constify parameter.
	(write_translation_unit): Do not forget to emit referenced types
	that were maybe not canonicalized.  Also, avoid using noop_deleter
	when it's not necessary.
	(write_namespace_decl): Do not forget to emit canonicalized types
	that are present in namespaces other than the global namespace.
	* tests/runtestslowselfcompare.sh.in: New test that compares
	libabigail.so against its own ABIXML representation.
	* tests/Makefile.am: Add the new test runtestslowselfcompare.sh to
	source distribution.  This test is too slow to be run during the
	course of 'make check'.  It takes more than 5 minutes on my slow
	box here.  Rather, it can be run using 'make check-self-compare'.
	I plan to run this before releases now.
	* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Adjust.
	* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
	* tests/data/test-annotate/test0.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/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Likewise.
	* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
	* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
	Likewise.
	* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
	* tests/data/test-read-dwarf/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/libtest24-drop-fns-2.so.abi:
	Likewise.
	* tests/data/test-read-dwarf/libtest24-drop-fns.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/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/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/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/test28-without-std-fns-ref.xml:
	Likewise.
	* tests/data/test-read-write/test28-without-std-vars-ref.xml:
	Likewise.
	* tests/data/test-read-write/test6.xml: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-23 13:02:34 +01:00
Dodji Seketeli
de5de7b447 Make sure to canonicalize all types but decl-only classes
While looking at the remaining potential sources of instability in the
generation of type ids at abixml generation time, it occurred to me
that we still had several non canonicalized types in the system.

When a given type T is not canonicalized,
hash_as_canonical_type_or_constant later hashes it as an arbitrary
constant.  So in the ID hash map of the abixml writer, all the
non-canonicalized types will tend to end-up in the same hash map
bucket, so to speak.  More precisely, if T is equivalent to another
type T' which is canonicalized, then T and T' will end-up in different
buckets (in the same hash map) because they will have different hash
values as returned by hash_as_canonical_type_or_constant.  They will
thus end-up having different type-ids because they are in different
buckets.

The solution is to make sure that T is *also* canonicalized.  That
way, if T and T' are equivalent, they'll end-up in the same bucket and
they will have the same type-id.

In other words, all types should be canonicalized now.

The only exception to that rule is declaration-only classes and
unions.  The reason why a declaration-only type 'struct foo' needs to
stay non-canonicalized is that it must equal all the definitions of
'struct foo' that can be found elsewhere.

This patch thus canonicalizes all types that were not still not being
canonicalized.  It also adds an assert in
hash_as_canonical_type_or_constant to ensure that only
declaration-only class_or_union types are not canonicalized.

	* include/abg-fwd.h (is_declaration_only_class_or_union_type):
	Declare new ...
	* src/abg-ir.cc (is_declaration_only_class_or_union_type):
	... function.
	(clone_array): Add the cloned array subrange to
	its scope so that it can later be canonicalized.
	(synthesize_type_from_translation_unit)
	(synthesize_function_type_from_translation_unit): Canonicalize the
	synthesized types.
	(hash_as_canonical_type_or_constant): Ensure that all types are
	canonicalized.
	* src/abg-dwarf-reader.cc (maybe_canonicalize_type): Remove
	useless overload.
	(build_ir_node_for_variadic_parameter_type)
	(schedule_array_tree_for_late_canonicalization): Define new static
	functions.
	(maybe_strip_qualification): Schedule type canonicalization for
	types cloned prior to editing.
	(build_function_type): Use the new
	build_ir_node_for_variadic_parameter_type.  It takes care of
	canonicalizing variadic parameter types.
	(build_function_decl): Canonicalize the function type that is
	created here.
	(build_ir_node_from_die): Use the overload of
	maybe_canonicalize_type which canonicalizes class_or_union nodes
	directly, rather than the one which handles DIE offsets.  The
	latter was used as an optimization to reduce the size of the array
	of types scheduled for canonicalization, as DIE offsets take less
	space than pointers to IR types.  Now that we have DIE
	de-duplication, my bet is that we can do away with the former.
	And that also ensures that we miss no type for canonicalization
	purposes.
	* src/abg-reader.cc (build_array_type_def): Canonicalize the
	subrange types of the array.
	(build_type): Canonicalize all types.
	* tests/data/test-annotate/libtest23.so.abi: Adjust.
	* 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/test0.abi: Likewise.
	* tests/data/test-annotate/test13-pr18894.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/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Likewise.
	* tests/data/test-annotate/test21-pr19092.so.abi: 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-read-dwarf/PR22015-libboost_iostreams.so.abi:
	Likewise.
	* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
	* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.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-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/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/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/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.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-16 09:16:09 +01:00
Dodji Seketeli
90eff56227 ir: Add equality op to array_type_def::subrange_type::bound_value
* include/abg-ir.h
	(array_type_def::subrange_type::bound_value::operator==): Declare
	new ...
	* src/abg-ir.cc
	(array_type_def::subrange_type::bound_value::operator==):
	... equality operator.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-13 09:19:40 +01:00
Dodji Seketeli
2cc1ab7ee8 writer: Sort decls and fix topological sorting for types
When emitting the declarations of a given translation unit, those
declarations are not sorted.  Ooops.  This patch adds topological
sorting for those declarations, making the decls defined first to be
emitted first.  When the decls are defined at the same location then
the pretty representation is used for lexicographic sorting instead.

It turns out that during the topological sorting for types there was
some uncertainty when the declarations of the types had the same
definition location.  This patch re-uses the declaration sorting above
for the declarations of these types.

	* include/abg-ir.h (scope_decl::get_sorted_member_decls): Declare
	new member function.
	* src/abg-ir.cc (struct decl_topo_comp): New sorting functor.
	(type_topo_comp::operator()): Re-use the decl_topo_comp to sort
	type declarations.
	(scope_decl::priv::sorted_members_): Add new data member.
	(scope_decl::get_sorted_member_decls): Define new member function.
	* src/abg-writer.cc (write_translation_unit): Use the new
	scope_decl::get_sorted_member_decls.
	* tests/data/test-annotate/libtest23.so.abi: Adjust.
	* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
	* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
	* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
	Likewise.
	* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
	Likewise.
	* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Likewise.
	* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
	* tests/data/test-read-dwarf/libtest23.so.abi: Likewise.
	* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
	* tests/data/test-read-dwarf/test15-pr18892.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/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-write/test2.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>
2020-11-12 16:37:30 +01:00
Dodji Seketeli
3c87247cb4 Bug PR26739 - Handle qualified typedef array types
CV-qualifiers of a typedef of an array type apply to the elements of
the array.  So this can be transformed into a typedef of array type
which element type is similarly CV-qualified.

That transformation helps avoiding spurious changes that might occur
when comparing the latter form against the former even though both are
equivalent.

This patch performs that transformation, just like we already do for
CV-qualified array types which are transformed into an array of
similarly CV-qualified elements.

Performing that transformation amounts to editing the type of the
array elements.  As those types might be used by other parts of the
type graph (type node sharing), the patch "clones" the type sub-tree
of interest and edits the cloned version.  That way, the shared type
nodes are not edited.  It appears that in the existing version of
maybe_strip_qualification, the transformation of CV-qualified arrays
into arrays of CV-qualified elements was unfortunately editing shared
type nodes.  The patch fixes that and re-works the logic of*
maybe_strip_qualification to make it handle this new case and the
previous one in a somewhat generic manner.

	* include/abg-fwd.h (is_typedef_of_array, clone_array)
	(clone_typedef, clone_qualified_type, clone_array_tree): Declare
	new functions.
	(peel_qualified_or_typedef_type): Declare new overload.
	(is_array_of_qualified_element): Constify the parameter.
	* include/abg-ir.h ({qualified_type,
	typedef}_def::set_underlying_type): Add new member functions.
	(array_type_def::subrange_type::subrange_type): Make constify the
	reference to the underlying type parameter.
	* src/abg-ir.cc (is_array_of_qualified_element): Constify the
	parameter.
	(peel_qualified_or_typedef_type): Define new
	overload for type_base_sptr.
	(clone_typedef_array_qualified_type): Define static function.
	(clone_array clone_typedef, clone_qualified_type)
	(clone_array_tree, is_typedef_of_array): Define new functions.
	(qualified_type_def::get_underlying_type): Rename the return type
	shared_ptr<type_base> into type_base_sptr.
	({typedef, qualified_type}_def::set_underlying_type): Define new
	member function.
	(array_type_def::subrange_type::priv::priv): Initialize the
	'infinite_' data member.
	* src/abg-dwarf-reader.cc (maybe_strip_qualification): Handle
	qualified typedef of arrays.  Merge this with the handling of
	qualified arrays.  Note that before editing the elements of the
	array to make the array (or typedef) qualifier apply to the
	element the sub-tree is cloned to make its type nodes be
	'un-shared'.  This prevents us from editing type nodes that are
	shared by other type expressions.
	* tests/data/test-diff-filter/test-PR26739-report-0.txt: New
	reference test output.
	* tests/data/test-diff-filter/test-PR26739-2-report-0.txt: Likewise.
	* tests/data/test-diff-filter/test-PR26739-v{0,1}.c: Source code
	of new binary test input.
	* tests/data/test-diff-filter/test-PR26739-2-v{0,1}.c: Likewise.
	* tests/data/test-diff-filter/test-PR26739-v{0,1}.o: New binary
	test inputs.
	* tests/data/test-diff-filter/test-PR26739-2-v{0,1}.o: Likewise.
	* tests/data/Makefile.am: Add the new test material above to
	source distribution.
	* tests/test-diff-filter.cc (in_out_specs): Add the test inputs
	above to this harness.
	* tests/data/test-annotate/test15-pr18892.so.abi: Adjust.
	* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
	* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
	Likewise.
	* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
	* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
	* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
	* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
	* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
	* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.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.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-12 10:53:40 +01:00
Dodji Seketeli
baee817d3e Update test-libandroid.so.abi
Update this test reference output as per the commit below:

  b7ad7f2b Bug 26770 - Spurious declaration-only-ness induces spurious type changes

	* tests/data/test-read-dwarf/test-libandroid.so.abi: Update.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-03 08:47:26 +01:00
Matthias Maennich
38e72b262d dwarf-reader: Ignore zero length location expressions from DW_AT_location
Location expressions might occasionally be of length 0. E.g. a reason
for that are thread local variables that do not exactly have a
location to refer to. Compilers/Linkers may choose an empty location
description. E.g.  see the dwarfdump output for the added testcase
based on libandroid.so (from AOSP).

$ dwarfdump libandroid.so|egrep -B9 "gChoreographerE$"

LOCAL_SYMBOLS:
< 1><0x00000022> DW_TAG_namespace
                   DW_AT_name           android
< 2><0x00000027>   DW_TAG_variable
                     DW_AT_name           gChoreographer
                     DW_AT_type           <0x00000b65>
                     DW_AT_decl_file      0x00000003 .../choreographer.cpp
                     DW_AT_decl_line      0x00000059
                     DW_AT_location       len 0x0000: :
                     DW_AT_linkage_name   _ZN7androidL14gChoreographerE

The DW_AT_location is properly read by elfutils' dwarf_location(), but
is not useful for us to proceed with. Hence early exit on this.

	* src/abg-dwarf-reader.cc (die_location_expr): Ignore zero
	  length location expressions.
	* tests/data/Makefile.am: Add new test files.
	* tests/data/test-read-dwarf/test-libandroid.so: New test file.
	* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
	* tests/test-read-dwarf.cc: Add new test case.

Reported-by: Dan Albert <danalbert@google.com>
Reviewed-by: Giuliano Procida <gprocida@google.com>
Cc: Mark Wielaard <mark@klomp.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-02 18:07:01 +01:00
Giuliano Procida
8a0825e319 Improve enum synthetic type names
The ordering of canonical types (in an abi-instr XML element) appears
to be sensitive to the particular C++ library used and the presence of
other threads doing heap allocation. This patch forces distinct
synthetic enum-underlying types to have distinct names, which ensures
deterministic XML output order.

	* src/abg-dwarf-reader.cc
	(build_internal_underlying_enum_type_name): Add a size
	argument (and don't default is_anonymous argument). Append
	size of type to synthetic type name.
	(build_enum_underlying_type): Pass type size to
	build_internal_underlying_enum_type_name.
	* tests/data/test-abidiff-exit/test-decl-enum-report-3.txt:
	Update. Note that there may be an issue with leaf-mode
	reporting of pointer type changes.
	* tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0.abi:
	Regenerate this (catching up with various abidw updates).
	* tests/data/test-annotate/test-anonymous-members-0.o.abi:
	Refresh with new type names.
	* tests/data/test-annotate/test0.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/test20-pr19025-libvtkParallelCore-6.1.so.abi:
	Likewise.
	* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
	* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
	Likewise.
	* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
	* tests/data/test-read-dwarf/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/test0.abi: Likewise.
	* tests/data/test-read-dwarf/test0.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/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.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
2020-11-02 17:37:42 +01:00
Giuliano Procida
32343f8091 Improve and stabilise sort of member functions
The functor virtual_member_function_less_than did not take into
account linkage name which can be the only difference when multiple
destructors with differing mangled names are present.

This change adds a check for linkage names and also flattens the
control flow in the comparison method to make the logic clearer.

Lastly, this change also uses std::stable_sort, in case all that
remains is insertion order.

	* src/abg-ir.cc
	(virtual_member_function_less_than::operator()): Name
	temporaries like offsets and symbols to reduce repetition;
	test each pair of elements (including symbol presence) and
	return immediately if there's a difference; add a comparison
	of linkage name just after comparing symbol names.
	(sort_virtual_member_functions): Use stable_sort instead of
	sort.
	* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
	Update with new ordering of member functions.
	* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
	Likewise.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
2020-11-02 16:27:33 +01:00
Dodji Seketeli
b4ec62929c update-test-output.py: Update syntax
* tests/update-test-output.py: Update syntax for python3.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-01 14:30:00 +01:00
Dodji Seketeli
b7ad7f2b2d Bug 26770 - Spurious declaration-only-ness induces spurious type changes
Sometimes when amending a C++ class to add new members or properties
to it as directed by the DWARF debug information, we can end-up with
discrepancies related to declaration-only-ness.  That is, an instances
of a given type Foo can be wrongly assigned declaration-only-ness that
should have been only carried by another instance Foo.  Then, later,
comparing two pointers to Foo might wrongly lead to spurious reported
changes due to the spurious differences of declaration-only-ness in
two instances of Foo.

By fixing the setting of the declaration-only-ness, especially when
amending a C++ class this patch fixes that spurious change detected.

	* src/abg-dwarf-reader.cc (add_or_update_class_type): When
	creating a class, set declaration-only-ness unconditionally.  When
	updating the class however, only set the declaration-only-ness
	when the current one is not consistent with the size of the class.
	* tests/data/test-annotate/test14-pr18893.so.abi: Adjust.
	* 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-read-dwarf/test14-pr18893.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.
	* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-11-01 14:29:51 +01:00
Matthias Maennich
4f93fed261 dwarf-reader: fix lookup for repeated translation unit paths
When using relative compilation unit paths in DWARF, the lookup for an
existing one was done with an incorrect path. In particular, a '/' was
prefixed to the path regardless of whether the compilation_dir is set.
That led to the instantiation of an additional translation unit that and
failed when adding it to the corpus due to violating translation unit
uniqueness. Fix that by correcting the lookup.

	* src/abg-dwarf-reader.cc(build_translation_unit_and_add_to_ir):
	  Fix lookup for potentially already existing translation units.

Reported-by: Dan Albert <danalbert@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
2020-10-28 17:28:20 +01:00
Giuliano Procida
f8ef1958da Stabilise sort of canonical types
Some types like unnamed-enum-underlying-type are not distinguished by
type_topo_comp. This can result in nondeterministic output and flakey
tests.

While a more complete ordering from type_topo_comp would be nice, the
nondeterminism can reduced by preserving the relative order of
identically-named types.

	* src/abg-ir.cc (scope_decl::get_sorted_canonical_types): Sort
	canonical types with std::stable_sort(..., type_topo_comp()).

Signed-off-by: Giuliano Procida <gprocida@google.com>
Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-10-28 16:23:17 +01:00
Mark Wielaard
67db22f50c Assume subrange bounds types are unsigned if no underlying type is given.
When running abidiff on test34-libjemalloc.so.2-intel-16.0.3 it would
crash in array_type_def::subrange_type::get_length on the ABG_ASSERT
get_upper_bound() >= get_lower_bound(). This was because that file
contained a subrange upper_bound value encoded with data1 or data2
without an underlying type. In that case we assumed the value was
encoded as a signed value which caused some of the upper bounds to
be negative (while the lower bound, which wasn't given, was assumed
to be zero).

	* src/abg-dwarf-reader.cc (build_subrange_type): Default
	is_signed to false.

Signed-off-by: Mark Wielaard <mark@klomp.org>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-10-28 10:26:58 +01:00
Mark Wielaard
ee059a95cc dwarf-reader: get subrange_type bounds signedness from underlying type
This patch was originally submitted by in a comment of a problem
report at https://sourceware.org/bugzilla/show_bug.cgi?id=26684#c10.

When reading the bounds of a subrange_type, we need to know if the
constant value of those bounds is signed or not.

For that, we used to just look at the form of those constant and infer
the signedness from there.

The problem is that the signedness is really determined by the value
of the DW_AT_encoding attribute present on the underlying type of the
subrange_type; the form of the bound is mere implementation detail
that is downstream of the information of carried by the DW_AT_encoding
attribute.

This patch thus does the right thing by looking at the underlying type
of the subrange_type and by doing away with the unnecessary messing
with attribute value forms.

	* src/abg-dwarf-reader.cc (die_attribute_has_form)
	(die_attribute_is_signed, die_attribute_is_unsigned)
	(die_attribute_has_no_signedness): Remove static functions.
	(die_constant_attribute): Add the 'is_signed' parameter.
	(die_address_attribute): Adjust comment.
	(build_subrange_type): Determine signedness of the bounds by
	looking at the DW_AT_encoding attribute of the underlying type.

Signed-off-by: Mark Wielaard <mark@klomp.org>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-10-27 10:27:18 +01:00
Dodji Seketeli
247f50dbf3 abg-tools-utils: Fix comment
* include/abg-tools-utils.h (enum abidiff_status): Fix a comment.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-10-23 10:07:57 +02:00
Dodji Seketeli
8d7ffe3d06 configure: Support ABIGAIL_NO_OPTIMIZATION_DEBUG environment variable
When working in development environments with compiler versions that
might be very bleeding edge (like the Fedora Rawhide distribution) it
might be worthwhile to disable all compiler optimization to have a
better debugging experience.  In practice, I bumped into this need
again and again.

So I am adding this ABIGAIL_NO_OPTIMIZATION_DEBUG environment variable
to basically allow the "-g -O0" combination, if need be.

This patch obviously doesn't change any existing behaviour if the user
doesn't set this newly introduced environment variable.

	* configure.ac: Set the CXXFLAGS and CFLAGS to "-g -O0 -Wall
	-Wextra -Werror" if the ABIGAIL_NO_OPTIMIZATION_DEBUG is set.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-10-14 13:10:08 +02:00
Dodji Seketeli
470e10ff52 Structurally compare the few non-canonicalized types in general
In the abixml writer, we recently stopped using
type_base::dynamic_hash as a slow path to hash a non-canonicalized
type.  Rather, we use an arbitrary constant as a hash for those
non-canonicalized types.  This amounts to using structural comparison
for those types.  The function that implements this hashing scheme is
hash_as_canonical_type_or_constant.

A potential concern for that approach was the possible negative impact
on speed.  As it turns out since the change went in, there was no
noticeable speed impact raised after testing.

In insight, this is understandable as the number of non-canonicalized
types should be extremely reduced now that we canonicalize pretty much
all types.  As far as I understand it at this point, the only types
that would not be canonicalized are unresolved declaration-only
types.  And, all in all, structurally comparing unresolved
declaration-only types should be "fast enough".

If we ever stumble across any other non-canonicalized type, I think
that would be an artifact of a bug that ought to be fixed.

On that basis, I went ahead and used
hash_as_canonical_type_or_constant throughout the code base and did
away with the use of type_base::dynamic_hash for now, until it's
properly audited, regression tested and got ready for the use cases
where it might make sense.

This patch thus makes hash_type use
hash_as_canonical_type_or_constant.  The writer is then back to using
hash_type again, as it used to; but at the same time, it's still using
structural comparison for non-canonilized types.  So is
hash_type_or_decl, which now uses hash_type to hash types, rather than
using type_base::dynamic_hash.  Note that the comparison engine
heavily uses hash_type_or_decl to hash diff nodes.

So with this small patch the comparison engine is now using structural
comparison of non-canonicalized types (and diff nodes), just as the
abixml writer does.

	* include/abg-fwd.h (hash_as_canonical_type_or_constant): Remove
	public declaration of this function.
	* src/abg-hash.cc (type_base::dynamic_hash::operator()): Add a
	comment.
	* src/abg-ir.cc (hash_as_canonical_type_or_constant): Make this
	function static now.
	(hash_type_or_decl): Use hash_type for types.
	* src/abg-writer.cc (type_hasher::operator()): Use hash_type.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-10-14 12:09:17 +02:00
Dodji Seketeli
fd0f3ac341 Fix redundancy detection in the diff graph
A recent change[1] triggered a variation in the ABI changes reported,
depending on the platform.

It turned out the change made the redundancy detector walk more diff
nodes (like inserted/deleted virtual member functions and their
implicit parameter type) and that uncovered several underlying issues
that has been latent for a long time.

First, we were not walking the inserted/deleted virtual member
functions in a deterministic manner at reporting time.  Rather than
walking the unordered maps containing those functions, this patch now
walk them in lexicographic order.  The patch also does something
similar for the changed data members, but this time during the diff
graph analysis.  That order affects how we consider a given type
change to be redundant.

Second, when looking a diff node named N, if another diff node N'
equivalent to N has already been marked redundant (and thus filtered
out already), we were sometimes wrongly failing to detect and mark N
as redundant.  This patch fixes that.

I realized that some code was now unnecessary so I removed it.

A lot of reference output of tests are adjusted by this patch.
Mostly, these were cases we were failing to properly detect (and
filter out) as redundant reports.  So the change reports should
hopefully look more concise and to the point now.

[1] the recent change is this one:
    2f92777d Consider the implicit 'this' parameter when comparing methods

	* src/abg-comparison-priv.h
	(diff_context::priv::last_visited_diff_node_): Remove unnecessary
	data member.
	(class_or_union_diff::priv::sorted_{deleted,inserted}_member_functions_):
	Add new data members.
	(sort_string_member_function_sptr_map): Declare new function.
	* src/abg-comparison.cc (sort_string_member_function_sptr_map):
	Define new function.
	(redundancy_marking_visitor::visit_begin): If the current diff
	node is equivalent to another one that has been already marked
	redundant, then consider the current diff node as redundant as
	well.  Considering the fact an ancestor node has been filtered out
	is now useless because if that's the case then the current
	descendant node wouldn't even be walked at reporting time.  So
	remove the call to diff_has_ancestor_filtered_out.
	(categorize_redundancy): Remove useless call here as well.
	(diff_has_ancestor_filtered_out, diff_has_ancestor_filtered_out)
	(diff_context::{mark_last_diff_visited_per_class_of_equivalence,
	clear_last_diffs_visited_per_class_of_equivalence,
	get_last_visited_diff_of_class_of_equivalence}): Remove
	unnecessary functions.
	(redundancy_marking_visitor::visit_end): Add comment.
	(class_diff::ensure_lookup_tables_populated): Lexicographically
	sort inserted/deleted member functions.
	(class_or_union_diff::chain_into_hierarchy): Chain changed data
	members diff nodes in a sorted manner.
	* src/abg-default-reporter.cc (default_reporter::report): Report
	deleted/inserted member functions in lexicographic order.
	* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt:
	Adjust.
	* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt:
	Likewise.
	* 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:
	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.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-10-12 16:34:56 +02:00
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