Dodji Seketeli [Wed, 15 Feb 2023 14:53:02 +0000 (15:53 +0100)]
comp-filter: Don't re-visit node while applying filters to diff nodes
When applying a filter to a corpus_diff node, visit each diff node
only once. This can have some serious performance impact when there
are a lot of diff nodes to visit.
* src/abg-comp-filter.cc (apply_filter): In the overload for
corpus_diff, visit each diff node only once.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Wed, 15 Feb 2023 15:26:20 +0000 (16:26 +0100)]
tools-utils: Support kernel stablelist
Up until now, a kernel whitelist was expected to be a ini file with a
section having a name ending with the word "whitelist". Nowadays,
they are called "stablelist", so the name of the section ends up with
"stablelist". This patch makes
gen_suppr_spec_from_kernel_abi_whitelists support that.
* src/abg-tools-utils.cc
(gen_suppr_spec_from_kernel_abi_whitelists): Support section name
that ends with the word 'stablelist'.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Wed, 15 Feb 2023 12:18:07 +0000 (13:18 +0100)]
abidiff: Add extensive logging
While looking at something else I felt the need for having
"abidiff --verbose" emit more timing information. I have thus added
a lot more logging around.
* include/abg-comparison.h ({diff, corpus_diff,
diff_context}::do_log): Declare member functions.
* include/abg-corpus.h (corpus::do_log): Likewise.
* src/abg-comparison-priv.h (diff_context::priv::do_log_): Add new
data member.
(diff_context::priv::priv): Initialize the new data member.
* src/abg-comparison.cc ({diff, corpus_diff,
diff_context}::do_log): Define member functions.
(diff_context::maybe_apply_filters): Add timing logs to applying
filters and propagating categories.
(corpus_diff::priv::apply_filters_and_compute_diff_stats): Add
timing logs to applying and propagating filters to changed
functions, variables, unreachable & leaf type changes,
suppressions application.
* src/abg-corpus-priv.h (corpus::priv::do_log): Add new data
member.
(corpus::priv::priv): Initialize it.
* src/abg-corpus.cc (corpus::do_log): Define member functions.
* src/abg-reader.cc (reader::do_log): Likewise.
(reader::read_corpus): Add timing log around the invocation of
perform_late_type_canonicalizing.
* tools/abidiff.cc (set_diff_context_from_opts): Set logging.
(main): Add timing logging for diff computing, changes analysis &
report generation.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Sun, 29 Jan 2023 18:58:21 +0000 (19:58 +0100)]
Misc white space fixes
* include/abg-suppression.h (class
type_suppression::insertion_range::end): Fix indentation.
* src/abg-default-reporter.cc (default_reporter::report): Fix
indentation in the overload for corpus_diff.
* src/abg-suppression-priv.h
(type_suppression::priv::source_locations_to_keep_): Fix alignment.
* src/abg-suppression.cc (read_type_suppression): Fix alignment of
comment.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Sun, 29 Jan 2023 18:57:33 +0000 (19:57 +0100)]
comparison, suppression: Support [allow_type] directive
This patch adds support for a new 'allow_type' suppression directive.
It suppresses all the changes that are NOT matched by the directive.
In other words, this directive determines the set of type changes that
are NOT suppressed. Any other change is suppressed. This thus called
a "negated suppression directive".
The way these negated suppression directives interact with the direct
suppression directives that already exist is the following.
The suppression evaluation pass visits every single diff node
(carrying a type change) of the diff graph. Negated suppressions are
evaluated first, in order of occurrence.
There are thus, two alternatives:
1/ At least one negated suppression matches the current diff node.
or
2/ No negated suppression matches the current diff node.
In case of 1/ then direct suppression specifications are
considered. There are two alternatives:
1.1/ At least one direct suppression matches the current diff node.
The diff node is suppressed: categorized as being in the
SUPPRESSED_CATEGORY category)
or
1.2/ No direct suppression matches the current diff node.
The diff node is not suppressed: categorized as being in the
HAS_ALLOWED_CHANGE_CATEGORY category.
In case of 2/ then direct suppression specifications are
considered. There are two alternatives:
2.1 At least one direct suppression matches the current diff node.
The diff node is categorized as being in the
SUPPRESSED_CATEGORY category, just like in 1.1.
2.2 No direct suppression matches the current diff node.
The diff node is not suppressed and not categorized.
As a result of the category propagation pass, a node which has a
parent node categorized as HAS_ALLOWED_CHANGE_CATEGORY is itself
categorized as HAS_PARENT_WITH_ALLOWED_CHANGE_CATEGORY. A node which
has a descendant categorized as HAS_ALLOWED_CHANGE_CATEGORY will
itself be categorized as HAS_DESCENDANT_WITH_ALLOWED_CHANGE_CATEGORY.
Nodes that are categorized as HAS_ALLOWED_CHANGE_CATEGORY,
HAS_DESCENDANT_WITH_ALLOWED_CHANGE_CATEGORY and
HAS_PARENT_WITH_ALLOWED_CHANGE_CATEGORY are not suppressed by the
reporting passes. This is needed for the reporting passes to emit the
impact sub-tree up to the diff node which carry the change that was
actually categorized as HAS_ALLOWED_CHANGE_CATEGORY.
* include/abg-comparison.h: Include abg-suppression.h
(diff, diff_context, diff_sptr, diff_context_sptr): Remove these
forward decls from here.
(enum diff_category::{HAS_ALLOWED_CHANGE_CATEGORY,
HAS_DESCENDANT_WITH_ALLOWED_CHANGE_CATEGORY,
HAS_PARENT_WITH_ALLOWED_CHANGE_CATEGORY}): Add new enumerators.
(enum diff_category::EVERYTHING_CATEGORY): Update enumerator.
(diff_context::{negated_suppressions, direct_suppressions}): Declare
new member functions.
(diff_context::suppressions): Add overload.
(diff::{is_filtered_out_without_looking_at_allowed_changes,
is_allowed_by_specific_negated_suppression,
has_descendant_allowed_by_specific_negated_suppression,
has_parent_allowed_by_specific_negated_suppression}): Declare new
member functions.
* include/abg-suppression.h (class negated_suppression_base, class
negated_type_suppression): Declare new classes.
(negated_suppression_sptr, negated_suppression_type): Define new
typedefs.
(is_negated_suppression): Declare new functions.
* src/abg-suppression.cc
(negated_suppression_base::{negated_suppression_base,
~negated_suppression_base}): Define member functions.
(negated_type_suppression::{negated_type_suppression,
suppresses_diff, ~negated_type_suppression}): Likewise.
(is_negated_suppression): Define functions.
(read_type_suppression): Allow parsing the "allow_type" directive
and instantiate a negated_type_suppression.
* src/abg-comparison-priv.h
(diff_context::priv::{negated_suppression_type_,
direct_suppressions}): Define new data members.
(diff::priv::is_filtered_out): A node categorized as
HAS_DESCENDANT_ALLOWED_BY_SPECIFIC_NEGATED_SUPPRESSION,
HAS_PARENT_ALLOWED_BY_SPECIFIC_NEGATED_SUPPRESSION and
HAS_ALLOWED_CHANGE_CATEGORY is not filtered out.
* src/abg-comparison.cc (diff_context::suppressions): Add a
non-const overload.
(diff_context::{negated,direct}_suppressions): Define new member
function.
(diff_context::add_suppression): Invalidate the cache data members
diff_context::priv::{negated,direct}_suppressions_.
(diff::is_filtered_out): A node categorized as
HAS_DESCENDANT_ALLOWED_BY_SPECIFIC_NEGATED_SUPPRESSION,
HAS_PARENT_ALLOWED_BY_SPECIFIC_NEGATED_SUPPRESSION and
HAS_ALLOWED_CHANGE_CATEGORY is not filtered out.
(diff::is_filtered_out_without_looking_at_allowed_changes): Define
new member function.
(diff::is_suppressed): If there is at least one negated
suppression that match the diff node, then it's not suppressed,
unless it's matched by a direct suppression.
(diff::{is_allowed_by_specific_negated_suppression,
has_descendant_allowed_by_specific_negated_suppression,
has_parent_allowed_by_specific_negated_suppression}): Define new
member functions.
(operator<<(ostream& o, diff_category c)): Serialize
HAS_{DESCENDANT_WITH,PARENT_WITH}_ALLOWED_CHANGE_CATEGORY
enumerators.
(category_propagation_visitor::visit_end): Do not propagate
HAS_ALLOWED_CHANGE_CATEGORY,
HAS_DESCENDANT_WITH_ALLOWED_CHANGE_CATEGORY and
HAS_PARENT_WITH_ALLOWED_CHANGE_CATEGORY categories.
(suppression_categorization_visitor::visit_begin): Categorize a
node that is not suppressed by a direct suppression and is
suppressed by a negated one as
HAS_ALLOWED_CHANGE_CATEGORY. Propagate it to descendant nodes as
HAS_PARENT_WITH_ALLOWED_CHANGE_CATEGORY ...
(suppression_categorization_visitor::visit_end): ... and to parent
node as HAS_DESCENDANT_WITH_ALLOWED_CHANGE_CATEGORY.
* src/abg-default-reporter.cc (default::reporter): In the overload
for typedef_diff, qualified_type_diff, reference_diff,
fn_parm_diff, function_type_diff, array_diff, base_diff,
function_decl_diff, report local changes only
on node that are not filtered out wrt allowed changed.
* tests/data/test-abidiff-exit/test-allow-type-array-suppr.txt:
New test input.
* tests/data/test-abidiff-exit/test-allow-type-array-v0--v1-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-array-v0--v2-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-array-v0--v3-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-array-v{0,1,2,3}.c:
Source code of new binary test inputs.
* tests/data/test-abidiff-exit/test-allow-type-array-v{0,1,2,3}.o:
New binary test inputs.
* tests/data/test-abidiff-exit/test-allow-type-region-suppr.txt:
New test input.
* tests/data/test-abidiff-exit/test-allow-type-region-v0--v1-report-{1,2}.txt:
* tests/data/test-abidiff-exit/test-allow-type-region-v0--v2-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-region-v0--v3-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-region-v0--v4-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-region-v0--v5-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-region-v{0,1,2,3,4,5}.c:
Source code of new binary test input.
* tests/data/test-abidiff-exit/test-allow-type-region-v{0,1,2,3,4,5}.o:
New binary test inputs.
* tests/data/test-abidiff-exit/test-allow-type-suppr{1,2}.txt: New
test inputs.
* tests/data/Makefile.am: Add the new testing files above to
source distribution.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 27 Jan 2023 16:00:42 +0000 (17:00 +0100)]
suppression: Support offset_of_{first,last}_data_member_regexp offset selectors
This patch adds support for two data member offset selector
expressions for properties of the [suppress_type] directive:
offset_of_first_data_member_regexp() and offset_of_last_data_member_regexp().
These function-call expressions take a regular expression argument and
evaluate to the offset of the first (resp. last) data member matching
the regular expression argument.
An example of their use would be be:
[suppress_type]
type_kind = struct
has_data_member_inserted_between =
{
offset_of_first_data_member_regexp(^__special_padding_space),
offset_of_last_data_member_regexp(^__special_padding_space)
}
This would be useful to suppress change reports involving a struct
which has "padding" data members added on-purpose like:
struct S
{
int member0;
char member1;
unsigned __special_padding_space1;
unsigned __special_padding_space2;
unsigned __special_padding_space3;
};
* doc/manuals/libabigail-concepts.rst: Document the new
properties.
* include/abg-fwd.h: Forward declare comparison::{diff_context,
diff_context_sptr, diff_context_wptr, diff, diff_wptr} and
regex::regex_t_sptr.
(find_first_data_member_matching_regexp)
(find_last_data_member_matching_regexp): Declare new functions.
* include/abg-suppression.h: Inject std::{string, shared_ptr,
vector} and comparison::{diff, diff_context_sptr} into the suppr
namespace. Remove the "abg-comparison.h" header.
* src/abg-elf-helpers.cc: Include sstream.
* src/abg-ir.cc (find_first_data_member_matching_regexp)
(find_last_data_member_matching_regexp): Define new functions.
* src/abg-suppression.cc
(type_suppression::insertion_range::eval_boundary): Support
evaluating "offset_of_first_data_member_regexp" and
"offset_of_first_data_member_regexp".
* src/abg-ctf-reader.cc: Include sstream.
* tests/data/test-diff-suppr/test-has-data-member-inserted-between-1-report-[1-4].txt:
New test reference outputs.
* tests/data/test-diff-suppr/test-has-data-member-inserted-between-1-v[0-4].c:
Source code of new test input.
* tests/data/test-diff-suppr/test-has-data-member-inserted-between-1-v[0-4].o:
New binary test input.
* tests/data/test-diff-suppr/test-has-data-member-inserted-between-1.suppr:
New suppression specification.
* tests/data/Makefile.am: Add the new test input files to source
distribution.
* tests/test-diff-suppr.cc (in_out_specs): Add the new test input
to this test harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 27 Jan 2023 16:30:35 +0000 (17:30 +0100)]
suppression: Support the has_size_change property for suppress_type
The "has_data_member_inserted_between" and
"has_data_members_inserted_between" properties of the [suppress_type]
directive allows the suppression of type changes when a data member is
inserted in a given range. It turns out that suppressing type changes
that incur a change in the size of the type might not be what the user
wants by default, because the type size in itself might actually be an
incompatible ABI change that would then fly under the radar because of
this suppression specification.
An arguably better default behavior in this case would be to NOT
suppress the type change if the data member insertion does incur a
change in the size of the type.
But then, there would be cases where the user would really want to
suppress the type change due to data member insertion in a given range
even if it incurs a change in the type size. This is where this patch
enters into play.
The patch introduces the "has_size_change" property of the
[suppress_type] directive. In the presence of
"has_data_members_inserted_between" or
"has_data_member_inserted_between" properties, if the
"has_size_change" property is set to "yes", then the type change would
be suppressed if data members are inserted in the given range even if
the insertion incurs a type size change.
Otherwise, with this patch, in the absence of the "has_size_change"
property, the "has_data_member_inserted_between" and
"has_data_members_inserted_between" properties won't trigger the type
change suppression if the data member insertion incurs a type size
change.
* doc/manuals/libabigail-concepts.rst: Document the new
has_size_change property.
* include/abg-suppression.h
(type_suppression::{g,s}et_has_size_change): Declare new accessors.
* src/abg-suppression-priv.h
(type_suppression::priv::has_size_change_): Define new data
member.
(type_suppression::priv::priv): Initialize the new data member.
* src/abg-suppression.cc
(type_suppression::{g,s}et_has_size_change): Define new accessors.
(type_suppression::suppresses_diff): Make the
has_data_member_inserted_* clauses have effect only if the class
size hasn't changed, unless the class has as the "has_size_change"
property. Also, allow members to be deleted in the right
insertion range if the resulting size stays the same or if the
has_size_change property is present. This allows some custom
behaviours where "padding" data members would be removed while
some new data members would be added, resulting in a type which
size would not change.
(read_type_suppression): Support parsing the "has_size_change"
property.
* tests/data/test-diff-suppr/test11-add-data-member-0.1.suppr: New
test suppression specification.
* tests/data/test-diff-suppr/test11-add-data-member-1.1.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-2.1.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-3.1.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-4.1.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-report-1.1.txt:
Likewise.
* tests/data/test-diff-suppr/test12-add-data-member-0.1.suppr:
Likewise.
* tests/data/test-diff-suppr/test12-add-data-member-report-1.1.txt:
New test reference output.
* tests/data/test-diff-suppr/test13-suppr-through-pointer-0.1.suppr:
New test suppression specification.
* tests/data/test-diff-suppr/test13-suppr-through-pointer-report-1.1.txt:
New test reference output.
* tests/data/test-diff-suppr/test35-leaf-report-0.1.txt: Likewise.
* tests/data/test-diff-suppr/test35-leaf.1.suppr: New test
suppression specification.
* tests/data/Makefile.am: Add the new testing material to source
distribution.
* tests/data/test-diff-suppr/test11-add-data-member-1.suppr: Add
the has_size_change property to explicitly allow suppressing type
changes involving data member insertion even when the type size
changes.
* tests/data/test-diff-suppr/test11-add-data-member-0.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-2.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-3.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-4.suppr:
Likewise.
* tests/data/test-diff-suppr/test12-add-data-member-0.suppr:
Likewise.
* tests/data/test-diff-suppr/test13-suppr-through-pointer-0.suppr:
Likewise.
* tests/data/test-diff-suppr/test35-leaf.suppr: Likewise.
* tests/test-diff-suppr.cc (in_out_specs): Add the new test input
to the test harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 27 Jan 2023 14:58:37 +0000 (15:58 +0100)]
suppression: Factorize out is_data_member_offset_in_range
In preparation of subsequent changes, this patch factorizes a function
is_data_member_offset_in_range() out of
type_suppression::suppression(). This is useful to determine if a
data member offset is within an "offset range" expressed by the
type_suppression::insertion_range type.
This function is useful to implement the
offset_of_first_data_member_regexp and
offset_of_last_data_member_regexp properties to come in subsequent
patches.
Please note that is_data_member_offset_in_range works on data members
of unions and classes, not just on classes like what the original code
of inside type_suppression::suppresses_diff was doing.
This patch should not have any functional impact on the code.
* include/abg-fwd.h (get_last_data_member)
(get_next_data_member_offset): Declare functions.
* src/abg-ir.cc (get_next_data_member): Add an overload for
class_or_union and write the overload for class_or_union_sptr in
term of the former.
(get_last_data_member): Add overloads form class_or_union& and
class_or_union*. Write the overload for class_or_union_sptr in
terms of the one for class_or_union*.
(get_next_data_member_offset): Add an overload for
class_or_union* and write the overload for class_or_union_sptr in
terms of the former.
* include/abg-suppression.h
(type_suppression::insertion_range::eval_boundary): Take a
class_or_union* for the context, as opposed to a class_decl_sptr.
This makes this static function work for unions as well.
(is_data_member_offset_in_range): Declare new function.
* src/abg-suppression.cc (type_suppression::suppression_diff):
Factorize ...
(is_data_member_offset_in_range): ... this function out.
(type_suppression::insertion_range::eval_boundary): Adjust this to
make it take a class_or_union* rather than a class_decl_sptr.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Thu, 26 Jan 2023 16:49:41 +0000 (17:49 +0100)]
suppr: Support has_data_member and has_data_member_regexp properties
In the [supress_type] directive, this patch adds support for two new
properties:
* has_data_data_member = {foo, bar, blah}
Suppresses change reports involving a type which has data members
with names specified by the value of this property.
* has_data_member_regexp = some-regexp
Suppresses change reports involving a type which has data members
with names specified by the regular expression given as a value of
this property.
* include/abg-fwd.h (string_set_type): Define new typedef.
* src/abg-suppression-priv.h
* include/abg-suppression.h
(type_suppression::{get,set}_potential_data_member_names[_regex_str]):
Declare new data member.
(type_suppression::priv::{potential_data_members_,
potential_data_members_regex_str_,
potential_data_members_regex_}): Define new data members.
(type_suppression::priv::{get,set}_potential_data_member_names_regex):
Define new member functions.
* src/abg-suppression.cc
(type_suppression::{get,set}_potential_data_member_names): Define new
member functions.
(type_suppression::{get,set}_potential_data_member_names_regex_str):
Likewise.
(type_suppression::suppresses_diff): Implement suppression using
the new "has_data_member" and "has_data_member_regexp" properties.
(read_type_suppression): Support parsing the new "has_data_member"
and "has_data_member_regexp" properties of the type suppression
directive.
* tests/data/test-diff-suppr/has-data-member-[1-7].suppr: New
suppression specifications for test purposes.
* tests/data/test-diff-suppr/test-has-data-member-output-{1,2}.txt:
New reference test outputs.
* tests/data/test-diff-suppr/test-has-data-member-v{0,1}.cc:
Source code of new input binary tests.
* tests/data/test-diff-suppr/test-has-data-member-v{0,1}.o: New
binary test inputs.
* tests/data/Makefile.am: Add the test inputs below to source
distribution.
* tests/test-diff-suppr.cc (in_out_specs): Add the new test inputs
above to this test harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Thu, 26 Jan 2023 14:39:14 +0000 (15:39 +0100)]
ini: Fix parsing list property values
While looking at something else, I came across an issue in
read_context::read_list_property_value. This function requires
elements of a list property value (separated by a comma) to be on a
single line, for instance. This is because the code forgets to parse
white spaces after the comma.
Fixed thus.
* src/abg-ini.cc (read_context::read_list_property_value): Expect
white spaces after the comma.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Guillermo E. Martinez [Mon, 27 Feb 2023 18:45:11 +0000 (19:45 +0100)]
abipkgdiff: Fix kernel package detection when comparing kABIs
Using abipkgdiff to analyze kABIs from Oracle Linux packages with CTF
debug format, abipkgdiff is not able to identify kernel packages
because the naming of OL kernel packages differs from the naming used
on other RPM-based distributions.
As abipkgdiff fails to see that it's looking at a Linux kernel
package, the binaries are analyzed as user space binaries and that is
not what we want.
This patch addresses the issue by looking for the "vmlinuz" binary
inside the package to determine that it's a kernel package. In other
words, tools_utils::file_is_kernel_package is changed to look for
"vmlinuz" inside the package, rather than look for a particular
pattern in the package name of the package.
Additionally, when the kernel package contains CTF debug information,
the `vmlinux.ctfa' file is not necessarily shipped the debuginfo
package. This patch thus adjusts the search path of that file in that
case.
* include/abg-tools-utils.h (rpm_contains_file): Declare new
function.
* src/abg-ctf-reader.cc (ctf::reader::find_ctfa_file): Use
`find_file_under_dir' utility function to locate `vmlinux.ctfa'
file.
(ctf::reader::process_ctf_archive): Adjust dictionary name
according to module name, removing characters after dot.
* src/abg-tools-utils.cc (file_has_ctf_debug_info): Use
`find_file_under_dir' utility function to locate `vmlinux.ctfa'
file.
(rpm_contains_file): Define new function.
(file_is_kernel_package): Use the new `rpm_contains_file' to look
for the `vmlinuz' file inside the RPM package. For Debian
packages however, we don't keep looking at the naming pattern as
we don't yet have a deb_contains_file function. Also, this
function now takes the full path to the RPM.
(build_corpus_group_from_kernel_dist_under): for CTF, add the
`root' directory of the extracted package to the set of
directories under which we should look for debug info.
* tools/abipkgdiff.cc (maybe_handle_kabi_whitelist_pkg)
(create_maps_of_package_content, compare_prepared_package, main):
Adjust call to file_is_kernel_package as it now takes the full
path to the package.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 27 Feb 2023 17:38:26 +0000 (18:38 +0100)]
abipkgdiff: Emit error when no vmlinux is found in debug package
When given linux kernel packages to analyze using DWARF, the tool
expects the (uncompressed) vmlinux binary to be found in the debug
info package.
This patch emits an error message when no vmlinux binary is found in
the debug info package in that case.
* tools/abipkgdiff.cc (compare_prepared_linux_kernel_packages):
When no vmlinux binary is found in the debug info package, emit an
error message.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 27 Feb 2023 18:37:00 +0000 (19:37 +0100)]
abipkgdiff: Emit better logs in verbose mode
When invoked with --verbose, abipkgdiff emits some logs that could use
better clarity. Fixed thus.
* tools/abipkgdiff.cc (package::erase_extraction_directory): Say
explicitly what's DONE.
(extract_rpm): Add newline to log. Say explicitly what's DONE on
which package.
(extract_deb, extract_tar)
(erase_created_temporary_directories_parent, compare_to_self)
(create_maps_of_package_content): Say explicitly what's DONE on
which package.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Tue, 21 Feb 2023 15:22:19 +0000 (16:22 +0100)]
ctf-reader: Fix GCC 13 warnings
GCC 13 was recently introduced to Fedora Rawhide and it's complaining
(and rightly so) about some missing parenthesis. Fixed thus.
* src/abg-ctf-reader.cc (process_ctf_typedef)
(process_ctf_base_type, process_ctf_forward_type)
(process_ctf_struct_type, process_ctf_union_type)
(process_ctf_enum_type): Add missing parenthesis.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Tue, 21 Feb 2023 13:57:20 +0000 (14:57 +0100)]
ir: Add missing virtual methods overloads
Fedora Rawhide recently moved to GCC 13 and so its emitting new
warnings about libabigail's code base, and rightly so.
This patch thus adds some missing virtual method overloads that are
spotted by GCC 13.
* include/abg-ir.h (type_decl::operator!=): Declare missing
virtual overloads.
(array_type_def::subrange_type::operator!=): Likewise.
(template_decl::operator==): Likewise.
(type_tparameter::operator==): Likewise.
(class_decl::operator==): Likewise.
(union_decl::operator==): Likewise.
(member_class_template::operator==): Likewise.
* src/abg-ir.cc (type_decl::operator!=)
(array_type_def::subrange_type::operator!=)
(class_decl::operator==, member_class_template::operator==)
(union_decl::operator==, template_decl::operator==)
(type_tparameter::operator==, type_tparameter::operator==)
(template_tparameter::operator==): Define new virtual overloads.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 10 Feb 2023 12:18:19 +0000 (13:18 +0100)]
configure: Bump the CURRENT library number
The interface has changed in an incompatible way since the last
release as the vtable of fe_iface has changed in an incompatible, at
very least. So bump the LIBABIGAIL_SO_CURRENT version number to
reflect that.
* configure.ac: Bump LIBABIGAIL_SO_CURRENT to 2.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 3 Feb 2023 00:15:41 +0000 (01:15 +0100)]
PR30048 - wrong pretty representation of qualified pointers
A qualified type is textually represented as the following:
<qualifier> <underlying-type>
were "qualifier" is the qualifier carried by the qualified type and
"underlying-type" is the type being qualified.
In this case, the qualifier prefixes the textual representation of the
qualified type.
This is true if the underlying type of the qualified type is a
non-const, non-reference type. For instance:
const int;
But when the underlying type is a pointer, then the qualified type is
represented as:
int* const;
In that later case, the qualifier comes /after/ the textual
representation of the underlying type.
Now suppose the underlying type is itself a qualified type. In that
case, for a non-const underlying type, we'd have, e.g:
const volatile int;
where the qualifier precedes the qualified type (which is itself a
qualified type) /IF/ the ultimate underlying type (a.k.a the leaf
underlying type) is itself a non-const, non-reference type.
But if the ultimate underlying type is a pointer, a qualified type
with an underlying qualified type would be textually represented as,
e.g:
int* const volatile;
In other words, if the leaf type is a pointer, the qualifier suffixes
the textual representation the underlying qualified type.
Libabigail is failing to apply this later rule.
As the type name is used as a key to cache IR nodes of DIEs (among
other things), getting it wrong can lead to a bumpy ride down the
road.
Fixed thus.
* src/abg-dwarf-reader.cc
(die_is_pointer_array_or_reference_type): Rename
die_is_pointer_or_reference_type into this. This new name
reflects more what the function does as it tests if a DIE is for
pointer, an array or a reference.
(pointer_or_qual_die_of_anonymous_class_type): Adjust to use the
newly (and better) named die_is_pointer_array_or_reference_type.
(die_is_pointer_or_reference_type): Make this really test if a DIE
is for a pointer or a reference. Now the name matches what the
function does.
(die_peel_qualified): Define new function.
(die_qualified_type_name): When a qualified name Q has another
qualified name as its underlying type, it's important to know if
the leaf type is a pointer type or not to know how to construct
the name of Q. This change now peels the potential qualifiers
from the underlying type of the qualified type to see if the leaf
type is a pointer or not.
* src/abg-ir.cc (get_name_of_qualified_type): Likewise. Also, the
name of array types doesn't follow the same rule as for pointers
and references.
* tests/data/test-abidiff-exit/PR30048-test-report-0.txt: Add new
reference test output.
* tests/data/test-abidiff-exit/PR30048-test-2-report-1.txt:
Likewise.
* tests/data/test-abidiff-exit/PR30048-test-v{0,1}.c: Add source
code of binary input data.
* tests/data/test-abidiff-exit/PR30048-test-2-v{0,1}.cc: Likewise.
* tests/data/test-abidiff-exit/PR30048-test-v{0,1}.o: Add binary
input data.
* tests/data/test-abidiff-exit/PR30048-test-2-v{0,1}.o: Likewise.
* tests/data/Makefile.am: Add the new test material above to
source distribution.
* tests/test-abidiff-exit.cc (in_out_specs): Add the input
binaries to this test harness.
* tests/data/test-abidiff-exit/qualifier-typedef-array-report-1.txt:
Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 10 Feb 2023 09:50:27 +0000 (10:50 +0100)]
default-reporter: Fix source location in functions change report
While looking at something else, I realized that the default-reporter
mentions the source location of the new function in lieu of the old function
when reporting about a function change. Fixed thus.
* src/abg-default-reporter.cc (default_reporter::report): In the
overload of function_decl, use the source location of the old
function.
* tests/data/test-abicompat/test0-fn-changed-report-2.txt: Adjust.
* tests/data/test-abidiff-exit/qualifier-typedef-array-report-1.txt:
Likewise.
* tests/data/test-abidiff-exit/test-PR28316-report.txt: Likewise.
* tests/data/test-abidiff-exit/test-decl-enum-report.txt:
Likewise.
* tests/data/test-abidiff-exit/test-decl-struct-report.txt:
Likewise.
* tests/data/test-abidiff-exit/test-fun-param-report.txt:
Likewise.
* tests/data/test-abidiff-exit/test-headers-dirs/test-headers-dir-report-2.txt:
Likewise.
* tests/data/test-abidiff-exit/test-loc-with-locs-report.txt:
Likewise.
* tests/data/test-abidiff-exit/test-member-size-report0.txt:
Likewise.
* tests/data/test-abidiff-exit/test-rhbz2114909-report-1.txt:
Likewise.
* tests/data/test-diff-filter/libtest45-basic-type-change-report-0.txt:
Likewise.
* tests/data/test-diff-filter/test-PR24731-report-1.txt: Likewise.
* tests/data/test-diff-filter/test-PR25661-1-report-2.txt:
Likewise.
* tests/data/test-diff-filter/test-PR25661-2-report-2.txt:
Likewise.
* tests/data/test-diff-filter/test-PR25661-3-report-2.txt:
Likewise.
* tests/data/test-diff-filter/test-PR25661-4-report-2.txt:
Likewise.
* tests/data/test-diff-filter/test-PR25661-5-report-2.txt:
Likewise.
* tests/data/test-diff-filter/test-PR25661-6-report-3.txt:
Likewise.
* tests/data/test-diff-filter/test-PR25661-7-report-3.txt:
Likewise.
* tests/data/test-diff-filter/test-PR26739-2-report-0.txt:
Likewise.
* tests/data/test-diff-filter/test-PR29387-report.txt: Likewise.
* 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/test31-pr18535-libstdc++-report-1.txt:
Likewise.
* tests/data/test-diff-filter/test36-report-0.txt: Likewise.
* tests/data/test-diff-filter/test37-report-0.txt: Likewise.
* tests/data/test-diff-filter/test39/test39-report-0.txt:
Likewise.
* tests/data/test-diff-filter/test41-report-0.txt: Likewise.
* tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt:
Likewise.
* tests/data/test-diff-pkg/dirpkg-3-report-2.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.
* 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-1.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/symlink-dir-test1-report0.txt:
Likewise.
* tests/data/test-diff-pkg/tarpkg-1-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-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/PR28073/PR28073-output-2.txt:
Likewise.
* tests/data/test-diff-suppr/libtest48-soname-abixml-report-1.txt:
Likewise.
* tests/data/test-diff-suppr/test30-report-0.txt: Likewise.
* tests/data/test-diff-suppr/test41-enumerator-changes-report-0.txt:
Likewise.
* tests/data/test-diff-suppr/test42-negative-suppr-type-report-0.txt:
Likewise.
* tests/data/test-diff-suppr/test42-negative-suppr-type-report-1.txt:
Likewise.
* tests/data/test-diff-suppr/test6-fn-suppr-report-0-1.txt:
Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Tue, 31 Jan 2023 17:17:08 +0000 (18:17 +0100)]
{dwarf,elf_based}-reader,writer: Avoid duplicating corpora in corpus_group
It's been brought to my attention on IRC that running
abidw --linux-tree <kernel-build-tree>
would result in a corpus group that duplicates every single corpus in
the resulting abixml. Oops.
This is because both dwarf::reader::read_corpus() and
elf_based_reader::read_and_add_corpus_to_group() add the corpus to the
corpus_group, and yet, the later function calls the former. So the
corpus is added to the corpus_group twice.
This patch ensures that
elf_based_reader::read_and_add_corpus_to_group() is the only one to
add the corpus to the group. It also ensures that this happens before
the corpus is constructed from the debug info because that is useful
for sharing types among the various corpora. Otherwise, those types
are potentially duplicated in the IR of each corpus.
The patch also ensures the abixml writer enforces the fact that each
corpus is emitted only once.
* src/abg-dwarf-reader.cc (reader::read_debug_info_into_corpus):
Do not add the corpus to the group here ...
* src/abg-elf-based-reader.cc
(elf_based_reader::read_and_add_corpus_to_group): ... because it's
already added here. But then, let's add it here /before/ reading
type & symbols information into the corpus.
* src/abg-writer.cc (write_context::m_emitted_corpora_set): Add
new data member.
(write_context::{corpus_is_emitted, record_corpus_as_emitted}):
Define new member functions.
(write_corpus): Invoke the new
write_context::record_corpus_as_emitted here.
(write_corpus_group): Ensure that each corpus is emitted only
once.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Thu, 2 Feb 2023 10:30:16 +0000 (11:30 +0100)]
corpus: Handle empty symbol table cases
There can be cases where the symbol table associated with a given
corpus is empty. This patch handles those cases to avoid crashes.
* src/abg-corpus.cc (corpus::priv::{get_sorted_fun_symbols,
get_sorted_undefined_fun_symbols, get_sorted_var_symbols,
get_sorted_undefined_var_symbols}): If the symbol is null, then
return an empty vector of symbols.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Wed, 1 Feb 2023 16:46:53 +0000 (17:46 +0100)]
dwarf-reader: Remove unused code
* src/abg-dwarf-reader.cc (reader::{find_symbol_table_section,
lookup_native_elf_symbol_from_index}): Remove these dead
functions.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Wed, 1 Feb 2023 16:18:16 +0000 (17:18 +0100)]
fe-iface: Add missing virtual destructor
* include/abg-fe-iface.cc (fe_iface::~fe_iface): Make this
virtual.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Aleksei Vetrov [Sat, 28 Jan 2023 00:08:18 +0000 (00:08 +0000)]
symtab: fix getting CRC in relocatable modules
In ELF with ET_REL type symbol value holds not absolute but relative to
section value. This patch applies adjustment to the address, used in CRC
value extraction.
* src/abg-elf-helpers.cc (get_crc_for_symbol): Rename
crc_symbol_value to crc_symbol_address and adjust it for
relocatable ELF types.
Signed-off-by: Aleksei Vetrov <vvvvvv@google.com>
Dodji Seketeli [Fri, 13 Jan 2023 15:29:19 +0000 (16:29 +0100)]
Update CTF's ctf_dict_t detection
As ctf_dict_t can be an opaque type depending on the version of
ctf-api.h, using AC_CHECK_TYPE won't work to detect it because that
macro invokes sizeof(ctf_dict_t).
With this change, we don't require that ctf_dict_t be fully defined.
* configure.ac: Use AC_COMPILE_IFELSE to try and compile a code
snippet that doesn't need that ctf_dict_t be fully defined.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 9 Jan 2023 17:01:32 +0000 (18:01 +0100)]
Better detect suitable libctf version
On some el9 distros, the version of libctf installed might not have
all the necessary features for the libabigail CTF reader, leading to
compilation errors due to missing types from the ctf-api.h header
file. For instance, the ctf-api.h on some of those distros lacks the
definition of the type struct ctf_dict_t.
This patch adds a configure test for that struct and disables the CTF
support if that type is absent.
* configure.ac: If the "struct ctf_dict_t" type is not present in
the version of ctf-api.h that is present, then switch the support
of CTF off.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Giuliano Procida [Mon, 9 Jan 2023 09:20:44 +0000 (09:20 +0000)]
DWARF reader: avoid C++20 operator!= overload ambiguity
C++20 automatically generates overloads for certain comparison
operators based on others and this can create ambiguity with older
code. The type expr_result has various operators defined and comparing
expr_result != int becomes ambiguous.
This change just avoids this comparison by extracting the underlying
value, rather than making changes to the type itself. There should be
no change in behaviour and no tests are affected.
* (src/abg-dwarf-reader.cc) op_is_control_flow: In the
DW_OP_bra case, when testing the popped value, use the
expr_result's const_value explicitly.
Signed-off-by: Giuliano Procida <gprocida@google.com>
Dodji Seketeli [Sat, 7 Jan 2023 00:15:28 +0000 (01:15 +0100)]
btf-reader: Use abigail::ir::canonicalize_types to canonicalize types
In the btf::reader::canonicalize_types function, this patch now uses
the abigail::ir::canonicalize_types() function to canonicalize types,
just like the other front-ends. The advantage of this function is
that it can perform some sanity checking for type canonicalization
that might be useful for later debugging purposes.
* src/abg-btf-reader.cc (btf::reader::canonicalize_types): Use the
abigail::ir::canonicalize_types function from abg-ir-priv.h
file to canonicalize types.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 6 Jan 2023 20:40:44 +0000 (21:40 +0100)]
Update the copyright notice for the BTF reader
* include/abg-btf-reader.h: Update the copyright notice for 2023.
* src/abg-btf-reader.cc: Likewise.
* tests/test-read-btf.cc: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 31 Oct 2022 09:10:52 +0000 (10:10 +0100)]
Add support for BTF
This adds support for the BTF debug information format. It provides a
new BTF front-end which can be instantiated by the function
tools::create_best_elf_based_reader().
For now, the BTF front-end supports the basic types (integers,
pointers, qualified types, typedefs, struct and unions and function
pointers) for functions and variables as emitted for the C language by
GCC. It seems to be able to support the BTF debug information emitted
for the vmlinux kernel by the pahole tool as well.
When configured with the --enable-btf option, the WITH_BTF
pre-processor macro is defined, enabling the BTF support. That option
is turned on by default if the /usr/include/bpf/btf.h header is found
on the system. To disable this, one can use the --disable-btf option.
The abidw and abidiff programs have been adapted to use the BTF
front-end when provided with the '--btf' option, or if BTF debug
information is the only one present in the binary.
* configure.ac: If the header /usr/include/bpf/btf.h exists, then
define the WITH_BTF pre-processor macro, unless --disable-btf was
provided.
* doc/manuals/abidiff.rst: Document the new --btf option.
* doc/manuals/abidw.rst: Likewise.
* doc/manuals/kmidiff.rst: Likewise.
* doc/manuals/abipkgdiff.rst: Likewise.
* include/abg-btf-reader.h: New header file. Contains the
declaration of the new btf::reader class.
* src/abg-btf-reader.cc: New source file. Contains the
definitions of the new btf::reader class.
* include/Makefile.am: Add the new include/abg-btf-reader.h header
file to source distribution.
* include/abg-corpus.h (enum origin): Add a new BTF_ORIGIN
enumerator.
* include/abg-tools-utils.h (file_has_btf_debug_info): Declare new
function.
* src/abg-tools-utils.cc (file_has_btf_debug_info): Define new
function.
(create_best_elf_based_reader): Adapt to support BTF input. If
the user requested the BTF front-end, instantiate it. Otherwise,
if the input file has only BTF debug info, instantiate the BTF
front end.
* include/abg-elf-reader.h (elf::reader::find_btf_section):
Declare new member function.
(elf::reader::{function, variable}_symbol_is_exported): Add new
overloads.
* src/abg-elf-reader.cc (reader::priv::btf_section): New data
member.
(reader::find_btf_section): Define new member function.
* src/Makefile.am: Add the new abg-ctf-reader.cc file to source
distribution.
* tools/abidw.cc (options::use_btf): New data member.
(display_usage): Add a help string for the new --btf option.
(parse_command_line): Support the new --btf option.
(load_corpus_and_write_abixml): If the user asked to use the btf
front-end then use that one.
* tools/abidiff.cc (options::use_btf): New data member.
(options::options): Initialize it.
(display_usage):: Add a help string to the new --btf options.
(parse_command_line): Support the new --btf options.
(main): If the user asked to use the btf front-end, then use that
one.
* tools/abidw.cc (options::use_btf): New data member.
(options::options): Initialize it.
(parse_command_line): Add a help string to the new --btf options.
(load_corpus_and_write_abixml): If the user asked to use the btf
front-end, then use that one.
* tools/kmidiff.cc (options::use_btf): New data member.
(options::options): Initialize it.
(display_usage): Add a help string to the new --btf options.
(parse_command_line): Add a help string to the new --btf options.
(main): If the user asked to use the btf front-end, then use that
one.
* tools/abipkgdiff.cc (options::use_btf): New data member.
(options::options): Initialize it.
(display_usage): Add a help string to the new --btf options.
(parse_command_line): Add a help string to the new --btf options.
(compare, compare_to_self)
(compare_prepared_linux_kernel_packages): If the user asked to use
the btf front-end, then use that one.
* tests/data/test-read-btf/test{0,1}.o: New binary test input
file.
* tests/data/test-read-btf/test{0,1}.c: Source code of the binary
input file above.
* tests/data/test-read-btf/test{0,1}.o.abi: Reference ABIXML
output.
* tests/data/test-abidiff-exit/btf/test0-report-{1,2}.txt: New
test reference output.
* tests/data/test-abidiff-exit/btf/test0-v{0,1}.o: New binary test
input.
* tests/data/test-abidiff-exit/btf/test0-v{0,1}.c: The source
files of the binary inputs above.
* tests/test-read-btf.cc: New test file to run the btf/abixml
tests.
* tests/Makefile.am: Add the new test files to the source
distribution.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Thu, 5 Jan 2023 13:04:16 +0000 (14:04 +0100)]
configure: Enable the CTF front-end by default
The patch enables the CTF front-end by default, if the
ctf.h header file and its associated libctf.so shared library are
detected at configure time. That front-end can of course still be
disabled by using the --disable-ctf option.
* configure.ac: If --disable-ctf hasn't been passed, test for the
presence of ctf.h and then for libctf.so. If both are found then
enable the CTF front end.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Petr Pavlu [Tue, 3 Jan 2023 10:28:55 +0000 (11:28 +0100)]
abidiff: Fix handling of linux-kernel-mode
This fixes
08e76022 ("Support Linux Kernel ABI whitelist files") and
8fd02e0a ("Use the CTF reader by default when applicable").
The first commit removed handling of option --no-linux-kernel-mode. The
second one stopped passing opts.linux_kernel_mode down to the library
which also caused changing the default linux-kernel-mode of abidiff from
true to false. Both changes look unintentional as they are not mentioned
in either commit message.
Restore handling of the linux-kernel-mode in abidiff to match what is
described in doc/manuals/abidiff.rst and make it again consistent with
abidw.
* tools/abidiff.cc (parse_command_line): Recognize
--no-linux-kernel-mode.
(main): Pass opts.linux_kernel_mode down to the library.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 30 Dec 2022 23:43:51 +0000 (00:43 +0100)]
fix comparing array subrange DIEs
When looking at something else in the DWARF reader, I noticed that the
DIE comparison algorithm for DW_TAG_subprogram DIEs was not taking
into account non-set DW_AT_upper_bound attributes.
Fixed thus.
* src/abg-dwarf-reader.cc (compare_dies): For DW_TAG_subprogram,
non-set DW_AT_{lower,upper}_bound is not the same as when they are
set to zero.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 30 Dec 2022 23:58:51 +0000 (00:58 +0100)]
Bug 29811 - Better categorize harmless unknown array size changes
Let's compile the code snippet:
$ echo "unsigned int is_basic_table[];" | gcc -g -c -o test-v0.o -x c -
Let's see what abidw sees from it:
$ abidw test-v0.o | cat -n
1 <abi-corpus version='2.1' path='test-v0.o' architecture='elf-amd-x86_64'>
2 <elf-variable-symbols>
3 <elf-symbol name='is_basic_table' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
4 </elf-variable-symbols>
5 <abi-instr address-size='64' path='<stdin>' comp-dir-path='/home/dodji/git/libabigail/PR29811/prtests' language='LANG_C11'>
6 <type-decl name='unsigned int' size-in-bits='32' id='type-id-1'/>
7 <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='infinite' id='type-id-2'>
8 <subrange length='infinite' id='type-id-3'/>
9 </array-type-def>
10 <var-decl name='is_basic_table' type-id='type-id-2' mangled-name='is_basic_table' visibility='default' filepath='/home/dodji/git/libabigail/PR29811/prtests/<stdin>' line='1' column='1' elf-symbol-id='is_basic_table'/>
11 </abi-instr>
12 </abi-corpus>
See how the at line 7, the array type of ID 'type-id-2' has an unknown
size. This is the type of the 'is_basic_table' variable defined at
line 10. Note however that the symbol size of the is_basic_table
symbol is 4 bytes (32 bits).
Now, let's compile a similar code where the is_basic_table variable is
now initialized:
$ echo "unsigned int is_basic_table[] = {0};" | gcc -g -c -o test-v1.o -x c -
$ $ abidw test-v1.o | cat -n
1 <abi-corpus version='2.1' path='test-v1.o' architecture='elf-amd-x86_64'>
2 <elf-variable-symbols>
3 <elf-symbol name='is_basic_table' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
4 </elf-variable-symbols>
5 <abi-instr address-size='64' path='<stdin>' comp-dir-path='/home/dodji/git/libabigail/PR29811/prtests' language='LANG_C11'>
6 <type-decl name='unsigned int' size-in-bits='32' id='type-id-1'/>
7 <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='32' id='type-id-2'>
8 <subrange length='1' type-id='type-id-3' id='type-id-4'/>
9 </array-type-def>
10 <type-decl name='unsigned long int' size-in-bits='64' id='type-id-3'/>
11 <var-decl name='is_basic_table' type-id='type-id-2' mangled-name='is_basic_table' visibility='default' filepath='/home/dodji/git/libabigail/PR29811/prtests/<stdin>' line='1' column='1' elf-symbol-id='is_basic_table'/>
12 </abi-instr>
13 </abi-corpus>
Now, see like at line 7, the array type is now of 4 bytes (32 bits).
Note that the size of is_basic_table is still 32 bits.
Normally, abidiff-ing test-v0 and test-v1 should tell us that the two
versions of the is_basic_table variable are compatible because
fundamentally the structure and the size of the ELF symbol
is_basic_table hasn't changed, even if in the first case, it's an
array of unknown size. It's ELF symbol size was already 32 bits.
Here is what abidiff says:
$ abidiff test-v0.o test-v1.o
Functions changes summary: 0 Removed, 0 Changed, 0 Added function
Variables changes summary: 0 Removed, 1 Changed, 0 Added variable
1 Changed variable:
[C] 'unsigned int is_basic_table[]' was changed to 'unsigned int is_basic_table[1]' at <stdin>:1:1:
type of variable changed:
type name changed from 'unsigned int[]' to 'unsigned int[1]'
array type size changed from infinity to 32
array type subrange 1 changed length from infinity to 1
$
This is because the comparison engine doesn't recognize we are looking
at a type change that is harmless because the ELF size hasn't changed
and because this is an array of one dimension so fundamentally, the
"meaning" of the type of the array hasn't fundamentally changed for
ABI-related purposes.
This patch teaches the diff node categorizer to recognise that we are
in a case where the (one dimension) array of unknown size actually is
the type of an array which symbol size is 4 bytes. In the second
case, the one dimension array has a size of 4 bytes, just as its ELF
symbol size. The diff node categorizer then categorizes the diff node
into the existing category BENIGN_INFINITE_ARRAY_CHANGE_CATEGORY,
which is a harmless diff node category. Everything then falls into
place to filter the change out. Also, the patch adapts the diff
reporter to better describe this type of harmless array variable type
changes.
The output then becomes:
$ abidiff test-v0.o test-v1.o
Functions changes summary: 0 Removed, 0 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added variable
$
The change is filtered out.
To have details about the change that has been filtered out, one has
to use "--harmless" option:
$ abidiff --harmless test-v0.o test-v1.o
Functions changes summary: 0 Removed, 0 Changed, 0 Added function
Variables changes summary: 0 Removed, 1 Changed, 0 Added variable
1 Changed variable:
[C] 'unsigned int is_basic_table[]' was changed to 'unsigned int is_basic_table[1]' at <stdin>:1:1:
size of variable symbol ( 32 (in bits)) hasn't changed
but it does have a harmless type change
type of variable changed:
type name changed from 'unsigned int[]' to 'unsigned int[1]'
array type size changed from 'unknown' to 32
array type subrange 1 changed length from 'unknown' to 1
$
* include/abg-comp-filter.h
(is_var_1_dim_unknown_size_array_change): Declare new function.
* src/abg-comp-filter.cc (is_var_1_dim_unknown_size_array_change):
Define new function.
(has_benign_array_of_unknown_size_change): Rename
has_benign_infinite_array_change into this. Make this call the
new is_var_1_dim_unknown_size_array_change.
(categorize_harmless_diff_node): Adjust the call to
has_benign_infinite_array_change into the new
has_benign_array_of_unknown_size_change.
* include/abg-ir.h (var_equals_modulo_types): Declare new
function. Make it friend of class decl_base.
* src/abg-default-reporter.cc (default_reporter::report): In the
overload for var_diff, call the new
maybe_report_diff_for_variable.
* src/abg-ir.cc (var_equals_modulo_types): Factorize this out of
the equals() function for var_decl.
(equals): In the overload for var_decl, call the new
var_equals_modulo_types.
* src/abg-reporter-priv.h (maybe_report_diff_for_variable):
Declare new function.
* src/abg-reporter-priv.cc (maybe_report_diff_for_variable):
Define new function.
* tests/data/test-diff-filter/test-PR29811-0-report-0.txt: Add
new reference test output.
* tests/data/test-diff-filter/test-PR29811-0-report-1.txt:
Likewise.
* tests/data/test-diff-filter/test-PR29811-0-v{0,1}.o: Add new
binary test inputs.
* tests/data/test-diff-filter/test-PR29811-0-v{0,1}.c: Add source
code of the binary test inputs.
* tests/data/Makefile.am: Add new test input files above to source
distribution.
* tests/test-diff-filter.cc (in_out_specs): Add new tests to
harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 30 Dec 2022 21:15:46 +0000 (22:15 +0100)]
dwarf-reader: Bug 29811 - Support updating of variable type
Let's look at the source code reported at
https://sourceware.org/bugzilla/show_bug.cgi?id=29811:
extern unsigned int is_basic_table[];
unsigned int is_basic_table[] = {0};
Let's look at the DWARF output from GCC. The variable is_basic_table
is described by the DIE at offset 0x51:
[ 51] variable abbrev: 7
specification (ref4) [ 2f]
decl_line (data1) 3
decl_column (data1) 14
type (ref4) [ 3b]
location (exprloc)
[ 0] addr .bss+0 <is_basic_table>
The type of the variable is defined at the offset 0x3b:
[ 3b] array_type abbrev: 1
type (ref4) [ 29]
sibling (ref4) [ 4b]
[ 44] subrange_type abbrev: 6
type (ref4) [ 4b]
upper_bound (data1) 0
But then, we see that the DIE at 0x51 has a DW_AT_specification
attribute that refers to the DIE at offset 0x2f:
[ 2f] variable abbrev: 5
name (strp) "is_basic_table"
decl_file (data1) test-v2.c (1)
decl_line (data1) 1
decl_column (data1) 21
type (ref4) [ 1e]
external (flag_present) yes
declaration (flag_present) yes
That DIE at offset 0x2f represents the first external variable
declared in the source code. It's type is an array defined at offset
0x1e:
[ 1e] array_type abbrev: 1
type (ref4) [ 29]
sibling (ref4) [ 29]
[ 27] subrange_type abbrev: 4
This array has one dimension of 'unknown' size; this is because the
dimension is described by the DIE at offset 0x27 of kind
DW_TAG_subrange_type and has no DW_AT_upper_bound DIE.
But then, I said earlier, the real type of the is_basic_table variable
is the DIE at offset 0x3b, which is an array which single dimension
described by the DIE at offset 0x44 of kind DW_TAG_subrange_type with
a DW_AT_upper_bound attribute of value 0.
Let's see the output of abidw on this program, from the DWARF debug info:
1 <abi-corpus version='2.1' path='test-PR29811-unknown-size-array-dwarf-ctf-DWARF.o' architecture='elf-amd-x86_64'>
2 <elf-variable-symbols>
3 <elf-symbol name='is_basic_table' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
4 </elf-variable-symbols>
5 <abi-instr address-size='64' path='test-PR29811-unknown-size-array-dwarf-ctf.c' comp-dir-path='/home/dodji/git/libabigail/PR29811/prtests' language='LANG_C11'>
6 <type-decl name='unsigned int' size-in-bits='32' id='type-id-1'/>
7 <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='32' id='type-id-2'>
8 <subrange length='1' type-id='type-id-3' id='type-id-4'/>
9 </array-type-def>
10 <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='infinite' id='type-id-5'>
11 <subrange length='infinite' id='type-id-6'/>
12 </array-type-def>
13 <type-decl name='unsigned long int' size-in-bits='64' id='type-id-3'/>
14 <var-decl name='is_basic_table' type-id='type-id-5' mangled-name='is_basic_table' visibility='default' filepath='/home/dodji/git/libabigail/PR29811/prtests/test-PR29811-unknown-size-array-dwarf-ctf.c' line='10' column='1' elf-symbol-id='is_basic_table'/>
15 </abi-instr>
16 </abi-corpus>
The variable is_basic_table is described by the element at line 14:
14 <var-decl name='is_basic_table' type-id='type-id-5' mangled-name='is_basic_table' visibility='default' filepath='/home/dodji/git/libabigail/PR29811/prtests/test-PR29811-unknown-size-array-dwarf-ctf.c' line='10' column='1' elf-symbol-id='is_basic_table'/>
Its type has the ID 'type-id-5' which is defined at line 10:
10 <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='infinite' id='type-id-5'>
11 <subrange length='infinite' id='type-id-6'/>
12 </array-type-def>
Which has an unknown size.
But the, at line 7, there is another array type defined with a size of
32 bits:
7 <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='32' id='type-id-2'>
8 <subrange length='1' type-id='type-id-3' id='type-id-4'/>
9 </array-type-def>
So, libabigail links the is_basic_table variable to the wrong array
type.
This is because when the DWARF reader builds the internal
representation for the DW_TAG_variable DIE at offset 0x51, it first
builds it with the type (and the other properties such as the name for
instance) of the "declaration" DIE specified by the
DW_AT_specification attribute. But then, this DW_TAG_variable DIE has
its own type at offset 0x3b ; libabigail should update the internal
representation it just built to set the type to the one referred to at
offset 0x3b. It's that updating that is not being done. So the
variable wrongly points to the type of the "declaration" DIE at offset
0x2f.
This patch fixes build_var_decl to make it update the type of the
variable when necessary.
* include/abg-ir.h (var_decl::set_type): Declare new member
function.
* src/abg-ir.cc (var_decl::priv::set_type): Define new member
function.
(var_decl::set_type): Likewise.
* src/abg-dwarf-reader.cc (build_var_decl): In "updating mode",
update the type of the variable as well.
* tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-CTF.o:
Add new test binary input.
* tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-DWARF.o:
Likewise.
* tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-report.txt:
Add test reference output.
* tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf.c:
Add source code of the new test binary input.
* tests/data/Makefile.am: Add the new files above to source
distribution.
* tests/test-diff-filter.cc (in_out_specs): Add the input binaries
to the test harness.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Adjust.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 30 Dec 2022 22:25:06 +0000 (23:25 +0100)]
Don't use the "infinite" keyword for arrays of unknown size
In the ABIXML format and in diff reports, array dimensions of unknown
size are described with the "infinite" keyword. This is not explicit
enough. This patch uses the keyword "unknown" instead. Note that the
keyword "infinite" is still recognized by the ABIXML reader.
* src/abg-reader.cc (build_subrange_type, build_array_type_def):
Support the "unknown" keyword, as well as the "infinite" keyword.
* src/abg-reporter-priv.cc (report_size_and_alignment_changes):
Emit the "unknown" keyword, not the "infinity" one.
* src/abg-writer.cc (write_array_size_and_alignment): Likewise.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Adjust.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-annotate/test14-pr18893.so.abi: Likewise.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-annotate/test7.so.abi: Likewise.
* tests/data/test-read-ctf/test-array-size.abi: Likewise.
* tests/data/test-read-ctf/test-dynamic-array.o.abi: Likewise.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.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-libandroid.so.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/test14-pr18893.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/test7.so.abi: Likewise.
* tests/data/test-read-dwarf/test7.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
* tests/data/test-read-write/test25.xml: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Sun, 1 Jan 2023 17:14:26 +0000 (18:14 +0100)]
Update copyright year for 2023
Update the copyright years for 2023, using the script update-copyright.sh.
* update-copyright.sh: Update the copyright years in this script.
* include/abg-comp-filter.h: Update copyright year for 2023.
* include/abg-comparison.h: Likewise.
* include/abg-config.h: Likewise.
* include/abg-corpus.h: Likewise.
* include/abg-ctf-reader.h: Likewise.
* include/abg-cxx-compat.h: Likewise.
* include/abg-diff-utils.h: Likewise.
* include/abg-dwarf-reader.h: Likewise.
* include/abg-elf-based-reader.h: Likewise.
* include/abg-elf-reader.h: Likewise.
* include/abg-fe-iface.h: Likewise.
* include/abg-fwd.h: Likewise.
* include/abg-hash.h: Likewise.
* include/abg-ini.h: Likewise.
* include/abg-interned-str.h: Likewise.
* include/abg-ir.h: Likewise.
* include/abg-libxml-utils.h: Likewise.
* include/abg-reader.h: Likewise.
* include/abg-regex.h: Likewise.
* include/abg-reporter.h: Likewise.
* include/abg-sptr-utils.h: Likewise.
* include/abg-suppression.h: Likewise.
* include/abg-tools-utils.h: Likewise.
* include/abg-traverse.h: Likewise.
* include/abg-viz-common.h: Likewise.
* include/abg-viz-dot.h: Likewise.
* include/abg-viz-svg.h: Likewise.
* include/abg-workers.h: Likewise.
* include/abg-writer.h: Likewise.
* src/abg-comp-filter.cc: Likewise.
* src/abg-comparison-priv.h: Likewise.
* src/abg-comparison.cc: Likewise.
* src/abg-config.cc: Likewise.
* src/abg-corpus-priv.h: Likewise.
* src/abg-corpus.cc: Likewise.
* src/abg-ctf-reader.cc: Likewise.
* src/abg-default-reporter.cc: Likewise.
* src/abg-diff-utils.cc: Likewise.
* src/abg-dwarf-reader.cc: Likewise.
* src/abg-elf-based-reader.cc: Likewise.
* src/abg-elf-helpers.cc: Likewise.
* src/abg-elf-helpers.h: Likewise.
* src/abg-elf-reader.cc: Likewise.
* src/abg-fe-iface.cc: Likewise.
* src/abg-hash.cc: Likewise.
* src/abg-ini.cc: Likewise.
* src/abg-internal.h: Likewise.
* src/abg-ir-priv.h: Likewise.
* src/abg-ir.cc: Likewise.
* src/abg-leaf-reporter.cc: Likewise.
* src/abg-libxml-utils.cc: Likewise.
* src/abg-reader.cc: Likewise.
* src/abg-regex.cc: Likewise.
* src/abg-reporter-priv.cc: Likewise.
* src/abg-reporter-priv.h: Likewise.
* src/abg-suppression-priv.h: Likewise.
* src/abg-suppression.cc: Likewise.
* src/abg-symtab-reader.cc: Likewise.
* src/abg-symtab-reader.h: Likewise.
* src/abg-tools-utils.cc: Likewise.
* src/abg-traverse.cc: Likewise.
* src/abg-viz-common.cc: Likewise.
* src/abg-viz-dot.cc: Likewise.
* src/abg-viz-svg.cc: Likewise.
* src/abg-workers.cc: Likewise.
* src/abg-writer.cc: Likewise.
* tests/print-diff-tree.cc: Likewise.
* tests/test-abicompat.cc: Likewise.
* tests/test-abidiff-exit.cc: Likewise.
* tests/test-abidiff.cc: Likewise.
* tests/test-alt-dwarf-file.cc: Likewise.
* tests/test-core-diff.cc: Likewise.
* tests/test-cxx-compat.cc: Likewise.
* tests/test-diff-dwarf-abixml.cc: Likewise.
* tests/test-diff-dwarf.cc: Likewise.
* tests/test-diff-filter.cc: Likewise.
* tests/test-diff-pkg.cc: Likewise.
* tests/test-diff-suppr.cc: Likewise.
* tests/test-diff2.cc: Likewise.
* tests/test-dot.cc: Likewise.
* tests/test-elf-helpers.cc: Likewise.
* tests/test-ini.cc: Likewise.
* tests/test-ir-walker.cc: Likewise.
* tests/test-kmi-whitelist.cc: Likewise.
* tests/test-lookup-syms.cc: Likewise.
* tests/test-read-ctf.cc: Likewise.
* tests/test-read-dwarf.cc: Likewise.
* tests/test-read-write.cc: Likewise.
* tests/test-svg.cc: Likewise.
* tests/test-symtab-reader.cc: Likewise.
* tests/test-symtab.cc: Likewise.
* tests/test-tools-utils.cc: Likewise.
* tests/test-types-stability.cc: Likewise.
* tests/test-utils.cc: Likewise.
* tests/test-utils.h: Likewise.
* tools/abicompat.cc: Likewise.
* tools/abidiff.cc: Likewise.
* tools/abidw.cc: Likewise.
* tools/abilint.cc: Likewise.
* tools/abipkgdiff.cc: Likewise.
* tools/abisym.cc: Likewise.
* tools/binilint.cc: Likewise.
* tools/kmidiff.cc: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Wed, 28 Dec 2022 11:44:15 +0000 (12:44 +0100)]
ir: Add sanity checking to canonical type propagation confirmation
To understand the problem reported at
https://sourceware.org/bugzilla/show_bug.cgi?id=29934 where a type was
left non-canonicalized when analysing the binary
/usr/lib64/dovecot/libdovecot-sieve.so.0.0.0 from
https://vault.centos.org/7.6.1810/os/x86_64/Packages/dovecot-2.2.36-3.el7.x86_64.rpm
and
http://debuginfo.centos.org/7/x86_64/dovecot-debuginfo-2.2.36-3.el7.x86_64.rpm,
I had to add some sanity checking code to ensure that types that have
seen their propagated canonical cleared during the canonicalization
process are fully canonicalized at the end of the canonicalization
process.
In order to performg that sanity checking this patch tracks the set of
types which propagated canonical type has been cleared during the
canonicalization of a particular type. When a type with such a
cleared propagated canonical type is finally canonicalized, it is
removed from the set of tracked types. At the end of the
canonicalization process, the set of tracked types must be empty.
This sanity check is compiled in only if the WITH_DEBUG_CT_PROPAGATION
preprocessor macro is defined. That macro is defined if the
--enable-debug-ct-propagation configure switch is used.
* configure.ac: Add a new --enable-debug-ct-propagation configure
flag that defines the WITH_DEBUG_CT_PROPAGATION preprocessor
macro.
* src/abg-ir-priv.h
(environment::priv::types_with_cleared_propagated_ct_): Define new
data member for tracking types with cleared propagated canonical
type.
(environment::priv::types_with_cleared_propagated_ct): Add getter
and setter for the new data member above.
(environment::priv::{record_type_with_cleared_propagated_canonical_type,
erase_type_with_cleared_propagated_canonical_type}): Add
book-keeping functions for the set of types with cleared
propagated canonical type.
(type_base::priv::clear_propagated_canonical_type): Make this
return true if the propagated canonical type is cleared.
(environment::priv::clear_propagated_canonical_type): Define a new
function that takes a type_base* and clears its propagated
canonical type. This also adds the type to the set of tracked
types returned by
environment::priv::types_with_cleared_propagated_ct().
(environment::priv::{cancel_ct_propagation_for_types_dependant_on,
cancel_ct_propagation}): Call the new
environment::priv::clear_propagated_canonical_type() rather than
calling the now low-level
type_base::priv::clear_propagated_canonical_type().
(environment::priv::propagate_ct): Remove the type which just
gained a propagated canonical type from the set of tracked types
returned by environment::priv::types_with_cleared_propagated_ct.
(canonicalize_types): Define new function that canonicalizes all
the types of the system (passed in parameter) and performs sanity
checking to make sure all types with cleared propagated canonical
types have been canonicalized.
* include/abg-ir.h (string_type_base_sptr_map_type): Define new
typedef for an unordered_map<string, type_base_sptr>.
* src/abg-ir.cc (canonicalize): Remove the type which has just
been canonicalized from the set of tracked types returned by
environment::priv::types_with_cleared_propagated_ct.
* src/abg-ctf-reader.cc (reader::types_map): Use the new
string_type_base_sptr_map_type typedef for the type of this map.
(reader::canonicalize_all_types): Use the new function
abigail::ir::canonicalize_types to canonicalize the types of the
system and perform necessary sanity checking.
* src/abg-dwarf-reader.cc (reader::canonicalize_types_scheduled):
Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Tue, 27 Dec 2022 16:14:16 +0000 (17:14 +0100)]
ir: Bug 29934 - Fix propagated canonical type confirmation
When canonicalization a type T, it can happen that one subtype S of T
compares equal to a type S' where S' is already canonicalized. In
that case, we can deduce that the canonical type of S equals the
canonical type of S', even if we are currently in the process of
canonicalizing T.
In other words, the canonical type of S' is "propagated to S", in the
process of canonicalizing T.
This optimization is called "canonical type propagation" and is meant
to spead up the overall canonicalization process.
However, in some cases, the propagated canonical type can be
"cancelled" for the optimization to be correct. In those cases, the
propagated canonical type is set to nil.
When analysing the binary libdovecot-sieve.so from the problem
reported at https://sourceware.org/bugzilla/show_bug.cgi?id=29934, we
encounter a case where a function type's propagated type is
erroneously cancelled. That leaves the canonical type of that
function type not set and that later violates the assert
ABG_ASSERT(is_non_canonicalized_type(t)) in
abigail::ir::hash_as_canonical_type_or_constant.
I tracked this down to return_comparison_result which fails to confirm
a case of propagated canonical type and thus, some of them can end up
being erroneously cancelled.
Fixed thus.
* src/abg-ir.cc (return_comparison_result): A type whose canonical
type has been propagated must have its canonical type confirmed if
that type is not recursive and is not dependant on any recursive
type. In that case, the canonical type will never be cancelled.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Wed, 28 Dec 2022 11:51:38 +0000 (12:51 +0100)]
ir: misc cleanups
When looking at something else, I noticed some useless friend function
declaration. Namely, the "canonicalize()" function is declared friend
to the decl_base and scope_decl clases, which is now useless.
This patch removes those declarations.
* include/abg-ir.h (decl_base, scope_decl): Remove the declaration
of canonicalize() as friend to these classes.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Wed, 28 Dec 2022 17:15:09 +0000 (18:15 +0100)]
elf-reader: Don't free CTF resources too early
elf::reader::locate_alt_ctf_debug_info frees the memory for the
alternate CTF debug info too early, leading to some segmentation
violation down the road, when the rest of the code tries to access the
CTF section afterwards. Many thanks to the Valgrind tool and its
hackers for showing me this.
This patch thus keeps the file descriptor and ELF data structure of
the alternate CTF debug info around for the lifetime of the reader.
* src/abg-elf-reader.cc (reader::priv::{alt_ctf_fd,
alt_ctf_handle}): Add new data members.
(reader::priv::clear_alt_ctf_debug_info_data): Define new member
function.
(reader::priv::~priv): Call the new
priv::clear_alt_ctf_debug_info_data
(reader::priv::initialize): Likewise. Initialize the new
alt_ctf_handle and alt_ctf_fd data members.
(reader::priv::locate_alt_ctf_debug_info): Do not free the fd and
ELF resources early here. Store them in the new
reader::priv::alt_ctf_{fd,handle} instead.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Sat, 24 Dec 2022 14:29:15 +0000 (15:29 +0100)]
dwarf-reader: Bug 29932 - Handle function DIE as type as needed
When building the IR for a function type, the DWARF reader considers
the function DIE we are looking at as a type DIE.
In dwarf::reader::lookup_fn_type_from_die_repr_per_tu, the call to
get_die_pretty_representation doesn't enforce the fact that the DIE we
are looking at must be considered as a type. This is usually not a
problem because even if get_die_pretty_representation considers the
function DIE as a decl, the representation of a function and a
function type are almost the same.
In this particular case, we run into a function DIE that has an empty
name:
[ 51e54] subprogram abbrev: 18
external (flag_present) yes
name (strp) ""
decl_file (data1) catgets.c (1)
decl_line (data1) 89
prototyped (flag_present) yes
type (ref4) [ 51ac5]
low_pc (addr) +0x0000000000034cc0
high_pc (data8) 133 (+0x0000000000034d45)
frame_base (exprloc)
[ 0] call_frame_cfa
GNU_all_call_sites (flag_present) yes
sibling (ref4) [ 51edb]
Note that this is from the /lib64/libc-2.17.so from the
https://vault.centos.org/7.6.1810/os/x86_64/Packages/glibc-2.17-260.el7.x86_64.rpm
package, associated with the debuginfo package at http://debuginfo.centos.org/7/x86_64/glibc-debuginfo-2.17-260.el7.x86_64.rpm.
In that case, get_die_pretty_representation returns an empty string
because it doesn't expects a function decl with an empty name.
If we make dwarf::reader::lookup_fn_type_from_die_repr_per_tu
explicitly be in the context of a type by invoking
get_die_pretty_type_representation instead, the problem disapears as
the latter function treats the DIE as a function type DIE, so it
doesn't need its name.
Thus, this patch makes
dwarf::reader::lookup_fn_type_from_die_repr_per_tu invoke
get_die_pretty_type_representation instead.
* src/abg-dwarf-reader.cc
(reader::lookup_fn_type_from_die_repr_per_tu): Invoke
get_die_pretty_type_representation instead of
get_die_pretty_representation when looking at a function DIE
without a name.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Sat, 24 Dec 2022 13:21:06 +0000 (14:21 +0100)]
Bug 29934 - Handle buggy data members with empty names
When handling the changes between two ABI corpora, the diff graph
building pass chokes on a data member which seems to erroneously have
an empty name.
The steps to reproduce the issue are explained in the problem report
at https://sourceware.org/bugzilla/show_bug.cgi?id=29934.
The root cause of the problem is that the "struct mailbox" data
structure from the binary /usr/lib64/dovecot/libdovecot-storage.so.0
coming from the dovecot-2.2.36-3.el7.x86_64.rpm package contains a
data member that has an empty name. The source code of that data
structure can be browsed at
https://github.com/dovecot/core/blob/release-2.2.36/src/lib-storage/mail-storage-private.h
We see that the mailbox::storage data structure at line 352 ends up in
the DWARF debug info with an empty name. Let's look at the DWARF
dump as emitted by "eu-readelf --debug-dump=info" on the
/usr/lib/debug//usr/lib64/dovecot/libdovecot-storage.so.0.debug file
from the dovecot-debuginfo-2.2.36-3.el7.x86_64.rpm package.
A relevant DIE for the "struct mailbox" is the following:
[ 3e3e] structure_type abbrev: 9
name (strp) "mailbox"
byte_size (data2) 768
decl_file (data1) mail-storage-private.h (24)
decl_line (data2) 348
sibling (ref_udata) [ 41f9]
[ 3e4a] member abbrev: 59
name (strp) "name"
decl_file (data1) mail-storage-private.h (24)
decl_line (data2) 349
type (ref_addr) [ 84]
data_member_location (data1) 0
[ 3e57] member abbrev: 59
name (strp) "vname"
decl_file (data1) mail-storage-private.h (24)
decl_line (data2) 351
type (ref_addr) [ 84]
data_member_location (data1) 8
[ 3e64] member abbrev: 47
name (strp) ""
decl_file (data1) mail-storage-private.h (24)
decl_line (data2) 352
type (ref_udata) [ 3bee]
data_member_location (data1) 16
[...]
You can see here that the DW_TAG_member DIE at offset 0x3e64 has an
empty name. Its DW_AT_type attribute references the DIE at offset
0x3bee.
The DIE at offset 0x3bee is this one:
[ 3bee] pointer_type abbrev: 95
byte_size (data1) 8
type (ref_udata) [ 3a90]
[...]
It's a pointer to the type which DIE is at offset 0x3a90, which is:
[ 3a90] structure_type abbrev: 48
name (strp) "mail_storage"
byte_size (data2) 352
decl_file (data1) mail-storage-private.h (24)
decl_line (data1) 132
sibling (ref_udata) [ 3bee]
So, the data member of "struct mailbox" which has an empty name has a
type "pointer to struct mail_storage", aka "struct mail_storage*".
That indeed corresponds to the "storage" data member that we see at
line 352 of the mail-storage-private.h file, browsable at
https://github.com/dovecot/core/blob/release-2.2.36/src/lib-storage/mail-storage-private.h.
The fact that this data member has an empty name seems to me as a bug
of the DWARF emitter.
Libabigail ought to gently handle this bug instead of choking.
This patch assigns an artificial name to that empty data member to
handle this kind of cases in the future. The names looks like
"unnamed-@-<location>" where "location" is the location of the data
member.
Please note that there can be normal cases of anonymous data members
where the data member has an empty name. In those cases, the data
member must be of type union or struct. This is to describe the
"unnamed fields" C feature described at
https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html.
The buggy case we are seeing here is different from the "unnamed
field" case because the type of the anonymous data member is neither
struct nor union.
* src/abg-dwarf-reader.cc (die_is_anonymous_data_member): Define
new static function.
(die_member_offset): Move the declaration of this up so that it
can be used more generally.
(reader::build_name_for_buggy_anonymous_data_member): Define new
member function.
(add_or_update_class_type): Generate an artificial name for buggy
data members with empty names.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Guillermo E. Martinez [Thu, 22 Dec 2022 15:49:49 +0000 (09:49 -0600)]
ctf-reader: Fix missing initializer for member in test suite
With -Werror=missing-field-initializers on, the compiler chokes on CTF
test suite. Fixed thus.
* tests/test-read-ctf.cc (in_out_specs): Add initializer for
`option' field in test entry.
* tests/data/test-read-ctf/test-alias.o.abi: Adjust.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Guillermo E. Martinez [Wed, 21 Dec 2022 20:12:33 +0000 (14:12 -0600)]
ctf-front-end: Add test for alias symbols
This patch adds a new test case in the ctf-front-end test suite to
to test for alias symbols support.
* tests/data/test-read-ctf/test-alias.o: New binary test input file.
* tests/data/test-read-ctf/test-alias.o.abi: New exported abixml file.
* tests/data/Makefile.am: Add the new test input above
* tests/test-read-ctf.cc: Add the new test input above to the test
harness.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Xiaole He [Tue, 20 Dec 2022 13:06:34 +0000 (21:06 +0800)]
elf-reader: reclaim fd and mem before break
In elf::reader::priv::locate_alt_ctf_debug_info from
src/abg-elf-reader.cc, the resources held by the hdl and fd variables
aren't necessary released because the control-flow gets out of the
loop too early. This patch fixes the problem.
* src/abg-elf-reader.cc
(elf::reader::priv::locate_alt_ctf_debug_info): Reclaim fd and mem
before break. Also, do not try to locate the debug info it's
already been located.
Signed-off-by: Xiaole He <hexiaole@kylinos.cn>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Tue, 20 Dec 2022 15:06:33 +0000 (16:06 +0100)]
Bug 29901 - abidiff hangs when comparing libgs.so.10 with itself
This is a follow-up patch to this one:
88c6e080 Bug 29857 - Better detect comparison cycles in type graph
When looking at the type comparison stack, it looks like a type that
is on the left-hand side of the comparison can appear on the
right-hand side later, in the comparison stack. The cycle detection
algorithm doesn't take that scenario into account and so that cycle in
the graph of types being compared is not detected.
This patch takes that case into account.
* src/abg-ir-priv.h (environment::priv::comparison_started): Look
for each operand of the comparison in both the right-hand and
left-hand operand stacks.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 5 Dec 2022 15:32:24 +0000 (16:32 +0100)]
Update website documentation for 2.2
* mainpage.txt: Update for 2.2.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 2 Dec 2022 21:43:51 +0000 (22:43 +0100)]
configure: Bump version number to 2.3
* configure.ac: Bump version number to 2.3
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 2 Dec 2022 16:27:31 +0000 (17:27 +0100)]
ChangeLog: Update for 2.2 release
* ChangeLog: Update by doing "make update-changelog".
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 2 Dec 2022 16:25:43 +0000 (17:25 +0100)]
NEWS: Update for 2.2 release
* NEWS: Update for 2.2 release, with the output of
`git shortlog libabigail-2.1..HEAD`
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Petr Pavlu [Sun, 18 Dec 2022 19:33:40 +0000 (20:33 +0100)]
Fix de-initialization of elf::reader::priv
This fixes
7bd69830 ("Make Front Ends first class citizens").
Add a destructor for elf::reader::priv which releases any allocated alt
DWARF data and fix the initialize() method to fully reset the object.
The latter fixes a crash observed when handling multiple files. For
instance, when reading the Linux kernel tree, load_vmlinux_corpus()
processes vmlinux and all modules. Member dwarf_handle was never reset
after setting it for the first file which could later result in use of
invalid DWARF data in dwarf::reader::build_die_parent_maps().
* src/abg-elf-reader.cc (priv::~priv): Release alt debug
information.
(priv::initialize): Reset all members.
(priv::clear_alt_dwarf_debug_info_data): New helper function.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 19 Dec 2022 14:26:23 +0000 (15:26 +0100)]
ir: Cache more aggregate type comparison results
In the aftermath of
https://sourceware.org/bugzilla/show_bug.cgi?id=29857, I figured
caching comparison results at one place (in a macro) leads to better
maintainability. Also, using that macro in the equal() overload for
class_decl, union_decl and function_type results in slightly more
results of aggregate type comparison being cached during type
canonicalization. And that might speed things up.
When measuring the impact on the comparison of the bug mentionned
above, comparison that takes ~ 43 minutes, we gain 30secs with this
patch. We went from:
$ time ~/build/tools/abidiff --d1 ghostscript-9.07-31.el7.x86_64/usr/lib/debug --d2 ghostscript-9.52-5.oe1.x86_64/usr/lib/debug/ ghostscript-9.07-31.el7.x86_64/usr/lib64/libgs.so.9 ghostscript-9.52-5.oe1.x86_64/usr/lib64/libgs.so.9
Functions changes summary: 342 Removed, 940 Changed (2084 filtered out), 586 Added functions
Variables changes summary: 70 Removed, 432 Changed (333 filtered out), 81 Added variables
Function symbols changes summary: 2 Removed, 19 Added function symbols not referenced by debug info
Variable symbols changes summary: 0 Removed, 0 Added variable symbol not referenced by debug info
(...)
real 42m33,633s
user 41m54,699s
sys 0m1,381s
$
to:
$ time ~/build/tools/abidiff --d1 ghostscript-9.07-31.el7.x86_64/usr/lib/debug --d2 ghostscript-9.52-5.oe1.x86_64/usr/lib/debug/ ghostscript-9.07-31.el7.x86_64/usr/lib64/libgs.so.9 ghostscript-9.52-5.oe1.x86_64/usr/lib64/libgs.so.9
Functions changes summary: 342 Removed, 940 Changed (2084 filtered out), 586 Added functions
Variables changes summary: 70 Removed, 432 Changed (333 filtered out), 81 Added variables
Function symbols changes summary: 2 Removed, 19 Added function symbols not referenced by debug info
Variable symbols changes summary: 0 Removed, 0 Added variable symbol not referenced by debug info
(...)
real 42m2,364s
user 41m23,911s
sys 0m0,326s
$
* src/abg-ir.cc (CACHE_AND_RETURN_COMPARISON_RESULT): Define new
macro.
(equals): In the overloads for function_type, class_decl, and
union_decl use the new CACHE_AND_RETURN_COMPARISON_RESULT.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 19 Dec 2022 10:44:30 +0000 (11:44 +0100)]
Bug 29857 - Better detect comparison cycles in type graph
When comparing two aggregates A and B, it can happen that has a
sub-type of A is itself; similarly for B. This is a cycle in the type
graph, obviously. Also, A and B are said to be recursive types.
Structally comparing two recursive types (i.e, comparing them
member-wise) can lead to an endless loop.
So, during type canonicalization, comparison cycles must be detected
to avoid those endless loop.
This is currently done in the equals() overload for class_decl,
union_decl, class_or_union as well as function_type. Those are the
aggregate types that are subject to these cycles when comparing
recursive types.
Currently, detecting comparison cycles is based on
ir::environment::priv::mark_as_being_compared(),
ir::environment::priv::priv::unmark_as_being_compared(),
ir::environment::priv::comparison_started() considering the pair of
pointer to aggregate types being currently compared. If that pair
appears more than once in the stack of aggregates being compared a
cycle is detected.
But then, watching the stack of aggregates during the analsysis of the
/usr/lib64/libgs.so.9 binary from the ghostscript-9.52-5.oe1.x86_64
package from https://sourceware.org/bugzilla/show_bug.cgi?id=29857 it
appears that the stack of aggregates (structs) was growing endlessly,
when comparing the "struct gs_memory_s" type, and the same struct
gs_memory_s aggregate was appearing several times on the right-hand
side of the comparison stack. Clearly, this is a comparison cycle.
The gs_memory struct is appearing several times as the right hand side
operand in the stack of operands being compared, even though no pair
of operands being compared was present more than once.
This prompted me to use another heuristic to detect comparison cycles:
If the same aggregate type is appearing several times on either the
left or right hand side of the comparison stack then a cycle is
detected.
This fixes the cycle detection failure above and comparing the two
binaries completes in ~ 43 minutes on my laptop, with a non optimized
libabigail build.
* src/abg-ir-priv.h (class_set_type, fn_set_type): Define new typedefs.
* src/abg-ir.cc (environment::priv::{left_classes_being_compared_,
right_classes_being_compared_, left_fn_types_being_compared_,
right_fn_types_being_compared_}): Define new data members.
(environment::priv::{classes_being_compared_,
fn_types_being_compared_}): Erase data members.
(environment::priv::{dump_classes_being_compared,
dump_fn_types_being_compared}): Erase member functions.
(environment::priv::{mark_as_being_compared,
unmark_as_being_compared, comparison_started}): Change this to use
the left-hand-side and right-hand-side comparison stack introduced
above.
(dump_classes_being_compared, dump_fn_types_being_compared):
Remove functions.
Signed-off-by: Dodji Seketeli <dodji@seketeli.org>
Dodji Seketeli [Fri, 16 Dec 2022 14:52:14 +0000 (15:52 +0100)]
Bug 29857 - dwarf-reader: Resolve decl-only unions
When looking at https://sourceware.org/bugzilla/show_bug.cgi?id=29857
I noticed that decl-only unions where not resolved to their definition
union, unlike what is done for classes and enums.
At type canonicalization, a type A defined in a translation unit TU,
that depends on a decl-only union U will compare different from a type
A defined in a translation unit TU', that depends on the definition of
U, even though the types A should be equal.
This patch teaches the decl-only class resolver to also resolve
decl-only unions, as opposed to resolving just decl-only classes.
* include/abg-fwd.h (typedef classes_or_unions_type): Declare new
typedef.
(lookup_union_types): Declare new function.
* src/abg-dwarf-reader.cc (reader::decl_only_classes_map_): Change
the type of this from string_classes_map to
string_classes_or_unions_map.
(reader::declaration_only_classes): Return a
string_classes_or_unions_map, no more a string_classes_map.
(reader::{maybe_schedule_declaration_only_class_for_resolution,
is_decl_only_class_scheduled_for_resolution}): Handle
class_or_union, not just class_decl. This is a way to make this
handle unions as well as classes.
(get_opaque_version_of_type): Adjust.
* src/abg-ir.cc (lookup_union_types): Define new function.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Adjust.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 19 Dec 2022 13:52:05 +0000 (14:52 +0100)]
Bug 29857 - Don't pop comparison operands that haven't been pushed
While looking at bug
https://sourceware.org/bugzilla/show_bug.cgi?id=29857, I noticed a
crash that is happening due to the fact that the equal overload for
classes tries pop comparison operands that haven't been pushed to the
stack of comparison operands. Oops. Fixed thus.
* src/abg-ir.cc (equals): In the overload for class_or_union,
don't try to pop, operands that haven't been pushed.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 16 Dec 2022 16:43:34 +0000 (17:43 +0100)]
ir: Add a debug_comp_stack debugging function
When debugging type comparison, it can useful to see what the
comparison stack is. This patch adds the a debug_comp_stack to dump
the comparison stack as a string.
* include/abg-fwd.h (debug_comp_stack): Declare function.
* src/abg-ir.cc (debug_comp_vec, print_comp_stack): Define static
functions.
(debug_comp_stack): Define new function.
Signed-off-by: Dodji Seketeli <dodji@seketeli.org>
Dodji Seketeli [Fri, 16 Dec 2022 17:01:41 +0000 (18:01 +0100)]
ir: Improve get_debug_representation
* src/abg-ir.cc (get_debug_representation): Prefix
classes/structs/unions with the 'classes/struct/union' prefix.
Signed-off-by: Dodji Seketeli <dodji@seketeli.org>
Dodji Seketeli [Fri, 2 Dec 2022 15:59:13 +0000 (15:59 +0000)]
Bug 29829 - dwarf-reader: Allow DIEs to be in a lexical block
Normally, ABI-relevant DWARF DIEs (types and decls) should be at
namespace level. There are real-life cases where such a DIE might be
defined in a lexical block.
This patch teaches the DWARF reader to handle such cases.
* src/abg-dwarf-reader.cc (get_scope_for_die): Support
DW_TAG_lexical_block as DIE scope.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 2 Dec 2022 14:59:39 +0000 (14:59 +0000)]
dwarf-reader: Make die_peel_{qual_ptr,typedef} always set peeled type
When die_peel_{qual_ptr,typedef} don't actually peel anything, they
don't set the peeled type output argument. So callers expecting that
argument to be set can be surprised by the fact the peeled argument
might not be set.
* src/abg-dwarf-reader.cc (die_peel_qual_ptr, die_peel_typedef):
If the function returned true, then set the peeled argument even
if the function hasn't peeled anything.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Thu, 24 Nov 2022 16:26:35 +0000 (17:26 +0100)]
dwarf-reader: Avoid duplicating member functions
Depending on how the DWARF DIEs are encountered, it can happen that
the reader mistakenly adds the same member function several times.
This is because due to DIE canonicalization, when we encounter the
class DIE C' that is equivelent to a class C that we have already
encountered, C' is dropped on the floor and C is kept. But then, the
member functions of C' should not added to C, rather, if they are
already present in C, we shouldn't add them anymore. This is what
build_or_get_fn_decl_if_not_suppressed is supposed to do, but fails to
do in a subtle way.
This patch fixes it.
* src/abg-dwarf-reader.cc
(build_or_get_fn_decl_if_not_suppressed): Fix the code that is
supposed to avoid duplicating a member function.
* 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/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.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/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Wed, 23 Nov 2022 10:54:08 +0000 (11:54 +0100)]
dwarf-reader: Leverage ODR & DWZ
When DWARF debug info has been preprocessed with the DWZ tool, I think
we can assume that if two DIEs originating from the .gnu_debugaltlink
section have different offset, it means they are different, even if
they represent two types of the same nature and of the same name.
This is the whole point of DWZ.
When we process two DIEs originating from C++, it's possible "in
general" to assume that the One Definition Rule is in effect, meaning
that if two types of the same nature have the same name, they ought to
represent the same entity, meaning, they are the same type.
These two observations can lead to faster comparison of two aggregate
types of the same nature and of the same name.
This patch implements these two optimizations and use them by
default.
The first one is used by default on binaries that contains a
.gnu_debugaltlink section, which is the hint we use to detect that DWZ
was used to factorize DWARF DIEs.
The second of is used by default on DWARF DIEs originating from C++.
These optimizations can be de-activated on abidw and abidiff using the
--no-leverage-dwarf-factorization and --no-assume-odr-for-cplusplus.
* doc/manuals/abidiff.rst: Add documentation for
--no-leverage-dwarf-factorization and
--no-assume-odr-for-cplusplus
* doc/manuals/abidw.rst: Likewise.
* doc/manuals/abipkgdiff.rst: Likewise.
* include/abg-fe-iface.h (options::{leverage_dwarf_factorization,
assume_odr_for_cplusplus}): New data members.
* src/abg-dwarf-reader.cc (reader::leverage_dwarf_factorization_):
New data member.
(reader::leverage_dwarf_factorization): New accessor.
(compare_dies): If we are allowed to leverage the DWARF
factorization and if two type DIEs coming from the
.gnu_debugaltlink DWARF section have different offset, then they
are different. Also, if we are allowed to assume ODR, use it to
speed up class/struct/unions comparisons.
* tools/abidiff.cc (options::{assume_odr_for_cplusplus,
leverage_dwarf_factorization}): Define new data members.
(options::options): Initialize them.
(display_usage): Add new help strings for
--no-leverage-dwarf-factorization and
--no-assume-odr-for-cplusplus.
(parse_command_line): Parse these new options.
(set_generic_options): New function.
(main): Use the new set_generic_options function.
* tools/abidw.cc (options::{assume_odr_for_cplusplus,
leverage_dwarf_factorization}): Define new data members.
(options::options): Initialize them.
(display_usage): Add new help strings for
--no-leverage-dwarf-factorization and
--no-assume-odr-for-cplusplus.
(parse_command_line): Parse these new options.
(set_generic_options): New function.
(load_corpus_and_write_abixml): Use the new set_generic_options
function.
* tools/abipkgdiff.cc (options::{assume_odr_for_cplusplus,
leverage_dwarf_factorization}): Define new data members.
(options::options): Initialize them.
(display_usage): Add new help strings for
--no-leverage-dwarf-factorization and
--no-assume-odr-for-cplusplus.
(parse_command_line): Parse these new options.
(set_generic_options): New function.
(compare): Use it.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 25 Nov 2022 19:37:00 +0000 (20:37 +0100)]
Fix spurious deleted/added virtual destructor change report
While looking at something else, I noticed this spurious change
report:
1 member function deletion:
'method virtual tbb::internal::concurrent_queue_base_v3::~concurrent_queue_base_v3(int)' at concurrent_queue.cpp:361:1
1 member function insertion:
'method virtual tbb::internal::concurrent_queue_base_v3::~concurrent_queue_base_v3(int)' at concurrent_queue.cpp:370:1
This is when running tests/runtestdiffpkg and the result is in
tests/data/test-diff-pkg/tbb-4.1-9.
20130314.fc22.x86_64--tbb-4.3-3.
20141204.fc23.x86_64-report-0.txt.
To understand what is going on, one need to refer to the definitions
of the C++ ABI at
https://itanium-cxx-abi.github.io/cxx-abi/abi.html#definitions.
When a class has a virtual destructor, historically, there might be 3
different and co-existing actual implementations of the destructors:
the D0, D1 and D2 destructors. There might even be a D3 destructor,
actually.
In https://github.com/itanium-cxx-abi/cxx-abi/issues/73, one can see
that there might be new D4 and D5 destructors. Each one of them might
replace the set of the D0,D1,D2 destructors.
So, in a new version of a binary, the virtual D4 destructor might
replace the previous ones, without it being an ABI issue.
The switch to the D4 virtual destructor is what libabigail is naively
reporting in the change report above.
This patch detects this kind of changes in the pipeline when the edit
script from the diff2 algorithm is interpreted for virtual member
functions and drops it on the floor.
* src/abg-comparison.cc (find_virtual_dtor_in_map): Define new
static function.
(class_diff::ensure_lookup_tables_populated): If a virtual
destructor is removed from the old binary version but is added to
the new one (but through a different name), let's assume the
virtual destructor is still there so there is no ABI issue
from that point of view.
* tests/data/test-diff-pkg/tbb-4.1-9.
20130314.fc22.x86_64--tbb-4.3-3.
20141204.fc23.x86_64-report-0.txt: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Guillermo E. Martinez via Libabigail [Thu, 17 Nov 2022 03:43:05 +0000 (21:43 -0600)]
ctf-reader: Fix array size representation
A variable length array (VLA), or a flexible array member (with its
size set to zero to recognize such cases) is represented in the
libabigail IR as an array with "non-finite" length. This is a way to
say that the length of the array is not statically known.
The ABIXML array-type-def element looks like:
<array-type-def dimensions='1' ... size-in-bits='infinite' ...>
<subrange length='infinite' type-id='type-id-3' .../>
</array-type-def>
The patch teaches the ctf-reader to correctly set the size of the
array for VLAs.
* src/abg-ctf-reader.cc (build_array_ctf_range): Use
* tests/data/Makefile.am: Add new test.
`upper_bound' and number of elements to indicate infinite
array size.
* tests/data/test-read-ctf/test-array-size.abi: New test.
* tests/data/test-read-ctf/test-array-size.c: Likewise.
* tests/data/test-read-ctf/test-array-size.o: Likewise.
* tests/test-read-ctf.cc: Update testsuite.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Guillermo E. Martinez [Thu, 17 Nov 2022 03:43:04 +0000 (21:43 -0600)]
ctf-reader: Fix representation of multidimensional arrays
To build an IR for multidimensional array the CTF front-end iterates
over the element types recursively.
So, consider the array definition:
char a[2][3][4][5];
It's represented as
'char[2][3][4] a[5]'
instead of:
'char a[2][3][4][5]'
It always considers multidimensional arrays as unidimensional creating
a `array-type-def' node for each dimension:
<array-type-def dimensions='1' type-id='type-id-1' ... >
<subrange length='2' type-id='type-id-3' id='type-id-4'/>
</array-type-def>
<array-type-def dimensions='1' type-id='type-id-2' ... >
<subrange length='3' type-id='type-id-3' id='type-id-6'/>
</array-type-def>
...
Instead of:
<array-type-def dimensions='4' type-id='type-id-1' ... >
<subrange length='2' type-id='type-id-3' id='type-id-4'/>
<subrange length='3' type-id='type-id-3' id='type-id-5'/>
...
</array-type-def>
Fixed thus.
* src/abg-ctf-reader.cc (+build_array_ctf_range): New definition.
* tests/data/Makefile.am: Add new testcase.
* tests/data/test-read-ctf/test-array-mdimension.abi: New testcase.
* tests/data/test-read-ctf/test-array-mdimension.c: Likewise.
* tests/data/test-read-ctf/test-array-mdimension.o: Likewise.
* tests/data/test-read-ctf/test9.o.abi: Adjust.
* tests/test-read-ctf.cc: Update testsuite.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Guillermo E. Martinez [Thu, 17 Nov 2022 03:43:03 +0000 (21:43 -0600)]
ctf-reader: Strip qualification from a qualified array type
Sometimes, GCC emits some redundant const qualifiers around arrays.
For instance, consider this function:
$ cat -n test.c
1 const char a[32];
2
3 char
4 foo()
5 {
6 return a[0];
7 }
$
Notice how at line 1, the type of the variable 'a' is "array of const
char".
Let's compile the function and emit CTF debug info:
$ gcc -gctf -c test.c
$
Let's see what IR libabigail emits from the CTF information:
$ abidw --ctf --annotate test.o | cat -n
1 <abi-corpus version='2.1' path='test.o' architecture='elf-amd-x86_64'>
2 <elf-function-symbols>
3 <!-- foo -->
4 <elf-symbol name='foo' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
5 </elf-function-symbols>
6 <elf-variable-symbols>
7 <!-- signed char -->
8 <elf-symbol name='a' size='32' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
9 </elf-variable-symbols>
10 <abi-instr address-size='64' language='LANG_C'>
11 <!-- char -->
12 <type-decl name='char' size-in-bits='8' alignment-in-bits='8' id='type-id-1'/>
13 <!-- const char[32] -->
14 <array-type-def dimensions='1' type-id='type-id-2' size-in-bits='256' alignment-in-bits='8' id='type-id-3'>
15 <!-- <anonymous range>[32] -->
16 <subrange length='32' type-id='type-id-4' id='type-id-5'/>
17 </array-type-def>
18 <!-- unsigned long int -->
19 <type-decl name='unsigned long int' size-in-bits='64' alignment-in-bits='64' id='type-id-4'/>
20 <!-- const char -->
21 <qualified-type-def type-id='type-id-1' const='yes' id='type-id-2'/>
22 <!-- const char[32] const -->
23 <qualified-type-def type-id='type-id-3' const='yes' id='type-id-6'/>
24 <!-- const char[32] const a -->
25 <var-decl name='a' type-id='type-id-6' mangled-name='a' visibility='default' elf-symbol-id='a'/>
26 <!-- char foo() -->
27 <function-decl name='foo' visibility='default' binding='global' size-in-bits='64' alignment-in-bits='8' elf-symbol-id='foo'>
28 <!-- char -->
29 <return type-id='type-id-1'/>
30 </function-decl>
31 </abi-instr>
32 </abi-corpus>
$
Notice how at line 25, the variable 'a' is described as having the
type which ID is 'type-id-6' defined at line 23. It's a "const array
of const char".
GCC has thus added a redundant "const" qualifier to the array.
The C language specification in paragraph [6.7.3]/8 says:
If the specification of an array type includes any type
qualifiers, the element type is so- qualified, not the array type.
This means that a "const array of char" is the same as an "array of
const char". So a "const array of const char" is the same an "array
of const char".
This patch performs that removal of redundant qualifier.
* src/abg-ctf-reader.cc (maybe_strip_qualification): New
definition.
(process_ctf_qualified_type): Strip redundant qualifiers.
* tests/data/test-read-ctf/test-const-array.abi: New test.
* tests/data/test-read-ctf/test-const-array.c: Likewise.
* tests/data/test-read-ctf/test-const-array.o: Likewise.
* tests/Makefile.am: Add the new test material to source
distribution.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Tue, 29 Nov 2022 12:42:43 +0000 (13:42 +0100)]
test-read-ctf: Update tests for fixing size and name for underlying types
When applying the patch
"
217f579b ctf-reader: Fix size and name for underlying types"
the testing parts felt through the cracks. Oops.
This patch adds back the testing parts of the original patch.
Here is the link to the initial post:
https://inbox.sourceware.org/libabigail/871qpmou3k.fsf@seketeli.org/T/#m5f37b419e580a2a4ea9d91a9394382d886358809
* src/abg-ctf-reader.cc (process_ctf_{base_type,enum_type}):
Look at ctf refence type to build the underlying type if present.
* tests/data/Makefile.am: New test cases.
* tests/data/test-read-ctf/PR27700/test-PR27700.abi: New test input.
* tests/data/test-read-ctf/test-bitfield-enum.abi: Likewise.
* tests/data/test-read-ctf/test-bitfield-enum.c: Likewise.
* tests/data/test-read-ctf/test-bitfield-enum.o: Likewise.
* tests/data/test-read-ctf/test-bitfield.abi: Likewise.
* tests/data/test-read-ctf/test-bitfield.c: Likewise.
* tests/data/test-read-ctf/test-bitfield.o: Likewise.
* tests/data/test-read-ctf/test-enum-many.o.hash.abi: Adjust.
* 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/test0.abi: 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/test5.o.abi: Likewise.
* tests/test-read-ctf.cc: Update test suite.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
Guillermo E. Martinez [Thu, 17 Nov 2022 03:43:02 +0000 (21:43 -0600)]
ctf-reader: Fix size and name for underlying types
This patch fixes an incorrect representation in size and name of the
underlying type of enums as well as underlying types of bitfield data
members types.
For instance, consider this struct.
struct foo
{
unsigned bar : 2;
unsigned baz : 1;
};
The data members bar and baz have an underlying type that is "unsigned
int". Yet, the CTF front-end represents the underlying type of these
data members as:
<type-decl name='' is-anonymous='yes' size-in-bits='2' id='type-id-1'/>
The name property is empty, and it should be "unsigned int".
The size in bit is '2', but it should be the size of the underlying
"unsigned int", in bits, which is 32.
In other words, the underlying type of bar and baz should be:
<type-decl name='unsigned int' size-in-bits='32' id='type-id-4'/>
Note that today, libabigail doesn't represent the bitfield properties
of the data member. Those bitfield properties are properties of the
data member, not of their type. This is a known "Not Yet Implemented"
feature request that has been filed upstream at
https://sourceware.org/bugzilla/show_bug.cgi?id=27334.
Similarly, the underlying type of enums is not properly represented by
the CTF front-end.
Fixed thus.
* src/abg-ctf-reader.cc (process_ctf_{base_type,enum_type}):
Look at ctf refence type to build the underlying type if present.
* tests/data/Makefile.am: New test cases.
* tests/data/test-read-ctf/PR27700/test-PR27700.abi: New test input.
* tests/data/test-read-ctf/test-bitfield-enum.abi: Likewise.
* tests/data/test-read-ctf/test-bitfield-enum.c: Likewise.
* tests/data/test-read-ctf/test-bitfield-enum.o: Likewise.
* tests/data/test-read-ctf/test-bitfield.abi: Likewise.
* tests/data/test-read-ctf/test-bitfield.c: Likewise.
* tests/data/test-read-ctf/test-bitfield.o: Likewise.
* tests/data/test-read-ctf/test-enum-many.o.hash.abi: Adjust.
* 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/test0.abi: 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/test5.o.abi: Likewise.
* tests/test-read-ctf.cc: Update test suite.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Guillermo E. Martinez [Thu, 17 Nov 2022 03:43:01 +0000 (21:43 -0600)]
ctf-reader: Set alignment-in-bits property to 0
When comparing the IR generated with the CTF front-end against the one
generated with the DWARF front-end, the report shows changes in type
alignments:
1 Changed variable:
[C] 'int a' was changed at test-abi.c:8:1:
type of variable changed:
type size hasn't changed
type alignment changed from 32 to 0
For the sake of consistency, this patch makes the CTF front-end set
the alignment to `0' for base types, function types and struct types
similarly to what the DWARF front-end does.
* src/abg-ctf-reader.cc (process_ctf_base_type)
(process_ctf_function_type, process_ctf_struct_type): Adjust
`align_in_bits' argument to `0'.
* tests/data/test-diff-pkg-ctf/dirpkg-3-report-1.txt: Adjust test.
* tests/data/test-diff-pkg-ctf/dirpkg-3-report-2.txt: Likewise.
* tests/data/test-diff-pkg-ctf/gmp-6.x.x86_64-report-0.txt: Likewise.
* tests/data/test-read-ctf/PR27700/test-PR27700.abi: Likewise.
* 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.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-anonymous-fields.o.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-fallback.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-linux-module.abi: Likewise.
* tests/data/test-read-ctf/test-linux-module.abi: Likewise.
* tests/data/test-read-ctf/test-list-struct.abi: Likewise.
* tests/data/test-read-ctf/test0.abi: 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>
Guillermo E. Martinez [Tue, 22 Nov 2022 16:00:50 +0000 (10:00 -0600)]
Use the CTF reader by default when applicable
At the moment, the tools abidw, abidiff, abipkgdiff and kmidiff all
use the DWARF front-end by default. When the "--ctf" option is added
to the command line, they use the CTF front-end.
This patch changes that behaviour in the way described below.
If the "--ctf" command line option is passed to the tool and if the
binary to analyze contains CTF debug info, then the CTF front-end is
used.
If the binary contains ONLY CTF debug info, then the CTF front-end is
used, even if no "--ctf" option was provided.
In all the other cases, the DWARF front-end is used.
Of course, the CTF front-end is not used at all if the CTF
functionality hasn't been enabled at configure time.
This new behaviour is effective for user space and Linux kernel
binaries.
* doc/manuals/abidiff.rst: Adjust.
* doc/manuals/abidw.rst: Likewise.
* doc/manuals/abipkgdiff.rst: Likewise.
* doc/manuals/kmidiff.rst: Likewise.
* include/abg-elf-based-reader.h (initialize): Add member function.
* include/abg-elf-reader.h (has_{dwarf,ctf}_debug_info): Add predicate
functions.
* include/abg-tools-utils.h (create_best_elf_based_reader): Add arguments.
* src/abg-ctf-reader.cc (process_ctf_typedef, process_ctf_base_type)
(process_ctf_function_type, process_ctf_sou_members, process_ctf_forward_type)
(process_ctf_struct_type, process_ctf_union_type, process_ctf_array_type)
(process_ctf_qualified_type, process_ctf_pointer_type, process_ctf_enum_type):
Remove arguments. Using getters to access required information instead.
(reader::cur_tu_): Add data member.
(initialize): Add arguments.
(cur_transl_unit): Add {get,set)ter.
(slurp_elf_info): Clear `STATUS_DEBUG_INFO_NOT_FOUND' if corpus is
`LINUX_KERNEL_BINARY_ORIGIN'.
(reader::lookup_type): Remove.
(reader::build_type): New member function.
* src/abg-elf-reader.cc (reader::reader): Locate ctf debug info
from binary file.
(reader::reader): Reset base `fe_iface' constructor.
(reader::has_{dwarf,ctf}_debug_info): New definitions.
(reader::read_corpus): Set `STATUS_DEBUG_INFO_NOT_FOUND' according
to corpus::origin.
* src/abg-tools-utils.cc (dir_contains_ctf_archive): Define new member.
(file_has_ctf_debug_info): Looks for kernel ctf debug information archive.
(maybe_load_vmlinux_{dwarf,ctf}_corpus): Remove.
(load_vmlinux_corpus): Define function to load IR from kernel
regardless of the corpus::origin.
(build_corpus_group_from_kernel_dist_under): Use
create_best_elf_based_reader to select the front-end.
(create_best_elf_based_reader): Adjust to allow fallback behaviour
for different front-ends.
* tests/data/Makefile.am: Add tests.
* tests/data/test-diff-pkg-ctf/dirpkg-3-report-2.txt: Adjust.
* tests/data/test-read-ctf/test-fallback.abi: New test case.
* tests/data/test-read-ctf/test-fallback.c: Likewise.
* tests/data/test-read-ctf/test-fallback.o: Likewise.
* tests/data/test-read-dwarf/test-fallback.abi: Likewise.
* tests/data/test-read-dwarf/test-fallback.c: Likewise.
* tests/data/test-read-dwarf/test-fallback.o: Likewise.
* tests/test-diff-pkg.cc: Adjust.
* tests/test-read-common.cc (test_task::run_abidw): Use the
`options:option' field.
* tests/test-read-common.h (InOutSpec): Add new member.
* tests/test-read-ctf.cc (in_out_specs): Add option field to test
suite. Add new test case.
* tests/test-read-dwarf.cc: Likewise.
* tools/abidiff.cc (main): Use create_best_elf_based_reader.
* tools/abidw.cc: Likewise.
* tools/abipkgdiff.cc: Likewise.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Tue, 15 Nov 2022 16:26:37 +0000 (17:26 +0100)]
Make Front Ends first class citizens
This patch is a reorganization of the code to better support the need
for having several different front-ends.
In the libabigail pipeline of operation, a front-end is the part of
the pipeline that analyses the input file. For instance, to analyse
an ELF file, there is going to be one front-end. To analyse an ABIXML
file, there is going to be another front-end.
The middle-end is the part of the pipeline that interacts with the
internal representation of ABIs. The middle-end knows how to analyse,
compare ABI corpora provide an internal representation of the
comparison result and analyse it as well.
The back-end would be the part of the front-end that knows how to
serialize internal representations of ABIs and ABI comparison results.
One could thus imagine a front-end that understands the DWARF debug
info format embedded in an ELF file. Another front-end would be
dedicated to the CTF debug info format, and so on.
Front-ends can share capabilities. For instance, DWARF and CTF
front-ends are ELF based front end. As such, they share capabilities
to understand the ELF format. They don't share much with the ABIXML
front-end, however, as it's based on XML, which has almost nothing in
common with ELF.
To support this organization of concepts, this patch introduces a new
hierarchy of types in the form of C++ classes.
All front-ends implements the "Front End Interface". As such, they
all inherit the abigail::fe_iface class.
That class provides properties and behaviours that are shared by all
front-ends that libabigail supports today. Namely, that class
provides access to some of the options that are relevant to operating
the front-end, to the ABI corpus or corpus group being constructed and
to the suppression specifications that are considered. It also
provides an abstract interface to perform the actual loading of the
ABI corpus. That abstract interface has to be implemented by every
single concrete front-end that is provided in libabigail.
Then, there is the "ELF Reader" front-end. Its class name is
abigail::elf::reader. It inherits the abigail::fe_iface class and
implements the fe_iface::load_corpus() so that the ELF properties of
the ELF file be loaded and exposed in the ABI corpus as returned by
the fe_iface::corpus() accessor. This ELF reader front-end also
provides lots of capabilities that are specific to accessing ELF
content.
Then, there is a common base class for ELF-based front-ends to come,
named abigail::elf_based_reader, which inherits the abigail::elf::reader
class. The purpose of this base class is to provide common properties
and behaviours that are necessary to implement things like a DWARF or
a CTF front-end, or any other front-end to support an ELF-based debug
info format.
Then, there is a CTF front-end which class is named
abigail::ctf::reader. It inherits the abigail::elf_based_reader class
and implements the fe_iface::load_corpus() interface to load and
analyse the CTF-specific properties of the ELF file. To do this,
abigail::ctf::reader::load_corpus() re-uses the abigail::elf::load_corpus() member
function to load the generic ELF parts of the ABI corpus. This reader
then constructs the internal representation of the ABI corpus and
passes it to the middle-end for further analysis.
Then, there is a DWARF front-end which class is named
abigail::dwarf::reader. It inherits the abigail::elf_based_reader
class and implements the fe_iface::load_corpus() interface to load and
analyse the DWARF-specific properties of the ELF file. To do this,
abigail::dwarf::reader re-uses the abigail::elf::load_corpus() member
function to load the generic ELF parts of the ABI corpus, just like
what the CTF front-end does. And then, just like the CTF front-end,
this reader then constructs the internal representation of the ABI
corpus and passes it to the middle-end for further analysis.
Lastly, there is an ABIXML front-end which class is named
abigail::abixml::reader. It inherits the abigail::fe_iface class
directly. Note that unlike the two previous front-ends, this one
doesn't inherit the elf_based_reader base class, for reasons that
should be obvious to the astute reader by now. So, this front-end
implements the abigail::fe_iface::load_corpus() abstract interface to
load the properties for the ABI corpus represented in the ABIXML
format, construct the internal representation and pass it to the
middle-end for further analysis.
The code of the tools got adapted to use these front-ends.
The support of CTF is still guarded by #ifdef WITH_CTF pre-processor
macros, but the one cool side effect is that the amount of guarded
code is reduced. Basically, it's only the creation of the CTF
front-end that is guarded. After its creation, what is returned is an
instance of abigail::elf_based_reader::reader, exactly like what is
returned by the creation of a DWARF front-end. Thus, the rest of the
code is exactly the same, regardless of the kind of front-end. I
believe this results in a more elegant and maintainable code.
As a proof-of-concept, this patch also provides the
create_best_elf_based_reader function. This function analyses the ELF
file and depending on the kind of debug info it provides, returns the
right front-end for it. Maybe at some point, all the #ifdef WITH_CTF
guard pre-processing macros will be constrained in a single function
like this one that will take the decision of instantiating the right
front-end. The rest of the code will be as generic as it gets.
The patch adjusts the reference abixml files produced by the CTF
front-end because it now emits the <elf-needed> XML element which was
not emitted before. This is done because the CTF front-end inherits
the elf-reader which reads the "elf-needed" property from the binary,
without explicit intervention from the CTF front-end.
The patch passes 'make distcheck' on all the supported platforms.
* include/abg-fwd.h (build_internal_underlying_enum_type_name):
Move this here from src/abg-dwarf-reader.cc.
* include/abg-elf-reader-common.h: Delete this file. Its content
is going to be put in the new include/abg-elf-reader.h.
* src/abg-elf-reader-common.cc: Likewise.
* include/abg-{elf-based-reader, elf-reader, fe-iface}.h: Add new
files.
* src/abg-fe-iface.cc: Likewise.
* include/Makefile.am: Add the new file abg-fe-iface.h,
abg-elf-based-reader.h and abg-elf-reader.h to source distribution
and remove include/abg-elf-reader-common.h from source
distribution.
* src/abg-ir.cc (build_internal_underlying_enum_type_name): Move
this here from abg-dwarf-reader.cc so that it can be used by other
readers.
* include/abg-reader.h (abigail::abixml::reader): Rename the
namespace abigail::xml_reader into this one.
(read_context, create_native_xml_read_context)
(read_context_get_path, read_corpus_from_native_xml)
(read_corpus_from_native_xml_file)
(read_corpus_group_from_native_xml)
(read_corpus_group_from_native_xml_file): Remove.
(read_translation_unit_from_file)
(read_translation_unit_from_buffer)
(read_translation_unit_from_istream)
(read_translation_unit)
(consider_types_not_reachable_from_public_interfaces)
(get_types_from_type_id, get_artifact_used_by_relation_map)
(load_canonical_type_ids): Take an fe_iface&, not a read_context.
(create_reader): Declare new function that returns a
fe_iface_sptr.
(read_corpus_from_abixml, read_corpus_from_abixml_file)
(read_corpus_group_from_abixml)
(read_corpus_group_from_abixml_file): Declare new functions.
* src/abg-reader.cc (namespace abixml): Rename the
xml_reader namespace into this.
(abixml::reader_sptr): New typedef.
(abixml::reader): Rename read_context into this. Make it
inherit the fe_iface interface.
(abixml::reader::{m_path, m_env, m_corpus, m_corpus_group,
m_exported_decls_builder, m_supprs}): Remove these data members
that are now part of the fe_iface parent type.
(abixml::reader::{set_environment, get_corpus, set_corpus,
set_corpus_group, maybe_add_fn_to_exported_decls,
maybe_add_var_to_exported_decls,
maybe_check_abixml_canonical_type_stability,
suppression_matches_function_sym_name,
suppression_matches_variable_name,
suppression_matches_variable_sym_name}): Remove.
(read_corpus_from_input): Remove. Actually the code of this went
into abixml::reader::read_context().
(abixml::reader::get_libxml_reader): Rename the get_reader
member function into this.
(abixml::add_reader_suppressions): Rename
add_read_context_suppressions into this.
(abixml::reader::read_corpus): Implement this virtual
member function if the fe_iface parent interface.
(maybe_set_naming_typedef, advance_cursor)
(handle_version_attribute, walk_xml_node_to_map_type_ids)
(read_elf_needed_from_input, read_symbol_db_from_input)
(get_or_read_and_add_translation_unit, build_needed)
(read_elf_needed_from_input, add_read_context_suppressions)
(maybe_set_artificial_location, maybe_set_naming_typedef)
(build_namespace_decl, build_elf_symbol)
(build_elf_symbol_from_reference, build_elf_symbol_db)
(build_function_parameter, build_function_decl)
(build_function_decl_if_not_suppressed, function_is_suppressed)
(type_is_suppressed, build_var_decl_if_not_suppressed)
(variable_is_suppressed, variable_is_suppressed, build_var_decl)
(build_type_decl, build_qualified_type_decl)
(build_pointer_type_def, build_reference_type_def)
(build_function_type, build_subrange_type, build_array_type_def)
(build_enum_type_decl_if_not_suppressed, build_enum_type_decl)
(build_typedef_decl, build_class_decl_if_not_suppressed)
(build_union_decl_if_not_suppressed, build_class_decl)
(build_union_decl, build_function_tdecl, build_class_tdecl)
(build_type_tparameter, build_type_composition)
(build_non_type_tparameter, build_non_type_tparameter)
(build_template_tparameter, build_template_parameter, build_type)
(handle_type_decl, handle_namespace_decl)
(handle_qualified_type_decl, handle_pointer_type_def)
(handle_reference_type_def, handle_function_type)
(handle_array_type_def, handle_enum_type_decl)
(handle_typedef_decl, handle_var_decl, handle_function_decl)
(handle_class_decl, handle_union_decl, handle_function_tdecl)
(read_translation_unit_from_istream): Take or use an
abixml::reader rather than a read_context.
(read_translation_unit, read_translation_unit_from_input)
(consider_types_not_reachable_from_public_interfaces)
(get_types_from_type_id, get_artifact_used_by_relation_map)
(read_corpus_group_from_input, read_translation_unit)
(handle_element_node, read_location, read_artificial_location)
(load_canonical_type_ids) : Take an fe_iface&, not a read_context.
(create_abixml_reader): Rename create_native_xml_read_context
into this. Make it return a fe_iface_sptr.
(read_corpus_from_abixml): Rename read_corpus_from_abixml into
this.
(read_corpus_from_abixml_file): Rename
read_corpus_from_native_xml_file into this.
(read_context_get_path): Remove.
* include/abg-tools-utils.h
(abigail::tools_utils::{file_has_dwarf_debug_info,
file_has_ctf_debug_info}): Declare new functions.
(create_best_elf_based_reader): Declare new function.
* include/abg-corpus.h (corpus::add): Pass the translation unit by
reference.
(corpus::exported_decls_builder::maybe_add_{fn,var}_to_exported_fns):
Take a const parameter.
* src/abg-corpus-priv.h
(corpus::exported_decls_builder::priv::add_{fn,var}_to_exported):
Take a const parameter and adjust.
* src/abg-corpus.cc
(corpus::exported_decls_builder::maybe_add_{fn,var}_to_exported_fns):
Take a const parameter.
(corpus::add): Take a reference to translation_unit_sptr.
* include/abg-suppression.h (abigail::fe_iface): Forward-declare
this.
(abigail::{suppression_sptr, suppressions_type}): Declare these
types here.
(abigail::suppr::{suppression_can_match,
suppression_matches_function_name,
suppression_matches_function_sym_name,
suppression_matches_variable_name,
suppression_matches_variable_sym_name,
suppression_matches_type_name_or_location,
is_elf_symbol_suppressed, is_elf_symbol_suppressed,
is_function_suppressed, is_variable_suppressed,
is_type_suppressed}): Declare these functions here.
* src/abg-suppression-priv.h (function_is_suppressed)
(variable_is_suppressed, type_is_suppressed)
(is_elf_symbol_suppressed): Remove these template functions.
* src/abg-suppression.cc (suppression_matches_function_name)
(suppression_matches_function_sym_name): Remove.
(variable_is_suppressed, suppression_can_match)
(suppression_matches_function_name)
(suppression_matches_function_sym_name)
(suppression_matches_variable_name)
(suppression_matches_variable_sym_name)
(suppression_matches_type_name_or_location)
(is_elf_symbol_suppressed, is_elf_symbol_suppressed)
(is_function_suppressed, is_variable_suppressed)
(is_type_suppressed): New functions.
* include/abg-ctf-reader.h (abigail::ctf::{read_context,
create_read_context, read_corpus,
read_and_add_corpus_to_group_from_elf,
set_read_context_corpus_group, reset_read_context, dic_type_key}):
Remove.
(ctf::{create_reader, reset_reader}): Declare new
functions.
* src/abg-ctf-reader.cc (read_context): Remove.
(process_ctf_typedef, process_ctf_base_type)
(build_ir_node_for_variadic_parameter_type)
(process_ctf_function_type, process_ctf_sou_members)
(process_ctf_forward_type, process_ctf_struct_type)
(process_ctf_union_type, process_ctf_array_type)
(process_ctf_qualified_type, process_ctf_pointer_type)
(process_ctf_enum_type, fill_ctf_section)
(lookup_symbol_in_ctf_archive, dic_type_key): Forward-declare
these static functions.
(ctf::reader): New class that is the abstraction
of the CTF reader. It extends the abigail::elf_based_reader
class. This is a renaming of the
abigail::ctf::read_context class.
(ctf::reader::{elf_handler, elf_fd,
elf_handler_dbg, elf_fd_dbg, symtab, debug_info_root_paths_,
debug_info_root_paths_}): Remove these data members as they are
now properties of the abigail::elf_reader class, which is a parent
class of this abigail::ctf::reader class.
(ctf::reader::{exported_decls_builder,
maybe_add_fn_to_exported_decls, current_corpus_group,
has_corpus_group, main_corpus_from_current_group,
current_corpus_is_main_corpus_from_current_group,
should_reuse_type_from_corpus_group}): Remove these accessors
that can now be used from the parent classes abigail::{elf_reader,
elf_based_reader}.
(ctf::reader::reader): This now delegates to the constructor of
elf_based_reader. It doesn't pass any argument to initialize()
anymore.
(ctf::reader::initialize): Add an overload with no
parameter. In the other overload, do not take a pointer to an
environment as no new environment can be passed to the instance of
reader that is being reset. Adjust the code of the initializer to
reflect all the data members that got removed.
(ctf::{env, find_ctfa_file, slurp_elf_info,
process_ctf_archive, process_ctf_type, lookup_type, read_corpus,
~reader}): New member functions. Most of these were free-form
functions that took ctf::read_context as first parameter.
In read_corpus, do not set the corpus::LINUX_KERNEL_BINARY_ORIGIN
origin as that is now done by elf::reader when it reads the
binary.
(lookup_type): Remove. These are now member functions of the
ctf::reader class.
(process_ctf_typedef, process_ctf_base_type)
(build_ir_node_for_variadic_parameter_type)
(process_ctf_function_type, process_ctf_sou_members)
(process_ctf_forward_type, process_ctf_struct_type)
(process_ctf_union_type, process_ctf_array_type)
(process_ctf_qualified_type, process_ctf_pointer_type): Take a
ctf::reader rather an ctf::read_context. Adjust the
content of the functions.
(process_ctf_type, lookup_type, process_ctf_archive): Remove these
and turn them into member functions of ctf::reader.
(open_elf_handler, close_elf_handler, find_alt_debuginfo): Remove
these ELF handling functions as ELF handling is now done by the
elf_reader parent class.
(fill_ctf_section): Take a const pointer to Elf_Scn.
(slurp_elf_info, find_ctfa_file): Remove this and make it be a
member of ctf::reader. Also, make it handle only CTF
reader specific pieces. slurp_elf_info now delegates the reading
of generic ELF properties to elf::reader by calling
elf::reader::read_corpus().
(create_read_context, read_corpus, set_read_context_corpus_group)
(read_and_add_corpus_to_group_from_elf): Remove these functions.
(create_reader, reset_reader): Create new functions
(dic_type_key): Make this static.
* include/abg-dwarf-reader.h (abigail::dwarf::elf_type):
Move this enum into the namespace abigail::elf_reader in the file
include/abg-elf-reader.h.
(abigail::dwarf::{read_context, read_context_sptr,
create_read_context, read_context_get_path, reset_read_context,
add_read_context_suppressions, set_read_context_corpus_group,
read_corpus_from_elf, read_and_add_corpus_to_group_from_elf,
read_and_add_corpus_to_group_from_elf,
add_read_context_suppressions, refers_to_alt_debug_info,
has_alt_debug_info, get_soname_of_elf_file, get_type_of_elf_file,
set_debug_info_root_path, get_debug_info_root_path,
get_show_stats, set_show_stats, set_drop_undefined_syms,
set_do_log, set_environment, get_environment}): Remove.
* src/abg-dwarf-reader.cc (struct dwfl_deleter, dwfl_sptr)
(addr_elf_symbol_sptr_map_type, address_set_type)
(address_set_sptr): Delete these types.
(read_context::options_type): Remove. The data members of this
type got moved to struct fe_iface::options_type.
(find_alt_debug_info_link, find_alt_debug_info_path)
(find_alt_debug_info, lookup_data_tag_from_dynamic_segment)
(elf_file_type, refers_to_alt_debug_info, has_alt_debug_info)
(get_soname_of_elf_file, get_type_of_elf_file) : Remove these ELF
specific functions from here; move them to the elf_reader
namespace.
(dwarf::reader): Create new class that extends
elf_based_reader. dwarf::read_context is renamed into this
type, actually.
(dwarf::reader::die_source_dependant_container_set::get_container):
Adjust.
(dwarf::reader::{supprs_, dwarf_version_,
offline_callbacks_, debug_info_root_paths_, handle_, dwarf_,
alt_fd_, alt_dwarf_, alt_debug_info_path_, elf_module_,
elf_handle_, elf_path_, symtab_section_, cur_corpus_group_,
cur_corpus_, dt_needed_, dt_soname_, elf_architecture_,
exported_decls_builder_, options_, drop_undefined_syms_}): Remove
these ELF-related data members to move them into the elf_reader
namespace.
(maybe_propagate_canonical_type)
(build_translation_unit_and_add_to_ir, build_ir_node_from_die)
(add_or_update_class_type, add_or_update_union_type)
(build_ir_node_for_void_type)
(build_ir_node_for_variadic_parameter_type, build_function_decl)
(function_is_suppressed, build_or_get_fn_decl_if_not_suppressed)
(build_var_decl, build_or_get_var_decl_if_not_suppressed)
(variable_is_suppressed)
(propagate_canonical_type)
(get_parent_die, get_scope_die, die_is_at_class_scope)
(die_location, die_qualified_type_name, die_qualified_name)
(die_qualified_type_name_empty)
(die_return_and_parm_names_from_fn_type_die)
(die_function_signature, die_function_type_is_method_type)
(die_pretty_print_type, die_pretty_print_decl, die_pretty_print)
(maybe_canonicalize_type, build_subrange_type)
(build_subranges_from_array_type_die, compare_dies, die_location)
(die_loc_and_name, die_is_effectively_public_decl)
(maybe_cache_type_comparison_result)
(get_cached_type_comparison_result)
(maybe_get_cached_type_comparison_result, die_is_at_class_scope)
(die_function_type_is_method_type, die_member_offset)
(die_qualified_type_name, die_qualified_decl_name)
(die_qualified_name, die_qualified_type_name_empty)
(die_return_and_parm_names_from_fn_type_die)
(die_function_signature, die_pretty_print_type)
(die_pretty_print_decl, die_pretty_print)
(at_least_one_decl_only_among_odr_relevant_dies)
(compare_as_type_dies, compare_as_decl_and_type_dies)
(fn_die_equal_by_linkage_name, try_canonical_die_comparison)
(maybe_propagate_canonical_type, propagate_canonical_type)
(compare_dies, compare_dies_during_canonicalization)
(find_import_unit_point_between_dies, get_parent_die)
(get_scope_die, find_lower_bound_in_imported_unit_points)
(build_translation_unit_and_add_to_ir)
(build_namespace_decl_and_add_to_ir, build_type_decl)
(build_enum_underlying_type, build_enum_type)
(finish_member_function_reading)
(maybe_finish_function_decl_reading)
(lookup_class_or_typedef_from_corpus)
(is_function_for_die_a_member_of_class)
(add_or_update_member_function, add_or_update_class_type)
(add_or_update_union_type, build_qualified_type)
(schedule_array_tree_for_late_canonicalization)
(maybe_strip_qualification, build_pointer_type_def)
(build_reference_type, build_function_type, build_subrange_type)
(build_subranges_from_array_type_die, build_array_type)
(build_typedef_type, build_or_get_var_decl_if_not_suppressed)
(build_var_decl, function_is_suppressed)
(build_or_get_fn_decl_if_not_suppressed, variable_is_suppressed)
(type_is_suppressed, type_is_suppressed)
(get_opaque_version_of_type, create_default_fn_sym)
(build_function_decl, maybe_canonicalize_type)
(build_ir_node_from_die)
(build_ir_node_for_variadic_parameter_type): Take a reference to
the new dwarf::reader rather than to the previous
read_context. Adjust the function body.
(return_comparison_result): Adjust.
(dwarf::reader::reader): Adjust this from
read_context::read_context.
(dwarf::reader::initialize): Adjust from
dwarf::read_context::initialize.
(dwarf::reader::create): New factory static member
function.
(dwarf::reader::~reader): This doesn't have to clear
anything for now.
(dwarf::reader::read_corpus): New virtual member function
which implements the fe_iface::read_corpus pure virtual interface.
This now delegates the reading of the generic ELF properties to
elf::reader by calling elf::reader::read_corpus().
Newer front-ends will be able to do the same.
(dwarf::reader::reset_corpus): New member function.
(dwarf::reader::read_debug_info_into_corpus): Adjust. This
is now a member function. Also, do not set the
corpus::LINUX_KERNEL_BINARY_ORIGIN here as it's now set by the
elf::reader when it loads the binary.
(dwarf::reader::{env, drop_undefined_syms,
drop_undefined_syms, dwarf_elf_handle, dwarf_per_die_source,
elf_path, compute_canonical_die_offset, get_die_source,
get_die_from_offset, get_die_qualified_name,
get_die_pretty_type_representation, get_die_qualified_type_name,
get_die_pretty_representation, odr_is_relevant,
set_canonical_die_offset, get_canonical_die_offset,
erase_canonical_die_offset, die_wip_classes_map,
die_wip_function_types_map, compare_before_canonicalisation,
resolve_declaration_only_classes, resolve_declaration_only_enums,
symbol_already_belongs_to_a_function,
fixup_functions_with_no_symbols, canonicalize_types_scheduled,
tu_die_imported_unit_points_map, die_parent_map,
find_symbol_table_section, get_variable_address,
exported_decls_builder, load_all_types, load_in_linux_kernel_mode,
show_stats, do_log, build_die_parent_maps): Adjust.
(offset_pairs_stack_type::rdr_): Changed the ctxt_ into this.
(offset_pairs_stack_type::offset_pairs_stack_type): Adjust.
(offset_pairs_stack_type::{erase_redundant_type_pair_entry,
cancel_canonical_propagated_type}): Adjust.
(dwarf::reader::{get_suppressions, offline_callbacks,
create_default_dwfl, dwfl_handle, elf_module, elf_handle,
add_debug_info_root_paths, add_debug_info_root_path,
find_alt_debug_info, dwarf, alt_dwarf, alt_debug_info_path,
current_corpus, reset_current_corpus, current_corpus_group,
has_corpus_group, main_corpus_from_current_group,
current_corpus_is_main_corpus_from_current_group,
should_reuse_type_from_corpus_group, function_symbol_is_exported,
variable_symbol_is_exported, symtab, dt_needed, dt_soname,
elf_architecture, is_elf_symbol_suppressed,
load_dt_soname_and_needed, load_elf_architecture,
load_elf_properties, maybe_add_fn_to_exported_decls,
maybe_add_var_to_exported_decls}): Remove these member functions
as they got moved into the elf_reader namespace or into the
fe_iface class.
(dwarf::read_context::{suppression_can_match,
suppression_matches_function_sym_name,
suppression_matches_function_name,
suppression_matches_variable_name,
suppression_matches_variable_sym_name,
suppression_matches_type_name_or_location}): Move these into the
suppr namespace. Make it take an additional parameter that is
reference fe_iface.
(dwarf::reader::load_debug_info): Remove. This became
merged into dwarf::read_debug_info_into_corpus.
(dwarf::{set_debug_info_root_path,
get_debug_info_root_path, get_show_stats, set_drop_undefined_syms,
set_do_log}): Remove.
(add_read_context_suppressions)
(set_read_context_corpus_group, read_corpus_from_elf): Remove.
(read_debug_info_into_corpus): This became a member function of
dwarf::reader.
(create_reader): Renamed create_read_context into this.
Make it return an elf_based_reader_sptr, like the other front-end
factory functions. Adjust.
(reset_dwarf_reader): Renamed reset_read_context into this.
Adjust.
(read_corpus_from_elf): Adjust.
* src/abg-elf-based-reader.cc: New file.
* src/abg-elf-helpers.h (struct dwfl_deleter, dwfl_sptr)
(addr_elf_symbol_sptr_map_type, address_set_sptr): Move these
types here from abg-dwarf-reader.cc
(initialize_dwfl_callbacks, lookup_data_tag_from_dynamic_segment):
* src/abg-elf-helpers.cc (lookup_data_tag_from_dynamic_segment)
(lookup_data_tag_from_dynamic_segment, initialize_dwfl_callbacks)
(create_new_dwfl_handle, get_soname_of_elf_file): New functions
that got moved here from the factorizing of abg-dwarf-reader.cc
and abg-ctf-reader.cc.
* src/abg-tools-utils.cc (file_has_dwarf_debug_info)
(file_has_ctf_debug_info): New functions.
(load_generate_apply_suppressions): Take an elf_based_reader, not
a dwarf::read_context.
(maybe_load_vmlinux_dwarf_corpus): Adjust the body to use the new
front-end types.
* src/Makefile.am: Add the new files src/abg-{fe-iface,
elf-based-reader, elf-reader}.cc to source distribution. Remove
src/abg-elf-reader-common.cc.
* tools/Makefile.am: Factorize linking to libabigail.so by using
LDADD.
* tools/abicompat.cc (read_corpus, main): Adjust.
* tools/abidiff.cc (set_suppressions)
(set_native_xml_reader_options, handle_error, main): Adjust.
* tools/abidw.cc (set_suppressions, load_corpus_and_write_abixml)
(load_kernel_corpus_group_and_write_abixml): Adjust.
* tools/abilint.cc (build_type_use_tree, show_how_type_is_used)
(set_suppressions, main): Adjust.
* tools/abipkgdiff.cc (elf_file::type, compare, compare_to_self)
(create_maps_of_package_content)
(compare_prepared_userspace_packages)
(self_compare_prepared_userspace_package): Adjust.
* tools/abisym.cc: Adjust invocation to
abigail::dwarf::lookup_symbol_from_elf, from
abigail::dwarf_reader::lookup_symbol_from_elf.
* tools/kmidiff.cc (main): Adjust.
* tests/print-diff-tree.cc (main): Adjust.
* tests/test-abidiff.cc (main): Likewise.
* tests/test-diff-dwarf.cc (main): Likewise.
* tests/test-ir-walker.cc (main): Likewise.
* tests/test-read-ctf.cc (test_task_ctf::perform): Likewise.
* tests/test-read-dwarf.cc: Remove the useless "using" statements.
* tests/test-read-write.cc: Likewise.
* tests/test-symtab.cc (read_corpus, TEST_CASE)
(assert_symbol_count): Adjust.
* tests/data/test-read-ctf/test0.abi: Adjust.
* 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.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Thu, 10 Nov 2022 11:00:44 +0000 (12:00 +0100)]
Use environment by reference.
This patch simplifies how the environment is created and passed around
the functions that create front ends. With this change, the
environment can simply be allocated on the stack and passed by
reference to the libabigail pipeline.
At the core of this change, type_or_decl_base::priv now carries a
const reference to an environment rather than a pointer. The side
effect is that type_or_decl_base can no longer be copied. This is not
a problem because throughout the years I could see that the use case
to copy ABI artifacts is just not there. Similarly, elf_symbol::priv
carries a const reference to environment now, no more a pointer.
Getters, setters and code that use the environment from the ABI
artifacts are updated accordingly.
The DWARF front-end can now be created by code that looks like this,
for instance:
vector<char**> debug_info_paths;
abigail::ir::environment env;
abigail::ctf_reader::read_context_sptr reader =
abigail::dwarf_reader::create_read_context("elf/file/to/analyze",
debug_info_paths, env);
elf_reader::status reading_status;
corpus_sptr abi_corpus =
abigail::dwarf_reader::read_corpus_from_elf(reader, reading_status);
/* then do something with the resulting abi_corpus*/
Note how, you don't need to use the "new" operator to instantiate the
"env" object of type environment. It can sit on the stack and it's
passed to the read_corpus_from_elf function by reference.
In other words, the internal representation types have been changed to
refer to the environment by reference, rather than requiring a pointer
to it.
* include/abg-corpus.h (corpus::corpus)
(corpus_group::corpus_group): Take environment&, not environment*
as parameter.
(corpus::{get_environment, set_environment}): Take or return
environment&, not environment*.
* src/abg-corpus.cc (corpus::corpus): Likewise.
(corpus::{get_environment, set_environment}): Likewise.
(corpus::add): Don't update the environment of the translation
unit.
(corpus::{record_type_as_reachable_from_public_interfaces,
type_is_reachable_from_public_interfaces, init_format_version,
add_corpus}): Adjust for accessing a reference to environment,
rather than a pointer.
* include/abg-ctf-reader.h (create_read_context): Take or return
environment&, not environment*.
* src/abg-ctf-reader.cc (read_context::ir_env): Make this a
reference to environment, not pointer anymore.
(read_context::read_context): Initialize the reference to
environment.
(read_context::initialize): Do not re-set the environment.
(process_ctf_base_type)
(build_ir_node_for_variadic_parameter_type)
(process_ctf_enum_type, read_corpus): Adjust for accessing a
reference to environment, rather than a pointer.
(create_read_context, reset_read_context): Take environment&, not
environment*.
* include/abg-dwarf-reader.h (create_read_context)
(reset_read_context, read_corpus_from_elf)
(lookup_symbol_from_elf, lookup_public_function_symbol_from_elf):
Likewise.
* src/abg-dwarf-reader.cc (lookup_symbol_from_sysv_hash_tab)
(lookup_symbol_from_gnu_hash_tab)
(lookup_symbol_from_elf_hash_tab, lookup_symbol_from_symtab)
(lookup_symbol_from_elf, lookup_public_function_symbol_from_elf):
Likewise.
(read_context::options_type::env): Make this be a reference to
environment, not a pointer.
(read_context::options::options): Remove the default constructor.
Add a new one to initialize the environment data member.
(read_context::read_context): Take environment&, not environment*.
Initialize the options_ data member.
(read_context::initialize): Do not take or initialize an
environment anymore.
(read_context::env): Return or take environment&, not
environment*.
(read_context::{get_die_qualified_name,
get_die_qualified_type_name, get_die_pretty_type_representation,
get_die_pretty_representation, compare_before_canonicalisation})
(build_translation_unit_and_add_to_ir, build_function_type)
(build_typedef_type, read_debug_info_into_corpus)
(read_debug_info_into_corpus, build_ir_node_from_die)
(build_ir_node_for_variadic_parameter_type, has_alt_debug_info):
Adjust to use an environment&, not a pointer.
(create_default_fn_sym, create_read_context)
(read_corpus_from_elf, lookup_symbol_from_elf)
(lookup_public_function_symbol_from_elf): Take environment&, not
environment*.
(reset_read_context): Do not take or reset environment* anymore.
* include/abg-fwd.h (type_or_void): Likewise.
* include/abg-ir.h (translation_unit::translation_unit): Likewise.
(translation_unit::{get_environment, set_environment}): Likewise.
(elf_symbol::elf_symbol): Likewise.
(elf_symbol::create): Remove the overload that takes no
parameter. Then for overload that take parameters then take
environment&, not environment*.
(elf_symbol::get_environment): Take environment&, not
environment*.
(type_or_decl_base::type_or_decl_base): Make the copy constructor
and assignment operator private.
(type_or_decl_base::{s,g}et_environment): Take or return
environment& not environment*.
(type_or_decl_base::set_environment_for_artifact): Erase these
methods.
(decl_base::decl_base): Make copy constructor private. Take or
return environment&, not environment* for the other constructors.
(scope_decl::scope_decl): Take or return environment&, not
environment*.
(type_base::type_base): Likewise.
(scope_type_decl::scope_type_decl): Likewise.
(namespace_decl::namespace_decl): Likewise.
(qualified_type_def::qualified_type_def): Likewise.
(pointer_type_def::pointer_type_def): Likewise.
(reference_type_def::reference_type_def): Likewise.
(array_type_def::subrange_type::subrange_type): Likewise.
(enum_type_def::enumerator::enumerator): Likewise.
(enum_type_def::enumerator::{get_name, get_qualified_name}):
Return a string&, no more interned_string&. As the enumerator
don't have an enumerator anymore, there is no way to intern the
string anymore. Hopefully this won't incur a performance loss.
(typedef_decl::typedef_decl, function_type::function_type)
(method_type::method_type, template_decl::template_decl)
(function_tdecl::function_tdecl, class_tdecl::class_tdecl)
(class_or_union::class_or_union, class_decl::class_decl)
(union_decl::union_decl): Take or return environment&, not
environment*.
* include/abg-reader.h (read_translation_unit_from_file)
(read_translation_unit_from_buffer)
(read_translation_unit_from_istream)
(create_native_xml_read_context, create_native_xml_read_context)
(read_corpus_from_native_xml, read_corpus_from_native_xml_file)
(read_corpus_group_from_native_xml)
(read_corpus_group_from_native_xml_file): Likewise.
* include/abg-tools-utils.h
(build_corpus_group_from_kernel_dist_under): Likewise.
* src/abg-tools-utils.cc (maybe_load_vmlinux_dwarf_corpus)
(maybe_load_vmlinux_ctf_corpus)
(build_corpus_group_from_kernel_dist_under): Likewise.
* include/abg-writer.h (create_write_context): Likewise.
* src/abg-writer.cc (id_manager::m_env, id_manager::id_manager)
(id_manager::get_environment, id_manager::get_id)
(id_manager::get_id_with_prefix): Adjust.
(write_context::m_env, write_context::write_context)
(write_context::get_environment, write_context::get_config)
(write_context::get_id_for_type, write_context::decl_is_emitted)
(write_context::record_decl_as_emitted, create_write_context)
(write_class_decl): Likewise.
* src/abg-comparison.cc (compute_diff): Stop ensuring that the two
artifacts being compare are in the same environment. Now that the
environment is passed by reference, the potential for
accendentally comparing artifacts coming from different
environments is very low, given how the API is used in practice.
This is in the overloads for decl_base_sptr, type_base_sptr,
var_decl_sptr, pointer_type_def_sptr, array_type_def_sptr,
reference_type_def_sptr, qualified_type_def_sptr,
enum_type_decl_sptr, class_decl_sptr, class_decl::base_spec_sptr,
union_decl_sptr, scope_decl_sptr, function_decl::parameter_sptr,
function_type_sptr, function_decl_sptr, type_decl_sptr,
typedef_decl_sptr, translation_unit_sptr, corpus_sptr.
* src/abg-corpus-priv.h (corpus::priv::env): Make this be a
reference to environments, not a pointer.
(corpus::priv::priv): Pass environment&, not environment*.
* src/abg-ir-priv.h (translation_unit::priv::env_): Make this an
environment&, not an environment* anymore.
(translation_unit::priv::priv): Take an environment&, not an
environment*.
(environment::priv::{confirm_ct_propagation_for_types_dependant_on,
confirm_ct_propagation,
cancel_ct_propagation_for_types_dependant_on,
mark_as_being_compared, unmark_as_being_compared,
comparison_started, mark_as_being_compared, comparison_started}):
Adjust to use an environment&, not a pointer.
* src/abg-ir.cc (class environment_setter): Remove this class.
(push_composite_type_comparison_operands)
(pop_composite_type_comparison_operands, try_canonical_compare)
(return_comparison_result, translation_unit::{get_global_scope,
bind_function_type_life_time}): Adjust.
(translation_unit::{translation_unit, get_environment}): Take or
get an environment&, not an environment*. Remove the getter that
returns an environment*.
(elf_symbol::priv::env_): Make this an environment&, not an
environment*.
(elf_symbol::priv::priv): Adjust.
(elf_symbol::elf_symbol): Remove the default constructor. Change
the one that takes an environment.
(elf_symbol::create): Remove the default one. Adjust the one that
takes an environment.
(elf_symbol::get_environment): Adjust.
(elf_symbol::set_environment_for_artifact): Remove.
(environment::{get_void_type, get_variadic_parameter_type}):
Adjust.
(type_or_decl_base::priv::env_): Make this be a const
environment&, not a const environment*.
(type_or_decl_base::priv::priv): Adjust.
(type_or_decl_base::type_or_decl_base): Remove the default and
copy constructor.
(type_or_decl_base::{set_environment, operator=})
(set_environment_for_artifact): Remove.
(type_or_decl_base::get_environment): Adjust.
(decl_base::{decl_base, set_name, set_naming_typedef,
set_linkage_name}): Adjust.
(get_decl_name_for_comparison, strip_typedef)
(strip_useless_const_qualification): Adjust.
(scope_decl::{scope_decl, add_member_decl, insert_member_decl}):
Adjust.
(get_generic_anonymous_internal_type_name, get_type_name)
(get_name_of_pointer_to_type, get_name_of_reference_to_type)
(get_name_of_qualified_type, get_function_type_name)
(get_method_type_name, is_void_pointer_type, lookup_basic_type)
(lookup_union_type, lookup_union_type_per_location)
(lookup_enum_type, lookup_typedef_type, lookup_pointer_type)
(lookup_type, lookup_basic_type_per_location)
(lookup_basic_type_per_location, lookup_basic_type)
(lookup_class_type, lookup_class_types)
(lookup_class_type_per_location, lookup_union_type)
(lookup_enum_type, lookup_enum_types)
(lookup_enum_type_per_location, lookup_typedef_type)
(lookup_typedef_type_per_location, maybe_update_types_lookup_map)
(maybe_update_types_lookup_map)
(synthesize_type_from_translation_unit)
(synthesize_function_type_from_translation_unit)
(demangle_cplus_mangled_name, type_or_void)
(types_defined_same_linux_kernel_corpus_public)
(compare_types_during_canonicalization)
(type_base::get_canonical_type_for, type_base::type_base)
(type_base::get_cached_pretty_representation)
(type_decl::type_decl, type_decl::get_qualified_name): Adjust.
(scope_type_decl::scope_type_decl)
(namespace_decl::namespace_decl, qualified_type_def::build_name)
(qualified_type_def::qualified_type_def)
(qualified_type_def::get_qualified_name)
(qualified_type_def::set_underlying_type)
(pointer_type_def::pointer_type_def)
(pointer_type_def::set_pointed_to_type)
(reference_type_def::reference_type_def)
(reference_type_def::set_pointed_to_type)
(array_type_def::subrange_type::subrange_type)
(array_type_def::array_type_def, array_type_def::update_size)
(array_type_def::set_element_type)
(array_type_def::append_subranges)
(array_type_def::get_qualified_name, enum_has_non_name_change):
Adjust.
(enum_type_decl::enumerator::priv::env_): Remove this pointer to
env. This is because the enumerator must be copy-able. As the
enumerator doesn't have an env anymore, it can't intern strings.
So the enumerator name and qualified name is not going to be
interned. If that incurs a performance hit, we'll reconsider this
decision. For now, it seems to work OK. As it simplifies things,
I am keeping this for now.
(enum_type_decl::enumerator::priv::{name, qualified_name}): Make
this be string, not interned_string.
(enum_type_decl::enumerator::get_environment): Remove.
(enum_type_decl::enumerator::priv::priv): Adjust.
(enum_type_decl::enumerator::enumerator)
(enum_type_decl::enumerator::operator=)
(enum_type_decl::enumerator::get_name)
(enum_type_decl::enumerator::get_qualified_name)
(enum_type_decl::enumerator::set_name): Likewise.
(typedef_decl::typedef_decl): Adjust.
(var_decl::get_id, var_decl::get_qualified_name): Adjust.
(function_type::function_type, method_type::method_type)
(function_decl::get_pretty_representation_of_declarator)
(function_decl::set_symbol): Likewise.
(function_decl::get_id, function_decl::parameter::get_type)
(function_decl::parameter::get_type_name)
(function_decl::parameter::get_type_pretty_representation)
(function_decl::parameter::get_name_id)
(class_or_union::class_or_union, class_decl::class_decl)
(class_decl::add_base_specifier, union_decl::union_decl)
(union_decl::union_decl, template_decl::template_decl)
(class_tdecl::class_tdecl)
(maybe_cancel_propagated_canonical_type)
(dump_classes_being_compared)
(dump_fn_types_being_compared, copy_member_function)
(maybe_propagate_canonical_type, keep_type_alive)
(is_non_canonicalized_type, qualified_name_setter::do_update):
Likewise.
(equals): Adjust the overloads for var_decl, function_type,
class_or_union, class_decl, union_decl.
* src/abg-reader.cc (read_context::m_env): Make this be an
environment&, not an environment*.
(read_context::read_context): Adjust
(read_context::set_environment): Remove.
(read_context::{get_environment,
maybe_check_abixml_canonical_type_stability}): Adjust.
(read_corpus_from_input, read_corpus_group_from_native_xml)
(read_corpus_group_from_native_xml_file)
(read_translation_unit_from_file)
(read_translation_unit_from_buffer, read_translation_unit)
(maybe_map_type_with_type_id, build_namespace_decl)
(build_elf_symbol, build_function_parameter, build_function_decl)
(build_function_type, build_enum_type_decl, build_class_decl)
(build_union_decl, build_function_tdecl, build_class_tdecl)
(build_type_tparameter, read_translation_unit_from_istream)
(create_native_xml_read_context, read_corpus_from_native_xml):
Likewise.
* src/abg-symtab-reader.h (symtab::load): Likewise.
* src/abg-symtab-reader.cc (symtab::load): Likewise.
* tests/print-diff-tree.cc (main): Likewise.
* tests/test-abidiff.cc (main): Likewise.
* tests/test-diff-dwarf.cc (main): Likewise.
* tests/test-ir-walker.cc (main): Likewise.
* tests/test-read-ctf.cc (test_task_ctf::perform): Likewise.
* tests/test-symtab.cc (read_corpus): Likewise.
* tools/abicompat.cc (read_corpus, main): Likewise.
* tools/abidiff.cc (main): Likewise.
* tools/abidw.cc (load_corpus_and_write_abixml)
(load_kernel_corpus_group_and_write_abixml, main): Likewise.
* tools/abilint.cc (main): Likewise.
* tools/abipkgdiff.cc (compare, compare_to_self)
(compare_prepared_linux_kernel_packages, compare_task::perform):
Likewise.
* tools/abisym.cc (main): Likewise.
* tools/kmidiff.cc (main): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Aleksei Vetrov [Fri, 18 Nov 2022 09:09:07 +0000 (09:09 +0000)]
symtab: add support for CRC values from __kcrctab
New kernels changed the format of storing CRC values from absolute
symbol value to the address in __kcrctab or __kcrctab_gpl section.
This change adds support for CRC values described in this format.
* src/abg-elf-helpers.h (get_crc_for_symbol): Defined new
helper function to extract CRC from ELF symbol.
* src/abg-elf-helpers.cc (get_crc_for_symbol): Implemented this
function with support of old and new CRC values format.
* src/abg-symtab-reader.cc (symtab::load_): Used the new
function when building CRC values map.
Change-Id: I7de5c737d5caaef0c5b7b2ea0d448368889a16be
Signed-off-by: Aleksei Vetrov <vvvvvv@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Sam James [Tue, 8 Nov 2022 07:35:03 +0000 (07:35 +0000)]
Use xz as the default tarball compression format
xz is quite a common format for software to be distributed nowadays
because of its size reductions. With this patch, here is the result
on the master branch at
db716e3b15912b7162def1faa704eb7823bbf34:
.rw-r--r-- 406M sam 8 Nov 07:27 libabigail-2.2.tar.gz
.rw-r--r-- 348M sam 8 Nov 07:26 libabigail-2.2.tar.xz
* configure.ac: add dist-xz to AM_INIT_AUTOMAKE.
* Makefile.am: adjust $(TARBALL) to new extension.
* Makefile.am: pass XZ_OPT="-0" for distcheck-fast.
Signed-off-by: Sam James <sam@gentoo.org>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Giuliano Procida [Fri, 11 Nov 2022 13:21:51 +0000 (13:21 +0000)]
Narrow Linux symbol CRCs to 32 bits
MODVERSIONS CRCs are 32-bit hashes of strings representing C type
elements or typed symbols. The hashes are calculated using a 32-bit
CRC, hence the name. The kernel module loading code (implicitly)
truncates any provided CRC value to 32 bits before comparing it with
anything.
When support was added to libabigail, values up to 64 bits wide were
supported. This change narrows libabigail's concept of Linux CRC to 32
bits. No tests are affected.
* include/abg-ir.h (elf_symbol::elf_symbol): Change CRC type
from optional<uint64_t> to optional<uint32_t>.
(elf_symbol::create): Likewise.
(elf_symbol::get_crc): Likewise.
(elf_symbol::set_crc): Likewise.
* src/abg-ir.cc (elf_symbol::priv) Change CRC type from
optional<uint64_t> to optional<uint32_t>.
(elf_symbol::priv::priv): Likewise.
(elf_symbol::elf_symbol): Likewise.
(elf_symbol::create): Likewise.
(elf_symbol::get_crc): Likewise.
(elf_symbol::set_crc): Likewise.
* src/abg-reporter-priv.cc (maybe_report_diff_for_symbol):
Change CRC type from optional<uint64_t> to
optional<uint32_t>.
* src/abg-symtab-reader.cc (symtab::load_): Change crc_values
value type from uint64_t to uint32_t.
Signed-off-by: Giuliano Procida <gprocida@google.com>
Dodji Seketeli [Tue, 8 Nov 2022 10:31:31 +0000 (11:31 +0100)]
kmidiff: Fix spacing in the help string
* tools/kmidiff.cc (display_usage): Fix spacing.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 14 Oct 2022 14:06:54 +0000 (16:06 +0200)]
ir: Don't crash when looking at corpus-less translation units
Back in the early days, there was the possibility to have an ABIXML
output that was just a translation unit. There was no XML corpus
element holding the abi-instr element. We still need to support this
going forward.
While looking at something else, I stumbled across spots in the source
code that would crash upon encountering such corpus-less ABIXML
inputs.
Fixed thus.
* src/abg-ir.cc ({decl,type}_topo_comp::operator()): Support types
that inherits from an empty corpus.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Thu, 20 Oct 2022 10:18:05 +0000 (12:18 +0200)]
rhbz2114909 - Refer to changed base classes using their non-qualified names
This was filled at https://bugzilla.redhat.com/show_bug.cgi?id=
2114909
When analysing the edit script representing the changes in the base
base classes of a given class, libabigail refers to the changed base
classes using their names.
The problem is that there can be several changed base classes that
have the same name, even though their /qualified/ names are
different. That was wreaking having havoc down the road.
To solve the problem, use the qualified name of the base classes,
instead of their simple name.
Note that this change did also ameliorate other existing change
reports in the test suite.
While testing the fix, I realized that class_decl::base_spec doesn't
set its qualified name. That leads to the fact that it's the
non-qualified name that is used for sorting the base class names in
the change reports. This can lead to instabilities, depending on the
platform, in test output. This is also fixed.
Fixed thus.
* src/abg-comparison.cc
(class_diff::ensure_lookup_tables_populated): Refer to the added
or removed base class using its qualified name.
* src/abg-ir.cc (class_decl::base_spec::base_spec): Set the
qualified name of the base_spec.
* tests/data/test-abidiff-exit/test-rhbz2114909-report-1.txt: New
reference test output.
* tests/data/test-abidiff-exit/test-rhbz2114909-v{0,1}.o: New
input test binaries.
* tests/data/test-abidiff-exit/test-rhbz2114909-v{0,1}.cc: Source
code of the binaries, above.
* tests/data/Makefile.am: Add the new test materials above to
source distribution.
* tests/test-abidiff-exit.cc (in_out_specs): Add the new tests to
this harness.
* tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Xiaole He [Sun, 16 Oct 2022 07:02:25 +0000 (07:02 +0000)]
abg-diff-utils: fix typo in comments
Fix typo in comments, from 'pased' to 'passed'.
* src/abg-diff-utils.h: fix typo in comments
Signed-off-by: Xiaole He <hexiaole@kylinos.cn>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Xiaole He [Sun, 16 Oct 2022 06:47:03 +0000 (06:47 +0000)]
abg-reader: optimize if construction
In 'build_enum_type_decl' function of 'src/abg-reader.cc', the
'for loop' walk through all the child nodes of the '<enum-decl>' for
seeking '<underlying-type>' and '<enumerator>':
/* original src/abg-reader.cc begin */
static enum_type_decl_sptr
build_enum_type_decl(read_context& ctxt,
const xmlNodePtr node,
bool add_to_current_scope)
{
...
for (xmlNodePtr n = xmlFirstElementChild(node);
n;
n = xmlNextElementSibling(n))
{
if (xmlStrEqual(n->name, BAD_CAST("underlying-type")))
{
...
}
if (xmlStrEqual(n->name, BAD_CAST("enumerator")))
{
...
}
}
...
}
/* original src/abg-reader.cc end */
Here uses 2 separate 'if' statements for seeking, that is, for any
child node of the '<enum-decl>', there involves 2 'if' comparations.
Because the child node of the '<enum-decl>' is either
'<underlying-type>' or '<enumerator>', there would be a slight
optimization when use 'if-else if' construction instead, like below:
/* optimized src/abg-reader.cc begin */
for (xmlNodePtr n = xmlFirstElementChild(node);
n;
n = xmlNextElementSibling(n))
{
if (xmlStrEqual(n->name, BAD_CAST("underlying-type")))
{
...
}
else if (xmlStrEqual(n->name, BAD_CAST("enumerator")))
{
...
}
}
/* optimized src/abg-reader.cc end */
Supposing there has the test case:
/* test case begin */
<abi-instr version='1.0'>
<enum-decl name='E' filepath='../../abitests/test-enum0-v0.cc' line='1' column='6' id='type-id-2'>
<underlying-type type-id='type-id-1'/>
<enumerator name='e0' value='0'/>
<enumerator name='e2' value='1'/>
</enum-decl>
</abi-instr>
/* test case end */
When parsing the '<underlying-type>' xml tag, for the original
'src/abg-reader.cc', there involves 2 'if' comparations. But involves
only 1 'if' comparation for the optimized 'src/abg-reader.cc'.
Signed-off-by: Xiaole He <hexiaole@kylinos.cn>
Tested-by: Xiaole He <hexiaole@kylinos.cn>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Xiaole He [Sun, 16 Oct 2022 04:26:42 +0000 (04:26 +0000)]
abg-ir: add missing else
In 'bind_function_type_life_time' function of 'src/abg-ir.cc', the code
obtains the member 'const environment*' of 'class function_type', that
is, the 'e' variable in below code. And assure the obtained
'environment*' is same as the 'const environment*' of the
'class translation_unit', that is, the'env' variable in below code:
/* src/abg-ir.cc begin */
1 void
2 translation_unit::bind_function_type_life_time(function_type_sptr ftype)
3 {
4 ...
5 const environment* env = get_environment();
6 ...
7 if (const environment* e = ftype->get_environment())
8 ABG_ASSERT(env == e);
9 ftype->set_environment(const_cast<environment*>(env));
10
11 if (const translation_unit* existing_tu = ftype->get_translation_unit())
12 ABG_ASSERT(existing_tu == this);
13 else
14 ftype->set_translation_unit(const_cast<translation_unit*>(this));
15 ...
/* src/abg-ir.cc end */
There was a missing 'else' between the 'line 8' and line 9', as the
explicit 'else' at the 'line 13'. Without the 'else' between the
'line 8' and line 9', there will be a redundant assignment at 'line 9'
under the condition when the 'env' is equal to 'e'.
This patch add the missing 'else' between the 'line 8' and 'line 9'.
* src/abg-ir.cc (bind_function_type_life_time): add missing
else
Signed-off-by: Xiaole He <hexiaole@kylinos.cn>
Tested-by: Xiaole He <hexiaole@kylinos.cn>
Dodji Seketeli [Wed, 12 Oct 2022 10:12:16 +0000 (12:12 +0200)]
dwarf-reader: Fix class size setting bug
While looking at something else, I saw cases in DWARF where we don't
set the size of some classes, especially when the DIE of the class is
an implementation of a specification (which obviously has a zero
size).
And those cases lead to some classes wrongly considered as having zero
size.
Fixed thus with test cases output updated.
* src/abg-dwarf-reader.cc (add_or_update_class_type): If we are
looking at a class DIE with children node, if it's advertized as
having non-zero size, then update the size.
* tests/data/test-annotate/libtest23.so.abi: Adjust.
* 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/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1-report-0.txt:
Likewise.
* tests/data/test-read-dwarf/libtest23.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/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 10 Oct 2022 10:44:59 +0000 (12:44 +0200)]
ir: Properly indent overload of equals() for class_decl
When looking at something else, I noticed that half of the overload of
equals() for class_decl wasn't properly indented. Grrr. Fixed thus.
* src/abg-ir.cc (equals): Fix indentation.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 10 Oct 2022 10:42:15 +0000 (12:42 +0200)]
ir: Fix a wrong comment in canonicalize()
While looking at something else, I noticed a comment that doesn't make
sense in the canonicalize() Fixed thus.
* src/abg-ir.cc (canonicalize): Fix a comment.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 10 Oct 2022 10:37:15 +0000 (12:37 +0200)]
ir: remove redundant cycle detection code in equals
While looking at something else, I realized that in the overload for
the equals() function for class_or_union, the cycle detection
management code was redundant for the case where we are looking
comparing a decl-only class to another class.
This patch removes that redundant code.
* src/abg-ir.cc (equals): In the overload for class_or_union
remove redundant cycle detection code when comparison a decl-only
class to another class.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 7 Oct 2022 21:02:15 +0000 (23:02 +0200)]
Bug 29650 - Caching class comparison result potentially too early
When structurally comparing two classes T and T' the overload of the
equals() function for abigail::ir::class_decl calls the overload of
equals() for abigail::ir::class_or_union to compare the data members
of the class. If data members are equal, that later call caches the
result of comparing the data-members-only sub-object of T and T'.
That caching appears as if it's the result of comparing all of T and
T' that was cached, leading to misleading results down the road.
Result caching should not take place until the end of fulling
comparing T and T'. Fixed thus.
* src/abg-ir.cc (equal): In the overload of class_or_union do not
cache the result comparing just the data members sub-types of of
classes. In the overload for class_decl, put cycle detection
management code /after/ the call to equals for class_or_union,
because that called function does perform the cycle detection
management as well; otherwise, that introduces an unwarranted
redundancy. In the overload of equals for union_decl, cache the
result of the comparison.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
* tests/data/test-diff-pkg/tbb-4.1-9.
20130314.fc22.x86_64--tbb-4.3-3.
20141204.fc23.x86_64-report-0.txt:
Adjust.
* tests/data/test-diff-pkg/tbb-4.1-9.
20130314.fc22.x86_64--tbb-4.3-3.
20141204.fc23.x86_64-report-1.txt:
Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 7 Oct 2022 20:50:14 +0000 (22:50 +0200)]
abidiff: add a --debug-tc option
Like what was done for abidw, this patch adds a --debug-tc option to
abidiff to debug type canonicalization issues.
With this option, just like for abidw, during type canonicalization,
each type comparison is done twice: once using structural comparison
and once using canonical comparison. Both comparisons should yield
the same result otherwise, an abort signal is emitted, asking for
in-depth debugging to understand reason of the difference.
This option is enabled by the configure option
--enable-debug-type-canonicalization.
It proved useful in debugging some comparison errors I was looking at
recently.
* doc/manuals/abidiff.rst: Add documentation for the new
--debug-tc option. Fix the existing documentation for
--debug-self-comparison.
* tools/abidiff.cc (options::do_debug_self_comparison): Renamed
options::do_debug into this.
(options::do_debug_type_canonicalization): Add new data member.
(display_usage): Fix help string for the --debug option that is
now --debug-self-comparison. Also, add a help string for the new
option --debug-tc option.
(main): Adjust use options::do_debug into
options::do_debug_self_comparison. Call
environment::debug_type_canonicalization() if the user provided
the --debug-tc option.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Sun, 2 Oct 2022 17:04:50 +0000 (19:04 +0200)]
ir: Fix documentation of canonical type propagation
* src/abg-ir.cc (on-the-fly-canonicalization): Enclose the comment
into <pre> html tag to make the ascii-art graph look good.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Sun, 2 Oct 2022 16:29:53 +0000 (18:29 +0200)]
Update website for 2.1 release.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 26 Sep 2022 06:32:32 +0000 (08:32 +0200)]
Bump version number to 2.2
* configure.ac: Bump version number to 2.2
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Sun, 25 Sep 2022 05:45:51 +0000 (07:45 +0200)]
Update NEWS file for 2.1
* NEWS: Update for 2.1.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Sun, 25 Sep 2022 04:11:19 +0000 (06:11 +0200)]
Update ChangeLog for 2.1 release.
* ChangeLog: Update by running "make update-changelog".
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Sat, 24 Sep 2022 12:48:55 +0000 (14:48 +0200)]
ir: Avoid cancelling a "confirmed" propagated canonical type
When canonicalizing a type T at the IR level, the canonical type of T
(aka C(T)) can be deduced in two ways:
1/ Either T structurally (aka member-wise) compares equal to another
T' which has a canonical type C(T'). In that case we deduce that
C(T) equals C(T').
2/ Or, during the canonicalization of another type Y of which T is a
sub-type, the comparison engine comes to compare T against a type
T' that is already canonicalized and finds that T structurally
compares equal to T'. In that case we deduce that C(T) equals
C(T'), even though we were canonicalizing Y in the first place.
In 2/ we C(T) is stored and so when comes the time to canonicalize T,
we already know C(T).
What happens in 2/ is called "canonical type propagation". Because the
canonical type of T' is propagated to T while canonicalizing Y.
There can be cases however where the propagated canonical type has to
be "cancelled". This is described in the source code in abg-ir.cc in
the "@ref OnTheFlyCanonicalization" chapter of the doxygen
documentation.
There are also cases where the propagated canonical type has been
"confirmed", meaning, it cannot be cancelled anymore.
Unfortunately, there seems to be (wrong) cases where confirmed
propagated canonical types are being cancelled. Oops. That leads to
having types that are missing canonical types down the road.
This is exhibited by the self-comparison check of the
avr-binutils-2.39-1.fc36.armv7hl.rpm package (more precisely when
analyzing the 'objdump' binary from that package). This was triggered
by the command:
$ fedabipkgdiff --self-compare --from fc36 avr-binutils
This patch improves the book keeping around IR canonical type
propagation to avoid cancelling confirmed propagated canonical types.
It introduces a flag to the type_base::priv sub-object to track the
confirmation status of propagated canonical types. It then updates
the book-keeping routines to make them avoid cancelling confirmed
propagated canonical types.
* src/abg-ir-priv.h
(type_base::priv::propagated_canonical_type_confirmed_): Define
new data member.
(type_base::priv::priv): Initialize it.
(type_base::priv::{propagated_canonical_type_confirmed,
set_propagated_canonical_type_confirmed}): Define new member
functions.
(type_base::priv::clear_propagated_canonical_type): Do not clear
the propagated canonical type if it has been confirmed already.
(type_base::priv::confirm_ct_propagation_for_types_dependant_on): Rename
type_base::confirm_ct into this. Mark the confirmed propagated
types as being confirmed.
(type_base::priv::confirm_ct_propagation): This is now a new member
function that calls
type_base::confirm_ct_propagation_for_types_dependant_on and that
does the book-keeping that was being done in
return_comparison_result.
(type_base::priv::cancel_ct_propagation_for_types_dependant_on):
Renamed type_base::priv::cancel_ct_propagation in this.
(type_base::priv::cancel_ct_propagation): In this new one, call
type_base::priv::cancel_ct_propagation_for_types_dependant_on. Perform here
the book-keeping that was being done in return_comparison_result.
Also, do not cancel a confirmed propagated canonical type.
(type_base::priv::add_to_types_with_non_confirmed_propagated_ct):
Define new member function.
* src/abg-ir.cc (return_comparison_result): Consider only types
with non-confirmed propagated canonical types for the
non-confirmed type queue. Also, only sub-types can be considered
non-confirmed. Split out some of the book-keeping into
type_base::priv::{confirm_ct_propagation, cancel_ct_propagation}
and call these instead. Confirm the propagated canonical types of
all types that remain after the comparison is fully done and is
successful.
(canonicalize): Assert the rule "The result of canonicalizing must
always been a confirmed canonical type".
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Thu, 22 Sep 2022 15:43:21 +0000 (17:43 +0200)]
dwarf-reader: Accept SHT_PROGBITS sections in .dynamic segment
Apparently, some shared libraries from TCL can have a section of type
SHT_PROGBITS in the .dynamic segment. get_soname_of_elf_file
unexpectedly encounters this when trying to poke at the soname of the
usr/lib/irsim/tcl/diglib.so library and asserts out. It was expecting
the section to be of type SHT_DYNAMIC, obviously. The different
between theory and practise, I guess. Fixed thus.
This fixes the run of the following command:
$ fedabipkgdiff --self-compare --from fc36 irsim
* src/abg-dwarf-reader.cc (get_soname_of_elf_file): Accept
SHT_PROGBITS sections in the dynamic segment.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Thu, 22 Sep 2022 13:26:10 +0000 (15:26 +0200)]
ir: Support cloning data members of unions
When cloning a data member, var_decl::clone wrongly assumes we are
looking at a data member of a class. It could also be a data member
of an union. Fixed thus.
This fixes crashes when self comparing Fedora 36 packages of the qt6
group with the command:
$ fedabipkgdiff --self-compare --from fc36 qt6-qtsensors
There are lot of other packages from the qt group that were making
this command fail, but I am not mentioning them here for the sake
conciseness.
* src/abg-ir.cc (var_decl::clone): Support cloning data members
for unions.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Thu, 22 Sep 2022 12:58:53 +0000 (14:58 +0200)]
Better support for golang programs
When analyzing the bettercap program written in Golang, the DWARF
reader goes into an infinite loop due to this recursive DWARF
construct:
[ 8bbf9] subroutine_type abbrev: 40
name (string) "gopkg.in/sourcemap%2ev1.fn"
byte_size (udata) 4
lo_user+0x900 (data1) 19
lo_user+0x904 (addr) +
0000000000
[ 8bc1b] formal_parameter abbrev: 41
type (ref_addr) [ 8ba8b]
[ 8bc20] formal_parameter abbrev: 41
type (ref_addr) [ 8bc4b]
[ 8bc25] formal_parameter abbrev: 41
type (ref_addr) [ 6d43e]
[ 8bc2b] typedef abbrev: 39
name (string) "gopkg.in/sourcemap%2ev1.fn"
type (ref_addr) [ 8bbf9]
[ 8bc4b] pointer_type abbrev: 43
name (string) "*gopkg.in/sourcemap%2ev1.fn"
type (ref_addr) [ 8bc2b]
lo_user+0x900 (data1) 0
lo_user+0x904 (addr) +
0000000000
Note how the typedef DIE at offset [ 8bc2b] references the function
type DIE at offset [ 8bbf9] which second parameter DIE at offset
[8bc20] has a pointer type described by the DIE [ 8bc4b]. This last
pointer type is a pointer to the typedef type which DIE has the offset
[ 8bc2b], which started this paragraph. This is a recursive
construct.
First, there is die_qualified_type_name in the DWARF reader that goes
look unnecessarily into the underlying type of a typedef. This makes
that function end-up in an infinite loop. That is especially
unfortunate because we do not need to do that to construct the name of
the typedef. This looks like an old relic of ancient unrelated code
that needs to go. This patch lets it go.
Second, when building the IR for function type, build_function_type
also ends up in a infinite loop because it's written naively. To fix
that, this patch does what we do to handle recursively defined
classes. The function type IR for that function type DIE is
"forward-declared" as being "Work In Progress" aka WIP; then when a
construct references that same DIE, the WIP IR is returned. When we
are done constructing the function type IR for that DIE, the IR is no
longer marked WIP. That way, the infinite recursion is avoided.
Now that all function types can be represented in the IR,
function_decl::get_pretty_representation_of_declarator is crashing
because it wrongly forgets that a parameter can have a function type.
The patch fixes that.
Last but not least, it appears that the name of elf symbols and
functions can contain characters that need to be escaped (to respect
the lexical rules of XML) in the emitted ABIXML. The patch fixes
that.
Together, this patch makes it so that running fedabipkgdiff to compare
packages against themselves now succeeds on the f36 distribution, for
the following Golang packages:
$ fedabipkgdiff --self-compare --from fc36 {containerd, bettercap,
apptainer, rclone, singularity}
* src/abg-dwarf-reader.cc (die_qualified_type_name): Don't look at
the underlying type unnecessarily.
(build_function_type): Look for the WIP type first to avoid
infinite recursion.
* src/abg-ir.cc
(function_decl::get_pretty_representation_of_declarator): A
parameter can have a function type.
* src/abg-writer.cc (write_elf_symbol_reference)
(write_function_decl): Escape symbol names, function names and
symbol references.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Tue, 19 Jul 2022 14:15:08 +0000 (16:15 +0200)]
comparison: Ensure that fn parms with basic types can't be redundant
When comparing the two libc binaries from
"https://the.owo.foundation/Ac1Ksw8.tgz", abidiff crashes.
It appears to be due to an assert that is hit because the overload of
default_reporter::report for fn_parm_diff momentarily removes the
redundant-ness from the categorization of the diff node of the type of
a function parameter. The problem is that the sole child diff node of
that type diff node is itself redundant. So the function parameter
type diff node should really be redundant too. Oops, there is a logic
violation there, hence the assert violation.
I commented out the line that removes the redundant-ness. After all,
if function parameter types shouldn't be redundant, that should have
been taken care of by the redundancy_marking_visitor code in
abg-comparison.cc as well as its associated category propagation code.
But then consider what happens with a reproducer of the libc binaries
above:
$ cat test-PR29387-v0.c
typedef int Integer;
void
f0(Integer i, char c)
{
i + c;
}
void
f1(Integer i, unsigned char c)
{
i + c;
}
$
$ cat test-PR29387-v1.c
typedef long int Integer;
void
f0(Integer i, char c)
{
i + c;
}
void
f1(Integer i, unsigned char c)
{
i + c;
}
$ gcc -g test-PR29387-v0.c
$ gcc -g test-PR29387-v1.c
$
$ abidiff test-PR29387-v0.o test-PR29387-v1.o
Functions changes summary: 0 Removed, 2 Changed, 0 Added functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
2 functions with some indirect sub-type change:
[C] 'function void f0(Integer, char)' at PR29387-test-v1.c:4:1 has some indirect sub-type changes:
parameter 1 of type 'typedef Integer' changed:
underlying type 'int' changed:
type name changed from 'int' to 'long int'
type size changed from 32 to 64 (in bits)
[C] 'function void f1(Integer, unsigned char)' at PR29387-test-v1.c:10:1 has some indirect sub-type changes:
$
$
So, the problem is this peace of report:
[C] 'function void f1(Integer, unsigned char)' at PR29387-test-v1.c:10:1 has some indirect sub-type changes:
You see that the report is empty; the reporter could not say what changed.
What changed is the typedef "Integer" that changed from "int" to "long
int". The redundancy_marking_visitor pass marked the change of the
underlying type of the Integer typedef as being redundant. This is
because that typedef change has already been reported on the f0
function interface.
The problem is that by design, the 'int' to 'long int' change should
not have been marked as being redundant by the
redundancy_marking_visitor pass. This is because, we want to see all
the "basic type changes" on function parameters types. They are
deemed "local changes" of the function types, and we want to see all
local changes to functions because it is almost 100% sure these are
non-compatible changes.
The root cause of the problem is that the function
has_basic_type_change_only in abg-comparison.cc fails to detect that
the parameter change carries a typedef-to-basic-type change, so the
function parameter is wrongly marked as being redundant even though it
ultimately carries a basic type change.
This patch thus teaches has_basic_type_change_only to look through
parameter changes to better detect basic type changes.
* include/abg-comparison.h (peel_fn_parm_diff)
(peel_typedef_qualified_type_or_parameter_diff): Declare ...
* src/abg-comparison.cc (peel_fn_parm_diff)
(peel_typedef_qualified_type_or_parameter_diff): ... new
functions.
(has_basic_type_change_only): Look through function parameters,
typedefs and qualified types.
* src/abg-default-reporter.cc (default_reporter::report): Remove
the temporary removal of the redundant categorisation.
Redundant-ness should have been handled by the
redundancy_marking_visitor pass.
* tests/data/test-diff-filter/test-PR29387-report.txt: Reference
test output.
* tests/data/test-diff-filter/test-PR29387-v{0,1}.c: Source of the
input tests.
* tests/data/test-diff-filter/test-PR29387-v{0,1}.o: Input test
binaries.
* tests/data/Makefile.am: Add the new test material above to the
source distribution.
* tests/test-diff-filter.cc (in_out_specs): Add the test binaries
above to the test harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Wed, 21 Sep 2022 09:08:07 +0000 (11:08 +0200)]
test-read-ctf: Update test output files after typedef canonicalization
* tests/data/test-read-ctf/test-ambiguous-struct-A.o.hash.abi:
Adjust.
* 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-linux-module.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.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 12 Sep 2022 22:10:59 +0000 (00:10 +0200)]
ir, writer: Go back to canonicalizing typedefs in the IR
Now that DIE and IR canonicalizing has progressed in precision,
especially due to fixing the canonical type propagation it seems like
we can go back to canonicalizing typedefs again. This makes the
writer not needing to handle non-canonicalized types because only a
short number and types of types are still non-canonicalized.
Then I ran the test suite and hell broke lose. Things would work on a
given platform but won't on others, etc.
So a great deal of the patch actually fixes issues uncovered by the
fact that we are back to canonicalizing typedefs.
The symptoms of many of the issues are related to emitting abixml in a
stable manner across toolchains. But in reality those issues are
real, deeper ones.
This patch is thus ALSO an attempt at fixing all the issues at once
because they can't be separated. Either the abixml is stable and
"make check" passes on all platforms, or it is not and testing fails
on some platforms.
I believe that at the core of the problems lies the observation that a
given typedef (of a given name) could appear several times (the number
of times not being quite constant) in a given class or namespace.
That would lead to some instabilities in the sorting and ordering of
type-ids depending on the toolchain being used, for instance.
To fix this add_or_update_class_type in the DWARF reader is taught to
avoid adding a typedef to a class if a member type of the same name
already exists at that class scope. This handles the fact that a
given class can be built piece-wise in the DWARF. Also, when handling
a typedef type, build_ir_node_from_die is taught to avoid creating a
typedef IR for a given scope (namespace, union or class) if said scope
already contains a type of the same name. To be able to do that, the
handling of member types is moved from the ir::class_or_union type to
the ir::scope_decl type. That way, all scopes (not just classes or
unions) can handle looking up for the (member) types they contain.
Another issue that was uncovered is that when emitting decl-only class
named A (in a namespace for instance) there can be some instability as
to which decl-only class A is emitted. This is due to the fact that
libabigail considers all decl-only classes named A to be equal. The
problem however is that a decl-only class A might have member types.
And a decl-only A declared in a translation unit somewhere might have
some member types that are different from the same decl-only A
declared in another translation unit. This doesn't necessarily
violate the ODR, but rather, can be the result of compiler
optimization. For instance, in a given translation unit, only a given
member type of the decl-only A is used by the code, whereas in another
one, another member type of the same decl-only A is used. So only
partial views of A are emitted in the translation unit depending on
what is used. Anyway, to handle that case, when comes the time to
emit the decl-only A in the ABIXML writer, write_class_decl now emits
all the member types of all the instances A known to the system. This
hopefully removes the instability I was seeing. To do that,
maybe_update_types_lookup_map<class_decl> has been taught to also keep
track of decl-only classes. A new lookup_decl_only_class_types has
been defined to gather all the instances of a given decl-only class
known to the system.
Last but not least, the topological type sorting facilities carried by
the types {decl,type}_topo_comp has been fixed to ensure a more stable
sorting and also to ensure that "abilint foo.abi" emits the decls and
types in the same order as the one defined in foo.abi. This fixes
another abixml instability I was seeing in the test suite across
different toolchains. While doing that, I noticed that the ABIXML
reader was forgetting to properly mark the corpus as originating from
ABIXML. So I fixed that too.
* include/abg-fwd.h (lookup_decl_only_class_types): Declare new
function.
* src/abg-ir-priv.h (class_or_union::priv::member_types_): Move
this data member into scope_decl::priv.
(class_or_union::priv::priv): Do not take member types.
* include/abg-ir.h (sort_type): Likewise.
(namespaces_type): Add new typedefs.
(scope_decl::insert_member_decl): Make this be non-virtual.
(scope_decl::{insert_member_type, add_member_type,
remove_member_type, get_member_types, get_sorted_member_types,
find_member_type}): Move these methods here from ...
(class_or_union::{insert_member_type, add_member_type,
remove_member_type, get_member_types, get_sorted_member_types,
find_member_type}): ... here.
(class_or_union::insert_member_decl): Make this be non-virtual.
(class_decl::insert_member_decl): Make this be non-virtual.
(class_or_union::get_sorted_member_types): Declare ...
* src/abg-dwarf-reader.cc (add_or_update_class_type): Do not add a
member type to the class type being built if it already exists
there.
(build_ir_node_from_die): If a scope (namespace, union or class)
already has a typedef, do not create a new one to add it there
again.
* src/abg-ir.cc (equals): In the overload for typedefs take into
account the name of typedefs again.
(lookup_decl_only_class_types): Define new function.
(compare_using_locations): Define new static function that has
been factorized out of ...
(decl_topo_comp::operator()): ... here. Also, two decls originate
from an abixml corpus, compare them only using their artificial
locations, meaning make them keep the same order as the order of
the original abixml file.
(type_top_comp::operator()): Likewise.
(sort_types): Define new function.
(scope_decl::priv::member_types_): Move this here from
class_or_union::priv.
(scope_decl::priv::sorted_member_types_): Define new data member.
(scope_decl::{get_member_types, find_member_type,
insert_member_type, add_member_type, remove_member_type}): Move
these methods here from class_or_union.
(scope_decl::get_sorted_member_types): Define new method.
(is_non_canonicalized_type): Canonicalize typedefs.
(lookup_type_in_map): Allow this to look through decl-only types
to get their definition. Otherwise, if only a decl-only was
found, return it.
(maybe_update_types_lookup_map<class_decl>): Allow adding
decl-only class types to the map of classes held per TU and per
corpus.
(class_or_union::{class_or_union, add_member_decl,
has_no_member}): Adjust.
(class_or_union::{insert_member_type, add_member_type,
add_member_type, remove_member_type, get_member_types,
get_sorted_member_types, find_member_type}): Move these methods to
scope_decl.
({class_decl, class_or_union}::insert_member_decl): Remove the
"before" parameter as it was not used anymore due to the re-use of
the scope_decl::add_member that handles member types.
* src/abg-reader.cc (read_corpus_group_from_input)
(create_native_xml_read_context): Set the corpus origin.
* src/abg-writer.cc (write_context::{m_nc_type_id_map,
m_emitted_non_canonicalized_type_set,
m_referenced_non_canonicalized_types_set}): Remove these maps that
hold stuff for non-canonicalized types.
(write_context::{type_has_existing_id, get_id_for_type,
clear_type_id_map, has_non_emitted_referenced_types,
record_type_as_referenced, type_is_referenced,
record_type_as_emitted, type_is_emitted, clear_referenced_types,
write_referenced_types, write_canonical_type_ids}): Adjust these
as the maps for non-canonicalized types are gone.
(write_context::{get_referenced_non_canonicalized_types,
get_emitted_non_canonicalized_type_set}): Remove these methods.
(write_decl_in_scope)
(write_class_decl_opening_tag, write_union_decl_opening_tag)
(write_union_decl): Adjust comments.
(write_class_decl): Sort member types. Also, When emitting a
decl-only class, get all of the decl-only classes of the same name
declared in several other TUs and emit all their member types into
this decl-only class just once. This reduces redundancies and
sorting instabilities because for libabigail, all decl-only
classes of a given name are equal, even if they have member types.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
* tests/data/test-annotate/libtest23.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-annotate/test-anonymous-members-0.o.abi:
Likewise.
* tests/data/test-annotate/test0.abi: Likewise.
* tests/data/test-annotate/test1.abi: Likewise.
* tests/data/test-annotate/test13-pr18894.so.abi: Likewise.
* tests/data/test-annotate/test14-pr18893.so.abi: Likewise.
* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-annotate/test2.so.abi: Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt:
Likewise.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt:
Likewise.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt:
Likewise.
* tests/data/test-diff-filter/test41-report-0.txt: Likewise.
* tests/data/test-diff-pkg/PR24690/PR24690-report-0.txt: Likewise.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Likewise.
* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Likewise.
* tests/data/test-read-dwarf/PR26261/PR26261-exe.abi: Likewise.
* tests/data/test-read-dwarf/PR27700/test-PR27700.abi: Likewise.
* tests/data/test-read-dwarf/libtest23.so.abi: Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi:
Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-read-dwarf/test-PR26568-1.o.abi: Likewise.
* tests/data/test-read-dwarf/test-PR26568-2.o.abi: Likewise.
* tests/data/test-read-dwarf/test-libaaudio.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/test0.abi: Likewise.
* tests/data/test-read-dwarf/test0.hash.abi: Likewise.
* tests/data/test-read-dwarf/test1.abi: Likewise.
* tests/data/test-read-dwarf/test1.hash.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-read-dwarf/test2.so.abi: Likewise.
* tests/data/test-read-dwarf/test2.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
* tests/data/test-read-write/test18.xml: Likewise.
* tests/data/test-read-write/test28-without-std-fns-ref.xml:
Likewise.
* tests/data/test-read-write/test28-without-std-vars-ref.xml:
Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Thu, 8 Sep 2022 17:09:33 +0000 (19:09 +0200)]
Fix IR comparison result caching and canonical type propagation tracking
Caching the result of IR comparison cannot happen on IR nodes that are
being compared in the context of type canonicalization if one of the
nodes is the target of canonical type propagation. This is especially
true if the recursive IR node which comparison "temporarily" did yield
true (to avoid an infinite loop) at least until the comparison of the
rest of the sub-tree of the recursive type is done. In that case, we
should not cache the result the comparison as it might change later.
The patch adds a way to track recursive types so that we know when not
to cache their comparison result.
As we now have a facility to track recursive types during canonical
type propagation (rather than just types that depend on recursive
types) we can be a bit more precise when confirming or cancelling
types that have been subject to canonical type propagation.
Also the patch cleans up the detection of comparison cycle to make it
more typesafe so that the macro
RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED can be used for class_decl
and union_decl, not just class_or_union. It makes the code more
readable/maintainable is the equals overload for class_decl and
union_decl all have their proper call to
RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED. The same is true for
mark_types_as_being_compared.
Doing that cleans up the detection of recursive types as well as the
types that depends on those recursive types.
* src/abg-ir-priv.h (environment::priv::recursive_types_): Define
new data member.
(environment::priv::cache_type_comparison_result): Cache results
only non-recursive and not dependant types, or when the result is
"false". In all these cases, the result, once cached, will not
change.
(environment::priv::mark_dependant_types_compared_until): Mark
types as recursive at the same time others are marked as
dependant.
(environment::priv::{is_recursive_type, set_is_not_recursive}):
Define new member functions.
(environment::priv::{confirm_ct_propagation,
cancel_ct_propagation}): Confirming canonical type propagation
should happen for recursive types as well as their dependant
types.
* src/abg-ir.cc (return_comparison_result): Keep up with the
book-keeping at all time when type canonicalization process is
on-going. Whenever we expect types that depends on recursive
types, expect recursive types too, obviously.
(type_base::get_canonical_type_for): Do not erase the comparison
result cache between the canonicalization of two different types.
(is_comparison_cycle_detected)
(mark_types_as_being_compared, unmark_types_as_being_compared):
Define new overloads for class_decl;
(equals): In the overloads for class_decl and union_decl, use
RETURN_COMPARISON_RESULT and mark_types_as_being_compared without
casting it to class_or_union.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
Adjust.
* tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.txt:
Adjust.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Adjust.
* tests/data/test-diff-pkg/tbb-4.1-9.
20130314.fc22.x86_64--tbb-4.3-3.
20141204.fc23.x86_64-report-0.txt:
Adjust.
* tests/data/test-diff-pkg/tbb-4.1-9.
20130314.fc22.x86_64--tbb-4.3-3.
20141204.fc23.x86_64-report-1.txt:
Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Fri, 2 Sep 2022 14:19:23 +0000 (16:19 +0200)]
Allow restricting analyzed decls to exported symbols
Profiling showed that the DWARF reader scans too much data.
Basically, in build_translation_unit_and_add_to_ir,
build_ir_node_from_die is called on every single DIE that is seen, for
a given translation unit.
There are interfaces (function and variable decls) that are not
associated with exported ELF symbols and that are analyzed by
build_ir_node_from_die nonetheless. For instance, interfaces that are
visible outside of their translation units are analyzed and the types
that are reachable from those interfaces are analyzed as well.
Once that is done, an ABI corpus is built with the subset of
interfaces that have exported ELF symbol (strictly those that are part
of the ABI), but types that are not necessarily reachable from those
ABI interfaces can also be put into the ABI corpus.
Some tools make use of this "lose" behaviour of libabigail. For
instance, abicompat precisely wants to analyze interfaces with
undefined symbols. For an application, those interfaces represents
the interfaces that the application expects to be provided by some
shared library.
When analyzing the exported interface of the Linux Kernel (or any
other huge application) however, analyzing more types than necessary
appears to incur a huge time penalty.
So, this patch introduces an optional behaviour whereby
build_translation_unit_and_add_to_ir is restricted to analyzing
interfaces that have exported ELF symbols only. So only the types
reachable from those interfaces are analyzed. This more than halves
the time spent by "abidw --noout vmlinux".
Strictly speaking, this new behaviour is triggered by a new option named
--exported-interfaces-only, supported by the tools abidw, abidiff,
abipkgdiff and kmidiff.
When looking at the Linux Kernel however, this option is enabled by
default.
Note that an option --allow-non-exported-interfaces is also introduce
to function under the previous model of operations. This option is
enabled by default on all the tools when they are not looking at the
Linux Kernel.
With this enabled, analyzing the Linux Kernel is back to taking less
than a minute on a reasonable machine.
* doc/manuals/tools-use-libabigail.txt: New doc text.
* doc/manuals/Makefile.am: Add the new tools-use-libabigail.rst
tool to the source distribution.
* doc/manuals/abidiff.rst: Include the new
tools-use-libabigail.rst. Document the --exported-interfaces-only
and --allow-non-exported-interfaces.
* doc/manuals/abidw.rst: Likewise.
* doc/manuals/abipkgdiff.rst: Likewise.
* doc/manuals/kmidiff.rst: Likewise.
* include/abg-ir.h
(environment::{user_set_analyze_exported_interfaces_only,
analyze_exported_interfaces_only}): Declare new accessors.
* src/abg-ir.cc
(environment::{user_set_analyze_exported_interfaces_only,
analyze_exported_interfaces_only}): Define new accessors.
* src/abg-dwarf-reader.cc (die_is_variable_decl)
(die_is_function_decl): Define new static functions.
(read_context::is_decl_die_with_exported_symbol): Define new
member function.
(read_context::get_{function,variable}_address): Const-ify the
Dwarf_Die* parameter.
(build_translation_unit_and_add_to_ir): If the user asks to
analyze exported interfaces only, the analyze only interfaces
that have exported ELF symbols.
(read_debug_info_into_corpus): If we are looking at the Linux
Kernel, then only analyze exported interfaces unless the user asks
otherwise.
* src/abg-ir-priv.h
(environment::priv::analyze_exported_interfaces_only_): Define new
data member.
* tools/abidiff.cc (options::exported_interfaces_only): Define new
data member.
(display_usage): Add new help strings for
--exported-interfaces-only and --allow-non-exported-interfaces.
(parse_command_line): Parse the new options
--exported-interfaces-only and --allow-non-exported-interfaces.
(main): Pass the value of opts.exported_interfaces_only to the
environment.
* tools/abidw.cc (options::exported_interfaces_only): Define new
data member.
(display_usage): Add new help strings for
--exported-interfaces-only and --allow-non-exported-interfaces.
(parse_command_line): Parse the new options
(load_corpus_and_write_abixml)
(load_kernel_corpus_group_and_write_abixml): Pass the value of
opts.exported_interfaces_only onto the environment.
* tools/abipkgdiff.cc (options::exported_interfaces_only): Define new
data member.
(display_usage): Add new help strings for
--exported-interfaces-only and --allow-non-exported-interfaces.
(parse_command_line): Parse the new options
(compare_task::perform, self_compare_task::perform): Pass the
value of opts.exported_interfaces_only onto the environment.
(compare_prepared_linux_kernel_packages): Likewise.
* tools/kmidiff.cc(options::exported_interfaces_only): Define new
data member.
(display_usage): Add new help strings for
--exported-interfaces-only and --allow-non-exported-interfaces.
(parse_command_line): Parse the new options
(main): Pass the value of opts.exported_interfaces_only onto the
environment.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
projects / platform / upstream / libabigail.git / log