v2: More code simplification. Tests unchanged.
There is distributed responsibility for horizontal and vertical
whitespace in the reporting code with indent and new line control
information being manipulated by and passed in and out of functions.
Occasionally, this information is ignored or incorrect and the code
tends to err on the side of more rather than fewer new lines.
The outcome is that abidiff output sometimes contains extra blank
lines which can be confusing.
This patch eliminates some of the more obvious cases:
- after data member deletions
- in enumerator change lists
- after "type size hasn't changed"
- before listing impacted interfaces
A lot of passing of "new line needed" booleans between functions has
been eliminated in the process.
The patch cleans up the reporting of data members. The code will
either emit indentation, a short description and a new line or do
nothing at all.
The patch also removes some stray location reporting code for array
diffs which would have produced some oddly placed output. I could not
get this code to trigger as loc = decl->get_location() was never
present on the array decls in question.
* src/abg-default-reporter.cc (report): In the type_decl_diff,
enum_diff, array_diff, class_diff, union_diff and var_diff
overrides, simplify new line logic which no longer needs to be
threaded through report_name_size_and_alignment_changes. In
the distinct_diff override, simplify new line logic which no
longer needs to be threaded through
report_size_and_alignment_changes. In the enum_diff override,
emit just one blank line after each enum. In the array_diff
override, remove stray location reporting which doesn't appear
to ever trigger; fix new line logic. In the
class_or_union_diff override, simplify new line logic for
deleted members; pass indentation to represent_data_member.
* src/abg-leaf-reporter.cc (report): In the array_diff,
class_diff, union_diff and var_diff overrides, simplify new
line logic which no longer needs to be threaded through
report_name_size_and_alignment_changes. In the distinct_diff
override, simplify new line logic which no longer needs to be
threaded through report_size_and_alignment_changes. In the
array_diff override, remove stray location reporting which
doesn't appear to ever trigger; fix new line handling. In the
class_or_union_diff override, simplify new line logic for
deleted members; pass indentation to represent_data_member.
In the corpus_diff override, tabify source indentation.
* src/abg-reporter-priv.cc (represent_data_member): Handle
indentation; fix new line logic.
(report_size_and_alignment_changes): Fix new line logic
for "type size hasn't changed" message; simplify new line
logic and replace local bool n with argument bool nl for
clarity.
(report_size_and_alignment_changes): Remove bool nl argument
and associated code as it had become always false; take
responsibility for emitting terminating new lines and change
return type to void.
(report_name_size_and_alignment_changes): Fix new line logic;
remove bool nl argument and associated code as it had become
always false; take responsibility for emitting terminating new
lines and change return type to void.
(maybe_report_interfaces_impacted_by_diff) In both overrides,
remove new line prefix code and new_line_prefix argument.
* src/abg-reporter-priv.h (represent_data_member): Add indent
argument.
(report_size_and_alignment_changes) Remove bool nl argument;
change return type to void.
(report_name_size_and_alignment_changes) Remove bool nl
argument; change return type to void.
(maybe_report_interfaces_impacted_by_diff) In both overrides,
remove new_line_prefix argument.
* tests/data/test-*/*report*.txt: Remove some blank lines.
Signed-off-by: Giuliano Procida <gprocida@google.com>
The --redundant (meaning show-redundant-changes) option is supposed to
be implied by --leaf-changes-only and this is currently implemented by
making diff_context's --leaf-changes-only setter also duplicate the
behaviour of its --redundant setter.
In both abidiff and abipkgdiff, the diff_context setters are called
unconditionally, but the relative order of the calls for these two
options is different in each case, resulting in two different issues.
In abidiff, the --redundant setter is called second, undoing the
intended side-effect of any --leaf-changes-only flag. So --redundant
is not actually turned on in --leaf-changes-only mode unless requested
explicitly.
In abipkgdiff, the leaf-changes-only setter is called second, undoing
(in non-leaf mode) the effect of any --redundant flag. So --redundant
has no effect in default reporting mode.
The fix is move to move the "--leaf-changes-only implies --redundant"
logic from the setter to the set_diff_context_from_opts functions.
This patch also documents the implied behaviour in the usage strings.
* src/abg-comparison.cc (diff_context::show_leaf_changes_only):
Remove "--leaf-changes-only implies --redundant" logic.
* tools/abidiff.cc (display_usage): Mention that
--leaf-changes-only implies --redundant.
(set_diff_context_from_opts): Make --leaf-changes-only imply
--redundant; document this behaviour in a comment.
* tools/abipkgdiff.cc: Ditto.
* tests/data/Makefile.am: Add new test case files.
* tests/data/test-abidiff-exit/test-leaf3-report.txt: Add new
test case, to show --leaf-changes-only implies --redundant.
* tests/data/test-abidiff-exit/test-leaf3-v0.c: Ditto.
* tests/data/test-abidiff-exit/test-leaf3-v0.o: Ditto.
* tests/data/test-abidiff-exit/test-leaf3-v1.c: Ditto.
* tests/data/test-abidiff-exit/test-leaf3-v1.o: Ditto.
* tests/test-abidiff-exit.cc: Run new test case.
* tests/data/test-diff-pkg/libcdio-0.94-1.fc26.x86_64--libcdio-0.94-2.fc26.x86_64-report.1.txt:
Update abipkgdiff report with --redundant output.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-1.txt:
Ditto.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Ditto.
Signed-off-by: Giuliano Procida <gprocida@google.com>
The top-level corpus diff routines in abidiff have varied ways of
tracking whether or not to emit a new line after each section. Reuse
of state variables (which aren't always cleared) between sections
means that spurious new lines are sometimes output.
This patch replaces this new line logic in the functions with the same
simple pattern of using a local boolean state variable.
* src/abg-default-reporter.cc (report): In the corpus_diff
overload, just use a local boolean emitted state variable
within each section to determine whether or not to follow the
section with an extra new line.
* src/abg-leaf-reporter.cc: Ditto.
* tests/data/test-*/*report*.txt: Remove unwanted new lines
from 27 files.
Signed-off-by: Giuliano Procida <gprocida@google.com>
All such tags are now followed by a space and are more readable.
* src/abg-default-reporter.cc (report): In the overload for
corpus_diff, output space after "[C]".
* src/abg-leaf-reporter.cc (report): Likewise.
* tests/data/test-*/*report*.txt: Update all the test
reports.
Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This is a follow-up patch for the commit:
43d56de Handle several member anonymous types of the same kind
It allows support for severan anonymous types even when these are not
members of a class/unions.
The patch introduces the concept of a scoped name. It's a qualified
name for a decl made of the name of the decl appended to the
*unqualified* name of its scope. Unlike for qualified names, the
scoped name won't have a "__anonymous_*__" string in its name if its
directly containing scope is not anonymous; a qualified name might
still have that string in its name because the decl has a parent scope
(not necessarily its directly containing scope though) that is
anonymous.
The patch goes on to update the logic for comparison of decls that are
anonymous. For a decl which direct scope is *NOT* anonymous, the
scoped name is what's used in the comparison. Otherwise, only the
name of the decl is used.
The patch also updates how we detect changes in data members and
member types, in the comparison engine. It now uses the names of the
data members, rather than their qualified name. This is in the scope
of the current class/union anyway. The improvement is that the fact
that the class/union itself is anonymous (even if its anonymous name
changes to another anonymous name) won't have any spurious impact on
the detection of name change of the members.
The patch considers the change of an anonymous decl name which
anonymous name changes to another anonymous name as being harmless.
The patch updates the logic of category propagation in the comparison
engine. Although a public typedef to private underlying type needs to
stay public and thus not propagate the PRIVATE_TYPE_CATEGORY from its
child diff node to himself, it still needs to suppress the changes to
the private underlying diff node that were suppressed (because of the
private-ness), unless that typedef has local changes.
* include/abg-ir.h (decl_base::get_scoped_name): Declare new
member function.
(scope_decl::get_num_anonymous_member_{classes, unions, enums}):
Declare new virtual member functions.
(class_decl::get_num_anonymous_member_{classes, unions, enums}):
Adjust to make these virtual. It's not necessary but I feel
redundancy is a kind of self-documentation here.
* src/abg-comp-filter.cc (has_harmless_name_change): Consider
anonymous name changes as harmless.
* src/abg-comparison.cc
(class_or_union_diff::ensure_lookup_tables_populated): Consider
the names of the members rather than their qualified names.
(suppression_categorization_visitor::visit_end): Suppress the
changes to the private underlying diff node that were suppressed
because of the private-ness, unless that typedef has local
changes.
* src/abg-dwarf-reader.cc (build_enum_type)
(add_or_update_class_type, add_or_update_union_type): Handle
anonymous types in namespaces as well, not just in class/unions.
* src/abg-ir.cc (decl_base::priv::scoped_name_): Define new data
member.
(decl_base::get_scoped_name): Define new member function.
(equals): For the decl_base overload, use scoped name in the
comparison, unless the decl belongs to an anonymous type. For the
class_or_union_diff, only consider scoped_name during comparison.
Avoid name comparison between anonymous types.
(scope_decl::get_num_anonymous_member_{classes, unions, enums}):
Define new member functions.
(types_have_similar_structure): Do not compare names between
anonymous types.
(qualified_name_setter::do_update): Update scoped names too.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
* tests/data/test-annotate/libtest23.so.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/test21-pr19092.so.abi: Likewise.
* tests/data/test-diff-dwarf/test43-PR22913-report-0.txt:
Likewise.
* tests/data/test-diff-dwarf/test46-rust-report-0.txt: Likewise.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.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-0.txt:
Likewise.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt:
Likewise.
* tests/data/test-diff-filter/test33-report-0.txt: Likewise.
* tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-0.txt:
Likewise.
* tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt:
Likewise.
* 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:
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/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.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/libtest23.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/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/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>
When the type of a variable changes and that change is only a CV qual
change, we want libabigail to automatically classify that change as
being harmless.
This patch thus introduces a new VAR_TYPE_CV_CHANGE_CATEGORY change
category (enumerator of the abigail::comparison::diff_category enum)
for this kind of changes and classifies that category as being
harmless.
The patch then detects diff nodes that carry CV qualifiers change on
variable types and flags them as belonging to the new
VAR_TYPE_CV_CHANGE_CATEGORY.
* include/abg-comparison.h (VAR_TYPE_CV_CHANGE_CATEGORY): Add new
enumerator to diff_category enum.
(EVERYTHING_CATEGORY): Update this enumerator.
* src/abg-comp-filter.cc (type_diff_has_cv_qual_change_only):
Support array diff nodes carrying a cv qual change on the element
type.
(has_var_type_cv_qual_change): Define new static function.
(categorize_harmless_diff_node): Use the new
has_var_type_cv_qual_change to categorize variable diff node with
cv qual change on its type as harmless.
* src/abg-comparison.cc
(get_default_harmless_categories_bitmap): Update this.
(operator<<(ostream& o, diff_category c)): Likewise.
* include/abg-ir.h (equals_modulo_cv_qualifier): Declare new ...
* src/abg-ir.cc (equals_modulo_cv_qualifier): ... function.
* 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:
Update expected test output.
* 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/Makefile.am: Add the new test material below to
source distribution.
* tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.txt:
New expecte test output.
* tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64.rpm: New
test input.
* tests/data/test-diff-pkg/nss-3.24.0-1.0.fc23.x86_64.rpm: Likewise.
* tests/data/test-diff-pkg/nss-debuginfo-3.23.0-1.0.fc23.x86_64.rpm: Likewise.
* tests/data/test-diff-pkg/nss-debuginfo-3.24.0-1.0.fc23.x86_64.rpm: Likewise.
* tests/data/test-diff-pkg/nss-devel-3.23.0-1.0.fc23.x86_64.rpm: Likewise.
* tests/data/test-diff-pkg/nss-devel-3.24.0-1.0.fc23.x86_64.rpm: Likewise.
* tests/test-diff-pkg.cc (in_out_specs): Add the test input above
to the test harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When referring to an anonymous struct or union, libabigail used to
name it __anonymous_struct__ or __anonymous_union__.
Now, with this patch, libabigail rather uses the flat representation
of the struct/union, e.g, struct {int foo; char bar;}.
* src/abg-ir.cc (get_class_or_union_flat_representation): Take a
const class_or_union* (like what the declaration in the header
file says), rather than just a class_or_union*.
({class,union}_decl::get_pretty_representation): For anonymous
classes and unions, use the flat representation.
* tests/data/test-annotate/libtest23.so.abi: Adjust.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-annotate/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/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-diff-dwarf/test43-PR22913-report-0.txt: Likewise.
* tests/data/test-diff-dwarf/test44-anon-struct-union-report-0.txt:
New test reference output.
* tests/data/test-diff-dwarf/test44-anon-struct-union-v{0,1}.cc:
Source code of new test binary outputs.
* tests/data/test-diff-dwarf/test44-anon-struct-union-v{0,1}.o:
New test binary outputs.
* tests/data/Makefile.am: Add the new test materials above to
source districution.
* tests/test-diff-dwarf.cc (in_out_specs): Add the new test
material above to the test harness here.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.txt: Adjust.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report1.txt: Likewise.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report2.txt: Likewise.
* tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-0.txt: Likewise.
* tests/data/test-diff-pkg/libcdio-0.94-1.fc26.x86_64--libcdio-0.94-2.fc26.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-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
An anonymous data member is a data member of a struct or a union which
has no name. The type of such data member is either a struct or a
union. For instance:
struct foo {
int a;
struct { // <-- this is an anonymous data member
char a;
char b;
};
int c;
};
In DWARF (as emitted by GCC at least), an anonymous data member is
represented as a data member with an empty name. Libabigail sees it
just fine, but then when representing *changes* to that kind of data
member, it needs special treatment, otherwise users cannot make sense
of the reports.
This patch adds initial support to represent changes to anonymous data
members.
* include/abg-comparison.h (is_class_or_union_diff)
(is_anonymous_class_or_union_diff): Declare new functions.
* include/abg-fwd.h (is_class_type): Declare new overload for
type_or_decl_base&.
(is_data_member): Declare new overload for decl_base*.
(is_anonymous_data_member)
(anonymous_data_member_to_class_or_union)
(get_class_or_union_flat_representation)
(data_member_has_anonymous_type): Declare new functions.
(is_at_class_scope): Return the class or union scope.
* include/abg-ir.h (var_decl::get_qualified_name): New virtual
data member which overloads decl_base::get_qualified_name.
* src/abg-comparison.cc (is_class_or_union_diff)
(is_anonymous_class_or_union_diff): Define new functions
(leaf_diff_node_marker_visitor::visit_begin): Don't mark anonymous
class or union diff nodes as diff nodes.
* src/abg-ir.cc (is_data_member): Define new overload for
decl_base*.
(is_class_type, is_union_type): Define new overload for type_or_decl_base&.
(is_anonymous_data_member)
(anonymous_data_member_to_class_or_union)
(get_class_or_union_flat_representation)
(data_member_has_anonymous_type): Define new function overloads.
(var_decl::get_qualified_name): Define new virtual member
function.
(is_at_class_scope): Return the class or union scope.
(var_decl::get_pretty_representation): Support anonymous data
members.
(equals): In the overload for class_or_union_diff, mark data
member textual representation changes as local changes.
* src/abg-reporter-priv.cc (represent): In the overload for
var_diff, support changes to anonymous data members.
* src/abg-leaf-reporter.cc (leaf_reporter::report): Report sorted
-- by offset -- data member changes before the ones that are
sorted by other things.
* tests/data/test-diff-filter/libtest44-anonymous-data-member-v{0,1}.so:
New binary test input
* tests/data/test-diff-filter/test44-anonymous-data-member-report-{0,1}.txt:
New reference test outputs.
* tests/data/test-diff-filter/test44-anonymous-data-member-v{0,1}.c:
Source code of the new binary test output above.
* tests/data/Makefile.am: Add the new test files above to the
source distribution.
* tests/data/test-annotate/libtest23.so.abi: Adjust test reference
output.
* 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/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/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-diff-dwarf/test43-PR22913-report-0.txt: Likewise.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.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-0.txt: Likewise.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt: Likewise.
* tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-0.txt: Likewise.
* tests/data/test-diff-pkg/libcdio-0.94-1.fc26.x86_64--libcdio-0.94-2.fc26.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.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When a class is forward-declared, resolving it to a definition that
appears later in the same translation unit or in another translation
is an interesting problem.
Until now, the declaration would be resolved to the definition of that
class found in the binary. The problem is that there can be different
such definitions, especially in C where there is no "One Definition
Rule". In that case, the definition chosen is random.
This patch resolves that randomness.
For a given class declaration, if there is just one possible
definition in the binary, then the declaration is resolved to that
definition. If there is one definition for that declaration in the
same translation unit, then the declaration is resolved to that
definition. If there are more than one definitions in translation
units that are not the one of the declaration, then the declaration is
left unresolved. This is what I call "selective class declaration resolution".
Note that an unresolved class declaration now compares different to a
definition of a class of the same name. This is so that we can have
an unresolved class be present in the resulting .abi file, alongside
an (incompatible) definition of the same class. The change from a class
declaration to its definition is filtered out by default, though.
* include/abg-fwd.h (type_base_wptrs_type)
(istring_type_base_wptrs_map_type): Define new typedefs.
(lookup_class_types): Declare new functions.
* include/abg-ir.h
(environment::decl_only_class_equals_definition): Declare new
accessor.
(type_maps::{*_types}): Make these accessors return
istring_type_base_wptrs_map_type& instead of
istring_type_base_wptr_map_type&.
* src/abg-dwarf-reader.cc
(read_context::resolve_declaration_only_classes): Implement the
new selective declaration resolution scheme.
* src/abg-ir.cc (type_maps::priv::{*_types_}): Change the type of
these data members from istring_type_base_wptr_map_type to
istring_type_base_wptrs_map_type.
(type_maps::{*_types}): Make these accessors definitions return
istring_type_base_wptrs_map_type& instead of
istring_type_base_wptr_map_type&.
(translation_unit::bind_function_type_life_time): Adjust.
(environment::priv::decl_only_class_equals_definition_): New data
member.
(environment::priv::priv): Initialize it. By default, a decl-only
class is now considered different from its definition.
(environment::decl_only_class_equals_definition): Define new
accessor.
(lookup_types_in_map, lookup_class_types): Define new functions.
(lookup_type_in_map, lookup_union_type_per_location)
(lookup_basic_type, lookup_basic_type_per_location)
(lookup_class_type, lookup_class_type_per_location)
(lookup_union_type, lookup_enum_type)
(lookup_enum_type_per_location, lookup_typedef_type)
(lookup_typedef_type_per_location, lookup_qualified_type)
(lookup_pointer_type, lookup_reference_type, lookup_array_type)
(lookup_function_type, maybe_update_types_lookup_map)
(maybe_update_types_lookup_map<class_decl>)
(maybe_update_types_lookup_map<function_type>): Adjust.
(type_base::get_canonical_type_for): When doing type comparison
here, we can now consider that an unresolved class declaration
compares different to an incompatible class definition of the same
name. So no need to look through decl-only classes in that case.
(equals): In the overload for class_or_union, if
environment::decl_only_class_equals_definition() is false, then an
unresolved class declaration of name "N" compares different to a
class definition named "N".
* tests/data/test-annotate/test15-pr18892.so.abi: Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Adjust.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Adjust.
* tests/data/test-diff-dwarf/test28-vtable-changes-report-0.txt:
Adjust.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Adjust.
* tests/data/test-diff-filter/test38/Makefile: New test material.
* tests/data/test-diff-filter/test38/test38-a.c: Likewise.
* tests/data/test-diff-filter/test38/test38-b.c: Likewise.
* tests/data/test-diff-filter/test38/test38-c.c: Likewise.
* tests/data/test-diff-filter/test38/test38-report-0.txt: Likewise.
* tests/data/test-diff-filter/test38/test38-v0: Likewise.
* tests/data/test-diff-filter/test38/test38-v1: Likewise.
* tests/data/test-diff-filter/test38/test38.h: Likewise.
* tests/data/test-diff-filter/test39/Makefile: Likewise.
* tests/data/test-diff-filter/test39/test39-a-v0.c: Likewise.
* tests/data/test-diff-filter/test39/test39-a-v1.c: Likewise.
* tests/data/test-diff-filter/test39/test39-b-v0.c: Likewise.
* tests/data/test-diff-filter/test39/test39-b-v1.c: Likewise.
* tests/data/test-diff-filter/test39/test39-c-v0.c: Likewise.
* tests/data/test-diff-filter/test39/test39-c-v1.c: Likewise.
* tests/data/test-diff-filter/test39/test39-main.c: Likewise.
* tests/data/test-diff-filter/test39/test39-report-0.txt: Likewise.
* tests/data/test-diff-filter/test39/test39-v0: Likewise.
* tests/data/test-diff-filter/test39/test39-v1: Likewise.
* tests/data/test-diff-filter/test39/test39.h: Likewise.
* tests/data/Makefile.am: Add the new test material above to the
source distribution.
* tests/test-diff-filter.cc (in_out_specs): Add the new test
inputs above to the test harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch introduces the concept of canonical DIEs.
To date, when two C types have the same string representation [1] and
are defined at the same location, they are considered to be the same,
just as in C++.
[1]: String representation of a type: For a type named
"Foo" that is a structure, the representation is "struct Foo"
This patch introduces more finesse in determining if two types are
different.
It implements comparing the DIEs of the types, directly from the DWARF
representation. When comparing pointers, typedefs and qualified
types, the underlying type is compared recursively.
The type de-duplication scheme is now centered around two data
structures.
1/ A map that associates the string representation of a type with
a vector of the offsets of the type DIEs that have the same
representation. Members of this vector denotes DIEs of types that
are *different* even if they all have the same representation.
Each DIE in a given vector is the canonical DIE of its class of
equivalence. This map is the map of all canonical DIEs organized
by the representation of those canonical DIEs.
2/ A map that associates the offset of the canonical type DIE with
the resulting internal representation of the type. Here, the
internal representation is an instance of the
abigail::ir::type_base type. This map is the map of the types
associated to the canonical type DIEs.
There is also a vector that associates a DIE 'D' to its canonical DIE
'C'. The index of the vector is the offset of 'D' and the value of
the element at that index is 'C'.
Thus, each time we are about to create (or get) an internal
representation for a type DIE denoted 'D', we first get the canonical
DIE of 'D', denoted C. If C doesn't exist, we create it. That is, we
add a new entry in the map 1/. Then we look in map 2/ to see if the C
(and thus D) has a associated type.
If C has an associated type, we return it.
If C has no associated type, we create a type for it (and thus for D)
and we associate the new type to the offsets of both D and C.
The rest of the patch is mostly boiler plate and adjustment to
accomodate this new de-duplication scheme.
* src/abg-dwarf-reader.cc (die_decl_map_type, die_type_map_type):
Remove these typedefs.
(die_artefact_map_type, istring_dwarf_offsets_map_type): New
typedefs.
(die_is_at_class_scope, die_qualified_type_name)
(die_qualified_decl_name, die_qualified_type_name_empty)
(die_return_and_parm_names_from_fn_type_die)
(die_function_type_is_method_type):
Const-ify the read_context& parameter.
(read_context::die_source_dependant_container_set::get_container):
Likewise.
(read_context::{name_artefacts_map_, per_tu_name_artefacts_map_,
die_decl_map_, alternate_die_decl_map_, type_unit_die_decl_map_,
die_type_map_, alternate_die_type_map_, type_unit_die_type_map_}):
Remove data members.
(read_context::{die_decl_map, alternate_die_decl_map,
associate_die_to_decl_primary, associate_die_to_decl_alternate,
associate_die_to_decl_from_type_unit,
lookup_decl_from_die_offset_primary,
lookup_decl_from_die_offset_alternate,
lookup_decl_from_type_unit_die_offset,
lookup_type_artifact_from_die_per_tu,
lookup_artifact_from_per_tu_die_representation,
associate_die_to_artifact_by_repr,
associate_die_to_artifact_by_repr_internal, clear_die_type_maps}):
Remove member functions.
(read_context::{decl_die_repr_die_offsets_maps_,
type_die_repr_die_offsets_maps_, decl_die_artefact_maps_,
type_die_artefact_maps_, dwarf_expr_eval_context_}): Add new data
members.
(read_context::clear_per_translation_unit_data): Don't clear
read_context::per_tu_name_artefacts_map_ data member as it's
removed.
(read_context::clear_per_corpus_data): Don't clear
read_context::name_artefacts_map_ and all the other relevant data
members that got removed.
(read_context::{dwarf_per_die_source,
decl_die_repr_die_offsets_maps, type_die_repr_die_offsets_maps,
get_canonical_die, get_die_from_offset, decl_die_artefact_maps,
type_die_artefact_maps, dwarf_expr_eval_ctxt}): Add new member
functions.
(compare_dies, compare_as_decl_dies)
(compare_as_type_dies, maybe_finish_function_decl_reading)
(die_is_anonymous): Define new functions.
(read_context::associate_die_to_decl): Remove the
do_associate_by_repr_per_tu parameter. Use the new
read_context::{decl_die_artefact_maps_, get_canonical_die} member
functions.
(read_context::lookup_decl_from_die_offset): Use Dwarf_Off rather
than size_t for the type of the die_offset parameter. Use the
lookup_artifact_from_die_offset member function.
(read_context::lookup_type_artifact_from_die): Const-ify. In one
overload, take a new 'die_as_type' parameter. Use the new
get_canonical_die, type_die_artefact_maps and
decl_die_artefact_maps member functions. In the second overload,
use the first overload.
(read_context::odr_is_relevant): Add an overload that takes a DIE.
(read_context::associate_die_to_type): Remove the
do_associate_by_repr and do_associate_per_tu parameters. Use the
new get_canonical_die and type_die_artefact_maps member functions.
(read_context::lookup_type_from_die): Use the new
lookup_artifact_from_die member function.
(read_context::lookup_type_from_die_offset): Use the new
type_die_artefact_maps member function. When the found artifact
is a function_decl, return its type.
(read_context::schedule_type_for_late_canonicalization): Use the
new get_canonical_die and type_die_artefact_maps member functions.
(die_function_signature): Const-ify. Get the scope name right
even for scopes that are not types.
(die_member_offset): Make eval_last_constant_dwarf_sub_expr use
the new cached DWARF expression evalution context.
(get_parent_die): Support where_offset equals to zero. This means
we are looking at a C binary, basically.
(build_enum_type) : Use the new overload of
read_context::odr_is_relevant that takes a DIE. Adjust.
(add_or_update_union_type, add_or_update_class_type): Don't lookup
classes/unions per location anymore. Now that we can compare DIEs
in a fined grain manner, the approximation of the location is not
useful anymore.
(build_pointer_type)
(build_function_type): Associate DIE to type if we reuse an
existing type.
(build_or_get_fn_decl_if_not_suppressed): When re-using a
function decl internal representation from an equivalent DIE that
we've seen before, it can happen that we want to augment that
function decl internal representation with new properties coming
from the DIE we are currently looking at; do that here.
(is_function_for_die_a_member_of_class): Remove the "where_offset"
parameter.
(add_or_update_member_function): Adjust.
* tests/data/test-annotate/libtest23.so.abi: Adjust.
* tests/data/test-annotate/test13-pr18894.so.abi: Adjust.
* tests/data/test-annotate/test14-pr18893.so.abi: Adjust.
* tests/data/test-annotate/test15-pr18892.so.abi: Adjust.
* tests/data/test-annotate/test17-pr19027.so.abi: Adjust.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Adjust.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Adjust.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Adjust.
* tests/data/test-annotate/test21-pr19092.so.abi: Adjust.
* tests/data/test-diff-pkg/libICE-1.0.6-1.el6.x86_64.rpm--libICE-1.0.9-2.el7.x86_64.rpm-report-0.txt: Adjust.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-0.txt: Adjust.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-1.txt: Adjust.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt: Adjust.
* tests/data/test-read-dwarf/libtest23.so.abi: Adjust.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Adjust.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Adjust.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Adjust.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Adjust.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Adjust.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Adjust.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Adjust.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Adjust.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Adjust.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Adjust.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Adjust.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Under certain circumstances, a diff node (which we shall name N) that
belongs to the SUPPRESSED_CATEGORY category sees its belonging to that
category been propagated to its parent diff node, effectively
suppressing that parent diff node as well. This is how some function
diff nodes ends up being suppressed just because some of their
children diff nodes were suppressed.
This suppression category propagation is performed by a pass that
walks the diff nodes graph. To avoid infinite cycles, the pass avoids
visiting a diff node that is equivalent to a node that has already
been visited.
As a result, diff nodes equivalent to N (the class of equivalence of
N) are not traversed by the propagation pass. So their parent diff
nodes are not suppressed.
This leads to some functions not being suppressed as they should.
This phenomenon can be observed by comparing the two packages that are
provided in the regression test accompanying this patch using
abipkgdiff with the --redundant option.
Here is how the patch addresses the issue.
When the suppression category propagation pass considers a parent diff
node (named P) of a node N' (with N' being equivalent to N), it now
looks at the category of the *class of equivalence of N'* to determine
if that category should be propagated to P.
Previously, that pass would just look at the at the category of N'
(not the category of the class of equivalence of N') for the
propagation. But as N' has not been visited (because N has already
been visited and nodes equivalent to N are thus not visited) the
belonging of N' to the SUPPRESSED_CATEGORY hasn't been updated. So N'
isn't marked as being in SUPPRESSED_CATEGORY. So the
SUPPRESSED_CATEGORY wouldn't be propagated to P as it should.
So, with this patch abipkgdiff invoked with the --redundant option now
correctly yields:
$ abipkgdiff --redundant --d1 spice-debuginfo-0.12.4-19.el7.x86_64.rpm --d2 spice-debuginfo-0.12.8-1.el7.x86_64.rpm --devel1 spice-server-devel-0.12.4-19.el7.x86_64.rpm --devel2 spice-server-devel-0.12.8-1.el7.x86_64.rpm spice-server-0.12.4-19.el7.x86_64.rpm spice-server-0.12.8-1.el7.x86_64.rpm
================ changes of 'libspice-server.so.1.8.0'===============
Functions changes summary: 1 Removed, 2 Changed (62 filtered out), 8 Added functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
[...]
2 functions with some indirect sub-type change:
[C]'function spice_image_compression_t spice_server_get_image_compression(SpiceServer*)' at reds.c:3618:1 has some indirect sub-type changes:
return type changed:
underlying type 'enum __anonymous_enum__' changed:
type size hasn't changed
7 enumerator deletions:
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_INVALID' value '0'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_OFF' value '1'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_AUTO_GLZ' value '2'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_AUTO_LZ' value '3'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_QUIC' value '4'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_GLZ' value '5'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_LZ' value '6'
9 enumerator insertions:
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_INVALID' value '0'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_OFF' value '1'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_AUTO_GLZ' value '2'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_AUTO_LZ' value '3'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_QUIC' value '4'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_GLZ' value '5'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_LZ' value '6'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_LZ4' value '7'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_ENUM_END' value '8'
[C]'function int spice_server_set_image_compression(SpiceServer*, spice_image_compression_t)' at reds.c:3602:1 has some indirect sub-type changes:
parameter 2 of type 'typedef spice_image_compression_t' changed:
underlying type 'enum __anonymous_enum__' changed:
type size hasn't changed
7 enumerator deletions:
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_INVALID' value '0'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_OFF' value '1'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_AUTO_GLZ' value '2'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_AUTO_LZ' value '3'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_QUIC' value '4'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_GLZ' value '5'
'__anonymous_enum__::SPICE_IMAGE_COMPRESS_LZ' value '6'
9 enumerator insertions:
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_INVALID' value '0'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_OFF' value '1'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_AUTO_GLZ' value '2'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_AUTO_LZ' value '3'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_QUIC' value '4'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_GLZ' value '5'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_LZ' value '6'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_LZ4' value '7'
'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_ENUM_END' value '8'
================ end of changes of 'libspice-server.so.1.8.0'===============
$
* include/abg-comparison.h (diff::get_class_of_equiv_category):
Declare new member function.
* src/abg-comparison.cc: Update copyright year.
(diff::get_class_of_equiv_category): Define new member function.
(suppression_categorization_visitor::visit_end): When considering
children nodes category for propagation, consider the category of
the class of equivalence of children nodes.
* spice-debuginfo-0.12.4-19.el7.x86_64.rpm: New input test package.
* spice-debuginfo-0.12.8-1.el7.x86_64.rpm: Likewise.
* spice-server-0.12.4-19.el7.x86_64.rpm: Likewise.
* spice-server-0.12.8-1.el7.x86_64.rpm: Likewise.
* spice-server-devel-0.12.4-19.el7.x86_64.rpm: Likewise.
* spice-server-devel-0.12.8-1.el7.x86_64.rpm: Likewise.
* spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-0.txt:
New reference test output.
* spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-1.txt: Likewise.
* spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt: Likewise.
* tests/data/Makefile.am: Add the new test material above to
source distribution.
* tests/test-diff-pkg.cc (in_out_specs): Add the new test inputs
to the list of packages to test.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Giuliano Procida
Dodji Seketeli