When we need to get the version of a defined symbol, only the
SHT_GNU_versym and SHT_GNU_verdef sections are necessary.
SHT_GNU_verneed is not necessary, for instance.
So do not require that all of the three version-related sections to be
present when we want to get some symbol version information.
Otherwise, we just don't get the version of a defined symbol when the
SHT_GNU_verneed section is not present. I stumbled upon this while
looking the abidw's output of ld-2.17.so from
glibc-2.17-79.el7_1.x86_64. The _rtld_global_ro variable's symbol was
being seen as having no symbol version. In reality it has the
GLIBC_PRIVATE version, but because the binary lacks a SHT_GNU_verneed
section, we were not getting the symbols version information.
I am not adding that library to the test suite because it's too big.
But at least this change doesn't break the existing regression test
suite.
* src/abg-dwarf-reader.cc (get_symbol_versionning_sections): Allow
returning just some of the three version-related section, not
necessarily all of them. Adjust comment.
(get_version_for_symbol): Be ready to not necessarily having the
three version-related sections available.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch adds support for loading system and user level suppression
specifications for libabigail tools.
At launch time, relevant libabigail tools (abidiff, abipkgdiff
fedabipkgdiff for now) read the default system suppression
specification file, if it's present, from a file which path is the
value of the environment variable
LIBABIGAIL_DEFAULT_SYSTEM_SUPPRESSION_FILE, if set, or from the file
$libdir/libabigail/default.abignore.
Then it reads the user system suppression specification file, if it's
present, from a file which path is the value of the environment
variable LIBABIGAIL_DEFAULT_USER_SUPPRESSION_FILE, if set, or from the
file $HOME/.abignore.
The content of the user system suppression file is merged with the
content of default system suppression file.
That content is also merged with the content of the suppression
specification files that might be provided by the --suppressions
command line option of the invoked tools.
The resulting set of all these suppression specifications is thus used
to filter the ABI change reports that are emitted.
abidiff, abipkgdiff and abipkgdiff gain a --no-default-suppression
option to avoid loading any of these default suppression specification
files.
The patch also installs a default.abignore file into $(pkglibdir).
Note that on x86_64, that directory is /usr/lib64/libabigail. Now we
just need to think about the content of that default.abignore file.
* doc/manuals/abidiff.rst: Document the default suppression
scheme, its interaction with the --supprs option and the new
--no-default option.
* doc/manuals/abipkgdiff.rst: Likewise.
* doc/manuals/fedabipkgdiff.rst: Likewise.
* configure.ac: Generate the tests/runtestdefaultsupprs.py file
from the new tests/runtestdefaultsupprs.py.in template.
* default.abignore: New file.
* Makefile.am: Add it to source distribution.
* src/Makefile.am: Define the ABIGAIL_ROOT_SYSTEM_LIBDIR
preprocessor macro that is set the value of the $libdir autotools
macro.
* include/abg-tools-utils.h: Update copyright years.
(get_system_libdir, get_default_system_suppression_file_path)
(get_default_user_suppression_file_path)
(load_default_system_suppressions)
(load_default_user_suppressions): Declare new functions
* src/abg-tools-utils.cc (get_system_libdir)
(get_default_system_suppression_file_path)
(get_default_user_suppression_file_path)
(load_default_system_suppressions)
(load_default_user_suppressions): Define new functions.
(is_regular_file): Amend this so that it return true for symlinks
to regular files too.
(is_dir): Amend this so that it returns true for symlinks to
directories too.
* tools/abidiff.cc (options::no_default_supprs): New data member.
(options::options): Initialize the new data member.
(display_usage): Display a new help string for the new
--no-default-suppression command line option.
(parse_command_line): Parse this new command line option.
(set_diff_context_from_opts): Load the default suppression
specifications, unless --no-default-suppression or --supprs was
provided.
* tools/abipkgdiff.cc (options::no_default_supprs): New data
member.
(options::options): Initialize the new data member.
(parse_command_line): Parse the new --no-default-suppression
command line option.
(main): Load the default suppression specifications, unless
--no-default-suppression or --supprs was provided.
* tools/fedabipkgdiff (abipkgdiff): Add --no-default-suppression
to the invocation of abipkgdiff if it was provided on the command
line.
(build_commandline_args_parser): Parse the new
--no-default-suppression command line option.
* tests/runtestdefaultsupprs.py.in: New test harness template.
* tests/Makefile.am: Add the new runtestdefaultsupprs.py to the
set of tests.
* tests/data/test-default-supprs/test0-type-suppr-0.suppr: New
test input.
* tests/data/test-default-supprs/test0-type-suppr-report-0.txt: Likewise.
* tests/data/test-default-supprs/test0-type-suppr-v0.o: Likewise.
* tests/data/test-default-supprs/test0-type-suppr-v1.o: Likewise.
* tests/data/test-default-supprs/dirpkg-1-dir-report-0.txt:
Likewise.
* tests/data/test-default-supprs/dirpkg-1-dir1: Likewise.
* tests/data/test-default-supprs/dirpkg-1-dir2: Likewise.
* tests/data/Makefile.am: Add new the new tests input above to
Makefile.am.
* tests/runtestcanonicalizetypes.sh.in: Pass
--no-default-suppression to abidiff invocations.
* tests/runtestdefaultsupprs.py.in: Likewise.
* tests/test-abidiff-exit.cc: Likewise.
* tests/test-diff-dwarf-abixml.cc: Likewise.
* tests/test-diff-filter.cc: Likewise.
* tests/test-diff-suppr.cc: Likewise.
* tools/abidiff.cc: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When running the 'fedabipkgdiff' tool that is in the build directory,
before "make install", we want to use the 'abipkgdiff' command line
tool that is in the build directory, not the one that might be
at $PATH/bin/abipkgdiff.
To do so, this patch adds a --abipkgdiff <path/to/a/given/abipkgdiff>
option so that the user can chose where to find the 'abipkgdiff'
binary to use.
Note that there is no regression test for this option yet because for
that, we'd this bug to be fixed first:
https://sourceware.org/bugzilla/show_bug.cgi?id=20147.
* tools/fedabipkgdiff (build_path_to_abipkgdiff): Define new
function.
(abipkgdiff): Invoke the new build_path_to_abipkgdiff() here.
(build_commandline_args_parser): Parse the new --abipkgdiff
option.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When abipkgdiff is launched with three packages to compare rather than
just two, it gives a nonsensical error, just like if a wrong option was
given. This patch fixes that.
* tools/abipkgdiff.cc (options::wrong_arg): New data member.
(parse_command_line): Set options::wrong_arg
to the wrong argument passed.
(main): Tell wrong argument case apart, and report it.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* doc/manuals/libabigail-concepts.rst: Do not refer just to
abidiff when talking about suppression specification. Also
refer to abipkgdiff and other tools.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While reading the overview manual page some things stroke me as
needing some cleanup, especially, the confusion about 'ABIGAIL' the
framework and libabigail the library.
* doc/manuals/libabigail-overview.rst: Cleanup some confusion
about Abigail-the-framework and libabigail-the-library.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While applying the patch 59917a8 and resolving the conflicts that
arose then, I forgot to git add the RPMs that were needed by the
accompanying test.
This test fixes that.
* tests/data/test-fedabipkgdiff/dbus-glib-0.104-3.fc23.x86_64.rpm:
New file.
* tests/data/test-fedabipkgdiff/dbus-glib-0.80-3.fc12.x86_64.rpm: Likewise.
* tests/data/test-fedabipkgdiff/dbus-glib-debuginfo-0.104-3.fc23.x86_64.rpm:
Likewise.
* tests/data/test-fedabipkgdiff/dbus-glib-debuginfo-0.80-3.fc12.x86_64.rpm:
Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The check-valgrind target was only accessible from the tests/
sub-directory. Make it be accessible from the top-most directory too.
* Makefile.am (check-valgrind): Add this new target here.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
abipkgdiff supports --dso-only to compare only ABI of shared libraries
instead of all binaries. So, with adding this option to fedabipkgdiff,
its user is also able to let fedabipkgdiff compare ABI of shared
libraries, or not.
* tools/fedabipkgdiff: Do not import shlex anymore.
(ABIDIFF_OK, ABIDIFF_ERROR, ABIDIFF_USAGE_ERROR)
(ABIDIFF_ABI_CHANGE): New global constant variables.
(abipkgdiff): Pass the --dso-only option to the abipkgdiff command
line tool, if that option was passed to fedabipkgdiff. Build this
abipkgdiff command invocation from an array of strings, rather
than from formatting a string. This makes us get rid of the shlex
module. Fix typo in dry-run logged string. If there was an
internal error reported by abipkgdiff, report it to stderr.
(build_commandline_args_parser): Parse the --dso-only command line
option.
* tests/runtestfedabipkgdiff.py.in (fedabipkgdiff_mod): Fix a typo
in initializing this global variable.
(test_data_dir): New global variable, that is used to reference
tests/data/test-fedabipkgdiff/.
(RunAbipkgdiffTest.{test_all_success, test_partial_failure}): Fix
typo.
(Mockglobalconfig.{koji_topdir, dso_only}): New data members.
(GetPackageLatestBuildTest.{test_get_latest_one,
test_cannot_find_a_latest_build_with_invalid_distro,
test_succeed_to_download_a_rpm, test_failed_to_download_a_rpm}):
Fix typo.
(BrewListRPMsTest.test_select_specific_rpms): Fix typo.
(RunAbipkgdiffWithDSOOnlyOptionTest): New test case class.
* doc/manuals/fedabipkgdiff.rst: update document for this new
--dso-only option.
* tests/data/test-fedabipkgdiff/dbus-glib-0.104-3.fc23.x86_64.rpm:
New symbolic link to
test-diff-pkg/dbus-glib-0.104-3.fc23.x86_64.rpm.
* tests/data/test-fedabipkgdiff/dbus-glib-0.80-3.fc12.x86_64.rpm:
New symbolic link to
test-diff-pkg/dbus-glib-0.80-3.fc12.x86_64.rpm.
* tests/data/test-fedabipkgdiff/dbus-glib-debuginfo-0.104-3.fc23.x86_64.rpm:
New symbolic link to
test-diff-pkg/dbus-glib-debuginfo-0.104-3.fc23.x86_64.rpm.
* tests/data/test-fedabipkgdiff/dbus-glib-debuginfo-0.80-3.fc12.x86_64.rpm:
New symbolic link to
test-diff-pkg/dbus-glib-debuginfo-0.80-3.fc12.x86_64.rpm.
* tests/data/Makefile.am: add tests/data/test-fedabipkgdiff so
that this data directory and all things within it can be included
in tarball.
Signed-off-by: Chenxiong Qi <cqi@redhat.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Previously, abipkgdiff was extracting packages and its debuginfo into path
provided in TMPDIR environment variable if set, otherwise into
/tmp/ directory. In distro like Fedora, extracting packages into /tmp/
directory causes abipkgdiff to fail on packages with larger size.
This is because /tmp/ directory in Fedora is of type tmpfs and due to which
it consumes spaces from main memory. In addition, type information for packages
and its debuginfo to be compared gets loaded into main memory. Overall,
extraction of packages in /tmp/ reduces free main memory which can be used
in other activities like loading type information for shared libraries to be
compared.
With this commit, by default temporary package extraction will occur in
$HOME/.cache/libabigail/ directory which uses space from hard disk.
Also, by setting XDG_CACHE_HOME environment variable, we can change default
package extraction location.
It also changes temporary directory name created inside parent directory
which is abipkgdiff-tmp-dir-XXXXXX instead of libabigail-tmp-dir-XXXXXX
* tools/abipkgdiff.cc (extracted_packages_parent_dir): Change
TMPDIR environment variable to XDG_CACHE_HOME and default
temporary parent directory to $HOME/.cache/libabigail/
Signed-off-by: Sinny Kumari <sinny@redhat.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This is to address the following enhancement requests:
#19588 - Add a --headers-dir1 and --headers-dir2 option to abidiff
#19948 - Add --devel-pkg1 and --devel-pkg2 options to abipkgdiff
When the user specifies where to find header files for two binaries
(or packages) being compared, this patch generates a type suppression
specification that filters out change reports about types that are
defined in files that are not in the set of header files specified.
The type suppression specification also suppresses change reports
about changed/added/removed virtual member functions which enclosing
class type matches the type suppression specification.
There is a corner case that the patch handles too, and that is
exhibited by the accompanying test case for abidiff. There can be a
class defined by DWARF as having no source location and as being a
pure declaration. This can be a class declaration that has inline
virtual members only, and one or several non-defined virtual methods
too. When that declaration is included in a source file, GCC
generates debug info that describes that class as being a
declaration-only class with no source declaration. This patch
considers such a class as being non defined; you know, like a true
opaque type. So it's considered being not defined in any public
header file. Changes to this kind of class are thus filtered out.
* include/abg-comp-filter.h: Update copyright year.
* src/abg-comp-filter.cc (has_virtual_mem_fn_change): Make this
static function become exported.
(has_virtual_mem_fn_change): Declare new function.
* include/abg-suppression.h
(suppression_base::{get,set}_is_artificial): Declare new
accessors.
(type_suppression::get_source_locations_to_keep): Return an
unordered set of strings, not a vector. Add a non-const overload.
(type_suppression::set_source_locations_to_keep): Set an unordered
set of strings, not a vector.
* src/abg-suppression.cc (suppression_base::priv::is_artificial_):
New data member.
(suppression_base::priv::priv): Initialize the new data member.
(suppression_base::{get,set}_is_artificial): Define new accessors.
(type_suppression::priv::source_locations_to_keep_): Change the
vector of strings representing source file names into unordered
set of string.
(type_suppression::get_source_locations_to_keep): Return an
unordered set of strings, not a vector. Define a non-const
overload.
(type_suppression::set_source_locations_to_keep): Set an unordered
set of strings, not a vector.
(type_suppression::suppresses_diff): Make this suppress virtual
member function diffs if the enclosing type of the changed virtual
member is suppressed by the current type_suppression.
(read_type_suppression): Adjust to use the fact that the source
locations are not stored in an unordered set, not in a vector
anymore. Otherwise, using a vector here make things too slow.
(type_suppression::suppresses_type): Likewise. Also, If the type
we are looking at has no location because it's a true opaque type
and if the current suppression is an artificial suppression that
is meant to suppress change reports about non-public types, then
suppress the type.
* include/abg-tools-utils.h (gen_suppr_spec_from_headers): Declare
new public function.
* src/abg-tools-utils.cc (PRIVATE_TYPES_SUPPR_SPEC_NAME): Define a
new constant variable.
(handle_fts_entry): Define new static function.
(gen_suppr_spec_from_headers): Define new public function.
* src/abg-comparison.cc
(corpus_diff::priv::apply_suppressions_to_added_removed_fns_vars):
If a type suppression suppresses a given class C, make it change
added/removed virtual functions whose enclosing type is C.
* tools/abidiff.cc (options::{headers_dir1, headers_dir2}): New
data members.
(display_usage): Add help strings for --headers-dir1 and
--headers-dir2.
(parse_command_line): Parse the new --headers-dir1 and
--headers-dir2 options.
(set_diff_context_from_opts): Generate suppression specifications
to filter out changes on private types, if --headers-dir1 or
--headers-dir2 is given.
* tools/abipkgdiff.cc (options::{devel_package1, devel_package2}):
New data members.
(typedef package_sptr): New typedef.
(enum package::kind): New enum.
(package::kind_): New data member. This replaces ...
(package::is_debug_info_): ... this data member.
(package::{devel_package_, private_types_suppressions_}): New data
members.
(package::package): Adjust.
(package::get_kind): Define new member function. This replaces
...
(package::is_debug_info): ... this member function overload.
(package::set_kind): Define new member functin. It replaces ...
(package::is_debug_info): ... this member function overload.
(package::{devel_package, private_types_suppressions}): Define new
accessors.
(package::erase_extraction_directies): Erase the sub-directory
where development packages are extracted to.
(compare_args::private_types_suppr{1,2}): New data members.
(compare_args::compare_args): Adjust.
(display_usage): Add help strings for --devel-pkg1/--devel-pkg2.
(compare): Make the overload that compares elf files take private
types suppressions. Add the private types suppressions to the
diff context.
(pthread_routine_compare): Adjust the call to compare.
(maybe_create_private_types_suppressions): Define new static
function.
(pthread_routine_extract_pkg_and_map_its_content): If a devel
package was specified for the main package then extract it in
parallel with the other package extraction. When the extraction
is done, create private types suppressions by visiting the
directories that contain the header files.
(compare): In the overload that compares packages by scheduling
comparison of individual elf files that are in the packages, pass
in the private type suppressions too.
(parse_command_line): Parse the new --devel-pkg{1,2} command line
options.
(main): Associate the devel package to the main package, if the
--devel-pkg{1,2}.
* doc/manuals/abidiff.rst: Add documentation about the new
--headers-dir1 and --headers-dir2 options.
* doc/manuals/abipkgdiff.rst: Likewise, add documentation about
the new --devel-pkg1 and --devel-pkg2 libraries.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt:
New test reference output.
* tests/data/test-diff-pkg/tbb-devel-4.1-9.20130314.fc22.x86_64.rpm:
New test input package.
* tests/data/test-diff-pkg/tbb-devel-4.3-3.20141204.fc23.x86_64.rpm: Likewise.
* tests/test-diff-pkg.cc b/tests/test-diff-pkg.cc
(InOutSpec::{first,second}_in_devel_package_path): New data
members.
(in_out_specs): Adjust. Also, add a new entry describing the new
test inputs above.
(test_task::perform): When the new test entry contains devel
packages, pass them to abipkgdiff using the --devel1 and --devel2
options.
* tests/data/test-diff-suppr/test30-include-dir-v0/test30-pub-lib-v0.h:
A new test input source code.
* tests/data/test-diff-suppr/test30-include-dir-v1/test30-pub-lib-v1.h: Likewise.
* tests/data/test-diff-suppr/test30-priv-lib-v0.cc: Likewise.
* tests/data/test-diff-suppr/test30-priv-lib-v0.h: Likewise.
* tests/data/test-diff-suppr/test30-priv-lib-v1.cc: Likewise.
* tests/data/test-diff-suppr/test30-priv-lib-v1.h: Likewise.
* tests/data/test-diff-suppr/test30-pub-lib-v0.cc: Likewise.
* tests/data/test-diff-suppr/test30-pub-lib-v0.so: Add new test
binary input.
* tests/data/test-diff-suppr/test30-pub-lib-v1.cc: Add new test
input source code.
* tests/data/test-diff-suppr/test30-pub-lib-v1.so: Add new test
binary input.
* tests/data/test-diff-suppr/test30-report-0.txt: Add new test
reference output.
* tests/data/test-diff-suppr/test30-report-1.txt: Add new test
reference output.
* tests/test-diff-suppr.cc (InOutSpec::headers_dir{1,2}): New data
members.
(InOutSpec::abidiff_options): Renamed the bidiff_options data
member into this.
(in_out_specs): Adjust. Also, added the new test input above to
this.
(main): Adjust to invoke abidiff with the new --hd1 and --hd2
options if the input specs for the tests has the new
InOutSpec::headers_dir{1,2} data member set. Renamed bidiff into
abidiff.
* tests/data/Makefile.am: Add the new test inputs to the source
distribution.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Right now, the number of removed functions that is shown in the change
report is the total number of removed functions.
But it should be the *net* number of removed functions. That is, the
total number of removed functions minus the number of removed
functions that were filtered out.
This patch fixes that. A one liner.
* src/abg-suppression.cc (corpus_diff::report): Show the net
number of removed functions, not the total number of the removed
functions.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
There was not user documentation about how to execute regression
tests. This patch fixes that by adding a description of the
regression tests and how to launch them.
I have also added a new "make check-valgrind-recursive" target that
execute the tests under memcheck by tracing children processes too,
notably libabigail command line tool processes.
* CONTRIBUTING: Add a section about regression tests.
* Makefile.am: Add a check-valgrind-recursive target.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Profiling showed that getting the pointed-to type of a pointer type is
on the hot spot when comparing corpora with a lot of entry points with
a lot of pointer types reachable from the entry points of the corpora.
So avoid handling the shared_ptr to the pointed-to type for that case
can save us a few CPU cycles in a noticeable way.
This patch does this by creating a new
pointer_type_def::get_naked_pointed_to_type() (that returns a naked
pointer) to supplement the classic
pointer_type_def::get_pointer_to_type() function, and uses that in a
code spot that profiling revealed to be a critical path.
* include/abg-ir.h (pointer_type_def::get_naked_pointed_to_type):
Declare new member function.
* src/abg-ir.cc (pointer_type_def::priv::naked_pointed_to_type_):
New data member.
(pointer_type_def::priv::priv): Adjust to initialize the new data
member.
(pointer_type_def::pointer_type_def): Adjust to use the
constructor pointer_type_def::priv::priv to initialize the
pointed-to type (including its new naked pointer variant). So we
do not have to initialize the priv_->pointed_to_type_ explicitely
in the constructor anymore.
(pointer_type_def::get_naked_pointed_to_type): Define new data
member.
(pointer_type_def::get_qualified_name): Use a naked pointer to the
pointed-to type, rather than a smart pointer.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Profiling shows that some smart pointers were unnecessarily created
here and there, and that had a noticeable effect on performance, when
comparing two gtk3 packages.
This patch passes references to smart pointers in those cases.
* include/abg-fwd.h (get_type_name): Take a reference to type_sptr.
* src/abg-ir.cc (get_type_name): Take a reference to type_sptr.
(suppression_base::priv::{get_file_name_regex,
get_file_name_not_regex, get_soname_regex, get_soname_not_regex}):
Return a reference to regex_t_sptr.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Profiling showed that abigail::tools_utils::string_ends_with can be
quite a hot spot when evaluating suppression specifications that
involve file names in the source location of definition of files.
In that function, it appears that we were calling string::length
several times on the same strings. Calling it just once does make a
noticeable difference when type suppressions involving source
locations are evaluated against a lot of types.
This patch thus calls string::length in
abigail::tools_utils::string_ends_with just once.
* src/abg-tools-utils.cc (string_ends_with): Call string::length
just once on each instance of string that matters.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When evaluating type suppression specifications, looking at the name
of the type can take a noticeable time, at least when the name of that
type is being queried for the first time.
Now that every single type involved in a change that might be reported
is involved in a type suppression evaluation (at least when operating
in the mode where the comparison tool is asked to filter out private
types), the evaluation of type suppression becomes a hot spot, at
least on relatively big diff graphs.
Luckily, private type suppression specifications don't involve the
name of the type, so we don't have to query the name of the type in
this case.
This patch makes it so that the type name is queried only when the
suppression specification requires it. It makes us save a few
noticeable CPU cycles when evaluating suppression specifications that
don't involve looking at the type name.
* src/abg-suppression.cc (type_suppression::suppresses_type): If
neither the type suppression "name" or "name_regex" properties
where provided in the suppression specification, then do not try
to look at the type name.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
On certain work loads, cycles in the tree of diffs of member variables
can appear. This because:
1/ each diff tree nodes holds a reference on each of its children
nodes
2/ each var_diff tree node holds a reference on the diff tree node
representing its type changes.
There can thus be reference cycles in involving diff the tree --> child
node relationship and the var_diff tree --> type diff tree
relationship.
This patch fixes the issue be make sure that a diff tree node does not
hold a reference on its children nodes. That is, rather than having a
vector of shared pointers to its children diff nodes, it has a vector
of naked pointers to those. The patch goes further by making sure
that a var_diff node does not hold a reference to its type diff node
either. It holds a weak pointer to the type diff node, rather than a
shared pointer.
The patch should be followed by a patch make sure that all kinds of
diff nodes follow this pattern; that is, if a diff node needs to carry
a sub-type diff node, it should do so by either using a naked pointer
to the sub-type diff node, or a weak pointer.
For now, the patch fixes the leaks reported by running all the tests
of the suite under Valgrind.
* include/abg-comparison.h (diff_wptr, unordered_diff_sptr_set): New typedefs.
(struct diff_sptr_hasher): Define new type.
(diff_context::keep_diff_alive): Declare new member function.
(diff::children_nodes): Return a vector of diff*, rather than a
vector of diff_sptr.
* src/abg-comparison.cc (diff_context::priv::live_diffs_): New
data member.
(diff_context::keep_diff_alive): Define new data member.
(diff::priv::children_): Make this be a vector of diff*, rather
than a vector of diff_sptr.
(diff_less_than_functor::operator()): Add a new overload for
diff*. Make the existing overload of diff_sptr use the new one.
(diff::children_nodes): Adjust;
(diff::append_child_node): Make sure the child node is kept
alive. Only add the naked pointer to the child node to the vector
of children.
(diff::traverse): Adjust.
(var_diff::priv::type_diff_): Make this be a weak pointer, rather
than a shared pointer.
(var_diff::type_diff): The var_diff::priv::type_diff_ data member
is now a weak pointer, so make this accessor convert it to a
shared pointer.
(corpus_diff::priv::children_): Turn this into a vector of diff*,
rather than a vector of diff_sptr.
(corpus_diff::children_nodes): Adjust.
(corpus_diff::append_child_node): Make sure the child node is kept
alive. Only add the naked pointer to the child node to the vector
of children.
(category_propagation_visitor::visit_end): Adjust.
(suppression_categorization_visitor::visit_end): Adjust.
(redundancy_marking_visitor::{visit_begin, visit_end}): Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Diff not chilrend nodes must remain sorted. Before this patch, this
was achieved by sorting the vector of children each time a new member
was inserted.
But profiling showed that doing that for corpus_diff nodes was
relatively slow on corpora with lots function/variable changes. At
least enough to become a hot spot.
This patch addresses that by inserting the new child at the right
point in the vector of children, for the vector to remain sorted.
* src/abg-comparison.cc (corpus_diff::append_child_node): Insert
the new child at the right point in the vector of children, so
that it remains sorted.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
On recent elfutils where the libdw's function dwarf_getalt exists, we
don't need to try and find the alternate debug info file ourselves by
using the function dwfl_standard_find_debuginfo. Furthermore, when we
use that function on those recent elfutils versions, we leak the elf
resources allocated in the debug info resources; we also leak the file
descriptor to access the alternate debug info sections.
More generally, we also leak debug info handles used to access debug
info when using get_soname_of_elf_file and get_type_of_elf_file.
This patch plugs those leaks.
In the first case, if the function dwarf_getalt exists, the patch just
uses it to get the alternate debug info. Otherwise, the patch uses
the dwfl_standard_find_debuginfo function like we used to, but then it
tries hard to free the file descriptor and debuginfo memory of the
alternate debug info.
* configure.ac: Check the presence of dwarf_getalt in libdw. If
it's present, define the preprocessor macro
LIBDW_HAS_DWARF_GETALT. Update the autoconf configuration
summary.
* src/abg-dwarf-reader.cc: Add config.h.
(find_alt_debug_info_location): Factorize this out of ...
(find_alt_debug_info): ... this function. Use dwarf_getalt if
present, otherwise, keep using dwfl_standard_find_debuginfo. In
the later case, return the file descriptor opened to access the
alternate debug info, by parameter, so that the caller can fclose
it.
(read_context::alt_fd_): New data member.
(read_context::read_context): Initialize the new alt_fd_ data
member.
(read_context::load_debug_info): Store the file descriptor used to
access the alternate debug info into the new alt_fd_ data member.
(read_context::~read_context): New desctructor.
(get_soname_of_elf_file, get_type_of_elf_file): Free the elf
handle.
(read_context::load_debug_info): Be paranoid in making sure we
never override alt_dwarf_.
* tests/data/test-alt-dwarf-file/test0-report.txt: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Some time ago, a memory optimization was put in place to reduce the
memory usage of the IR used to represent the result of a comparison
between two corpora.
The principle of that optimization was to share the data of the
class_diff::priv member among instances of class_diff that are in the
same class of equivalence.
In practice, the class_diff::priv member of an instance that has a non
empty class of equivalence was set to the class_diff::priv of its
canonical type.
Because clas_diff::priv is a shared pointer, setting it to the
class_diff::priv of another instance was creating cycles in the graph
of class_diff, sometimes. And those cycles lead to memory leaks as
the reference count of shared pointers to class_diff would not go down
to zero anymore.
This patch fixes the problem of those cycles by not setting the
class_diff::priv data member. Rather, when a class_diff is part of a
non-empty class of equivalence, its class_diff::priv is left nil. A
new class_diff::get_priv() accessor is provided; it returns the shared
class_diff::priv of the canonical type when the current
class_diff::priv is nil; otherwise it just returns the current
class_diff::priv.
* include/abg-comparison.h (class_diff::get_priv): Declare new
member function.
(class_diff::get_priv): Define new member function.
(class_diff::{chain_into_hierarchy, base_changes, deleted_bases,
inserted_bases, changed_bases, base_changes, member_types_changes,
member_types_changes, data_members_changes, inserted_data_members,
deleted_data_members, member_fns_changes, changed_member_fns,
member_fns_changes, deleted_member_fns, inserted_member_fns,
member_fn_tmpls_changes, member_class_tmpls_changes,
member_class_tmpls_changes, report}): Rather than accessing
class_diff::priv directly, use the new class_diff::get_priv.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It appears that there are cases where the data member
class_decl::priv::definition_of_declaration_ can point to the current
instance of class_decl, leading to a circular reference and thus a
leak because the reference count of the current instance of class_decl
will never reach zero.
This patch fixes that by making
class_decl::priv::definition_of_declaration_ be a weak pointer, rather
than a smart pointer.
* include/abg-ir.cc (class_decl::get_definition_of_declaration):
Return a shared pointer, rather than a reference to a shared pointer.
* src/abg-ir.cc (class_decl::priv::definition_of_declaration_):
Make this be a weak pointer.
(class_decl::get_definition_of_declaration):
Likewise. And return the shared pointer built out of the weak
pointer we have in there now.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The suppression engine was not creating the shared pointer to
regex_t in the proper way that would correctly free the memory
allocated by the glibc's regcomp function.
This patch fixes that.
* include/abg-sptr-utils.h (build_sptr<T>): Declare an overload that
allocates a T* and wraps it into a shared_ptr<T>.
(build_sptr<regex_t>): Declare a specialization for regex_t.
* src/abg-corpus.cc (build_sptr<regex_t>()): Define the
specialization here.
* src/abg-suppression.ccp
(suppression_base::priv::{get_file_[not]_name_regex,
get_soname_[not]_regex}): Use the new build_sptr<regex_t>().
(type_suppression::priv::{get_type_name_regex,
get_source_location_to_keep_regex}): Likewise.
(function_suppression::parameter_spec::priv::get_type_name_regex):
Likewise.
(function_suppression::priv::{get_name_regex,
get_return_type_regex, get_symbol_name_regex,
get_symbol_version_regex}): Likewise.
(variable_suppression::priv::{get_name_regex,
get_symbol_name_regex, get_symbol_version_regex,
get_type_name_regex}): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
After the function compute_diff is invoked to compute the diff against
to corpora, the reference count of the diff_context_sptr that it takes
get incremented several times.
So the diff_context_sptr is not automatically released (freed) when
its scope is left.
The reason why the reference count of diff_context_sptr is incremented
is because several diff types (the diff type itself or types extending
it) actually holds a reference to the diff_context_sptr they get
created with. So the lifetime of the diff_context_sptr becomes tied
to the lifetime the different diff objects in a hard-to-predict way.
It actually creates some cyclic references that, in this case, creates
a leak. The diff_context_sptr never gets released.
This patch fixes that by making the diff types hold a weak reference
to the diff_context_sptr they are created with.
* src/abg-comparison.cc (diff::priv::ctxt_): Make this a weak_ptr.
(diff::priv::get_context): Convert the weak pointer to the context
into a shared_ptr and return it.
(diff::priv::is_filtered_out): Adjust to use
diff::priv::get_context() to access the context.
(diff::context): Likewise.
(corpus_diff::priv::ctxt_): Make this a weak_ptr.
(corpus_diff::priv::priv): Add a new overload that takes two
corpora and a diff context.
(corpus_diff::priv::get_context): Convert the weak pointer to the
context into a shared_ptr and return it.
(corpus_diff::priv::ensure_lookup_tables_populated): Adjust to use
the new corpus_diff::priv::get_context to get the context.
(variable_is_suppressed): Likewise.
(corpus_diff::priv::{apply_suppressions_to_added_removed_fns_vars,
apply_filters_and_compute_diff_stats, emit_diff_stats,
categorize_redundant_changed_sub_nodes,
clear_redundancy_categorization}): Likewise.
(corpus_diff::{corpus_diff, context,
apply_filters_and_suppressions_before_reporting}): Adjust.
* tools/abipkgdiff.cc (compare): Make the overload that compares
elf binaries take a diff context output parameter. After the
context is created by this function, it's return to the caller, so
that it's life time is bound to the scope this function was
called from.
(pthread_routine_compare): Create a shared pointer to hold a
reference on a diff context. Pass that shared pointer by
reference to the compare function that compares elf binaries.
Rather than storing corpora in the reports_map, (as those corpora
would then out-live the diff context and thus create memory
corruption issues), emit the report directly into an ostringstream
and store that stream in reports_map.
(compare): In the overoad that compares packages, rather than
trying to get corpora from the report_map, just emit the content
of the ostringstream that is now there.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When reading in an array of the GNU ELF hash table, we read one
integer right after the end of the array.
This patch fixes that.
* src/abg-dwarf-reader.cc (lookup_symbol_from_gnu_hash_tab): Do
not read passed the end of the array.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch allows maintainers to run:
make -C <builddir>/tests check-valgrind
This runs the test suite under the Valgrind memory checker.
It also adds this target:
make -C <builddir>/tests check-valgrind-memcheck-recursive
It runs the memcheck tool on the tests so that programs forked by them
are memchecked too. This is to allow to memcheck the libabigail tools
that are forked by the individual tests.
* autoconf-archive/ax_valgrind_check.m4: Add new file. Copied it
from http://www.gnu.org/software/autoconf-archive/ax_valgrind_check.html.
* configure.ac: Include the new ax_valgrind_check.m4 file.
Initialize the valgrind checking on tests. Update the configure
status.
* tests/test-valgrind-suppressions.supp: New valgrind suppression
file to silence memcheck leak errors from python.
* tests/Makefile.am: Add test-valgrind-suppressions.supp to source
distribution. Add check-valgrind-memcheck-recursive target.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Explain how ChangeLog entries need to refer to each modified
function. Also, give more information more about the expected spirit
of the commit log.
* COMMIT-LOG-GUIDELINES: Various enhancements.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Positional argument specifiers can be omitted for example '{} {}'. This
is introduced in Python 2.7. Not sure if fedabipkgdiff would be used by
someone with Python 2.6, anyway using consistent string format is a good
way.
* tools/fedabipkgdiff (download_rpm): do not omit positional
argument specifiers in string format.
Signed-off-by: Chenxiong Qi <cqi@redhat.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This is an autogenerated file, it has nothing to do in the repository.
* config.h.in: Remove from repository.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
fedabipkgdiff is a convenient way to compare the ABI of Fedora
packages easily.
The first version of fedabipkgdiff introduced by this patch lets users
perform operations like:
fedabipkgdiff --from fc23 foo-0.1-1.fc23.x86_64.rpm
fedabipkgdiff --from fc23 --to fc24 foo
fedabipkgdiff foo-0.1-1.fc23 foo-0.1-1.fc24
fedabipkgdiff foo-0.1-1.fc23.i686 foo-0.1-1.fc24.i686
fedabipkgdiff --all-subpackages foo-0.1-1.fc23 foo-0.1-1.fc24
* autoconf-archive/ax_compare_version.m4: New file copied from the
autoconf-archive project.
* autoconf-archive/ax_prog_python_version.m4: Likewise.
* autoconf-archive/ax_python_module.m4: Likewise.
* Makefile.am: Add the new files above to the source distribution.
* configure.ac: Include the new m4 macros from the autoconf
archive. Add a new --enable-fedabipkgdiff option. Update the
report at the end of the configure process to show the status of
the fedabipkgdiff feature. Add check for prerequisite python
modules argparse, glob, logging, os, re, shlex, subprocess, sys,
itertools, urlparse, itertools, shutil, unittest, xdg, koji and
mock. These are necessary for the unit test of
fedabipkgdiff. Generate tests/runtestfedabipkgdiff.py into the
build directory, from the tests/runtestfedabipkgdiff.py.in input
file.
* tools/Makefile.am: Include the fedabipkgdiff to the source
distribution and install it if the "fedabipkgdiff" feature is
enabled.
* tests/Makefile.am: Rename runtestfedabipkgdiff.sh into
runtestfedabipkgdiff.py. Add the new runtestfedabipkgdiff.py.in
autoconf template file in here.
* tests/runtestfedabipkgdiff.py.in: New unit test file.
* tools/fedabipkgdiff: New fedabipkgdiff tool.
* doc/manuals/fedabipkgdiff.rst: New manual.
Signed-off-by: Chenxiong Qi <cqi@redhat.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
abidiff, abipkgdiff and abicompat now recognize a [suppress_file]
directive in suppression specifications. That directive instructs the
tool to avoid loading some binaries altogether.
This is the first directive that won't act on the result of the
comparison of two binaries. It actually acts earlier and prevents the
tool from loading some binaries altogether.
The directive looks like:
[suppress_file]
# Don't load any library named lib_private*.so
file_name_regexp = lib_private.*\\.so
This prevents the tool from loading (and thus comparing) any library
which name matches the pattern "lib_private*.so".
[suppress_file]
# Only load libraries name lib_public*.so
file_name_not_regexp = lib_public.*\\.so
This instructs the tool to only load (and compare) files which name
match the pattern "lib_public*.so".
* doc/manuals/libabigail-concepts.rst: Document the new
'suppress_file' directive.
* include/abg-suppression.h (file_suppression): Define new class.
(file_suppression_sptr): Define new typedef.
(is_file_suppression, file_is_suppressed): Declare new functions.
* src/abg-suppression.cc ():
(read_file_suppression, is_file_suppression, file_is_suppressed):
Define new functions.
(file_suppression::{file_suppression, suppresses_file,
~file_suppression}): Define new member functions.
* tools/abidiff.cc (main): If a suppression specification
suppresses one of the input files, then do not perform the
comparison.
* tools/abipkgdiff.cc (compare): If a suppression specification
suppresses a file that is to be compared, then do not perform the
comparison.
* tools/abicompat.cc (create_diff_context): New static function.
(perform_compat_check_in_normal_mode)
(perform_compat_check_in_weak_mode): Adjust to take a context in
parameter. Do not create a diff context here anymore, do not load
suppression files here either.
(main): Use the new create_diff_context to create a diff context
and initialize it, including loading suppression specifications.
If any suppression specification suppresses a file to load, then
do not load perform any compatibility checking. Adjust
invocations of perform_compat_check_in_weak_mode and
perform_compat_check_in_normal_mode to pass the diff context.
* tests/data/test-diff-suppr/test0-type-suppr-3.suppr: New test
input.
* tests/data/test-diff-suppr/test0-type-suppr-4.suppr: Likewise.
* tests/data/test-diff-suppr/test0-type-suppr-report-4.txt: Likewise.
* tests/data/test-diff-suppr/test0-type-suppr-5.suppr: Likewise.
* tests/data/test-diff-suppr/test0-type-suppr-report-5.txt:
Likewise.
* tests/data/test-diff-suppr/test0-type-suppr-6.suppr: Likewise.
* tests/data/test-diff-suppr/test0-type-suppr-report-6.txt:
Likewise.
* tests/data/test-diff-suppr/test0-type-suppr-report-7.txt:
Likewise.
* tests/test-diff-suppr.cc (in_out_specs): Use the new test
inputs.
* tests/data/test-abicompat/test0-fn-changed-1.suppr: New test
input.
* tests/data/test-abicompat/test0-fn-changed-report-3.txt:
Likewise.
* tests/test-abicompat.cc (in_out_specs):: Use the new test
inputs.
* tests/data/Makefile.am: Add the new test material to source
distribution.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Until now, the suppression engine was part of the comparison engine.
The code of both was in the abg-comparison.{cc,h} files.
For the sake of greater modularity, this patch separates the suppression
engine from the comparison engine. The suppression engine now lives
in include/abg-suppression.h and src/abg-suppression.cc. The patch
also updates logical consumers of the suppression engine to adapt them
to the change.
* include/Makefile.am: Add abg-suppression.h to source
distribution.
* include/abg-comparison.h: Remove abg-ini.h include directive.
(suppression_sptr, suppressions_type): Move these typedefs to
abg-fwd.h.
(class suppression_base, type_suppression)
(type_suppression::insertion_range)
(type_suppression::insertion_range::boundary)
(type_suppression::insertion_range::integer_boundary)
(type_suppression::insertion_range::fn_call_expr_boundary)
(function_suppression, function_suppression::parameter_spec)
(variable_suppression): Move these type definitions to the new
abg-suppression.h.
(read_suppressions, is_type_suppression, is_integer_boundary)
(is_fn_call_expr_boundary, is_function_suppression)
(is_variable_suppression, operator&)
(operator|): Move these function declarations to the new
abg-suppression.h.
(type_suppression, type_suppression_sptr, type_suppression_type)
(function_suppression, function_suppression_sptr)
(function_suppressions_type, variable_suppression)
(variable_suppression_sptr, variable_suppressions_type): Move
these forward declaration and typedefs to the new
abg-suppression.h.
(diff_context::suppressions): Adjust return type to
suppr::suppressions_type&.
(diff_context::add_suppression): Adjust parameter type to
suppr::suppressions_sptr.
(diff_context::add_suppressions): Adjust parameter type
suppr::suppressions_type&.
(is_type_diff, is_decl_diff, is_var_diff, is_function_decl_diff)
(is_pointer_diff, is_reference_diff, is_fn_parm_diff)
(is_base_diff, is_child_node_of_function_parm_diff)
(is_child_node_of_base_diff): Declare these new functions. They
were previously static, local to abg-comparison.cc only. Now they
need to be exported because they are used by the suppression
engine's code that now lives in its one files.
* include/abg-fwd.h (suppr::{suppression_base, suppression_sptr,
suppressions_type}): Forward declare these here.
* include/abg-suppression.h (class suppression_base)
(type_suppression, type_suppression::insertion_range)
(type_suppression::insertion_range::boundary)
(type_suppression::insertion_range::integer_boundary)
(type_suppression::insertion_range::fn_call_expr_boundary)
(function_suppression, function_suppression::parameter_spec)
(variable_suppression): Move these type definitions here, in the
namespace suppr.
(read_suppressions, is_type_suppression, is_integer_boundary)
(is_fn_call_expr_boundary, is_function_suppression)
(is_variable_suppression, operator&)
(operator|): Move these function decalration here, in the
namespace suppr.
(type_suppression_sptr, type_suppressions_type)
(function_suppression_sptr, function_suppressions_type)
(variable_suppression_sptr, variable_suppressions_type): Move
these typedefs here, in the namespace suppr.
* src/Makefile.am: add src/abg-suppression.cc to source
distribution.
* src/abg-comparison.cc (is_type_diff, is_decl_diff, is_var_diff)
(is_function_decl_diff, is_pointer_diff, is_reference_diff)
(is_reference_or_pointer_diff, is_fn_parm_diff, is_base_diff)
(is_child_node_of_function_parm_diff, is_child_node_of_base_diff):
Export these functions.
(*suppression*): Move all the suppression-related definitions to
the new abg-suppression.cc.
* src/abg-suppression.cc: New file. Contains all the *suppression*
definitions from src/abg-comparison.cc, that are put in the suppr
namespace.
* tools/abicompat.cc: Adjust.
* tools/abidiff.cc: Likewise.
* tools/abipkgdiff.cc: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch adds the following convenience command line options
shortcuts:
--suppr, --appd, --libd1, --libd2
for the following command line options:
--suppressions, --app-debug-info-dir, --lib-debug-info-dir1,
--lib-debug-info-dir2
The patch also updates the documentation accordingly.
* doc/manuals/abicompat.rst: Update documentation.
* tools/abicompat.cc (display_usage): Update help strings.
(parse_command_line): Add shortcuts --suppr, --appd, --libd1 and
--libd2.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When a suppression specification file was not found, from the command line
of abidiff, the error message fails to mention the name of the tool.
This patch fixes that.
* src/abg-tools-utils.cc (emit_prefix): Try to emit the prefix
only if the program name was provided.
* abidiff.cc (maybe_check_suppression_files): Pass the name of the
tool to the check_file function.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* doc/manuals/libabigail-concepts.rst: Do not refer to abidiff
specifically for suppressions because several tools use
suppressions.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
