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.
Dodji Seketeli [Mon, 7 Feb 2022 10:21:08 +0000 (11:21 +0100)]
Bug 26646 - unexpected declaration-only types
In a version of the kernel binary referred to in this problem report,
the parameter 'skb' of the udp4_hwcsum function, which is of type
"pointer to struct sk_buff", indirectly refers to a pointer to a
declaration-only struct ip_mc_list.
In another version of that kernel binary, the same parameter skb of
the udp4_hwcsum function is still of type "pointer to struct sk_buff",
but in that case, the sk_buff indirectly refers to a pointer to a
fully defined struct ip_mc_list.
The first kernel only contains a decl-only struct ip_mc_list whereas
the second one contains a fully defined struct ip_mc_list.
This problem comes from the fact that in add_or_update_class_type, we
"reuse" the "struct sk_buff" that we've already seen in the same
binary, if any. Depending on the order in which types are defined in
the debug information, if the DIE for struct sk_buff that refers to a
decl-only struct ip_mc_list has already been "seen" by the
DWARF-reader, then add_or_update_class_type re-uses the IR of that DIE
that's been constructed already; otherwise, the IR for the struct
sk_buff represented by the current DIE is constructed.
This patch fixes the problem by always constructing an IR for the
DIE that is being seen, in add_or_update_class_type.
* src/abg-dwarf-reader.cc (add_or_update_class_type): Do not reuse
the IR for a DIE with the same textual representation as the one
we are seeing now.
* tests/data/test-annotate/test21-pr19092.so.abi: Adjust.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
Dodji Seketeli [Mon, 7 Feb 2022 09:42:32 +0000 (10:42 +0100)]
symtab-reader: Remove an over-agressive assertion
In symtab::load, the symtab reader walks the symbol table and records
each relation "symbol <-> address".
So, the relation "foo <-> address-of-foo" is going to be recorded.
The relation "foo.cfi <-> address-of-foo.cfi" is going to be recorded
as well.
But then, because the symbol foo.cfi has a special meaning, in the
realm of "control flow integrity", the relation "foo.cfi <->
address-of-foo.cfi" (as well as all the *.cfi <-> address-of*.cfi
relations) is going to be recorded (again but) in a particular way by
calling symtab::add_alternative_address_lookups.
The problem is that in, symtab::add_alternative_address_lookups there
is an assert that (wrongly) assumes that the relation foo.cfi <->
address-of-foo.cfi is being seen for the first time. This is wrong
because the loop in symtab::load that records all the "symbol <->
address" relations has seen and recorded this foo.cfi <->
address-of-foo.cfi relation once already.
This patch removes that assert so that the kernel referred to in the bug
report of PR26646, as mentioned in
https://sourceware.org/bugzilla/show_bug.cgi?id=26646#c5, can be
processed by abidw without crashing.
David Seifert [Sat, 29 Jan 2022 19:31:07 +0000 (20:31 +0100)]
Find fts-standalone on musl
When using the musl C library fts is optional. So we need to detect
its presence by looking at the fts-standalone pkgconfig module.
This patch does that.
This comes from Gentoo bug https://bugs.gentoo.org/831571
* configure.ac: Invoke AC_CANONICAL_HOST to compute the host_cpu,
host_vendor, host_os parts of the 'host" variable. Then if the
host_os ends up with "musl" then, check for the fts-standalone
pkgconfig module and record the fts library into
FTS_{LIBS,CFLAGS}.
* src/Makefile.am: Link to $FTS_LIBS and use $FTS_CFLAGS for
compilation.
* tools/Makefile.am: Likewise.
* tools/abisym.cc: Include libgen.h
* tools/kmidiff.cc: Remove useless fts.h header file.
Signed-off-by: David Seifert <soap@gentoo.org> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
abidiff: improve whitespace generation in symbol diff report
maybe_report_diff_for_symbol() has a few issues:
1. The responsibility for newline emission is somewhat unclear and
indeed it would emit spurious blank lines before most of the
sub-diffs it reports.
2. Different sub-diff text and terminal commas are emitted according to
whether or not there had been a previous sub-diff - making the output
harder to grep and post-process.
3. The function also returns a bool but that return value is never used.
Hence, change the function to return void, the function stanzas to
always emit newline-terminated lines and ensure the wording and
punctuation of each sub-diff do not vary. This also tweaks (shortens)
the wording used for CRC diffs.
This is a performance and safety improvement made possible by previous
changes which ensure that the same pointers are used for insertion and
look-up.
This change affects two test cases. In more detail:
The test case test-read-dwarf/PR22122-libftdc.so.abi has many
duplicate type-id-60 which appear to all be types defined with a DWARF
DW_AT_signature attribute. These are made into separate types by this
change, but remain incomplete.
The test case test-read-dwarf/PR25007-sdhci.ko.abi has duplicate
declarations and these get split into duplicate declarations with new
type ids following this change. The test suite runs with an implicit
--no-linux-kernel-mode so the duplicates are treated separately. They
presumably had the same type ids before this change due to deep
equality considering them equal.
* src/abg-writer.cc (type_ptr_map): use default equality on
type_base pointer.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Refresh
test case, as described above.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.
This is a performance and safety improvement made possible by the
previous changes which ensure that the same pointers are inserted and
looked up.
This essentially removes the now unnecessary deep comparison.
* src/abg-writer.cc (type_ptr_set_type): Change typedef
container type to use default equality and hashing for pointer
keys.
(fn_type_ptr_set_type): Likewise.
Giuliano Procida [Tue, 18 Jan 2022 09:03:15 +0000 (09:03 +0000)]
abidiff: include ABI XML versions when reporting a mismatch
In the rare event of an XML version mismatch it would be helpful to
have the versions in the error message, particularly if abidiff is
being run from automation.
* tools/abidiff.cc
(emit_incompatible_format_version_error_message): Add version1
and version2 arguments. Add versions to error message.
(main): Pass emit_incompatible_format_version_error_message
mismatching versions.
Mark Wielaard [Tue, 18 Jan 2022 11:56:45 +0000 (11:56 +0000)]
DWARF reader: use size_t for DWARF expression length cont.
A recent change broke 32-bit builds due to an implicit assumption that
size_t == uint64_t. Note that size_t is part of the elfutils
dwarf_getlocation* functions' types.
The previous fix omitted some instances of uint64_t. This commit
updates further functions to consistently use size_t for DWARF
expression lengths and indexes.
* src/abg-dwarf-reader.cc (eval_last_constant_dwarf_sub_expr):
Change expr_len argument type to size_t.
(op_pushes_constant_value): Update ops_len and index argument
types to size_t. Update next_index argument type to size_t&.
(op_pushes_non_constant_value): Likewise.
(op_is_arith_logic): Update expr_len and index argument types
to size_t. Update next_index argument type to size_t&.
(op_is_control_flow): Likewise.
Giuliano Procida [Mon, 17 Jan 2022 14:51:15 +0000 (14:51 +0000)]
DWARF reader: use size_t for DWARF expression length
A recent change broke 32-bit builds due to an implicit assumption that
size_t == uint64_t. This appears in the elfutils dwarf_getlocation*
functions' types.
This commit updates callers and other functions to use size_t
consistently for such expression lengths and indexes.
* src/abg-dwarf-reader.cc (die_location_expr): Change expr_len
argument type to size_t*.
(op_manipulates_stack): Change expr_len and index argument
types to size_t; change next_index argument type to size_t&.
(eval_last_constant_dwarf_sub_expr): Change expr_len argument
and local variables index and next_index types to size_t.
(die_member_offset): Change local variable expr_len type to
size_t.
(die_location_address): Likewise.
(die_virtual_function_index): Likewise.
Giuliano Procida [Thu, 13 Jan 2022 08:56:48 +0000 (08:56 +0000)]
Bug 28191 - Interpret DWARF 5 addrx locations
This change uses libdw facilities to interpret location expressions
instead of using libabigail's own mini-interpreter. With the fix for
elfutils https://sourceware.org/bugzilla/show_bug.cgi?id=28220 in
elfutils-0.186, abidw will correctly interpret Clang DWARF 5 symbol
addresses. Without that fix many declarations will not be linked to
their corresponding symbols due to the incorrect interpretation of
location attribute data.
* src/abg-dwarf-reader.cc (die_location_address): Use
dwarf_attr_integrate, dwarf_getlocation and
dwarf_getlocation_attr to decode addreses, instead of
die_location_expr and eval_last_constant_dwarf_sub_expr.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Refresh test reference output; two more symbols have types.
This is a fix for bug https://bugzilla.redhat.com/show_bug.cgi?id=1951526.
Basically, this commits makes is so that two enums below are
considered equal by libabigail:
enum foo // This is foo #1
{
e0 = 0;
e1 = 1;
e2 = 2;
};
enum foo // This is foo #2
{
e0 = 0;
e1 = 1;
e2 = 2;
e_added = 1; // This enumerator is considered redundant
// with the enumerator e1 because their values
// are the same.
};
With this patch, foo #1 and foo #2 are considered equal, just like in
the original commit 1cfbff1b. In the original commit however, this
was achieved by comparing the enums without considering their
enumerator names. This was named "binary-only enum comparison". In
reality, that approach was too big of a hammer and was causing the
issues raised in the bug. Namely, type canonicalization would
conflate anonymous enums that were unrelated (precisely because their
enumerator names were different), leading to spurious type change
reports when comparing abixml files pre-dating commit 1cfbff1b with
posterior abixml files.
If I refer to the example above with foo #1 and #2, this patch detects
that the value of the enumerator 'e_added' is redundant with the value
of the enumerator e1. As such, the two foo #1 and #2 are considered
equal. Enumerator names are now fully taken into account.
With this precise approach, it now seems we can do away with the
careful dance of using "binary-only enum comparison" at some precise
times of the libabigail pipeline. Now, we can just use the new enum
comparison scheme all the time. Leading to less (complicated) code
and a hopefully accurate representation.
* include/abg-ir.h (environment::use_enum_binary_only_equality):
Remove.
* src/abg-comparison.cc (compute_diff): In the overload for
enum_type_decl, stop using binary-only-equality for enums.
* src/abg-dwarf-reader.cc
(read_context::compare_before_canonicalisation): Likewise.
* src/abg-ir.cc (environment::use_enum_binary_only_equality):
Remove.
(enumerators_values_are_equal)
(is_enumerator_value_present_in_enum)
(is_enumerator_value_redundant): Define new static functions.
(equals): In the overload for enum_type_decl, use the new
is_enumerator_value_redundant to detect if two enums are equal
modulo a redundant enumerator value. In that case, consider they
are equal.
* tests/data/test-abidiff/test-enum0-report.txt: Adjust.
* tests/data/test-annotate/test-anonymous-members-0.o.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-diff-dwarf/PR25058-liblttng-ctl-report-1.txt: 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/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise.
ctf-reader: Assert on ir::hash_as_canonical_type_or_constant
In some scenarios where we declare same data types `recursively' such
as: like linked list, functions that accept the same pointer to function
as arguments, forward types declarations to build structures with member
fields with mutual dependencies, etc., an assertion is trigger:
It is happening because the recursively behavior of `process_ctf_type'
and `process_ctf_*' used to register ctf types doesn't verify when a
ctf_type was processed and registered before by their subsequence
_recursive_ calls, so `type_base' object is built more than once and the
second time when it is inserted in `types_maps', it refuses in a silent
way, being that the key was already inserted, however
`add_decl_to_scope', `bind_function_type_life_time' successfully
registered the ctf type object. In this patch `process_ctf_type'
delegates register types task to `process_ctf_*' functions guaranteeing
a single ctf type registration, also it improves the performance looking
for the type before start to build it again.
* src/abg-ctf-reader.cc (process_ctf_base_type): Add new
`translation_unit_sptr' parameter. Add condition to validate
success 'base_type' construction and register type object.
(process_ctf_typedef): Add `lookup_type' to get a `type_base'
object when this was previously created, if this is not the
case, register ctf type. Add condition to validate success
'base_type' construction and register type object.
(process_ctf_function_type): Likewise.
(process_ctf_array_type): Likewise.
(process_ctf_qualified_type): Likewise.
(process_ctf_pointer_type): Likewise.
(process_ctf_struct_type): Add `add_decl_to_scope'.
(process_ctf_union_type): Likewise.
(process_ctf_type): Add `lookup_type' to get a `type_base'
object when this was previously created. Delegate register
type object to `process_ctf_*'.
* tests/data/Makefile.am: Add tests I/O and expected files.
* tests/data/test-read-ctf/test-array-of-pointers.[co]: New
testcase.
* tests/data/test-read-ctf/test-list-struct.[co]: Likewise.
* tests/data/test-read-ctf/test-callback.[co]: Likewise.
* tests/data/test-read-ctf/test-callback2.[co]: Likewise.
* tests/data/test-read-ctf/test-functions-declaration.[co]: Likewise.
* tests/data/test-read-ctf/test-forward-type-decl.[co]: Likewise.
* tests/data/test-read-ctf/test-array-of-pointers.abi:
Expected test output.
* tests/data/test-read-ctf/test-callback.abi: Likewise.
* tests/data/test-read-ctf/test-callback2.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/test-read-ctf.cc: Add testcases to CTF test harness.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
File "/srv/buildbot/worker/libabigail-centos-x86_64/build/tests/mockfedabipkgdiff", line 73, in <module>
fedabipkgdiff_mod = importlib.machinery.SourceFileLoader('fedabipkgdiff', FEDABIPKGDIFF).load_module()
AttributeError: 'module' object has no attribute 'machinery'
Again, I've asked The Internet what to do about that, and this commit is the
result. But beware: I'm still not a Python wizard.
* tests/mockfedabipkgdiff.in: Replace Python 'import importlib'
with 'import importlib.machinery'.
Otherwise, 'is_rpm_file' fails, resulting in a few test cases failing with an
unhelpful 'Unknown arguments. Please refer to --help.' message (similar to what
had been observed in PR22077 "runtestfedabipkgdiff.py fails on centos-x86_64").
* tools/fedabipkgdiff (is_rpm_file): Also accept MIME type
'application/x-redhat-package-manager' for RPM files.
CC: Chenxiong Qi <cqi@redhat.com> Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
Thomas Schwinge [Fri, 17 Dec 2021 22:26:30 +0000 (23:26 +0100)]
Replace use of deprecated Python 'imp' module with 'importlib'
In the test logs, I've found a number of:
[...]/tests/mockfedabipkgdiff:42: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
import imp
I've asked The Internet what to do about that, and this commit is the result.
But beware: I'm not a Python wizard.
* tests/mockfedabipkgdiff.in: Replace use of deprecated Python
'imp' module with 'importlib'.
CC: Chenxiong Qi <cqi@redhat.com> Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
Documenting/providing a way to enable such testing, this commit can be
considered a sequel to commit 90d236a03343d75ea504d53c73270e50b9000e75
"Bug 22076 - Disable fedabipkgdiff for old koji clients", for Mark Wielaard's
PR22076 "runtestfedabipkgdiff.py fails on debian-amd64".
Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
Thread 4 "abipkgdiff" hit Breakpoint 1, package::create_abi_file_path (this=0x5555555a7990,
elf_file_path="/media/[...]/home/thomas/.cache/libabigail/abipkgdiff-tmp-dir-upGgLK/package1/usr/lib64/libGLU.so.1.3.1", abi_file_path="") at [...]/tools/abipkgdiff.cc:668
668 create_abi_file_path(const string &elf_file_path,
(gdb) n
671 string abi_path, dir, parent;
(gdb) n
672 if (!abigail::tools_utils::string_suffix(elf_file_path,
(gdb) n
675 return false;
So we unexpectedly 'return false' here. That's because of 'elf_file_path' as
above ('realpath'ed) vs. 'extracted_dir_path()' as follows (not 'realpath'ed):
[...]
abipkgdiff: Could not create the directory tree to store the abi for '[...]'
abipkgdiff: Writting ABIXML file '' ...
abipkgdiff: Wrote ABIXML file '' OK
abipkgdiff: Reading ABIXML file '' ...
abipkgdiff: Could not read temporary ABIXML file ''
==== Error happened during self check of '[...]' ====
[...]
That is, after a failed 'create_abi_file_path', we proceed with an empty
'abi_file_path' -- because that one only gets set "iff the function return
true". So we ought to 'return abigail::tools_utils::ABIDIFF_ERROR' in that
case.
(It's likewise strange why 'create_write_context'/'write_corpus' succeed with
an empty 'abi_file_path', but that's for another day...)
Thomas Schwinge [Fri, 10 Dec 2021 16:52:46 +0000 (17:52 +0100)]
configure: Instead of for rpm 4.15+ version, test actual rpm/zstd support
If no 'rpm' is available, we currently get:
[...]
checking for rpm... no
../git/configure: line 13119: rpm: command not found
configure: detected rpm version:
configure: rpm support in abipkgdiff is disabled
[...]
Here is the configuration of the package:
[...]
Enable rpm support in abipkgdiff : no
Enable rpm 4.15 support in abipkgdiff tests : no
[...]
Notice intermixed error output: 'rpm: command not found'.
If Ubuntu focal 'rpm' 4.14.2.1+dfsg1-1build2 is available, we currently get:
[...]
checking for rpm... yes
configure: detected rpm version: 4.14.2.1
configure: rpm support in abipkgdiff is enabled
configure: rpm 4.15 support in abipkgdiff tests is enabled
[...]
Here is the configuration of the package:
[...]
Enable rpm support in abipkgdiff : yes
Enable rpm 4.15 support in abipkgdiff tests : yes
[...]
Notice wrong 4.15+ version detection (due to '[[ "$rpmversion" > "4.14.0" ]]'),
which is satisfied by '4.14.2.1'. (Comparing versions with shell '[['
generally is fragile; instead use 'autoconf-archive/ax_compare_version.m4'
or similar?)
Also, 'configure'ing with '--disable-rpm415' doesn't work; same output as
before. That's due to commit 26c41c060bf30750fe2cded87edaf1ae47027523
"Fix thinko in configure.ac", where either there was no thinko in fact (the
original idea, I suppose, was only if 'test x$ENABLE_RPM = xyes' to do the
4.15+ 'auto' checking?), and/or a typo: instead of 'test x$ENABLE_RPM = xyes',
the first conditional should 'test x$ENABLE_RPM415 = xyes'?
And, 'configure'ing with '--enable-rpm415' doesn't raise a hard error if 'rpm'
actually isn't 4.15+.
But all that said, we don't actually need to check for rpm 4.15+ version, but
instead may simply check for the rpm/zstd support that we need: 'rpm2cpio'.
* configure.ac: Instead of for rpm 4.15+ version, test actual
rpm/zstd support.
* tests/test-diff-pkg.cc: Adjust.
Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
Thomas Schwinge [Tue, 14 Dec 2021 16:44:53 +0000 (17:44 +0100)]
Further update 'make distcheck-fast'
What got pushed in recent commit 497357cfd5793e5364a92080d9fae40cf94dc7dd
"Better highlight 'make distcheck-fast'" was the initial submission, before the
changes I made after Matthias Maennich's review. So here they are again.
* CONTRIBUTING: Further update 'make distcheck-fast'.
Suggested-by: Matthias Maennich <maennich@google.com> Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
If 'configure' finds some Python koji module, but it's "insufficient" per
the testing once added in commit 90d236a03343d75ea504d53c73270e50b9000e75
"Bug 22076 - Disable fedabipkgdiff for old koji clients", we currently get,
for example:
[...]
checking python3 module: koji... yes
[...]
checking checking if koji client is recent enough ...... Traceback (most recent call last):
File "<string>", line 3, in <module>
File "[...]/koji/__init__.py", line 2016, in read_config
raise ConfigurationError("no configuration for profile name: %s"
koji.ConfigurationError: no configuration for profile name: koji
no, disabling fedpkgdiff
[...]
Here is the configuration of the package:
[...]
Enable fedabipkgdiff : auto
[...]
Note repeated 'checking' and '...', intermixed error output, 'fedpkgdiff'
typo, final 'auto' result.
Changing that to:
[...]
checking if koji client is recent enough... no
configure: WARNING: disabling fedabipkgdiff
[...]
Here is the configuration of the package:
[...]
Enable fedabipkgdiff : no
[...]
... with 'config.log':
[...]
configure:13774: checking if koji client is recent enough
configure:13784: result: no
Traceback (most recent call last):
File "<string>", line 3, in <module>
File "[...]/koji/__init__.py", line 2016, in read_config
raise ConfigurationError("no configuration for profile name: %s"
koji.ConfigurationError: no configuration for profile name: koji
configure:13792: WARNING: disabling fedabipkgdiff
[...]
Similarly, with explicit '--enable-fedabipkgdiff', we currently get:
[...]
checking checking if koji client is recent enough ...... Traceback (most recent call last):
File "<string>", line 3, in <module>
File "[...]/koji/__init__.py", line 2016, in read_config
raise ConfigurationError("no configuration for profile name: %s"
koji.ConfigurationError: no configuration for profile name: koji
no, disabling fedpkgdiff
[...]
Here is the configuration of the package:
[...]
Enable fedabipkgdiff : yes
[...]
... instead of a fatal error.
Changing that to:
[...]
checking if koji client is recent enough... no
configure: error: unsuitable koji client
Thomas Schwinge [Sat, 11 Dec 2021 17:58:22 +0000 (18:58 +0100)]
CONTRIBUTING: Move "Coding language and style" section
This section got added in commit 4f8c9b170d80b920d13e8f291b347df74db34307
"Use C++11 for the code base", but unfortunately got placed in the middle
of the "Regression tests" section. Move it after that one.
* CONTRIBUTING: Move "Coding language and style" section.
Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
elfutils libdw before 0.184 would not correctly handle a
DW_AT_data_member_location when encoded as a DW_FORM_implicit const
in dwarf_location_expression.
Work around this by first trying to read a data_member_location as a
constant value and only try to get it as a DWARF expression if that
* src/abg-dwarf-reader.cc (die_constant_data_member_location):
New function.
(die_member_offset): Use die_constant_data_member_location
before calling die_location_expr and eval_quickly.
This patch implements some regression tests for ctf reading.
Since the code shares a lot of functionalities already used
in the readi-dwarf test, a library was built and test common
harness were moved to a common location. So input files for
test-read-{dwarf,ctf}.cc now are located in:
tests/data/test-read-common directory, ABIs description are
stored in the same location but in a separate file, one for
each binary debugging information: (e.g, test4-ctf.so.abi
and test4-dwarf.so.abi)
* tests/test-read-ctf.cc: New ctf reading regression test.
* tests/test-read-common.cc: New library to be used with
test-read-{ctf,dwarf}.cc.
* tests/test-read-common.h: Likewise.
* tests/test-annotate.cc (in_out_specs): Adjust path for input files.
* tests/Makefile.am: Build new tests/test-read-ctf.cc file.
* tests/data/Makefile.am: Add test inputs and expected files.
Add libtestreadcommon.a test library and use it for test-read-{ctf,dwarf}.
* tests/test-read-dwarf.cc: Adapt test to use libtestreadcommon.a in
test-read-common.{cc,h}.
* tests/data/test-annotate/test3.so.abi: Adjust ELF input path file
location to ./tests/data/test-read-common.
* tests/data/test-annotate/test4.so.abi: Likewise.
* tests/data/test-read-common/PR26261: Move test harness to
test-read-common directory.
* tests/data/test-read-common/PR27700: Likewise.
* tests/data/test-read-common/test-PR26568-*: Likewise.
* tests/data/test-read-common/test3.{c,so}: Likewise.
* tests/data/test-read-common/test4.{c,so}: Likewise.
* tests/data/test-read-common/crti*: Helper object to export
_init and _fini sysmbols.
* tests/data/test-read-ctf/test-ambiguous-struct-A.c: New testcase.
* tests/data/test-read-ctf/test-ambiguous-struct-B.c: Likewise.
* tests/data/test-read-ctf/test-conflicting-type-syms-a.c: Likewise.
* tests/data/test-read-ctf/test-conflicting-type-syms-b.c: Likewise.
* tests/data/test-read-ctf/test-enum.c: Likewise.
* tests/data/test-read-ctf/test-enum-many.c: Likewise.
* tests/data/test-read-ctf/test-enum-symbol.c: Likewise.
* tests/data/test-read-ctf/test-struct-iteration.c: Likewise.
* tests/data/test-read-ctf/test-dynamic-array.c: Likewise.
* tests/data/test-read-ctf/test0.c: Likewise.
* tests/data/test-read-ctf/test1.c: Likewise.
* tests/data/test-read-ctf/test2.c: Likewise.
* tests/data/test-read-ctf/test5.c: Likewise.
* tests/data/test-read-ctf/test7.{c,h}: Likewise.
* tests/data/test-read-ctf/test8.c: Likewise.
* tests/data/test-read-ctf/test9.c: Likewise.
* tests/data/test-read-ctf/test-ambiguous-struct-A.o.hash.abi: Testcase
expected result.
* tests/data/test-read-ctf/PR26261/PR26261-exe.abi: Likewise.
* tests/data/test-read-ctf/PR27700/test-PR27700.abi: Likewise.
* tests/data/test-read-ctf/test-ambiguous-struct-A.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-ambiguous-struct-B.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-enum.o.abi: Likewise.
* tests/data/test-read-ctf/test-enum-many.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-enum-symbol.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-struct-iteration.o.abi: Likewise.
* tests/data/test-read-ctf/test-dynamic-array.o.abi: Likewise.
* tests/data/test-read-ctf/test0: Likewise.
* tests/data/test-read-ctf/test0*.abi: Likewise.
* tests/data/test-read-ctf/test1.so: Likewise.
* tests/data/test-read-ctf/test1*.abi: Likewise.
* tests/data/test-read-ctf/test2.so: Likewise.
* tests/data/test-read-ctf/test2*.abi: Likewise.
* tests/data/test-read-ctf/test3.so.abi: Likewise.
* tests/data/test-read-ctf/test4*.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.
* tests/data/test-read-dwarf/PR26261/PR26261-exe.abi: Update
expected abixml file.
* tests/data/test-read-dwarf/PR27700/test-PR27700.abi: Likewise.
* tests/data/test-read-dwarf/test-PR26568-1.*.abi: Likewise.
* tests/data/test-read-dwarf/test3*.abi: Likewise.
* tests/data/test-read-dwarf/test4*.abi: Likewise.
* doc/api/libabigail.doxy: Add tests/test-read-common.{cc,h} to
doxygen.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
ctf-reader: Fix length in dynamic array definition
Defining an array type with dynamic length, node `subrange'
in the abixml file doesn't write the accurate `length'
property `infinite', instead `1' is written:
<subrange length='1' .../>
So, member function `array_type_def::subrange_type::is_infinite'
is set when `upper_bound' value is equal to `0'.
* src/abg-ctf-reader.cc (process_ctf_array_type):
set subrange_type::is_infinite when `upper_bound' value
is equal to `0'.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
XML writer: use exemplar types for tracking referenced types
The emitted type sets are used with some referenced type sets (which use
bare type pointers). To keep consistency between what is being recorded
in each set, switch to storing exemplar type pointers in the referenced
type sets.
This change results in the omission of a small number of duplicate
types from various test cases. In each case the duplicates were
previously caused by a referenced type being emitted for one
translation unit and then the same type being emitted as a canonical
type for a later translation unit.
It also causes the movement of some function types in some test cases.
Some of those types are now considered referenced and appear earlier as
a result.
* src/abg-writer.cc (record_type_as_referenced): Use exemplar
type with referenced type sets.
(type_is_referenced): Likewise.
(tests/data/test-annotate/test14-pr18893.so.abi): Duplicate
type(s) removed, as described above.
(tests/data/test-read-dwarf/test14-pr18893.so.abi): Likewise.
(tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi):
Likewise.
(tests/data/test-read-dwarf/test16-pr18904.so.abi): Likewise.
(tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi):
Likewise.
(tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi):
Likewise.
(tests/data/test-read-dwarf/PR25007-sdhci.ko.abi): Some
function type(s) reordered, as described above.
(tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi):
Likewise.
(tests/data/test-annotate/test15-pr18892.so.abi):: Duplicate
type(s) removed and some function type(s) reordered, as
described above.
(tests/data/test-read-dwarf/test15-pr18892.so.abi): Likewise.
(tests/data/test-annotate/test21-pr19092.so.abi): Likewise.
(tests/data/test-read-dwarf/test21-pr19092.so.abi): Likewise
XML writer: use consistent type pointers for type ids and emission tracking
Insertion uses the canonical type, if available, but look-up did
not. Given that type id insertion and look-up also use canonical
types, it makes sense to adjust the remaining code accordingly.
Neither decl_only_type_is_emitted nor record_decl_only_type_as_emitted
do the check, but very few types end up being recorded this way.
The functions write_class_decl and write_union_decl (but not
write_class_decl_opening_tag and write_union_decl_opening_tag which
can be called in other contexts) resolve declaration-only types to a
definition where possible.
To ensure type ids consistently refer to the same canonical type we
should use canonical types and definitions-of-declarations more
consistently.
This change introduces get_exemplar_type to return the exemplar type
that should be used for type id and emitted checks. That exemplar
type is the canonical type of a given type, or the canonical type of
the definition-of-declaration-only-type when applicable.
However, it does not also change all the write functions to write out
the exemplar types.
* include/abg-fwd.h (get_exemplar_type): Declare new function.
* src/abg-ir.cc (get_exemplar_type): Define new function.
* src/abg-writer.cc (type_has_existing_id): use get_exemplar_type
for resolution.
(get_id_for_type): Likewise.
(record_type_as_emitted): Likewise.
(type_is_emitted): Likewise.
Dodji Seketeli [Mon, 6 Dec 2021 16:45:46 +0000 (17:45 +0100)]
reader: Build array types with their element type "a priori"
In some ancient settings, we were building array types with no element
type set, then, when we have the element type built, we'd set it to
the array type. This was to avoid looping indefinitely in cases where
the element type would indirectly depend on the array type itself.
Since then, we've built infrastructure to avoid those loops.
So we don't need those 'temporarily empty arrays' anymore. Besides,
trying get the pretty representation of a variable declaration which
type is one of those temporarily empty arrays crashes because that
code doesn't expect empty arrays.
This patch sets the element type at array type building type now. The
code also asserts that element types of arrays are not empty, in the
pretty representation of variable declarations.
* src/abg-ir.cc (var_decl::get_pretty_representation): Assert that
array element types are not empty.
* src/abg-reader.cc (build_array_type_def): Set array element
types a priori.
This should fix bug https://sourceware.org/bugzilla/show_bug.cgi?id=28073
There is at least a case where the evaluation of the suppression
specification rule incarnated by the property
has_data_member_inserted_between doesn't work. This is in the context
of the following suppression specification:
[suppress_type]
name = struct_foo
has_data_member_inserted_between = {offset_of(dm1), offset_of(dm2)}
The evaluation of the rule incarnated by
has_data_member_inserted_between fails in the context of a type change
where the data member "dm1" is removed from the type struct_foo. In
that case, the evaluation of the suppression should ALWAYS yield to
the suppression specification NOT suppressing the change. But in some
cases the change is suppressed nonetheless.
This patch fixes that.
The idea of the patch is that if the class has a removed data member
or if its size shrinks then no type change on that class can be
suppressed. This is because those two kinds of change are
incompatible ABI (or at least API) changes. So they should be
reported.
The patch also fixes the evaluation of the boundaries of the insertion
range expressed as an "offset_after" expression.
* doc/manuals/libabigail-concepts.rst: Update the documentation to
reflect that has_data_member* properties will never suppress any
type change if the change carries a data member suppression or a
type size reduction.
* include/abg-fwd.h (get_last_data_member)
(get_next_data_member_offset): Declare new functions.
* include/abg-suppression.h
(insertion_range::boundary_value_is_end): Declare new static
member function.
(type_supression::insertion_range::eval_boundary): Make this
static function take an uint64_t rather than ssize_t.
(type_suppression::insertion_range::integer_boundary::{integer_boundary,
as_integer, operator int}): Make these member functions and
operator take or return uint64_t rather than int.
* src/abg-ir.cc (get_last_data_member)
(get_next_data_member_offset): Define new functions.
* src/abg-suppression.cc
(type_suppression::suppresses_diff): Rework logic to better handle
"has_data_member_inserted_*" properties in the context of class
diffs. If the diff object carries data member removal or size
reduction, the diff object is not suppressed by the current type
suppression. Also, the property "has_data_member_inserted_at =
end", is now represented by an insertion range where the beginning
and the end of the range are both the max possible value of
insertion range boundaries; the code is made to recognize that.
(type_suppression::insertion_range::eval_boundary): Make this
static function take an uint64_t rather than ssize_t. If the
boundary is expressed as a "offset_after" expression, make sure
the offset of the next data member is considered if it's present.
(type_suppression::insertion_range::integer_boundary::{integer_boundary,
as_integer, operator int}): Make these take or return uint64_t
rather than int.
(type_suppression::insertion_range::boundary_value_is_end): Define
new member function.
(type_suppression::insertion_range::integer_boundary::priv::value_):
Turn the type of this into uint64_t, from int.
(type_suppression::insertion_range::integer_boundary::priv::priv):
The parameter of this is now uint64_t, from int.
* tests/data/test-diff-suppr/PR28073/PR28073-bitfield-removed.c:
New test source code.
* tests/data/test-diff-suppr/PR28073/PR28073-bitfield-removed.o:
New test binary.
* tests/data/test-diff-suppr/PR28073/PR28073-bitfield-removed.o.abi:
New test input.
* tests/data/test-diff-suppr/PR28073/PR28073-output-{1,2}.txt: New
test reference output.
* tests/data/test-diff-suppr/PR28073/PR28073.after.o: New test
binary.
* tests/data/test-diff-suppr/PR28073/PR28073.after.o.abi: New test
input.
* tests/data/test-diff-suppr/PR28073/PR28073.before.o: New test
binary.
* tests/data/test-diff-suppr/PR28073/PR28073.before.o.abi: New
test input.
* tests/data/test-diff-suppr/PR28073/PR28073.c: New test source
code.
* tests/data/test-diff-suppr/PR28073/bitfield.suppr: New test
input.
* tests/data/Makefile.am: Add the new test material to source
distribution.
* tests/test-diff-suppr.cc: Add the new test input to this test
harness.
XML writer: adjust tracking of emitted declarations
Replace the std::unordered_map used to track emitted declarations with a
std::unordered_set as the map only ever held "true". The container
itself does not need to be marked mutable because record_decl_as_emitted
is called in a non-const context and can itself be made non-const. In
addition, the method decl_is_emitted calls a helper which no longer used
anywhere else and can be inlined. The remaining two methods are always
called on non-type declarations, so the test that existed in
decl_is_emitted can be dropped.
* abg-writer.cc (write_context): Replace mutable
m_emitted_decls_map with plain m_emitted_decls_set.
(decl_name_is_emitted): Inlined into decl_is_emitted; dropped.
(decl_is_emitted): Turn the is_type check into an assert and
inline decl_name_is_emitted. Look up in set instead of map.
(record_decl_as_emitted): Make non-const. Insert into set
instead of map.
runtestreadctf fails executed by make check-valgrind, a memory leak is
reported:
at 0x48397B5: malloc (vg_replace_malloc.c:381)
by 0x56E6316: ctf_bufopen_internal (ctf-open.c:1388)
by 0x56E5FD3: ctf_bufopen (ctf-open.c:1297)
by 0x56CD688: ctf_arc_bufopen (ctf-archive.c:444)
by 0x4EB31FD: abigail::ctf_reader::read_corpus(abigail::ctf_reader::read_context*,\
abigail::elf_reader::status&) (abg-ctf-reader.cc:1111)
by 0x10C6BB: test_task_ctf::perform() (test-read-ctf.cc:236)
by 0x4E8E9AF: abigail::workers::worker::wait_to_execute_a_task\
(abigail::workers::queue::priv*) (abg-workers.cc:400)
by 0x543BEAD: start_thread (pthread_create.c:463)
by 0x5369A5E: clone (clone.S:95)
This is happening because `ctf_variable_next' bumps the dictionary
reference counter, so when `ctf_close' is called it doesn't free the
memory allocated by the dict, because `ctf_refcnt' is greater than one.
Therefore `ctf_dict_close' is required.
git://sourceware.org / libabigail.git / log