Dodji Seketeli [Mon, 30 Nov 2020 04:53:11 +0000 (05:53 +0100)]
dwarf-reader: Bug 26908 - don't crash on empty DW_TAG_partial_unit
Sometimes a DW_TAG_partial_unit (imported via a DW_TAG_imported_unit)
has no children node. That means the the DW_TAG_partial_unit has no
sub-tree.
In those cases, the dwarf-reader crashes when
ctxt.build_die_parent_relations_under tries to record the point at
which the sub-tree (which is non-existent here) of the partial unit is
imported by the DW_TAG_imported_unit DIE.
This patch avoids crashing in those cases. As this problem is
reported on libclang-cpp.so (on Fedora 33) which takes "abidw --abidiff"
five hours and ~ 20GB of ram to complete at this point (on a power7 box)
this patch doesn't have a regression test attached.
* src/abg-dwarf-reader.cc (die_has_children): Define new static
function.
(read_context::build_die_parent_relations_under): Do not try to
instantiate an imported_unit_point type for an imported unit with
no children node.
(imported_unit_point::imported_unit_point): Assert that the
imported die has a sub-tree.
(imported_unit_point::imported_unit_point): Remove useless spaces.
Dodji Seketeli [Fri, 27 Nov 2020 19:51:45 +0000 (20:51 +0100)]
writer: fix off-by-one error in assertion
* src/abg-writer.cc (write_array_subrange_type): Fix off-by-one
error in assertion.
* src/abg-dwarf-reader.cc (build_subrange_type): Assert the length
of the array complies with its bounds.
Dodji Seketeli [Fri, 27 Nov 2020 12:03:38 +0000 (13:03 +0100)]
abipkgdiff: make --self-check to fail on any change against own ABIXML
Now that several subtle causes of spurious ABI change report when
comparing a binary against its own ABIXML have been addressed, this
patch makes 'abipkgdiff --self-check' to fail on any ABI change
reported. That is, harmless changes are not ignored anymore.
* tools/abipkgdiff.cc (compare_to_self): Report *any* ABI change.
Not just the "net" changes.
Dodji Seketeli [Thu, 26 Nov 2020 12:23:03 +0000 (13:23 +0100)]
abidw: make --abidiff report any change against own ABIXML
Sometimes, "abidw --abidiff <binary>" would pass while
"abidw <binary> > abi; abidiff <binary> abi" would fail.
This is because "abidw --abidiff" emits an error only when the
comparison between the binary and its ABIXML representation yields and
incompatible change.
Now that many subtle causes of spurious ABI change report emitted when
comparing a binary against its own ABIXML have been fixed, this patch
makes it so that *any* change would make abidw --abidiff to emit an
error.
* tools/abidw.cc (load_corpus_and_write_abixml): Emit an error
when comparing the binary to its ABIXML representation yields any
change.
Dodji Seketeli [Fri, 27 Nov 2020 11:13:26 +0000 (12:13 +0100)]
dwarf-reader: Avoid having several functions with the same symbol
In the DWARF debug info, a C++ class can be represented by pieces
throughout a given binary.
In this picture, a given virtual member function can be represented
several times; each time in one different piece of the C++ class. In
a given piece of the class, a virtual member function can be
represented with its ELF symbol set. In another one, the same virtual
member function can be represented without that ELF symbol set. And
there can also be pieces of the class that don't have a given virtual
function.
To handle this, the DWARF reader constructs one class from all its
pieces scattered around. It adds each virtual member function to the
class as it comes across them while scanning the DWARF.
Then there is a pass at the end of the process that sets ELF symbols
to the (virtual) member functions that need it.
The problem with that pass is that it sometimes sets the same ELF
symbol to more than one virtual member function. Those virtual member
functions all have the same mangled name that correspond to the ELF
symbol; but only one of them is meant to be associated with the ELF
symbol. In essence, that one is the one that is exported by the ELF
binary.
This patch teaches the pass that sets the ELF symbols of function to
avoid setting the same ELF symbol to more than one function.
The patch also avoids build_function_decl to set symbol to a function
if that symbol was already set to an existing function.
This patch thus fixes a class of issues what arise when comparing a
binary against its own ABIXML representation. Those several functions
having the same ELF symbol would cause spurious changes in that
context.
* src/abg-dwarf-reader.cc
(read_context::symbol_already_belongs_to_a_function): Define new
member function.
(read_context::fixup_functions_with_no_symbols): Use the new
symbol_already_belongs_to_a_function function to avoid setting a
symbol that already belongs to a function.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Likewise.
Dodji Seketeli [Fri, 27 Nov 2020 10:54:13 +0000 (11:54 +0100)]
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.
Dodji Seketeli [Fri, 27 Nov 2020 06:06:48 +0000 (07:06 +0100)]
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.
Dodji Seketeli [Thu, 26 Nov 2020 13:58:31 +0000 (14:58 +0100)]
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.
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.
Dodji Seketeli [Wed, 25 Nov 2020 11:57:47 +0000 (12:57 +0100)]
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.
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.
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.
Dodji Seketeli [Mon, 23 Nov 2020 09:48:01 +0000 (10:48 +0100)]
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.
Dodji Seketeli [Fri, 20 Nov 2020 12:23:03 +0000 (13:23 +0100)]
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.
Dodji Seketeli [Thu, 19 Nov 2020 16:52:08 +0000 (17:52 +0100)]
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:
==== 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.
Dodji Seketeli [Mon, 16 Nov 2020 11:22:05 +0000 (12:22 +0100)]
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.
Dodji Seketeli [Fri, 13 Nov 2020 15:11:48 +0000 (16:11 +0100)]
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.
Dodji Seketeli [Thu, 12 Nov 2020 13:24:14 +0000 (14:24 +0100)]
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.
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.
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).
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>
Giuliano Procida [Thu, 29 Oct 2020 12:20:59 +0000 (12:20 +0000)]
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.
Giuliano Procida [Thu, 29 Oct 2020 12:20:58 +0000 (12:20 +0000)]
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.
Dodji Seketeli [Sat, 31 Oct 2020 10:40:44 +0000 (11:40 +0100)]
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.
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>
Giuliano Procida [Wed, 21 Oct 2020 11:08:14 +0000 (12:08 +0100)]
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()).
Mark Wielaard [Tue, 27 Oct 2020 13:06:03 +0000 (14:06 +0100)]
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>
Mark Wielaard [Fri, 23 Oct 2020 09:14:38 +0000 (11:14 +0200)]
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>
Dodji Seketeli [Tue, 13 Oct 2020 16:15:05 +0000 (18:15 +0200)]
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.
Dodji Seketeli [Tue, 13 Oct 2020 08:20:42 +0000 (10:20 +0200)]
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.
Dodji Seketeli [Mon, 12 Oct 2020 13:38:00 +0000 (15:38 +0200)]
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.
Dodji Seketeli [Thu, 8 Oct 2020 09:25:37 +0000 (11:25 +0200)]
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.
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".
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.
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.
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:
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.
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.
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.
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.
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.
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.
* 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.
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.
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.
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.
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.
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.
Following a change which eliminated the output of extra blank lines in
XML output, it now also makes sense to remove such blank lines from
saved generated XML files.
The classes class_decl, class_or_union and scope_decl derive from each
other. The method insert_member_decl is declared virtual and defined
in each of these. Unfortunately, it has different argument types in
the base scope_decl class.
Most calls to insert_member_decl are at a statically known class, but
in insert_decl_into_scope the method is called via a scope_decl
pointer. There is the possibility that this could be a type derived
from scope_decl rather than scope_decl itself, in which case the base
method would be called, not as intended.
This commit adjusts the type of the member argument to
scope_decl::insert_member_decl to match the other two classes and
eliminates the last trigger of Clang's -Werror-overloaded-virtual.
* include/abg-ir.h (scope_decl::insert_member_decl): Change
type of member argument from const decl_base_sptr& to plain
decl_base_sptr.
* src/abg-ir.cc (scope_decl::insert_member_decl): Likewise.
Pimpl-ify traversable_base and remove its unused traverse method
The traverse method is defined in the traversable_base class but is
never used as the synonymous methods in the derived classes have
different argument types and so hide it.
It's never used because it's mostly intended as documentation for what
the implementations of this interface should look like. Namely they
should define a traversable method and its parameter type should
derive from the parameter type of traversable_base::traverse.
But apparently, clang's -Werror-overloaded-virtual is not happy about
this. It flags it as an error. It's not. But hey, let's work-around
it then.
So this patch just comments that method out and document its intent.
To make the change somewhat useful, this patch pimpl-ifies this
abg-traverse.h header file to get us one step closer to some a{b,p}i
stability. The definitions are moved into abg-traverse.cc.
* include/abg-traverse.h (traversable_base::priv): Declare new type.
(traverse_base::priv_sptr): Add pointer to private data
member.
(traverse_base::visiting_): Move this data member definition into
traverse_base::priv.
(traverse_base::{visiting, traverse_base, ~traverse_base}): Move
definitions out-of-oline.
(traverse_base::traverse): Comment out.
* src/abg-traverse.cc (struct traversable_base::priv): Define new
type.
(traversable_base::{traversable_base, ~traversable_base, traverse,
visiting}): Move these previous inline definitions here.
The second argument to this function controls whether CV qualifiers
should be stripped as well as the elements mentioned in the function
name. While it defaults to true, it is now always passed in as false.
The function contains incomplete code for peeling array types. This is
not needed or indeed wanted by its current callers.
This commits removes the peel_qual_type argument and removes the
associated behaviour. It removes the array peeling code.
There are no functional changes apart from no longer performing early
canonicalisation of certain array types.
I looked at the history of this function to see where the behaviours
originated.
bd161caa Make type_has_non_canonicalized_subtype() tighter
The function is added to help make more types, self-referential ones,
candidates for early canonicalisation.
5822798d Bug 18894 - Fix representation of enumerators in abixml format
This undid the previous change but the function remained.
c20c8c79 Fix infinite loop in peel_typedef_pointer_or_reference_type
As it says, fixing the overload that is currently in use.
6e36a438 Late canonicalize all types that reference classes when reading DWARF
This reintroduced the use of the function to control canonicalisation
by the detection of class types. It also added array peeling to the
function, but in a broken fashion as it would only work for certain
combinations of pointers, references or typedefs referring to arrays.
e9bdb488 Bug 19025 - abixml writer forgets to emit some member types
This added a use of the function in a map key comparison function.
8cc382c8 Fix emitting of referenced type in abixml writer
This undid the previous change.
1bee40c0 Do not forget to peel qualified type off when peeling types
This made the function remove CV qualifiers unconditionally.
e73901a5 Do not mark "distinct" diff nodes as being redundant
This made behaviour to remove CV qualifiers optional and newly added
is_mostly_distinct_diff disabled it.
5d6af8d5 Delay canonicalization for array and qualified types
This change switches maybe_canonicalize_type to not request CV
qualifer peeling from peel_typedef_pointer_or_reference_type.
It partially resolves the array type issue as they are separately
checked for. Presumably they shouldn't be peeled, but still are under
some circumstances.
The tests here could be subject to further refinement. Many types have
delayed canonicalisation already.
This change replaced the use of the function with a more delicate
matched peeling process for pointer and reference types plus
peel_qualified_or_typedef_type. It obsoleted the behaviour where CV
qualifiers were stripped.
* include/abg-fwd.h (peel_qualified_or_typedef_type): Remove
second argument in declarations of both overloads.
* src/abg-comp-filter.cc (is_mostly_distinct_diff): Remove
second argument to peel_qualified_or_typedef_type.
* src/abg-dwarf-reader.cc (maybe_canonicalize_type): Likewise.
* src/abg-ir.cc (peel_qualified_or_typedef_type): In both
overloads, remove second argument peel_qual_type, simplify
code with the assumption it was always false and remove
incomplete array type peeling logic. In type_base_sptr
overload, remove stray space.
The function is_reference_or_pointer_diff was added in commit
85929105 Fix redundancy marking for change of types used directly
and was updated to peel typedefs as a first step in
ef9d20c9 Fix redundancy detection through fn ptr and typedef paths
which, however, also made it obsolete.
This commit removes the function's declaration and definition.
There are no functional changes.
* include/abg-comparison.h (is_reference_or_pointer_diff):
Drop function declaration.
* src/abg-comparison.cc (is_reference_or_pointer_diff): Drop
function definition.
abg-ir.cc: Refactor operator== methods with helper function
Many of the operator== definitions in this source file follow the same
pattern:
- First, canonical comparison is attempted if canonical types are
present.
- Otherwise, the comparison is performed structurally using the
'equals' function.
This commit refactors the common logic into a templated helper
function named "try_canonical_compare".
This commit fixes up some code whitespace for style and consitency,
renames a poorly-named variable and fixes a comment typo.
There are no behavioural changes.
* src/abg-comparison.cc (corpus_diff::priv::emit_diff_stats):
Adjust code whitespace; rename the second instance of
total_nb_variable_changes to
total_nb_unreachable_type_changes.
(corpus_diff::has_incompatible_changes): Fix comment typo.
Fix --type-id-style hash for empty internal names.
libabigail's intern_string class treats the empty string specially. It
is not safe to call the raw method without checking for a empty
pointer. It is safe to convert to std::string.
This commit changes the XML writer to convert interned strings to
std::strings before computing their hashes.
* src/abg-writer.cc (write_context::get_id_for_type): When
hashing internal type names, convert to std::string rather
than using the raw method directly as this will avoid a null
pointer dereference in the case of an empty string; tabify
code indentation.
Many of the operator== definitions in this source file follow the same
pattern:
- the address of the argument is dynamic_cast to type of 'this'
- naked canonical type pointers are compared, if both present
- the types are compared structurally with 'equals'
In a couple of cases extra work is done to fetch the canonical type
of the definition of a declaration.
This commit adjusts a few cases so they more closely follow the common
form. This is to make the next refactoring trivial.
There are no behavioural changes.
* src/abg-irc.cc (scope_type_decl::operator==): Compare naked
canonical type pointers instead of the shared pointers.
(qualified_type_def::operator==): Remove excess blank line.
(function_type::operator==): Do dynamic_cast and check of
argument before comparing naked canonical type pointers.
(class_or_union::operator==): Eliminate temporary reference.
(class_decl::operator==): Likewise.
(union_decl::operator==): Likewise.
This patch makes corpus_diff::has_net_changes mirror emit_diff_stats,
modulo flags like --non-reachable-types which if absent can still
result in discrepancies between output and return code.
To achieve this in a more maintainable way, the patch introduces a new interface
reporter_base::diff_has_net_changes. That interface is implemented by
all current reporters. Each reporter focuses on its own
particularities to provide the required behavious. Then
corpus_diff:has_net_changes just has to invoke
reporter_base::diff_has_net_changes on the reporter that is currently
in used.
The tests below verify that the exit code is zero when all the changes
between the test files are suppressed.
* include/abg-reporter.h ({reporter_base, default_reporter,
leaf_reporter}::diff_has_net_changes): Add new virtual function.
This breaks binary compatibility but should conserve source
compatibility.
* src/abg-default-reporter.cc
(default_reporter::diff_has_net_changes): Define new member
function.
* src/abg-leaf-reporter.cc (leaf_reporter::diff_has_net_changes):
Likewise.
* src/abg-comparison.cc (corpus_diff::has_net_changes): Invoke
reporter_base::diff_has_net_changes on the current reporter,
rather than trying to handle all the different kinds of reporters
here.
(corpus_diff::priv::apply_filters_and_compute_diff_stats): Add a
TODO to possibly delegate the implementation of this function to
the reporters.
* tests/data/Makefile.am: Add new test case files.
* tests/data/test-abidiff-exit/test-net-change-report0.txt:
Normal mode, nothing suppressed.
* tests/data/test-abidiff-exit/test-net-change-report1.txt:
Normal mode, everything suppressed.
* tests/data/test-abidiff-exit/test-net-change-report2.txt:
Leaf mode, nothing suppressed.
* tests/data/test-abidiff-exit/test-net-change-report3.txt:
Leaf mode, everything suppressions.
* tests/data/test-abidiff-exit/test-net-change-v0.c: Test file
* tests/data/test-abidiff-exit/test-net-change-v0.o: Test file
* tests/data/test-abidiff-exit/test-net-change-v1.c: Test file
* tests/data/test-abidiff-exit/test-net-change-v1.o: Test file
* tests/data/test-abidiff-exit/test-net-change.abignore: This
suppresses changes for all variables, functions and types in
the test files, except for the 'victim' function.
* tests/test-abidiff-exit.cc: Run new test cases.
* src/abg-reporter-priv.cc (represent): Add missing space to
string split across two lines in certain anonymous data member
diffs.
* src/abg-default-reporter.cc (report): In the array_diff
overload, eliminate trailing space at end of line.
* tests/data/test-diff-dwarf/test10-report.txt: Delete
trailing whitespace.
* tests/data/test-diff-filter/test24-compatible-vars-report-1.txt:
Ditto.
* tests/data/test-diff-pkg/libICE-1.0.6-1.el6.x86_64.rpm--libICE-1.0.9-2.el7.x86_64.rpm-report-0.txt:
Ditto.
* tests/data/test-diff-pkg/libcdio-0.94-1.fc26.x86_64--libcdio-0.94-2.fc26.x86_64-report.1.txt:
Ditto.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt:
Ditto.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Ditto.
The XML writer has a few different styles of new line handling in
different places. Some functions are responsible for line termination,
others are not and there is bespoke logic and state variables in a few
places.
Extra or missing newlines should have no impact on the semantics of
any given ABI file but they do affect textual diffs and diff
statistics.
By insisting the XML emitted should have exactly one XML tag (or
comment) per line, we can simplify the code and make it more
composable.
This commit does this, yielding a modest reduction in code size and
eliminating all blank lines in XML output (7127 blank lines in current
tests). The commit also fixes some code whitespace.
* src/abg-writer.cc (annotate): In the
function_decl::parameter_sptr overload, fix code whitespace.
(write_decl_in_scope): Remove wrote_context state variable and
associated logic; emit new line unconditionally after end of
XML tags and nowhere else.
(write_canonical_types_of_scope): Emit new line after end of
XML comment and nowhere else.
(write_translation_unit): Emit new line after end of XML tags
and nowhere else.
(write_type_decl): Likewise.
(write_namespace_decl): Likewise.
(write_qualified_type_def): Emit new line after end of XML tag.
(write_pointer_type_def): Likewise.
(write_reference_type_def): Likewise.
(write_array_type_def): Emit new line after end of XML tags
and nowhere else.
(write_enum_type_decl): Emit new line after end of XML tag.
(write_elf_symbol): Likewise.
(write_elf_symbols_table): Emit no new lines.
(write_elf_needed): Emit new line unconditionally after end of
XML tags.
(write_typedef_decl): Emit new line after end of XML tag.
(write_var_decl): Emit new line after end of XML tag.
(write_function_decl): Likewise.
(write_function_type): Fold two output statements into
one; emit new line after end of XML tag.
(write_class_decl_opening_tag): Emit new line unconditionally
after end of XML tags and simplify empty element tag logic.
(write_union_decl_opening_tag): Likewise.
(write_class_decl): Emit new line after end of XML tag and
nowhere else.
(write_union_decl): Likewise.
(write_member_type_opening_tag): Emit new line after end of
XML tag.
(write_member_type): Emit new lines only after XML tags.
(write_type_tparameter): Emit new line after XML tag.
(write_non_type_tparameter): Likewise.
(write_template_tparameter): Emit new line after XML tag and
nowhere else.
(write_type_composition): Likewise.
(write_template_parameters): Emit no new lines.
(write_function_tdecl): Emit new line after XML tag and
nowhere else.
(write_class_tdecl): Likewise.
(write_corpus): Emit new lines only after XML tags.
(dump): In the decl_base_sptr overload, don't emit final new
line as this is now done by write_decl. In the var_decl_sptr
overload, don't emit final new line (mistakenly done to cerr
instead of o) as this is now done by write_var_decl. In the
translation_unit overload, don't emit final new line as this
doubles that emitted by write_translation_unit.
* tests/data/test-annotate/libtest23.so.abi: Delete all blank
lines.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Ditto.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Ditto.
* tests/data/test-annotate/test-anonymous-members-0.o.abi:
Ditto.
* tests/data/test-annotate/test1.abi: Ditto.
* tests/data/test-annotate/test13-pr18894.so.abi: Ditto.
* tests/data/test-annotate/test14-pr18893.so.abi: Ditto.
* tests/data/test-annotate/test15-pr18892.so.abi: Ditto.
* tests/data/test-annotate/test17-pr19027.so.abi: Ditto.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Ditto.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Ditto.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Ditto.
* tests/data/test-annotate/test21-pr19092.so.abi: Ditto.
* tests/data/test-annotate/test7.so.abi: Ditto.
* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
Ditto.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Ditto.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Ditto.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Ditto.
* tests/data/test-read-dwarf/libtest23.so.abi: Ditto.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi:
Ditto.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Ditto.
* tests/data/test-read-dwarf/test1.abi: Ditto.
* tests/data/test-read-dwarf/test1.hash.abi: Ditto.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Ditto.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Ditto.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Ditto.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Ditto.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Ditto.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Ditto.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Ditto.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Ditto.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Ditto.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Ditto.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Ditto.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Ditto.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Ditto.
* tests/data/test-read-dwarf/test7.so.abi: Ditto.
* tests/data/test-read-dwarf/test7.so.hash.abi: Ditto.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi:
Ditto.
* tests/data/test-read-write/test10.xml: Ditto.
* tests/data/test-read-write/test15.xml: Ditto.
* tests/data/test-read-write/test21.xml: Ditto.
* tests/data/test-read-write/test25.xml: Ditto.
* tests/data/test-read-write/test28-without-std-fns-ref.xml:
Ditto.
* tests/data/test-read-write/test28-without-std-vars-ref.xml:
Ditto.
In the abixml reader, WIP types are tracked to know if a type has been
fully constructed yet or not. This information is later useful to
know if a given type should be canonicalized right away, or if its
canonicalization should be delayed until the entire abixml file has
been read.
Right now, with all the evolutions that happened in the abixml reader,
only scalar types are canonicalized right away. All other types are
canonicalized late, meaning, after the entire abixml file is read.
This doesn't have any noticeable performance impact because the volume
of types coming from an abixml file is relatively small enough,
compared to what we can see in a DWARF/ELF binary due to type
duplication.
So the whole WIP tracking becomes is now pretty much useless, in
practise. So this patch does away with it altogether.
* src/abg-reader.cc (read_context::m_wip_types_map): Remove data
member.
(read_context::{clear_wip_classes_map, mark_type_as_wip,
unmark_type_as_wip, is_wip_type}): Remove member functions.
(read_context::maybe_canonicalize_type): Remove use of
is_wip_type.
Giuliano Procida [Fri, 19 Jun 2020 16:39:24 +0000 (17:39 +0100)]
Use pointers not strings in type graph comparison.
During structural comparison of types there is the possibilitiy of
infinite recursion as types can have self-references and there can
be more elaborate mutual references between them.
The current comparison algorithm keeps track of currently seen (struct
and function) types by name. This causes earlier caching of names than
is needed and, more significantly, may result in types comparing equal
unexpectedly. This commit switches to storing their addresses instead.
This change affects some tests which show more diffs than previously.
src/abg-ir.cc: (environment::priv): Change types of
classes_being_compared_ and fn_types_being_compared_ to be
simple sets of pointers.
(function_type::priv::mark_as_being_compared): Just add
address to set.
(function_type::priv::unmark_as_being_compared): Just remove
address from set.
(function_type::priv::comparison_started): Just look up
address in set.
(class_or_union::priv::mark_as_being_compared): Just add
address to set.
(class_or_union::priv::unmark_as_being_compared): Just remove
address from set.
(class_or_union::priv::comparison_started): Just look up
address in set.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
Update.
* tests/data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64-multiple-sym-vers-report-0.txt:
Update.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Update.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt:
Update.
Giuliano Procida [Wed, 10 Jun 2020 11:59:40 +0000 (12:59 +0100)]
Add tests for declaration-only enums.
* tests/data/test-abidiff-exit/test-decl-enum-report-2.txt: Add
new test reference output.
* tests/data/test-abidiff-exit/test-decl-enum-report-3.txt: Likewise.
* tests/data/test-abidiff-exit/test-decl-enum-report.txt: Likewise.
* tests/data/test-abidiff-exit/test-decl-enum-v{0,1}.c: Add source
code for the binaries below.
* tests/data/test-abidiff-exit/test-decl-enum-v{0,1}.o: Add new
binary test inputs.
* tests/data/Makefile.am: Add the new files above to source
distribution.
* tests/test-abidiff-exit.cc: Add the test inputs above to the
test harness.
Giuliano Procida [Wed, 10 Jun 2020 11:59:39 +0000 (12:59 +0100)]
Add declaration-only enums to XML reader/writer.
* src/abg-reader.cc (build_enum_type_decl): Detect a
declaration-only enum and flag it as such.
(build_type_decl): Support reading the "is-declaration" attribute.
(build_class_decl): Adjust.
* src/abg-writer.cc (write_is_declaration_only): Renamed
write_class_or_union_is_declaration_only into this.
(write_enum_is_declaration_only): Remove.
(write_type_decl, write_enum_type_decl)
(write_class_decl_opening_tag, write_union_decl_opening_tag): Use
write_is_declaration_only.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
Dodji Seketeli [Wed, 10 Jun 2020 11:59:38 +0000 (12:59 +0100)]
Support declaration-only enums in DWARF reader.
This patch adds declaration-only handling enums to the DWARF reader.
* src/abg-dwarf-reader.cc (string_enums_map): Define new
convenience typedef.
(read_context::decl_only_enums_map_): Define new data member.
(read_context::{declaration_only_enums,
is_decl_only_enum_scheduled_for_resolution,
resolve_declaration_only_enums}): Define new member functions.
(build_internal_underlying_enum_type_name)
(build_enum_underlying_type): Factorize these functions out of ...
(build_enum_type): ... here. Detect a decl-only enum and flag it
as such. If the enum type is decl-only, then set its underlying
type as decl-only as well.
(build_enum_underlying_type): Mark the underlying type as
artificial.
(get_opaque_version_of_type): Make this handle enums as well. So
make its return type be type_or_decl_base_sptr, rather than just
class_or_union_sptr as it used to be.
(read_debug_info_into_corpus): Add logging to trace decl-only
enums resolution.
(build_ir_node_from_die): Detect when a suppression specification
makes an enum opaque. In that case, get an opaque version of the
enum type by invoking get_opaque_version_of_type. Note that
get_opaque_version_of_type doesn't support returning opaque
-- i.e, decl-only enum types -- yet, but this is going to be
handled in a subsequent patch.
Giuliano Procida [Wed, 10 Jun 2020 11:59:35 +0000 (12:59 +0100)]
Support incomplete enums in core and diff code.
This is an initial implementation of the support for incomplete, also
known as forward-declared, enum types. I've not made any attempt to
refactor or share logic with the struct/union code.
* include/abg-comp-filter.h (has_decl_only_def_change) : Declare
New function.
* src/abg-comp-filter.cc (there_is_a_decl_only_enum): Define new
static function and ...
(type_size_changed): ... use it here.
(has_decl_only_def_change): Define new function and ...
(categorize_harm{less, ful}_diff_node): ... use it here.
* include/abg-fwd.h (enums_type, decl_base_wptr): Declare new
typedefs.
(look_through_decl_only_class): Declare new overload for
class_or_union*.
(is_compatible_with_enum_type, is_compatible_with_enum_type)
(look_through_decl_only, lookup_enum_types, lookup_enum_types):
Declare new functions.
* include/abg-ir.h (decl_base::{get_is_declaration_only,
set_is_declaration_only, set_definition_of_declaration,
get_definition_of_declaration,
get_naked_definition_of_declaration}): Declare new member
functions. They were moved here from the class_or_union class.
(class_or_union::{get_earlier_declaration,
set_earlier_declaration, get_definition_of_declaration,
set_definition_of_declaration,
get_naked_definition_of_declaration, get_is_declaration_only,
set_is_declaration_only}): Remove these member functions.
* src/abg-ir.cc (decl_base::priv::{declaration_,
definition_of_declaration_, naked_definition_of_declaration_,
is_declaration_only_}): Define data members. Moved here from
class_or_union.
(decl_base::priv::priv): Adjust to initialize the new data
members.
(decl_base::{get_earlier_declaration, set_earlier_declaration,
get_definition_of_declaration,
get_naked_definition_of_declaration, get_is_declaration_only,
set_is_declaration_only, set_definition_of_declaration}): Define
member functions.
(operator|): In the overload for (change_kind, change_kind),
adjust the return type of the call to
decl_base::get_definition_of_declaration.
(look_through_decl_only): Define new function.
(look_through_decl_only_class): Adjust.
(look_through_decl_only_enum): Likewise.
(maybe_update_types_lookup_map<class_decl>): Adjust return type of
call to decl_base::get_definition_of_declaration.
(types_defined_same_linux_kernel_corpus_public): Use
look_through_decl_only_class rather than open coding it.
(class_or_union::priv::{declaration_, definition_of_declaration_,
naked_definition_of_declaration_, is_declaration_only_}): Remove
these data members. They are now carried by decl_base::priv.
(class_or_union::{g,s}et_alignment_in_bits): Adjust.
(class_or_union::{g,s}et_size_in_bits): Likewise.
(class_or_union::operator==): Likewise.
(equals): Adjust the overload for class_or_union.
(is_compatible_with_enum_type)
* src/abg-comparison.cc (try_to_diff<class_decl>): Adjust the
return type of decl_base::get_definition_of_declaration.
(leaf_diff_node_marker_visitor::visit_begin): Use
filtering::has_decl_only_def_change rather than
filtering::has_class_decl_only_def_change. Decl-only changes to
enums (or any other type really) will thus not be recorded as leaf
changes.
* src/abg-dwarf-reader.cc (get_scope_for_die): Adjust return type
of decl_base::get_definition_of_declaration.
* src/abg-default-reporter.cc (default_reporter::report): Report
enum decl-only <-> definition changes.
* src/abg-hash.cc (class_or_union::hash::operator()): In the
overload for class_or_union& adjust the return type for
decl_base::get_definition_of_declaration.
Dodji Seketeli [Wed, 24 Jun 2020 16:16:40 +0000 (18:16 +0200)]
Bug 26135 - Wrong linkage name causes anonymous classes miscomparison
When comparing decls, the overload of the 'equals' function for
instances of decl_base compares their linkage names. If they are
different, then the decls are generally considered different.
Class declarations (and definitions) also use the 'equals' function
referred to above. So when two classes have different linkage names,
they are always considered different.
Now let's consider the case of an anonymous class. It doesn't have
any user-provided name, by definition. Libabigail does, however,
assigns it an internal name for various (internal) purposes. That
internal name is generally ignored for the purpose of (anonymous) type
comparison. So by design, two anonymous classes can have different
internal anonymous names and yet still happen to be equal.
The root issue in this problem report is that by default, the linkage
name of a class is set to its name. And when that class is anonymous,
its internal name is used as its linkage name. Oops. That leads to
anonymous classes being wrongly considered different.
This patch fixes the issue by providing additional constructors for a
class type to avoid using the internal anonymous name as its linkage
name.
Note that the same issue is present for unions so the patch does the
a similar thing for union types.
Enums are properly handled so we don't need to do anything in that
regard.
For good measure, the patch also adds an assert to
type_base::get_canonical_types_for to ensure that anonymous class or
union types don't have linkage names for now.
* include/abg-ir.h (class_decl::class_decl): Add two overloads
that take the "is_anonymous" flag.
(union_decl::union_decl): Likewise.
* src/abg-ir.cc (class_decl::class_decl): Define two overloads
that take the "is_anonymous" flag and set the linkage name
accordingly.
(union_decl::union_decl): Likewise.
(type_base::get_canonical_type_for): Assert that an anonymous
class or union can't have a linkage name for now.
* src/abg-dwarf-reader.cc (add_or_update_class_type)
(add_or_update_union_type): Use a new overload for the constuctor
of {class, union}_decl and set the "is_anonymous" flag. Don't use
decl_base::set_is_anonymous anymore.
* src/abg-reader.cc (build_class_decl, build_union_decl):
Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
Giuliano Procida [Mon, 29 Jun 2020 15:24:34 +0000 (17:24 +0200)]
Rename declaration-definition change category.
This patch renames CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY to
TYPE_DECL_ONLY_DEF_CHANGE_CATEGORY.
* include/abg-comparison.h (TYPE_DECL_ONLY_DEF_CHANGE_CATEGORY):
Rename CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY into this.
(EVERYTHING_CATEGORY): In the value of this enumerator, rename
CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY into
TYPE_DECL_ONLY_DEF_CHANGE_CATEGORY.
* src/abg-comp-filter.cc (categorize_harmless_diff_node):
Likewise.
* src/abg-comparison.cc (get_default_harmless_categories_bitmap):
Likewise.
(operator<<(ostream& o, diff_category c)): Likewise.
* src/abg-default-reporter.cc (default_reporter::report): Likewise
in the overload for class_or_union_diff.
* src/abg-leaf-reporter.cc (leaf_reporter::report): Likewise in
the overload for class_or_union_diff.
Giuliano Procida [Wed, 10 Jun 2020 11:59:33 +0000 (12:59 +0100)]
Tidy build_enum_type state variables.
This patch brings the enum code closer to the class/union code, in the
hope that this will ease future code maintenance.
There are no behavioural changes.
* src/abg-dwarf-reader.cc (build_enum_type): Rename local
variable enum_is_anonymous to is_anonymous. Move initilisation
of local variable is_artificial to location corresponding to
that in add_or_update_class_type and add_or_update_union_type
functions.
Giuliano Procida [Wed, 10 Jun 2020 11:59:31 +0000 (12:59 +0100)]
Improve code comments and whitespace.
These are zero impact changes.
* include/abg-fwd.h: Correct doc-comment reference to
enum_type_decl.
* src/abg-comp-filter.cc: Fix doc-comment syntax.
* src/abg-comparison.cc (operator<<): In the diff_category
overload, fix code indentation.
* src/abg-default-reporter.cc (report): In the
class_or_union_diff overload, adjust comment to reflect that
the code is reporting changes between declaration-only and
defined types, in either direction.
The method type_base::get_canonical_type_for contains some logic which
temporarily changes a couple of control flags in the type's
environment. It then restores these, but not consistently.
This patch ensures the flags are restored unconditionally.
* src/abg-ir.cc (get_canonical_type_for): Ensure the
do_on_the_fly_canonicalization and
decl_only_class_equals_definition flags are restored
unconditionally.
Dodji Seketeli [Thu, 18 Jun 2020 11:29:42 +0000 (13:29 +0200)]
Bug 26127 - abidw --annotate emits incomplete function types
When we get the qualified name of a pointer type, the result is cached
so that subsequent invocations of the getter yields a faster result.
When the pointed-to type is not yet fully constructed at the time of
the first invocation of the getter of the qualified name, what is
cached is the name of the pointer to a non-yet fully qualified type.
Then after the pointed-to type is fully constructed (and
canonicalized), the pointer type also becomes canonicalized (in that
order) and thus, the cache needs to be invalidated so that the
qualified name of the pointer to the fully qualified type is cached
again by a subsequent invocation of the getter.
The problem in this problem report is that the cache doesn't get
invalidated when the pointer type is canonicalized.
This patch fixes that. A similar issue exists with reference and
qualified types so the patch addresses it for those types as well.
* include/abg-ir.h (decl_base::clear_qualified_name): Declare new
protected member function.
({pointer_type_def, reference_type_def, qualified_type_def,
function_type}::on_canonical_type_set): Declare virtual member
functions.
* src/abg-ir.cc (decl_base::clear_qualified_name): Define new
protected member function.
({pointer_type_def, reference_type_def, qualified_type_def,
function_type}::on_canonical_type_set): Define virtual member
functions.
* tests/data/test-annotate/test17-pr19027.so.abi: Adjust.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
Giuliano Procida [Thu, 11 Jun 2020 19:18:53 +0000 (20:18 +0100)]
Fix bug that suppressed DWARF read tests.
Commit 4252dfd6 added code to allow the ABI write/reread and compare
phases of the tests to be skipped in the case that no ABI files are
given for comparison.
Unfortunately, the new code skipped those phases unconditionally.
This patch changes the in_abi_path and out_abi_path values used in
in_out_specs used to trigger the early termination from "" to NULL and
updates the conditional logic checking them. Several subsequent
commits which affect ABI output were missing these changes to the test
data files.
This change fixes the following list of commits.
4252dfd6 dwarf-reader: handle symtab.section_header.sh_entsize == 0 4457c10e dwarf-reader: handle binaries with missing symtab 34e867e7 dwarf-reader: remove superfluous ABG_ASSERT 2d5389f2 Fix size calculations for multidimensional arrays. 246ca200 corpus/writer: sort emitted translation units by path name e8bf5b80 Bug 25989 - type_topo_comp doesn't meet irreflexive requirements
Finally, this commit also corrects some bad code formatting.
* tests/test-read-dwarf.cc (in_out_specs): Use NULL instead of
empty ABI paths for test25, test26 and test27. (perform):
Check members of spec, rather than locals with same name, when
deciding to terminate testing early; fix some code whitespace.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Update
multidimensional array sizes.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Ditto.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Ditto.
* tests/data/test-read-dwarf/test7.so.abi: Ditto.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Update following translation unit ordering change.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Ditto.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Ditto.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Ditto.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Ditto.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Ditto.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Ditto.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Ditto.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Ditto.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Ditto.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Ditto.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Update
following code changes affecting ordering of some ABI
elements.
* tests/data/test-read-dwarf/test16-pr18904.so.abi
on std::weak_ptr<T> can be simplified by using std::weak_ptr<T>::lock.
Since weak_ptr::lock does this atomically, this patch also addresses
potential data races between the call to expired() and the construction
based on the assumption that the shared_ptr is still around.
Hence, apply this simplification/fix to the code base.
Giuliano Procida [Tue, 16 Jun 2020 12:12:10 +0000 (13:12 +0100)]
Fix leaf-mode formatting of decl <-> defn diffs.
This commit adds a missing newline.
* src/abg-leaf-reporter.cc (leaf_reporter::report): In the
class_or_union_diff overload, add a terminating new line after
reporting a declaration-only <-> definition difference.
Giuliano Procida [Mon, 15 Jun 2020 21:07:41 +0000 (22:07 +0100)]
abg-writer: Add support for stable hash type ids.
The type ids currently emitted by the XML writer are simply type-id-1,
type-id-2 etc. Additions or removals of types early in this sequence
result in cascading changes to many other XML elements.
This commit adds support for stable type ids in the form of hashes of
libabigail's internal type names. On fairly rare occasions (typically
involving unnamed types), the names of two distinct types can be the
same. In any case, if there is a hash collision the XML writer will
find the next unused id and so preserve uniqueness.
Diffs between large XML files produced using --type-id-style hash will
be much smaller and easier to review.
This also commit adds some test cases to verify that the hashing is
actually stable across architectures.
* doc/manuals/abidw.rst: Replace stray documentation of
--named-type-ids with documention of new --type-id-style
option.
* include/abg-writer.h (type_id_style_kind): Add new enum.
(set_type_id_style): Add new write_context setter.
(set_common_options): Set type id style in write context.
* include/abg-hash.h (fnv_hash): Declare new 32-bit FNV-1a
hash function in abigail::hashing namespace.
* src/abg-hash.h (fnv_hash): Define new 32-bit FNV-1a hash
function in abigail::hashing namespace.
* src/abg-writer.cc (write_context): Add m_type_id_style
member to record type style to use, defaulting to
SEQUENCE_TYPE_ID_STYLE; add m_used_type_id_hashes to record
already-used hashes.
(write_context::get_type_id_style): Add new getter.
(write_context::set_type_id_style): Add new setter.
(get_id_for_type): Add support for HASH_TYPE_ID_STYLE style.
(set_type_id_style): Add new helper function.
* tools/abidw.cc (options): Add type_id_style member.
(display_usage): Add description of --type-id-style option.
(parse_command_line): Parse --type-id-style option.
* tests/data/Makefile.am: Add new hash type id ABI files.
* tests/test-read-dwarf.cc: (InOutSpec): Add type_id_style
member.
(in_out_specs): Set type_id_style to SEQUENCE_TYPE_ID_STYLE in
existing test specifications. Duplicate first 9 test cases
with type_id_style set to HASH_TYPE_ID_STYLE.
* tests/data/test-read-dwarf/test0.hash.abi: New ABI XML file
with hash type ids.
* tests/data/test-read-dwarf/test1.hash.abi: Ditto.
* tests/data/test-read-dwarf/test2.so.hash.abi: Ditto.
* tests/data/test-read-dwarf/test3.so.hash.abi: Ditto.
* tests/data/test-read-dwarf/test4.so.hash.abi: Ditto.
* tests/data/test-read-dwarf/test5.o.hash.abi: Ditto.
* tests/data/test-read-dwarf/test6.so.hash.abi: Ditto.
* tests/data/test-read-dwarf/test7.so.hash.abi: Ditto.
* tests/data/test-read-dwarf/test8-qualified-this-pointer.so.hash.abi:
Ditto.
Giuliano Procida [Fri, 12 Jun 2020 10:17:01 +0000 (11:17 +0100)]
Eliminate non-ASCII characters.
There are few non-ASCII characters in various code comments. A few are
typos and the rest have obvious ASCII equivalents. This commit
replaces them all with ASCII characters.
* include/abg-diff-utils.h: Replace "’’" with "'".
* src/abg-elf-helpers.cc: Replace "⋅" with ".".
* src/abg-ini.cc: Replace "@êef" with "@ref".
* src/abg-ir.cc: Ditto.
* src/abg-tools-utils.cc: Replace "–" with "-".
dwarf-reader: get_die_source: always initialize return value
GCC9 with ABIGAIL_DEVEL=1 and ABIGAIL_DEBUG=1 set, regularly emits
-Werror=maybe-uninitialized for the values gathered by get_die_source.
As a counter measure, some of them were initialized before the call to
NO_DEBUG_INFO_DIE_SOURCE, but not all of them, leading to said warning.
In order to overcome this, let get_die_source always initialize the
source to NO_DEBUG_INFO_DIE_SOURCE and adjust the caller to consistently
not do that anymore. This solves the warning and maybe but unlikely (due
to the ABG_ASSERT) avoids some UB.
Changing the interface of get_die_source to return the source directly,
lead to the ability to initialize the value const and to defer the
assertion to the function itself. Some occurrences could be removed
entirely as the die_source was not used at all.
abg-reporter: fully qualify std::string and std::ostream
In order to not unnecessarily pollute the abigail namespace and to avoid
ambiguity at compile time, fully qualify std::string and std::string.
This also resolves the issue that include/abg-reporter.h could not be
compiled standalone and therefore was broken as a public header. It
required a `using std::string` from a prior include.
* include/abg-reporter.h (reporter_base::report) fully qualify
std::string and std::ostream for all overloads.
(default_reporter::report_local_typedef_changes): Likewise.
(default_reporter::report_local_qualified_type_changes): Likewise.
(default_reporter::report_local_reference_type_changes): Likewise.
(default_reporter::report_local_function_type_changes): Likewise.
(default_reporter::report): Likewise.
(leaf_reporter::report_changes_from_diff_maps): Likewise.
(leaf_reporter::report): Likewise.
Suggested-by: Mark Barolak <mbar@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
The enumerator HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY has a spelling
mistake in it. This changes fixes this and also corrects the spelling
of both this and HARMLESS_UNION_CHANGE_CATEORY when output.
* include/abg-comparison.h (enum diff_category): Rename
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY enumerator to
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEGORY.
* src/abg-comp-filter.cc (categorize_harmless_diff_node): Replace
use of HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY enumerator with
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEGORY.
* src/abg-comparison.cc (get_default_harmless_categories_bitmap):
Replace use of HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY enumerator
with HARMLESS_SYMBOL_ALIAS_CHANGE_CATEGORY.
(operator<<): In the diff_category overload, replace use of
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY enumerator with
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEGORY and output correct
spelling for both this and HARMLESS_UNION_CHANGE_CATEGORY.
* tools/abicompat.cc (create_diff_context): Replace use of
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY enumerator with
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEGORY.
dwarf-reader: read_context: drop unused symbol versioning code
Since the symbol versioning helper code has been moved to elf_helpers in
commit 9815efe7a5be ("abg-elf-helpers: move some versioning helpers
from abg-dwarf-reader"), remove the leftover unused code from
read_context.
* src/abg-dwarf-reader.cc
(read_context::versym_section_): Delete.
(read_context::verdef_section_): Delete.
(read_context::verneed_section_): Delete.
(read_context::symbol_versionning_sections_loaded_): Delete.
(read_context::symbol_versionning_sections_found_): Delete.
(read_context::initialize): Remove initializations of deleted members.
(read_context::get_symbol_versionning_sections): Delete.
(read_context::get_version_for_symbol): Delete.
(read_context::lookup_elf_symbol_from_index): Use
get_version_for_symbol provided by elf_helpers.
Cleanup code that is never used to reduce overall compiled code and
complexity. Some are leftovers from prior refactorings, some are unused
overloads where the more suitable overload is still left.
Add (Catch based) test suite to test kernel symbol table (ksymtab)
reading through the result persisted in abigail::corpus.
The test cases are created through simple kernel module C source files
targeting the desired properties ((gpl) exported / local functions /
variables). The kernel binaries are built without debug information to
keep them reasonably small and reproducible. That is just enough
sufficient to analyze ksymtabs. The Makefile that comes with them
recreates the test cases from the sources, given a kernel source tree
with the appropriate version, e.g.
make KDIR=/path/to/4.14
This covers ksymtab reading and ensuring we detect the kernel binary
correctly. The kernel versions are selected based on features introduced
into the Linux kernel that affect the ksymtab representation.
- Linux v4.14 - reasonably old kernel to start with (actually v4.14.180)
- Linux v4.19 - first version having position relative relocations
(PREL) in ksymtab entries on some platforms
(actually v4.19.123)
- Linux v5.4 - first version having symbol namespaces (actually v5.4.41)
- Linux v5.6 - latest released stable kernel as of writing (actually v5.6.13)
* tests/data/Makefile.am: add new test data for runtestsymtab
* tests/data/test-symtab/kernel-4.14/Makefile: New test case makefile.
* tests/data/test-symtab/kernel-4.14/empty.c: Likewise.
* tests/data/test-symtab/kernel-4.14/one_of_each.c: Likewise.
* tests/data/test-symtab/kernel-4.14/single_function.c: Likewise.
* tests/data/test-symtab/kernel-4.14/single_function_gpl.c: Likewise.
* tests/data/test-symtab/kernel-4.14/single_variable.c: Likewise.
* tests/data/test-symtab/kernel-4.14/single_variable_gpl.c: Likewise.
* tests/data/test-symtab/kernel-4.14/empty.ko: New test data.
* tests/data/test-symtab/kernel-4.14/one_of_each.ko: Likewise.
* tests/data/test-symtab/kernel-4.14/single_function.ko: Likewise.
* tests/data/test-symtab/kernel-4.14/single_function_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-4.14/single_variable.ko: Likewise.
* tests/data/test-symtab/kernel-4.14/single_variable_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-4.19/Makefile: New test case makefile.
* tests/data/test-symtab/kernel-4.19/empty.c: Likewise.
* tests/data/test-symtab/kernel-4.19/one_of_each.c: Likewise.
* tests/data/test-symtab/kernel-4.19/single_function.c: Likewise.
* tests/data/test-symtab/kernel-4.19/single_function_gpl.c: Likewise.
* tests/data/test-symtab/kernel-4.19/single_variable.c: Likewise.
* tests/data/test-symtab/kernel-4.19/single_variable_gpl.c: Likewise.
* tests/data/test-symtab/kernel-4.19/empty.ko: New test data.
* tests/data/test-symtab/kernel-4.19/one_of_each.ko: Likewise.
* tests/data/test-symtab/kernel-4.19/single_function.ko: Likewise.
* tests/data/test-symtab/kernel-4.19/single_function_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-4.19/single_variable.ko: Likewise.
* tests/data/test-symtab/kernel-4.19/single_variable_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-5.4/Makefile: New test case makefile.
* tests/data/test-symtab/kernel-5.4/empty.c: Likewise.
* tests/data/test-symtab/kernel-5.4/one_of_each.c: Likewise.
* tests/data/test-symtab/kernel-5.4/single_function.c: Likewise.
* tests/data/test-symtab/kernel-5.4/single_function_gpl.c: Likewise.
* tests/data/test-symtab/kernel-5.4/single_variable.c: Likewise.
* tests/data/test-symtab/kernel-5.4/single_variable_gpl.c: Likewise.
* tests/data/test-symtab/kernel-5.4/empty.ko: New test data.
* tests/data/test-symtab/kernel-5.4/one_of_each.ko: Likewise.
* tests/data/test-symtab/kernel-5.4/single_function.ko: Likewise.
* tests/data/test-symtab/kernel-5.4/single_function_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-5.4/single_variable.ko: Likewise.
* tests/data/test-symtab/kernel-5.4/single_variable_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-5.6/Makefile: New test case makefile.
* tests/data/test-symtab/kernel-5.6/empty.c: Likewise.
* tests/data/test-symtab/kernel-5.6/one_of_each.c: Likewise.
* tests/data/test-symtab/kernel-5.6/single_function.c: Likewise.
* tests/data/test-symtab/kernel-5.6/single_function_gpl.c: Likewise.
* tests/data/test-symtab/kernel-5.6/single_variable.c: Likewise.
* tests/data/test-symtab/kernel-5.6/single_variable_gpl.c: Likewise.
* tests/data/test-symtab/kernel-5.6/empty.ko: New test data.
* tests/data/test-symtab/kernel-5.6/one_of_each.ko: Likewise.
* tests/data/test-symtab/kernel-5.6/single_function.ko: Likewise.
* tests/data/test-symtab/kernel-5.6/single_function_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-5.6/single_variable.ko: Likewise.
* tests/data/test-symtab/kernel-5.6/single_variable_gpl.ko: Likewise.
* tests/data/test-symtab/kernel/Makefile: New test case source file.
* tests/data/test-symtab/kernel/empty.c: Likewise.
* tests/data/test-symtab/kernel/one_of_each.c: Likewise.
* tests/data/test-symtab/kernel/single_function.c: Likewise.
* tests/data/test-symtab/kernel/single_function_gpl.c: Likewise.
* tests/data/test-symtab/kernel/single_variable.c: Likewise.
* tests/data/test-symtab/kernel/single_variable_gpl.c: Likewise.
* tests/test-symtab.cc: New test case to test kernel symtabs.
Add (Catch based) test suite to test symbol table reading through the
result persisted in abigail::corpus.
The test cases are created through simple C source files targeting the
desired properties (having an undefined/export function or both). The
Makefile that comes with them recreates the test cases from the sources.
This covers reading sorted_(undefined_)var|fun_symbols as well as the
corresponding symbols maps accessible through the accessors of
abigail::corpus.
* tests/Makefile.am: add new test runtestsymtab
* tests/data/Makefile.am: add new test data for runtestsymtab
* tests/data/test-symtab/Makefile: Add this to build the binaries
below from their source code.
* tests/data/test-symtab/basic/empty.c: New test case source.
* tests/data/test-symtab/basic/link_against_me.c: Likewise.
* tests/data/test-symtab/basic/no_debug_info.c: Likewise.
* tests/data/test-symtab/basic/one_function_one_variable.c: Likewise.
* tests/data/test-symtab/basic/one_function_one_variable_undefined.c: Likewise.
* tests/data/test-symtab/basic/single_function.c: Likewise.
* tests/data/test-symtab/basic/single_undefined_function.c: Likewise.
* tests/data/test-symtab/basic/single_undefined_variable.c: Likewise.
* tests/data/test-symtab/basic/single_variable.c: Likewise.
* tests/data/test-symtab/basic/empty.so: New test data, built from
the Makefile above.
* tests/data/test-symtab/basic/link_against_me.so: Likewise.
* tests/data/test-symtab/basic/no_debug_info.so: Likewise.
* tests/data/test-symtab/basic/one_function_one_variable.so: Likewise.
* tests/data/test-symtab/basic/one_function_one_variable_undefined.so: Likewise.
* tests/data/test-symtab/basic/single_function.so: Likewise.
* tests/data/test-symtab/basic/single_undefined_function.so: Likewise.
* tests/data/test-symtab/basic/single_undefined_variable.so: Likewise.
* tests/data/test-symtab/basic/single_variable.so: Likewise.
* tests/test-symtab.cc: New test driver.
When emitting abixml types inside a given scope are sorted
topologically. Types that don't have source definition location
information are sorted lexicographically.
There are certain types however that need more careful consideration.
Those are empty-qualified types. That is, qualified types (like
cv-qualified types) that carry no qualifier. The patch explains
in-extenso in comments where those types come from. You can also look
at the comments of the function maybe_strip_qualification for even
more context.
Simply put, an empty qualified type like 'NONE reference type' equals it's
non-qualified variant 'reference type'.
During the topological sorting, we chose to have the empty-qualified
variant "come before" (i.e, be "less than") the non-qualified variant.
This is alright.
The bug however is that we failed to handle the case were we are
looking at two empty-qualified types that are equal. In that case, of
course, they are meant to be topologically equivalent.
Fixed thus.
* src/abg-ir.cc (type_topo_comp::operator()): In the comparison
operator consider two equivalent empty-qualified types as being
topologically equivalent.
git://sourceware.org / libabigail.git / log