Internal names (and pretty representation) of types are used for type
canonicalization. These were not being correctly computed for
pointers typedefs and arrays because we were forgetting sometimes to
use internal names of the underlying types, especially because of caching issues.
This patch addresses that.
Note that I noticed this while comparing the two versions of
libgromacs_d.so.0.0.0 involved in the comparison referenced by bug
https://bugzilla.redhat.com/show_bug.cgi?id=1283906. But then that
library is too big (and takes too much time) to be included as a non
regression test :(
* include/abg-ir.h (pointer_type_def::priv_): New data structure.
The type is now pimpled.
(typedef_decl::priv_): Likewise.
* src/abg-ir.cc (struct pointer_type_def::priv): New struct.
(pointer_type_def::pointer_type_def): Adjust.
(pointer_type_def::get_pointed_to_type): Likewise.
(pointer_type_def::get_qualified_name): Store temporary/internal
names into different caches.
(array_type_def::priv::{temp_internal_qualified_name_,
internal_qualified_name_}): New data members.
(get_type_representation): In the overload for array_type_def,
take requests for internal names into account.
(array_type_def::get_qualified_name): Take requests for internal
names into account. Store temporary/internal names into different
caches.
(typedef_decl::priv): New struct.
(typedef_decl::typedef_decl): Adjust.
(typedef_decl::get_underlying_type): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-comparison.cc (qualified_type_diff::has_changes): Make
this stupid and simple, now that we have (fast) canonical type
based comparison.
* include/abg-ir.h (qualified_type_diff::operator==): Add an
overload for qualified_type_diff here.
(operator==): Likewise.
* src/abg-ir.cc (qualified_type_diff::operator==): Define it.
(operator==): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Sometimes we can see const references in DWARF. But then, a reference
is always const, so that qualified reference is redundant.
Furthermore, having that construct make its way into the internal
representation can cause awkward diagnostics.
The DWARF reader was thus eliding such redundant qualifiers in the
function "maybe_strip_qualification". It was doing so by stripping
the qualifier from the qualified type. So const reference, for
instance, becomes a (non-qualified) reference. In that case, we are
turning a qualified type into a non-qualified one.
But as the accompanying problem report suggests, this can cause issues
during the DWARF parsing. This is because a given Debug Information
Entry (DIE) of qualified type kind can be referenced elsewhere, by
another type. That other type expects that DIE to be a qualified
type. And libabigail's DWARF reader code enforces that. So the
internal representation of a type resulting from a qualified type DIE
must be a qualified type itself.
So the way the function "maybe_strip_qualification" was doing the
redundancy elision was wrong. This patch fixes that by keeping the
type qualified, but introducing a "no-op" qualifier. Actually, the IR
already has that "no-op" qualifier: abigail::ir::qualified_type_def::CV_NONE.
So now "maybe_strip_qualification" just turns the CV_CONST qualifier
into a CV_NONE one when the former is redundant.
Now that the libabigail type system actually *has* types qualified
with this no-op qualifier, we need to handle things like printing the
name of such qualified types. When we are printing the name of the
type for internal reasons (i.e, for type canonicalization purposes) we
need to make a difference between the name of a no-op qualified type
and the name of the underlying type of the qualified type, otherwise,
the canonicalizer wrongly considers the two types as being equal. But
then when we are printing the name of the no-op qualified type for
diagnostics reasons, then the name is the same as the name of its
underlying unqualified type.
* src/abg-dwarf-reader.cc (maybe_strip_qualification): Do not nuke
the qualified type. Rather, just turn the redundant const
qualifier into a no-op one.
* src/abg-comparison.cc (compute_diff_for_types): Look through
no-op qualified types.
* include/abg-ir.h
(decl_base::{peek,set}_temporary_qualified_name): Declare new
accessors.
* src/abg-ir.cc (decl_base::priv::temporary_qualified_name_): New
data member.
(decl_base::{peek,set}_temporary_qualified_name): Define new
accessors.
(qualified_type_def::priv::{temporary_internal_name_,
internal_name}): New data members.
(qualified_type_def::build_name): For a no-op qualified type, the
internal name (which contains the 'none' qualifier) is different
from the non-internal name.
(qualified_type_def::get_qualified_name): Handle temporary names
and non-temporary names in two different caches. Also handle
internal and non-internal names in two different caches. This
makes four different caches.
(qualified_name_setter::do_update): Do not touch the non-internal,
non-temporary qualified name cache if the qualified parent name is
empty.
* tools/abidw.cc (main): change --check-alternate-debug-info to
make it *not* display the name/path to the alternate debug info,
when it's found. Rather, only
--check-alternate-debug-info-base-name keeps displaying the base
name of the alternate debug info.
* tests/data/test-alt-dwarf-file/test1-libgromacs-debug-dir/*: New
test material.
* tests/data/Makefile.am: Add the new test material to the build
system.
* tests/test-alt-dwarf-file.cc (in_out_specs): Take the new test
input into account.
* tests/data/test-read-dwarf/test1.abi: Adjust.
* tests/data/test-read-dwarf/test7.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/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/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When peeling off typedefs, references and pointers to see if a type is
made of a class type, we forget to peel qualified types off.
This is in the context of parsing type info from DWARF and to
determine if we should delay type canonicalization (because a given
type is made of a class) or not.
Fixed thus.
* include/abg-fwd.h (peel_qualified_type): Declare new function
...
* src/abg-ir.cc (peel_qualified_type): ... and define it.
(peel_typedef_pointer_or_reference_type): Peel qualified types
here too.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While looking at something else, I stumbled across spots where we
forget to discriminate between internal and non-internal type names.
This can lead to two types (that are equivalent) to be considered as
being different just because one is based on a struct foo, and the
other one is based on a class foo.
Fixed thus.
* src/abg-ir.cc (get_type_name, get_method_type_name)
({typedef_decl,var_decl,function_decl,class_decl}::get_pretty_representation):
Propagate the internal-ness to the call to get_qualified_name().
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* include/abg-fwd.h (is_qualified_type): Make this take a const
parameter.
* src/abg-ir.cc (is_qualified_type): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When given a package which has no binary to compare, abipkgdiff
aborts. This patch fixes that.
* tests/data/test-diff-pkg/empty-pkg-libvirt-0.9.11.3-1.el7.ppc64.rpm:
New input test package.
* tests/data/test-diff-pkg/empty-pkg-libvirt-1.2.17-13.el7_2.2.ppc64.rpm:
Likewise.
* tests/data/test-diff-pkg/empty-pkg-report-0.txt: New test
reference output.
* data/Makefile.am: Add the new test material above to the build system.
* tests/test-diff-pkg.cc (int_out_specs): Add the new test inputs
to the set of tests.
* tools/abipkgdiff.cc (compare): Do not abort if there is no
binary to compare.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Make sure to read enum values in the widest possible integer (size_t)
but write them (in abixml writer) using a signed type to ease
comparison.
This makes the runtestreaddwarf pass on 32 bit x86, because we were
losing some precision reading enum values using a signed integer.
* include/abg-ir.h (enum_type_def::enumerator::get_value): Return
a size_t.
* src/abg-ir.cc (enum_type_decl::enumerator::get_value): Likewise.
* src/abg-dwarf-reader.cc (die_signed_constant_attribute): #if-out
this static function that is not used anymore.
(build_enum_type): Read the value of the enumerator using a size_t
value.
* src/abg-reader.cc (build_enum_type_decl): Read the enum value
using a long long int.
* src/abg-writer.cc (write_enum_type_decl): Write using a ssize_t.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
g++ 4.4.7 of el6 does not support designated initializers, so
libabigail fails to build on el6 for that reason.
* tools/abipkgdiff.cc (prepare_packages): Do not use designated
initializers syntax.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch changed the revision number of the libabigail library to
make it reflect the fact that we are not in "release candidate" mode,
before the first 1.0 release. So the revision number is now "rc0".
The configuration manager has been updated to support version numbers
that are strings, so that it can supports things like "rc0".
Then, several libabigail tools have been modified to support the
--version option to display their version number.
* configure.ac: Set the version revision to "rc0".
* doc/manuals/abicompat.rst: Adjust manual for new --version
option.
* doc/manuals/abidiff.rst: Likewise.
* doc/manuals/abidw.rst: Likewise.
* doc/manuals/abilint.rst: Likewise.
* doc/manuals/abipkgdiff.rst: Likewise.
* include/abg-config.h (config::{m_format_minor, m_format_major}):
Make these be strings.
(config::{get,set}_format_minor_version_number): Make these return
strings.
(config::{get,set}_format_major_version_number): Make these return
or take strings.
(abigail_get_library_version): Make this take strings.
* src/abg-config.cc (config::config): Adjust.
(config::{get,set}_format_major_version_number): Make these return
or take strings.
(config::{get,set}_format_minor_version_number): Make these return
strings.
(abigail_get_library_version): Make this take strings.
* include/abg-version.h.in: Make the version variables be strings.
* src/abg-writer.cc (write_translation_unit): The version numbers
are now strings so adjust.
* tools/{abicompat,abidiff,abidw,abilint,abipkgdiff,abisym}.cc
(options::display_version): New data member.
(options::options): Initialize it.
(display_usage): Add documentation for new --version option.
(parse_command_line): Parse new --version option.
(main): Support --version.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When a function decl DIE is in the main DWARF file but has an abstract
origin that is in the alternate DWARF file, the dwarf reader
mistakenly considers the resulting function decl as being for a DIE
that is in the alternate DWARF file.
Fixed thus.
* src/abg-dwarf-reader.cc (build_ir_node_from_die): Consider that
the function decl is for a DIE in the alternate debug info file only if
the DIE itself comes from the alternate debug info file, not if
the specification or the origin of the function comes from the
alternate debug info file.
* tests/data/test-diff-pkg/qemu-img-rhev-2.3.0-20.el7.ppc64.rpm:
New test input rpm.
* tests/data/test-diff-pkg/qemu-img-rhev-2.3.0-7.el7.ppc64.rpm: Likewise.
* tests/data/test-diff-pkg/qemu-kvm-rhev-debuginfo-2.3.0-20.el7.ppc64.rpm: Likewise.
* tests/data/test-diff-pkg/qemu-kvm-rhev-debuginfo-2.3.0-7.el7.ppc64.rpm: Likewise.
* tests/data/test-diff-pkg/qemu-img-rhev-2.3.0-7.el7.ppc64--qemu-img-rhev-2.3.0-20.el7.ppc64-report-0.txt:
New test reference output.
* tests/data/Makefile.am: Add the new test material to the source
distribution.
* tests/test-diff-pkg.cc (in_out_specs): Use the new test rpm
inputs.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Abipkgdiff now attempts to extract packages and compare the resulting
ELF pairs in parallel. First off, a thread is spawned to extract each
package and each debuginfo package. After the ELF files are extracted,
mapped and the threads are successfully collected, the resulting
ELF vectors are traversed to identify existing pairs and a list of
arguments for future comparison is made. This list is then sorted
by size from largest to smallest. Unless --no-parallel is
specified on the command line, MIN(pairs,active_processors) threads
are spawned, sharing a single list of arguments. This list is
processed and a map of (ELF_PATH,DIFF_CORPUS) is created. Meanwhile,
the main thread checks this same map for results in the original order
of the ELF pairs, ensuring sequential output. After all the diffing
and reporting is done, the threads are collected again.
* doc/manuals/abipkgdiff.rst: Mention the new --no-parallel option.
* tools/Makefile.am: Add -pthread to abipkgdiffs link options.
* tools/abipkgdiff.cc (elf_file_paths_tls_key): New key for the
thread-local vector of ELF filepaths.
(reports_map): A map of the path of the first ELF of a compared pair
and a corpus representing the difference.
(env_map): A map of the corpus difference and a corresponding
environment needed to be kept alive until the diff is reported.
({arg,map}_lock): mutexes to control access to the comparison argument
list and the {reports,env}_map respectively.
(options): Add a new member "parallel" and set it to true in the ctor.
(elf_file): Add a new "size" member and set it in the ctor.
(package descriptor): Arguments passed to extract_package_set.
(compare_args): Arguments passed to the ELF comparison function.
(display_usage): Mention the new "--no-parallel" option.
(pthread_routine_extract_package): A wrapper function around
extract_package to be used in a multi-threaded environment.
({first_second}_package_tree_walker_callback_fn): Add the new ELF file
paths to a thread-specific vector.
(compare): In an overload of compare, verbose output is updated to
always mention the ELF files being compared for each reported stage.
Reporting is no longer done in this function, the resulting difference
is instead passed back to the calling function for reporting in the
main thread, along with a corresponding environment.
(pthread_routine_compare): Accept a pointer to a vector of comparison
arguments. This function is to be called NTHREAD times and share the
vector passed to it with its other invocations. Create the environment
for compare() and store its output in a map if there is a difference.
(create_maps_of_package_content): Allocate memory for a thread local
vector of ELF paths and dispose of it before returning.
(pthread_routine_extract_pkg_and_map_its_content): Renamed from
extract_package_and_map_its_content. Extract the debuginfo as well as
the regular package in this function. Spawn a separate thread for the
extraction of the debug package.
(pthread_join): A function handling thread joining throughout package
extractions.
(prepare_packages): Spawn a thread to extract each set of packages.
(elf_size_is_greater): New comparison function used to order ELF pairs
by size.
(compare): In the overload of compare, pass through the ELF path
vectors and identify pairs to be diffed. Put them in a vector and sort
it by the summed ELF pair size. Spawn comparison threads and safely
check for results in the proper order of the ELF pairs. Report any
differences ASAP and collect the threads after all the reporting is
done, checking their return status.
(parse_command_line): Check for the "--no-parallel" option.
Signed-off-by: Ondrej Oprala <ooprala@redhat.com>
In the DWARF reader it can happen that a function type being built is
canonicalized too early, before the type is done building. This leads
to some spurious type differences later at comparison time. It
typically happens when a sub-type of the function type refers to the
function type itself. We correctly handle similar cases for class
types, but not for function types. Oops.
This patch handles this case for function types in the dwarf reader
and in the abixml reader.
* src/abg-dwarf-reader.cc (die_function_type_map_type): New
typedef.
* ():
(read_context::die_wip_function_types_map_): New data member.
(read_context::{die_wip_function_types_map,
is_wip_function_type_die_offset}): New methods.
(build_function_type): Mark the function being built as "work in
progress".
(maybe_canonicalize_type): Do not early-canonicalize WIP function
types.
* src/abg-reader.cc (build_function_type): Mark the function being
built as "work in progress".
* tests/test-diff-dwarf-abixml.cc: New test harness.
* tests/Makefile.am: Add new test harness runtestdiffdwarfabixml
to the build system.
* tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1:
New test binary input.
* tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1.abi:
New test input.
* tests/data/Makefile.am: Add new test inputs to source
distribution.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It appears that when setting a base class specifier for a given class,
the environment of the class is not propagated to the base specifier.
This patch fixes that.
* src/abg-comparison.cc (compute_diff): In the overload for
class_decl::base_spec_sptr, assert that the environment of the
base classes are equal and that the environment the base class is
the same as the environment of the base specifier.
* src/abg-ir.cc (add_base_specifier): Propagate the environment of
the class to its base specifiers.
* tests/data/test-types-stability/pr19026-libvtkIOSQL-6.1.so.1:
New test binary input.
* tests/data/Makefile.am: Add the new test input to the build
system.
* tests/test-types-stability.cc (elf_paths): Add new binary to the
test harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Libabigail's DWARF reader does not support the DW_AT_count attribute
used to specify the number of elements in an array subrange. Rather,
it uses the DW_AT_lower_bound and DW_AT_upper_bound attributes that
are emitted by GCC. Recent versions of Clang, on the other hand, use
the DW_AT_count attribute.
This patch adds support for the DW_AT_count attribute too.
* src/abg-dwarf-reader.cc (get_default_array_lower_bound): Define
new static function.
(build_array_type): Support the DW_AT_count attribute.
* tests/data/test-diff-dwarf/test35-pr19173-libfoo-long-clang.so:
New test binary input.
* tests/data/test-diff-dwarf/test35-pr19173-libfoo-long-clang2.so: Likewise.
* tests/data/test-diff-dwarf/test35-pr19173-libfoo-long-clang-report-0.txt:
New test reference output.
* tests/data/test-diff-dwarf/test35-pr19173-libfoo-long-gcc.so:
New test binary input.
* tests/data/test-diff-dwarf/test35-pr19173-libfoo-long-gcc2.so:
New test binary input.
* tests/data/test-diff-dwarf/test35-pr19173-libfoo-long-gcc-report-0.txt:
New test reference output.
* tests/data/test-diff-dwarf/test35-pr19173-libfoo-long.c: Source
code for the binaries above.
* tests/data/Makefile.am: Add the new test material to the build
system.
* tests/test-diff-dwarf.cc (in_out_specs): Add the new test inputs
to the harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It appears that libabigail does not represent the size of ELF symbols,
so it doesn't detect when a symbol size changes without impacting the
size of the type of said symbol, as described by debug info.
It appears that Address Sanitizer as implemented by Clang does change
the size of variable symbols when it instruments those variables. And
of course, the size of type of said symbols (as described by debug
information) remains unchanged.
This patch makes Libabigail become aware of symbol sizes, especially
for variables. Symbol sizes for functions are ignored for now,
because a change in a function symbol size is not an ABI change.
The patch makes libabigail detect and report symbol size changes for
variables, but looking at the ELF information, independently from the
debug information.
The patch adjusts the existing tests and adds a new test using the
binaries that were filed in the bug report.
* include/abg-ir.h (elf_symbol::{elf_symbol, create}): Take a size
parameter.
(elf_symbol::{get,set}_size): New accessors.
* src/abg-ir.cc (elf_symbol::priv::size_): New data member.
(elf_symbol::priv::priv): Initialize it.
(elf_symbol::{elf_symbol, create}) Take a size parameter.
(textually_equals): Compare the size of variable symbols.
(elf_symbol::{get, set}_size): New accessors.
* src/abg-comparison.cc (maybe_report_diff_for_symbol): New static
function.
({function_decl_diff,var_diff}::report): Use it.
* src/abg-dwarf-reader.cc (lookup_symbol_from_sysv_hash_tab)
(lookup_symbol_from_gnu_hash_tab, lookup_symbol_from_symtab)
(read_context::lookup_elf_symbol_from_index): Set the size of the
elf symbols' internal representation.
* src/abg-reader.cc (build_elf_symbol): Read the size attribute if
present.
* src/abg-writer.cc (write_elf_symbol): Write the size attribute
for variable symbols, if it's not zero.
* tests/data/test-diff-dwarf/test34-pr19173-libfoo.so: New test
input binary.
* tests/data/test-diff-dwarf/test34-pr19173-libfoo2.so: Likewise.
* tests/data/test-diff-dwarf/test34-pr19173-libfoo-report-0.txt:
New reference test output.
* tests/data/Makefile.am: Add the new test input binaries to the
build system.
* tests/test-diff-dwarf.cc (in_out_specs): Add the new test input
above to the test harness.
* tests/data/test-diff-dwarf/test9-report.txt: Adjust.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.txt: Likewise.
* tests/data/test-read-dwarf/test0.abi: Likewise.
* tests/data/test-read-dwarf/test1.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/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.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/test6.so.abi: Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* include/abg-ir.h (operator==): In the overload for
elf_symbol_sptr, pass the parameters by reference.
* src/abg-ir.cc (operator==): Do the same at definition site.
* src/abg-comparison.cc (maybe_report_diff_for_member): Pass
parameters by reference.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In this bug, the DWARF debug info of the binary (generated by Intel's
ICC compiler) has interesting constructs like:
[ 6b5a0] subprogram
decl_line (data2) 787
decl_column (data1) 15
decl_file (data1) 46
declaration (flag)
accessibility (data1) public (1)
type (ref4) [ 6b56a]
prototyped (flag)
name (string) "ldiv"
MIPS_linkage_name (string) "ldiv"
[ 6b5b6] formal_parameter
type (ref4) [ 5f2aa]
name (string) "$Ë2"
[ 6b5bf] formal_parameter
type (ref4) [ 5f2aa]
name (string) "$Ë3"
Note the strings that make up the name of the formal parameters of the
function, near the end:
[ 6b5b6] formal_parameter
type (ref4) [ 5f2aa]
name (string) "$Ë2"
[ 6b5bf] formal_parameter
type (ref4) [ 5f2aa]
name (string) "$Ë3"
The strings "$Ë2" and $Ë3" (which are the names of the
parameters of the function) are garbage.
Libabigail's DWARF reader naively uses those strings as names for the
function parameters, in the type of the function.
Then, the abixml writer emits an XML document, with these strings as
property values, representing the name of the type of the function.
And of course, the XML later chokes when it tries to read that XML
document, saying that the property is not valid UTF-8.
This patch addresses the issue by dropping those garbage names on the
floor, for function type names. In that context, any string that is
not made of ASCII characters is considered as being garbage, for now.
The patch, in the abixml writer, also escapes function parameters
names so that they don't contain characters that are not allowed in
XML. The abixml reader already handles the un-escaping of the names
it reads, so I think there is nothing to do there.
Ultimately, I guess I should get the unicode value of the characters
of that string, encode the string into UTF-8 and use the result as the
name for the parameter. That would mean using UTF-8 strings for
function parameter names, and, for all declarations names. But that
is too much for worfk too little gain for now. The great majority of
the binaries we are dealing with are still using ASCII for declaration
names.
The patch also introduces a new test harness that runs "abidw
--abidiff" on a bunch of input binaries. This harness runs over the
binaries that were submitted in this bug report.
* include/abg-tools-utils.h (string_is_ascii): Declare new
function ...
* src/abg-tools-utils.cc (string_is_ascii): ... and define it.
* src/abg-writer.cc (write_function_type): Escape forbidden XML
characters in function type names.
* src/abg-dwarf-reader.cc (build_function_type): If a parameter
name is not ascii, drop it on the floor.
* tests/data/test-types-stability/pr19139-DomainNeighborMapInst.o:
New test input binary.
* tests/data/test-types-stability/pr19202-libmpi_gpfs.so.5.0:
Likewise.
* tests/data/Makefile.am: Add the new binaries above to the build
system.
* tests/test-types-stability.cc: New test harness.
* tests/Makefile.am: Add the new test harness to the build system.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Until now, the native XML format of libabigail didn't have a name. I
am introducing the name "abixml" that I am slowing starting to use
these days. So I am adding that name in comments of the files where
we emit and read that format.
* src/abg-reader.cc: Mention abixml in the comment at the top of
the file.
* src/abg-writer.cc: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This is a second attempt at fixing this bug. The previous attempt was
on the good path, but a thinko got in the way, oops. We are not adding
the new binary of that bug report (which got re-opened) to the test
suite because it's too big and it takes too much time and memory to
abidw to analyze it.
* src/abg-ir.cc (type_base::get_canonical_type_for): Really
compare the size of the type to be canonicalized against the size
of the *current* potential canonical type of the same name.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This script parses the output of runtestreaddwarf (that output is
non-empty when the reference output *.abi files need updating, but
when there is no ABI change error). As a result, the script emits a
series of "cp <source-file> <destination-file>" command to issue, to
update the reference output of the runtestreaddwarf test program.
To use this script to update the reference output *.abi files for
runtestreaddwarf,
<build-dir>/tests/runtestreaddwarf > changed-output.txt
python update-test-read-dwarf-output.py changed-output.txt > shell-update-commands.sh
sh shell-update-commands.sh
* tests/update-test-read-dwarf-output.py: New helper python program.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* tests/test-read-dwarf.cc (for test test21-pr19092.so.abi): Fix
typo in the output path of that test.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This is the last patch of a series of patches which aims at fixing bug
libabigail/19097. The short titles of the patches of the set are,
including this one:
Don't canonicalize types not added to their context in abixml reader
Support updating a class in the abixml reader
Fix emitting of referenced type in abixml writer
Use abidw --abidiff in test-read-dwarf.cc
Adjust regression tests reference output for the current patch set
Below is the cover letter of the last patch of the set.
The current patch set needs big reference output adjustments, that we
are doing at the end, here.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so:
New test input binary.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
New test reference output.
* tests/data/Makefile.am: Add the new test files above to the
source distribution.
* tests/test-read-dwarf.cc (in_out_specs): Add the two new test
files above to the set of test input files.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Adjust.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Adjust.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Adjust.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Adjust.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Adjust.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Adjust.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Adjust.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Adjust.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Adjust.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
That test was doing several sub-tests that amount to just calling
abidw --abidiff. So, let's use that, now that we have it.
* tests/test-read-dwarf.cc (handle_in_out_spec): Rather than
calling abilint on the abixml and abidiff-ing the .so file against
its .so.abi, call abidw --abidiff on the .so file and voila. Ok,
it does one extra save of abixml, but then that won't hurt. And
things are faster now than what they were anyway :-)
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
There are several issues with the abixml writer in how it handles the
process of emitting referenced types that are not directly reachable
but just walking the scopes (namespaces) of the translation units;
think about member types of a class A that are not necessarily present
in all the declarations of A, in all translation units, for instance.
This patch addresses them all because they are all intermingled.
* Use of canonical pointers in the hash map of referenced types
The abixml writer was using canonical types pointer values to hash
referenced types in a map. It was doing so "by hand"; and it was thus
messing things up for types without canonical types (like some class
declarations) etc.
This patch changes that by using the generic solution of
abigail::ir::hash_type_or_decl(), which also uses the same canonical
pointer type values. For types with no canonical types, that
functions knows has to gracefully fallback. At worst, it will just
make things slower, not wrong.
* Sorting of referenced types
The patch also changes the sorting function used for the hash map of
referenced types. The previous solution was sorting the pretty
representation of types; but then when two types have the same pretty
representation (think, typedefs, for instance) then their relative
position in the sorted result was random. This causes some stability
issues, in that emitting the abixml for the same binary several times
can lead to the some types being sorted differently -- they have the
same name, but not necessarily the same type *IDs*, as they are
different types.
The new sorting code handles this better; it also uses the pretty
representations of types, when they are equal, it uses the type IDs to
tell the types apart. At least this brings stability in the abixml
output, for a given binary.
* Avoiding duplicating declaration-only types when emitting the
context of referenced member types.
We don't keep track of declaration-only classes that are emitted.
This is because we allow a given class declaration (that carries no
definition) to appear several times in a given ABI corpus. So when a
referenced type is a class declaration, it always appears as if that
referenced type has not been emitted. So when we specifically emit
the not-emitted referenced types, it can happen that declaration-only
classes can appear a lot of times. This is unnecessary duplication,
aka bloat.
This patch thus introduces a new hash map that tracks emitted
declaration-only classes, so that we can allow duplication of class
declarations when they follow what's done in the IR read from DWARF,
and disallow that duplication when it's totally artificial and
useless.
* Better tracking of referenced types
We were blatantly forgetting to mark some referenced types as such.
So those were missing in some abixml output.
This patch fixes the spots where we were forgetting that important
information.
* Better representation of the scopes of the referenced types that
were specifically emitted.
The previous code was failing at properly representing the class scope
of some referenced types that were specifically emitted, or sometimes,
for member types, representing the scope would be so screwed that the
(referenced) member type itself wouldn't be emitted at all.
This is because I thought that to emit a given member type, just
emitting its parent scope would be enough. I thought that would
automatically trigger emitting the member type itself. First, that
would emit too much information at times; the other members of the
scope are not necessarily needed. And second the "duplication
detection code" would sometime refuse to emit the scope class, because
it has already been emitted earlier! But the incarnation that got
emitted didn't have this member type as member, then. Yes, in DWARF,
the same class A can be declared several times with different member
types in it. The complete representation of A would be a union of all
those declarations of A that are seen.
This patch addresses this issue by carefully emitting just the
information that is needed from the scope of the referenced type.
Basically the scope is declared just to declare/define the type we are
interested in; period. The abixml reader is now properly geared to
re-construct the scope by merging its different parts that are now
scattered around, in the ABI corpus. That support is part of this
patch set.
instance, a member typedef would be emitted with the information of
its parent class badly formatted.
* src/abg-writer.cc (struct type_ptr_comp_functor): Remove this.
(sort_type_ptr_map): Likewise.
(write_context::record_type_as_referenced): Do not add the
canonical type of the type to record as referenced directly.
(write_context::type_is_referenced): Adjust accordingly.
(struct write_context::type_ptr_cmp): New comparison functor.
(write_context::sort_types): New sorting function.
(write_context::{record_decl_only_type_as_emitted,
decl_only_type_is_emitted}): New member functions.
(write_member_type_opening_tag): Factorize out of ...
(write_member_type): ... here.
(write_class_decl_opening_tag): Factorize out of ...
(write_class_decl): ... here. Now, keep track also of
declaration-only classes that are emitted.
(write_decl_in_scope): Use the new write_member_type_opening_tag
and write_class_decl_opening_tag. Now write class scopes
ourselves; they only contain the type declarations that we are
emitting.
(write_translation_unit): Use the new sorting code to sort the
referenced types to emit. Do not emit referenced types that are
declaration-only classes that have already been emitted. Handle
the fact that emitting the referenced types might make those
emitted type *reference* other types too! So handle those new
referenced types as such, and emit them too.
(write_qualified_type_def, write_typedef_decl, write_var_decl): Do
not forget to mark referenced types as such.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In DWARF, the same class declaration can be present several times but
with different "views", that is, it can be present in a first
translation unit, but without any member type; then in a subsequent
translation unit, its member types are defined. In another, it'll be
completely defined, with all its data members and base classes. The
DWARF reader knows how to amend the class to add new members to it, as
they show up in the debug information.
This patch adds the same functionality to the abixml reader. The
writer has already started to write class declarations with different
"views" too, since it's started to avoid duplicating full class
definitions in every translation unit that uses them.
Without this patch, abixml misses some class members, and that is a
bug.
* include/abg-ir.h (class_decl::{find_base_class,
find_member_type, find_data_member}): Declare new member functions ..
* src/abg-ir.cc (class_decl::{find_base_class,
find_member_type, find_data_member}): ... and define them.
* src/abg-reader.cc (build_class_decl): Add the ability to update
a class to add new data members, member types and base classes to
it, if necessary.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This is the first patch of a series which aims at fixing bug
libabigail/19097.
The problem in that report is a result of several underlying issues.
This patch series address them in isolation. The patches do not
update the reference output of the regression test as they should,
because they are all needed to get to a sane working state. The test
reference output are thus adjusted in the last patch of the series.
Here are the short titles of the patches of the set, including this
one:
Don't canonicalize types not added to their context in abixml reader
Support updating a class in the abixml reader
Fix emitting of referenced type in abixml writer
Use abidw --abidiff in test-read-dwarf.cc
Adjust regression tests reference output for the current patch set
Below is the cover letter of the first patch of the set.
The abixml reader sometimes (wrongly) canonicalizes types that are not
(yet) added to their context. This can lead to comparison issues
because some information carried by some types are dependant on their
context (e.g, access specifiers) and can be important for type
comparison. Right now, access specifiers for member types are no more
taken into account when comparing member types because DWARF emitters
do not necessarily keep a correct track of those; but when they do, we
better be prepared. And in any case, it's wrong to have type
canonicalization happen on half backed types anyway.
So this patch fixes several spots where type canonicalization happens
on types that are not added to their scope.
* src/abg-reader.cc (read_context::maybe_canonicalize_type):
Assert that a class type that is scheduled for canonicalization
must be in a scope. We do this only for classes, for now. The
assert here helped to spot (and fix) a lot of places where we
were canonicalizing types without scope.
(read_context::build_or_get_type_decl): Canonicalize types here,
when they are built and (hopefully) added to their scope. There
might be cases here where we try to canonicalize types that are
not added to their scope. That should bomb in the assert above,
at least for class types, for now. We'll then fix the places where
the types are created, to make them properly scoped.
(build_type_decl, build_qualified_type_decl)
(build_pointer_type_def, build_reference_type_def)
(build_array_type_def, build_enum_type_decl, build_typedef_decl):
Do not try to canonicalize the types early, right when they are
created. Canonicalization should happen at the point where (or
after) they are added to their scope.
(build_class_decl): Likewise. Also, schedule member types for
canonicalization once they've been added to their scope.
(build_class_tdecl): Schedule the pattern of the class template
for canonicalization once it has been added to its scope. I am
not sure I should do this, as the pattern is not yet a real type,
but I am taking my bet.
(build_type_composition): Schedule the composed type for
canonicalization once it's been added to its scope.
(handle_type_decl, handle_qualified_type_decl)
(handle_pointer_type_def, handle_reference_type_def)
(handle_function_type, handle_array_type_def)
(handle_enum_type_decl, handle_typedef_decl, handle_class_decl):
At this point, we should know if the type is to be added to a
scope or not. If it's in a scope, then schedule for
canonicalization.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It appears that two different types from two different translation
units might have the same name in a DSO, like in the example of this
bug. This violates the One Definition Rule, which we rely on to go
fast, and more importantly, it introduces type canonicalization errors.
This patch recognizes more of these ODR violation cases by looking at
the size of the types. That is, if two types (from the same DSO) with
the same name have different sizes, then they are different.
* src/abg-ir.cc (type_base::get_canonical_type_for): Look at the
size of types with the same name which could be considered
ODR-equal, to spot possible violations that would induce a type
canonicalization error.
* tests/data/test-read-dwarf/test21-pr19092.so: New test input
binary.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: New reference
abixml for the binary above.
* tests/data/Makefile.am: Add the new test input above to source
distribution.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/test-read-dwarf.cc (int_out_specs): Add the two test input
above.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* include/abg-fwd.h: Remove unnecessary declaration of class
parameter.
* src/abg-ir.cc: Remove trailing space in a comment.
* src/abg-reader.cc: Fix a comment.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch makes abidw --stats emit statitics about class declarations
that got resolved, and those that we missed.
* src/abg-dwarf-reader.cc
(read_context::resolve_declaration_only_classes): Emit statistics
about resolved classes and the missed ones.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-reader.cc (read_corpus_from_input): Add the necessary
call to xmlTextReaderNext call after the xmlTextReaderExpand call.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Profiling as shown that we might gain some precious cycles by passing
some well chosen parameters by reference.
* include/abg-ir.h (operator==): For the type_base_sptr and
decl_base_sptr overloads, pass the parameters by reference.
({var,function}_decl::{set,get}_symbol): Pass the elf_symbol_ptr
by reference.
* src/abg-ir.cc (operator==): For the type_base_sptr and
decl_base_sptr overloads, pass the parameters by reference, now in
the definition.
({var,function}_decl::{set,get}_symbol): Pass the elf_symbol_ptr
by reference, now in the definition.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This is the last patch of the series of patches whose titles are
(including this one):
Force late canonicalizing of function types read from abixml
Fix strip_typedef issues
Do not compare access specs for member types & functions
Fix "is-anonymous" abixml property impact on some tests
Fix const-ness of a function parameter
Handle aliased function decls when comparing decls in general
Make canonicalization non sensitive to struct-ness of subtypes
Set the corpus of all ABI artifact reads from abixml
Implement fast type lookup in a corpus
Accelerate a slow path in hash_type_or_decl()
A series of small speed optimizations here and there
Allow only one definition of a given type per corpus in abixml
Make abidw --abidiff not show definitely harmless changes
Adjust tests for the patchset
This patch carries the numerous adjustments necessary for the
regresion tests output after this patch set.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust.
* 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/test9-pr18818-clang.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When comparing the ABI of the input ELF binary with that same ABI
saved to abixml and read back again, there can be some minor and
harmless changes that are seen, because libabigail makes some
approximations for performance reasons. For instance, if there are
two types that are equivalent, but have different names (because of
typedefs) then libabigail will consider that they are the same type,
and might save them (to abixml) and read them back (from abixml) in
different order.
That can lead to subtle changes that are reported (and filtered out)
by the command "abidw --abixml".
This patch arranges for abidw --abixml to avoid emitting a report
saying that a filtered out change was detected, as those cases are
considered OK.
The patch also fixes a little issue where abidw would abort because
the user forgot to provide the binary to analyze, on the command line.
* tools/abidw.cc (set_diff_context): New function.
(main): Use that new function. Do not show any output for
--abidiff if only compatible changes were detected. Also, do not
abort if no input binary was giving.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In abixml, the same type T can be defined in several translation
units. This leads to a lot of duplication and, for some big binaries,
to a *lot* of memory use. For instance, loading the abixml
representing the abi of the r300_dri.so library takes more that 10GB
or ram on a 64 bits system!
This patch addresses the issue by allowing declarations to be
duplicated, but by allowing only one definition per type, per corpus.
With it, loading the abixml of r300_dri.so now takes less than 2GB or
ram.
* src/abg-writer.cc (write_translation_unit): Do not clear some
important per-translation unit maps here. There are needed to
keep track of the emitted and referenced types through the entire
corpus. Avoid (wrongly) recording function types twice.
(write_array_type_def, write_function_decl, write_function_type):
Record referenced types.
(write_class_decl): Record referenced types, and, allow only
declarations to be duplicated in a corpus.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
These are small speed optimizations that are induced by some lesser
hot spot identified by profiling.
* src/abg-comparison.cc (var_diff::has_changes): Just compare the
two var_decl. It's (way) faster now than using recursive hashing
for that.
* src/abg-ir.cc (elf_symbol::does_alias): Get out early if the two
main symbols are equal.
(equals): In the overload for function_decl, start by comparing
types. This can be very fast for functions with different types,
as it amounts to a pointer comparison. In the overload for
class_decl, avoid a map lookup when it's not necessary.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
