Many of the operator== definitions in this source file follow the same
pattern:
- the address of the argument is dynamic_cast to type of 'this'
- naked canonical type pointers are compared, if both present
- the types are compared structurally with 'equals'
In a couple of cases extra work is done to fetch the canonical type
of the definition of a declaration.
This commit adjusts a few cases so they more closely follow the common
form. This is to make the next refactoring trivial.
There are no behavioural changes.
* src/abg-irc.cc (scope_type_decl::operator==): Compare naked
canonical type pointers instead of the shared pointers.
(qualified_type_def::operator==): Remove excess blank line.
(function_type::operator==): Do dynamic_cast and check of
argument before comparing naked canonical type pointers.
(class_or_union::operator==): Eliminate temporary reference.
(class_decl::operator==): Likewise.
(union_decl::operator==): Likewise.
This patch makes corpus_diff::has_net_changes mirror emit_diff_stats,
modulo flags like --non-reachable-types which if absent can still
result in discrepancies between output and return code.
To achieve this in a more maintainable way, the patch introduces a new interface
reporter_base::diff_has_net_changes. That interface is implemented by
all current reporters. Each reporter focuses on its own
particularities to provide the required behavious. Then
corpus_diff:has_net_changes just has to invoke
reporter_base::diff_has_net_changes on the reporter that is currently
in used.
The tests below verify that the exit code is zero when all the changes
between the test files are suppressed.
* include/abg-reporter.h ({reporter_base, default_reporter,
leaf_reporter}::diff_has_net_changes): Add new virtual function.
This breaks binary compatibility but should conserve source
compatibility.
* src/abg-default-reporter.cc
(default_reporter::diff_has_net_changes): Define new member
function.
* src/abg-leaf-reporter.cc (leaf_reporter::diff_has_net_changes):
Likewise.
* src/abg-comparison.cc (corpus_diff::has_net_changes): Invoke
reporter_base::diff_has_net_changes on the current reporter,
rather than trying to handle all the different kinds of reporters
here.
(corpus_diff::priv::apply_filters_and_compute_diff_stats): Add a
TODO to possibly delegate the implementation of this function to
the reporters.
* tests/data/Makefile.am: Add new test case files.
* tests/data/test-abidiff-exit/test-net-change-report0.txt:
Normal mode, nothing suppressed.
* tests/data/test-abidiff-exit/test-net-change-report1.txt:
Normal mode, everything suppressed.
* tests/data/test-abidiff-exit/test-net-change-report2.txt:
Leaf mode, nothing suppressed.
* tests/data/test-abidiff-exit/test-net-change-report3.txt:
Leaf mode, everything suppressions.
* tests/data/test-abidiff-exit/test-net-change-v0.c: Test file
* tests/data/test-abidiff-exit/test-net-change-v0.o: Test file
* tests/data/test-abidiff-exit/test-net-change-v1.c: Test file
* tests/data/test-abidiff-exit/test-net-change-v1.o: Test file
* tests/data/test-abidiff-exit/test-net-change.abignore: This
suppresses changes for all variables, functions and types in
the test files, except for the 'victim' function.
* tests/test-abidiff-exit.cc: Run new test cases.
* src/abg-reporter-priv.cc (represent): Add missing space to
string split across two lines in certain anonymous data member
diffs.
* src/abg-default-reporter.cc (report): In the array_diff
overload, eliminate trailing space at end of line.
* tests/data/test-diff-dwarf/test10-report.txt: Delete
trailing whitespace.
* tests/data/test-diff-filter/test24-compatible-vars-report-1.txt:
Ditto.
* 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:
Ditto.
* tests/data/test-diff-pkg/libcdio-0.94-1.fc26.x86_64--libcdio-0.94-2.fc26.x86_64-report.1.txt:
Ditto.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.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.
The XML writer has a few different styles of new line handling in
different places. Some functions are responsible for line termination,
others are not and there is bespoke logic and state variables in a few
places.
Extra or missing newlines should have no impact on the semantics of
any given ABI file but they do affect textual diffs and diff
statistics.
By insisting the XML emitted should have exactly one XML tag (or
comment) per line, we can simplify the code and make it more
composable.
This commit does this, yielding a modest reduction in code size and
eliminating all blank lines in XML output (7127 blank lines in current
tests). The commit also fixes some code whitespace.
* src/abg-writer.cc (annotate): In the
function_decl::parameter_sptr overload, fix code whitespace.
(write_decl_in_scope): Remove wrote_context state variable and
associated logic; emit new line unconditionally after end of
XML tags and nowhere else.
(write_canonical_types_of_scope): Emit new line after end of
XML comment and nowhere else.
(write_translation_unit): Emit new line after end of XML tags
and nowhere else.
(write_type_decl): Likewise.
(write_namespace_decl): Likewise.
(write_qualified_type_def): Emit new line after end of XML tag.
(write_pointer_type_def): Likewise.
(write_reference_type_def): Likewise.
(write_array_type_def): Emit new line after end of XML tags
and nowhere else.
(write_enum_type_decl): Emit new line after end of XML tag.
(write_elf_symbol): Likewise.
(write_elf_symbols_table): Emit no new lines.
(write_elf_needed): Emit new line unconditionally after end of
XML tags.
(write_typedef_decl): Emit new line after end of XML tag.
(write_var_decl): Emit new line after end of XML tag.
(write_function_decl): Likewise.
(write_function_type): Fold two output statements into
one; emit new line after end of XML tag.
(write_class_decl_opening_tag): Emit new line unconditionally
after end of XML tags and simplify empty element tag logic.
(write_union_decl_opening_tag): Likewise.
(write_class_decl): Emit new line after end of XML tag and
nowhere else.
(write_union_decl): Likewise.
(write_member_type_opening_tag): Emit new line after end of
XML tag.
(write_member_type): Emit new lines only after XML tags.
(write_type_tparameter): Emit new line after XML tag.
(write_non_type_tparameter): Likewise.
(write_template_tparameter): Emit new line after XML tag and
nowhere else.
(write_type_composition): Likewise.
(write_template_parameters): Emit no new lines.
(write_function_tdecl): Emit new line after XML tag and
nowhere else.
(write_class_tdecl): Likewise.
(write_corpus): Emit new lines only after XML tags.
(dump): In the decl_base_sptr overload, don't emit final new
line as this is now done by write_decl. In the var_decl_sptr
overload, don't emit final new line (mistakenly done to cerr
instead of o) as this is now done by write_var_decl. In the
translation_unit overload, don't emit final new line as this
doubles that emitted by write_translation_unit.
* tests/data/test-annotate/libtest23.so.abi: Delete all blank
lines.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Ditto.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Ditto.
* tests/data/test-annotate/test-anonymous-members-0.o.abi:
Ditto.
* tests/data/test-annotate/test1.abi: Ditto.
* tests/data/test-annotate/test13-pr18894.so.abi: Ditto.
* tests/data/test-annotate/test14-pr18893.so.abi: Ditto.
* tests/data/test-annotate/test15-pr18892.so.abi: Ditto.
* tests/data/test-annotate/test17-pr19027.so.abi: Ditto.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Ditto.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Ditto.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Ditto.
* tests/data/test-annotate/test21-pr19092.so.abi: Ditto.
* tests/data/test-annotate/test7.so.abi: Ditto.
* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
Ditto.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Ditto.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Ditto.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Ditto.
* tests/data/test-read-dwarf/libtest23.so.abi: Ditto.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi:
Ditto.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Ditto.
* tests/data/test-read-dwarf/test1.abi: Ditto.
* tests/data/test-read-dwarf/test1.hash.abi: Ditto.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Ditto.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Ditto.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Ditto.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Ditto.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Ditto.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Ditto.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Ditto.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Ditto.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Ditto.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Ditto.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Ditto.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Ditto.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Ditto.
* tests/data/test-read-dwarf/test7.so.abi: Ditto.
* tests/data/test-read-dwarf/test7.so.hash.abi: Ditto.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi:
Ditto.
* tests/data/test-read-write/test10.xml: Ditto.
* tests/data/test-read-write/test15.xml: Ditto.
* tests/data/test-read-write/test21.xml: Ditto.
* tests/data/test-read-write/test25.xml: Ditto.
* tests/data/test-read-write/test28-without-std-fns-ref.xml:
Ditto.
* tests/data/test-read-write/test28-without-std-vars-ref.xml:
Ditto.
In the abixml reader, WIP types are tracked to know if a type has been
fully constructed yet or not. This information is later useful to
know if a given type should be canonicalized right away, or if its
canonicalization should be delayed until the entire abixml file has
been read.
Right now, with all the evolutions that happened in the abixml reader,
only scalar types are canonicalized right away. All other types are
canonicalized late, meaning, after the entire abixml file is read.
This doesn't have any noticeable performance impact because the volume
of types coming from an abixml file is relatively small enough,
compared to what we can see in a DWARF/ELF binary due to type
duplication.
So the whole WIP tracking becomes is now pretty much useless, in
practise. So this patch does away with it altogether.
* src/abg-reader.cc (read_context::m_wip_types_map): Remove data
member.
(read_context::{clear_wip_classes_map, mark_type_as_wip,
unmark_type_as_wip, is_wip_type}): Remove member functions.
(read_context::maybe_canonicalize_type): Remove use of
is_wip_type.
Giuliano Procida [Fri, 19 Jun 2020 16:39:24 +0000 (17:39 +0100)]
Use pointers not strings in type graph comparison.
During structural comparison of types there is the possibilitiy of
infinite recursion as types can have self-references and there can
be more elaborate mutual references between them.
The current comparison algorithm keeps track of currently seen (struct
and function) types by name. This causes earlier caching of names than
is needed and, more significantly, may result in types comparing equal
unexpectedly. This commit switches to storing their addresses instead.
This change affects some tests which show more diffs than previously.
src/abg-ir.cc: (environment::priv): Change types of
classes_being_compared_ and fn_types_being_compared_ to be
simple sets of pointers.
(function_type::priv::mark_as_being_compared): Just add
address to set.
(function_type::priv::unmark_as_being_compared): Just remove
address from set.
(function_type::priv::comparison_started): Just look up
address in set.
(class_or_union::priv::mark_as_being_compared): Just add
address to set.
(class_or_union::priv::unmark_as_being_compared): Just remove
address from set.
(class_or_union::priv::comparison_started): Just look up
address in set.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
Update.
* tests/data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64-multiple-sym-vers-report-0.txt:
Update.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Update.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt:
Update.
Giuliano Procida [Wed, 10 Jun 2020 11:59:40 +0000 (12:59 +0100)]
Add tests for declaration-only enums.
* tests/data/test-abidiff-exit/test-decl-enum-report-2.txt: Add
new test reference output.
* tests/data/test-abidiff-exit/test-decl-enum-report-3.txt: Likewise.
* tests/data/test-abidiff-exit/test-decl-enum-report.txt: Likewise.
* tests/data/test-abidiff-exit/test-decl-enum-v{0,1}.c: Add source
code for the binaries below.
* tests/data/test-abidiff-exit/test-decl-enum-v{0,1}.o: Add new
binary test inputs.
* tests/data/Makefile.am: Add the new files above to source
distribution.
* tests/test-abidiff-exit.cc: Add the test inputs above to the
test harness.
Giuliano Procida [Wed, 10 Jun 2020 11:59:39 +0000 (12:59 +0100)]
Add declaration-only enums to XML reader/writer.
* src/abg-reader.cc (build_enum_type_decl): Detect a
declaration-only enum and flag it as such.
(build_type_decl): Support reading the "is-declaration" attribute.
(build_class_decl): Adjust.
* src/abg-writer.cc (write_is_declaration_only): Renamed
write_class_or_union_is_declaration_only into this.
(write_enum_is_declaration_only): Remove.
(write_type_decl, write_enum_type_decl)
(write_class_decl_opening_tag, write_union_decl_opening_tag): Use
write_is_declaration_only.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
Dodji Seketeli [Wed, 10 Jun 2020 11:59:38 +0000 (12:59 +0100)]
Support declaration-only enums in DWARF reader.
This patch adds declaration-only handling enums to the DWARF reader.
* src/abg-dwarf-reader.cc (string_enums_map): Define new
convenience typedef.
(read_context::decl_only_enums_map_): Define new data member.
(read_context::{declaration_only_enums,
is_decl_only_enum_scheduled_for_resolution,
resolve_declaration_only_enums}): Define new member functions.
(build_internal_underlying_enum_type_name)
(build_enum_underlying_type): Factorize these functions out of ...
(build_enum_type): ... here. Detect a decl-only enum and flag it
as such. If the enum type is decl-only, then set its underlying
type as decl-only as well.
(build_enum_underlying_type): Mark the underlying type as
artificial.
(get_opaque_version_of_type): Make this handle enums as well. So
make its return type be type_or_decl_base_sptr, rather than just
class_or_union_sptr as it used to be.
(read_debug_info_into_corpus): Add logging to trace decl-only
enums resolution.
(build_ir_node_from_die): Detect when a suppression specification
makes an enum opaque. In that case, get an opaque version of the
enum type by invoking get_opaque_version_of_type. Note that
get_opaque_version_of_type doesn't support returning opaque
-- i.e, decl-only enum types -- yet, but this is going to be
handled in a subsequent patch.
Giuliano Procida [Wed, 10 Jun 2020 11:59:35 +0000 (12:59 +0100)]
Support incomplete enums in core and diff code.
This is an initial implementation of the support for incomplete, also
known as forward-declared, enum types. I've not made any attempt to
refactor or share logic with the struct/union code.
* include/abg-comp-filter.h (has_decl_only_def_change) : Declare
New function.
* src/abg-comp-filter.cc (there_is_a_decl_only_enum): Define new
static function and ...
(type_size_changed): ... use it here.
(has_decl_only_def_change): Define new function and ...
(categorize_harm{less, ful}_diff_node): ... use it here.
* include/abg-fwd.h (enums_type, decl_base_wptr): Declare new
typedefs.
(look_through_decl_only_class): Declare new overload for
class_or_union*.
(is_compatible_with_enum_type, is_compatible_with_enum_type)
(look_through_decl_only, lookup_enum_types, lookup_enum_types):
Declare new functions.
* include/abg-ir.h (decl_base::{get_is_declaration_only,
set_is_declaration_only, set_definition_of_declaration,
get_definition_of_declaration,
get_naked_definition_of_declaration}): Declare new member
functions. They were moved here from the class_or_union class.
(class_or_union::{get_earlier_declaration,
set_earlier_declaration, get_definition_of_declaration,
set_definition_of_declaration,
get_naked_definition_of_declaration, get_is_declaration_only,
set_is_declaration_only}): Remove these member functions.
* src/abg-ir.cc (decl_base::priv::{declaration_,
definition_of_declaration_, naked_definition_of_declaration_,
is_declaration_only_}): Define data members. Moved here from
class_or_union.
(decl_base::priv::priv): Adjust to initialize the new data
members.
(decl_base::{get_earlier_declaration, set_earlier_declaration,
get_definition_of_declaration,
get_naked_definition_of_declaration, get_is_declaration_only,
set_is_declaration_only, set_definition_of_declaration}): Define
member functions.
(operator|): In the overload for (change_kind, change_kind),
adjust the return type of the call to
decl_base::get_definition_of_declaration.
(look_through_decl_only): Define new function.
(look_through_decl_only_class): Adjust.
(look_through_decl_only_enum): Likewise.
(maybe_update_types_lookup_map<class_decl>): Adjust return type of
call to decl_base::get_definition_of_declaration.
(types_defined_same_linux_kernel_corpus_public): Use
look_through_decl_only_class rather than open coding it.
(class_or_union::priv::{declaration_, definition_of_declaration_,
naked_definition_of_declaration_, is_declaration_only_}): Remove
these data members. They are now carried by decl_base::priv.
(class_or_union::{g,s}et_alignment_in_bits): Adjust.
(class_or_union::{g,s}et_size_in_bits): Likewise.
(class_or_union::operator==): Likewise.
(equals): Adjust the overload for class_or_union.
(is_compatible_with_enum_type)
* src/abg-comparison.cc (try_to_diff<class_decl>): Adjust the
return type of decl_base::get_definition_of_declaration.
(leaf_diff_node_marker_visitor::visit_begin): Use
filtering::has_decl_only_def_change rather than
filtering::has_class_decl_only_def_change. Decl-only changes to
enums (or any other type really) will thus not be recorded as leaf
changes.
* src/abg-dwarf-reader.cc (get_scope_for_die): Adjust return type
of decl_base::get_definition_of_declaration.
* src/abg-default-reporter.cc (default_reporter::report): Report
enum decl-only <-> definition changes.
* src/abg-hash.cc (class_or_union::hash::operator()): In the
overload for class_or_union& adjust the return type for
decl_base::get_definition_of_declaration.
Dodji Seketeli [Wed, 24 Jun 2020 16:16:40 +0000 (18:16 +0200)]
Bug 26135 - Wrong linkage name causes anonymous classes miscomparison
When comparing decls, the overload of the 'equals' function for
instances of decl_base compares their linkage names. If they are
different, then the decls are generally considered different.
Class declarations (and definitions) also use the 'equals' function
referred to above. So when two classes have different linkage names,
they are always considered different.
Now let's consider the case of an anonymous class. It doesn't have
any user-provided name, by definition. Libabigail does, however,
assigns it an internal name for various (internal) purposes. That
internal name is generally ignored for the purpose of (anonymous) type
comparison. So by design, two anonymous classes can have different
internal anonymous names and yet still happen to be equal.
The root issue in this problem report is that by default, the linkage
name of a class is set to its name. And when that class is anonymous,
its internal name is used as its linkage name. Oops. That leads to
anonymous classes being wrongly considered different.
This patch fixes the issue by providing additional constructors for a
class type to avoid using the internal anonymous name as its linkage
name.
Note that the same issue is present for unions so the patch does the
a similar thing for union types.
Enums are properly handled so we don't need to do anything in that
regard.
For good measure, the patch also adds an assert to
type_base::get_canonical_types_for to ensure that anonymous class or
union types don't have linkage names for now.
* include/abg-ir.h (class_decl::class_decl): Add two overloads
that take the "is_anonymous" flag.
(union_decl::union_decl): Likewise.
* src/abg-ir.cc (class_decl::class_decl): Define two overloads
that take the "is_anonymous" flag and set the linkage name
accordingly.
(union_decl::union_decl): Likewise.
(type_base::get_canonical_type_for): Assert that an anonymous
class or union can't have a linkage name for now.
* src/abg-dwarf-reader.cc (add_or_update_class_type)
(add_or_update_union_type): Use a new overload for the constuctor
of {class, union}_decl and set the "is_anonymous" flag. Don't use
decl_base::set_is_anonymous anymore.
* src/abg-reader.cc (build_class_decl, build_union_decl):
Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
Giuliano Procida [Mon, 29 Jun 2020 15:24:34 +0000 (17:24 +0200)]
Rename declaration-definition change category.
This patch renames CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY to
TYPE_DECL_ONLY_DEF_CHANGE_CATEGORY.
* include/abg-comparison.h (TYPE_DECL_ONLY_DEF_CHANGE_CATEGORY):
Rename CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY into this.
(EVERYTHING_CATEGORY): In the value of this enumerator, rename
CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY into
TYPE_DECL_ONLY_DEF_CHANGE_CATEGORY.
* src/abg-comp-filter.cc (categorize_harmless_diff_node):
Likewise.
* src/abg-comparison.cc (get_default_harmless_categories_bitmap):
Likewise.
(operator<<(ostream& o, diff_category c)): Likewise.
* src/abg-default-reporter.cc (default_reporter::report): Likewise
in the overload for class_or_union_diff.
* src/abg-leaf-reporter.cc (leaf_reporter::report): Likewise in
the overload for class_or_union_diff.
Giuliano Procida [Wed, 10 Jun 2020 11:59:33 +0000 (12:59 +0100)]
Tidy build_enum_type state variables.
This patch brings the enum code closer to the class/union code, in the
hope that this will ease future code maintenance.
There are no behavioural changes.
* src/abg-dwarf-reader.cc (build_enum_type): Rename local
variable enum_is_anonymous to is_anonymous. Move initilisation
of local variable is_artificial to location corresponding to
that in add_or_update_class_type and add_or_update_union_type
functions.
Giuliano Procida [Wed, 10 Jun 2020 11:59:31 +0000 (12:59 +0100)]
Improve code comments and whitespace.
These are zero impact changes.
* include/abg-fwd.h: Correct doc-comment reference to
enum_type_decl.
* src/abg-comp-filter.cc: Fix doc-comment syntax.
* src/abg-comparison.cc (operator<<): In the diff_category
overload, fix code indentation.
* src/abg-default-reporter.cc (report): In the
class_or_union_diff overload, adjust comment to reflect that
the code is reporting changes between declaration-only and
defined types, in either direction.
The method type_base::get_canonical_type_for contains some logic which
temporarily changes a couple of control flags in the type's
environment. It then restores these, but not consistently.
This patch ensures the flags are restored unconditionally.
* src/abg-ir.cc (get_canonical_type_for): Ensure the
do_on_the_fly_canonicalization and
decl_only_class_equals_definition flags are restored
unconditionally.
Dodji Seketeli [Thu, 18 Jun 2020 11:29:42 +0000 (13:29 +0200)]
Bug 26127 - abidw --annotate emits incomplete function types
When we get the qualified name of a pointer type, the result is cached
so that subsequent invocations of the getter yields a faster result.
When the pointed-to type is not yet fully constructed at the time of
the first invocation of the getter of the qualified name, what is
cached is the name of the pointer to a non-yet fully qualified type.
Then after the pointed-to type is fully constructed (and
canonicalized), the pointer type also becomes canonicalized (in that
order) and thus, the cache needs to be invalidated so that the
qualified name of the pointer to the fully qualified type is cached
again by a subsequent invocation of the getter.
The problem in this problem report is that the cache doesn't get
invalidated when the pointer type is canonicalized.
This patch fixes that. A similar issue exists with reference and
qualified types so the patch addresses it for those types as well.
* include/abg-ir.h (decl_base::clear_qualified_name): Declare new
protected member function.
({pointer_type_def, reference_type_def, qualified_type_def,
function_type}::on_canonical_type_set): Declare virtual member
functions.
* src/abg-ir.cc (decl_base::clear_qualified_name): Define new
protected member function.
({pointer_type_def, reference_type_def, qualified_type_def,
function_type}::on_canonical_type_set): Define virtual member
functions.
* tests/data/test-annotate/test17-pr19027.so.abi: Adjust.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
Giuliano Procida [Thu, 11 Jun 2020 19:18:53 +0000 (20:18 +0100)]
Fix bug that suppressed DWARF read tests.
Commit 4252dfd6 added code to allow the ABI write/reread and compare
phases of the tests to be skipped in the case that no ABI files are
given for comparison.
Unfortunately, the new code skipped those phases unconditionally.
This patch changes the in_abi_path and out_abi_path values used in
in_out_specs used to trigger the early termination from "" to NULL and
updates the conditional logic checking them. Several subsequent
commits which affect ABI output were missing these changes to the test
data files.
This change fixes the following list of commits.
4252dfd6 dwarf-reader: handle symtab.section_header.sh_entsize == 0 4457c10e dwarf-reader: handle binaries with missing symtab 34e867e7 dwarf-reader: remove superfluous ABG_ASSERT 2d5389f2 Fix size calculations for multidimensional arrays. 246ca200 corpus/writer: sort emitted translation units by path name e8bf5b80 Bug 25989 - type_topo_comp doesn't meet irreflexive requirements
Finally, this commit also corrects some bad code formatting.
* tests/test-read-dwarf.cc (in_out_specs): Use NULL instead of
empty ABI paths for test25, test26 and test27. (perform):
Check members of spec, rather than locals with same name, when
deciding to terminate testing early; fix some code whitespace.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Update
multidimensional array sizes.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Ditto.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Ditto.
* tests/data/test-read-dwarf/test7.so.abi: Ditto.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Update following translation unit ordering change.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Ditto.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Ditto.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Ditto.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Ditto.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Ditto.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Ditto.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Ditto.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Ditto.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Ditto.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Ditto.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Update
following code changes affecting ordering of some ABI
elements.
* tests/data/test-read-dwarf/test16-pr18904.so.abi
on std::weak_ptr<T> can be simplified by using std::weak_ptr<T>::lock.
Since weak_ptr::lock does this atomically, this patch also addresses
potential data races between the call to expired() and the construction
based on the assumption that the shared_ptr is still around.
Hence, apply this simplification/fix to the code base.
Giuliano Procida [Tue, 16 Jun 2020 12:12:10 +0000 (13:12 +0100)]
Fix leaf-mode formatting of decl <-> defn diffs.
This commit adds a missing newline.
* src/abg-leaf-reporter.cc (leaf_reporter::report): In the
class_or_union_diff overload, add a terminating new line after
reporting a declaration-only <-> definition difference.
Giuliano Procida [Mon, 15 Jun 2020 21:07:41 +0000 (22:07 +0100)]
abg-writer: Add support for stable hash type ids.
The type ids currently emitted by the XML writer are simply type-id-1,
type-id-2 etc. Additions or removals of types early in this sequence
result in cascading changes to many other XML elements.
This commit adds support for stable type ids in the form of hashes of
libabigail's internal type names. On fairly rare occasions (typically
involving unnamed types), the names of two distinct types can be the
same. In any case, if there is a hash collision the XML writer will
find the next unused id and so preserve uniqueness.
Diffs between large XML files produced using --type-id-style hash will
be much smaller and easier to review.
This also commit adds some test cases to verify that the hashing is
actually stable across architectures.
* doc/manuals/abidw.rst: Replace stray documentation of
--named-type-ids with documention of new --type-id-style
option.
* include/abg-writer.h (type_id_style_kind): Add new enum.
(set_type_id_style): Add new write_context setter.
(set_common_options): Set type id style in write context.
* include/abg-hash.h (fnv_hash): Declare new 32-bit FNV-1a
hash function in abigail::hashing namespace.
* src/abg-hash.h (fnv_hash): Define new 32-bit FNV-1a hash
function in abigail::hashing namespace.
* src/abg-writer.cc (write_context): Add m_type_id_style
member to record type style to use, defaulting to
SEQUENCE_TYPE_ID_STYLE; add m_used_type_id_hashes to record
already-used hashes.
(write_context::get_type_id_style): Add new getter.
(write_context::set_type_id_style): Add new setter.
(get_id_for_type): Add support for HASH_TYPE_ID_STYLE style.
(set_type_id_style): Add new helper function.
* tools/abidw.cc (options): Add type_id_style member.
(display_usage): Add description of --type-id-style option.
(parse_command_line): Parse --type-id-style option.
* tests/data/Makefile.am: Add new hash type id ABI files.
* tests/test-read-dwarf.cc: (InOutSpec): Add type_id_style
member.
(in_out_specs): Set type_id_style to SEQUENCE_TYPE_ID_STYLE in
existing test specifications. Duplicate first 9 test cases
with type_id_style set to HASH_TYPE_ID_STYLE.
* tests/data/test-read-dwarf/test0.hash.abi: New ABI XML file
with hash type ids.
* tests/data/test-read-dwarf/test1.hash.abi: Ditto.
* tests/data/test-read-dwarf/test2.so.hash.abi: Ditto.
* tests/data/test-read-dwarf/test3.so.hash.abi: Ditto.
* tests/data/test-read-dwarf/test4.so.hash.abi: Ditto.
* tests/data/test-read-dwarf/test5.o.hash.abi: Ditto.
* tests/data/test-read-dwarf/test6.so.hash.abi: Ditto.
* tests/data/test-read-dwarf/test7.so.hash.abi: Ditto.
* tests/data/test-read-dwarf/test8-qualified-this-pointer.so.hash.abi:
Ditto.
Giuliano Procida [Fri, 12 Jun 2020 10:17:01 +0000 (11:17 +0100)]
Eliminate non-ASCII characters.
There are few non-ASCII characters in various code comments. A few are
typos and the rest have obvious ASCII equivalents. This commit
replaces them all with ASCII characters.
* include/abg-diff-utils.h: Replace "’’" with "'".
* src/abg-elf-helpers.cc: Replace "â‹…" with ".".
* src/abg-ini.cc: Replace "@êef" with "@ref".
* src/abg-ir.cc: Ditto.
* src/abg-tools-utils.cc: Replace "–" with "-".
dwarf-reader: get_die_source: always initialize return value
GCC9 with ABIGAIL_DEVEL=1 and ABIGAIL_DEBUG=1 set, regularly emits
-Werror=maybe-uninitialized for the values gathered by get_die_source.
As a counter measure, some of them were initialized before the call to
NO_DEBUG_INFO_DIE_SOURCE, but not all of them, leading to said warning.
In order to overcome this, let get_die_source always initialize the
source to NO_DEBUG_INFO_DIE_SOURCE and adjust the caller to consistently
not do that anymore. This solves the warning and maybe but unlikely (due
to the ABG_ASSERT) avoids some UB.
Changing the interface of get_die_source to return the source directly,
lead to the ability to initialize the value const and to defer the
assertion to the function itself. Some occurrences could be removed
entirely as the die_source was not used at all.
abg-reporter: fully qualify std::string and std::ostream
In order to not unnecessarily pollute the abigail namespace and to avoid
ambiguity at compile time, fully qualify std::string and std::string.
This also resolves the issue that include/abg-reporter.h could not be
compiled standalone and therefore was broken as a public header. It
required a `using std::string` from a prior include.
* include/abg-reporter.h (reporter_base::report) fully qualify
std::string and std::ostream for all overloads.
(default_reporter::report_local_typedef_changes): Likewise.
(default_reporter::report_local_qualified_type_changes): Likewise.
(default_reporter::report_local_reference_type_changes): Likewise.
(default_reporter::report_local_function_type_changes): Likewise.
(default_reporter::report): Likewise.
(leaf_reporter::report_changes_from_diff_maps): Likewise.
(leaf_reporter::report): Likewise.
Suggested-by: Mark Barolak <mbar@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
The enumerator HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY has a spelling
mistake in it. This changes fixes this and also corrects the spelling
of both this and HARMLESS_UNION_CHANGE_CATEORY when output.
* include/abg-comparison.h (enum diff_category): Rename
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY enumerator to
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEGORY.
* src/abg-comp-filter.cc (categorize_harmless_diff_node): Replace
use of HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY enumerator with
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEGORY.
* src/abg-comparison.cc (get_default_harmless_categories_bitmap):
Replace use of HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY enumerator
with HARMLESS_SYMBOL_ALIAS_CHANGE_CATEGORY.
(operator<<): In the diff_category overload, replace use of
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY enumerator with
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEGORY and output correct
spelling for both this and HARMLESS_UNION_CHANGE_CATEGORY.
* tools/abicompat.cc (create_diff_context): Replace use of
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY enumerator with
HARMLESS_SYMBOL_ALIAS_CHANGE_CATEGORY.
dwarf-reader: read_context: drop unused symbol versioning code
Since the symbol versioning helper code has been moved to elf_helpers in
commit 9815efe7a5be ("abg-elf-helpers: move some versioning helpers
from abg-dwarf-reader"), remove the leftover unused code from
read_context.
* src/abg-dwarf-reader.cc
(read_context::versym_section_): Delete.
(read_context::verdef_section_): Delete.
(read_context::verneed_section_): Delete.
(read_context::symbol_versionning_sections_loaded_): Delete.
(read_context::symbol_versionning_sections_found_): Delete.
(read_context::initialize): Remove initializations of deleted members.
(read_context::get_symbol_versionning_sections): Delete.
(read_context::get_version_for_symbol): Delete.
(read_context::lookup_elf_symbol_from_index): Use
get_version_for_symbol provided by elf_helpers.
Cleanup code that is never used to reduce overall compiled code and
complexity. Some are leftovers from prior refactorings, some are unused
overloads where the more suitable overload is still left.
Add (Catch based) test suite to test kernel symbol table (ksymtab)
reading through the result persisted in abigail::corpus.
The test cases are created through simple kernel module C source files
targeting the desired properties ((gpl) exported / local functions /
variables). The kernel binaries are built without debug information to
keep them reasonably small and reproducible. That is just enough
sufficient to analyze ksymtabs. The Makefile that comes with them
recreates the test cases from the sources, given a kernel source tree
with the appropriate version, e.g.
make KDIR=/path/to/4.14
This covers ksymtab reading and ensuring we detect the kernel binary
correctly. The kernel versions are selected based on features introduced
into the Linux kernel that affect the ksymtab representation.
- Linux v4.14 - reasonably old kernel to start with (actually v4.14.180)
- Linux v4.19 - first version having position relative relocations
(PREL) in ksymtab entries on some platforms
(actually v4.19.123)
- Linux v5.4 - first version having symbol namespaces (actually v5.4.41)
- Linux v5.6 - latest released stable kernel as of writing (actually v5.6.13)
* tests/data/Makefile.am: add new test data for runtestsymtab
* tests/data/test-symtab/kernel-4.14/Makefile: New test case makefile.
* tests/data/test-symtab/kernel-4.14/empty.c: Likewise.
* tests/data/test-symtab/kernel-4.14/one_of_each.c: Likewise.
* tests/data/test-symtab/kernel-4.14/single_function.c: Likewise.
* tests/data/test-symtab/kernel-4.14/single_function_gpl.c: Likewise.
* tests/data/test-symtab/kernel-4.14/single_variable.c: Likewise.
* tests/data/test-symtab/kernel-4.14/single_variable_gpl.c: Likewise.
* tests/data/test-symtab/kernel-4.14/empty.ko: New test data.
* tests/data/test-symtab/kernel-4.14/one_of_each.ko: Likewise.
* tests/data/test-symtab/kernel-4.14/single_function.ko: Likewise.
* tests/data/test-symtab/kernel-4.14/single_function_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-4.14/single_variable.ko: Likewise.
* tests/data/test-symtab/kernel-4.14/single_variable_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-4.19/Makefile: New test case makefile.
* tests/data/test-symtab/kernel-4.19/empty.c: Likewise.
* tests/data/test-symtab/kernel-4.19/one_of_each.c: Likewise.
* tests/data/test-symtab/kernel-4.19/single_function.c: Likewise.
* tests/data/test-symtab/kernel-4.19/single_function_gpl.c: Likewise.
* tests/data/test-symtab/kernel-4.19/single_variable.c: Likewise.
* tests/data/test-symtab/kernel-4.19/single_variable_gpl.c: Likewise.
* tests/data/test-symtab/kernel-4.19/empty.ko: New test data.
* tests/data/test-symtab/kernel-4.19/one_of_each.ko: Likewise.
* tests/data/test-symtab/kernel-4.19/single_function.ko: Likewise.
* tests/data/test-symtab/kernel-4.19/single_function_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-4.19/single_variable.ko: Likewise.
* tests/data/test-symtab/kernel-4.19/single_variable_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-5.4/Makefile: New test case makefile.
* tests/data/test-symtab/kernel-5.4/empty.c: Likewise.
* tests/data/test-symtab/kernel-5.4/one_of_each.c: Likewise.
* tests/data/test-symtab/kernel-5.4/single_function.c: Likewise.
* tests/data/test-symtab/kernel-5.4/single_function_gpl.c: Likewise.
* tests/data/test-symtab/kernel-5.4/single_variable.c: Likewise.
* tests/data/test-symtab/kernel-5.4/single_variable_gpl.c: Likewise.
* tests/data/test-symtab/kernel-5.4/empty.ko: New test data.
* tests/data/test-symtab/kernel-5.4/one_of_each.ko: Likewise.
* tests/data/test-symtab/kernel-5.4/single_function.ko: Likewise.
* tests/data/test-symtab/kernel-5.4/single_function_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-5.4/single_variable.ko: Likewise.
* tests/data/test-symtab/kernel-5.4/single_variable_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-5.6/Makefile: New test case makefile.
* tests/data/test-symtab/kernel-5.6/empty.c: Likewise.
* tests/data/test-symtab/kernel-5.6/one_of_each.c: Likewise.
* tests/data/test-symtab/kernel-5.6/single_function.c: Likewise.
* tests/data/test-symtab/kernel-5.6/single_function_gpl.c: Likewise.
* tests/data/test-symtab/kernel-5.6/single_variable.c: Likewise.
* tests/data/test-symtab/kernel-5.6/single_variable_gpl.c: Likewise.
* tests/data/test-symtab/kernel-5.6/empty.ko: New test data.
* tests/data/test-symtab/kernel-5.6/one_of_each.ko: Likewise.
* tests/data/test-symtab/kernel-5.6/single_function.ko: Likewise.
* tests/data/test-symtab/kernel-5.6/single_function_gpl.ko: Likewise.
* tests/data/test-symtab/kernel-5.6/single_variable.ko: Likewise.
* tests/data/test-symtab/kernel-5.6/single_variable_gpl.ko: Likewise.
* tests/data/test-symtab/kernel/Makefile: New test case source file.
* tests/data/test-symtab/kernel/empty.c: Likewise.
* tests/data/test-symtab/kernel/one_of_each.c: Likewise.
* tests/data/test-symtab/kernel/single_function.c: Likewise.
* tests/data/test-symtab/kernel/single_function_gpl.c: Likewise.
* tests/data/test-symtab/kernel/single_variable.c: Likewise.
* tests/data/test-symtab/kernel/single_variable_gpl.c: Likewise.
* tests/test-symtab.cc: New test case to test kernel symtabs.
Add (Catch based) test suite to test symbol table reading through the
result persisted in abigail::corpus.
The test cases are created through simple C source files targeting the
desired properties (having an undefined/export function or both). The
Makefile that comes with them recreates the test cases from the sources.
This covers reading sorted_(undefined_)var|fun_symbols as well as the
corresponding symbols maps accessible through the accessors of
abigail::corpus.
* tests/Makefile.am: add new test runtestsymtab
* tests/data/Makefile.am: add new test data for runtestsymtab
* tests/data/test-symtab/Makefile: Add this to build the binaries
below from their source code.
* tests/data/test-symtab/basic/empty.c: New test case source.
* tests/data/test-symtab/basic/link_against_me.c: Likewise.
* tests/data/test-symtab/basic/no_debug_info.c: Likewise.
* tests/data/test-symtab/basic/one_function_one_variable.c: Likewise.
* tests/data/test-symtab/basic/one_function_one_variable_undefined.c: Likewise.
* tests/data/test-symtab/basic/single_function.c: Likewise.
* tests/data/test-symtab/basic/single_undefined_function.c: Likewise.
* tests/data/test-symtab/basic/single_undefined_variable.c: Likewise.
* tests/data/test-symtab/basic/single_variable.c: Likewise.
* tests/data/test-symtab/basic/empty.so: New test data, built from
the Makefile above.
* tests/data/test-symtab/basic/link_against_me.so: Likewise.
* tests/data/test-symtab/basic/no_debug_info.so: Likewise.
* tests/data/test-symtab/basic/one_function_one_variable.so: Likewise.
* tests/data/test-symtab/basic/one_function_one_variable_undefined.so: Likewise.
* tests/data/test-symtab/basic/single_function.so: Likewise.
* tests/data/test-symtab/basic/single_undefined_function.so: Likewise.
* tests/data/test-symtab/basic/single_undefined_variable.so: Likewise.
* tests/data/test-symtab/basic/single_variable.so: Likewise.
* tests/test-symtab.cc: New test driver.
When emitting abixml types inside a given scope are sorted
topologically. Types that don't have source definition location
information are sorted lexicographically.
There are certain types however that need more careful consideration.
Those are empty-qualified types. That is, qualified types (like
cv-qualified types) that carry no qualifier. The patch explains
in-extenso in comments where those types come from. You can also look
at the comments of the function maybe_strip_qualification for even
more context.
Simply put, an empty qualified type like 'NONE reference type' equals it's
non-qualified variant 'reference type'.
During the topological sorting, we chose to have the empty-qualified
variant "come before" (i.e, be "less than") the non-qualified variant.
This is alright.
The bug however is that we failed to handle the case were we are
looking at two empty-qualified types that are equal. In that case, of
course, they are meant to be topologically equivalent.
Fixed thus.
* src/abg-ir.cc (type_topo_comp::operator()): In the comparison
operator consider two equivalent empty-qualified types as being
topologically equivalent.
Dodji Seketeli [Thu, 7 May 2020 10:34:21 +0000 (12:34 +0200)]
Bug 25661 - Support data member replacement by anonymous data member
We ought to detect when a data member is replaced by an anonymous data
member in a way that doesn't change the ABI in an incompatible way,
especially when that change is non equivocal.
For instance, consider this ABI-visible struct:
struct S
{
int a;
};
Now, consider that it's changed into:
struct S
{
union
{
int a;
char b;
};
};
Stricto sensu, the bit-layout of struct S doesn't change and so that
change isn't ABI-incompatible.
The current version of libabigail however flags that change as a
/potential/ issue and asks the user for further review. It appears
that this class of changes is frequent enough to be annoying,
especially in semi-automatic ABI compliance checking setups where we
want the least possible "false positives".
This patch detects that kind of change patterns where a data member is
replaced by an anonymous data member in a benign way, in terms of ABI.
So now let's look at a more complicated example where an ABI-visible
type looks like:
struct S
{
int a;
int b;
int c;
};
Now suppose that type was changed into:
struct S
{
union
{
int tag[3];
struct
{
int a;
int b;
int c;
};
};
};
The patch allows abidiff to recognise that kind of pattern, filter out
the detected change and report by default that the two binaries are
ABI compatible.
[C] 'function void foo(S*)' at test-v1.cc:18:1 has some indirect sub-type changes:
parameter 1 of type 'S*' has sub-type changes:
in pointed to type 'struct S' at test-v1.cc:1:1:
type size hasn't changed
data members 'S::a', 'S::b', 'S::c' were replaced by anonymous data member:
'union {int tag[3]; struct {int a; int b; int c;};}'
And using the leaf-node reporter, that would give:
'struct S at test-v0.cc:1:1' changed:
type size hasn't changed
data members 'S::a', 'S::b', 'S::c' were replaced by anonymous data member:
'union {int tag[3]; struct {int a; int b; int c;};}'
* include/abg-comp-filter.h (has_data_member_replaced_by_anon_dm):
Declare new function.
* include/abg-comparison.h (changed_var_sptr)
(changed_var_sptrs_type): Declare new typedefs.
(HARMLESS_DATA_MEMBER_CHANGE_CATEGORY): Add a new enumerator to
the diff_category enum.
(EVERYTHING_CATEGORY): In the diff_category, adjust this
enumerator to OR the new HARMLESS_DATA_MEMBER_CHANGE_CATEGORY into
it.
(SUPPRESSED_CATEGORY, PRIVATE_TYPE_CATEGORY)
(SIZE_OR_OFFSET_CHANGE_CATEGORY, VIRTUAL_MEMBER_CHANGE_CATEGORY)
(CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY)
(FN_PARM_TYPE_TOP_CV_CHANGE_CATEGORY)
(FN_RETURN_TYPE_CV_CHANGE_CATEGORY, VAR_TYPE_CV_CHANGE_CATEGORY)
(VOID_PTR_TO_PTR_CHANGE_CATEGORY)
(BENIGN_INFINITE_ARRAY_CHANGE_CATEGORY): Adjust the value of these
enumerators of the diff_category enum.
(class_or_union_diff::{data_members_replaced_by_adms,
ordered_data_members_replaced_by_adms}): Declare new member
functions.
* include/abg-fwd.h (var_decl_wptr): Declare new typedef.
(get_next_data_member, get_first_non_anonymous_data_member)
(find_data_member_from_anonymous_data_member)
(get_absolute_data_member_offset): Declare new functions.
* include/abg-ir.h (struct anonymous_dm_hash): Declare new type.
(anonymous_data_member_sptr_set_type): Declare new typedef.
(class decl_base): Befriend class class_or_union.
(class dm_context_rel): Pimpl-ify this class.
(dm_context_rel::{g,s}et_anonymous_data_member_types): Declare new
member functions.
(var_decl::get_anon_dm_reliable_name): Declare new member
function.
(class var_decl): Make get_absolute_data_member_offset,
get_absolute_data_member_offset be friends of this.
(class_or_union::maybe_fixup_members_of_anon_data_member): Declare
new protected member function.
* src/abg-comp-filter.cc (has_data_member_replaced_by_anon_dm):
Define new function.
(categorize_harmless_diff_node): Use the above.
* src/abg-comparison-priv.h
(class_or_union_diff::priv::{dms_replaced_by_adms_,
changed_var_sptrs_type dms_replaced_by_adms_ordered_}): Add new
data members.
(data_member_comp::compare_data_members): Factorize this out of ...
(data_member_comp::operator()(decl_base_sptr&, decl_base_sptr&)):
... this.
(data_member_comp::operator()(changed_var_sptr&,
changed_var_sptr&)): Add new member function.
(sort_changed_data_members): Declare ...
* src/abg-comparison.cc (sort_changed_data_members): ... new
function.
(get_default_harmless_categories_bitmap): Adjust to take the new
abigail::comparison::HARMLESS_DATA_MEMBER_CHANGE_CATEGORY into
account.
(operator<<(ostream& o, diff_category c)): Likewise.
(class_or_union_diff::ensure_lookup_tables_populated): Handle
Handle the insertion of anonymous data members to replace existing
data members.
(class_or_union_diff::{data_members_replaced_by_adms,
ordered_data_members_replaced_by_adms}): Define new accessors.
(suppression_categorization_visitor::visit_end): Propagate the
SUPPRESSION_CATEGORIZATION_VISITOR from changes to the type of the
data member if the data member doesn't have real local changes.
* src/abg-default-reporter.cc (default_reporter::report): Report
about anonymous data members that replace data members.
* src/abg-ir.cc (struct dm_context_rel::priv): Define new data
structure.
(dm_context_rel::{dm_context_rel, get_is_laid_out,
set_is_laid_out, get_offset_in_bits, set_offset_in_bits,
operator==, operator!=, get_anonymous_data_member,
set_anonymous_data_member}): Define the member functions here as
they are not inline anymore.
(class_or_union::maybe_fixup_members_of_anon_data_member): Define
new member function.
(class_or_union::add_data_member): Use it.
(get_first_non_anonymous_data_member, get_next_data_member)
(get_absolute_data_member_offset)
(find_data_member_from_anonymous_data_member): Define new
functions.
* src/abg-reporter-priv.h
(maybe_report_data_members_replaced_by_anon_dm): Declare ...
* src/abg-reporter-priv.cc
(maybe_report_data_members_replaced_by_anon_dm): ... new function.
* src/abg-leaf-reporter.cc (leaf_reporter::report): Report data
members replaced by anonymous data members.
* tests/data/test-diff-filter/test-PR25661-[1-6]-report-[1-4].txt: New
test reference outputs.
* tests/data/test-diff-filter/test-PR25661-[1-6]-v{0,1}.c: Test
source code files.
* tests/data/test-diff-filter/test-PR25661-[1-6]-v{0,1}.o: Test
binary input files.
* tests/data/Makefile.am: Add the new test files above to source
distribution.
* tests/test-diff-filter.cc (in_out_specs): Add the binary test
inputs above to this test harness.
* tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt:
Adjust.
Dodji Seketeli [Thu, 7 May 2020 09:03:54 +0000 (11:03 +0200)]
dwarf-reader: support several anonymous data members in a given class
At the moment, if a class has more than one anonymous data member, we
are just keeping the first one and dropping the others.
This patch fixes that.
Now that there is the possibility of having several anonymous data
members in a given class we need to be able to name them correctly, to
tell them apart. That means we cannot use the name returned by
var_decl::get_name() as that name is empty for an anonymous data
member. This patch thus introduces a new method
var_decl::get_anon_dm_reliable_name() which returns a name that is
reliable (non-empty) even when the var_decl designates an anonymous
data member. Note that there are many situations where we still need
to have var_decl::get_name() behave like it used to, so we can't make
it invariably return what var_decl::get_anon_dm_reliable_name()
returns today.
* include/abg-ir.h (class_or_union::find_anonymous_data_member):
Declare a new member function.
(class_or_union::find_data_member): Declare a new overload.
(var_decl::get_anon_dm_reliable_name): Declare new member
function.
* src/abg-ir.cc (var_decl::get_pretty_representation): Make this
work on a var_decl is going to be used to represent an anonymous
data member even before the var_decl has been added to its finale
scope. This is useful to make class_or_union::find_data_member
work on a var_decl that is to be used as an anonymous data member.
(var_decl::get_anon_dm_reliable_name): Define new member function.
(class_or_union::find_data_member): In the existing overload that
takes a string, look for the named data member inside the
anonymous data members. Define a new overload that takes a
var_decl_sptr, to look for anonymous data members.
(class_or_union::find_anonymous_data_member): Define a new member
function.
(lookup_data_member): Use the existing
class_or_union::find_data_member.
* src/abg-reader.cc: (build_class_decl): Use the full anonymous
variable for lookup, rather than its name which is empty and will
thus give false positives.
* src/abg-dwarf-reader.cc (add_or_update_class_type): Likewise.
* src/abg-comparison.cc
(class_or_union_diff::ensure_lookup_tables_populated): Name
anonymous data members properly - as opposed to wrongly using
their empty name.
* src/abg-reporter-priv.cc (represent): In the overload for
var_diff_sptr, make sure that changes to the /type/ of a variable
declaration are always reported.
* tests/data/test-abidiff-exit/test-member-size-report0.txt:
Adjust as we now emit more detailed changes about anonymous data
members.
* tests/data/test-abidiff-exit/test-member-size-report1.txt:
Likewise.
* tests/data/test-annotate/test-anonymous-members-0.o.abi: Adjust
to reflect the fact that a class can now have several anonymous
data members.
* tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0.abi:
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/test35-pr18754-no-added-syms-report-0.txt:
Likewise.
Dodji Seketeli [Thu, 7 May 2020 08:03:21 +0000 (10:03 +0200)]
{default,leaf}-reporter: group data members changes reports together
There are two kinds of data member changes:
1/ changes to the type or offset of a given data member
basically, in this kind of change, the name of the data member
remained the same.
2/ changes where the data member (at a given offset) was replaced by
something else completely.
Today, the comparison engine recognizes these two kinds of changes and
records them in two different data structures. This is useful because
it allows for a finer grain analysis.
But when we report these changes, today, we report them separately and
sometimes that doesn't make sense. For instance, because there is no
change of the 1/ kind, the reporter would say "no data member
changes", and yet go ahead and emit data member changes of the 2/ kind.
This patch groups the reporting of the two kinds of changes so that
when it says "no data member changes", it means there was no data
member changes at all.
* include/abg-comparison.h
(class_or_union_diff::{sorted_changed_data_members,
count_filtered_changed_data_members,
sorted_subtype_changed_data_members,
count_filtered_subtype_changed_data_members}): Declare ...
* src/abg-comparison.cc
(class_or_union_diff::{sorted_changed_data_members,
count_filtered_changed_data_members,
sorted_subtype_changed_data_members,
count_filtered_subtype_changed_data_members}): ... accessors for
existing private data members.
* src/abg-default-reporter.cc (default_reporter::report): In the
class_or_union_diff& overload, group the reporting of the changes
to data member sub-types with the replacement of data members.
These are just data member changes after all. Use the newly
declared accessors for better measure.
* src/abg-leaf-reporter.cc (leaf_reporter::report): Likewise.
* tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt: Adjust.
* src/abg-leaf-reporter.cc (leaf_reporter::report): 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.
Dodji Seketeli [Mon, 18 May 2020 10:01:14 +0000 (12:01 +0200)]
Add -g back to ABIGAIL_DEVEL
I forgot to set the -g option in C{XX}FLAGS when applying the patch
https://sourceware.org/git/?p=libabigail.git;a=commit;h=d02de5a7846d7f44621c1e100e60f36f5a0c051e.
Oops.
Fixed thus.
* configure.ac: Don't forget to set -g when ABIGAIL_DEVEL is set.
Giuliano Procida [Fri, 15 May 2020 06:50:12 +0000 (07:50 +0100)]
clang-format: set continuation indentation to 2
This is a further tweak to the clang-format rules, bringing them
(on average) closer to the existing corpus of code.
The existing code mostly uses an indentaton of 2 spaces, rather than
4, for expressions broken across lines. There are exceptions such as
conditional expressions that are initialisers but clang-format's
indentation may be preferable to emacs' zero indentation here.
* .clang-format: Set ContinuationIndentWidth to 2.
configure: add more diagnostic options when ABIGAIL_DEVEL is set
When exporting ABIGAIL_DEVEL=1, add more flags to ABIGAIL_DEVEL that are
suitable for development to find issues during edit/compile/test time.
The subsequent changes to source and test code are needed to make the
code compile with ABIGAIL_DEVEL=yes.
Note, unless bug #25989 is addressed, runtestannotate is failing.
See https://sourceware.org/bugzilla/show_bug.cgi?id=25989 for details.
* configure.ac: add -D_FORTIFY_SOURCE=2 and -D_GLIBCXX_DEBUG
compilation defines if ABIGAIL_DEVEL is set. Note that with GCC 4.8.5,
-D_FORTIFY_SOURCE=2 requires options to be set. So I am setting
the optimization level to -Og.
* src/abg-dwarf-reader.cc (read_context::{compute_canonical_die,
get_or_compute_canonical_die, associate_die_to_decl,
set_canonical_die_offset, schedule_type_for_late_canonicalization,
compare_dies}, get_scope_for_die, add_or_update_union_type)
(read_debug_info_into_corpus, build_ir_node_from_die): Initialize
the 'source' variable.
* tests/test-diff-filter.cc (main): Check the return value of the
system function.
* tests/test-diff-pkg.cc (main): Likewise.
* tests/test-read-write.cc (main): Likewise.
When exporting ABIGAIL_DEBUG=1, the binaries compiled are especially
suitable for debugging. The CFLAGS and CXXFLAGS that are added reduce
optimization to a reasonable amount and increase debug information levels.
* configure.ac: add ABIGAIL_DEBUG environment variable for
improved debugging capabilities
The virtual function get_pretty_representation exists in many
classes (derived from type_or_decl_base). It takes a qualified_name
argumenta and this is defaulted to true in all but one case which
seems to be an oversight.
This commit changes this for no better reason than consistency.
* include/abg-ir.h (type_decl::get_pretty_representation)
Change default for qualified_name parameter to true.
Dodji Seketeli [Wed, 13 May 2020 14:22:01 +0000 (16:22 +0200)]
Bug 25986 - Wrong name of function type used in change report
We have introduced type name caching long ago to speed up operations
involving type names.
Then last year, I was looking at some speed optimization in the
xml-writer and I started playing with the caching to quantify the
speed impact that early caching could have on emitting writting out
the abixml, independantly from the potential accuracy issues that
could have.
And somehow I accidentally committed one of those experimental
patches:
https://sourceware.org/git/?p=libabigail.git;a=commit;h=7699dfc921d248fa6d5cbdbeab9d501b17ef3f52.
This commit reverts that bogus patch. There should be no speed impact
because the writter now de-duplicates types at writting time by
"simply" writting out the canonical types, as opposed to the naive
approach we were using then.
This fixes the bug reported at
https://sourceware.org/bugzilla/show_bug.cgi?id=25986 which shows the
impact of caching the wrong name of the type, which happens when
caching occurs before the type is canonicalized.
* src/abg-ir.cc (function_type::get_cached_name): Don't cache
names for non-canonicalized types.
* tests/data/test-abidiff-exit/test-fun-param-report.txt: Add
reference output for new test.
* tests/data/test-abidiff-exit/test-fun-param-v{0,1}.abi: Add new test
input files.
* tests/data/test-abidiff-exit/test-fun-param-v{0,1}.c: Add source
files for the above.
* tests/data/test-abidiff-exit/test-fun-param-v{0,1}.o: Add
binaries for the above.
Mark Wielaard [Mon, 11 May 2020 16:55:03 +0000 (18:55 +0200)]
Don't iterate before the start of a RandomAccessOutputIterator.
Found with -D_GLIBCXX_DEBUG. You cannot go before the start of an
RandomAccessOutputIterator. Iterator -1 + 1 might seem to work,
but is actually undefined behaviour.
* include/abg-diff-utils.h (compute_diff): Put brackets around
p[ux].[xy]() + 1 calculation.
Giuliano Procida [Tue, 12 May 2020 10:10:08 +0000 (11:10 +0100)]
doc: Fix sufficient suppression property lists.
At present there are discrepancies between the suppression
specification parsing code and the documented lists of properties (per
suppression type) that are consider sufficient to prevent a
suppression specification being ignored altogether.
This patch updates the documentation to match the code and adjusts
some confusing language in the documentation.
* doc/manuals/libabigail-concepts.rst: Add missing
sufficient properties for suppress_file, suppress_function and
suppress_variable. Use consistent language about what will
happen when no property in the list in provided.
dwarf-reader: read_context: drop some unused accessor methods
Those methods were effectively dead code. Hence only keep the actually
used accessors around:
const string_elf_symbols_map_sptr& some_sptr();
string_elf_symbols_map_sptr& some();
tests/.gitignore: ignore all files starting with runtest*
We only have runtest* executables that we can ignore there with this
prefix. Hence, stop adding each and every test and use an exclusion
pattern instead.
* tests/.gitignore: gitignore all files named runtest*
configure: set -Wno-error-overloaded-virtual for clang builds
When compiling with clang, several locations in the code emit the
warning -Woverloaded-virtual. That warning is not trivial to fix. In order
to allow CXX=clang++ ABIGAIL_DEVEL=1 development, demote the warning to
not be an error when compiling with clang.
* configure.ac: set -Wno-error-overloaded-virtual for clang++
This patch replaces some verbose code with one-liners that rely on
std::unordered_map::operator[] to insert a missing key and return a
reference to the default-constructed inserted value.
* src/abg-reader.cc (read_context::key_type_decl): Rely on
std::unordered_map::operator[] to create map entries if they
are missing. (build_elf_symbol_db): Ditto.
This function extracts either a string or a regex from the same input.
This patch simplifies the string vs regex conditional logic, in order
to make following patches simpler.
There are no behavioural changes.
* src/abg-suppression.cc (read_parameter_spec_from_string):
Use separate string and regex variables to simplify the
creation of returned parameter specification.
Giuliano Procida [Tue, 12 May 2020 10:14:46 +0000 (11:14 +0100)]
Tidy checks for sufficient suppression properties.
Each suppression specification must have at least one of a documented
per-suppression type list of properties defined if it is to be
considered at all.
At present:
- suppression specifications which fail the check are silently ignored
- in the function suppression case, the check does not trigger an
early return and risks a later null pointer dereference.
This commit:
- reimplements the checks using arrays and helper function calls
- adds a helper function to determine if a suppression specification
is going to be ignored due a lack of properties
- makes the parsing functions return failure early if the check fails
Inconsistencies between suppression types (in particular, the
treatment of the "label" property) remain.
The only behavioural change may be to how present but empty properties
are handled. This is an edge case that may be worth revisiting in a
more general fashion in a later commit.
* src/abg-suppression.cc (check_sufficient_props): New helper
function to check for sufficient properties in a section.
(read_type_suppression): Replace conditional logic with call
to check_sufficient_props.
(read_function_suppression): Ditto.
(read_variable_suppression): Ditto.
(read_file_suppression): Ditto.
* tests/data/test-diff-suppr/test15-suppr-added-fn-4.suppr:
Explain why the suppression will be ignored.
* tests/data/test-diff-suppr/test16-suppr-removed-fn-4.suppr:
Ditto.
* tests/data/test-diff-suppr/test17-suppr-added-var-4.suppr:
Ditto.
* tests/data/test-diff-suppr/test18-suppr-removed-var-4.suppr:
Ditto.
1 Removed function:
@@ -13,13 +13,6 @@
[A] 'method virtual long int ops::added_fn()' {_ZN3ops8added_fnEv}
note that this adds a new entry to the vtable of struct ops
-1 function with some sub-type change:
-
- [C] 'method virtual int ops::changed_fn()' at test-leaf-cxx-members-v1.cc:5:1 has some sub-type changes:
- return type changed:
- type name changed from 'int' to 'long int'
- type size changed from 32 to 64 (in bits)
-
'struct ops at test-leaf-cxx-members-v0.cc:2:1' changed:
type size changed from 128 to 192 (in bits)
1 member function deletion:
@@ -27,10 +20,6 @@
1 member function insertion:
'method virtual long int ops::added_fn()' at test-leaf-cxx-members-v1.cc:11:1, virtual at voffset 1/1 {_ZN3ops8added_fnEv}
there are member function changes:
- 'method virtual int ops::changed_fn()' has some changes:
- return type changed:
- type name changed from 'int' to 'long int'
- type size changed from 32 to 64 (in bits)
1 data member deletion:
'int ops::deleted_var', at offset 96 (in bits) at test-leaf-cxx-members-v0.cc:5:1
1 data member insertion:
$
This is because we wrongly consider the canonical diff node for the
change on the type of function "int ops::changed_fn()" as being
suppressed. This is because of an old thinko in the suppression
categorization and propagation pass for local types changes to
functions. Oops.
Fixed thus.
* abg-comparison.cc:
(suppression_categorization_visitor::visit_end): Don't suppress
the entire class of equivalence of a function diff node if that
function diff node itself was not suppressed.
libabigail code uses the POSIX regex library consistently:
- compile std::string to regex, with the flag REG_EXTENDED
- store regex using a shared pointer wrapper
- check match of regex against std::string
All the C string / std::string logic and so on is repeated at every
call site. This patch introduces wrapper functions to take care of
this logic.
There are no behavioural changes.
* include/abg-regex.h (compile): Declare new function.
(match): Declare new function.
* src/abg-regex.cc (compile): Add new function wrapping
regcomp. (match): Add new function wrapping regexec.
In abg-suppression.cc most of the read_*_suppression functions
populate and return a single variable "result". Others vary a little
from this, having variables "nil" or "suppr".
This patch makes all the functions use a single variable "result".
This will make the next patches a bit simpler.
There are no behavioural changes.
* src/abg-suppression.cc (read_type_suppression): Rename "nil"
variable to "result" and reset it instead of introducing
"suppr" later on. (read_function_suppression): Rename "nil"
variable to "result" and reset it instead of introducing
"result" later on.
There is the theoretical possibility that symbols may contain special
regex characters like '.' and '$'. This patch ensures all such
characters in symbol names are escaped before they are added to the
whitelisting regex.
* include/regex.h (escape): New string reference holder
class. (operator<<): Declaration of std::ostream,
regex::escape overload.
* include/regex.cc (operator<<): New std::ostream,
regex::escape overload that outputs regex-escaped strings.
* src/abg-tools-utils.cc
(gen_suppr_spec_from_kernel_abi_whitelists): Make sure any
special regex characters in symbol names are escaped.
Following review of the change to simplify generation of symbol
whitelist regexes, this commit clarifies the purpose of the ^_^ regex
as one which will not match any string.
Following review of a recent change to move regex definitions into
their own files, this commit ensures that #include directives are
separated into categories and sorted alphabetically within the
categories, subject to known ordering constraints (such as "config.h"
first, "abg-internal.h" before visibility directives etc.).
There are no behavioural changes.
* src/abg-corpus-priv.h: Separate and sort #include
directives.
* src/abg-corpus.cc: Ditto.
* src/abg-regex.cc: Ditto.
* src/abg-suppression-priv.h: Ditto.
* src/abg-suppression.cc: Ditto.
tests: reorder test execution to optimize 'make check' runtime
Reorder the test definition to first start expensive tests and later on
start tests with short runtime. This optimizes the 'make check' runtime
by starting the tests on the critical path.
* tests/Makefile.am(TESTS): split up in expensive and non
expensive tests, sort the expensive ones by runime, the cheap
ones alphabetically
test-types-stability: parallelize test case alternatives
Commit a9f5fb408946 ("Add --no-write-default-sizes option.") introduced
a new test variant for test-types-stability that is actually independent
of the original test case in terms of execution. Hence it can be
expressed as a separate test case. So, do that by parametrizing the
test_task struct with a new no_default_sizes flag and schedule a
separate test case in the main loop.
That test runs now ~twice as fast dropping from roughly 20s on my
machine to 10s. That effectively removes it from the critical path of
make check, which is now back to about 15s on my machine with my
configuration.
* tests/test-types-stability.cc (test_task): add field no_default_sizes
(test_task::perform) Switch on the new flag to test a different
behaviour.
(main): Schedule an additional test case to test with the new flag.
Dodji Seketeli [Mon, 4 May 2020 10:46:30 +0000 (12:46 +0200)]
Fix compilation with g++ 4.8.5 on el7
G++ 4.8.5 doesn't allow a function template specialization to appear
in a namespace that is different from the namespace where the function
template was declared. Also, in that namespace, the name of the
specialized function template must be non-fully-qualified.
Fixed thus.
* include/abg-libxml-utils.h (build_sptr<xmlTextReader>): Wrap
this specialization in the namespace sptr_utils and do away with
the sptr_utils qualifier.
* include/abg-regex.h (build_sptr<regex_t>): Likewise.
The code to build the symbol whitelist regex uses things like seekp
and tellp to generate regexes like "^foo$|^bar$".
This patch simplifies the code, for further enhancement, resulting in
generated regexes like "^(foo|bar)$".
There should be no change in behaviour, unless whitelisted symbol
names contain special regex characters.
* include/abg-regex.h (generate_from_strings): Declare new
function to build a regex from some strings, representing a
membership test.
* src/abg-regex.cc (generate_from_strings): Implement new
function to build a regex from some strings, representing a
membership test, in a straightfoward fashion.
* src/abg-tools-utils.cc
(gen_suppr_spec_from_kernel_abi_whitelists): Replace
regex-building code with a call to generate_from_strings.
* tests/test-kmi-whitelist.cc: Update regexes in test.
The header file abg-sptr-utils.h contains generic things relating to
shared pointers. It also contains shared pointer typedefs (in the
sptr_utils namespace) and template specialisations for XML types.
The last of these more naturally belong in abg-libxml-utils.h (and in
the xml namespace). This patch moves them.
There are no behavioural changes.
* include/abg-sptr-utils.h: Remove reader_sptr and
xml_char_sptr typedefs, from namespace
sptr_utils. (build_sptr): Remove corresponding template
function specialisations for these types.
* include/abg-libxml-utils.h: Add reader_sptr and
xml_char_sptr typedefs, to namespace xml. (build_sptr): Add
corresponding template function specialisations for these
types.
As a prelude to adding wrapper and helper functions for regex
functionality, it makes sense to move the existing regex code (the
shared pointer type and its specialised deleter) to their own files.
This patch does this and also moves various entities into a new
namespace, abigail::regex. It removes the file abg-sptr-utils.cc which
only contained regex things.
There are no behavioural changes.
* include/Makefile.am: Add abg-regex.h.
* src/Makefile.am: Remove abg-sptr-utils.h, add abg-regex.cc
* include/abg-sptr-utils.h (regex_t_sptr): Remove this
typedef, from namespace abigail::sptr_utils.
(regex_t_deleter): Remove this struct, from namespace
abigail::sptr_utils. (build_sptr): Remove these template
specialisations, in duplicate, for regex_t_sptr.
* include/abg-regex.h: New file, introduces namespace
abigail::regex. (regex_t_sptr): Add this typedef, to namespace
abigail::regex. (regex_t_deleter): Add this struct, to
namespace abigail::regex. (build_sptr): Add these template
specialisations for regex_t_sptr
* src/abg-sptr-utils.cc: Remove this file.
* src/abg-regex.cc: Add new file with contents effectively
the same as abg-sptr-utils.cc.
* src/abg-corpus-priv.h: Update regex_t_sptr namespace
qualification.
* src/abg-corpus.cc: Ditto.
* src/abg-suppression-priv.h: Ditto.
* src/abg-suppression.cc: Ditto.
Mark Wielaard [Sun, 26 Apr 2020 23:28:52 +0000 (01:28 +0200)]
Add --no-write-default-sizes option.
abidw will write out the exact same size-in-bits address for every
pointer type, reference type, function declaration and function type
even though it is always the same as the translation unit address
size. When giving the --no-write-default-sizes option these aren't
written out anymore. The reader is updated to set the default size
when none is given in the XML description.
Even though size and alignment are handled together in the reader,
default alignment is still set to zero, following commit a05384675
Note that this isn't backward compatible with older libabigail
readers, which will set the size to zero when none is given. So this
option isn't the default.
* doc/manuals/abidw.rst: Document --no-write-default-sizes.
* include/abg-writer.h (set_write_default_sizes): New function
declaration.
(set_common_options): Call set_write_default_sizes.
* src/abg-reader.cc (build_function_decl): Get default size.
(build_pointer_type_def): Likewise.
(build_reference_type_def): Likewise.
(build_function_type): Likewise.
* src/abg-writer.cc (write_context): Add m_write_default_sizes
bool.
(get_write_default_sizes): New method.
(set_write_default_sizes): Likewise.
(write_size_and_alignment): Add default size and alignment
parameters.
(set_write_default_sizes): New function.
(write_type_decl): Set default size and alignment.
(write_pointer_type_def): Likewise.
(write_reference_type_def): Likewise.
(write_function_decl): Likewise.
(write_function_type): Likewise.
(write_class_decl_opening_tag): Likewise.
(write_union_decl_opening_tag): Likewise.
* tests/test-types-stability.cc (perform): Also test --abidiff
with --no-write-default-sizes.
* tools/abidw.cc (option): Add default_sizes bool.
(parse_command_line): Parse --no-write-default-sizes.
(display_usage): Add doc string for --no-write-default-sizes.
Signed-off-by: Mark Wielaard <mark@klomp.org> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Mark Wielaard [Sun, 26 Apr 2020 20:56:22 +0000 (22:56 +0200)]
Rename read_elf_symbol_binding to read_elf_symbol_visibility.
read_elf_symbol_binding was overloaded so that it could be called to
read either a binding or a visibility attribute. This is slightly
confusing, since another elf_symbol attribute, type, does have its own
read_elf_symbol_type. Rename the read_elf_symbol_binding that reads
the visibility to read_elf_symbol_visibility for claritiy.
When reporting declaration-only type changes, the size is reported as
changing to or from 0, which is not correct. The declaration type is
of unknown size. This patch eliminates such size reports.
* src/abg-reporter-priv.cc (report_size_and_alignment_changes):
Filter out declaration-only / defined type size changes
unconditionally.
* tests/data/test-abidiff-exit/test-decl-struct-report.txt:
Update test.
In the case where a type change is summarised as declaration-only to
defined or vice versa, the trailing new line was missing.
* src/abg-default-reporter.cc (default_reporter::report): In
the class_or_union_diff overload, emit a new line at the end
of the declaration-only reporting path.
* tests/data/test-abidiff-exit/test-decl-struct-report.txt:
Add missing blank lines.
There were no tests exercising this reporting path. The new test is
run with --harmless as these changes are considered harmless.
* tests/data/Makefile.am: Add new test case files.
* tests/data/test-abidiff-exit/test-decl-struct-report.txt:
New test case generating "declaration-only" output.
* tests/data/test-abidiff-exit/test-decl-struct-v0.c: Ditto.
* tests/data/test-abidiff-exit/test-decl-struct-v0.o: Ditto.
* tests/data/test-abidiff-exit/test-decl-struct-v1.c: Ditto.
* tests/data/test-abidiff-exit/test-decl-struct-v1.o: Ditto.
* tests/test-abidiff-exit.cc: Run new test case.
abipkgdiff: Fix race condition while using private types suppr specs
When comparing two packages, abipkgdiff potentially spawns one thread
per pair of binaries to compare. The suppression specifications
generated for the private types of the package are shared among those
threads. As some internals of the suppression specifications are
constructed lazily, that can lead do some data races when more than
two threads access one of those internals.
One effective way to handle that is to make sure each thread has its
own copy of suppression specifications.
That's what this patch does.
* tools/abipkgdiff.cc (compare_args::private_types_suppr{1,2}):
Make these data member *not* be a reference anymore.
(maybe_create_private_types_suppressions): Rename this into ...
(create_private_types_suppressions): ... this. Also, make this
function return a copy of the vector of suppression specifications
for private types created.
(compare_prepared_userspace_packages): Use the new
create_private_types_suppressions to create a copy of private
types suppression specifications, rather than sharing it from
package::private_types_suppressions_.
(extract_package_and_map_its_content): Adjust to avoid creating
the shared suppression specifications for private types.
(package::private_types_suppressions_): Remove this data member
that was holding the shared suppressions for private types.
(package::private_types_suppressions): Remove these accessors.
The property name "reached_through" should have been
"accessed_through" and the property value "reference_or_pointer"
should have been "reference-or-pointer".
to combine their functionality and preserve the outer interface. We
just add the Elf* handle argument as usual as we are out of
read_context now.
* src/abg-dwarf-reader.cc
(maybe_adjust_et_rel_sym_addr_to_abs_addr)
(drop the wrapped overload completely): Move out functions.
* src/abg-elf-helpers.cc
(maybe_adjust_et_rel_sym_addr_to_abs_addr): New function.
* src/abg-elf-helpers.h
(maybe_adjust_et_rel_sym_addr_to_abs_addr): New function
declaration.
abg-dwarf-reader: migrate more ELF helpers to elf-helpers
This change migrates all ELF helpers related to section lookup to
abg-elf-helpers.{cc,h}. It also homogenizes the interface of those to
always return Elf_Scn* and NULL in case that section can't be found.
Though this smells like a functional change, this latter change is
purely cosmetic.
* src/abg-dwarf-reader.cc (read_context::{opd_section_,
ksymtab_section_, ksymtab_reloc_section_, ksymtab_gpl_section_,
ksymtab_gpl_reloc_section_, ksymtab_strings_section_}): Make these
data members mutable as they are set by their getter member
functions, as an implementation detail.
(read_context::find_symbol_table_section):
adjust to new interface of elf_helpers::find_symbol_table_section.
(find_opd_section): use elf_helpers::find_opd_section for lookup.
(find_ksymtab_section): use elf_helpers::find_ksymtab_section.
(find_ksymtab_gpl_section): use elf_helpers::find_ksymtab_gpl_section.
(find_relocation_section): Move out function.
(get_binary_load_address): Move out function.
(find_ksymtab_reloc_section): use elf_helpers::find_relocation_section
(find_ksymtab_gpl_reloc_section): use
elf_helpers::find_relocation_section
* src/elf-helpers.cc (find_symbol_table_section): change
interface to match other find_*_section functions.
(find_symbol_table_section_index): Adjust for the new interface of
find_symbol_table_section.
(find_opd_section): New function.
(find_ksymtab_section): New function.
(find_ksymtab_gpl_section): New function.
(find_relocation_section): New function.
(get_binary_load_address): New function.
* src/elf-helpers.h (find_symbol_table_section): Change declaration.
(find_opd_section): New function declaration.
(find_ksymtab_section): New function declaration.
(find_ksymtab_gpl_section): New function declaration.
(find_relocation_section): New function declaration.
(get_binary_load_address): New function declaration.
abg-elf-helpers: consolidate the is_linux_kernel* helpers
This consistently names the is_linux_kernel* helpers and keeps only the
copy in the elf-helpers. All users are adjusted.
* src/abg-dwarf-reader.cc
(read_context::function_symbol_is_exported): use is_linux_kernel
from elf_helpers.
(read_context::variable_is_exported): Likewise.
(read_context::get_symtab_format): Likewise.
(read_context::load_symbol_maps): Likewise.
(read_debug_info_into_corpus): Likewise.
(read_context::is_linux_kernel_binary): Drop function.
(read_context::is_linux_kernel_module): Drop function.
* src/abg-elf-helpers.cc (binary_is_linux_kernel): rename to
is_linux_kernel
(binary_is_linux_kernel_module): rename to is_linux_kernel_module
(find_ksymtab_strings_section): Adjust to function renames.
* src/abg-elf-helpers.h (binary_is_linux_kernel): rename to
is_linux_kernel
(binary_is_linux_kernel_module): rename to is_linux_kernel_module
abg-dwarf-reader split: create abg-elf-helpers.{h,cc} and test case
abg-elf-helpers.{h,cc} shall contain the ELF related parts of the
abg-dwarf-reader. Create the stub files, an empty unit test and hook
everything up in the make system.
* src/Makefile.am: Add new source files abg-elf-helpers.{h,cc}.
* src/abg-elf-helpers.cc: New source file.
* src/abg-elf-helpers.h: New header file.
* tests/.gitignore: Exclude runtestelfhelpers from being committed.
* tests/Makefile.am: Add new test case runtestelfhelpers.
* tests/test-elf-helpers.cc: New test source file.
Parallelize test execution for diff-suppr test by using abigail's multi
threaded worker queue. To accomplish that, follow a similar pattern like
other tests: Add a test_task struct with some precomputed values and
chunk up the work in main().
The test cases needed to be adjusted to allow parallel execution. In
particular the output files can't be shared anymore. To ensure this, a
small tests has been added that checks for unique output paths.
Finally, one redundant test case has been dropped.
This reduces the test time on my machine from ~31s to ~14s. There are
still two test cases that dominate the overall runtime. Since this test
is on the critical path for 'distcheck' (it is the test with the longest
runtime), this is effectively chopped off the overal make distcheck
time.
* tests/test-diff-suppr.cc(main): parallelize test execution.
(test_task) new abigail::workers::task implementation to run
test cases in this test as separate worker tasks.
For the update of fun_syms_, undefined_fun_syms_, var_syms_,
undefined_var_syms_, there is a pattern in
read_context::load_symbol_maps_from_symtab_section that can be
simplified: We try to find the correct entry and keep the iterator. If
we do not find it, we construct the value vector, find it again and
update the iterator. Eventually we push_back the symbol. That can be
simplified to looking up the value with operator[], which either returns
the existing value or default constructs it. The reference returned is
good for the push_back in any case.
* src/abg-dwarf-reader.cc
(read_context::load_symbol_maps_from_symtab_section): simplify
symbol map update for fun_syms_, undefined_fun_syms_, var_syms_,
undefined_var_syms_.
The default compression flag defined by automake via GZIP_ENV is --best.
That is pretty expensive for when you just want to make sure your change
is done properly to pass 'make distcheck'.
Hence add a convenience target 'make distcheck-fast' that sets GZIP_ENV
to --fast and runs distcheck with that.
On my machine, this almost cuts the 'make distcheck' time in half:
distcheck 4min 40s
distcheck-fast 2min 32s
This is not invasive as it does not change the existing distcheck
target.
Kernel symbol whitelisting symbol is implemented using a regex to
suppress all function and variable names that don't match the given
list. This was completely broken for variables, resulting in all
variables being filtered out if a kernel whitelist was specified.
* src/abg-suppression-priv.h: In get_symbol_name_not_regex
method of variable_suppression::priv, fix typo causing
miscompilation of regex.
Mark Wielaard [Mon, 13 Apr 2020 01:40:58 +0000 (03:40 +0200)]
Add --drop-undefined-syms to abidw.
Add a drop_undefined_syms properties to the read_context that
indicates the reader wants to drop any undefined symbols (which don't
have associated addresses in the corpus). Implement this for the
dwarf_reader and abidw (when using the --drop-undefined-syms option).
* include/abg-dwarf-reader.h (set_drop_undefined_syms):
New declaration.
* src/abg-dwarf-reader.cc (class read_context): Add private
bool drop_undefined_syms_.
(drop_undefined_syms): New getter and setter method.
(set_drop_undefined_syms): New function.
(function_is_suppressed): Check drop_undefined_syms on
read_context.
(variable_is_suppressed): Likewise.
* src/abg-reader.cc (read_context): Add private bool
m_drop_undefined_syms.
(drop_undefined_syms): New getter and setter method.
* tools/abidw.cc (struct options): Add drop_undefined_syms.
(display_usage): Print --drop-undefined-syms.
(parse_command_line): Parse --drop-undefined-syms.
(main): Call set_drop_undefined_syms.
Signed-off-by: Mark Wielaard <mark@klomp.org> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Mark Wielaard [Sun, 12 Apr 2020 01:47:56 +0000 (03:47 +0200)]
Add --header-file option to add individual public header files.
Sometimes public header files are in the same directory as private
header files. In such cases --headers-dir cannot be used. Add
--header-file to add individual public header files.
* include/abg-tools-utils.h (gen_suppr_spec_from_headers): Add
hdr_files string vector argument.
* src/abg-tools-utils.cc (handle_file_entry): New function that
adds one specific file to the type_suppression. Implementation
lifted from...
(handle_fts_entry): ...here. Call handle_file_entry for each file.
(gen_suppr_spec_from_headers): Also takes a header_files string
vector as argument. Call handle_file_entry for each file entry.
* tools/abidiff.cc (options): Add header_files1 and header_files2
string vectors.
(display_usage): Print --header-file1 and --header-file2 usage.
(parse_command_line): Handle --header-file1, --hf1 and
--header-file2, --hf2.
(set_diff_context_from_opts): Call gen_suppr_spec_from_headers
with header_files1 and header_files2.
(set_suppressions): Likewise.
* tools/abidw.cc (options): Add header_files string vector.
(display_usage): Print --header-file usage.
(parse_command_line): Handle --header-file1, --hf1.
(maybe_check_header_files): New function.
(set_suppressions): Call gen_suppr_spec_from_headers with
header_files.
(main): Call maybe_check_header_files.
* tools/abilint.cc (options): Add header_files string vector.
(display_usage): Print --header-file usage.
(parse_command_line): Handle --header-file1, --hf1.
(set_suppressions): Call gen_suppr_spec_from_headers with
header_files.
* doc/manuals/abidiff.rst: Document --header-file1, --hf1 and
--header-file2, --hf2. Add new options to documentation of
--drop-private-types.
* doc/manuals/abidw.rst: Document --header-file, --hf.
* doc/manuals/abilint.rst: Likewise.
Signed-off-by: Mark Wielaard <mark@klomp.org> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
abg-reporter-priv.cc: Improve readability of represent helper function.
This change:
- makes local variables const where possible
- gives local variables more descriptive names
- factors out some more expressions as local variables
- simplifies the logic around anonymous data members
There are no behavioural changes.
* src/abg-reporter-priv.cc (represent): In the var_diff_sptr
overload, rename pretty_representation to o_pretty_representation,
introduce n_pretty_representation where needed and replace the
duplicate tr1 and tr2 with these; rename all other variables foo1
and foo2 to o_foo and n_foo respectively, using _type instead of
_t; introduce o_anon and n_anon and use them to make the local
variable is_strict_anonymous_data_member_change const, make
ABG_ASSERT in anonymous data member handling more obvious in the
case where anonymity has changed and allow simplification of
formatting in this case; move declarations of const local
variables above those of the non-const, state-tracking, variables.
abg-reporter-priv.cc: Fix anonymous member size change reports.
Data members have names (unless anonymous), types, sizes and
offsets (if a member of a class or struct). It is the responsibility
of the represent function to detail all differences of a changed
member. The size of a member is the same as the size of its type.
A recent change to the function focused on cleaning up the formatting
logic and added a catch-all case to the end of it in case no diff
information had been emitted by then.
This catch-all triggered for anonymous structs and union diffs under
certain circumstances when these anonymous members had changed in
size.
This patch ensures size change information for a member is emitted
exactly once (unless the type change has been or is being reported
already). It also ensures anonymous members are identified
appropriately and includes a test case would otherwise reach the
catch-all case in both default and --leaf-changes-only modes.
* src/abg-reporter-priv.cc (represent): In the var_diff_sptr
overload, factor out some expressions as local variables, rely
on diff_to_be_reported to decide whether to emit a change,
fold together local/non-local change reporting using
local_changes to preserve current name formatting differences,
keep track explicitly of whether size information has been
emitted and ensure it happens if needed, make offset and size
change reporting for anonymous data members more meaningful.
* tests/test-abidiff-exit.cc: Run new test cases.
* tests/data/Makefile.am: Add new test files.
* tests/data/test-abidiff-exit/test-member-size-v0.cc: New
test.
* tests/data/test-abidiff-exit/test-member-size-v0.o: Ditto.
* tests/data/test-abidiff-exit/test-member-size-v1.cc: Ditto.
* tests/data/test-abidiff-exit/test-member-size-v1.o: Ditto.
* tests/data/test-abidiff-exit/test-member-size-report0.txt:
New test, default mode.
* tests/data/test-abidiff-exit/test-member-size-report1.txt:
New test, --leaf-changes-only mode.
* tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt:
Eliminate duplicate reporting of member sizes.
* tests/data/test-abidiff-exit/test-leaf-more-report.txt: Ditto.
* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
Ditto.
* tests/data/test-abidiff-exit/test-no-stray-comma-report.txt:
Ditto.
* tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt:
Add size report for anonymous data member.
* tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt:
Ditto.
* 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:
Add missing size change report.
* tests/data/test-diff-suppr/test36-leaf-report-0.txt: Remove
size change report for previously reported type.
* tests/data/test-diff-suppr/test46-PR25128-report-1.txt:
Eliminate duplicate reporting of member size change.
* tests/data/test-diff-suppr/test46-PR25128-report-2.txt:
Ditto.
git://sourceware.org / libabigail.git / log