comparison: Ensure that fn parms with basic types can't be redundant
When comparing the two libc binaries from
"https://the.owo.foundation/Ac1Ksw8.tgz", abidiff crashes.
It appears to be due to an assert that is hit because the overload of
default_reporter::report for fn_parm_diff momentarily removes the
redundant-ness from the categorization of the diff node of the type of
a function parameter. The problem is that the sole child diff node of
that type diff node is itself redundant. So the function parameter
type diff node should really be redundant too. Oops, there is a logic
violation there, hence the assert violation.
I commented out the line that removes the redundant-ness. After all,
if function parameter types shouldn't be redundant, that should have
been taken care of by the redundancy_marking_visitor code in
abg-comparison.cc as well as its associated category propagation code.
But then consider what happens with a reproducer of the libc binaries
above:
$ cat test-PR29387-v0.c
typedef int Integer;
void
f0(Integer i, char c)
{
i + c;
}
void
f1(Integer i, unsigned char c)
{
i + c;
}
$
$ cat test-PR29387-v1.c
typedef long int Integer;
[C] 'function void f0(Integer, char)' at PR29387-test-v1.c:4:1 has some indirect sub-type changes:
parameter 1 of type 'typedef Integer' changed:
underlying type 'int' changed:
type name changed from 'int' to 'long int'
type size changed from 32 to 64 (in bits)
[C] 'function void f1(Integer, unsigned char)' at PR29387-test-v1.c:10:1 has some indirect sub-type changes:
$
$
So, the problem is this peace of report:
[C] 'function void f1(Integer, unsigned char)' at PR29387-test-v1.c:10:1 has some indirect sub-type changes:
You see that the report is empty; the reporter could not say what changed.
What changed is the typedef "Integer" that changed from "int" to "long
int". The redundancy_marking_visitor pass marked the change of the
underlying type of the Integer typedef as being redundant. This is
because that typedef change has already been reported on the f0
function interface.
The problem is that by design, the 'int' to 'long int' change should
not have been marked as being redundant by the
redundancy_marking_visitor pass. This is because, we want to see all
the "basic type changes" on function parameters types. They are
deemed "local changes" of the function types, and we want to see all
local changes to functions because it is almost 100% sure these are
non-compatible changes.
The root cause of the problem is that the function
has_basic_type_change_only in abg-comparison.cc fails to detect that
the parameter change carries a typedef-to-basic-type change, so the
function parameter is wrongly marked as being redundant even though it
ultimately carries a basic type change.
This patch thus teaches has_basic_type_change_only to look through
parameter changes to better detect basic type changes.
* include/abg-comparison.h (peel_fn_parm_diff)
(peel_typedef_qualified_type_or_parameter_diff): Declare ...
* src/abg-comparison.cc (peel_fn_parm_diff)
(peel_typedef_qualified_type_or_parameter_diff): ... new
functions.
(has_basic_type_change_only): Look through function parameters,
typedefs and qualified types.
* src/abg-default-reporter.cc (default_reporter::report): Remove
the temporary removal of the redundant categorisation.
Redundant-ness should have been handled by the
redundancy_marking_visitor pass.
* tests/data/test-diff-filter/test-PR29387-report.txt: Reference
test output.
* tests/data/test-diff-filter/test-PR29387-v{0,1}.c: Source of the
input tests.
* tests/data/test-diff-filter/test-PR29387-v{0,1}.o: Input test
binaries.
* tests/data/Makefile.am: Add the new test material above to the
source distribution.
* tests/test-diff-filter.cc (in_out_specs): Add the test binaries
above to the test harness.
ir, writer: Go back to canonicalizing typedefs in the IR
Now that DIE and IR canonicalizing has progressed in precision,
especially due to fixing the canonical type propagation it seems like
we can go back to canonicalizing typedefs again. This makes the
writer not needing to handle non-canonicalized types because only a
short number and types of types are still non-canonicalized.
Then I ran the test suite and hell broke lose. Things would work on a
given platform but won't on others, etc.
So a great deal of the patch actually fixes issues uncovered by the
fact that we are back to canonicalizing typedefs.
The symptoms of many of the issues are related to emitting abixml in a
stable manner across toolchains. But in reality those issues are
real, deeper ones.
This patch is thus ALSO an attempt at fixing all the issues at once
because they can't be separated. Either the abixml is stable and
"make check" passes on all platforms, or it is not and testing fails
on some platforms.
I believe that at the core of the problems lies the observation that a
given typedef (of a given name) could appear several times (the number
of times not being quite constant) in a given class or namespace.
That would lead to some instabilities in the sorting and ordering of
type-ids depending on the toolchain being used, for instance.
To fix this add_or_update_class_type in the DWARF reader is taught to
avoid adding a typedef to a class if a member type of the same name
already exists at that class scope. This handles the fact that a
given class can be built piece-wise in the DWARF. Also, when handling
a typedef type, build_ir_node_from_die is taught to avoid creating a
typedef IR for a given scope (namespace, union or class) if said scope
already contains a type of the same name. To be able to do that, the
handling of member types is moved from the ir::class_or_union type to
the ir::scope_decl type. That way, all scopes (not just classes or
unions) can handle looking up for the (member) types they contain.
Another issue that was uncovered is that when emitting decl-only class
named A (in a namespace for instance) there can be some instability as
to which decl-only class A is emitted. This is due to the fact that
libabigail considers all decl-only classes named A to be equal. The
problem however is that a decl-only class A might have member types.
And a decl-only A declared in a translation unit somewhere might have
some member types that are different from the same decl-only A
declared in another translation unit. This doesn't necessarily
violate the ODR, but rather, can be the result of compiler
optimization. For instance, in a given translation unit, only a given
member type of the decl-only A is used by the code, whereas in another
one, another member type of the same decl-only A is used. So only
partial views of A are emitted in the translation unit depending on
what is used. Anyway, to handle that case, when comes the time to
emit the decl-only A in the ABIXML writer, write_class_decl now emits
all the member types of all the instances A known to the system. This
hopefully removes the instability I was seeing. To do that,
maybe_update_types_lookup_map<class_decl> has been taught to also keep
track of decl-only classes. A new lookup_decl_only_class_types has
been defined to gather all the instances of a given decl-only class
known to the system.
Last but not least, the topological type sorting facilities carried by
the types {decl,type}_topo_comp has been fixed to ensure a more stable
sorting and also to ensure that "abilint foo.abi" emits the decls and
types in the same order as the one defined in foo.abi. This fixes
another abixml instability I was seeing in the test suite across
different toolchains. While doing that, I noticed that the ABIXML
reader was forgetting to properly mark the corpus as originating from
ABIXML. So I fixed that too.
* include/abg-fwd.h (lookup_decl_only_class_types): Declare new
function.
* src/abg-ir-priv.h (class_or_union::priv::member_types_): Move
this data member into scope_decl::priv.
(class_or_union::priv::priv): Do not take member types.
* include/abg-ir.h (sort_type): Likewise.
(namespaces_type): Add new typedefs.
(scope_decl::insert_member_decl): Make this be non-virtual.
(scope_decl::{insert_member_type, add_member_type,
remove_member_type, get_member_types, get_sorted_member_types,
find_member_type}): Move these methods here from ...
(class_or_union::{insert_member_type, add_member_type,
remove_member_type, get_member_types, get_sorted_member_types,
find_member_type}): ... here.
(class_or_union::insert_member_decl): Make this be non-virtual.
(class_decl::insert_member_decl): Make this be non-virtual.
(class_or_union::get_sorted_member_types): Declare ...
* src/abg-dwarf-reader.cc (add_or_update_class_type): Do not add a
member type to the class type being built if it already exists
there.
(build_ir_node_from_die): If a scope (namespace, union or class)
already has a typedef, do not create a new one to add it there
again.
* src/abg-ir.cc (equals): In the overload for typedefs take into
account the name of typedefs again.
(lookup_decl_only_class_types): Define new function.
(compare_using_locations): Define new static function that has
been factorized out of ...
(decl_topo_comp::operator()): ... here. Also, two decls originate
from an abixml corpus, compare them only using their artificial
locations, meaning make them keep the same order as the order of
the original abixml file.
(type_top_comp::operator()): Likewise.
(sort_types): Define new function.
(scope_decl::priv::member_types_): Move this here from
class_or_union::priv.
(scope_decl::priv::sorted_member_types_): Define new data member.
(scope_decl::{get_member_types, find_member_type,
insert_member_type, add_member_type, remove_member_type}): Move
these methods here from class_or_union.
(scope_decl::get_sorted_member_types): Define new method.
(is_non_canonicalized_type): Canonicalize typedefs.
(lookup_type_in_map): Allow this to look through decl-only types
to get their definition. Otherwise, if only a decl-only was
found, return it.
(maybe_update_types_lookup_map<class_decl>): Allow adding
decl-only class types to the map of classes held per TU and per
corpus.
(class_or_union::{class_or_union, add_member_decl,
has_no_member}): Adjust.
(class_or_union::{insert_member_type, add_member_type,
add_member_type, remove_member_type, get_member_types,
get_sorted_member_types, find_member_type}): Move these methods to
scope_decl.
({class_decl, class_or_union}::insert_member_decl): Remove the
"before" parameter as it was not used anymore due to the re-use of
the scope_decl::add_member that handles member types.
* src/abg-reader.cc (read_corpus_group_from_input)
(create_native_xml_read_context): Set the corpus origin.
* src/abg-writer.cc (write_context::{m_nc_type_id_map,
m_emitted_non_canonicalized_type_set,
m_referenced_non_canonicalized_types_set}): Remove these maps that
hold stuff for non-canonicalized types.
(write_context::{type_has_existing_id, get_id_for_type,
clear_type_id_map, has_non_emitted_referenced_types,
record_type_as_referenced, type_is_referenced,
record_type_as_emitted, type_is_emitted, clear_referenced_types,
write_referenced_types, write_canonical_type_ids}): Adjust these
as the maps for non-canonicalized types are gone.
(write_context::{get_referenced_non_canonicalized_types,
get_emitted_non_canonicalized_type_set}): Remove these methods.
(write_decl_in_scope)
(write_class_decl_opening_tag, write_union_decl_opening_tag)
(write_union_decl): Adjust comments.
(write_class_decl): Sort member types. Also, When emitting a
decl-only class, get all of the decl-only classes of the same name
declared in several other TUs and emit all their member types into
this decl-only class just once. This reduces redundancies and
sorting instabilities because for libabigail, all decl-only
classes of a given name are equal, even if they have member types.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
* tests/data/test-annotate/libtest23.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-annotate/test-anonymous-members-0.o.abi:
Likewise.
* tests/data/test-annotate/test0.abi: Likewise.
* tests/data/test-annotate/test1.abi: Likewise.
* tests/data/test-annotate/test13-pr18894.so.abi: Likewise.
* tests/data/test-annotate/test14-pr18893.so.abi: Likewise.
* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-annotate/test2.so.abi: Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt:
Likewise.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt:
Likewise.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt:
Likewise.
* tests/data/test-diff-filter/test41-report-0.txt: Likewise.
* tests/data/test-diff-pkg/PR24690/PR24690-report-0.txt: Likewise.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Likewise.
* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Likewise.
* tests/data/test-read-dwarf/PR26261/PR26261-exe.abi: Likewise.
* tests/data/test-read-dwarf/PR27700/test-PR27700.abi: Likewise.
* tests/data/test-read-dwarf/libtest23.so.abi: Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi:
Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-read-dwarf/test-PR26568-1.o.abi: Likewise.
* tests/data/test-read-dwarf/test-PR26568-2.o.abi: Likewise.
* tests/data/test-read-dwarf/test-libaaudio.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/test0.abi: Likewise.
* tests/data/test-read-dwarf/test0.hash.abi: Likewise.
* tests/data/test-read-dwarf/test1.abi: Likewise.
* tests/data/test-read-dwarf/test1.hash.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-read-dwarf/test2.so.abi: Likewise.
* tests/data/test-read-dwarf/test2.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
* tests/data/test-read-write/test18.xml: Likewise.
* tests/data/test-read-write/test28-without-std-fns-ref.xml:
Likewise.
* tests/data/test-read-write/test28-without-std-vars-ref.xml:
Likewise.
Fix IR comparison result caching and canonical type propagation tracking
Caching the result of IR comparison cannot happen on IR nodes that are
being compared in the context of type canonicalization if one of the
nodes is the target of canonical type propagation. This is especially
true if the recursive IR node which comparison "temporarily" did yield
true (to avoid an infinite loop) at least until the comparison of the
rest of the sub-tree of the recursive type is done. In that case, we
should not cache the result the comparison as it might change later.
The patch adds a way to track recursive types so that we know when not
to cache their comparison result.
As we now have a facility to track recursive types during canonical
type propagation (rather than just types that depend on recursive
types) we can be a bit more precise when confirming or cancelling
types that have been subject to canonical type propagation.
Also the patch cleans up the detection of comparison cycle to make it
more typesafe so that the macro
RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED can be used for class_decl
and union_decl, not just class_or_union. It makes the code more
readable/maintainable is the equals overload for class_decl and
union_decl all have their proper call to
RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED. The same is true for
mark_types_as_being_compared.
Doing that cleans up the detection of recursive types as well as the
types that depends on those recursive types.
* src/abg-ir-priv.h (environment::priv::recursive_types_): Define
new data member.
(environment::priv::cache_type_comparison_result): Cache results
only non-recursive and not dependant types, or when the result is
"false". In all these cases, the result, once cached, will not
change.
(environment::priv::mark_dependant_types_compared_until): Mark
types as recursive at the same time others are marked as
dependant.
(environment::priv::{is_recursive_type, set_is_not_recursive}):
Define new member functions.
(environment::priv::{confirm_ct_propagation,
cancel_ct_propagation}): Confirming canonical type propagation
should happen for recursive types as well as their dependant
types.
* src/abg-ir.cc (return_comparison_result): Keep up with the
book-keeping at all time when type canonicalization process is
on-going. Whenever we expect types that depends on recursive
types, expect recursive types too, obviously.
(type_base::get_canonical_type_for): Do not erase the comparison
result cache between the canonicalization of two different types.
(is_comparison_cycle_detected)
(mark_types_as_being_compared, unmark_types_as_being_compared):
Define new overloads for class_decl;
(equals): In the overloads for class_decl and union_decl, use
RETURN_COMPARISON_RESULT and mark_types_as_being_compared without
casting it to class_or_union.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
Adjust.
* tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.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-2.txt:
Adjust.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt:
Adjust.
* 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.
Allow restricting analyzed decls to exported symbols
Profiling showed that the DWARF reader scans too much data.
Basically, in build_translation_unit_and_add_to_ir,
build_ir_node_from_die is called on every single DIE that is seen, for
a given translation unit.
There are interfaces (function and variable decls) that are not
associated with exported ELF symbols and that are analyzed by
build_ir_node_from_die nonetheless. For instance, interfaces that are
visible outside of their translation units are analyzed and the types
that are reachable from those interfaces are analyzed as well.
Once that is done, an ABI corpus is built with the subset of
interfaces that have exported ELF symbol (strictly those that are part
of the ABI), but types that are not necessarily reachable from those
ABI interfaces can also be put into the ABI corpus.
Some tools make use of this "lose" behaviour of libabigail. For
instance, abicompat precisely wants to analyze interfaces with
undefined symbols. For an application, those interfaces represents
the interfaces that the application expects to be provided by some
shared library.
When analyzing the exported interface of the Linux Kernel (or any
other huge application) however, analyzing more types than necessary
appears to incur a huge time penalty.
So, this patch introduces an optional behaviour whereby
build_translation_unit_and_add_to_ir is restricted to analyzing
interfaces that have exported ELF symbols only. So only the types
reachable from those interfaces are analyzed. This more than halves
the time spent by "abidw --noout vmlinux".
Strictly speaking, this new behaviour is triggered by a new option named
--exported-interfaces-only, supported by the tools abidw, abidiff,
abipkgdiff and kmidiff.
When looking at the Linux Kernel however, this option is enabled by
default.
Note that an option --allow-non-exported-interfaces is also introduce
to function under the previous model of operations. This option is
enabled by default on all the tools when they are not looking at the
Linux Kernel.
With this enabled, analyzing the Linux Kernel is back to taking less
than a minute on a reasonable machine.
* doc/manuals/tools-use-libabigail.txt: New doc text.
* doc/manuals/Makefile.am: Add the new tools-use-libabigail.rst
tool to the source distribution.
* doc/manuals/abidiff.rst: Include the new
tools-use-libabigail.rst. Document the --exported-interfaces-only
and --allow-non-exported-interfaces.
* doc/manuals/abidw.rst: Likewise.
* doc/manuals/abipkgdiff.rst: Likewise.
* doc/manuals/kmidiff.rst: Likewise.
* include/abg-ir.h
(environment::{user_set_analyze_exported_interfaces_only,
analyze_exported_interfaces_only}): Declare new accessors.
* src/abg-ir.cc
(environment::{user_set_analyze_exported_interfaces_only,
analyze_exported_interfaces_only}): Define new accessors.
* src/abg-dwarf-reader.cc (die_is_variable_decl)
(die_is_function_decl): Define new static functions.
(read_context::is_decl_die_with_exported_symbol): Define new
member function.
(read_context::get_{function,variable}_address): Const-ify the
Dwarf_Die* parameter.
(build_translation_unit_and_add_to_ir): If the user asks to
analyze exported interfaces only, the analyze only interfaces
that have exported ELF symbols.
(read_debug_info_into_corpus): If we are looking at the Linux
Kernel, then only analyze exported interfaces unless the user asks
otherwise.
* src/abg-ir-priv.h
(environment::priv::analyze_exported_interfaces_only_): Define new
data member.
* tools/abidiff.cc (options::exported_interfaces_only): Define new
data member.
(display_usage): Add new help strings for
--exported-interfaces-only and --allow-non-exported-interfaces.
(parse_command_line): Parse the new options
--exported-interfaces-only and --allow-non-exported-interfaces.
(main): Pass the value of opts.exported_interfaces_only to the
environment.
* tools/abidw.cc (options::exported_interfaces_only): Define new
data member.
(display_usage): Add new help strings for
--exported-interfaces-only and --allow-non-exported-interfaces.
(parse_command_line): Parse the new options
(load_corpus_and_write_abixml)
(load_kernel_corpus_group_and_write_abixml): Pass the value of
opts.exported_interfaces_only onto the environment.
* tools/abipkgdiff.cc (options::exported_interfaces_only): Define new
data member.
(display_usage): Add new help strings for
--exported-interfaces-only and --allow-non-exported-interfaces.
(parse_command_line): Parse the new options
(compare_task::perform, self_compare_task::perform): Pass the
value of opts.exported_interfaces_only onto the environment.
(compare_prepared_linux_kernel_packages): Likewise.
* tools/kmidiff.cc(options::exported_interfaces_only): Define new
data member.
(display_usage): Add new help strings for
--exported-interfaces-only and --allow-non-exported-interfaces.
(parse_command_line): Parse the new options
(main): Pass the value of opts.exported_interfaces_only onto the
environment.
dwarf-reader: Revamp the canonical type DIE propagation algorithm
During the type DIEs comparison for canonicalization we need to
perform the optimization called "canonical type propagation". This is
very similar to what we do for type canonicalization at the IR level.
It's described in-extenso in abg-ir.cc, in the comment that starts
with "@defgroup OnTheFlyCanonicalization".
Basically during canonicalization, suppose we end-up comparing a
sub-type pair {Sl, Sr}. Suppose Sr does already have a canonical
type. Suppose also, that {Sl, Sr} ends up comparing equal. We can
thus deduce that Sl would have the same canonical type as Sr. So we
'propagate' the canonical type of Sr onto Sl. Sl is said to have been
canonical-type-propagated from Sr.
Now, I need to introduce the concept of redundant type.
Consider an aggregate type T and its sub-types ST0, ST1 and ST2 which
looks like this:
T
+-- ST0
|
+-- ST1
| +
| |
| +-- T
|
+-- ST2
Notice how the sub-type ST1 itself has a sub-type T.
Now, consider the type T', similar to T, which looks like:
Now consider how libabigail compares those two types T and T'
member-wise, aka structurally:
T T'
+-- ST0 +-- ST0'
| |
+-- ST1 +-- ST1' <--- Let's call this point #0.
| + | +
| | | |
| +-- T | +-- T' <--- Let's call this point #1.
| | Note that the sub-types of
+-- ST2 +-- ST2' T and T' are not
| represented at this point
+-- ST'3 but they do exist!
Representing them would lead
to a never-ending graph, right?
The properties of T are compared against the properties of T', but to
complete that comparison, the sub-type ST0 is compared against ST0',
etc. A comparison stack is thus maintained. Each member of the stack
is the pair of sub-types being compared. That content changes as
different sub-types are compared. Let's consider the content of the
stack when we reach the point #0 in the graph above. That stack
content would look like:
[{T,T'} | {ST0, ST0'}]
If we keep going and reach the point #1, the content of the stack
becomes:
[{T,T'} |{ST0, ST0'} | {T, T'}]
At this point, we see that {T,T'} appears twice in the stack. If we
keep going and explore again the sub-types of {T,T'}, an infinite loop
appears. The pair {T,T'} is said to be redundant.
To avoid those infinite loops, redundancy detection is done by
compare_dies in abg-dwarf-reader.cc. When compare_dies detects at #1 that
{T,T'} was already present in the stack then it returns "true" early,
as if in #1, T compares equal to T'.
But then what does that mean for the value of comparing {ST0,ST0'}?
The sub-type {T,T'} in #1 compared equal, so compare_dies assumes that
{ST0,ST0'} compares equal as well, right? That is what libabigail
used to assume before the commit
Canonicalize DIEs w/o assuming ODR & handle typedefs transparently
Well, it turns out we need to compare the entire tree of sub-types of
{T,T'} until we reach {ST2, ST2'}. At that point, compare_dies sees
that ST2' has a sub-type ST3' where ST2 has none. So {ST2,ST2'}
compares /different/. So {T,T'} compares different. So back to #0,
because {ST1,ST2'} has {T,T'} as sub-type (or said differently,
{ST1,ST2'} depends on the redundant pair {T,T'}) then {ST1,ST1'}
compares different.
So {ST1,ST1'} compares different even though it were initially thought to
compare equal because compare_dies had to get out early in #1,
considering that {T,T'} compared equal at that point.
Now, there are two ways to operate when we reach #1:
1/ Avoid performing canonical type propagation as soon as we detect
that {T,T'} is redundant. That avoidance lasts until we finish
comparing {ST2,ST2'}, that is, until the complete comparison of
{T,T'}. That means hat comparing every single sub-type is almost
assured to be done structurally, rather than canonically.
2/ Speculate that {T,T'} compare equal, unless proved otherwise. If
{T,T'} proves to be equal, then things are well and fast. If they
prove different, then {ST0,ST0'} must be edited afterwards to cancel
the canonical type propagation that might have taken place. In other
words, sub-types that depends on redundant types pairs must be tracked
until the comparison of those redundant type pairs is done. If a
redundant type pair compares different then all the sub-types that
depend on it must be walked to have their propagated canonical types
erase as if no canonical type propagation ever took place.
The first approach is the one I initially put in place with the patch
referred to above. It proved to be super slow. Analyzing the kernel
was taking literally hours. Oops.
This patch implements the second approach. It's more involved than
the first approach. But it brings the time down to around 2 minutes
on a reasonably fast machine. This is still slower than what I would
like to see, but it's way better than what had with the first
approach.
A subsequent patch will bring the analysis time for the kernel further
down. But in the mean time, this one is really needed, I think.
So the patch introduces a new type named offset_pairs_stack_type to
track the pairs of type DIEs being compared. Each DIE is now
identified by a new offset_type type. That type contains the
traditional DIE offset using the Dwarf_Off type from elfutils, but it
also contains the source of that DIE. The offset_pairs_stack_type
also tracks redundant type pairs and their dependant type pairs.
compare_dies is modified to return a value that is an instance of the
new enum comparison_result. The results can be
COMPARISON_RESULT_EQUAL when a type pair compares equal,
COMPARISON_RESULT_DIFFERENT when a type pair compares different,
COMPARISON_RESULT_CYCLE_DETECTED when a redundant type is detected,
leading to a comparison cycle, and COMPARISON_RESULT_UNKNOWN when the
outcome of the comparison cannot be known because we are comparing a
pair that depends on a redundant pair.
A new function return_comparison_result is introduced. It's intended
to be invoked right before returning from compare_dies. It looks at
the actual comparison result and depending on the state of the stack
of type pairs being compared, handles the book keeping of redundant
types and their dependant types. It also handles when to propagate
canonical types if necessary and when to cancel the canonical types
that might have been propagated.
The ABG_RETURN macro has been adapted to invoke
return_comparison_result before returning out from compare_dies.
* src/abg-ir-priv.h (enum comparison_result): Define new enum.
* src/abg-dwarf-reader.cc (type_comparison_result_to_be_cached)
(maybe_cache_type_comparison_result)
(get_cached_type_comparison_result)
(maybe_get_cached_type_comparison_result)
(maybe_propagate_canonical_type, propagate_canonical_type)
(return_comparison_result): Define new static functions.
(has_offset_pair, insert_offset_pair, erase_offset_pair)
(have_offset_pair_in_common): Remove static functions.
(read_context::die_comparison_visits_): Remove data member. The
concept supported by this data member is now replaced by caching
the results of comparing aggregate types, especially those that
are not yet canonicalized. This essentially prevents the same
aggregate type pair to be compared again and again.
(read_context::{die_comparison_results_, compare_count_,
canonical_propagated_count_, cancelled_propagation_count_}): New
data members.
(read_context::initialize): Initialize the new data members
compare_count_, canonical_propagated_count_,
cancelled_propagation_count_ of integral type.
(read_context::{erase_canonical_die_offset}): New member
functions.
(struct offset_pairs_stack_type): Define new type.
(die_offset): Remove.
(is_canon_type_to_be_propagated_tag): Add union types to the set
of types for which canonical type propagation might occur.
(is_type_die_to_be_canonicalized): Add function types and array
types to the types to be canonicalized.
(ABG_RETURN): Change this macro to consider
COMPARISON_RESULT_DIFFERENT rather than the "false" boolean.
Also, it uses the new return_comparison_result function.
(ABG_RETURN_FALSE): Likewise, use the new return_comparison_result
function.
(SET_RESULT_TO_FALSE): Make this return
COMPARISON_RESULT_DIFFERENT.
(SET_RESULT_TO, RETURN_IF_COMPARISON_CYCLE_DETECTED): Define new
macros.
(compare_dies): Make this return comparison_result rather than
just a bool. This is also the core of the overhaul of the
canonical DIE propagation algorithm. The algorithm is now similar
to the one implemented in the equals function for class_or_union
types in abg-ir.cc. It's described in the comment that starts
with '@defgroup OnTheFlyCanonicalization' in abg-ir.cc. The type
of the aggregates_being_compared parameter is now
offset_pairs_stack_type in parameter. No more need for the
redundant_aggregates_being_compared parameter. The new
offset_type that also encapsulates the source of the offset is now
used in lieu of the Dwarf_Off type. Results of comparing
aggregates being compared are now cached. When comparing
aggregates, use the RETURN_IF_COMPARISON_CYCLE_DETECTED to return
early if a cycle is detected. The invocation of the ABG_RETURN
macro (especially the call to return_comparison_result) is where
the book keeping for canonical types propagation takes place, so
remove the explicit code that was handling that from the end of
this function.
(read_debug_info_into_corpus): Print statistics about the number
of aggregates compared and canonical-type-propagated.
* tests/data/test-annotate/test14-pr18893.so.abi: Adjust.
* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-diff-pkg/GtkAda-gl-2.24.2-29.fc29.x86_64--2.24.2-30.fc30.x86_64-report-0.txt:
Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
Mark Wielaard [Tue, 19 Jul 2022 23:01:14 +0000 (01:01 +0200)]
Handle zero sh_entsize in get_soname_of_elf_file
Apparently guile produced ELF files don't set sh_entsize for the
dynamic section. Which would cause a divide by zero. Luckily we do
know how big an dynamic entry should be. So use gelf_fsize for
ELF_T_DYN if sh_entsize is zero.
* src/abg-dwarf-reader.cc (get_soname_of_elf_file):
Make sure entsize is non-zero before use.
In 'src/abg-reader.cc', the function
'walk_xml_node_to_map_type_ids(read_context& ctxt, xmlNodePtr node)'
finds all of the child nodes of 'node' that has the 'id' attribute,
and then put the child node into map where the 'id' of the child node
as key and the child node itself as the value.
But the comment for this function writes:
/* src/abg-reader.cc begin */
/// Walk an entire XML sub-tree to build a map where the key is the
/// the value of the 'id' attribute (for type definitions) and the key
/// is the xml node containing the 'id' attribute.
...
static void
walk_xml_node_to_map_type_ids(read_context& ctxt,
xmlNodePtr node)
...
/* src/abg-reader.cc end */
The second and third lines of the comment above says the the child node
as the key of the map, but it should be the value of the map.
This patch fix the problematic comment described above, from
'and the key is the xml node' to 'and the value is the xml node'.
When running runtestannotate on different architectures, some spurious
test failures can happen because the resulting abixml contains the
architecture of the original binary. This can be a problem for tests
where the binary is compiled on the fly. We don't yet have those, but
I was playing with some of these while debugging something else and
stumbled across that issue.
This patch thus removes mentions of the architecture of the binary,
just like what runtestreaddwarf does.
Bug PR29443 - Global variables not emitted to abixml in some cases
When a global variable named V has the same name as member variable
that appears in an anonymous scope and if the the abixml writer emits
the member variable V first, it gets confused when comes the time to
emit the global V as it wrongly thinks it's been already emitted.
This is because when emitting the "internal" pretty representation of
the member variable, libabigail fails to consider printing a qualified
name. So the two 'V' wrongly have names that can't be told apart.
For instance consider the testcase example:
struct A {
struct {
int xx; // #0
};
};
The qualified name of xx, for internal purposes (to name things
internally for the purpose of book keeping) would be:
'A::__anonymous_struct__::xx'.
Libabigail wrongly names it 'xx', hence the confusion with the global variable.
Fixed thus.
* src/abg-ir.cc (var_decl::get_pretty_representation): Always use
qualified name of vars in an anonymous scope for internal
purposes.
* tests/data/test-annotate/PR29443-missing-xx.o.annotated.abi: New
reference test output.
* tests/test-annotate.cc (in_out_specs): Add the above to the test
harness.
* tests/data/test-read-dwarf/PR29443-missing-xx.cc: New source
code for the test.
* tests/data/test-read-dwarf/PR29443-missing-xx.o: New test input
binary.
* tests/data/test-read-dwarf/PR29443-missing-xx.o.abi: New test
reference output.
* tests/data/Makefile.am: Add the new test material to source
distribution.
* tests/test-read-dwarf.cc (in_out_specs): Add the above to the
test harness.
ctf-reader: Lookup debug info for symbols in a non default archive member
The current mechanism used by the ctf reader for looking for debug
information given a specific Linux symbol is the following: it opens
the dictionary (default) which name matches the binary name being
processed in the current corpus, e.g. `vmlinux' or
`module-name`.ko. However there are symbols and information that are
not located in the default dictionary; this is evident comparing the
symbols in `Module.symvers' file with ABI XML file, so for example,
the ctf reader is expecting to find the information for
`LZ4_decompress_fast' symbol in the CTF `vmlinux' archive member,
because this symbols is defined in `vmlinux' binary:
But, it figures out that it is missing. The correct location is
`vmlinux#0' dictionary:
CTF archive member: vmlinux:
...
Function objects:
...
CTF archive member: vmlinux#0:
Function objects:
...
LZ4_decompress_fast -> 0x80037400: (kind 5) int (*) (const char *, char *, int) (aligned at 0x8)
...
Therefore, ctf reader must be looking for debug information in the
whole archive; fortunately `libctf' provides a fast lookup mechanism
using cache, dictionary references, etc., so the penalty performance
is ~10%.
Now, it make use of `ctf_lookup_by_symbol_name' at first instance
which is in charge to locate symbol information given a symbol name on
either CTF Function or Variable sections; if the symbol isn't found it
tries using `ctf_lookup_variable' to look into the CTF Variable
section; this could happens due to `ld' operating with the
`--ctf-variables' option which makes function types information to
reside in the CTF Variable section.
* src/abg-ctf-reader.cc (lookup_symbol_in_ctf_archive): New function.
(process_ctf_archive): Use `lookup_symbol_in_ctf_archive'.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Add libtool versioning to libabigail.so starting from 2.1 onward to
not cause problems with older utilities due to library version
incompatibility. This will also stop tools that look for libraries
which have a different ABI but the same version from complaining.
* configure.ac: Define the variables libabigail_so_{current,
revision, age}. These are to be adjusted after each releases
depending on how the compatibility status of the libabigail's
code.
* src/Makefile.am: Add -version-info
$(LIBABIGAIL_SO_CURRENT):$(LIBABIGAIL_SO_REVISION):$(LIBABIGAIL_SO_AGE)
to LDFLAGS.
Signed-off-by: Ben Woodard <woodard@redhat.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
dwarf-reader: Simplify the canonicalization decision of types added to IR
This is a long overdue clean-up.
Back in the day when we had no DWARF type DIE canonicalization, there
could be a LOT of IR types to be canonicalized. To store set of those
types to be canonicalized, we the used the already existing
association type DIE offset -> IR types that we had, and so we only
had to store the DIE offsets of the IR types that we wanted to
canonicalize. At canonicalization time, we'd walk the set of DIE
offsets we stored on the side, use the association type DIE offset ->
IR types to retrieve the IR type to canonicalize and we'd canonicalize
it. This is somewhat complicated.
Now that we have DWARF DIEs canonicalization, the number of IR types
dropped significantly so this complicated scheme is no more warranted.
So this patch drops that indirection and stores the IR types to be
canonicalize into a vector of IR types and that's it. Yay, less code.
* src/abg-dwarf-reader.cc (maybe_canonicalize_type): Remove
the overload that takes a Dwarf_Die* parameter.
(operator++(die_source& source)): Likewise.
(read_context::{types_to_canonicalize_,
alt_types_to_canonicalize_,
type_unit_types_to_canonicalize_}): Remove these data members
of type vector<Dwarf_Off>. (read_context::initialize): Remove
invocations to alt_types_to_canonicalize_.clear(),
type_unit_types_to_canonicalize_.clear(), and
extra_types_to_canonicalize_.clear().
(read_context::extra_types_to_canonicalize_): Rename this into
read_context::types_to_canonicalize_, of type
vector<type_base_sptr>.
(read_context::types_to_canonicalize): Remove these member
functions that take a parameter of type die_source.
(read_context::extra_types_to_canonicalize): Rename this
function into types_to_canonicalize. It returns a type const
vector<type_base_sptr>&.
(read_context::schedule_type_for_late_canonicalization):
Remove this overload that takes a type const Dwarf_Die*. In
the overload that takes a parameter const type_base_sptr
however, rename the invocation to
extra_types_to_canonicalize_.push_back(t) into
types_to_canonicalize_.push_back(t).
(read_context::canonicalize_types_scheduled): This doesn't
take a die_source parameter anymore. It now only cycles
through the types retrieved by types_to_canonicalize() to
canonicalize them.
(read_context::add_late_canonicalized_types_stats): Remove the
die_source parameter. Use types_to_canonicalize().
(read_context::perform_late_type_canonicalizing): Just call
read_context::canonicalize_types_scheduled().
(build_ir_node_from_die): Adjust calls to maybe_canonicalize_type.
Also, really canonicalize the function type when a function decl is
constructed.
* tests/data/test-annotate/test13-pr18894.so.abi: Adjust.
* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.txt:
Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
ir: translation_unit::is_empty should work without environment.
I noticed that some code failed trying to call
translation_unit::is_empty in the absence of the instance of the
abigail::ir::environment type used to create the IR.
This is because translation_unit::is_empty was trying to create
something. That's odd. It shouldn't need to create anything to test
for its emptiness. Fixed thus.
* src/abg-ir.cc (translation_unit::is_empty): If there is no
global scope, then we know its empty. No need to create one.
ir: Don't overdo canonical type propagation control when comparing classes
While looking at something else, I stumbled upon this problem.
When comparing a class, equals first calls the overload for
class_or_union to compare data members. If we are in the process of
type canonicalization, the right hand side operand might be
"canonical-type-propagated", during that call to the overload. In
other words, it can inherit the canonical type of the left-hand-side
operand. The problem is that that canonical type propagation, if it
happens, is too early because this equals function still needs to
compare other things like virtual member functions etc. So, the
original intend of the code was to erase the canonical type that might
have been propagated. This is all and well.
The problem however is that the code /always/ erases the canonical
type of the right hand side operand, even if it was the result of the
propagation optimization long before it entered this equals function.
Oops.
This patch fixes that issue.
* src/abg-ir.cc (equals): In the overload for const class_decl&,
do not cancel the propagated canonical type if the propagation is
not the result of invoking the equals overload for class_or_union.
Add test-abidiff-exit/ld-2.28-21{0,1}.so to source distribution
I forgot to update tests/data/Makefile.am when I added the test input
files test-abidiff-exit/ld-2.28-21{0,1}.so and
test-abidiff-exit/test-ld-2.28-210.so--ld-2.28-211.so.txt. Fixed thus.
Also, I am adding a new file that describes how to get the source code
of those binaries.
* tests/data/Makefile.am: Add the new files to source
distribution. Also, add the new
test-abidiff-exit/ld-2.28-21x.so.sources.txt.
* tests/data/test-abidiff-exit/ld-2.28-21x.so.sources.txt: New
file describing where the sources are.
Giuliano Procida [Thu, 25 Aug 2022 11:48:56 +0000 (12:48 +0100)]
abidw: resolve declaration-only enums the same as classes
The logic for resolving declaration-only enums and classes was almost
the same. However, the class code had a couple of extra improvements
that were missing from the enum code. One of these caused resolution
failures with Linux kernel ABIs, resulting in duplicate (declared /
defined) enums in ABI XML.
This change adds the improvements to the enum resolution code.
* src/abg-dwarf-reader.cc
(read_context::resolve_declaration_only_enums): Use an ordered
map to ensure TUs are always considered in the same order and
so improve ABI XML stability. Given multiple possible
definitions for a enum declaration, check to see if they are
equal and resolve the declaration to the first definition if
so.
Giuliano Procida [Thu, 25 Aug 2022 11:48:55 +0000 (12:48 +0100)]
abidw: remove always true test in resolve_declaration_only_classes
The code that makes the last attempt to resolve declaration-only types
was protected by a conditional checking that the number of TUs for a
given type was more than 1. The previous branch checked for exactly 1.
However, the entire block is inside a conditional where the number of
TUs is guaranteed to be greater than 0.
Removing the conditional makes it clear that this branch handles all
remaining cases.
Giuliano Procida [Thu, 25 Aug 2022 11:48:54 +0000 (12:48 +0100)]
abidw: fix --stats output for resolved classes and enums
The code to print out the remaining declaration-only types
unintentionally omitted the first such type. This change fixes the
logic and uses a single test to decide whether or not to print the
stats header line.
* src/abg-dwarf-reader.cc
(read_context::resolve_declaration_only_classes): Fix
conditional logic so that showing stats includes the first
unresolved type.
(read_context::resolve_declaration_only_enums): Likewise.
ctf-reader: looks for debug information in out-of-tree modules
The archive `vmlinux.ctfa' contain CTF debug information for
all the types used by more than one module, CTF for the core
kernel and CTF for each module compiled in Linux tree directory.
CTF information for out-of-tree module is not present in
`vmlinux.ctfa' file, even so, the compiler can emit the `.ctf'
section into the out-of-tree modules and it can be extracted
by the libabigail tools.
* src/abg-ctf-reader.cc (process_ctf_archive, read_corpus
slurp_elf_info): Avoid looking for `vmlinux.ctfa' when we aren't
processing a `cur_corpus_group_'. So CTF info is embedded in the
`.ko' file.
* tests/data/Makefile.am: Add test inputs and expected files.
* tests/data/test-read-ctf/test-linux-module.{ko,c,abi}: Add new
test input and reference kABI.
* tests/test-read-ctf.cc: Add new testcase.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 26 Aug 2022 13:14:37 +0000 (15:14 +0200)]
dwarf-reader: Better handle the absence of a die->parent map
When analysing the ld.so binary, the DWARF reader drops the
description of the rtld_global_ro global variable on the floor. This
is because it fails to get the decl that the rtld_global_ro variable
belongs to. So it doesn't know where the put the description of
rtld_global_ro, in the IR.
The root cause is that the DWARF partial_unit that contains the
description of the type of the rtld_global_ro variable doesn't contain
any description of the language its types originate from. The DWARF
reader needs to know the type, to know if it needs to construct a map
that associates each type/decl DIE to their parent DIE. This is
useful to determine which namespace a given DIE belongs to. But then
this is needed only for languages that support naming organising
devices like namespaces, namely, C++, Ada, Java etc. For C, the DWARF
reader knows that by default, all decls/types belong to the global
namespace. But then in this particular case, the partial_unit doesn't
carry any language information, so libabigail just drops the ball on
the floor, so there is no information for the rtld_global_ro variable.
The fix is thus to consider the absence of a DIE->PARENT map as
implying that all decls are defined in the global namespace.
* src/abg-dwarf-reader.cc (get_scope_for_die): Consider that in
the absence of the DIE->PARENT map, all decls are in the global.
namespace.
* tests/data/test-abidiff-exit/ld-2.28-210.so: New test.
* tests/data/test-abidiff-exit/ld-2.28-211.so: Likewise.
* tests/data/test-abidiff-exit/test-ld-2.28-210.so--ld-2.28-211.so.txt: New reference test output.
* tests/test-abidiff-exit.cc (in_out_specs): Add the test above to
the harness.
Dodji Seketeli [Mon, 29 Aug 2022 07:20:07 +0000 (09:20 +0200)]
Update test-read-ctf reference output
It seems like some recent changes have changed the reference expected
output of the test-read-ctf tests. I believe it's this change that is the culprit:
df28c220 writer: Make sorting referenced typedefs types stable in abixml
This patch updates the expected reference output accordingly.
ir: Consider integral types of same kind and size as equivalent
On some platforms, "long int" and "long long int" can have the same
size. In that case, we want those two types to be equivalent from ABI
standpoint. Otherwise, through the use of typedefs and pointers, two
structs "C" defined in different translation units where one uses
"long int" in a translation unit and "long long int" in another should
be considered ABI compatible if long int and long long int have the
same size on that platform.
That patch was doing several things, including fixing the parsing of
integral modifiers in parse_integral_type, fixing the sorting of types
for serialization purposes based on their pretty representation as
well as considering int types with the same size as equivalent,
independently from the short and long modifiers.
This patch just reverts the last item of the list above from the 'equals'
function that handle type_decls and updates the impacted tests
accordingly.
This is because there seems to be a consensus in the community about
the fact that libabigail should do as much as possible to detect API
incompatibilities when it's possible, rather than just focusing on ABI
incompatibilities. Point taken.
Please note that the following command will still fail now:
$ tools/fedabipkgdiff --debug --self-compare -a --from fc36 btrfs-progs
This is because the self-comparison check of the 'btrfs' program
from that package fails:
$ abidw --abidiff -d usr/lib/debug usr/sbin/btrfs
That issue will need to be addressed in a different manner, I guess.
* src/abg-ir.cc (equals): In the overload for type_decl, do not
consider int types of the same type as being equivalent by
overlooking their long and short modifiers.
* tests/data/test-annotate/libtest23.so.abi: Adjust.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-annotate/test0.abi: Likewise.
* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Likewise.
* tests/data/test-read-dwarf/libtest23.so.abi: Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi:
Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libaaudio.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/test0.abi: Likewise.
* tests/data/test-read-dwarf/test0.hash.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
* tests/data/test-read-write/test28-without-std-fns-ref.xml:
Likewise.
* tests/data/test-read-write/test28-without-std-vars-ref.xml:
Likewise.
writer: Make sorting referenced typedefs types stable in abixml
After the latest commits, it apears that sorting typedefs referenced
by by other types that are emitted in the abixml file is not stable
when the underlying types are integral types.
ir: Consider integral types of same kind and size as equivalent
This patch fixes that by using the non-internal pretty representation
(which disambiguates integral types) of types for sorting purposes.
* src/abg-writer.cc (read_context::type_ptr_cmp::operator()): In
the less-than operator the type pointer comparison functor, use
the non-internal pretty representation of types for sorting
purposes.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Adjust
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
ir: Consider integral types of same kind and size as equivalent
On some platforms, "long int" and "long long int" can have the same
size. In that case, we want those two types to be equivalent from ABI
standpoint. Otherwise, through the use of typedefs and pointers, two
structs "C" defined in different translation units where one uses
"long int" in a translation unit and "long long int" in another should
be considered ABI compatible if long int and long long int have the
same size on that platform.
Otherwise, that causes spurious type changes that lead to self
comparison change down the road. For instance, the following command
fails:
$ tools/fedabipkgdiff --debug --self-compare -a --from fc36 btrfs-progs
This patch thus changes the comparison engine of the IR so that the
"short, long and long long" modifiers don't change the result of
comparing integral types that share the same base type when they have
the same size.
* include/abg-fwd.h (is_integral_type): Declare new function.
* include/abg-ir.h (type_decl::get_qualified_name): Add a
declaration of an implementation of the virtual interface
get_qualified_name.
* src/abg-ir-priv.h (integral_type::set_modifiers): Define a new
setter.
(integral_type::to_string): Add an "internal" flag.
* src/abg-ir.cc (operator~, operator&=): Declare
new operators.
(get_internal_integral_type_name): Define new static function.
(decl_base::priv::{temporary_internal_qualified_name_,
internal_qualified_name_}): Define two new data members.
(get_type_name): For internal name of integral types, use the new
get_internal_integral_type_name function.
(is_integral_type): Define new function.
(integral_type::set_modifiers): Define new member function.
(operator|, operator&): Fix some indentation.
(operator~, operator&=): Define new operators.
(parse_integral_type): Fix the logic of this function. Namely, it
wasn't handling parsing "long long" correctly.
(integral_type::to_string): Add an "internal" flag.
(equals): In the overload for type_decl, do not take the short,
long and long long into account when comparing integral types of
the same size.
(type_decl::get_qualified_name): Define new method.
(type_decl::get_pretty_representation): For internal name of
integral types, use the new get_internal_integral_type_name
function.
({decl,type}_topo_comp::operator()): Use the non-internal pretty
representation of decls/types for sorting purpose.
* src/abg-reader.cc (build_type_decl): We don't expect the
integral type name from abixml to the same as the name of the
parsed integral type, as the abixml file can be old and have an
old format.
* tests/data/test-annotate/libtest23.so.abi: Adjust.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Adjust.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Adjust.
* tests/data/test-annotate/test0.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-dwarf/PR25058-liblttng-ctl-report-1.txt:
Adjust.
* tests/data/test-diff-filter/test41-report-0.txt: Adjust.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt:
Adjust.
* 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/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
Adjust.
* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
Adjust.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Adjust.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Adjust.
* tests/data/test-read-dwarf/libtest23.so.abi: Adjust.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi: Adjust.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Adjust.
* tests/data/test-read-dwarf/test-PR26568-1.o.abi: Adjust.
* tests/data/test-read-dwarf/test-PR26568-2.o.abi: Adjust.
* tests/data/test-read-dwarf/test-libaaudio.so.abi: Adjust.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Adjust.
* tests/data/test-read-dwarf/test0.abi: Adjust.
* tests/data/test-read-dwarf/test0.hash.abi: Adjust.
* tests/data/test-read-dwarf/test1.hash.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/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.
* tests/data/test-read-write/test22.xml: Adjust.
* tests/data/test-read-write/test23.xml: Adjust.
* tests/data/test-read-write/test28-without-std-fns-ref.xml: Adjust.
* tests/data/test-read-write/test28-without-std-vars-ref.xml: Adjust.
dwarf-reader: Remove redundant qualifiers from qualified types
While looking at something else, I noticed that there are some
qualified types built from the DWARF debug info that read:
const volatile const int m[5]
That's a tree of (chained) qualified types that end up having some
redundant qualifiers. That IR tree might look like:
[C] --> [C|V] --> [int]
We want to edit that IR tree to make it look like:
[C] --> [V] --> [int]
And that would serialize as
const volatile int m[5]
This patch introduces the editing of the qualified type IR tree to
remove the redundant qualifiers, right after the IR is built from
DWARF.
* include/abg-fwd.h (strip_redundant_quals_from_underyling_types):
Declare new function.
* include/abg-ir.h (operator&=): Declare new binary operator for
qualified_type_def::CV and ...
* src/abg-ir.cc (+operator&=): ... define it here.
(strip_redundant_quals_from_underyling_types): Define new function
and ...
* src/abg-dwarf-reader.cc (maybe_strip_qualification): ... Use it
here.
* tests/data/test-abidiff-exit/qualifier-typedef-array-report-1.txt:
Adjust.
When using non-internal pretty representation of array types (e.g, for
sorting of types in a given scope for the purpose of serialization),
some array element types might have the same name, even though they
don't have the same qualified name. In those cases, the serialized
abixml output is not stable.
This patches uses qualified names for array element names for type
sorting purposes.
However, this patch uncovers a problem that shows up in the tests
outputs for test-abidiff-exit and test-diff-filter, where emitting
qualified names of qualified types shows that there can be redundant
qualifiers in the serialized output. This issue will be fixed
separately in a later commit. For now, the output of these tests is
temporarily updated to have the tests pass.
* src/abg-ir.cc (get_type_representation): In the overload for
array_type_def, use qualified names for element types.
* tests/data/test-abidiff-exit/qualifier-typedef-array-report-1.txt: Adjust.
* tests/data/test-annotate/test15-pr18892.so.abi: Adjust.
* tests/data/test-annotate/test17-pr19027.so.abi: Adjust.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Adjust.
* tests/data/test-diff-filter/test-PR26739-2-report-0.txt: Adjust.
ir: Make pointers name stable wrt decl-only-ness of pointed-to types
When emitting the pretty representation of pointers from the IR, it
can so happen that the pointed type is a decl-only type which has been
resolved to a definition. In that case, the decl-only type might be
anonymous while the definition has a naming typedef, effectively
giving it a name. Then, using the decl-only type to construct the
name of the type might yield a different type name, more precisely,
the internal "anonymous" name of that type. This can lead to several
types having the same anonymous name, leading to instability with
respect to sorting.
The patch just looks through the decl-only pointed-to types before
using their name to construct the name of the pointer type. The patch
does that for reference types as well.
* src/abg-ir.cc (look_through_decl_only): New overload for
type_base.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.
[C] 'method virtual Wm5::ConvexHull<double>::~ConvexHull(int)' at
Wm5ConvexHull.h:24:1 has some indirect sub-type changes:
implicit parameter 0 of type 'Wm5::ConvexHull<double>*' has
sub-type changes:
in pointed to type 'class Wm5::ConvexHull<double>':
type size changed from 384 to 0 (in bits)
1 member function deletion:
'method virtual Wm5::ConvexHull<double>::~ConvexHull(int)'
at Wm5ConvexHull.cpp:32:1
no member function changes (2 filtered);
[C] 'method virtual Wm5::ConvexHull<float>::~ConvexHull(int)' at
Wm5ConvexHull.h:24:1 has some indirect sub-type changes:
implicit parameter 0 of type 'Wm5::ConvexHull<float>*' has
sub-type changes:
in pointed to type 'class Wm5::ConvexHull<float>':
type size changed from 320 to 0 (in bits)
1 member function deletion:
'method virtual Wm5::ConvexHull<float>::~ConvexHull(int)'
at Wm5ConvexHull.cpp:32:1
no member function changes (2 filtered);
[...]
It appears that some class type DIEs don't have any size information
and are marked as being declarations. These have an incomplete set of
data member / virtual member functions compared to DIEs for the SAME
types described elsewhere in the DWARF.
About these, the specification says:
5.7.1 Structure, Union and Class Type Entries
[...]
An incomplete structure, union or class type is represented by a
structure, union
or class entry that does not have a byte size attribute and that has a
DW_AT_declaration attribute.
But then the DWARF reader doesn't know about these so it creates
several types from these incomplete DIEs. Those types are thus
incorrect. And that leads to spurious self comparison changes down
the road.
This patch thus detects that the DIEs is for an incomplete type and
does not build an IR for its sub-types. It rather builds a
declaration-only type.
Then later, that declaration-only type is resolved to its
corresponding definition using the existing
read_context::resolve_declaration_only_classes function.
The patch also removes the temporary speed hack that was introduced
into read_context::resolve_declaration_only_classes() by the commit:
b9af1f35 dwarf-reader: Avoid long comparisons when resolving C++ decl-only classes
This is no longer a problem thanks to the great speed improvement we
have from the commit:
69795d1b Bug 29303 - Cache the result of structural aggregate comparison
While debugging a self comparison error, I realized that some types
where comparing different during IR type canonicalization for no good
reason. Looking deeper, I realized it's due to an anonymous enum
comparing different to the same anonymous enum defined elsewhere.
Both anonymous enums have naming typedefs. The discrepancy is due to
the fact that one of these enums didn't have its naming typedef
applied, by the DWARF reader. This is because the DWARF reader
"re-uses" typedef types whenever it can, and thus re-uses naming
typedefs too. Thinking deeply about this, I think naming typedefs
should not be re-used, otherwise, some anonymous types would lack
their naming typedefs. Oops.
So the patch avoids typedefs to be re-used altogether. There doesn't
seem to be any performance hit.
With that fixed, another problem came up: Some typedefs of type foo
are compared to said type foo and the comparison yields "false",
wrongly. This is in the context of a type 'foo' used by type 'bar'
defined in a translation unit, and the same type 'bar' uses a typedef
of 'foo' in another translation unit. Both types 'bar' should compare
equal, but don't, today. This is true especially for function types.
The patch strips typedefs off of the types of function parameters when
comparing function types. To make things consistent, typedefs are
stripped off of function parameters when function types (and decls)
are pretty printed for internal purposes, especially for type
canonicalization.
Bug 29302 - Don't edit fn linkage name when not appropriate
In this bug report, some function linkage names are edited even though
they are properly set in the DWARF. This was done because I was
thinking that those cases could exist because of the fact that a given
function with a linkage name could be associated to an *aliased* ELF
symbol that would have a different name.
But the proper way to represent that state of things is just to leave
the linkage name of the function as it is represented in the DWARF and
associate the function to the ELF aliased symbol.
This fixes the self comparison of the xerces-c package in Fedora 36:
$ time tools/fedabipkgdiff --debug --abipkgdiff build/tools/abipkgdiff --self-compare -a --from fc36 xerces-c
Fixed thus.
* src/abg-dwarf-reader.cc (build_function_decl): If the linkage
name is properly set in the DWARF, do not change it.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Adjust.
ctf-reader: add support to looks for debug information to extract kABI
With this patch, the abidiff tool is now able to looks for CTF debug
information to extract the kABI, it uses the standard
`--debug-info-dir' option to locate `vmlinux.ctfa`, looking at first
instance in the base directory where the ELF binary (vmlinux/module)
is want to be processed.
* src/abg-ctf-reader.cc (find_ctfa_file): Add new function
meant to locate the Linux Kernel debug information file
`vmlinux.ctfa'.
(ctf_reader::read_corpus): Use `find_ctfa_file' function.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This updates catch2 to the latest released version. That also drops a
workaround for missing MINSIGSTKSZ in newer glibcs that we introduced in 8ae8dcb8d5b8 ("tests: Update to catch.hpp v2.13.4 and fix #2178").
kmidiff: Add CTF support to comparing Kernel trees
This patch adds a new --ctf option to kmidiff to make it support CTF
type information when analysing Linux Kernel trees.
* doc/manuals/kmidiff.rst: Add documentation for the new --ctf option.
* tools/kmidiff.cc (options::use_ctf): Define new data member.
(display_usage): Add a help string for the --ctf option.
(main): Adjust call to pass
build_corpus_group_from_kernel_dist_under with origin being
corpus::CTF_ORIGIN when the user provides the --ctf option.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
dwarf-reader,ir: Don't canonicalize enums too early & too naively
When looking at several self comparison failures[1], I notice that the
DWARF reader was early-canonicalizing enum types. So, sometimes, when
there are declaration-only enum types that need to be resolved (later)
to their proper definition, by the time we reach
read_context::resolve_declaration_only_enums, canonicalization is
already done and so we fail to resolve the decl-only enum; in that
case, the decl-only enum is later wrongly considered as different from
its definition, leading to spurious errors down the road.
This patch thus delays canonicalizing of enum types from the DWARF
reader. Once that is done, the patch fixes the comparison of enum
types to look through decl-only enum types and compare their
definitions instead. The patch also look through decl-only enums
during canonicalization.
[1]: The self comparison failures could be reproduced by the commands:
$ tools/fedabipkgdiff --debug --abipkgdiff build/tools/abipkgdiff --self-compare -a --from fc36 dovecot
$ tools/fedabipkgdiff --debug --abipkgdiff build/tools/abipkgdiff --self-compare -a --from fc36 btrfs-progs
* src/abg-dwarf-reader.cc (maybe_canonicalize_type): Delay enum
type canonicalization.
* src/abg-ir.cc (type_base::get_canonical_type_for): Look through
all decl-only types, not just decl-only classes.
(equals): In the overload for enums, look through decl-only
enums. Also, fix redundant enumerators detection to make it more
robust, otherwise, some regression tests break.
Giuliano Procida [Mon, 13 Jun 2022 14:25:33 +0000 (15:25 +0100)]
add Linux kernel symbol namespace support
Bug 28954 - add Linux Kernel symbol namespace support
Each Linux kernel symbol can be exported to a specified named
namespace or left in the global (nameless) namespace.
One complexity is that the symbol values which identify a string in
the __ksymtab_strings section must be interpretated differently for
vmlinux and .ko loadable modules as the former has a fixed load
address but the latter are relocatable. For vmlinux, the section base
address needs to be subtracted to obtain a section-relative offset.
The global namespace is explicitly represented as the empty string, at
least when it comes to the value of __kstrtabns_FOO symbols, but the
common interpretation is that such symbols lack an export namespace.
I would rather not have to make use of "empty implies missing" in many
places, so the code here represents namespace as optional<string> and
only the symtab reader cares about empty strings in __ksymtab_strings.
* include/abg-ir.h (elf_symbol::elf_symbol): Add ns argument.
(elf_symbol::create): Add ns argument.
(elf_symbol::get_namespace): Declare new function.
(elf_symbol::set_namespace): Declare new function.
and set_namespace.
* src/abg-comp-filter.cc (namespace_changed): Define new
helper functions.
(categorize_harmful_diff_node): Also call namespace_changed().
* src/abg-ir.cc (elf_symbol::priv): Add namespace_ member.
(elf_symbol::priv::priv): Add namespace_ to initialisers.
(elf_symbol::elf_symbol): Take new ns argument and pass it to
priv constructor.
(elf_symbol::create): Take new ns argument and pass it to
elf_symbol constructor.
(elf_symbol::get_namespace): Define new function.
(elf_symbol::set_namespace): Define new function.
* src/abg-reader.cc (build_elf_symbol): If namespace
attribute is present, set symbol namespace.
* src/abg-reporter-priv.cc (maybe_report_diff_for_symbol): If
symbol namespaces differ, report this.
* src/abg-symtab-reader.cc (symtab::load): Get ELF header to
distinguish vmlinux from .ko. Try to get __ksymtab_strings
metadata and data. Use these to look up __kstrtabns_FOO
namespace entries. Set symbol namespace where found.
* src/abg-writer.cc (write_elf_symbol): Emit namespace
attribute, if symbol has a namespace.
* tests/data/Makefile.am: Add new test files.
* tests/data/test-abidiff/test-namespace-0.xml: New test file.
* tests/data/test-abidiff/test-namespace-1.xml: Likewise
* tests/data/test-abidiff/test-namespace-report.txt: Likewise.
* tests/test-abidiff.cc: Add new test case.
Giuliano Procida [Mon, 13 Jun 2022 14:25:32 +0000 (15:25 +0100)]
Linux symbol CRCs: support 0 and report presence changes
The CRC with value zero was used to mean "absent". This can be better
modelled using optional.
This commit makes this change and also tweaks reporting so that
disappearing / appearing CRCs are noted. This should be essentially
impossible unless CRCs are enabled / disabled altogether but would be
very noteworthy otherwise.
* include/abg-ir.h (elf_symbol::elf_symbol): Argument crc is
now an optional defaulted to absent.
(elf_symbol::create): Likewise.
(elf_symbol::get_crc): Now returns an optional uint64_t.
(elf_symbol::set_src): Now takes an optional uint64_t.
* src/abg-comp-filter.cc (crc_changed): Simplify comparison.
* src/abg-ir.cc (elf_symbol::priv): Member crc_ is now an
optional uint64_t.
(elf_symbol::priv::priv): Argument crc is now an optional
uint64_t.
(elf_symbol::elf_symbol): Likewise.
(elf_symbol::create): Argument crc is now an optional uint64_t
and defaults to absent.
(textually_equals): Simplify comparison.
(elf_symbol::get_crc): Now returns an optional uint64_t.
(elf_symbol::set_crc): Now takes an optional uint64_t.
* src/abg-reader.cc (build_elf_symbol): Treat CRC 0 the same
as other CRC values.
* src/abg-reporter-priv.cc (maybe_report_diff_for_symbol):
Treat CRC 0 the same as other CRC values and also report
changes to CRC presence.
* src/abg-writer.cc (write_elf_symbol): Treat CRC 0 the same
as other CRC values.
* tests/data/Makefile: Remove test-abidiff/test-crc-report.txt
and add test-abidiff/test-crc-report-{0-1,1-0,1-2}.txt.
* tests/data/test-abidiff/test-crc-report-0-1.txt: Report
showing additional of CRCs.
* tests/data/test-abidiff/test-crc-report-1-0.txt: Report
showing removal of CRCs.
* tests/data/test-abidiff/test-crc-report-1-2.txt: Renamed
from tests/data/test-abidiff/test-crc-report.txt.
* tests/test-abidiff.cc: Update test cases that no longer
generate empty reports.
* tests/test-symtab.cc: Update KernelSymtabsWithCRC test.
Giuliano Procida [Mon, 13 Jun 2022 14:25:30 +0000 (15:25 +0100)]
crc_changed: eliminate copying of shared_ptr values
As pointed out in a review of similar code, it is possible to avoid
copying a couple of shared pointers in this function, by taking
references instead.
This commit also splits declarations to one per line and removes the
unnecessary parentheses around the return expression.
* src/abg-comp-filter.cc (crc_changed): Take references to
avoid std::shared_ptr copying. Split declarations into one per
line. Remove unnecessary return expression parentheses.
Dodji Seketeli [Thu, 30 Jun 2022 11:14:20 +0000 (13:14 +0200)]
ir: Add some debugging facilities for the comparison machinery
When looking at something else, I felt the need to write the
debugging facilities below to dump the content of the set of types
being compared, to better understand why some comparison take more or
less time.
* src/abg-ir-priv.h: Include iostream to access std::cerr.
(environment::priv::{dump_classes_being_compared,
dump_fn_types_being_compared}): Define new member functions.
* src/abg-ir.cc (dump_classes_being_compared)
(dump_fn_types_being_compared): Define new functions.
Dodji Seketeli [Thu, 30 Jun 2022 10:33:33 +0000 (12:33 +0200)]
Bug 29303 - Cache the result of structural aggregate comparison
When we are forced to structurally compare aggregate types (classes
and function types), some sub-types pairs can be compared an
innumerable number of times over and over again. This leads to having
extremely slow comparison times for the portions of the code that are
subject to structural comparison because canonical comparison has not
yet been setup. For instance, this happens in the dwarf-reader in the
function read_context::resolve_declaration_only_classes or, in the ir
module in the function type_base::get_canonical_type_for when type
canonicalization is being done.
To overcome this, this patch caches the result of comparing two pairs
of aggregate (class or function) type, ensuring that a pair of
aggregate sub-type is compared at most once during the structural
comparison of a given type.
Note that this caching scheme is used only during declaration classes
resolution and type canonicalization.
This sped up things quite noticeably as self comparing both binutils
and dovecot package sets in Fedora 36 was literally taking forever
prior to the patch and is now completing with this patch.
* src/abg-ir-priv.h (struct uint64_t_pair_hash): Define new type.
(uint64_t_pair_type, uint64_t_pairs_set_type)
(type_comparison_result_type): Define new typedefs.
(environment::priv::{classes_being_compared_,
fn_types_being_compared_}): Use the new uint64_t_pairs_set_type
type for these.
(environment::priv::{type_comparison_results_cache_,
allow_type_comparison_results_caching_}): Define new data members.
(environment::priv::priv): Initialize the new
allow_type_comparison_results_caching_ scalar data member.
(environment::priv::{allow_type_comparison_results_caching,
cache_type_comparison_result, is_type_comparison_cached,
clear_type_comparison_results_cache}): Define new member
functions.
(environment::priv::{mark_as_being_compared,
unmark_as_being_compared, comparison_started}): Take a pair of
types.
(struct function_type::priv): Move this here, from ...
* src/abg-ir.cc (struct function_type::priv): ... here.
(is_comparison_cycle_detected, mark_types_as_being_compared)
(unmark_types_as_being_compared): Adjust call to the new
environment::priv::{comparison_started, mark_as_being_compared,
unmark_as_being_compared}.
(type_base::get_canonical_type_for): Use aggregate types
comparison result caching when doing type comparison.
(equals): In the overload for function_type, class_or_union and
class_decl, cache the result of aggregate type comparison and
re-use that cached result when it's available.
* src/abg-dwarf-reader.cc
(read_context::compare_before_canonicalisation): Use aggregate
types comparison result caching when doing type comparison.
Dodji Seketeli [Tue, 28 Jun 2022 10:38:12 +0000 (12:38 +0200)]
dwarf-reader: Don't consider top-level types as private
By default, the DWARF reader (wrongly) considers that any top-level
decl/type (inside a namespace) that doesn't have the DW_AT_external
attribute is a private decl/type.
It thus considers that function/variable decls that have that
attribute are public decls, and thus, the types they use are also
public. Which what we want.
But then, it also considers that top-level types (which never have a
DW_AT_external) are not public. And that causes unwanted side effects
like dropping some classes on the floor. As a result, some functions
that have those classes as parameter type would end-up being
considered as having no parameter of that type. Oops.
This patch fixes that by considering that only function and variable
DIEs ought to have the DW_AT_external flag to be considered as public
decls. Non-anonymous namespace are also considered as public decls.
This should fix the output of the command:
tools/fedabipkgdiff --debug --abipkgdiff build/tools/abipkgdiff --self-compare -a --from fc36 libabigail
* src/abg-dwarf-reader.cc (die_is_public_decl): Only function and
variable decls having the DW_AT_external, as well as non-anonymous
namespaces are considered public decls. The rest is considered
private.
(build_namespace_decl_and_add_to_ir): If the current namespace is
private (anonymous), then its content is also private. Otherwise,
use die_is_public_decl to know if its content is public or not.
Dodji Seketeli [Fri, 24 Jun 2022 14:29:16 +0000 (16:29 +0200)]
dwarf-reader: Avoid long comparisons when resolving C++ decl-only classes
When resolving decl-only classes, it can happen that a decl-only class
has several candidate classes definitions (in the same binary) to
resolve to.
In that case, read_context::resolve_declaration_only_classes compares
the classes definitions (of the same name) against each other. The
problem however is that at that point, we haven't done type
canonicalization yet. So comparing these types can have a quadratic
behaviour, i.e, take forever, especially in C++.
So, if we are looking at C++ (or any language where the ODR rules)
types, we can just do away with this comparison and assume the types
are equal, as they have the same name.
After canonicalization (which comes later), ODR violations can still
be detected (if we ware about that at all) anyway.
This patch does away with comparing aggregate types (struct/classes)
for languages that support the ODR.
In so doing, it avoids taking forever while loading the ABI corpus for
the libdyninstAPI.so.12 library from Fedora 36.
* src/abg-dwarf-reader.cc
(read_context::resolve_declaration_only_classes): Don't compare
same-name-types of a binary if the ODR is relevant.
* tests/data/test-annotate/test14-pr18893.so.abi: Adjust.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
Dodji Seketeli [Thu, 23 Jun 2022 13:53:56 +0000 (15:53 +0200)]
test-alt-dwarf: Add missing dwz alt-debug file
It seems I forgot to add a dwz alt-debug file to the test material
included in the previous commit.
Fixed thus.
* tests/data/test-alt-dwarf-file/libstdc++/libstdc++-report-1.txt:
New reference test output.
* tests/data/test-alt-dwarf-file/libstdc++/usr/lib/debug/.dwz/gcc-12.1.1-1.fc37.x86_64:
New dwz alt-debug info file.
* tests/data/Makefile.am: Add the test material above to source
distribution.
Dodji Seketeli [Thu, 23 Jun 2022 10:13:05 +0000 (12:13 +0200)]
ir: Make canonicalization stable wrt typedefs in fn return types
In the grand scheme of things, two function return types can be equal
modulo typedefs. Because those two function textual representations
are different, the two overall function types would end up having
different canonical types and thus, the two functions would be
considered as having different sub-types. The harmless change pass
would then kick in and flag that change as harmless. But then, "abidw
--abidiff" that is used for testing purposes would still be not happy.
This patch strips typedefs off of return types of function types when
the string representation is to be used for internal (e.g, type
canonicalization) purposes.
The fix for this change uncovered another issue:
When setting the naming typedefs for an (anonymous) C++ class, the
qualified name of the class was wrongly being set to the qualified
name of the typedef. Only the name of the class should be affected,
in essence. The qualified name would, ONLY as a result of the name
change, be adjusted.
This patch fixes those issues and adjusts the test suite accordingly.
* src/abg-ir.cc (get_function_type_name, get_method_type_name):
When the function type name is for internal purposes, strip
potential typedefs off.
(equal): In the overload for function_type, strip potential
typedefs off of return types before comparing them.
(decl_base::set_naming_typedef): Properly adjust the qualified
name of the type to which a naming typedef is being set.
* tests/data/test-alt-dwarf-file/libstdc++/libstdc++-report.txt:
New reference test output.
* tests/data/test-alt-dwarf-file/libstdc++/usr/lib/debug/usr/lib64/libstdc++.so.6.0.30-12.1.1-1.fc37.x86_64.debug:
New binary test input.
* tests/data/test-alt-dwarf-file/libstdc++/usr/lib64/libstdc++.so.6.0.30:
New binary test input.
* tests/data/Makefile.am: Add the new test material to source
distribution.
* tests/data/test-annotate/test15-pr18892.so.abi: Adjust.
* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.
* tests/data/test-read-dwarf/test-libaaudio.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
Dodji Seketeli [Tue, 21 Jun 2022 10:51:18 +0000 (12:51 +0200)]
reporter-priv: Passing a string parm by reference
While looking at something else, I noticed the
maybe_report_data_members_replaced_by_anon_dm function was passing a
string parameter by value. Pass this by reference.
* src/abg-reporter-priv.h
(maybe_report_data_members_replaced_by_anon_dm): Pass the string parm by ...
* src/abg-reporter-priv.cc
(maybe_report_data_members_replaced_by_anon_dm): ... reference.
This fix has been triggered by a fix posted by Thomas Schwinge in the
thread that started at https://sourceware.org/pipermail/libabigail/2022q1/004139.html.
Thomas rightfully notes that compare_dies_string_attribute_value is
wrong. In some cases, it wrongly considers only the first character
of the two strings to compare.
This patch fixes that and updates the regression tests accordingly.
The fix suppresses several spurious changes (in runtestdifffilter)
that were there and I never got the bottom of them. Now they are
gone.
Canonicalize DIEs w/o assuming ODR & handle typedefs transparently
When canonicalizing DIEs, if two type DIEs have the same textual
representation and we are looking at a C++ DIE, then we assume the two
DIEs are equivalent because of the "One Definition Rule" of C++.
There are binaries however where there seem to be type DIEs that are
slightly different and yet with the same textual representation. It
seems that this leads to some heisenbugs depending on which of the two
DIEs is "kept". That order would then depend on whatever the linker's
output is at a particular moment when linking the binary. Those
slight differences (i.e, modulo typedefs, for instance) are a pain to
reproduce across binaries that are re-linked. So I am removing this
optimization. As it doesn't seem to be so useful anymore, given all
the areas of the pipeline that go improved recently, speed-wise.
After doing that, it appeared the comparison engine was seeing more
DIE kinds. So rather than aborting when seeing an unexpected DIE
kind, we now assume the two DIEs we are seeing are different. They
are thus not canonicalized. That's not a problem because the IR level
type canonicalizer will pick that up and fully canonicalize the type
at that level.
So as that is in place, it appeared we were doing canonical DIE
propagation even in cases where we should not. Namely, when a subset
of the stack of aggregates being compared depends on a recursive (or
redundant) sub-type, no canonical type DIE propagation should be
performed. The patch thus detects that state of things and disables
the canonical type DIE propagation in that case.
Once this change in place, as the DIE canonicalizer started to look
into more types, the IR started to get more types that were deemed
equal to their decl-only counterpart before, and so were represented
those decl-only counterpart by virtue of the ODR.
That uncovered the long standing issue described below.
Canonicalizing typedefs is done today by considering that two typedefs
that have different names are different even if they have the same
underlying types.
That means we can have two types T and T' (in a binary) that are
equivalent modulo a sub-type that is a typedef with different names in
T and T', yet with equivalent underlying types. Today, libabigail
would consider T and T' different, even though they are equivalent
from an ABI standpoint.
This can lead to spurious changes that we have to handle later by
adding passes that would suppress those spurious changes.
This patch thus changes the structural comparison for typedefs by not
taking the typedef name into account anymore. Also, typedefs don't
carry canonical types anymore. That way, to compare typedefs,
libabigail always look into their underlying types.
To adapt the precision of the abixml output, the patch now uses a
special set (to track emitted non-canonicalized types) and a special
map (to track type ID for non-canonicalized types) as those can't be
put in the classical maps that track types using their canonical
types.
* include/abg-fwd.h (peel_typedef_pointer_or_reference_type):
Declare new function.
* src/abg-dwarf-reader.cc (read_context::{compute_canonical_die,
get_canonical_die, get_or_compute_canonical_die}): Don't
special-case ODR-relevant cases.
(is_canon_type_to_be_propagated_tag): Rename
is_canonicalizeable_type_tag into this.
(erase_offset_pair): Make this return a bool, not void.
(have_offset_pair_in_common, try_canonical_die_comparison)
(notify_die_comparison_failed): Define new static functions.
(ABG_RETURN, ABG_RETURN_FALSE): Define new macros.
(compare_dies): Handle DW_TAG_{class,unspecified}_type DIES. Also
if a comparing a DIE kind is not supported, assume the comparison
yields "false" for the purpose of canonicalization. Add a new
parameter for redundant aggregates being compared. Use the
redundant aggregate set to detect if a redundant aggregate has
been detected and is still being "explored"; if that's the case,
then canonical DIE propagation is de-activated. This might incur
a performance hit. Use the new ABG_RETURN and ABG_RETURN_FALSE
macros. Use the new try_canonical_die_comparison to compare
canonical DIEs rather than doing it by hand.
(build_typedef_type): Don't try to rely on canonical typedefs
DIEs.
* src/abg-ir.cc
(type_topo_comp::has_artificial_or_natural_location): Define new
method.
(type_topo_comp::operator(const type_base*, const type_base*)): In
this topological comparison operator of types, when two types have
the same textual representation, if they don't have any location,
if they are pointer/reference/typedef types, use the textual
representation of their ultimate underlying type for sorting
order.
(peel_typedef_pointer_or_reference_type): Define new function.
(compare_types_during_canonicalization): Perform the structural
canonical comparison first.
(equals): In the overload for array_type_def::subrange_type&,
remove unused code. In the overload for typedef_decl& don't
compare typedef names. In the overload for class_or_union, make
the RETURN macro *NOT* propagate canonical type because this equal
function is used as a subroutine by overloads for class and union.
That means that the class_or_union equal function can return true,
and still, the caller (overload for class_decl or union_decl) can
later return false; in that case, we don't want canonical type to
be propagated. In the overload for class_decl::base_spec, use the
ABG_RETURN macro when returning comparing decl-bases. In the
overload for class_decl, use ABG_RETURN when returning the result
of comparing class_or_union artifacts.
(maybe_propagate_canonical_type): If we are using canonical type
comparison, then do not propagate the type, if
WITH_DEBUG_TYPE_CANONICALIZATION is defined.
(is_non_canonicalized_type): Add typedefs to the set of
non-canonicalized types.
* src/abg-writer.cc (struct non_canonicalized_type_hash, struct
non_canonicalized_type_equal): Define new functors.
(typedef nc_type_ptr_set_type): Define a new non-canonicalized set
type.
(typedef nc_type_ptr_istr_map_type): Define a new map of
non-canonicalized types/interned string map.
(write_context::{m_nc_type_id_map,
m_emitted_non_canonicalized_type_set,
m_referenced_non_canonicalized_types_set}): New data members.
(write_context::type_has_existing_id): Look for non-canonicalized
types in the m_nc_type_id_map map.
(write_context::get_id_for_type): Use m_nc_type_id_map for
non-canonicalized types.
(write_context::clear_type_id_map): Clear m_nc_type_id_map too.
(write_context::get_referenced_non_canonicalized_types): Renamed
write_context::get_referenced_non_canonical_types into this. Make
it return the new
write_context::m_referenced_non_canonicalized_types_set.
(write_context::has_non_emitted_referenced_types): Just use
write_context::type_is_emitted as it knows where to look up for
non-canonicalized types.
(write_context::record_type_as_referenced): Use
m_referenced_non_canonicalized_types_set to store referenced
non-canonicalized types. The other types are stored in
m_emitted_set as previously.
(write_context::type_is_emitted): Look into
m_emitted_non_canonicalized_type_set for non-canonicalized types.
Other types are still in m_emitted_type_set.
(write_context::{record_decl_only_type_as_emitted,
decl_only_type_is_emitted, get_emitted_decl_only_types_set}):
Remove methods.
(write_context::get_emitted_non_canonicalized_type_set): Define
new method.
(write_context::clear_referenced_types): Clear
m_referenced_non_canonicalized_types_set too, as
m_referenced_non_canonical_types_set is no more.
(write_context::referenced_type_should_be_emitted): Use only
write_context::type_is_emitted as it knows how to check it all
now.
(write_referenced_types): Use the more compact ranged-base for
loops.
(write_translation_unit): Write the non-canonicalized types that
are not yet emitted.
(write_class_decl, write_union_decl): Adjust recording type as
emitted.
(write_canonical_type_ids): Adjust.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
* tests/data/test-annotate/libtest23.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-annotate/test-anonymous-members-0.o.abi: Likewise.
* tests/data/test-annotate/test0.abi: Likewise.
* tests/data/test-annotate/test1.abi: Likewise.
* tests/data/test-annotate/test13-pr18894.so.abi: Likewise.
* tests/data/test-annotate/test14-pr18893.so.abi: Adjust.
* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise.
* tests/data/test-annotate/test2.so.abi: Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-annotate/test3.so.abi: Likewise.
* tests/data/test-annotate/test5.o.abi: Likewise.
* tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0.abi:
Likewise.
* tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1.abi:
Likewise.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt: Likewise.
* tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt: Likewise.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt: Likewise.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt: Likewise.
* tests/data/test-diff-filter/test41-report-0.txt: Likewise.
* tests/data/test-diff-pkg/libICE-1.0.6-1.el6.x86_64.rpm--libICE-1.0.9-2.el7.x86_64:
Likewise.
* tests/data/test-diff-pkg/nmap-7.70-5.el8_testjcc.x86_64-self-check-report-0.txt:
Likewise
* tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.txt:
Likewise.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-0.txt:
Likewise.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Likewise.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt:
Likewise.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt:
Likewise.
* tests/data/test-diff-suppr/test39-opaque-type-report-0.txt:
Likewise.
* tests/data/test-read-ctf/test-ambiguous-struct-A.o.hash.abi:
Likewise.
* tests/data/test-read-ctf/test-conflicting-type-syms-a.o.hash.abi:
Likewise.
* tests/data/test-read-ctf/test-conflicting-type-syms-b.o.hash.abi:
Likewise.
* tests/data/test-read-ctf/test-functions-declaration.abi:
Likewise.
* tests/data/test-read-ctf/test1.so.abi: Likewise.
* tests/data/test-read-ctf/test1.so.hash.abi: Likewise.
* tests/data/test-read-ctf/test2.so.abi: Likewise.
* tests/data/test-read-ctf/test2.so.hash.abi: Likewise.
* tests/data/test-read-ctf/test5.o.abi: Likewise.
* tests/data/test-read-ctf/test7.o.abi: Likewise.
* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/PR24378-fn-is-not-scope.abi:
Likewise.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Likewise.
* tests/data/test-read-dwarf/PR26261/PR26261-exe.abi: Likewise.
* tests/data/test-read-dwarf/PR27700/test-PR27700.abi: Likewise.
* tests/data/test-read-dwarf/libtest23.so.abi: Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi:
Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-read-dwarf/test-PR26568-1.o.abi: Likewise.
* tests/data/test-read-dwarf/test-PR26568-2.o.abi: Likewise.
* tests/data/test-read-dwarf/test-libaaudio.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/test-suppressed-alias.o.abi:
Likewise.
* tests/data/test-read-dwarf/test0.abi: Likewise.
* tests/data/test-read-dwarf/test0.hash.abi: Likewise.
* tests/data/test-read-dwarf/test1.abi: Likewise.
* tests/data/test-read-dwarf/test1.hash.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-read-dwarf/test2.so.abi: Likewise.
* tests/data/test-read-dwarf/test2.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Likewise.
* tests/data/test-read-dwarf/test3-alias-1.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test3-alias-2.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test3-alias-3.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test3-alias-4.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test3.so.abi: Likewise.
* tests/data/test-read-dwarf/test3.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test5.o.abi: Likewise.
* tests/data/test-read-dwarf/test5.o.hash.abi: Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
* tests/data/test-read-write/test18.xml: Likewise.
* tests/data/test-read-write/test28-without-std-fns-ref.xml:
Likewise.
* tests/data/test-read-write/test28-without-std-vars-ref.xml:
Likewise.
Dodji Seketeli [Thu, 31 Mar 2022 16:31:01 +0000 (18:31 +0200)]
abidw, dwarf-reader: Add an option to debug DIE canonicalization
While looking at something else, it appeared that I needed an option
to turn on checks to debug DIE canonicalization. That option would
itself be enable only when the project is configured using the
--enable-debug-type-canonicalization configure option.
* include/abg-ir.h
(environment::debug_die_canonicalization_is_on): Declare new methods.
* src/abg-ir.cc (environment::debug_die_canonicalization_is_on):
Define them.
* src/abg-dwarf-reader.cc
(compare_dies_during_canonicalization): Define new static
function.
(read_context::{debug_die_canonicalization_is_on_,
use_canonical_die_comparison_}): Declare these data members.
(read_context::read_context): Initialize them.
(read_context::get_canonical_die): When ODR is considered, if DIE
canonicalization debugging is on, use the new
compare_dies_during_canonicalization for the type comparison to be
done both structurally and canonically; both comparison should
yield the same result. Also, make this method function non-const.
* src/abg-ir-priv.h
(environment::priv::debug_die_canonicalization_): Define new data
member.
(environment::priv::priv): Initialize it.
* tools/abidw.cc (options::debug_die_canonicalization_): Define
new data member.
(options::options): Initialize it.
(display_usage): Add a description for the --debug-dc option.
(parse_command): Parse the new --debug-dc option.
(load_corpus_and_write_abixml): Use the new
environment::debug_die_canonicalization_is_on.
Dodji Seketeli [Thu, 31 Mar 2022 13:24:12 +0000 (15:24 +0200)]
test-read-dwarf: Use abidw rather than using the library
When this test fails, sometimes it's difficult to reproduce the
problem using abidw. That's because this test is using the library
directly, so the exact setup is not always straightforward to
reproduce and eventually debug.
This test changes tests/test-read-dwarf.cc to make it use abidw rather
than using the library directly.
* tests/test-read-dwarf.cc (set_suppressions)
(set_suppressions_from_headers): Remove these static functions.
(test_task_dwarf::perform): Use abidw rather than using the
library. Remove the path and architecture from the abixml files.
Also, when the test fails, display the actual command that failed.
Dodji Seketeli [Thu, 31 Mar 2022 13:03:38 +0000 (15:03 +0200)]
Add better error messaging to several tests
When test-read-dwarf and test-diff-filter fail, it's important to know
what exact command line failed. This patch makes these tests display
the failing command.
Dodji Seketeli [Mon, 13 Jun 2022 13:45:01 +0000 (15:45 +0200)]
ir, test-read-ctf: Remove uncertainty in sorting anonymous types
For a reason, I am seeing changes in the test-read-ctf output. Those
are related to sorting anonymous structs that have no (internal) name.
The CTF reader emits such types.
This patch thus considers the flat representation of an anonymous
class/union when it doesn't have an internal name. The patch also
updates the test-read-ctf output tests accordingly.
* src/abg-ir.cc ({class_decl,
union_decl}::get_pretty_representation): If the anonymous class
has no internal name, use its flat representation as internal
representation.
* tests/data/test-read-ctf/test-PR26568-1.o.abi: Likewise.
* tests/data/test-read-ctf/test-PR26568-2.o.abi: Likewise.
* tests/data/test-read-ctf/test-anonymous-fields.o.abi: Likewise.
* tests/data/test-read-ctf/test-anonymous-fields.o.abi: Likewise.
* tests/data/test-read-ctf/test0.hash.abi: Likewise.
Exponential explosion of DIE comparison has been possible since the
limit of at most 5 pending struct/union DIE comparison pairs was
lifted.
This commit adds two things to control this (with a negligible chance of
falsely finding that two DIEs are equivalent when they are not).
- DIE self-comparisons immediately return true
- once a DIE pair has been compared 10000 times, always return true
* src/abg-dwarf-reader.cc (read_context): Add mutable
die_comparison_visits_ member.
(compare_dies): Return true if this is a self-comparison.
Return true if we have visited this comparison 10000 times.
Dodji Seketeli [Fri, 3 Jun 2022 14:52:51 +0000 (16:52 +0200)]
Bug 29144 - abidiff doesn't report base class re-ordering
Libabigail fails to report when a base class gets re-ordered, even
though the comparison engine detects the change.
This patch detects the base class re-ordering in
class_diff::ensure_lookup_tables_populated by interpreting the edit
script resulting of the comparison, populates a new
class_diff::priv::moved_bases_ vector with the bases that "changed
position" and adds a new class_diff::moved_bases() accessor for the "base
classes that changed position".
A new function maybe_report_base_class_reordering is defined to
actually report a base class re-ordering if class_diff::moved_bases()
returns a non-empty vector, meaning that class has some base classes
that got re-ordered.
That new function is thus called in the overload of
{default_reporter, leaf_reporter}::report() for class_diff to emit a
description of the change when necessary.
* include/abg-comparison.h (class_diff::moved_bases): Declare new
method.
* src/abg-comparison-priv.h (class_diff::priv::moved_bases_):
Define new data member.
* src/abg-reporter-priv.h (maybe_report_base_class_reordering):
Declare new function.
* src/abg-reporter-priv.cc (maybe_report_base_class_reordering):
Define new function.
* src/abg-comparison.cc
(class_diff::ensure_lookup_tables_populated): Detect that a base
class moved position and record it into
class_diff::priv::moved_bases_ data member.
* src/abg-default-reporter.cc (default_reporter::report): Use the
new maybe_report_base_class_reordering.
* src/abg-leaf-reporter.cc (leaf_reporter::report): Likewise.
* tests/data/test-abidiff-exit/test-PR29144-report-2.txt: New
testing input file.
* tests/data/test-abidiff-exit/test-PR29144-report.txt: Likewise.
* tests/data/test-abidiff-exit/test-PR29144-v0.cc: Likewise.
* tests/data/test-abidiff-exit/test-PR29144-v0.o: Likewise.
* tests/data/test-abidiff-exit/test-PR29144-v1.cc: Likewise.
* tests/data/test-abidiff-exit/test-PR29144-v1.o: Likewise.
* tests/data/Makefile.am: Add the new files to source
distribution.
* tests/test-abidiff-exit.cc (in_out_specs): Add the new tests to
this harness.
While abidw generates abidw and abidiff can consume that abidw
abicompat could only read from ELF objects with DWARF. To bring it in
line with the capabilities of abidiff, this patch adds the ability for
abicompat to read both abixml and CTF. The reason why being able to
handle abixml as well as an ELF file with DWARF is that it allows
someone to archive the abixml for an object and use it in place of
having to keep the entire object when doing abi compatibility studies.
The ability to handle CTF was also in the code that I copied over from
abidiff and so I brought it over at the same time. This should be
tested more extensively. The make check works and a trivial test using
the same object seems to work but I do not have any CTF other binaries
around to test against.
Also a feature that I asked for a long time ago was to fail if the
binaries don't have debug infomation which is needed for proper
comparison. This was added to abidiff but it didn't get added to
abicompat. Since the code was in the same area, I copied it over from
abidiff at the same time.
I do not think it makes sense to try to split these three features
apart because they are just replicating pre-existing code in a
different tool.
* tools/abicompat.cc (options::{fail_no_debug_info, use_ctf}): New
data members.
(options::options): Initialize them.
(display_usage): Add a help string for the new options
--fail-no-debug-info and --ctf.
(parse_command_line): Support parsing the new options
--fail-no-debug-info and --ctf.
(read_corpus): New static function.
(main): Use the new read_corpus in lieu of using
dwarf_reader::read_corpus_from_elf and/or ctf_reader::read_corpus
and/or xml::reader::read_corpus_from_input.
* doc/manuals/abicompat.rst: add documentation for new options.
Signed-off-by: Ben Woodard <woodard@redhat.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Ben Woodard [Fri, 3 Jun 2022 13:16:21 +0000 (06:16 -0700)]
abidiff: Remove redundant code
While working on abicompat, I noticed that the following two blocks of
code are redundant. They perform the same test as in the switch
statement a few lines below.
* tools/abidiff.cc (main): Remove redundant code.
Signed-off-by: Ben Woodard <woodard@redhat.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Empirically, the tracking of declaration-only type emission no longer
seems to make any difference. It is removed here.
* src/abg-writer.cc (write_context): Remove the
m_emitted_decl_only_set member.
(write_context::has_non_emitted_referenced_types): Remove the
calls to decl_only_type_is_emitted.
(write_context::record_decl_only_type_as_emitted): Removed.
(write_context::decl_only_type_is_emitted): Ditto.
(write_context::get_emitted_decl_only_types_set): Ditto.
(referenced_type_should_be_emitted): Remove the calls to
decl_only_type_is_emitted.
(write_class_decl): Just call record_type_as_emitted.
(write_union_decl): Ditto.
(write_enum_type_decl): Not changed, but now all 3 functions
have the same behaviour.
(write_canonical_type_ids): Remove the call to
get_emitted_decl_only_types_set.
Dodji Seketeli [Wed, 1 Jun 2022 11:55:44 +0000 (13:55 +0200)]
comparison: Fix leaf report summary
This patch is to address the problem reported at
https://sourceware.org/bugzilla/show_bug.cgi?id=29047 where the leaf
changes summary omits the number of (added/removed) ELF symbols that
have no debug info.
Fixed thus.
* src/abg-comparison.cc (corpus_diff::priv::emit_diff_stats): In
the "leaf change summary" section, add the number of removed/added
symbols not described by debug info.
Giuliano Procida [Mon, 30 May 2022 07:55:01 +0000 (08:55 +0100)]
test-annotate.cc: ignore whitespace during diff
The LLVM C++ demangler is being updated to remove the extra space
delimiter added between consecutive template closings. This change
ensures tests pass with both old and new versions.
Mark Wielaard [Mon, 16 May 2022 08:53:55 +0000 (10:53 +0200)]
symtab-reader: Setup aliases before checking ppc64 opd function entries
The update_function_entry_address_symbol_map function checks whether
the given symbol is an alias of another symbol or a special ppc64
ELFv1 function entry symbol. This requires the symbol aliases to
already been setup. But the alias entries were only setup after
calling update_function_entry_address_symbol_map. Make sure that the
symbol aliases have been setup and only then call the special ppc64
update_function_entry_address_symbol_map function. But make sure the
arm32 function symbol entry address cleanup is done before checking
for aliases.
This patch renames symtab::get_symbol_value into
symtab::setup_symbol_lookup_tables to better reflect what it does.
It is in that function that the call to
update_function_entry_address_symbol_map is performed, the arm32
symbol address cleanup is done and the ppc64 ELFv1 function address
plumbing is done.
So arranging for update_function_entry_address_symbol_map to be called
after symbol aliases are setup for ppc64 is also done in that
function. That way, symtab::load_ only have to call
symtab::setup_symbol_lookup_tables to have aliases setup.
This fixes runtestslowselfcompare.sh on ppc64 (ELFv1) with
ENABLE_SLOW_TEST=yes
* src/abg-elf-helpers.h (architecture_is_ppc32): Declare new
function.
* src/abg-elf-helpers.cc (architecture_is_ppc32): Define it.
* src/abg-symtab-reader.cc
(symtab::setup_symbol_lookup_tables): Rename
symtab::get_symbol_value into this. Setup symbol aliases before
setting up function entry address maps for ppc{32,64} ELFv1 and
after fixing up arm32/64 addresses.
(symtab::load_): Invoke the new setup_symbol_lookup_tables.
update_function_entry_address_symbol_map after setting up aliases.
No need to setup symbol aliases anymore.
(symtab::add_alternative_address_lookups): Invoke the new
setup_symbol_lookup_tables.
Signed-off-by: Mark Wielaard <mark@klomp.org> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
ctf-reader: add support to looking debug information in external path
When an ELF `stripped' file is used to get CTF debug information the ELF
symbols used by ctf reader (`symtab_reader::symtab') is split in a
separate file and even though CTF was designed to be in ELF file after
be `stripped' this .ctf section can be 'loaded' from and external
.debug file, for instance the script `find-debuginfo' used to generate
RPM debug packages split debug information in .debug files. The location
of such files is pass as a standard argument from libabigail tools and
the name of the file is gathering from the `.gnu_debuglink' section.
* include/abg-ctf-reader.h (ctf_reader::create_read_context):
Add `debug_info_root_paths' argument.
(ctf_reader::reset_read_context): Likewise.
* src/abg-ctf-reader.cc: Add `read_context::elf_{handler,fd}_dbg',
data members.
(read_context::read_context): Add new `debug_info_root_paths'
argument.
(read_context::initialize): Likewise.
(ctf_reader::create_read_context): Likewise.
(ctf_reader::close_elf_handler): Release
`read_context::elf_{handler,fd}_dbg' members.
(ctf_reader::find_alt_debuginfo): Add new function.
(ctf_reader::slurp_elf_info): Add new argument `status'. Use
`find_alt_debuginfo' and `elf_helpers::find_section_by_name'
to read the symtab and ctf information from an external .debug
file, the `status' reference is updated.
(ctf_reader::read_corpus): Verify `status' after `slurp_elf_info'.
(ctf_reader::reset_read_context): Add new `debug_info_root_path'
argument.
* src/abg-elf-helpers.cc (elf_helpers::find_section_by_name): Update
comment.
* src/abg-tools-utils.cc (maybe_load_vmlinux_ctf_corpus):
Adjust `ctf_reader::{create,reset}_read_context'.
* tests/test-read-ctf.cc: Likewise.
* tools/abidiff.cc (display_usage): Add `--ctf' command line
option.
(main): Adjust `ctf_reader::create_read_context'.
Likewise.
* tools/abidw.cc (load_corpus_and_write_abixml): Adjust
`ctf_reader::create_read_context'.
* tools/abilint.cc (main): Likewise.
* tools/abipkgdiff.cc (compare, compare_to_self): Likewise.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Ben Woodard [Mon, 16 May 2022 16:34:43 +0000 (09:34 -0700)]
Add github actions to support workflows
To facilitate collaboration with developers working on the BUILD-SI
project who are using github, I have been maintaining a clone of the
libabigail repo there. Having this repo also allows us to leverage
some of the tooling that github provides, in particular the ability to
have github actions test patches to verify that the builds succeed,
and all the regression tests run successfully. This will allow it to
better integrate with the normal agile workflow used by at least this
community of developers.
abipkgdiff: Add support to compare packages with CTF debug format
This patch add support in `abipkgdiff' to compare binaries with CTF
debug information inside of packages, when `--ctf' option is provided.
* tools/abipkgdiff.cc: Include `abg-ctf-reader.h'.
(options::use_ctf): Add new data member.
(display_usage): Add `--ctf' usage.
(compare): Add condition to use ctf-reader to extract
(parse_command_line): Set `options::use_ctf' when --ctf
option is provided.
and build CTF corpora when `options::use_ctf' is set.
(compare_to_self): Likewise.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
ctf-reader: CTF debug info for some symbols is not found
Running `abidw --ctf' tool for some executable ELF files, seems that ctf
reader is unable to get debug information for some symbols so, they will
not be neither the corpus nor in abixml file. This is happening because
internally the reader uses `ctf_arc_bufopen' expecting as argument the
sections: symbol table (.dynsym) and string table (.dynstr) for ELF
types: ET_{EXEC,DYN}, currently those sections are read by `elf_helpers'
but it uses .symtab when it is present and `symtab_shdr->sh_link' to
get .strtab, instead.
* src/abg-ctf-reader.cc (read_context::is_elf_exec): Remove
data member.
(ctf_reader::process_ctf_qualified_type): Add condition to avoid
use qualifier in functions.
(ctf_reader::process_ctf_archive): Use `ctf_lookup_by_symbol_name'
to find debug information when `ctf_lookup_variable' was not
lucky iff `corpus::CTF_ORIGIN'.
(ctf_reader::slurp_elf_info): Use `elf_helpers::find_section_by_name'
to read .dynsym and .dynstr when {EXEC,DYN}.
* src/abg-elf-helpers.h (elf_helpers::find_section_by_name): Add new
declaration.
* src/abg-elf-helpers.cc (elf_helpers::find_section_by_name): Add new
definition.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
During corpus comparison with ctf support, summary changes for functions
and variables are shown like to the binaries doesn't have ctf debug
information, even though they were compiled with '-gctf' compiler
modifier, e.g:
2 Removed function symbols not referenced by debug info:
ctf-reader: Add support to read CTF information from the Linux Kernel
This patch is meant to extract ABI information from the CTF data
stored in the Linux kernel build directory. It depends on the
vmlinux.ctfa archive file.
In order to generate the CTF information, the Linux Kernel build
system must support the 'make ctf' command, which causes the compiler
to be run with -gctf, thus emitting the CTF information for the
Kernel.
The target 'ctf' in the Linux Makefile generates a 'vmlinux.ctfa' file
that will be used by the ctf reader in libabigail. The 'vmlinux.ctfa'
archive has multiple 'ctf dictionaries' called "CTF archive members".
There is one CTF archive member for built-in kernel modules (in
`vmlinux') and one for each out-of-tree kernel module organized in a
parent-child hierarchy.
There is also a CTF archive member called `shared_ctf' which is a
parent dictionary containing shared symbols and CTF types used by more
than one kernel object. These common types are stored in 'types_map'
in the ctf reader, ignoring the ctf dictionary name. The CTF API has
the machinery for looking for a shared type in the parent dictionary
referred to in a given child dictionary. This CTF layout can be dumped
by using the objdump tool.
Due to the fact that the _same_ ctf archive is used to build the
vmlinux corpus the corpora of the kernel module (which, by the way,
all belong to the same corpus group), the high number of open/close on
the CTF archive is very time consuming during the ctf extraction.
So, the performance is improved up to 300% (from ~2m:50s to ~50s) by
keeping the ctf archive open for a given group, and thus, by using the
same ctf_archive_t pointer while building all the various corpora.
We just invoke `reset_read_context' for each new corpus. Note that
the `read_context::ctfa` data member should be updated if the
corpus::origin data member is set to `LINUX_KERNEL_BINARY_ORIGIN' and
the file to be process is not 'vmlinux'.
Note that `ctf_close' must be called after processing all group's
members so it is executed from the destructor of `reader_context'.
The basic algorithm used to generate the Linux corpus is the
following:
1. Looking for: vmlinux, *.ko objects, and vmlinux.ctfa files. The
first files are used to extract the ELF symbols, and the last one
contains the CTF type information for non-static variables and
functions symbols.
2. `process_ctf_archive' iterates on public symbols for vmlinux and
its modules, using the name of the symbol, ctf reader search for CTF
information in its dictionary, if the information was found it
builds a `var_decl' or `function_decl' depending of `ctf_type_kind'
result.
This algorithm is also applied to ELF files (exec, dyn, rel), so
instead of iterating on all ctf_types it just loops on the public
symbols.
* abg-elf-reader-common.h: Include ctf-api.h file.
(read_and_add_corpus_to_group_from_elf, set_read_context_corpus_group)
(reset_read_context, dic_type_key): Declare new member functions.
* include/abg-ir.cc (types_defined_same_linux_kernel_corpus_public): Use
bitwise to know the corpus `origin'.
* src/abg-ctf-reader.cc: Include map, algorithms header files.
(read_context::type_map): Change from unordered_map to std::map storing
ctf dictionary name as part of the key.
(read_context::is_elf_exec): Add new member variable.
(read_context::{cur_corpus_, cur_corpus_group_}): Likewise.
(read_context::unknown_types_set): Likewise.
(read_context::{current_corpus_group, main_corpus_from_current_group,
has_corpus_group, current_corpus_is_main_corpus_from_current_group,
should_reuse_type_from_corpus_group}): Add new member functions.
(read_context::{add_unknown_type, lookup_unknown_type, initialize}):
Likewise.
(read_context::{add_type, lookup_type}): Add new `ctf_dict_t' type
argument.
(ctf_reader::{process_ctf_typedef, process_ctf_base_type,
process_ctf_function_type, process_ctf_forward_type,
process_ctf_struct_type, process_ctf_union_type, process_ctf_array_type,
process_ctf_qualified_type, process_ctf_enum_type}): Add code to `reuse'
types already registered in main corpus `should_reuse_type_from_corpus_group'.
Use new `lookup_type' and `add_type' operations on `read_context::types_map'.
Replace function calls to the new ctf interface. Add verifier to not build
types duplicated by recursive calling chain.
(ctf_reader::process_ctf_type): Add code to return immediately if the
ctf type is unknown. Add unknown types to `unknown_types_set'.
(ctf_reader::process_ctf_archive): Change comment.
Add code to iterate over global symbols, searching by symbol name in the
ctf dictionary using `ctf_lookup_{variable,by_symbol_name}' depending of
the ELF file type and corpus type, creating a `{var,fuc}_decl' using the
return type of `ctf_type_kind'. Also close the ctf dict and call
`canonicalize_all_types'.
(slurp_elf_info): Set `is_elf_exec' depending of ELF type. Also return
success if corpus origin is Linux and symbol table was read.
(ctf_reader::read_corpus): Add current corpus. Set corpus origin to
`LINUX_KERNEL_BINARY_ORIGIN' if `is_linux_kernel' returns true. Verify
the ctf reader status, now the ctf archive is 'opened' using
`ctf_arc{open,bufopen}' depending if the corpus origin has
`corpus::LINUX_KERNEL_BINARY_ORIGIN' bit set. Use
`sort_{function,variables}' calls after extract ctf information.
`ctf_close' is called from `read_context' destructor.
(read:context::{set_read_context_corpus_group, reset_read_context,
read_and_add_corpus_to_group_from_elf, dic_type_key): Add new member
function implementation.
* include/abg-tools-utils.h (build_corpus_group_from_kernel_dist_under):
Add `origin' parameter with default `corpus::DWARF_ORIGIN'.
* src/abg-tools-utils.cc: Use `abg-ctf-reader.h' file.
(maybe_load_vmlinux_dwarf_corpus): Add new function.
(maybe_load_vmlinux_ctf_corpus): Likewise.
(build_corpus_group_from_kernel_dist_under): Update comments.
Add new `origin' argument. Use `maybe_load_vmlinux_dwarf_corpus'
or `maybe_load_vmlinux_ctf_corpus' according to `origin' value.
* src/abg-corpus.h (corpus::origin): Update `origin' type
values in enum.
* src/abg-corpus-priv.h (corpus::priv): Replace `origin' type
from `corpus::origin' to `uint32_t'.
* src/abg-corpus.cc (corpus::{get,set}_origin): Replace data
type from `corpus::origin' to `uint32_t'.
* tools/abidw.cc (main): Use of --ctf argument to set format debug.
* tests/test-read-ctf.cc: Add new tests to harness.
* tests/data/test-read-ctf/test-PR27700.abi: New test expected
result.
* tests/data/test-read-ctf/test-anonymous-fields.o.abi: Likewise.
* tests/data/test-read-ctf/test-enum-many-ctf.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-enum-many.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-enum-symbol-ctf.o.hash.abi: Likewise.
* tests/data/test-read-common/test-PR26568-2.o: Adjust.
* tests/data/test-read-ctf/test-PR26568-1.o.abi: Likewise.
* tests/data/test-read-ctf/test-PR26568-2.o.abi: Likewise.
* tests/data/test-read-ctf/test-ambiguous-struct-A.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-ambiguous-struct-B.c: Likewise.
* tests/data/test-read-ctf/test-ambiguous-struct-B.o: Likewise.
* tests/data/test-read-ctf/test-ambiguous-struct-B.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-array-of-pointers.abi: Likewise.
* tests/data/test-read-ctf/test-callback.abi: Likewise.
* tests/data/test-read-ctf/test-callback2.abi: Likewise.
* tests/data/test-read-ctf/test-conflicting-type-syms-a.o.hash.abi:
Likewise.
* tests/data/test-read-ctf/test-conflicting-type-syms-b.o.hash.abi:
Likewise.
* tests/data/test-read-ctf/test-dynamic-array.o.abi: Likewise.
* tests/data/test-read-ctf/test-enum-ctf.o.abi: Likewise.
* tests/data/test-read-ctf/test-enum-symbol.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-enum.o.abi: Likewise.
* tests/data/test-read-ctf/test-forward-type-decl.abi: Likewise.
* tests/data/test-read-ctf/test-functions-declaration.abi: Likewise.
* tests/data/test-read-ctf/test-list-struct.abi: Likewise.
* tests/data/test-read-ctf/test0: Likewise.
* tests/data/test-read-ctf/test0.abi: Likewise.
* tests/data/test-read-ctf/test0.c: Likewise.
* tests/data/test-read-ctf/test0.hash.abi: Likewise.
* tests/data/test-read-ctf/test1.so.abi: Likewise.
* tests/data/test-read-ctf/test1.so.hash.abi: Likewise.
* tests/data/test-read-ctf/test2.so.abi: Likewise.
* tests/data/test-read-ctf/test2.so.hash.abi: Likewise.
* tests/data/test-read-ctf/test3.so.abi: Likewise.
* tests/data/test-read-ctf/test3.so.hash.abi: Likewise.
* tests/data/test-read-ctf/test4.so.abi: Likewise.
* tests/data/test-read-ctf/test4.so.hash.abi: Likewise.
* tests/data/test-read-ctf/test5.o.abi: Likewise.
* tests/data/test-read-ctf/test7.o.abi: Likewise.
* tests/data/test-read-ctf/test8.o.abi: Likewise.
* tests/data/test-read-ctf/test9.o.abi: Likewise.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
vsoch [Wed, 4 May 2022 19:51:38 +0000 (12:51 -0700)]
Add Logic to detect file type by extension
Fedabipkgdiff uses mimetypes to detect what file type it is looking
at. In some minimal versions of the OS, in particular container
images, the package that includes all the mimetypes may not be
installed. This allows fedabipkgdiff to fall back to using the
extension.
* tools/fedabipkgdiff - add logic to detect file type by extension
Ben Woodard [Wed, 4 May 2022 17:42:29 +0000 (10:42 -0700)]
Add an option ignore SONAME differences in libraries
There are rare use cases where we do not want to compare the SONAME when
testing libraries for compatiblity or diffing libraries. This adds an
option to ignore the SONAME when doing the comparison. In these cases,
we will edit the application's DT_NEEDED to point to the other library.
This reuses the show_soname_change() function and slightly changes its
meaning to not only control if the sonames are printed but also if
they are compared. There didn't seem to be any other users of this
function and slight semantic change seemed harmless.
* doc/manuals/abicompat.rst - added new option
* doc/manuals/abidiff.rst - added new option to manpage
* src/abg-comparison.cc (compute_diff): don't bother comparing the
sonames if you aren't going to print them.
* tools/abicompat.cc (options::ignore_soname): Add new data
member.
(parse_command_line): Support the new --ignore-soname command line
option.
(display_usage): Add a description string for the new
--ignore-soname command line option.
(create_diff_context): Set the diff_context::show_soname_change
from the new options::ignore_soname data member.
* tools/abidiff.cc (options::ignore_soname): Add new data member.
(display_usage): Add a description string for the new
--ignore-soname command line option.
(parse_command_line): Support the new --ignore-soname command line
option.
(set_diff_context_from_opts): Set the
diff_context::show_soname_change from the new
options::ignore_soname.
Signed-off-by: Ben Woodard <woodard@redhat.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
symtab: fix up 64-bit ARM address which may contain tags
64-bit ARM addresses normally have bits 47 to 63 as either all 0 or
all 1. If tagging is used, bits 56 to 63 can vary, but the
interpretation of such values is as if the bits were all the same as
bit 55.
Such tagging is used for HWASAN and this affects the ELF symbol values
seen in shared libraries.
This commit changes the interpretation of 64-bit ARM symbol values by
unconditionally extending bit 55 into bits 56 to 63.
This fixes missing types for symbols in HWASAN-compiled libraries.
* src/abg-elf-helpers.cc: (architecture_is_arm64): Add helper.
* src/abg-elf-helpers.h: Likewise.
* src/abg-symtab-reader.cc: (get_symbol_value): Adjust 64-bit
ARM symbol values by extending bit 55 into bits 56 to 63.
A previous changes duplicated some logic for tweaking ELF symbol
values (and possibly updating some bookkeeping information).
This change refactors the code so the logic is in one place, in
symtab::get_symbol_value.
* src/abg-symtab-reader.cc (symtab::load_): Replace address
tweaking logic with a call to get_symbol_value.
(symtab::add_alternative_address_lookups): Likewise.
(symtab::get_symbol_value): New function containing address
tweaking logic for PPC and ARM.
ctf-reader: Fix multiple var-decl in anonymous struct/uninons
This patch avoids multiple inclusion of `var-decl' node for the same
field in anonymous struct/union declaration in the abixml, e.g:
struct uprobe_task {
union {
struct {
unsigned long vaddr;
};
struct {
int dup_xol_work;
};
};
};
Three `var-decl' nodes are written in abixml file, expected a single
one:
<var-decl name='dup_xol_work' .../>
* src/abg-ctf-reader.cc (process_ctf_sou_members): Remove
CTF_MN_RECURSE flag.
* tests/data/Makefile.am: Add new input test files.
* tests/data/test-read-ctf/test-anonymous-fields.c: New
test file.
* tests/data/test-read-ctf/test-anonymous-fields.o: New
expected test output.
* tests/data/test-read-ctf/test-anonymous-fields.o.abi:
Likewise.
* tests/test-read-ctf.cc: Add new test.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
tools-utils: `entry_of_file_with_name' returns incorrect result
entry_of_file_with_name uses `string_ends_with' to test if the
filename given in the `fts_path' member of `FTSENT' matches the
`fname' argument. This result is not correct when in the current
directory there are file with names: "./rmdir-xyx", "./dir-xyz" it
returns true for both files, this patch fixes this ambiguity by using
`basename' instead of `string_ends_with'.
* src/abg-tools-utils.cc (entry_of_file_with_name): Replace
call `string_ends_with' by `basename'.
* src/abg-ctf-reader.cc (process_ctf_forward_type): New
function.
(process_ctf_type): New CTF_K_FORWARD case.
* tests/data/test-read-ctf/test-forward-undefine-type-decl.c:
New testcase.
* tests/data/test-read-ctf/test-forward-undefine-type-decl.abi:
New expected result.
* tests/data/test-read-ctf/test-forward-undefine-type-decl.o
New test input.
* tests/test-read-ctf.cc: Add new testcase to test harness.
* tests/data/Makefile.am: Add new test input files to test harness.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 7 Mar 2022 16:55:05 +0000 (17:55 +0100)]
comparison: Avoid sorting diff nodes with wrong criteria
This should address PR28939, reported at
https://sourceware.org/bugzilla/show_bug.cgi?id=28939
In function_type_diff::chain_into_hierarchy, the diff details
represented by the diff nodes of the function parameters are already
sorted in the vector priv->sorted_changed_parms_by_id_. Note that the
sorting of function parameter diff nodes was done using a criterion
that takes into account the position of each function parameter.
Members of the vector priv->sorted_changed_parms_by_id_ are then added
to the diff graph using diff::append_child_node.
The problem is that diff::append_child_node sorts the children nodes
/again/, this time using a criterion that is different from the one
used in function_type_diff::chain_into_hierarchy. This is wrong.
This patch prevents diff::append_child_node from sorting the children
node because they have been sorted already.
* src/abg-comparison.cc (diff::append_child_node): Do not sort
children nodes here because they must have been sorted already.
* 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/test30-pr18904-rvalueref-report2.txt: Likewise.
* tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-0.txt: Likewise.
* tests/data/test-diff-filter/test41-report-0.txt: Likewise.
* tests/data/test-diff-pkg/libICE-1.0.6-1.el6.x86_64.rpm--libICE-1.0.9-2.el7.x86_64.rpm-report-0.txt:
Likewise.
Dodji Seketeli [Mon, 7 Mar 2022 16:29:13 +0000 (17:29 +0100)]
comparison: Factorize the code that inserts diff nodes to the graph
During the traversal of the graph of diff nodes, diff::traverse does
two things:
1/ If the generic view of the diff node of is not yet connected to
the diff graph, then connect it. Note that the typed view of the
diff node is always connected to the diff graph.
2/ Visit the diff node using its generic view and visit its
children nodes.
Looking at the part 1/, I realized that the code connecting the
generic view of the diff node to the diff graph was duplicated in
every single type of diff node.
This patch put that code into diff::finish_diff_type and makes all the
different kinds diff node use that (virtual) member function,
effectively factorizing that code.
* include/abg-comparison.h ({distinct, var, pointer, reference,
array, qualified_type, enum, class_or_union, class, union, base,
scope, fn_parm, function_type, function_decl, type_decl,
typedef}_diff::finish_diff_type): Remove these declarations.
* src/abg-comparison.cc ({distinct, var, pointer, reference,
array, qualified_type, enum, class_or_union, class, union, base,
scope, fn_parm, function_type, function_decl, type_decl,
typedef}_diff::finish_diff_type): Remove these definitions.
(diff::finish_diff_type): Add the code to connect the children
nodes to the current node, making the generic view of the diff
node usable to walk the graph.
Dodji Seketeli [Mon, 7 Mar 2022 16:18:48 +0000 (17:18 +0100)]
comparison: Describe the "generic view" of a diff node
To add a node to the diff graph, what is used is the "generic view"
(or generic API) of the diff node.
To understand the difference between the generic view and the typed
view of diff nodes, this patch adds comments to the diff node API.
* include/abg-comparison.h (class diff): Add comments to this class.
(diff::chain_into_hierarchy): Add comment to this method.
* src/abg-comparison-priv.h (diff::priv): Add comment to this class.
* src/abg-comparison.cc (diff::finish_diff_type): Add comment to
this method.
(diff::traverse): Add comment.
Dodji Seketeli [Thu, 3 Mar 2022 10:45:17 +0000 (11:45 +0100)]
Bug 28013 - Acknowledge variadic parameter type is not canonicalized
Variadic parameter types are one of the rare types that should not be
canonicalized, just like the void type. These types are created by
the system (compiler/libabigail) and thus can be compared in O(1)
time. So far, I forgot to prevent the canonicalization of variadic
parameter type everywhere, as should have been the case since the
recent introduction of the is_non_canonicalized_type function. This
patch fixes that.
Dodji Seketeli [Thu, 24 Feb 2022 09:49:43 +0000 (10:49 +0100)]
dwarf-reader: Don't propagate canonical type upon aggregate redundancy
This comes from trying to fix
https://sourceware.org/bugzilla/show_bug.cgi?id=26646.
During DIE comparison for the purpose of DIE canonicalization, we need
to detect a loop due to a recurring aggregate comparison. Thus, the
compare_dies function returns true in when it detects that it's
comparing two aggregate that are already being compared. In that
situation of "detected aggregate redundancy", even though the
comparison seemingly succeeds, no canonical type propagation should
happen.
This patch prevents canonical type propagation when compare_dies
return true to signal aggregate redundancy detection.
This addresses https://sourceware.org/bugzilla/show_bug.cgi?id=26646#c21.
* src/abg-dwarf-reader.cc (compare_dies): Do not propagate
canonical type when aggregate redundancy is detected.
This patch addresses the comment
https://sourceware.org/bugzilla/show_bug.cgi?id=26646#c15 of the
reported problem.
The core of the problem seems to be that when compare_dies compares
two DW_TAG_{structure,union}_type, it "gives up" when the comparison
stack contains more than 5 such DIEs being compared. Giving up means
that it considers the two DIEs to be equivalent if they have the same
size and kind, basically. This is to avoid infinite loops or too long
loops that we've seen with artificial DWARF input generated by
fuzzers.
This patch fixes that by better using the "aggregates_being_compared"
data structure that is meant to prevent looping infinitely over
aggregate DIEs that might be recursively defined. That data structure
now contains the DWARF offset pairs of the DIEs being compared, rather
than their "string representation". Lookups in that data structure
should now be faster and more precise.
The artificial limit of the comparison stack not having more than 5
DIEs is now lifted.
* src/abg-dwarf-reader.cc (struct dwarf_offset_pair_hash)
(dwarf_offset_pair_set_type): Define new type.
(die_offset, has_offset_pair, insert_offset_pair)
(erase_offset_pair): Define new static helper functions.
(compare_dies): Use a set of DWARF offsets for the
'aggregates_being_compared' data structure, rather than a set of
string representation of the DIEs. Always look at the size of the
types being compared first so that we can get out quickly if they
differ. For DIEs of DW_TAG_{union,struct}_type kind, don't limit
the depth of the stack of DIEs being compared to 5; so we don't
consider two types as being equal just because the depth of the
stack being compared is 5 and the two types have the same size and
are equal. Hopefully things don't take too long.
Dodji Seketeli [Mon, 28 Feb 2022 16:13:47 +0000 (17:13 +0100)]
abilint --show-type-use: Show results for global decls that have no symbols
In some abixml file, there can be global decls that don't have ELF
symbols. We still want to see how those decls use the type that is
being used, as analyzed by abilint --show-type-use <type-id>.
* include/abg-fwd.h (is_at_global_scope): Declare ...
* src/abg-ir.cc (is_at_global_scope): ... new overload.
* tools/abilint.cc (emit_artifact_use_trace): Emit the trace also
when the decl is at global scope or has a linkage name.
Dodji Seketeli [Thu, 17 Feb 2022 11:22:25 +0000 (12:22 +0100)]
abilint: add the --show-type-use option
"abilint --show-type-use <type-id> <abixml-file>" is a facility that
shows how a type defined in an abixml file is used. That is, it emits
a textual representation of how the use a type is used up until the
function or global variable that constitutes an entry point in the API
corpus. Here is an example of its use:
test-read-write$ abilint --noout --show-type-use type-id-5 test17.xml
Type ID 'type-id-5' is for type 'enum E'
The usage graph for that type is:
| -> enum E -> E S::m2 -> class S -> S* -> method void S::S()
| -> enum E -> E S::m2 -> class S -> S* -> method void S::__base_ctor ()
| -> enum E -> E S::m2 -> class S -> S* -> method void S::__comp_ctor ()
| -> enum E -> E S::m2 -> class S -> S* -> method void S::S(S&)
| -> enum E -> E S::m2 -> class S -> S* -> S& -> method void S::S(S&)
| -> enum E -> E S::m2 -> class S -> S* -> S& -> S var
| -> enum E -> E S::m2 -> class S -> S* -> S& -> E S::m2 -> class S -> S* -> method void S::S()
| -> enum E -> E S::m2 -> class S -> S* -> S& -> E S::m2 -> class S -> S* -> method void S::__base_ctor ()
| -> enum E -> E S::m2 -> class S -> S* -> S& -> E S::m2 -> class S -> S* -> method void S::__comp_ctor ()
| -> enum E -> E S::m2 -> class S -> S* -> S& -> E S::m2 -> class S -> S* -> method void S::S(S&)
| -> enum E -> E S::m2 -> class S -> S* -> S& -> E S::m2 -> class S -> S* -> S& -> method void S::S(S&)
| -> enum E -> E S::m2 -> class S -> S* -> S& -> E S::m2 -> class S -> S* -> S& -> S var
$
The screenshot above should be self explanatory.
This facility is useful to analyse type usage and find potential
issues in how libabigail represents some types.
To activate this feature, one needs to configure the package with the
configure option "--enable-show-type-use-in-abilint".
* configure.ac: Define the --enable-show-type-use-in-abilint
configure option. It defines the WITH_SHOW_TYPE_USE_IN_ABILINT
macro.
* include/abg-reader.h (read_translation_unit): Add an overload
that takes the read context.
(get_types_from_type_id, get_artifact_used_by_relation_map):
Declare new functions.
* src/abg-reader.cc (get_types_from_type_id)
(get_artifact_used_by_relation_map): Declare these functions as
friend of the read_context type.
(read_context::m_artifact_used_by_map):
(read_context::key_type_decl): Replace the shared_ptr<type_base>
type of the first parm by the equivalent type_base_sptr type.
(read_context::{record_artifact_as_used_by,
record_artifacts_as_used_in_fn_decl,
record_artifacts_as_used_in_fn_type}): Add new methods guarded by
the WITH_SHOW_TYPE_USE_IN_ABILINT macro.
(get_types_from_type_id, get_artifact_used_by_relation_map): Define
new functions guarded by the WITH_SHOW_TYPE_USE_IN_ABILINT macro.
(read_translation_unit): Define new overload.
(RECORD_ARTIFACT_AS_USED_BY, RECORD_ARTIFACTS_AS_USED_IN_FN_DECL)
(RECORD_ARTIFACTS_AS_USED_IN_FN_TYPE): Define new macros.
(build_function_decl, build_var_decl, build_qualified_type_decl)
(build_pointer_type_def, build_reference_type_def)
(build_function_type, build_array_type_def, build_enum_type_decl)
(build_typedef_decl, build_class_decl, build_union_decl): Use the
macros above to mark the relevant sub-types as used by the
artifact being built.
* tools/abilint.cc (struct artifact_use_relation_tree): Define new
type, guarded by the WITH_SHOW_TYPE_USE_IN_ABILINT macro.
(fill_artifact_use_tree, build_type_use_tree, emit_trace)
(emit_artifact_use_trace, emit_artifact_use_trace)
(show_how_type_is_used): Define static functions guarded by the
WITH_SHOW_TYPE_USE_IN_ABILINT macro.
(display_usage): Add doc string for the --show-type-use option,
guarded by the WITH_SHOW_TYPE_USE_IN_ABILINT macro.
(parse_command_line): Parse the --show-type-use option, guarded by
the WITH_SHOW_TYPE_USE_IN_ABILINT macro.
(main): Slight re-organisation to make the abixml file reading use
a read_context. That read context is then used to analyze how a
given type is used whenever the --show-type-use option is used.
Giuliano Procida [Fri, 21 Jan 2022 17:30:05 +0000 (17:30 +0000)]
XML writer: do not create extra temporary referenced type shared_ptr
In the loop of write_referenced_types temporary shared_ptr objects are
created. In the case of a declaration, two shared_ptrs are created. It
is possible to code this so only one object is created.
This is a small optimisation with no change to tests or behaviour.
* src/abg-writer.cc (write_referenced_types): Create temporary
shared_ptr objects within each conditional branch instead of
outside the conditionals and within one of the branches.
Giuliano Procida [Fri, 21 Jan 2022 17:30:04 +0000 (17:30 +0000)]
XML writer: improve slightly emission of top-level declarations
In the loop that emits declarations, the iterator already points to a
decl_base_sptr, there is no need to do another dynamic_cast. It is
also possible to simplify the loop and its conditionals.
There is no change to tests or behaviour.
* src/abg-writer.cc (decl_is_emitted): Make decl_base_sptr
argument a const reference.
(write_translation_unit): Eliminate a typedef and just use a
range-for loop without the extra dynamic cast for the non-type
case. Use else instead of continue to make it clear there are
only two possibilities.
git://sourceware.org / libabigail.git / log