Dodji Seketeli [Mon, 12 Jun 2017 17:10:20 +0000 (19:10 +0200)]
Symbols with the same zero value are not aliases
When looking at ELF file of the ET_REL kind (i.e, relocatable object
files), several kinds of symbols (for instance weak symbols) can have
the symbol value zero. Although they have the same value, the fact
that that value is zero prevents us from considering those symbols as
being aliases.
Libabigail was wrongly considering those symbols with value zero as
being aliases. So, in practice, it was considering all WEAK symbols
as being aliases, because the value of a weak symbols is zero.
When comparing two binaries originating from the same source code, one
compiled with g++ and the other one compiled with clang++, abidiff was
thus reporting spurious function aliases changes due to this issue.
Note that the two binaries in question come from the bug PR21486.
Comparing them using abidiff exhibits several other issues that were
fixed in previous commits, such as
- Reporting changes about top cv qualifier changes on function
parameter types.
- Not supporting ELF symbol visibility
* src/abg-dwarf-reader.cc (load_symbol_maps_from_symtab_section):
Do not consider symbols with zero value as being aliases.
* tests/data/test-diff-filter/test20-inline-report-0.txt: Adjust.
* tests/data/test-diff-filter/test20-inline-report-1.txt:
Likewise.
* test-diff-filter/test41-PR21486-abg-writer.gcc.o: New test
binary input.
* tests/data/test-diff-filter/test41-PR21486-abg-writer.llvm.o:
Likewise.
* tests/data/Makefile.am: Add the new test material to source
distribution.
* tests/test-diff-filter.cc (in_out_specs): Run the test harness
on the new test input above.
* tests/data/test-diff-dwarf/test5-report.txt: Adjust.
* tests/data/test-diff-filter/test9-report.txt: Adjust.
* tests/data/test-diff-filter/test20-inline-report-0.txt: Adjust.
* tests/data/test-diff-filter/test20-inline-report-1.txt: Adjust.
Dodji Seketeli [Mon, 12 Jun 2017 09:24:20 +0000 (11:24 +0200)]
Support ELF symbol visibility property
This patch models the ELF symbol visibility property and support
ignoring function and variable symbols that are HIDDEN and INTERNAL,
even if they have default binding.
Dodji Seketeli [Fri, 9 Jun 2017 08:37:59 +0000 (10:37 +0200)]
Filter top cv qualifier changes on function parameter types
When the type of a function parameter sees its top CV qualifier
change, that should never negatively affect ABI compliance.
So this patch filters out top cv qualifier changes on function
parameter types, by default.
* include/abg-comparison.h (enum diff_category): Add a new
FN_PARM_TYPE_TOP_CV_CHANGE_CATEGORY enumerator. "Or" the
enumerator to the EVERYTHING_CATEGORY enumerator.
* src/abg-comp-filter.cc (has_fn_parm_type_cv_qual_change): Define
new static function.
(categorize_harmless_diff_node): Categorize changes to top cv
qualifiers on function parameter types into the new
FN_PARM_TYPE_TOP_CV_CHANGE_CATEGORY.
* src/abg-comparison.cc (get_default_harmless_categories_bitmap):
Add the new FN_PARM_TYPE_TOP_CV_CHANGE_CATEGORY category to the
set of harmless categories.
(operator<<(ostream&, diff_category)): Adjust to serialize
the new FN_PARM_TYPE_TOP_CV_CHANGE_CATEGORY.
* tests/data/test-diff-filter/libtest40-v0.so: New test input binary.
* tests/data/test-diff-filter/libtest40-v1.so: Likewise.
* tests/data/test-diff-filter/test40-report-0.txt: New test
reference output.
* tests/data/test-diff-filter/test40-v0.cc: Source code of the
test binary above.
* tests/data/test-diff-filter/test40-v1.cc: Likewise.
* tests/data/Makefile.am: Add the new test material above to
source distribution.
* tests/test-diff-filter.cc (in_out_specs): Add new binaries to
compare.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.txt:
Adjust.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report1.txt:
Likewise.
* tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-0.txt:
Likewise.
* tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-1.txt:
Likewise.
Dodji Seketeli [Sun, 28 May 2017 17:40:51 +0000 (19:40 +0200)]
Do not report about voffset when it's not set in debug info
Sometimes some virtual member functions don't have any virtual offset
set in the debug info. This happens for virtual destructors
sometimes. In that case, the ABI change report should not refer to
that unset virtual offset as being '0'. Rather, it shouldn't refer to
it at all.
This is what this patch does.
* include/abg-ir.h (mem_fn_context_rel::mem_fn_context_rel):
Initialize the virtual offset to -1.
* src/abg-comparison.cc (represent): In the overload to represent
a method_decl, do not represent the vofffset if it's not set.
* src/abg-writer.cc (write_voffset): The virtual offset is signed
because if it's -1, it means no offset is set.
* tests/data/test-annotate/test14-pr18893.so.abi: Adjust.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Adjust.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Adjust.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Adjust.
* tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1.abi:
Adjust.
* tests/data/test-diff-dwarf/test28-vtable-changes-report-0.txt: Adjust.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Adjust.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Adjust.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Adjust.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Adjust.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Adjust.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Adjust.
Dodji Seketeli [Wed, 17 May 2017 08:19:30 +0000 (10:19 +0200)]
Allow selective resolution of class declaration
When a class is forward-declared, resolving it to a definition that
appears later in the same translation unit or in another translation
is an interesting problem.
Until now, the declaration would be resolved to the definition of that
class found in the binary. The problem is that there can be different
such definitions, especially in C where there is no "One Definition
Rule". In that case, the definition chosen is random.
This patch resolves that randomness.
For a given class declaration, if there is just one possible
definition in the binary, then the declaration is resolved to that
definition. If there is one definition for that declaration in the
same translation unit, then the declaration is resolved to that
definition. If there are more than one definitions in translation
units that are not the one of the declaration, then the declaration is
left unresolved. This is what I call "selective class declaration resolution".
Note that an unresolved class declaration now compares different to a
definition of a class of the same name. This is so that we can have
an unresolved class be present in the resulting .abi file, alongside
an (incompatible) definition of the same class. The change from a class
declaration to its definition is filtered out by default, though.
* include/abg-fwd.h (type_base_wptrs_type)
(istring_type_base_wptrs_map_type): Define new typedefs.
(lookup_class_types): Declare new functions.
* include/abg-ir.h
(environment::decl_only_class_equals_definition): Declare new
accessor.
(type_maps::{*_types}): Make these accessors return
istring_type_base_wptrs_map_type& instead of
istring_type_base_wptr_map_type&.
* src/abg-dwarf-reader.cc
(read_context::resolve_declaration_only_classes): Implement the
new selective declaration resolution scheme.
* src/abg-ir.cc (type_maps::priv::{*_types_}): Change the type of
these data members from istring_type_base_wptr_map_type to
istring_type_base_wptrs_map_type.
(type_maps::{*_types}): Make these accessors definitions return
istring_type_base_wptrs_map_type& instead of
istring_type_base_wptr_map_type&.
(translation_unit::bind_function_type_life_time): Adjust.
(environment::priv::decl_only_class_equals_definition_): New data
member.
(environment::priv::priv): Initialize it. By default, a decl-only
class is now considered different from its definition.
(environment::decl_only_class_equals_definition): Define new
accessor.
(lookup_types_in_map, lookup_class_types): Define new functions.
(lookup_type_in_map, lookup_union_type_per_location)
(lookup_basic_type, lookup_basic_type_per_location)
(lookup_class_type, lookup_class_type_per_location)
(lookup_union_type, lookup_enum_type)
(lookup_enum_type_per_location, lookup_typedef_type)
(lookup_typedef_type_per_location, lookup_qualified_type)
(lookup_pointer_type, lookup_reference_type, lookup_array_type)
(lookup_function_type, maybe_update_types_lookup_map)
(maybe_update_types_lookup_map<class_decl>)
(maybe_update_types_lookup_map<function_type>): Adjust.
(type_base::get_canonical_type_for): When doing type comparison
here, we can now consider that an unresolved class declaration
compares different to an incompatible class definition of the same
name. So no need to look through decl-only classes in that case.
(equals): In the overload for class_or_union, if
environment::decl_only_class_equals_definition() is false, then an
unresolved class declaration of name "N" compares different to a
class definition named "N".
* tests/data/test-annotate/test15-pr18892.so.abi: Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Adjust.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Adjust.
* tests/data/test-diff-dwarf/test28-vtable-changes-report-0.txt:
Adjust.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Adjust.
* tests/data/test-diff-filter/test38/Makefile: New test material.
* tests/data/test-diff-filter/test38/test38-a.c: Likewise.
* tests/data/test-diff-filter/test38/test38-b.c: Likewise.
* tests/data/test-diff-filter/test38/test38-c.c: Likewise.
* tests/data/test-diff-filter/test38/test38-report-0.txt: Likewise.
* tests/data/test-diff-filter/test38/test38-v0: Likewise.
* tests/data/test-diff-filter/test38/test38-v1: Likewise.
* tests/data/test-diff-filter/test38/test38.h: Likewise.
* tests/data/test-diff-filter/test39/Makefile: Likewise.
* tests/data/test-diff-filter/test39/test39-a-v0.c: Likewise.
* tests/data/test-diff-filter/test39/test39-a-v1.c: Likewise.
* tests/data/test-diff-filter/test39/test39-b-v0.c: Likewise.
* tests/data/test-diff-filter/test39/test39-b-v1.c: Likewise.
* tests/data/test-diff-filter/test39/test39-c-v0.c: Likewise.
* tests/data/test-diff-filter/test39/test39-c-v1.c: Likewise.
* tests/data/test-diff-filter/test39/test39-main.c: Likewise.
* tests/data/test-diff-filter/test39/test39-report-0.txt: Likewise.
* tests/data/test-diff-filter/test39/test39-v0: Likewise.
* tests/data/test-diff-filter/test39/test39-v1: Likewise.
* tests/data/test-diff-filter/test39/test39.h: Likewise.
* tests/data/Makefile.am: Add the new test material above to the
source distribution.
* tests/test-diff-filter.cc (in_out_specs): Add the new test
inputs above to the test harness.
Dodji Seketeli [Wed, 31 May 2017 08:30:23 +0000 (10:30 +0200)]
Allow re-using the ELF/DWARF read_context when loading a corpus group
Right now, when loading each corpus of a group, a new read_context is
created and destroyed. That makes thousands of corpora that are
created and destroyed. Profiling seems to argue that we'd gain in
performance by re-using the first read_context that was created
instead, and re-set it before loading a new corpus.
This is what this patch does.
* include/abg-dwarf-reader.h (reset_read_context): Declare new
function.
* src/abg-dwarf-reader.cc (read_context::elf_paths_): Make this to
be non const.
(read_context::initialize): New function to initialize all data
members.
(read_context::read_context): Use the new read_context::initialize
function, rather than initializing data members 'inline' here.
(reset_read_context): Define a new function to reset a
read_context so that it can be re-used to load a new corpus.
Dodji Seketeli [Wed, 31 May 2017 07:47:26 +0000 (09:47 +0200)]
Add --vmlinux{1,2} option to abidw and kmidiff
When using abidw to generate an abixml for a Linux Kernel build tree,
usually, people have to copy the vmlinux binary into the directory
where modules are, so that the tool can find it. This --vmlinux
option helps to avoid doing that copy.
Simarly, when comparing two Linux Kernel build trees, --vmlinux1 and
--vmlinux2 are there to make the tool find the vmlinux binaries to
compare, independantly from the directories under which the modules
are to be found.
* include/abg-tools-utils.h
(build_corpus_group_from_kernel_dist_under): Add a new
vmlinux_path parameter.
* src/abg-tools-utils.cc (find_vmlinux_and_module_paths): Do not
try to find a vmlinux binary if we already have the path to one.
(build_corpus_group_from_kernel_dist_under): Add a new
vmlinux_path parameter.
* tools/abidw.cc (options::vmlinux): New data member.
(display_usage): Add a usage string for --vmlinux
(parse_command_line): Parse the new --vmlinux option.
(load_kernel_corpus_group_and_write_abixml): Fix some return code
when the function fails. Verify the presence of the vmlinux
binary that was given. Adjust.
* tools/kmidiff.cc (options::{vmlinux1, vmlinux2}): New data
members.
(display_usage): Add a usage string for --vmlinux1 and --vmlinux2.
(parse_command_line): Parse the --vmlinux1 and --vmlinux2
options.
(main): Adjust.
Dodji Seketeli [Thu, 18 May 2017 12:45:32 +0000 (14:45 +0200)]
Cache function type name computation results
get_type_name can be called over and over again on function types.
This patch caches the result of the computation of function type names
in those cases.
This is a speed optimization.
* src/abg-ir.cc (get_type_name): Cache function type names.
Dodji Seketeli [Mon, 29 May 2017 08:26:37 +0000 (10:26 +0200)]
Fix innacurate test condition when reading an enum type from abixml
* src/abg-reader.cc (build_enum_type_decl): Do not check for
errno which might have been set earlier by something else.
Rather, check the returned value for overflow or underflow.
Dodji Seketeli [Sun, 28 May 2017 17:40:51 +0000 (19:40 +0200)]
Do not report about voffset when it's not set in debug info
Sometimes some virtual member functions don't have any virtual offset
set in the debug info. This happens for virtual destructors
sometimes. In that case, the ABI change report should not refer to
that unset virtual offset as being '0'. Rather, it shouldn't refer to
it at all.
This is what this patch does.
* include/abg-ir.h (mem_fn_context_rel::mem_fn_context_rel):
Initialize the virtual offset to -1.
* src/abg-comparison.cc (represent): In the overload to represent
a method_decl, do not represent the vofffset if it's not set.
* src/abg-writer.cc (write_voffset): The virtual offset is signed
because if it's -1, it means no offset is set.
* tests/data/test-annotate/test14-pr18893.so.abi: Adjust.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Adjust.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Adjust.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Adjust.
* tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1.abi:
Adjust.
* tests/data/test-diff-dwarf/test28-vtable-changes-report-0.txt: Adjust.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Adjust.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Adjust.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Adjust.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Adjust.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Adjust.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Adjust.
Dodji Seketeli [Thu, 18 May 2017 12:38:51 +0000 (14:38 +0200)]
Speedup DIE representation computing esp function signature in C
For DIE originating from C, we now compute canonical DIEs. We then
use that to compare DIEs to see if they are equal or not. So string
representation of DIEs are now used only to reduce the number of DIEs
comparisons that is performed during DIE canonicalization.
We can thus just use function names (rather than a full
die_function_signature) as a way to reduce the number of structural
comparisons of DIEs during canonicalization.
This patch does just that.
Note that in the future when we perform DIEs canonicalization and comparison for
C++, we can avoid computing full function DIE signatures for C++ too.
* src/abg-dwarf-reader.cc (die_function_signature): For C DIEs,
just return the (linkage) name of the function.
* tests/data/test-annotate/test15-pr18892.so.abi: Adjust.
* tests/data/test-annotate/test21-pr19092.so.abi: Adjust.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Adjust.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Adjust.
Dodji Seketeli [Wed, 17 May 2017 08:19:30 +0000 (10:19 +0200)]
Allow selective resolution of class declaration
When a class is forward-declared, resolving it to a definition that
appears later in the same translation unit or in another translation
is an interesting problem.
Until now, the declaration would be resolved to the definition of that
class found in the binary. The problem is that there can be different
such definitions, especially in C where there is no "One Definition
Rule". In that case, the definition chosen is random.
This patch resolves that randomness.
For a given class declaration, if there is just one possible
definition in the binary, then the declaration is resolved to that
definition. If there is one definition for that declaration in the
same translation unit, then the declaration is resolved to that
definition. If there are more than one definitions in translation
units that are not the one of the declaration, then the declaration is
left unresolved. This is what I call "selective class declaration resolution".
Note that an unresolved class declaration now compares different to a
definition of a class of the same name. This is so that we can have
an unresolved class be present in the resulting .abi file, alongside
an (incompatible) definition of the same class. The change from a class
declaration to its definition is filtered out by default, though.
* include/abg-fwd.h (type_base_wptrs_type)
(istring_type_base_wptrs_map_type): Define new typedefs.
(lookup_class_types): Declare new functions.
* include/abg-ir.h
(environment::decl_only_class_equals_definition): Declare new
accessor.
(type_maps::{*_types}): Make these accessors return
istring_type_base_wptrs_map_type& instead of
istring_type_base_wptr_map_type&.
* src/abg-dwarf-reader.cc
(read_context::resolve_declaration_only_classes): Implement the
new selective declaration resolution scheme.
* src/abg-ir.cc (type_maps::priv::{*_types_}): Change the type of
these data members from istring_type_base_wptr_map_type to
istring_type_base_wptrs_map_type.
(type_maps::{*_types}): Make these accessors definitions return
istring_type_base_wptrs_map_type& instead of
istring_type_base_wptr_map_type&.
(translation_unit::bind_function_type_life_time): Adjust.
(environment::priv::decl_only_class_equals_definition_): New data
member.
(environment::priv::priv): Initialize it. By default, a decl-only
class is now considered different from its definition.
(environment::decl_only_class_equals_definition): Define new
accessor.
(lookup_types_in_map, lookup_class_types): Define new functions.
(lookup_type_in_map, lookup_union_type_per_location)
(lookup_basic_type, lookup_basic_type_per_location)
(lookup_class_type, lookup_class_type_per_location)
(lookup_union_type, lookup_enum_type)
(lookup_enum_type_per_location, lookup_typedef_type)
(lookup_typedef_type_per_location, lookup_qualified_type)
(lookup_pointer_type, lookup_reference_type, lookup_array_type)
(lookup_function_type, maybe_update_types_lookup_map)
(maybe_update_types_lookup_map<class_decl>)
(maybe_update_types_lookup_map<function_type>): Adjust.
(type_base::get_canonical_type_for): When doing type comparison
here, we can now consider that an unresolved class declaration
compares different to an incompatible class definition of the same
name. So no need to look through decl-only classes in that case.
(equals): In the overload for class_or_union, if
environment::decl_only_class_equals_definition() is false, then an
unresolved class declaration of name "N" compares different to a
class definition named "N".
* tests/data/test-annotate/test15-pr18892.so.abi: Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Adjust.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Adjust.
* tests/data/test-diff-dwarf/test28-vtable-changes-report-0.txt:
Adjust.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Adjust.
* tests/data/test-diff-filter/test38/Makefile: New test material.
* tests/data/test-diff-filter/test38/test38-a.c: Likewise.
* tests/data/test-diff-filter/test38/test38-b.c: Likewise.
* tests/data/test-diff-filter/test38/test38-c.c: Likewise.
* tests/data/test-diff-filter/test38/test38-report-0.txt: Likewise.
* tests/data/test-diff-filter/test38/test38-v0: Likewise.
* tests/data/test-diff-filter/test38/test38-v1: Likewise.
* tests/data/test-diff-filter/test38/test38.h: Likewise.
* tests/data/test-diff-filter/test39/Makefile: Likewise.
* tests/data/test-diff-filter/test39/test39-a-v0.c: Likewise.
* tests/data/test-diff-filter/test39/test39-a-v1.c: Likewise.
* tests/data/test-diff-filter/test39/test39-b-v0.c: Likewise.
* tests/data/test-diff-filter/test39/test39-b-v1.c: Likewise.
* tests/data/test-diff-filter/test39/test39-c-v0.c: Likewise.
* tests/data/test-diff-filter/test39/test39-c-v1.c: Likewise.
* tests/data/test-diff-filter/test39/test39-main.c: Likewise.
* tests/data/test-diff-filter/test39/test39-report-0.txt: Likewise.
* tests/data/test-diff-filter/test39/test39-v0: Likewise.
* tests/data/test-diff-filter/test39/test39-v1: Likewise.
* tests/data/test-diff-filter/test39/test39.h: Likewise.
* tests/data/Makefile.am: Add the new test material above to the
source distribution.
* tests/test-diff-filter.cc (in_out_specs): Add the new test
inputs above to the test harness.
Dodji Seketeli [Thu, 4 May 2017 08:20:18 +0000 (10:20 +0200)]
Avoid de-duplicating different C types that have identical name
This patch introduces the concept of canonical DIEs.
To date, when two C types have the same string representation [1] and
are defined at the same location, they are considered to be the same,
just as in C++.
[1]: String representation of a type: For a type named
"Foo" that is a structure, the representation is "struct Foo"
This patch introduces more finesse in determining if two types are
different.
It implements comparing the DIEs of the types, directly from the DWARF
representation. When comparing pointers, typedefs and qualified
types, the underlying type is compared recursively.
The type de-duplication scheme is now centered around two data
structures.
1/ A map that associates the string representation of a type with
a vector of the offsets of the type DIEs that have the same
representation. Members of this vector denotes DIEs of types that
are *different* even if they all have the same representation.
Each DIE in a given vector is the canonical DIE of its class of
equivalence. This map is the map of all canonical DIEs organized
by the representation of those canonical DIEs.
2/ A map that associates the offset of the canonical type DIE with
the resulting internal representation of the type. Here, the
internal representation is an instance of the
abigail::ir::type_base type. This map is the map of the types
associated to the canonical type DIEs.
There is also a vector that associates a DIE 'D' to its canonical DIE
'C'. The index of the vector is the offset of 'D' and the value of
the element at that index is 'C'.
Thus, each time we are about to create (or get) an internal
representation for a type DIE denoted 'D', we first get the canonical
DIE of 'D', denoted C. If C doesn't exist, we create it. That is, we
add a new entry in the map 1/. Then we look in map 2/ to see if the C
(and thus D) has a associated type.
If C has an associated type, we return it.
If C has no associated type, we create a type for it (and thus for D)
and we associate the new type to the offsets of both D and C.
The rest of the patch is mostly boiler plate and adjustment to
accomodate this new de-duplication scheme.
* src/abg-dwarf-reader.cc (die_decl_map_type, die_type_map_type):
Remove these typedefs.
(die_artefact_map_type, istring_dwarf_offsets_map_type): New
typedefs.
(die_is_at_class_scope, die_qualified_type_name)
(die_qualified_decl_name, die_qualified_type_name_empty)
(die_return_and_parm_names_from_fn_type_die)
(die_function_type_is_method_type):
Const-ify the read_context& parameter.
(read_context::die_source_dependant_container_set::get_container):
Likewise.
(read_context::{name_artefacts_map_, per_tu_name_artefacts_map_,
die_decl_map_, alternate_die_decl_map_, type_unit_die_decl_map_,
die_type_map_, alternate_die_type_map_, type_unit_die_type_map_}):
Remove data members.
(read_context::{die_decl_map, alternate_die_decl_map,
associate_die_to_decl_primary, associate_die_to_decl_alternate,
associate_die_to_decl_from_type_unit,
lookup_decl_from_die_offset_primary,
lookup_decl_from_die_offset_alternate,
lookup_decl_from_type_unit_die_offset,
lookup_type_artifact_from_die_per_tu,
lookup_artifact_from_per_tu_die_representation,
associate_die_to_artifact_by_repr,
associate_die_to_artifact_by_repr_internal, clear_die_type_maps}):
Remove member functions.
(read_context::{decl_die_repr_die_offsets_maps_,
type_die_repr_die_offsets_maps_, decl_die_artefact_maps_,
type_die_artefact_maps_, dwarf_expr_eval_context_}): Add new data
members.
(read_context::clear_per_translation_unit_data): Don't clear
read_context::per_tu_name_artefacts_map_ data member as it's
removed.
(read_context::clear_per_corpus_data): Don't clear
read_context::name_artefacts_map_ and all the other relevant data
members that got removed.
(read_context::{dwarf_per_die_source,
decl_die_repr_die_offsets_maps, type_die_repr_die_offsets_maps,
get_canonical_die, get_die_from_offset, decl_die_artefact_maps,
type_die_artefact_maps, dwarf_expr_eval_ctxt}): Add new member
functions.
(compare_dies, compare_as_decl_dies)
(compare_as_type_dies, maybe_finish_function_decl_reading)
(die_is_anonymous): Define new functions.
(read_context::associate_die_to_decl): Remove the
do_associate_by_repr_per_tu parameter. Use the new
read_context::{decl_die_artefact_maps_, get_canonical_die} member
functions.
(read_context::lookup_decl_from_die_offset): Use Dwarf_Off rather
than size_t for the type of the die_offset parameter. Use the
lookup_artifact_from_die_offset member function.
(read_context::lookup_type_artifact_from_die): Const-ify. In one
overload, take a new 'die_as_type' parameter. Use the new
get_canonical_die, type_die_artefact_maps and
decl_die_artefact_maps member functions. In the second overload,
use the first overload.
(read_context::odr_is_relevant): Add an overload that takes a DIE.
(read_context::associate_die_to_type): Remove the
do_associate_by_repr and do_associate_per_tu parameters. Use the
new get_canonical_die and type_die_artefact_maps member functions.
(read_context::lookup_type_from_die): Use the new
lookup_artifact_from_die member function.
(read_context::lookup_type_from_die_offset): Use the new
type_die_artefact_maps member function. When the found artifact
is a function_decl, return its type.
(read_context::schedule_type_for_late_canonicalization): Use the
new get_canonical_die and type_die_artefact_maps member functions.
(die_function_signature): Const-ify. Get the scope name right
even for scopes that are not types.
(die_member_offset): Make eval_last_constant_dwarf_sub_expr use
the new cached DWARF expression evalution context.
(get_parent_die): Support where_offset equals to zero. This means
we are looking at a C binary, basically.
(build_enum_type) : Use the new overload of
read_context::odr_is_relevant that takes a DIE. Adjust.
(add_or_update_union_type, add_or_update_class_type): Don't lookup
classes/unions per location anymore. Now that we can compare DIEs
in a fined grain manner, the approximation of the location is not
useful anymore.
(build_pointer_type)
(build_function_type): Associate DIE to type if we reuse an
existing type.
(build_or_get_fn_decl_if_not_suppressed): When re-using a
function decl internal representation from an equivalent DIE that
we've seen before, it can happen that we want to augment that
function decl internal representation with new properties coming
from the DIE we are currently looking at; do that here.
(is_function_for_die_a_member_of_class): Remove the "where_offset"
parameter.
(add_or_update_member_function): Adjust.
* tests/data/test-annotate/libtest23.so.abi: Adjust.
* tests/data/test-annotate/test13-pr18894.so.abi: Adjust.
* tests/data/test-annotate/test14-pr18893.so.abi: Adjust.
* tests/data/test-annotate/test15-pr18892.so.abi: Adjust.
* tests/data/test-annotate/test17-pr19027.so.abi: Adjust.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Adjust.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Adjust.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Adjust.
* tests/data/test-annotate/test21-pr19092.so.abi: Adjust.
* tests/data/test-diff-pkg/libICE-1.0.6-1.el6.x86_64.rpm--libICE-1.0.9-2.el7.x86_64.rpm-report-0.txt: Adjust.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-0.txt: Adjust.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-1.txt: Adjust.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt: Adjust.
* tests/data/test-read-dwarf/libtest23.so.abi: Adjust.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Adjust.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Adjust.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Adjust.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Adjust.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Adjust.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Adjust.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Adjust.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Adjust.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Adjust.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Adjust.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Adjust.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust.
Speedup access to unreferenced symbols when loading corpus_group
This patch speeds up the invocation of
corpus_group::get_unreferenced_function_symbols and
corpus_group::get_unreferenced_variable_symbols. It does so by
caching the result of constructing the map of of unreferenced symbol
at the time where each corpus is added to the group.
* src/abg-corpus.cc (corpus_group::unrefed_{fun, var}_symbol_map):
New data members.
(corpus_group::priv::priv): Adjust.
(corpus_group::priv::add_unref_{fun,var}_symbols): Define new
member functions.
(corpus_group::add_corpus): Update the map of unreferenced
symbols.
(corpus_group::get_unreferenced_{function,variable}_symbols)
Adjust logic.
Initial support of de-serializing the KMI of a Linux Kernel Tree
With this patch, kmidiff knows how to compare a serialized
corpus_group that represents a Kernel Module Interface (a .kmi file)
against either another serialized .kmi file, or against a kernel tree.
The patch extends the abixml reader to make it parse an
'abi-corpus-group' element. To do that, the patch modifies
read_corpus_from_input to make it be capable of parsing several
'abi-corpus' in a row. That modified function is then used by a new
read_corpus_group_from_input, which is itself used by the new public
entry points read_corpus_group_from_native_xml_file and
read_corpus_group_from_native_xml.
With that read_corpus_group_from_native_xml_file building block
function, the kmidiff program is modified so that it can take either
two directory trees, two .kmi files or one directory tree and one .kmi
file.
* include/abg-libxml-utils.h (advance_to_next_sibling_element):
Declare new function.
* src/abg-libxml-utils.cc (go_to_next_sibling_element_or_stay)
(advance_to_next_sibling_element): Define new functions.
* include/abg-reader.h (read_corpus_group_from_input)
(read_corpus_group_from_native_xml)
(read_corpus_group_from_native_xml_file): Declare new functions.
* src/abg-reader.cc (read_context::m_corpus_group): New data
member.
(read_context::{get_corpus_group, set_corpus_group}): Define new
member functions.
(read_translation_unit_from_input): Cleanup logic.
(read_corpus_from_input): Don't assume that the document is
starting with an 'abi-corpus' element. Support the mode where a
caller called the xmlTextReaderExpand function (and so we are
given an expanded xmlNodePtr) and the mode where we need to use
the xmlTextReader API to walk through the 'abi-corpus' element.
Also, if we are building a corpus group, do not clear what used to
be 'per-corpus' data. That data must be shared by all the corpora
of a given abi-corpus-group.
(read_corpus_group_from_input, read_corpus_group_from_native_xml)
(read_corpus_group_from_native_xml_file): Define new functions.
* include/abg-tools-utils.h (FILE_TYPE_XML_CORPUS_GROUP): New
enumerator of the file_type enum.
* src/abg-tools-utils.cc (operator<<): In the overload for
file_type, add a case for the new FILE_TYPE_XML_CORPUS_GROUP.
(guess_file_type): Dectect abi-corpus-group xml element.
* tools/abidiff.cc (adjust_diff_context_for_kmidiff): Define new
static function.
(main): Adjust to handle the new FILE_TYPE_XML_CORPUS_GROUP. That
is, compare two FILE_TYPE_XML_CORPUS_GROUP if they are present.
* tools/abilint.cc (main): Likewise.
* tools/kmidiff.cc (main): Detect that one of two .kmi files are
passed. In that case, load the .kmi file(s), build a corpus_group
of it and use it in the comparison.
Initial support of the serialization of the KMI of a Linux Kernel Tree
We have the kmidiff program that takes two Linux Kernel trees
containing the vmlinux binary and its modules and compare their Kernel
Module Interface, aka KMI.
We need to be able to serialize (save in a file) a representation of
that KMI. We need to load that KMI back, and compare two serialized
KMIs.
This patch implements the serialization of the KMI of a Linux Kernel
tree. It actually serializes an instance of abigail::ir::corpus_group
that is a collection of instances of abigail::ir::corpus. The KMI of
each individual binary (vmlinux or kernel module) is represented by
one abigail::ir::corpus. All the corpora share the same definitions
of types and decls, whenever that makes sense.
The patch thus factorizes the routines used to walk a Linux kernel out
of the kmidiff program. These routines are then re-used in the abidw
program to make it walk a Linux kernel tree (when the --linux-tree
option is provided), load the vmlinux and module binaries as an
instance of abigail::corpus_group and serialize it out into an output
stream.
* include/abg-tools-utils.h (check_dir)
(get_binary_paths_from_kernel_dist)
(build_corpus_group_from_kernel_dist_under): Declare new
functions. The last two functions are being moved from
tools/kmidiff.cc so that they can be re-used.
* include/abg-writer.h (write_corpus): Declare one overload that
takes a write_context parameter.
(write_corpus_group): Declare three overloads of this new function.
* src/abg-tools-utils.cc (check_dir): Define new function.
(load_generate_apply_suppressions, is_vmlinux, is_kernel_module)
(find_vmlinux_and_module_paths)
(get_binary_paths_from_kernel_dist)
(build_corpus_group_from_kernel_dist_under): Define new functions.
* src/abg-writer.cc (write_context::set_annotate): Define new
member function.
(write_corpus): Add an overload that takes a write_context. Adapt
the existing overload to make it use this new one.
(write_corpus_group): Define this new function and two additional
overloads for it.
* tools/kmidiff.cc (set_suppressions, is_vmlinux)
(is_kernel_module, find_vmlinux_and_module_paths)
(get_binary_paths_from_kernel_dist)
(build_corpus_group_from_kernel_dist_under): Remove.
(main): Adjust the call to
build_corpus_group_from_kernel_dist_under as its arguments are now
adapted since it's been factorized out into abg-tools-utils.h.
* tools/abidw.cc (options::corpus_group_for_linux): Define new
data member.
(options::options): Adjust.
(display_usage): Add help strings for the new --linux-tree option.
(load_corpus_and_write_abixml): Factorize this function out of the
main function.
(load_kernel_corpus_group_and_write_abixml): Define new function.
(main): Use the factorized load_corpus_and_write_abixml and the
new load_corpus_and_write_abixml functions.
* tests/test-read-write.cc: Adjust.
* doc/manuals/abidw.rst: Add documentation for the new
--linux-tree option.
Adjust test reference outputs after changes in abg-writer.cc
After the latest commits to avoid emitting empty translation units and
dupicated decls, and also to fix some indention issues in the abixml
output, this patch updates the reference outputs of the abixml
regression tests.
This patch avoids emitting the same decl twice in the same ABI corpus
or corpus_group. For the purpose of this duplication detection, a
decl is designated by its pretty representation.
The patch helps to reduce the size of the abixml files drastically by
cutting some of them, especially those representing the KMI of the
linux kernel in more than half.
* src/abg-writer.cc (write_context::m_emitted_decls_map): New data
member.
(write_context::{decl_name_is_emitted, record_decl_as_emitted}):
Define new memeber functions.
(write_translation_unit): Do not emit a decl that has already been
emitted.
(write_var_decl, write_function): Record the decl as emitted.
Dodji Seketeli [Mon, 12 Jun 2017 13:19:01 +0000 (15:19 +0200)]
Introduce the --kmi-whitelist option to abidiff
abidiff had an --linux-kernel-abi-whitelist option. Rename it
--kmi-whitelist.
* doc/manuals/abidiff.rst: Add documentation for the
--kmi-whitelist option.
* tools/abidiff.cc (display_usage): Emit help string for the
--kmi-whitelist option
(parse_command_line): Parse the new --kmi-whitelist option, of the
-w shortcut.
When writting out the ABI of a Linux kernel binary we must be able to
restrict it to the set of functions and global variables which ELF
symbols have names defined in a white list.
This patch adds that support, using the --kmi-whitelist
option.
* tools/abidw.cc (options::{kabi_whitelist_paths,
kabi_whitelist_supprs}): New data members.
(display_usage): Add a help string for the new --kmi-whitelist
option.
(parse_command_line): Parse the new --kmi-whitelist option.
(maybe_check_suppression_files): Check the presence of the linux
kernel abi white list passed by the option --kmi-whitelist.
(main): Ignore loading the symbol table if the kernel abi white
list is provided.
* doc/manuals/abidw.rst: Add documentation for the new option.
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.
Avoid loading a translation unit twice from abixml
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.
* 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.
Dodji Seketeli [Fri, 31 Mar 2017 07:34:26 +0000 (09:34 +0200)]
Initial support to lookup types per location
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.
Dodji Seketeli [Thu, 30 Mar 2017 10:51:32 +0000 (12:51 +0200)]
Create a Corpus Group API extension
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.
Avoid comparing kernel.img file from the grub2 package
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.
Dodji Seketeli [Wed, 14 Jun 2017 09:08:19 +0000 (11:08 +0200)]
Bug 21567 - Fedabipkgdiff matches build distro names too tightly
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.
Sinny Kumari [Tue, 13 Jun 2017 21:09:02 +0000 (02:39 +0530)]
Check if return_codes list is empty in fedabipkgdiff
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
Ben Woodard [Thu, 11 May 2017 23:30:20 +0000 (16:30 -0700)]
Fix more clang build warnings
* 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.
Ben Woodard [Thu, 11 May 2017 22:53:44 +0000 (15:53 -0700)]
Fix some clang compile problems
* 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.
Sinny Kumari [Sun, 21 May 2017 09:40:12 +0000 (15:10 +0530)]
Add --self-compare option in fedabipkgdiff
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
Dodji Seketeli [Fri, 31 Mar 2017 10:44:38 +0000 (12:44 +0200)]
Avoid building DIE -> parent DIE map when analyzing a C binary
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.
Speed up access to the definition of a class declaration-only type
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.
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.
* 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.
Dodji Seketeli [Wed, 10 May 2017 07:03:15 +0000 (09:03 +0200)]
Don't consider changes to basic types as being redundant
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.
Dodji Seketeli [Wed, 3 May 2017 14:33:39 +0000 (16:33 +0200)]
Invalidate function and variable ID cache when invoking ::set_symbol
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.
Dodji Seketeli [Wed, 3 May 2017 14:23:45 +0000 (16:23 +0200)]
Remove useless overloads of is_type
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.
Fix a race condition in queue::priv::do_bring_workers_down
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.
Ondrej Oprala [Wed, 12 Apr 2017 07:53:10 +0000 (09:53 +0200)]
cppcheck: mitigate performance warnings
* 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>
Ondrej Oprala [Tue, 11 Apr 2017 14:57:07 +0000 (16:57 +0200)]
Clean up scripts/*
* 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>
Ondrej Oprala [Wed, 12 Apr 2017 10:32:42 +0000 (12:32 +0200)]
Fix comparison used instead of an assignment
* 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>
Fix some random deadlock while running fedabipkgidiff in tests
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.
Slava Barinov [Wed, 22 Mar 2017 09:21:39 +0000 (12:21 +0300)]
Fix types in header to meet sources
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.
Dodji Seketeli [Fri, 24 Mar 2017 12:09:32 +0000 (13:09 +0100)]
Launch fedabipkgdiff tests first
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.
Dodji Seketeli [Fri, 24 Mar 2017 11:04:53 +0000 (12:04 +0100)]
Bug 21296 - Reporting diff of const ref against non-const ref aborts
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.
Chenxiong Qi [Sun, 5 Mar 2017 05:54:16 +0000 (13:54 +0800)]
Bug 20087 - Clean cache before or after ABI comparison
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.
Dodji Seketeli [Wed, 15 Mar 2017 15:15:37 +0000 (16:15 +0100)]
Fix data race on worker::queue::priv::bring_workers_down
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.
Dodji Seketeli [Tue, 7 Mar 2017 10:14:52 +0000 (11:14 +0100)]
Bug 21228 - Handle cloning union member functions
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.
Dodji Seketeli [Fri, 3 Mar 2017 13:06:34 +0000 (14:06 +0100)]
Consider file path when sorting virtual member functions
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.
Dodji Seketeli [Fri, 3 Mar 2017 11:56:44 +0000 (12:56 +0100)]
Fix virtual members sorting to unbreak the build on EL6
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.
Dodji Seketeli [Wed, 1 Mar 2017 15:15:45 +0000 (16:15 +0100)]
Make the helgrind suppressions less specific
* tests/test-valgrind-suppressions.supp: Make the ostream writting
suppressions be less specific so that they can apply to all the
related false positives.
Dodji Seketeli [Wed, 1 Mar 2017 13:13:39 +0000 (14:13 +0100)]
Move test-read-dwarf.cc to abigail::workers
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.
Dodji Seketeli [Fri, 24 Feb 2017 13:16:35 +0000 (14:16 +0100)]
Make abipkgdiff.cc use the abigail::workers interface
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.
Dodji Seketeli [Fri, 24 Feb 2017 13:55:01 +0000 (14:55 +0100)]
Do not ignore valgrind checks returning an error
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.
Dodji Seketeli [Fri, 24 Feb 2017 13:57:52 +0000 (14:57 +0100)]
Add a "make check-valgrind-helgrind-recursive" target
* 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.
Dodji Seketeli [Fri, 24 Feb 2017 09:58:16 +0000 (10:58 +0100)]
Several fixes and enhancements to abigail::workers
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).
The program considers the two packages as not being "peers". This has
to do with the RPM.is_peer method which considers the two package as
not being "peers", just because they have the same release number
(1.fc22).
They should be considered peers, though, because they have the same
name but different {version, release} pairs.
This patch fixes the RPM.is_peer method and adds the aforementioned
packages to the regression test suite for good measure.
* tools/fedabipkgdiff (RPM.is_peer): Update comment. Fix logic.
* tests/data/test-fedabipkgdiff/packages/vte291/0.39.1/1.fc22/x86_64/vte291-0.39.1-1.fc22.x86_64.rpm:
New test input file.
* tests/data/test-fedabipkgdiff/packages/vte291/0.39.1/1.fc22/x86_64/vte291-debuginfo-0.39.1-1.fc22.x86_64.rpm: Likewise.
* tests/data/test-fedabipkgdiff/packages/vte291/0.39.1/1.fc22/x86_64/vte291-devel-0.39.1-1.fc22.x86_64.rpm: Likewise.
* tests/data/test-fedabipkgdiff/packages/vte291/0.39.90/1.fc22/x86_64/vte291-0.39.90-1.fc22.x86_64.rpm: Likewise.
* tests/data/test-fedabipkgdiff/packages/vte291/0.39.90/1.fc22/x86_64/vte291-debuginfo-0.39.90-1.fc22.x86_64.rpm: Likewise.
* tests/data/test-fedabipkgdiff/packages/vte291/0.39.90/1.fc22/x86_64/vte291-devel-0.39.90-1.fc22.x86_64.rpm:
Likewise.
* tests/data/test-fedabipkgdiff/vte291-0.39.1-1.fc22.x86_64--vte291-0.39.90-1.fc22.x86_64-report-0.txt: Likewise.
* tests/data/Makefile.am: Add the new test input data to source
distribution.
* tests/mockfedabipkgdiff.in: Update the package and build
information to add the new vte291-0.39.1-1.fc22.x86_64.rpm and
vte291-0.39.90-1.fc22.x86_64.rpm packages (as well as their devel
and debuginfo packages) into the "mock" Koji build database.
* tests/runtestfedabipkgdiff.py.in: Make this test harness run
over the two aforementioned packages.
Dodji Seketeli [Mon, 23 Jan 2017 23:33:22 +0000 (00:33 +0100)]
Add missing tests input files to distribution files
The runtestfedabipkgdiff.py test was missing some input files from the
source distribution.
This patch adds them back.
Also, the test file name
test-fedabipkgdiff/test6-missing-devel-debuginfo-nss-util-3.12.6-1.fc14.x86_64--nss-util-3.24.0-2.0.fc25.x86_64-report-0.txt
was too long for tar to handle so that file was left out of the final
source distribution tarball. This patch renames that file to a
smaller name.
make distcheck should pass again now.
* tests/data/Makefile.am: Add three missing test input files to
the source distribution tarball. Renamed
test-fedabipkgdiff/test6-missing-devel-debuginfo-nss-util-3.12.6-1.fc14.x86_64--nss-util-3.24.0-2.0.fc25.x86_64-report-0.txt
into
test-fedabipkgdiff/test6-nss-util-3.12.6-1.fc14.x86_64--nss-util-3.24.0-2.0.fc25.x86_64-report-0.txt.
* tests/runtestfedabipkgdiff.py.in (FEDABIPKGDIFF_TEST_SPECS):
Renamed
test6-missing-devel-debuginfo-nss-util-3.12.6-1.fc14.x86_64--nss-util-3.24.0-2.0.fc25.x86_64-report-0.txt
into
test6-nss-util-3.12.6-1.fc14.x86_64--nss-util-3.24.0-2.0.fc25.x86_64-report-0.txt.
Dodji Seketeli [Mon, 23 Jan 2017 23:37:22 +0000 (00:37 +0100)]
Fix silent failure of tests/runtestfedabipkgdiff.py
It turns out the test <builddir>/tests/runtestfedabipkgdiff.py is
failing, but "make check TESTS=runtestfedabipkgdiff.py" is not.
In other words, runtestfedabipkgdiff.py is failing silently; we don't
see it when doing "make check".
The reason why runtestfedabipkgdiff.py is failing is that
mockfedabipkgdiff is trying to patch the global variable
DEFAULT_KOJI_TOPDIR from the fedabipkgdiff file; and that global
variable is not present in that file. The right global variable that
we want is DEFAULT_KOJI_TOPURL.
This patch fixes that issue.
The reason why the failing above is silent is because the the main
function wasn't returning 0 upon success and 1 otherwise.
This patch fixes that issue as well.
So with this patch, <builddir>/tests/runtestfedabipkgdiff does not
fail anymore and when it does, the "make check TESTS=runtestfedabipkgdiff.py"
fails as well.
* tests/mockfedabipkgdiff.in (run_fedabipkgdiff): Patch
fedabipkgdiff.DEFAULT_KOJI_TOPURL instead of
fedabipkgdiff.DEFAULT_KOJI_TOPDIR.
* tests/runtestfedabipkgdiff.py.in (main): Properly return 0 upon
success, 1 otherwise.
Dodji Seketeli [Fri, 13 Jan 2017 22:11:00 +0000 (23:11 +0100)]
Bug 20476 - Compare virtual member functions when comparing classes
There are cases where a virtual member function doesn't have an
implementation that is defined and publicly exported. This is the
cases for virtual pure classes.
Even in those cases, users might want to detect vtable changes on
virtual member pure classes (interfaces).
Now that, for C++ binaries, all the partial representations of a class
are merged into one class (in a given binary), we can envision to
compare virtual member functions of classes as part of comparing two
classes.
This is what this patch does. While comparing two classes, the
virtual member functions are compared too.
Note that as there can be several virtual member functions that have
the same vtable offset (think about a virtual destructor that might
have several generated functions that 'conceptually' share the same
vtable offset), comparing the virtual functions can be a little bit
non-trivial.
The approach taken is to check that in the first set of virtual
functions, each virtual function actually matches at least one virtual
function in the second set. And the match is a "loose" one. That is,
it doesn't take into account the symbol name or the mangled name.
The patch also fixes the "less than" operator used to sort virtual
member functions.
There is also a buglet in the way we compute the highest vtable offset
number today. This patch provides a new function named
class_decl::get_biggest_vtable_offset that is used in the change reports.
The patch also fixes a related bug in where we were forgetting to
report about added and removed virtual member functions without an
associated elf symbol. This patch fixes that.
* include/abg-ir.h (class_decl::get_biggest_vtable_offset):
Declare new member function.
* src/abg-ir.cc (virtual_member_function_less_than::operator()):
Either compare the symbol id strings if the functions have
symbols or just compare their pretty representations.
(class_decl::get_biggest_vtable_offset): Define new member
function.
(methods_equal_modulo_elf_symbol)
(method_matches_at_least_one_in_vector): New static methods.
(equals): In the overload for classes, compare the virtual member
functions while comparing classes.
* src/abg-comparison.cc (represent): In the overload for
method_decl_sptr, fix the way we compute the highest vtable offset
number for a give class_decl.
(class_decl::ensure_lookup_tables_populated): Don't forget added
and removed virtual member functions in the report for changed
classes.
* tests/data/test-diff-dwarf/libtest41-PR20476-hidden-old.so: New
test binary input file.
* tests/data/test-diff-dwarf/libtest41-PR20476-hidden-new.so: Likewise.
* tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt:
New reference output.
* tests/data/Makefile.am: Add the new test material above to the
source distribution.
* tests/test-diff-dwarf.cc (in_out_spec): Add the new tests
here.
* tests/data/test-annotate/test10-pr18818-gcc.so.abi: Adjust.
* tests/data/test-annotate/test11-pr18828.so.abi: Adjust.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Adjust.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Adjust.
* tests/data/test-annotate/test22-pr19097-libstdc++.so.6.0.17.so.abi: Adjust.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Adjust.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Adjust.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Adjust.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Adjust.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Adjust.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Adjust.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Adjust.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Adjust.
Dodji Seketeli [Fri, 20 Jan 2017 10:00:37 +0000 (11:00 +0100)]
Add --harmless option to abipkgdiff
This option that is present for the abidiff tool was missing for
abipkgdiff.
* doc/manuals/abidiff.rst: Fix a typo.
* doc/manuals/abipkgdiff.rst: Document the --harmless option.
* tools/abipkgdiff.cc: Update copyright year.
(options::show_harmless_changes): Add new data member.
(options::options): Initialize the new data member.
(display_usage): Add a help string for the new --harmless option.
(parse_command_line): Parse the new --harmless option.
(set_diff_context_from_opts): Configure the diff context
accordingly, if the user provided the --harmless option.
Dodji Seketeli [Fri, 20 Jan 2017 09:28:29 +0000 (10:28 +0100)]
Fix suppression category propagation in diff node graph
Under certain circumstances, a diff node (which we shall name N) that
belongs to the SUPPRESSED_CATEGORY category sees its belonging to that
category been propagated to its parent diff node, effectively
suppressing that parent diff node as well. This is how some function
diff nodes ends up being suppressed just because some of their
children diff nodes were suppressed.
This suppression category propagation is performed by a pass that
walks the diff nodes graph. To avoid infinite cycles, the pass avoids
visiting a diff node that is equivalent to a node that has already
been visited.
As a result, diff nodes equivalent to N (the class of equivalence of
N) are not traversed by the propagation pass. So their parent diff
nodes are not suppressed.
This leads to some functions not being suppressed as they should.
This phenomenon can be observed by comparing the two packages that are
provided in the regression test accompanying this patch using
abipkgdiff with the --redundant option.
Here is how the patch addresses the issue.
When the suppression category propagation pass considers a parent diff
node (named P) of a node N' (with N' being equivalent to N), it now
looks at the category of the *class of equivalence of N'* to determine
if that category should be propagated to P.
Previously, that pass would just look at the at the category of N'
(not the category of the class of equivalence of N') for the
propagation. But as N' has not been visited (because N has already
been visited and nodes equivalent to N are thus not visited) the
belonging of N' to the SUPPRESSED_CATEGORY hasn't been updated. So N'
isn't marked as being in SUPPRESSED_CATEGORY. So the
SUPPRESSED_CATEGORY wouldn't be propagated to P as it should.
So, with this patch abipkgdiff invoked with the --redundant option now
correctly yields:
[C]'function spice_image_compression_t spice_server_get_image_compression(SpiceServer*)' at reds.c:3618:1 has some indirect sub-type changes:
return type changed:
underlying type 'enum __anonymous_enum__' changed:
type size hasn't changed
7 enumerator deletions:
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_INVALID' value '0'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_OFF' value '1'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_AUTO_GLZ' value '2'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_AUTO_LZ' value '3'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_QUIC' value '4'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_GLZ' value '5'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_LZ' value '6'
9 enumerator insertions:
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_INVALID' value '0'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_OFF' value '1'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_AUTO_GLZ' value '2'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_AUTO_LZ' value '3'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_QUIC' value '4'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_GLZ' value '5'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_LZ' value '6'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_LZ4' value '7'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_ENUM_END' value '8'
[C]'function int spice_server_set_image_compression(SpiceServer*, spice_image_compression_t)' at reds.c:3602:1 has some indirect sub-type changes:
parameter 2 of type 'typedef spice_image_compression_t' changed:
underlying type 'enum __anonymous_enum__' changed:
type size hasn't changed
7 enumerator deletions:
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_INVALID' value '0'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_OFF' value '1'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_AUTO_GLZ' value '2'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_AUTO_LZ' value '3'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_QUIC' value '4'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_GLZ' value '5'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_LZ' value '6'
9 enumerator insertions:
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_INVALID' value '0'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_OFF' value '1'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_AUTO_GLZ' value '2'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_AUTO_LZ' value '3'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_QUIC' value '4'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_GLZ' value '5'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_LZ' value '6'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_LZ4' value '7'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_ENUM_END' value '8'
================ end of changes of 'libspice-server.so.1.8.0'===============
$
* include/abg-comparison.h (diff::get_class_of_equiv_category):
Declare new member function.
* src/abg-comparison.cc: Update copyright year.
(diff::get_class_of_equiv_category): Define new member function.
(suppression_categorization_visitor::visit_end): When considering
children nodes category for propagation, consider the category of
the class of equivalence of children nodes.
* spice-debuginfo-0.12.4-19.el7.x86_64.rpm: New input test package.
* spice-debuginfo-0.12.8-1.el7.x86_64.rpm: Likewise.
* spice-server-0.12.4-19.el7.x86_64.rpm: Likewise.
* spice-server-0.12.8-1.el7.x86_64.rpm: Likewise.
* spice-server-devel-0.12.4-19.el7.x86_64.rpm: Likewise.
* spice-server-devel-0.12.8-1.el7.x86_64.rpm: Likewise.
* spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-0.txt:
New reference test output.
* spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-1.txt: Likewise.
* spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt: Likewise.
* tests/data/Makefile.am: Add the new test material above to
source distribution.
* tests/test-diff-pkg.cc (in_out_specs): Add the new test inputs
to the list of packages to test.
When using abipkgdiff types that are defined in files not present in
the devel packages provided by the --devel1 and --devel2 option are
dropped from the internal representation by default.
This has been designed as such, not only to avoid showing changes on
types that are not part of the public headers of a shared library, but
also to help lower the memory consumption of libabigail.
In this particular bug report, we see a library that uses types (in
its public interface) that are defined in headers of a *different*
package. For instance, suppose a particular package foo that uses
types defined in headers of the glib package. And some of those Glib
types can be present in its public interface.
So in this case, libabigail is dropping a type that is actually part
of the public interface of the library that is being analyzed, even if
the type was not defined in the devel package of the current package.
This patch addresses the issue by doing a number of things:
1/ If a type is defined in a file which path starts with
"/usr/include/", then consider it as a public type. This is so
that type coming from the public interface of other packages, and
that are defined in system headers are considered as part of the
public types of the package being analyzed.
2/ by default, don't drop types not defined in the associated
devel package. This will hinder our ability to decrease the
memory usage, but there have been a number of recent optimization
that help in that regard independently. So I am hoping this
shouldn't have a big impact now.
Incidentally, the option --dont-drop-private-types (from abidiff)
is changed into --drop-private-types, so that interested users can
still drop non-private types from the model, if they wish. That
--drop-private-types option is added to abipkgdiff too.
As the offended types are not dropped from the model anymore, the
usual filtering mechanisms of libabigail can take place.
* doc/manuals/abidiff.rst (--dont-drop-private-types): Remove documentation.
(--drop-private-types): Document this new option.
* src/abg-tools-utils.cc: Update copyright notice
(handle_fts_entry): On the generated suppression specification, do
not set the flag to drop matched types. Also, don't match types
defined in files which patch start with "/usr/include/".
* tools/abidiff.cc (options::options): Initialize the
drop_private_types data member to false.
(display_usage): Remove usage string for
--dont-drop-private-types. Add a new one for
--drop-private-types.
(parse_command_line): Don't part --dont-drop-private-types,
rather, parse --drop-private-types.
(set_suppressions): When the suppression for private types is
generated, if --drop-private-types was provided, then instruct the
suppression to drop matched types.
* tools/abipkgdiff.cc (options::drop_private_types): New option.
(options::options): Initialize the new drop_private_types data
member to false.
(display_usage): Add a usage string for --drop-private-types.
(parse_command_line): Parse the new --drop-private-types option.
(maybe_create_private_types_suppressions): Don't take just a
package, but a package_descriptor because the latter carries the
options. So when the user used the --drop-private-types option,
make the generated private types suppression to drop matched
types.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt:
Adjust.
* tests/test-diff-suppr.cc (in_out_specs): Likewise.
Dodji Seketeli [Mon, 16 Jan 2017 12:40:47 +0000 (13:40 +0100)]
Support virtual member functions with vtable offset not yet set
When reading C++ class informatin DWARF, it can happen that a given
virtual member function does not yet have a vtable offset. Right now,
that offset is set to zero. Just like a virtual member function which
actual vtable offset is zero. So we don't make a difference between a
virtual function with no vtable offset and a virtual function with a
vtable offset set to zero.
This can lead to confusions during class comparison.
This patch fixes that problem by setting the default vtable offset to
-1. So whenever a vtable offset is -1 it means that the virtual
member function doesn't yet have a vtable offset.
* include/abg-fwd.h (member_function_has_vtable_offset): Declare
new function.
(get_member_function_vtable_offset): Return a ssize_t, not a
size_t.
(set_member_function_vtable_offset): Take a ssize_t, not a size_t.
* include/abg-ir.h (class_decl::virtual_mem_fn_map_type): Adjust
the map typedef to make it take ssize_t as the type of the key.
(mem_fn_context_rel::vtable_offset_in_bits_): Make this data
member be of ssize_t type, not size_t.
(mem_fn_context_rel::mem_fn_context_rel): Initialize the
vtable_offset_in_bits_ data member to -1.
* src/abg-ir.cc (member_function_has_vtable_offset): Define new
function.
(get_member_function_vtable_offset): Return a ssize_t, not a
size_t.
(set_member_function_vtable_offset): Take a ssize_t, not a size_t.
* src/abg-dwarf-reader.cc (die_virtual_function_index): Take an
int64_t& rather than a uint64_t&.
(finish_member_function_reading): Don't set the vtable offset if
it's -1.
* src/abg-reader.cc (build_class_decl): Likewise.
Dodji Seketeli [Fri, 13 Jan 2017 23:18:44 +0000 (00:18 +0100)]
[dwarf reader] properly separate function decls and types in lookup
We were sometimes forgetting to properly add some types to the lookup
maps as such. Instead, the types were added as if they were decls,
leading to later type lookup to fail.
Fixed thus.
* src/abg-dwarf-reader.cc
(read_context::associate_die_to_artifact_by_repr_internal):
Choose the right type of representation depending on if we are
associating a type or a decl.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt: Adjust.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt: Adjust.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Adjust.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Adjust.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Adjust.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Adjust.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Adjust.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Adjust.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Adjust.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Adjust.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Adjust.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Adjust.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Adjust.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust.
Dodji Seketeli [Fri, 13 Jan 2017 23:14:34 +0000 (00:14 +0100)]
[dwarf reader] Allow updating and de-duplicating member functions
We forget to register member functions for lookup (by DWARF DIE
representation). So member functions are not correctly found (by
lookup) and thus are not properly de-duplicated or updated.
* src/abg-dwarf-reader.cc (add_or_update_class_type): Register
member functions for lookup by member function DIE representation.
Dodji Seketeli [Fri, 13 Jan 2017 22:47:55 +0000 (23:47 +0100)]
[dwarf reader] Fix pretty printing static methods from DWARF
When pretty printing static methods from DWARF, we were forgetting the
first parameter for static methods.
* src/abg-dwarf-reader.cc
(die_return_and_parm_names_from_fn_type_die): Take a new
'is_static' parameter.
(die_qualified_type_name, die_pretty_print_type): Adjust calling
die_return_and_parm_names_from_fn_type_die.
(die_function_signature): Likewise. Also, do not forget to print
the first parameter for a static method.
Dodji Seketeli [Fri, 13 Jan 2017 21:46:36 +0000 (22:46 +0100)]
Handle several virtual member functions having the same vtable offset
In the DWARF for C++, it can happens that a virtual constructor leads
to several (up to 3) destructor functions that all have the same
vtable offset. That vtable offset is the same as the offset of the
virtual destructor that the user actually defined in her source code.
This patch adds a map data structure to the private data of
class_decl. That map associates a vtable offset X to a vector of
virtual member functions that have the same vtable offset X.
That new map is populated whenever a virtual member function is added
to the class.
* include/abg-ir.h (class_or_union::virtual_mem_fn_map_type):
Define new typedef.
(class_decl::get_virtual_mem_fns_map): Declare new accessor.
* src/abg-ir.cc (class_decl::priv::virtual_mem_fns_map_): New data
member.
(class_decl::get_virtual_mem_fns_map): Define new accessor.
(fixup_virtual_member_function): Populate the new virtual member
functions map.
(class_decl::on_canonical_type_set): Sort the virtual member
function vectors stored in the new virtual member functions map.
(class_decl::add_member_function): Call
set_member_function_is_virtual *after* calling
set_member_function_vtable_offset because the former updates the
virtual function map, so it needs the vtable offset.
* src/abg-dwarf-reader.cc (finish_member_function_reading):
Likewise.
* src/abg-reader.cc (build_class_decl): Likewise.
git://sourceware.org / libabigail.git / log