This patch makes abipkgdiff compare two kernel packages. At the
moment the comparison is done by comparing each binary from the first
package to its counterpart in the second package. No optimization is
done do represent a vmlinux binary and its modules as one single
entity. So this is different from what kmidiff does.
* include/abg-suppression.h
(variable_suppression::variable_suppression): Add default arguments
to the parameters.
* include/abg-tools-utils.h (dir_exists, dir_is_empty)
(string_begins_with, get_rpm_name, get_rpm_arch, get_deb_name)
(file_is_kernel_package, file_is_kernel_debuginfo_package):
Declare new functions.
* src/abg-tools-utils.cc (dir_exists, dir_is_empty)
(string_begins_with, get_deb_name, get_rpm_name, get_rpm_arch)
(file_is_kernel_package, file_is_kernel_debuginfo_package): Define
new functions.
(gen_suppr_spec_from_kernel_abi_whitelist): The kernel ABI
whitelist is made of ELF symbols names that ought to match
functions *and* variables that have ELF symbols with those names.
So generate variable suppression specifications as well. Not just
function suppression specifications.
* tools/abipkgdiff.cc (options::{kabi_whitelist_package,
show_symbols_not_referenced_by_debug_info, kabi_whitelist_paths,
kabi_suppressions}): New data members.
(options::options): Adjust.
(package::KIND_KABI_WHITELISTS): New enumerator in the
package::kind enum.
(package::kabi_whitelist_package_): New data member.
(package::{base_name, kabi_whitelist_package, }): New member
functions.
(display_usage): Add a help string to the new
--linux-kernel-abi-whitelist and --no-unreferenced-symbols
options.
(parse_command_line): Parse the new --no-unreferenced-symbols,
--linux-kernel-abi-whitelist and --lkaw-pkg options.
(maybe_check_suppression_files): Check the presence of kabi
whitelist files.
(set_diff_context_from_opts): Consider (not) showing symbols not
referenced by debug info.
(compare): If we are looking at linux kernel packages, take the
kernel abi whitelist into account, apply the suppressions
resulting from the kabi whitelists to the ELF read context.
(maybe_collect_kabi_whitelists)
(get_kabi_whitelists_from_arch_under_dir)
(maybe_handle_kabi_whitelist_pkg, maybe_collect_kabi_whitelists)
(get_interesting_files_under_dir): Define new functions.
(maybe_update_vector_of_package_content): Take a new
file_name_to_look_for parameter.
(create_maps_of_package_content)
(extract_package_and_map_its_content): Consider the case of the
package being a linux kernel package.
(main): Take the potential --lkaw-pkg into account.
* doc/manuals/abipkgdiff.rst: Add documentation for options
--linux-kernel-abi-whitelist, --lkaw-pkg and
--no-unreferenced-symbols.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Sometimes, it can happen that a translation unit gets read twice while
loading an abixml. This patch fixes that.
* src/abg-reader.cc (read_translation_unit): Take (in parameter) a
reference as the resulting translation unit.
(get_or_read_and_add_translation_unit): Define new static
function.
(read_context::get_scope_for_node)
(read_translation_unit_from_input): Use the new
get_or_read_and_add_translation_unit.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* include/abg-dwarf-reader.h (set_read_context_corpus_group)
(read_and_add_corpus_to_group_from_elf, set_ignore_symbol_table)
(get_ignore_symbol_table): Declare new functions.
* abg-dwarf-reader.cc (read_context::options_type): Define new
type.
(die_dependant_container_set::clear): Define new member function.
(read_context::{bss, tesxt, rodata, data, data1}_section_): Add
new data members.
(read_context::{symbol_versionning_sections_loaded_,
symbol_versionning_sections_found_}): Likewise.
(read_context::corpus_group_): Likewise.
(read_context::{load_in_linux_kernel_mode, load_all_types,
show_stats, do_log_}): Replace these options by ..
(read_context::options_): ... this instance of the new
read_context:options_type.
(read_context::read_context): Adjust.
(read_context::{clear_alt_debug_info_data, clear_per_corpus_data,
env, get_data_section_for_variable_address, load_all_types,
load_in_linux_kernel_mode, show_stats, do_log}): Adjust.
(create_read_context): Adjust.
(read_context::~read_context): Define destructor.
(read_context::{options, bss_section, text_section,
rodata_section, data_section, data1_section, current_corpus_group,
has_corpus_group, main_corpus_from_current_group,
main_corpus_from_current_group,
current_corpus_is_main_corpus_from_current_group,
should_reuse_type_from_corpus_group}): Define new member
functions.
(read_context::get_die_qualified_type_name): Handle the name of
the current translation unit.
(read_context::load_symbol_maps): Really don't load (linux kernel
specific) symbol maps if we were told to ignore the ELF symbol
table.
(set_ignore_symbol_table, get_ignore_symbol_table)
(create_default_var_sym, create_default_fn_sym, add_symbol_to_map)
(set_read_context_corpus_group)
(read_and_add_corpus_to_group_from_elf): Define new functions.
(build_type_decl, build_typedef_type, build_enum_type)
(add_or_update_class_type)
(add_or_update_union_type): Reuse the type being built, from the
main corpus of the corpus group.
(build_qualified_type): Cleanup logic.
(build_var_decl, build_function_decl): Create a default symbol for
the variable or function if we are supposed to ignore the symbol
table of the current binary. Add that symbol to the symbol table
that is created in the read context.
(read_debug_info_into_corpus): Don't load the ELF symbol table
information if we are asked to ignore the symbol table. But set
the symbol table that we built artificially while loading
functions and variables, into the ABI corpus being built.
(read_context::maybe_adjust_var_sym_address): Adjust.
(build_ir_node_from_die): Add ir node to its logical scope. For
the C language, the scope of a type is the global scope.
(read_corpus_from_elf): Don't load ELF properties if we were asked
to avoid the ELF symbol table.
* include/abg-comparison.h (compute_diff): Declare ...
* src/abg-comparison.cc (compute_diff): ... an overload to compare
corpus_group.
* tools/kmidiff.cc: New tool.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch adds support to associate a type to its source location.
The location being a string of the form "filepath:line:column". The
association is done by a per-translation unit map which associates a
location to a type.
For the moment this only associates one type to a given location. For
the general case, though, we need to associate a vector or types to a
given location. We'll add that support later.
The patch provides lookup functions, each of which looking up a
particular kind of type by its location.
* include/abg-fwd.h (get_name_of_qualified_type)
(get_name_of_reference_to_type, lookup_basic_type_per_location)
(lookup_class_type_per_location, lookup_union_type_per_location)
(lookup_enum_type_per_location, lookup_typedef_type)
(lookup_typedef_type_per_location, lookup_pointer_type)
(lookup_reference_type, lookup_type_per_location)
(lookup_type_through_translation_units)
(lookup_type_from_translation_unit, odr_is_relevant): Declare new
functions or new function overloads.
* include/abg-ir.h (location::expand): Declare new member
function.
(type_maps::empty): Likewise.
(operator|=): Declare an overload for qualified_type_def::CV.
(get_string_representation_of_cv_quals)
(get_name_of_qualified_type, lookup_qualified_type): Declare new functions.
* src/abg-ir.cc (location::expand): Define new member function.
(type_maps::empty): Likewise.
(odr_is_relevant): Likewise.
(get_string_representation_of_cv_quals)
(get_name_of_reference_to_type, get_name_of_qualified_type)
(lookup_union_type_per_location): Define new functions or overloads.
(lookup_basic_type, lookup_enum_type, lookup_typedef_type)
(lookup_qualified_type, lookup_pointer_type)
(lookup_reference_type, lookup_type_from_translation_unit)
(lookup_basic_type_per_location, lookup_basic_type_per_location)
(lookup_class_type_per_location, lookup_class_type_per_location)
(lookup_enum_type_per_location, lookup_enum_type_per_location)
(lookup_typedef_type_per_location)
(lookup_typedef_type_per_location, lookup_type_per_location):
Define new overloads.
(maybe_update_types_lookup_map)
(maybe_update_types_lookup_map<class_decl>)
(maybe_update_types_lookup_map<function_type>): Add a new
use_type_name_as_key parameter. If it's false, then associates
the type to its location rather than to its name.
(maybe_update_types_lookup_map): In the overloads for type_decl,
class_decl, union_decl, enum_type, typedef_decl, array_type_def,
record the type in the lookup map per location, in addition to the
per-name recording.
(qualified_type_def::build_name): Use the new
get_name_of_qualified_type.
(qualified_type_def::get_cv_quals_string_prefix): Use the new
get_string_representation_of_cv_quals.
(operator|=): Define a new overload for qualified_type_def::CV.
(pointer_type_def::get_qualified_name): Use the new
get_name_of_pointer_to_type.
(reference_type_def::get_qualified_name): Use the new
get_name_of_reference_to_type.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
To support the upcomping analysis of the Linux kernel and its modules,
we need a way to represent a union of corpora. The first corpus
loaded would be the one representing the vmlinux binary. Subsequent
corpora loaded would be those representing the modules.
This patch provides the new abigail::ir::corpus_group type that
represents such a corpus group.
* include/abg-corpus.h (corpus::{find_translation_unit,
get_type_per_loc_map}): Declare new member functions.
(corpus::{get_architecture_name, is_empty}): Make these member functions
const.
(corpus::{get_sorted_fun_symbols, get_functions, get_variables,
get_unreferenced_function_symbols,
get_unreferenced_variable_symbols}): Make these member functions
virtual.
(class corpus_group): Declare a new type.
* include/abg-fwd.h (corpus_sptr, corpus_group_sptr)
(string_tu_map_type, istring_var_decl_ptr_map_type)
(istring_function_decl_ptr_map_type): Define new typedefs.
* src/abg-corpus-priv.h (corpus_priv::{path_tu_map,
type_per_loc_map_}): Add new data members.
* src/abg-corpus.cc (corpus_add): Complete the function comment.
Assert that at most one translation unit of a given path can be
added to the corpus.
(corpus::{find_translation_unit, get_type_per_loc_map}): Define
new member functions.
(corpus::{get_architecture_name}): Make this member function
const.
(struct corpus_group::priv): Define new type.
(corpus_group::{corpus_group, ~corpus_group, add_corpus,
get_corpora, is_empty, get_functions, get_variables,
get_var_symbol_map, get_fun_symbol_map, get_sorted_fun_symbols,
get_sorted_var_symbols, get_unreferenced_function_symbols,
get_unreferenced_variable_symbols}): Define member functions of
the new corpus_group type.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The command "fedabipkgdiff --self-compare --from fc25 grub2" returns
with an error because no debug info can be found for the file
kernel.img. That file doesn't have any debug info shipped for it.
And we shouldn't try to compare it anyway.
This patch updates the default suppression file shipped by libabigail
to make it avoid compare kernel.img files.
* default.abignore: Do not compare kernel.img files.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The build libxcb-1.12-3.fc26 has been tagged for Fedora 27 (i.e,
fc27).
When we ask fedabipkgdiff to get the builds of libxcb for Fedora 27,
it looks for builds which release string ends with 'fc27'. It thus
fails to pick libxcb-1.12-3.fc26.
This patch changes this behaviour by making
Brew.get_package_latest_build to first try to get builds which
release string match exactly the distro string we are looking at.
But then if no build was found, the member function then tries to get
builds for which the distro part of the release string is "less than"
(in a lexicographic way) the distro string we are looking at.
I haven't added any regression test specifically for this because we
are planning to use the libabigail-selfcheck external tool to perform
regression testing on tons of Fedora packages:
https://pagure.io/libabigail-selfcheck. Fixing this issue is a
pre-requisite for putting that regression test infrastructure in
place.
* tools/fedabipkgdiff (get_distro_from_string): Define new function.
(Brew.get_package_latest_build): Also consider builds which distro
property is less than the expected distro string that we were
given.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In fedabipkgdiff tool, some packages might have only
noarch sub-packages. In these cases, no package is
available for running abipkgdiff. This leads to
return_codes list being empty.
* tools/fedabipkgdiff (run_abipkgdiff()): Check if
return_codes list is empty
Signed-off-by: Sinny Kumari <sinny@redhat.com>
When abipkgdiff decides that two packages have no content to compare
it forgets to remove the temporary directories that were created.
* tools/abipkgdiff.cc (maybe_erase_temp_dirs): Define new static
function.
(compare): Call the new maybe_erase_temp_dirs on all return
points.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* include/abg-ini.h (config::section::priv): Make this be a class,
not a struct.
* src/abg-dwarf-reader.cc (build_translation_unit_and_add_to_ir)
(build_ir_node_from_die): Add parenthesis around assignment
expressions inside conditional expression.
* src/abg-suppression.cc (read_function_suppression): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* include/abg-comp-filter.h (class filter_base): Declare this as a
struct.
* include/abg-comparison.h (class filtering::filter_base):
Likewise.
(struct diff_traversable_base): Declare this as a class.
* include/abg-ir.h (function_decl::parameter): Declare this before
using it.
* src/abg-corpus.cc
(corpus::priv::build_unreferenced_symbols_tables): Add missing
parenthesis around assignment expressions inside conditional
expressions.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
fedabipkgdiff tool can communicate with Fedora koji and has
capability to download and perform ABI comparison between
specified NVRs.
With addition of --self-compare option, it will be possible
to perform ABI comparison on same package. One of the important
usecase of this option is to run automated ABI checks on
packages with known expected result i.e. no ABI change. This usecase
will be useful to ensure that libabigail functionality doesn't
break with new commits made.
This option can be invoked as:
fedabipkgdiff -a --self-compare -fc24 package_name
* bash-completion/fedabipkgdiff: Add new option --self-compare
* tests/data/Makefile.am: Add new test file
* tests/data/test-fedabipkgdiff/test7-self-compare-from-fc23-dbus-glib-report-0.txt:
New reference output for testing ABI comparison on same package
* tests/runtestfedabipkgdiff.py.in (FEDABIPKGDIFF_TEST_SPECS):
Add test case for --self-compare
* tools/fedabipkgdiff (build_commandline_args_parser()): Add
new option --self-compare
(generate_comparison_halves()): Find second comparision half in same
package list while doing self-compare
(self_compare_rpms_from_distro()): New function to perform ABI
comparision on same pacakge
(main()): Add if condition when --self-compare option is enabled
The C language doesn't support namespaces. So, in that language, the
context of a type is always the global scope. Hence, the context of a
type DIE is always the global context associated to the current
translation unit.
Thus, when we are analyzing a C binary, we can do away with building
the DIE -> DIE parent map that we later use to get the parent of a
type DIE when we need to determine the context of a given type DIE.
This can save significant time and space.
This patch implements this optimization.
* include/abg-ir.h (global_scope_sptr): Make this be a share_ptr
of scope_decl, not of global_scope.
(translation_unit::get_global_scope): Return a reference to
scope_decl_sptr.
* src/abg-ir.cc (translation_unit::get_global_scope): Return a
scope_decl not a global_scope.
* src/abg-dwarf-reader.cc (read_context::nil_scope_): Add new data
member.
(read_context::{global_scope, nil_scope}): Define new member functions.
(read_context::build_die_parent_maps): Do not build the map if we
are looking at a C (or asm) translation unit.
(get_scope_die, get_scope_for_die): If we are looking at a C
translation unit then do return the global scope.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While emitting an abixml output for a corpus_group representing the
KMI of a Linux kernel tree, profiling shows that during comparison of
class types, calling class_decl::get_definition_of_declaration is a
hotspot. Especially, the fact the function returns a shared pointer
that has to be "handled" shows up in the profile.
This patch introduces a
class_decl::get_naked_definition_of_declaration that returns a
pointer. Not a shared pointer.
The patch then uses that get_naked_definition_of_declaration function
in the comparison code for class_decl.
The patch also uses get_naked_canonical_type instead of
get_canonical_type when comparing a bunch of other types.
This makes things a little bit faster when compiled without
optimization between 2% and 5%.
* include/abg-ir.h
(class_or_union::get_naked_definition_of_declaration): Declare a
new member function.
(class_decl::get_naked_definition_of_declaration): Likewise.
* src/abg-ir.cc ({type_decl, qualified_type_def,
array_type_def, enum_type_decl}::operator==): Use the
get_naked_canonical_type and get_naked.
(class_or_union::priv::naked_definition_of_declaration_): Define
new data member.
(class_or_union::priv::priv): Adjust to initialize the new data
member.
(class_or_union::get_naked_definition_of_declaration): Define new
member function.
({class_or_union, class_decl}::operator==): Use the new
get_naked_definition_of_declaration instead of
get_definition_of_declaration.
(equals): In the overload for class_or_union, do the same.
(class_decl::get_naked_definition_of_declaration): Define new
member function.
(hash_type_or_decl): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Profiling shows that while writting out the abixml for a big
corpus_group, comparing decl-only classes is a hot spot.
This patch avoids calling the function
class_or_union::priv_->comparison_started (which appears to be
culprit) in that case.
This makes writting out the abixml of the corpus_group of the Linux
kernel be ~20% faster.
* src/abg-ir.cc (equals): In the overload for class_decl, if we
are looking at a decl-only class, then directly call the equals
function for class_or_union. That one knows how to perform the
comparison without calling the
class_or_union::priv_->comparison_started function, in that case.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This updates the description of what abipkgdiff in termes of sequence
of actions.
* tools/abipkgdiff.cc: Update the description of the sequence of
actions performed.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-writer.cc (class write_context): Fix indentation.
(write_location, write_visibility, write_binding)
(write_array_size_and_alignment, write_size_and_alignment): Fix
these declarations to use the *_sptr typedefs rather than the
explicit shared_ptr<*> types.
(write_translation_unit): Fix comment.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Suppose we have two types struct A and struct B. Suppose the two
types have members of integer type. Suppose there are members of
integer type in struct A that are modified to become members of char
type. Suppose, at last, that we also have members of integer type in
struct B that are modified to become members of char type.
In that case, we want to see all the changes of members which types
got changed from integer type to char type. None of these changes
should be considered redundant.
Today, unfortunately, only the first change is reported by default.
The subsequent changes are considered to be redundant.
This patch fixes that by considering that changes that modifies the type of a
decl from a basic type into another are never considered redundant.
* include/abg-comparison.h (is_diff_of_basic_type)
(has_basic_type_change_only): Declare these functions ...
* src/abg-comparison.cc (is_diff_of_basic_type)
(has_basic_type_change_only): ... and define them.
(redundancy_marking_visitor::visit_begin): Use the new
has_basic_type_change_only.
* tests/data/test-diff-filter/libtest37-v0.so: New binary test input.
* tests/data/test-diff-filter/libtest37-v1.so: Likewise.
* tests/data/test-diff-filter/test37-report-0.txt: New test
reference output.
* tests/data/test-diff-filter/test37-v0.cc: Source code of the new
binary test input.
* tests/data/test-diff-filter/test37-v1.cc: Likewise.
* tests/data/Makefile.am: Update to add the new test material to
the source distribution.
* tests/test-diff-filter.cc (in_out_spec): Add the new test input
to this test harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When {function, var}_decl::set_symbol is invoked, the cache of of the
ID of the decl must be invalidated because that function changes the
ID of the decl.
This patch does just that.
* src/abg-ir.cc ({function, var}_decl::set_symbol): Invalidate the
ID cache.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Now that there is a is_type predicate that takes a a type_or_decl_base
type, the overloads that take a decl_base or a type_base are useless
and can even lead to overload resolution issues. This patch removes
those useless overloads.
* include/abg-fwd.h (is_type): Remove the overloads that take
decl_base and type_base types.
* src/abg-ir.cc (is_type): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Const references are a useless redundancy because a reference is
always const.
Thus build_qualified_type can return a non-qualified type when it
figures out the qualifier is useless.
This patch adjusts build_qualified_type to recognize that. It makes
it return a type_base_sptr rather than a qualified_type_def_sptr.
In build_ir_node_from_die, in the case where we handle qualified
types, the call to build_qualified_type is adjusted accordingly.
* src/abg-dwarf-reader.cc (build_qualified_type): Return a
type_base_sptr.
(build_ir_node_from_die): Adjust the call to build_qualified_type.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In some case, some array subranges would all have the same lower
bound.
Fixed thus.
* src/abg-dwarf-reader.cc (build_subranges_from_array_type_die):
Consider the 'lower_bound' parameter as the default lower bound
for each sub-ranges.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
I noticed a buffer overrun in the equals overload for arrays. This
patch fixes that.
* src/abg-ir.cc (equals): In the overload for arrays, check for
the end of the subranges of the two arrays, not just for the first
one.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It can happen that queue::priv_::do_bring_workers_down stays forever
waiting for a task to finish (via pthread_join). The it waits for is
itself blocked in worker::wait_to_execute_a_task, in pthread_cond_wait.
It seems to me that this is because we forget to lock the
queue::priv::queue_cond_mutex before inspecting and updating the
variables on which the wait on the condition depend.
This patch fixes that.
The patch also moves tests/test-read-write.cc over to using the work queue
to increase test coverage for the work queue interface.
* src/abg-workers.cc (queue::priv::tasks_todo_mutex): Make this
data member mutable.
(more_tasks_to_execute):
(queue::priv::do_bring_workers_down): Update the
queue::priv::bring_workers_down only in the critical section
defined by queue::priv::queue_cond_mutex.
(worker::wait_to_execute_a_task): Testing for
queue::priv::bring_workers_down is done in the critical section
defined by queue::priv::queue_cond_mutex. The loop over waiting
ont the condition is also in the critical section, as it ought to
be.
* tests/test-read-write.cc (struct test_task): New type.
(main): Express in terms of the new test_task type.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* include/abg-diff-utils.h (print_snake): pass argument of type
snake by const reference.
* include/abg-ir.h (location::operator{==,<}): Likewise.
* include/abg-viz-dot.h (node_base::{node_base,parent_node,child_node}):
Likewise.
* include/abg-viz-svg.h (svg::svg) Likewise.
* src/abg-config.cc (config::config): Member initialization in ctor body.
* src/abg-dwarf-reader.cc (class_decl_sptr::add_or_update_class_type):
Initial value never used.
* src/abg-ir.cc: (decl_base::priv::priv) Member initialization in ctor body,
pass argument of type location by const reference.
(equals): Variable initial value never used.
* src/abg-reader.cc (read_corpus_from_input): Initial variable
value never used.
(build_elf_symbol_db): Use pre-increment.
* src/abg-suppression-priv.h
(suppression_matches_type_location): Pass argument of type
location by const reference.
* src/abg-suppression.cc: Likewise.
Signed-off-by: Ondrej Oprala <ondrej.oprala@gmail.com>
* src/abg-dwarf-reader.cc
(type_or_decl_base_sptr::lookup_artifact_from_per_tu_die_representation):
Fix an error found by cppcheck.
Signed-off-by: Ondrej Oprala <ondrej.oprala@gmail.com>
* scripts/dot_to_png.sh: Clean up the script according to
shellcheck warnings and remarks.
* scripts/dot_to_svg.sh: Likewise.
* scripts/svg_to_plain_svg.sh: Likewise.
* scripts/svg_to_png_and_pdf.sh: Likewise.
Signed-off-by: Ondrej Oprala <ondrej.oprala@gmail.com>
* src/abg-ir.cc (parse_integral_type): An attempt at clang
compilation has discovered there to be a comparison with
unused result, that apparently should be an assignment.
Signed-off-by: Ondrej Oprala <ondrej.oprala@gmail.com>
We are seeing some random cases where the regression test
runtestfedabipkgdiff.py hangs in what seems to be a deadlock. This
seems to happen in fedabipkgidiff when it spawns a process to run
abipkgdiff. More precisely, proc.communicate() hangs.
The documentation of that function at
https://docs.python.org/2/library/subprocess.html#subprocess.Popen.communicate
says:
Note: The data read is buffered in memory, so do not use this
method if the data size is large or unlimited.
So this patch avoids using proc.communicate() because the output of
abipkgdiff *can* be large. Rather, the patch manually waits for the
the spawned abipkgdiff to finish, and then, read its output.
* tools/fedabipkgdiff (abipkgidff): Do not use Popen.communicate()
as it might hang if the data is large. Rather, busy wait for the
abipkgdiff process to finish and then get its output.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Package fails to build for 32bit architectures since size_t is not 64 bits.
Make header consistent with source
* include/abg-fwd.h: Include stdint.h for uint64_t.
(ir::set_data_member_offset): Take uint64_t rather than size_t.
(ir::get_data_member_offset): Return uint64_t rather than size_t.
Signed-off-by: Slava Barinov <v.barinov@samsung.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This fixes a race condition that seems to occur sometimes. That is,
if the fedabipkgdiff test is run last, then it can stay idling for
ever. I'll need to investigate this later.
* tests/Makefile.am: Run the fedabipkgdiff test first.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
References are always const. But then GCC sometimes emits DWARF that
represents a const reference. This leads, for instance, to a given
reference to be considered as different from that same reference wraps
into a const qualifier. Which is wrong.
Libabigail then represents those const references as a particular case
of a "no-op qualifier". That is, a qualifier that should be ignored
by the comparison code.
In the case of this issue, the comparison engine considers the two
references (const and non-const) to be equal, but the reporting code
forgets to ignore the ignore no-op qualifier and thus (wrongly)
considers the two references as being different. That inconsistency
leads to an abort of the process.
This patch moves the code that ignores no-op qualifiers at a lower
level of the comparison engine so that whenever function parameters
are compared, no-op qualifiers are ignored as they should.
* include/abg-fwd.h (look_through_no_op_qualified_type): Declare
new function.
* src/abg-ir.cc (look_through_no_op_qualified_type): Define it.
(compute_diff_for_types): Use the new
look_through_no_op_qualified_type here rather than open-coding it.
(equals): In the overload for function_decl::parameter, use the
new look_through_no_op_qualified_type function.
* tests/data/test-diff-dwarf/test40-PR21296-clanggcc.cc: Source
code of the new test inputs.
* tests/data/test-diff-dwarf/test40-PR21296-clanggcc-report0.txt:
New test input.
* tests/data/test-diff-dwarf/test40-PR21296-libgcc.so: New binary
test input.
* tests/data/test-diff-dwarf/test40-PR21296-libclang.so: Likewise.
* tests/test-diff-dwarf.cc (in_out_specs): Add the new test inputs to
the test harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Cache data, currently containing downloaded RPM packages from Koji, is
stored in XDG_CACHE_HOME. This patch allows user to delete cache before
or after the ABI comparison, or both.
* configure.ac: Require shutil module.
* doc/manuals/fedabipkgdiff.rst: Add document for new option
clean-cache, clean-cache-before, and clean-cache-after.
* tools/fedabipkgdiff (build_commandline_args_parser): Add new
option --clean-cache, --clean-cache-before and
--clean-cache-after.
(diff_local_rpm_with_latest_rpm_from_koji): Delete download
cache directory before or after downloading RPMs.
(diff_latest_rpms_based_on_distros): Likewise.
(diff_two_nvras_from_koji): Likewise.
(diff_from_two_rpm_files): Likewise.
* bash-completion/fedabipkgdiff: Add new options.
* tests/mockfedabipkgdiff.in (get_download_dir): Rewrite to
behave just like the original get_download_dir.
(mock_get_download_dir): Removed.
(DOWNLOAD_CACHE_DIR): New global variable pointing directory
holding packages during tests.
(run_fedabipkgdiff): Mock original get_download_dir with the
rewrite get_download_dir.
* tests/runtestfedabipkgdiff.py.in (run_fedabipkgdiff_tests):
Add --clean-cache to run tests to ensure no regression.
Signed-off-by: Chenxiong Qi <cqi@redhat.com>
* tests/test-valgrind-suppressions.supp: Add a suppression that
occurs during an internal libc signal handling occasion.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When one thread calls queue::wait_to_execute_a_task and another calls
queue::priv::do_bring_workers_down, a data race occurs on the
queue::priv::bring_workers_down variable.
This patch protects the read of the queue::priv::bring_workers_down
variable (in worker::wait_to_execute_a_task) by the
queue::priv::tasks_todo_mutex mutex. Note that that mutex is the one
that protects the write to queue::priv::bring_workers_down in
queue::priv::do_bring_workers_down.
* src/abg-workers.cc (worker::wait_to_execute_a_task): Protect the
read of the queue::priv::bring_workers_down down variable with the
queue::priv::tasks_todo_mutex.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It turns out we allow member function cloning only for functions that
are members of classes, not for functions that are member of unions.
This patch fixes that by turning the method
class_decl::add_member_function into the
class_or_union::add_member_function. Note that there was already an
overload of class_or_union::add_member_function; now, all the member
functions add_member_function are members of the class_or_union type.
The patch then modifies function_decl::clone to make that code avoid
assuming that only member functions of classes can be cloned.
* include/abg-ir.h (class_or_union::add_member_function): Move the
class_decl::add_member_function overload declaration into the
class class_or_union class.
(class class_decl): Make the class class_or_union be a friend of
class_decl.
* src/abg-ir.cc (class_decl::add_member_function): Transform the
definition of this overload into ...
(class_or_union::add_member_function): ... this one. Make sure
that when setting the virtual-ness attributes of the member
function, we are effectively looking at the a function that is a
member of a class.
(function_decl::clone): Do not assert that a member function is
necessarily a member of a class_decl. It can also a member of a
union_decl!. So, rather, assert that the scope of the member
function is of type class_or_union.
* tests/data/test-diff-pkg/tbb-2017-8.20161128.fc26.x86_64.rpm:
New test input RPM.
* tests/data/test-diff-pkg/tbb-2017-9.20170118.fc27.x86_64.rpm:
* tests/data/test-diff-pkg/tbb-debuginfo-2017-8.20161128.fc26.x86_64.rpm:
Likewise.
* tests/data/test-diff-pkg/tbb-debuginfo-2017-9.20170118.fc27.x86_64.rpm:
Likewise.
* tests/data/test-diff-pkg/tbb-2017-8.20161128.fc26.x86_64--tbb-2017-9.20170118.fc27.x86_64.txt:
New reference test output.
* tests/data/Makefile.am: Add the new test input RPMs to the
source distribution.
* tests/test-diff-pkg.cc (in_out_specs): Take the new input tests
above into account.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When two virtual member functions have the same index, same ELF
symbol, same pretty representation and only differ from their source
file paths, then this patch takes the source path into account in the
sorting.
Otherwise, this breaks the runtestreaddwarf test on EL6.
* src/abg-ir.cc (virtual_member_function_less_than::operator()):
Take the file path into account in the sorting.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When two virtual members functions have the same virtual index, the
same pretty representation, and one of them has no ELF symbols, then
they might be sorted in a way or in an other. This sorting hazard
breaks the runtestreaddwarf test on EL6.
This patch addresses that to make the virtual member function that has
no ELF symbol to come before the one with an ELF symbol.
Also, it turned out that runtestannotate is broken on EL6 too because
we are trying to demangle c++11-mangled symbols in there, and the
demangler (which is C++<11 only) on that platform doesn't really like
that. So this patch removes the tests in which there are c++11 symbols
that we try to demangle.
* src/abg-ir.cc (virtual_member_function_less_than::operator()):
Update comment. When two virtual functions have the same virtual
index and one of them has no ELF symbol, then that function is
less than the one with an ELF symbol.
* tests/data/Makefile.am: Remove
test-annotate/{test9-pr18818-clang.so.abi, test11-pr18828.so.abi,
test12-pr18844.so.abi, test16-pr18904.so.abi,
test22-pr19097-libstdc++.so.6.0.17.so.abi}.
* tests/data/test-annotate/test10-pr18818-gcc.so.abi: Remove.
* tests/data/test-annotate/test11-pr18828.so.abi: Likewise.
* tests/data/test-annotate/test12-pr18844.so.abi: Likewise.
* tests/data/test-annotate/test16-pr18904.so.abi: Likewise.
* tests/data/test-annotate/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Likewise.
* tests/test-annotate.cc (in_out_specs): Remove those tests above
which input files have been removed.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* tests/test-valgrind-suppressions.supp: Make Helgrind
suppressions less specific to libgcc_s version.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* tests/test-valgrind-suppressions.supp: Make the ostream writting
suppressions be less specific so that they can apply to all the
related false positives.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Moving this test away from using pthreads directly to use
abigail::workers.
This patch also updates the valgrind suppression file to suppress some
Helgrind false positives. Those are due to:
- libstdc++ apparently having some benign data races when emitting
data to ostream. This seems related to some facet manipulation that
happen at that point.
- some benign data race in some elfutils functions.
* tests/test-read-dwarf.cc (iospec, spec_lock, write_lock)
(out_abi_base, in_elf_base, in_abi_base): Remove these global
variables.
(handle_in_out_spec): Remove this.
(struct test_task): Write this task that does what
handle_in_out_spec was doing.
(test_task_sptr): Define new typedef.
(main): Remove the pthreads artifacts. Use the new test_task type
along with the abigail::workers interface.
* tests/test-valgrind-suppressions.supp: Add more helgrind
suppressions for ostream writting false positives.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* tests/runtestfedabipkgdiff.py.in (run_fedabipkgdiff_tests): When
A test fails, display the fedabipkgdiff command that triggered the failure.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
With this patch, the code of abipkgdiff.cc doesn't use the pthreads
API directly anymore. Rather, it uses the higher level "Workers
Queue" API provided by the abigail::workers namespace.
The main benefits are:
- better code readability and maintainability. The code base doesn't
have any global variable anymore. This is going to be helpful when
adding new features to the abipkgdiff.cc code base.
- faster code. The tests/runtestdiffpkg test program executes on ~
17s without the patch, and on ~ 11s with the patch on my old X220
laptop.
* tools/abipkgdiff.cc: Remove ftw.h, pthread.h, unistd.h, add
fts.h and abg-workers.h.
(verbose, elf_file_paths_tls_key, reports_map, env_map, map_lock)
(arg_lock, prog_options): Remove all these global variables.
(struct package_descriptor): Remove this type.
(pthread_routine_extract_package)
(first_package_tree_walker_callback_fn)
(second_package_tree_walker_callback_fn, pthread_routine_compare)
(pthread_join, pthread_routine_extract_pkg_and_map_its_content):
Remove these functions.
(options::{num_workers, verbose}): Define new data members.
(options::options): Initialize the new verbose and num_workers data members.
(package::erase_extraction_directory)
(erase_created_temporary_directories_parent): Take the program
options in parameter. Don't use the global verbose variable
anymore.
(package::erase_extraction_directories)
(erase_created_temporary_directories, extract_package): Take the
program options in parameter.
(extract_rpm, extract_deb, extract_tar): Likewise. And don't use
the global verbose variable anymore.
(compare): Don't use the global verbose variable anymore. Use the
new compare_task type along with the abigail::workers::queue type.
(pkg_extraction_task, pkg_prepare_task, compare_task)
(comparison_done_notify): Define new classes.
(maybe_update_vector_of_package_content): Define new static
function.
(create_maps_of_package_content): Don't take the ftw_cp_type
anymore. Don't use the global verbose variable anymore. Use the
fts_{open,read,close} functions, rather than the ftw one.
(extract_package_and_map_its_content): Don't use pthreads anymore.
Use the new pkg_extraction_task type created along with the
abigail::workers::queue type.
(prepare_packages): Don't use pthreads anymore. Use the new
pkg_prepare_task type along with the abigail::workers::queue type.
(elf_size_is_greater): Adjust to use
abigail::workers::queue::tasks, rather than the previous
compaer_args_sptr type.
(parse_command_line): Adjust to stop using the global verbose
variable.
(main): Remove use of global variables prog_options and also the
packages variable.
* tests/data/test-diff-pkg/dbus-glib-0.104-3.fc23.x86_64--dbus-glib-0.104-3.fc23.armv7hl-report-0.txt:
Adjust.
* tests/data/test-diff-pkg/dirpkg-0-report-0.txt: Likewise.
* tests/data/test-diff-pkg/test-dbus-glib-0.80-3.fc12.x86_64-report-0.txt:
Likewise.
* tests/data/test-diff-pkg/test-rpm-report-0.txt: Likewise.
* tests/data/test-diff-pkg/test-rpm-report-1.txt: Likewise.
* tests/data/test-diff-pkg/test-rpm-report-2.txt: Likewise.
* tests/data/test-diff-pkg/test-rpm-report-3.txt: Likewise.
* tests/data/test-fedabipkgdiff/test0-from-fc20-to-fc23-dbus-glib-report-0.txt:
Likewise.
* tests/data/test-fedabipkgdiff/test1-from-fc20-to-dbus-glib-0.106-1.fc23.x86_64-report-0.txt:
Likewise.
* tests/data/test-fedabipkgdiff/test2-dbus-glib-0.100.2-2.fc20--dbus-glib-0.106-1.fc23-report-0.txt:
Likewise.
* tests/data/test-fedabipkgdiff/test3-dbus-glib-0.100.2-2.fc20.i686--dbus-glib-0.106-1.fc23.i686-report-0.txt:
Likewise.
* tests/data/test-fedabipkgdiff/test4-glib-0.100.2-2.fc20.x86_64.rpm-glib-0.106-1.fc23.x86_64.rpm-report-0.txt:
Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Under "make check-valgrindk", when valgrind returns errors, these
errors are ignored by make. It turns out it is the autoconf
VALGRIND_CHECK_RULES macro that does this. Fixed thus.
* autoconf-archive/ax_valgrind_check.m4 (check-valgrind): Don't
ignore errors.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* tests/Makefile.am (check-valgrind-helgrind-recursive): New
target to run the tests recursively under the control of
Valgrind's Helgrind tool.
* tests/test-valgrind-suppressions.supp: Update this suppression
file with suppressions for Helgrind.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While making abipkgdiff to use the abigail::workers API to do away
with using pthreads directory, it appeared that the abigail::workers
API needs fixes and enhancements.
Fixes
=====
* Don't try to schedule a task if the pointer to the task is nil
* Fix a data race when bringing workers (of a queue) down
* Always try to wake up all waiting threads when bringing down queue
workers.
* Fix a data race when accessing the queue condition variable
* Fix a data race when notifying listeners about the end of the job
performed by the task.
Enhancements
============
* Pass the "task done" notifier by reference, to the worker queue.
Without this, the worker queue needs to copy the "task done" notifier
by value. This implies that user code needs to provide task done
notifier instances that come with potentially complicated copy
constructors. By passing it by reference and by just re-using the
notifier from the user code, we do away with the need for copying
altogether. This also fixes some latent copying bugs.
* Add a workers::queue::schedule_tasks() method
This allows user code to schedule a vector of tasks at once.
* make workers::queue::get_completed_tasks() return a non-const vector
This enables user code to sort the completed tasks as they wish.
* include/abg-workers.h (queue::tasks_type): New typedef.
(queue::queue): Pass task_done_notify by reference.
(queue::schedule_tasks): Declare new member function.
(queue::get_completed_tasks): Return non-const vector.
* src/abg-workers.cc (queue::priv::default_notify): New data
member.
(queue::priv::notify): Make this data member be a reference.
(queue::priv::priv): Initialize the notify data member to either
the new default_notify (if no notifier is provided by the
constructor) or to the notifier provided by the constructor.
(queue::priv::schedule_task): Do not schedule a nil task. Update
comment.
(queue::priv::schedule_tasks): Add a new member function.
(queue::priv::do_bring_workers_down): Update comment. Protect
access to "bring_workers_down" with tasks_todo_mutex to prevent a
data race. Call pthread_cond_broadcast on the queue_cond
unconditionaly to prevent some worker threads to keep waiting for
ever. Also, protect the access to the queue_cond by the
queue_cond_mutex to precent a data race.
(queue::queue): Pass the notifier by reference. Update comment.
(queue::schedule_task): Update comment.
(queue::schedule_tasks): Define new member function.
(queue::wait_for_workers_to_complete): Update comment.
(queue::get_completed_tasks): Return a non-const vector. Update
comment.
(worker::wait_to_execute_a_task): Update several comments. Make
the execution of the notification code to be synchronized (on the
tasks_done_mutex).
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
