MM next stage #2
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Matthias Maennich <maennich@google.com>
myxoid
approved these changes
Sep 9, 2020
Contributor
myxoid
left a comment
myxoid
left a comment
There was a problem hiding this comment.
I was wondering what that was...
As of now, KMI whitelists are defined as ini-file style sections named with trailing "whitelist". A number of projects have recently introduced policies that discourage the use of terminology with negative connotations in an effort to make the language used in their code base more inclusive. One of such terms is 'whitelist'. In order to support projects choosing to avoid this term, add support for an alternative: "symbol_list". This allows e.g. a section name "[abi_symbol_list]". While commonly chosen alternatives are "allow_list", "include_list" or similar, I intentionally took out any positive/negative meaning as I think those are anyway misleading. (Is the whitelist referring to symbols that are allowed to change or is it referring to the list of symbols that are kept stable?) In an effort to make this more obvious, I went for "symbol list" to describe the symbols that are part of the ABI analysis either during extraction or during comparison. This is a backwards compatible change as we still support the prior suffix. I adjusted documentation and added tests accordingly. * doc/manuals/kmidiff.rst: Adjust documentation. * src/abg-tools-utils.cc (gen_suppr_spec_from_kernel_abi_whitelists): add support for additional whitelist section suffix. * tests/data/Makefile.am: Add new test files. * tests/data/test-kmi-whitelist/symbol-list-with-another-single-entry: New test case. * tests/data/test-kmi-whitelist/symbol-list-with-duplicate-entry: Likewise. * tests/data/test-kmi-whitelist/symbol-list-with-single-entry: Likewise. * tests/data/test-kmi-whitelist/symbol-list-with-two-sections: Likewise. * tests/test-kmi-whitelist.cc: Wire up new test cases. Reviewed-by: Giuliano Procida <gprocida@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
metti
force-pushed
the
mm-next-stage
branch
2 times, most recently
from
2071b8b to
58ecf73
Compare
During the canonicalization of a type T, the algorithm uses the
internal pretty representation of T to limit the number of types to
compare T to. That internal pretty representation is based on the
type name.
For anonymous types, the type name is not unique; it's constructed
just for internal purposes. So using that in the pretty
representation might negatively impact the accuracy of the
canonicalization; it might make it so that two types might wrongly be
considered canonicaly different.
To fix that, this change makes the internal pretty representation of
anonymous classes (and unions) use their flat representation.
For the record, the flat representation of an anonymous struct with a
an integer and a char data members is the string:
'struct {int i; char c;}'
* src/abg-ir.cc ({class, union}_decl::get_pretty_representation):
Use the flat representation of the class or union even for
internal purposes.
* tests/data/test-annotate/libtest23.so.abi: Adjust.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt: Likewise.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.txt: Likewise.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report1.txt: Likewise.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report2.txt: Likewise.
* tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-0.txt: Likewise.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt: Likewise.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.
* tests/data/test-read-dwarf/libtest23.so.abi: Likewise.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When using type maps the hash used for a given type T is usually the pointer value of the canonical type of T. In the rare cases where T doesn't have a canonical type, we were using a 'dynamic hash' computed by recursively walking the components of T and progressively building a hash that way. The dynamic hashing code hasn't been updated in a while and would need some overhaul to support hashing with schemes like MD5 or maybe sha even. It might be useful for various use cases that have been proposed by some users over the years but nobody was motivated enough to implement it. In the mean time, rather than trying to come up with a fully beefed up dynamic hashing code, we'd rather just return a constant number for non canonicalized types. In practise that amounts to forcing the code of the maps to always use structural comparison for those non canonicalized types. Note that the amount of non-canonicalized types should be fairly small. For now, the only non-canonicalized types should be declaration-only types and those are quite fast to compare anyway. This patch thus introduces a new hashing scheme for maps in the writer which just uses a numerical constant as the hash for non-canonicalized types. * include/abg-fwd.h (hash_as_canonical_type_or_constant): Declare ... * src/abg-ir.cc (hash_as_canonical_type_or_constant): ... new function. * src/abg-writer.cc (type_hasher::operator()): Use the new hash_as_canonical_type_or_constant. * tests/data/test-read-dwarf/test16-pr18904.so.abi: Adjust. Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* origin/master: writer: Avoid using dynamic hashing in type maps Use flat representation to canonicalize anonymous classes and unions Signed-off-by: Matthias Maennich <maennich@google.com>
metti
force-pushed
the
mm-next-stage
branch
from
58ecf73 to
94f5d4a
Compare
* src/abg-tools-utils.cc (get_vmlinux_path_from_kernel_dist): Fix thinko. Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In leaf mode, libabigail fails to report changes to the underlying
type of a typedef.
At its core, this is due to the fact that changes to the underlying
type of a typedef are not considered local. As the leaf reporter only
reports local changes (as opposed to non-local changes which are
changes to sub-types) it doesn't detect those non-local typedef
changes.
To handle this, this patch makes changes to the underlying type of a
typedef be considered as local changes. This is like what we already
do for pointer and qualified types.
Now that we have another set of changes to report in the leaf
reporter, we need to handle how to propagate the category and
redundancy status of those changes. The patch does this too.
Also, just like we do pointer and qualified type changes, the patch
avoids marking the diff node carrying the typedef change as being a
leaf change. That way, only existing leaf changes carrying that
typedef diff node will be reported. For instance, a function whose
parameter has a typedef change will be reported because that change to
the function is considered a leaf change. Otherwise, reporting the
typedef (or the pointer or qualified) type change on its own is not
useful unless it impacts those leaf changes that we deem useful.
The patch adds the example given in problem report to the testsuite.
* src/abg-ir.cc (equals): In the overload for typedef_decls,
report changes to the underlying type as being local of kind
LOCAL_TYPE_CHANGE_KIND.
* src/abg-comparison.cc
(leaf_diff_node_marker_visitor::visit_begin): Do not mark typedef
diff node as leaf node.
(suppression_categorization_visitor::visit_end): Propagate the
'suppressed' category of the underlying type to the parent typedef
unless the later has a local non-type change.
(redundancy_marking_visitor::visit_end): Likewise for the
'redundant' category.
* include/abg-reporter.h (report_non_type_typedef_changes): Rename ...
* src/abg-default-reporter.cc (report_non_type_typedef_changes):
... report_local_typedef_changes into this.
* src/abg-leaf-reporter.cc (leaf_reporter::report): Make the leaf
reporter invoke the reporting method of the default reporter for
typedefs as all typedef changes are now local.
* tests/data/test-diff-filter/test-PR26309-report-0.txt: Add new
test reference output.
* tests/data/test-diff-filter/test-PR26309-v{0,1}.o: Add new test
binary input.
* tests/data/test-diff-filter/test-PR26309-v{0,1}.c: Add source
code for new test binary input.
* tests/data/Makefile.am: Add the new text material above to
source distribution.
* tests/test-diff-filter.cc (in_out_specs): Add the new test input
above to this test harness.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When handling a binary with abidiff or abidw it can be useful to
provide several different header files directories, for the cases
where the header files of the binary are scathered in several
different directories.
It thus becomes possible to invoke abidiff like this:
abidiff --headers-dir1 first-header-dir1 \
--headers-dir1 second-header-dir1 \
--headers-dir2 first-header-dir2 \
--headers-dir2 second-header-dir2 \
binary1 binary2
This patch adds support for that. It also modifies the
tests/test-abidiff-exit.cc test harness to make it take header
directories. With that modification done, a new test is added in that
harness to exercise this new feature.
This should close the feature request over at
https://sourceware.org/bugzilla/show_bug.cgi?id=26565.
* doc/manuals/abidiff.rst: Update documentation for the
--headers-dir{1,2} options.
* doc/manuals/abidw.rst: Likewise for the --header-dir option.
* include/abg-tools-utils.h (gen_suppr_spec_from_headers): Add new
overload that takes a vector of headers root dirs.
* src/abg-tools-utils.cc (gen_suppr_spec_from_headers_root_dir):
Define new function.
(gen_suppr_spec_from_headers): Define a new overload that takes a
vector of head_root_dir strings; it uses the new
gen_suppr_spec_from_headers function. Use the new overload in the
previous one that takes just one head_root_dir string.
* tools/abidiff.cc (options::headers_dirs{1,2}): Rename
option::headers_dir{1,2} into this one and make it be a vector of
strings rather than just a string.
(parse_command_line): Support several --headers-dir{1,2} on the
command line.
(set_diff_context_from_opts, set_suppressions): Adjust.
* tools/abidw.cc (options::headers_dirs): Renamed
options::headers_dir into this and make it be a vector of strings
rather than just a string.
(parse_command_line): Support several --headers-dir on the command
line.
(set_suppressions): Adjust.
* tests/data/test-abidiff-exit/test-headers-dirs/headers-a/header-a-v{0,1}.h:
Header files of new binary test input.
* tests/data/test-abidiff-exit/test-headers-dirs/headers-b/header-b-v{0,1}.h:
Likewise.
* tests/data/test-abidiff-exit/test-headers-dirs/test-headers-dir-v{0,1}.c:
Source code of new binary test input.
* tests/data/test-abidiff-exit/test-headers-dirs/test-headers-dir-report-{1,2}.txt:
Reference output of new binary test input.
* tests/data/test-abidiff-exit/test-headers-dirs/test-headers-dir-v{0,1}.o:
New binary test input.
* tests/data/Makefile.am: Add the new files above to source
distribution.
* tests/test-abidiff-exit.cc (InOutSpec::in_elfv{0,1}_path): Add
new data members.
(in_out_specs): Adjust the content of this array as its type
changed. Also, add two new entries to run the test over the new
binary test inputs above.
(do_prefix_strings): Define new static function.
(main): Use it the new do_prefix_strings here. Make abidiff
use the --header-dir{1,2} option whenever header directories are
specified in an entry of the in_out_specs array.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* origin/master: Bug 26309 - Wrong leaf reporting of changes to typedef underlying type Fix thinko in get_vmlinux_path_from_kernel_dist Signed-off-by: Matthias Maennich <maennich@google.com>
If there ever is a discrepancy between the architectures of the corpuses of a corpus group, libabigail will just abort with an assertion, if enabled. However, this is a case of invalid input and the cause should be reported to the user. * src/abg-corpus.cc (corpus_group::add_corpus): Report architecture discrepancies. Signed-off-by: Giuliano Procida <gprocida@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
When building a union type we try to ensure that each member is
present only once. This is because the code to build the union is
also used to actually update a partially constructed union. To do so,
before adding a member to the union, the member is looked up (among
the current members) by name to see if it's already present or not.
But then for anonymous members, the name of the member is empty.
After the first anonymous member is added to the union, subsequent
look-ups with an empty name all succeed. So no more than one
anonymous member is added to the union. Oops.
A way to fix this is to perform the lookup by taking into account the
type of the anonymous data member, not its (empty) name. We already
do this for anonymous data members of classes/structs.
This patch thus uses that type-based anonymous data member lookup for
unions.
But then now that unions can have several anonymous members, another
issue was uncovered.
Array types whose elements are of anonymous type can be wrongly
considered different because of canonicalization issues.
Let's suppose we have these two arrays whose internal pretty
representation are:
"__anonymous_struct_1__ foo[5]"
and
"__anonymous_struct_2__ foo[5]"
These are arrays of 5 elements of type anonymous struct. Suppose the
anonymous structs "__anonymous_struct_1__" and
"__anonymous_struct_2__" are structurally equivalent. Because the
internal names of these array element types are different, the
internal pretty representations of the arrays are different. And thus
the canonical types of the two arrays are different. And that's
wrong. In this particular case, they should have the same canonical
type and thus be considered equivalent.
This patch thus teaches 'get_type_name' to make the internal type
name of anonymous types of a given kind be the same. Thus, making all
arrays of 5 anonymous struct have the same pretty representation:
"__anonymous_struct__ foo[5]"
This gives the type canonicalizer a chance to detect that those arrays
having the same canonical type.
These two changes while being seemingly unrelated need to be bundled
together to fix a number of issues in the existing test reference
outputs because fixing the first one is needed to uncover the later
issue.
* src/abg-dwarf-reader.cc (add_or_update_union_type): Don't use
the empty name of anonymous members in the lookup to ensure that
all data members are unique. Rather, use the whole anonymous
member itself for the lookup, just like is done to handle
anonymous data member in classes/structs.
* src/abg-reader.cc (build_union_decl): Likewise.
* src/abg-ir.cc (get_generic_anonymous_internal_type_name): Define
new static function.
(get_type_name): For internal purposes, make the type name of all
anonymous types of a given kind to be the same. This allows the
internal representation of anonymous types which are based on type
names to all be the same, so that they can be compared among
themselves during type canonicalization.
* tests/data/test-read-dwarf/test-PR26568-{1,2}.c: Source code of
binary test input.
* tests/data/test-read-dwarf/test-PR26568-{1,2}.o: New binary test input.
* tests/data/test-read-dwarf/test-PR26568-{1,2}.o.abi: New
reference test ouput.
* tests/data/Makefile.am: Add the new test material above to
source distribution.
* tests/test-read-dwarf.cc (in_out_specs): Add the new binary test
input above to this test harness.
* tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0.abi: Adjust.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
metti
force-pushed
the
mm-next-stage
branch
2 times, most recently
from
10decfc to
2fe4907
Compare
This works around an issue where the order matters for canonicalization depending on whether we see a type declaration-only or fully defined first. The fully defined type is to prefer, hence partition the types_to_canonicalize by whether they are declaration-only. Signed-off-by: Giuliano Procida <gprocida@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
metti
force-pushed
the
mm-next-stage
branch
from
2fe4907 to
1dca710
Compare
If there ever is a discrepancy between the architectures of the corpuses of a corpus group, libabigail will just abort with an assertion, if enabled. However, this is a case of invalid input and the cause should be reported to the user. * src/abg-corpus.cc (corpus_group::add_corpus): Report architecture discrepancies. Signed-off-by: Giuliano Procida <gprocida@google.com>
* origin/master:
abg-corpus.cc: report architecture discrepancies
Bug 26568 - Union should support more than one anonymous member
Make abidiff and abidw support several --headers-dir{1,2} options
Signed-off-by: Matthias Maennich <maennich@google.com>
Various test files were missing terminal newlines. * tests/data/test-diff-suppr/test0-type-suppr-2.suppr: Add final new line. * tests/data/test-diff-suppr/test22-suppr-removed-var-sym-4.suppr: Likewise. * tests/data/test-diff-suppr/test23-alias-filter-0.suppr: Likewise. * tests/data/test-diff-suppr/test23-alias-filter-4.suppr: Likewise. * tests/data/test-diff-suppr/test28-add-aliased-function-1.suppr: Likewise. * tests/data/test-diff-suppr/test28-add-aliased-function-2.suppr: Likewise. * tests/data/test-diff-suppr/test28-add-aliased-function-3.suppr: Likewise. * tests/data/test-diff-suppr/test28-add-aliased-function-4.suppr: Likewise. * tests/data/test-diff-suppr/test41-enumerator-changes-0.suppr: Likewise. * tests/data/test-diff-suppr/test7-var-suppr-7.suppr: Likewise. * tests/data/test-ini/test01-equal-in-property-string.abignore: Likewise. Signed-off-by: Giuliano Procida <gprocida@google.com>
The suppression specification in test38-char-class-in-ini.abignore was introduced in commit 1478d9c. Unfortunately it contains two errors. One causes the file name not to match as the string is the full path, not the base name. The other is a typo that causes the file name match not to even be attempted. The two mistakes cancel in the test, but result in a suppression specification that is broader than intended. * tests/data/test-diff-suppr/test38-char-class-in-ini.abignore: Don't anchor regex match to beginning of file name. Change "filename_regexp" to "file_name_regexp". Signed-off-by: Giuliano Procida <gprocida@google.com>
Since 2013 the implicit 'this' parameter has been excluded from the function parameters taken into account while comparing class member functions. This was an early measure to avoid infinite recursion that would then occur when comparing classes (and thus their member functions that are referenced in their vtable). But since then, we've built descent infrastructure to prevent this kind of recursion in a more generic manner. This patch thus removes that restriction and should therefore lift the concerns expressed in the bug https://sourceware.org/bugzilla/show_bug.cgi?id=26672. Namely, changes to (data members of) a class should now be detected when comparing member functions of that class. With this change, the reference output of several comparison regression tests changed because, obviously, some impacted member functions are now reported along with detecting changes in data membrers of classes. The patch thus adjusts those reference ouputs. The patch also adjust the behaviour of the predicate: "accessed_through = pointer|reference|reference-or-pointer" The idea is to make the predicate work on qualified version of a type. * include/abg-ir.h (function_type::get_first_parm): Declare new accessor. * src/abg-ir.cc (function_type::get_first_parm): Define new accessor. (equals): In the overload for function_type, always take the implicit "this" parameter into account in parameter comparisons. (function_type::get_first_non_implicit_parm): Adjust comment. * src/abg-comp-filter.cc (function_name_changed_but_not_symbol): Avoid potential NULL pointer dereferencing. * src/abg-comparison.cc (function_type_diff::ensure_lookup_tables_populated): Always take the changes to the implicit 'this' parameter into account in the function type diff. (compute_diff): In the overload for function_type, Always compare the implicit 'this' parameter when comparing function parameters. * src/abg-default-reporter.cc (default_reporter::report): Refer to "implicit parameter" when reporting changes on parameters artificially generated by the compiler. * src/abg-suppression.cc (type_suppression::suppresses_diff): Make the 'access_through' predicate work on a qualified version of type 'S', even if it was meant to work on type 'S'. This allows it to work on 'const S', especially when S is accessed through 'pointer to const S', which happens when we consider the implicit 'this' parameter of a const member function. * tests/data/test-abicompat/test5-fn-changed-report-0.txt: Adjust. * tests/data/test-abicompat/test5-fn-changed-report-1.txt: Likewise. * tests/data/test-abidiff-exit/test1-voffset-change-report0.txt: Likewise. * tests/data/test-abidiff/test-PR18791-report0.txt: Likewise. * tests/data/test-abidiff/test-struct1-report.txt: Likewise. * tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test0-report.txt: Likewise. * tests/data/test-diff-dwarf/test28-vtable-changes-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test36-ppc64-aliases-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt: Likewise. * tests/data/test-diff-dwarf/test5-report.txt: Likewise. * tests/data/test-diff-dwarf/test8-report.txt: Likewise. * tests/data/test-diff-filter/test0-report.txt: Likewise. * tests/data/test-diff-filter/test01-report.txt: Likewise. * tests/data/test-diff-filter/test10-report.txt: Likewise. * tests/data/test-diff-filter/test13-report.txt: Likewise. * tests/data/test-diff-filter/test2-report.txt: Likewise. * tests/data/test-diff-filter/test28-redundant-and-filtered-children-nodes-report-0.txt: Likewise. * tests/data/test-diff-filter/test28-redundant-and-filtered-children-nodes-report-1.txt: Likewise. * tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.txt: Likewise. * tests/data/test-diff-filter/test30-pr18904-rvalueref-report1.txt: Likewise. * tests/data/test-diff-filter/test30-pr18904-rvalueref-report2.txt: Likewise. * tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt: Likewise. * tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt: Likewise. * tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-0.txt: Likewise. * tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-1.txt: Likewise. * tests/data/test-diff-filter/test4-report.txt: Likewise. * tests/data/test-diff-filter/test41-report-0.txt: Likewise. * tests/data/test-diff-filter/test9-report.txt: Likewise. * tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt: Likewise. * tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt: Likewise. * tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-0.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-1.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-10.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-11.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-12.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-13.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-14.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-15.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-16.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-2.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-3.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-4.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-5.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-6.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-7.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-8.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-9.txt: Likewise. * tests/data/test-diff-suppr/test31-report-1.txt: Likewise. * tests/data/test-diff-suppr/test33-report-0.txt: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com>
A recent change[1] triggered a variation in the ABI changes reported,
depending on the platform.
It turned out the change made the redundancy detector walk more diff
nodes (like inserted/deleted virtual member functions and their
implicit parameter type) and that uncovered several underlying issues
that has been latent for a long time.
First, we were not walking the inserted/deleted virtual member
functions in a deterministic manner at reporting time. Rather than
walking the unordered maps containing those functions, this patch now
walk them in lexicographic order. The patch also does something
similar for the changed data members, but this time during the diff
graph analysis. That order affects how we consider a given type
change to be redundant.
Second, when looking a diff node named N, if another diff node N'
equivalent to N has already been marked redundant (and thus filtered
out already), we were sometimes wrongly failing to detect and mark N
as redundant. This patch fixes that.
I realized that some code was now unnecessary so I removed it.
A lot of reference output of tests are adjusted by this patch.
Mostly, these were cases we were failing to properly detect (and
filter out) as redundant reports. So the change reports should
hopefully look more concise and to the point now.
[1] the recent change is this one:
2f92777 Consider the implicit 'this' parameter when comparing methods
* src/abg-comparison-priv.h
(diff_context::priv::last_visited_diff_node_): Remove unnecessary
data member.
(class_or_union_diff::priv::sorted_{deleted,inserted}_member_functions_):
Add new data members.
(sort_string_member_function_sptr_map): Declare new function.
* src/abg-comparison.cc (sort_string_member_function_sptr_map):
Define new function.
(redundancy_marking_visitor::visit_begin): If the current diff
node is equivalent to another one that has been already marked
redundant, then consider the current diff node as redundant as
well. Considering the fact an ancestor node has been filtered out
is now useless because if that's the case then the current
descendant node wouldn't even be walked at reporting time. So
remove the call to diff_has_ancestor_filtered_out.
(categorize_redundancy): Remove useless call here as well.
(diff_has_ancestor_filtered_out, diff_has_ancestor_filtered_out)
(diff_context::{mark_last_diff_visited_per_class_of_equivalence,
clear_last_diffs_visited_per_class_of_equivalence,
get_last_visited_diff_of_class_of_equivalence}): Remove
unnecessary functions.
(redundancy_marking_visitor::visit_end): Add comment.
(class_diff::ensure_lookup_tables_populated): Lexicographically
sort inserted/deleted member functions.
(class_or_union_diff::chain_into_hierarchy): Chain changed data
members diff nodes in a sorted manner.
* src/abg-default-reporter.cc (default_reporter::report): Report
deleted/inserted member functions in lexicographic order.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt:
Adjust.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt:
Likewise.
* tests/data/test-diff-pkg/libICE-1.0.6-1.el6.x86_64.rpm--libICE-1.0.9-2.el7.x86_64.rpm-report-0.txt:
Likewise.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt:
Likewise.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt:
Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In the abixml writer, we recently stopped using type_base::dynamic_hash as a slow path to hash a non-canonicalized type. Rather, we use an arbitrary constant as a hash for those non-canonicalized types. This amounts to using structural comparison for those types. The function that implements this hashing scheme is hash_as_canonical_type_or_constant. A potential concern for that approach was the possible negative impact on speed. As it turns out since the change went in, there was no noticeable speed impact raised after testing. In insight, this is understandable as the number of non-canonicalized types should be extremely reduced now that we canonicalize pretty much all types. As far as I understand it at this point, the only types that would not be canonicalized are unresolved declaration-only types. And, all in all, structurally comparing unresolved declaration-only types should be "fast enough". If we ever stumble across any other non-canonicalized type, I think that would be an artifact of a bug that ought to be fixed. On that basis, I went ahead and used hash_as_canonical_type_or_constant throughout the code base and did away with the use of type_base::dynamic_hash for now, until it's properly audited, regression tested and got ready for the use cases where it might make sense. This patch thus makes hash_type use hash_as_canonical_type_or_constant. The writer is then back to using hash_type again, as it used to; but at the same time, it's still using structural comparison for non-canonilized types. So is hash_type_or_decl, which now uses hash_type to hash types, rather than using type_base::dynamic_hash. Note that the comparison engine heavily uses hash_type_or_decl to hash diff nodes. So with this small patch the comparison engine is now using structural comparison of non-canonicalized types (and diff nodes), just as the abixml writer does. * include/abg-fwd.h (hash_as_canonical_type_or_constant): Remove public declaration of this function. * src/abg-hash.cc (type_base::dynamic_hash::operator()): Add a comment. * src/abg-ir.cc (hash_as_canonical_type_or_constant): Make this function static now. (hash_type_or_decl): Use hash_type for types. * src/abg-writer.cc (type_hasher::operator()): Use hash_type. Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When working in development environments with compiler versions that might be very bleeding edge (like the Fedora Rawhide distribution) it might be worthwhile to disable all compiler optimization to have a better debugging experience. In practice, I bumped into this need again and again. So I am adding this ABIGAIL_NO_OPTIMIZATION_DEBUG environment variable to basically allow the "-g -O0" combination, if need be. This patch obviously doesn't change any existing behaviour if the user doesn't set this newly introduced environment variable. * configure.ac: Set the CXXFLAGS and CFLAGS to "-g -O0 -Wall -Wextra -Werror" if the ABIGAIL_NO_OPTIMIZATION_DEBUG is set. Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The function write_elf_symbol_reference iterates through aliases, looking for an unsuppressed alias to use. The existing code went wrong in the case when aliases are present. In the case of all symbols suppressed, it would also have selected the last alias, rather than the first, if the data structure invariants had matched the code's expectations. The main symbol is always distinguished. When aliases are absent, the sole symbol's next pointer is null, but when aliases are present, they form a circular list. This makes traversal of aliases a bit tricky. The code now picks the first symbol from the following: - the given symbol, if unsuppressed - the main symbol, if unsuppressed - the unsuppressed aliases in the remainder of the alias chain - the main symbol (suppressed) The given symbol, which need not be the same as the main symbol, will be tested twice, if suppressed, but addressing this would make the code even more elaborate and fragile. The last case may be unreachable if symbol suppression triggers when all aliases are suppressed. * src/abg-writer.cc (write_elf_symbol_reference): Check main symbol and aliases with more care. Fixes: f0fdfcb ("dwarf-reader/writer: consider aliases when dealing with suppressions") Signed-off-by: Giuliano Procida <gprocida@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
* origin/master: configure: Support ABIGAIL_NO_OPTIMIZATION_DEBUG environment variable Structurally compare the few non-canonicalized types in general Fix redundancy detection in the diff graph Consider the implicit 'this' parameter when comparing methods Fix two wrongs in test suppression regex Add missing newlines to end of test files. Signed-off-by: Matthias Maennich <maennich@google.com>
Applies to current master, 8d7ffe3 ("configure: Support ABIGAIL_NO_OPTIMIZATION_DEBUG environment variable") Signed-off-by: Matthias Maennich <maennich@google.com>
* include/abg-tools-utils.h (enum abidiff_status): Fix a comment. Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch was originally submitted by in a comment of a problem report at https://sourceware.org/bugzilla/show_bug.cgi?id=26684#c10. When reading the bounds of a subrange_type, we need to know if the constant value of those bounds is signed or not. For that, we used to just look at the form of those constant and infer the signedness from there. The problem is that the signedness is really determined by the value of the DW_AT_encoding attribute present on the underlying type of the subrange_type; the form of the bound is mere implementation detail that is downstream of the information of carried by the DW_AT_encoding attribute. This patch thus does the right thing by looking at the underlying type of the subrange_type and by doing away with the unnecessary messing with attribute value forms. * src/abg-dwarf-reader.cc (die_attribute_has_form) (die_attribute_is_signed, die_attribute_is_unsigned) (die_attribute_has_no_signedness): Remove static functions. (die_constant_attribute): Add the 'is_signed' parameter. (die_address_attribute): Adjust comment. (build_subrange_type): Determine signedness of the bounds by looking at the DW_AT_encoding attribute of the underlying type. Signed-off-by: Mark Wielaard <mark@klomp.org> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
…ven. When running abidiff on test34-libjemalloc.so.2-intel-16.0.3 it would crash in array_type_def::subrange_type::get_length on the ABG_ASSERT get_upper_bound() >= get_lower_bound(). This was because that file contained a subrange upper_bound value encoded with data1 or data2 without an underlying type. In that case we assumed the value was encoded as a signed value which caused some of the upper bounds to be negative (while the lower bound, which wasn't given, was assumed to be zero). * src/abg-dwarf-reader.cc (build_subrange_type): Default is_signed to false. Signed-off-by: Mark Wielaard <mark@klomp.org> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
metti
force-pushed
the
mm-next-stage
branch
2 times, most recently
from
08e1ee4 to
31aa0eb
Compare
Some types like unnamed-enum-underlying-type are not distinguished by type_topo_comp. This can result in nondeterministic output and flakey tests. While a more complete ordering from type_topo_comp would be nice, the nondeterminism can reduced by preserving the relative order of identically-named types. * src/abg-ir.cc (scope_decl::get_sorted_canonical_types): Sort canonical types with std::stable_sort(..., type_topo_comp()). Signed-off-by: Giuliano Procida <gprocida@google.com> Reviewed-by: Matthias Maennich <maennich@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
The functor virtual_member_function_less_than did not take into account linkage name which can be the only difference when multiple destructors with differing mangled names are present. This change adds a check for linkage names and also flattens the control flow in the comparison method to make the logic clearer. Lastly, this change also uses std::stable_sort, in case all that remains is insertion order. * src/abg-ir.cc (virtual_member_function_less_than::operator()): Name temporaries like offsets and symbols to reduce repetition; test each pair of elements (including symbol presence) and return immediately if there's a difference; add a comparison of linkage name just after comparing symbol names. (sort_virtual_member_functions): Use stable_sort instead of sort. * tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: Update with new ordering of member functions. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. Signed-off-by: Giuliano Procida <gprocida@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
The ordering of canonical types (in an abi-instr XML element) appears to be sensitive to the particular C++ library used and the presence of other threads doing heap allocation. This patch forces distinct synthetic enum-underlying types to have distinct names, which ensures deterministic XML output order. * src/abg-dwarf-reader.cc (build_internal_underlying_enum_type_name): Add a size argument (and don't default is_anonymous argument). Append size of type to synthetic type name. (build_enum_underlying_type): Pass type size to build_internal_underlying_enum_type_name. * tests/data/test-abidiff-exit/test-decl-enum-report-3.txt: Update. Note that there may be an issue with leaf-mode reporting of pointer type changes. * tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0.abi: Regenerate this (catching up with various abidw updates). * tests/data/test-annotate/test-anonymous-members-0.o.abi: Refresh with new type names. * tests/data/test-annotate/test0.abi: Likewise. * tests/data/test-annotate/test13-pr18894.so.abi: Likewise. * tests/data/test-annotate/test14-pr18893.so.abi: Likewise. * tests/data/test-annotate/test15-pr18892.so.abi: Likewise. * tests/data/test-annotate/test17-pr19027.so.abi: Likewise. * tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-annotate/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: Likewise. * tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise. * tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise. * tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi: Likewise. * tests/data/test-read-dwarf/test0.abi: Likewise. * tests/data/test-read-dwarf/test0.hash.abi: Likewise. * tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise. * tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise. * tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise. * tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise. * tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise. * tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise. * tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise. * tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise. * tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. * tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise. Signed-off-by: Giuliano Procida <gprocida@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
…tion
Location expressions might occasionally be of length 0. E.g. a reason
for that are thread local variables that do not exactly have a location
to refer to. Compilers/Linkers may choose an odd representation. E.g.
see the dwarfdump output for the added testcase based on libandroid.so
(from AOSP).
$ dwarfdump libandroid.so|egrep -B9 "gChoreographerE$"
LOCAL_SYMBOLS:
< 1><0x00000022> DW_TAG_namespace
DW_AT_name android
< 2><0x00000027> DW_TAG_variable
DW_AT_name gChoreographer
DW_AT_type <0x00000b65>
DW_AT_decl_file 0x00000003 .../choreographer.cpp
DW_AT_decl_line 0x00000059
DW_AT_location len 0x0000: :
DW_AT_linkage_name _ZN7androidL14gChoreographerE
The DW_AT_location is properly read by elfutils' dwarf_location(), but
is not useful for us to proceed with. Hence early exit on this.
* src/abg-dwarf-reader.cc (die_location_expr): Ignore zero
length location expressions.
* tests/data/Makefile.am: Add new test files.
* tests/data/test-read-dwarf/test-libandroid.so: New test file.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/test-read-dwarf.cc: Add new test case.
Reported-by: Dan Albert <danalbert@google.com>
Reviewed-by: Giuliano Procida <gprocida@google.com>
Cc: Mark Wielaard <mark@klomp.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
When using relative compilation unit paths in DWARF, the lookup for an existing one was done with an incorrect path. In particular, a '/' was prefixed to the path regardless of whether the compilation_dir is set. That led to the instantiation of an additional translation unit that and failed when adding it to the corpus due to violating translation unit uniqueness. Fix that by correcting the lookup. * src/abg-dwarf-reader.cc(build_translation_unit_and_add_to_ir): Fix lookup for potentially already existing translation units. Reported-by: Dan Albert <danalbert@google.com> Reviewed-by: Giuliano Procida <gprocida@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
Some types like unnamed-enum-underlying-type are not distinguished by type_topo_comp. This can result in nondeterministic output and flakey tests. While a more complete ordering from type_topo_comp would be nice, the nondeterminism can reduced by preserving the relative order of identically-named types. * src/abg-ir.cc (scope_decl::get_sorted_canonical_types): Sort canonical types with std::stable_sort(..., type_topo_comp()). Signed-off-by: Giuliano Procida <gprocida@google.com> Reviewed-by: Matthias Maennich <maennich@google.com> Signed-off-by: Matthias Maennich <maennich@google.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When using relative compilation unit paths in DWARF, the lookup for an existing one was done with an incorrect path. In particular, a '/' was prefixed to the path regardless of whether the compilation_dir is set. That led to the instantiation of an additional translation unit that and failed when adding it to the corpus due to violating translation unit uniqueness. Fix that by correcting the lookup. * src/abg-dwarf-reader.cc(build_translation_unit_and_add_to_ir): Fix lookup for potentially already existing translation units. Reported-by: Dan Albert <danalbert@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
Sometimes when amending a C++ class to add new members or properties to it as directed by the DWARF debug information, we can end-up with discrepancies related to declaration-only-ness. That is, an instances of a given type Foo can be wrongly assigned declaration-only-ness that should have been only carried by another instance Foo. Then, later, comparing two pointers to Foo might wrongly lead to spurious reported changes due to the spurious differences of declaration-only-ness in two instances of Foo. By fixing the setting of the declaration-only-ness, especially when amending a C++ class this patch fixes that spurious change detected. * src/abg-dwarf-reader.cc (add_or_update_class_type): When creating a class, set declaration-only-ness unconditionally. When updating the class however, only set the declaration-only-ness when the current one is not consistent with the size of the class. * tests/data/test-annotate/test14-pr18893.so.abi: Adjust. * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise. * tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* tests/update-test-output.py: Update syntax for python3. Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The functor virtual_member_function_less_than did not take into account linkage name which can be the only difference when multiple destructors with differing mangled names are present. This change adds a check for linkage names and also flattens the control flow in the comparison method to make the logic clearer. Lastly, this change also uses std::stable_sort, in case all that remains is insertion order. * src/abg-ir.cc (virtual_member_function_less_than::operator()): Name temporaries like offsets and symbols to reduce repetition; test each pair of elements (including symbol presence) and return immediately if there's a difference; add a comparison of linkage name just after comparing symbol names. (sort_virtual_member_functions): Use stable_sort instead of sort. * tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: Update with new ordering of member functions. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. Signed-off-by: Giuliano Procida <gprocida@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
The ordering of canonical types (in an abi-instr XML element) appears to be sensitive to the particular C++ library used and the presence of other threads doing heap allocation. This patch forces distinct synthetic enum-underlying types to have distinct names, which ensures deterministic XML output order. * src/abg-dwarf-reader.cc (build_internal_underlying_enum_type_name): Add a size argument (and don't default is_anonymous argument). Append size of type to synthetic type name. (build_enum_underlying_type): Pass type size to build_internal_underlying_enum_type_name. * tests/data/test-abidiff-exit/test-decl-enum-report-3.txt: Update. Note that there may be an issue with leaf-mode reporting of pointer type changes. * tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0.abi: Regenerate this (catching up with various abidw updates). * tests/data/test-annotate/test-anonymous-members-0.o.abi: Refresh with new type names. * tests/data/test-annotate/test0.abi: Likewise. * tests/data/test-annotate/test13-pr18894.so.abi: Likewise. * tests/data/test-annotate/test14-pr18893.so.abi: Likewise. * tests/data/test-annotate/test15-pr18892.so.abi: Likewise. * tests/data/test-annotate/test17-pr19027.so.abi: Likewise. * tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-annotate/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: Likewise. * tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise. * tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise. * tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi: Likewise. * tests/data/test-read-dwarf/test0.abi: Likewise. * tests/data/test-read-dwarf/test0.hash.abi: Likewise. * tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise. * tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise. * tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise. * tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise. * tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise. * tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise. * tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise. * tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise. * tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. * tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise. Signed-off-by: Giuliano Procida <gprocida@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
…tion
Location expressions might occasionally be of length 0. E.g. a reason
for that are thread local variables that do not exactly have a
location to refer to. Compilers/Linkers may choose an empty location
description. E.g. see the dwarfdump output for the added testcase
based on libandroid.so (from AOSP).
$ dwarfdump libandroid.so|egrep -B9 "gChoreographerE$"
LOCAL_SYMBOLS:
< 1><0x00000022> DW_TAG_namespace
DW_AT_name android
< 2><0x00000027> DW_TAG_variable
DW_AT_name gChoreographer
DW_AT_type <0x00000b65>
DW_AT_decl_file 0x00000003 .../choreographer.cpp
DW_AT_decl_line 0x00000059
DW_AT_location len 0x0000: :
DW_AT_linkage_name _ZN7androidL14gChoreographerE
The DW_AT_location is properly read by elfutils' dwarf_location(), but
is not useful for us to proceed with. Hence early exit on this.
* src/abg-dwarf-reader.cc (die_location_expr): Ignore zero
length location expressions.
* tests/data/Makefile.am: Add new test files.
* tests/data/test-read-dwarf/test-libandroid.so: New test file.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/test-read-dwarf.cc: Add new test case.
Reported-by: Dan Albert <danalbert@google.com>
Reviewed-by: Giuliano Procida <gprocida@google.com>
Cc: Mark Wielaard <mark@klomp.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Update this test reference output as per the commit below: b7ad7f2 Bug 26770 - Spurious declaration-only-ness induces spurious type changes * tests/data/test-read-dwarf/test-libandroid.so.abi: Update. Signed-off-by: Dodji Seketeli <dodji@redhat.com>
CV-qualifiers of a typedef of an array type apply to the elements of
the array. So this can be transformed into a typedef of array type
which element type is similarly CV-qualified.
That transformation helps avoiding spurious changes that might occur
when comparing the latter form against the former even though both are
equivalent.
This patch performs that transformation, just like we already do for
CV-qualified array types which are transformed into an array of
similarly CV-qualified elements.
Performing that transformation amounts to editing the type of the
array elements. As those types might be used by other parts of the
type graph (type node sharing), the patch "clones" the type sub-tree
of interest and edits the cloned version. That way, the shared type
nodes are not edited. It appears that in the existing version of
maybe_strip_qualification, the transformation of CV-qualified arrays
into arrays of CV-qualified elements was unfortunately editing shared
type nodes. The patch fixes that and re-works the logic of*
maybe_strip_qualification to make it handle this new case and the
previous one in a somewhat generic manner.
* include/abg-fwd.h (is_typedef_of_array, clone_array)
(clone_typedef, clone_qualified_type, clone_array_tree): Declare
new functions.
(peel_qualified_or_typedef_type): Declare new overload.
(is_array_of_qualified_element): Constify the parameter.
* include/abg-ir.h ({qualified_type,
typedef}_def::set_underlying_type): Add new member functions.
(array_type_def::subrange_type::subrange_type): Make constify the
reference to the underlying type parameter.
* src/abg-ir.cc (is_array_of_qualified_element): Constify the
parameter.
(peel_qualified_or_typedef_type): Define new
overload for type_base_sptr.
(clone_typedef_array_qualified_type): Define static function.
(clone_array clone_typedef, clone_qualified_type)
(clone_array_tree, is_typedef_of_array): Define new functions.
(qualified_type_def::get_underlying_type): Rename the return type
shared_ptr<type_base> into type_base_sptr.
({typedef, qualified_type}_def::set_underlying_type): Define new
member function.
(array_type_def::subrange_type::priv::priv): Initialize the
'infinite_' data member.
* src/abg-dwarf-reader.cc (maybe_strip_qualification): Handle
qualified typedef of arrays. Merge this with the handling of
qualified arrays. Note that before editing the elements of the
array to make the array (or typedef) qualifier apply to the
element the sub-tree is cloned to make its type nodes be
'un-shared'. This prevents us from editing type nodes that are
shared by other type expressions.
* tests/data/test-diff-filter/test-PR26739-report-0.txt: New
reference test output.
* tests/data/test-diff-filter/test-PR26739-2-report-0.txt: Likewise.
* tests/data/test-diff-filter/test-PR26739-v{0,1}.c: Source code
of new binary test input.
* tests/data/test-diff-filter/test-PR26739-2-v{0,1}.c: Likewise.
* tests/data/test-diff-filter/test-PR26739-v{0,1}.o: New binary
test inputs.
* tests/data/test-diff-filter/test-PR26739-2-v{0,1}.o: Likewise.
* tests/data/Makefile.am: Add the new test material above to
source distribution.
* tests/test-diff-filter.cc (in_out_specs): Add the test inputs
above to this harness.
* tests/data/test-annotate/test15-pr18892.so.abi: Adjust.
* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When emitting the declarations of a given translation unit, those declarations are not sorted. Ooops. This patch adds topological sorting for those declarations, making the decls defined first to be emitted first. When the decls are defined at the same location then the pretty representation is used for lexicographic sorting instead. It turns out that during the topological sorting for types there was some uncertainty when the declarations of the types had the same definition location. This patch re-uses the declaration sorting above for the declarations of these types. * include/abg-ir.h (scope_decl::get_sorted_member_decls): Declare new member function. * src/abg-ir.cc (struct decl_topo_comp): New sorting functor. (type_topo_comp::operator()): Re-use the decl_topo_comp to sort type declarations. (scope_decl::priv::sorted_members_): Add new data member. (scope_decl::get_sorted_member_decls): Define new member function. * src/abg-writer.cc (write_translation_unit): Use the new scope_decl::get_sorted_member_decls. * tests/data/test-annotate/libtest23.so.abi: Adjust. * tests/data/test-annotate/test15-pr18892.so.abi: Likewise. * tests/data/test-annotate/test17-pr19027.so.abi: Likewise. * tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-annotate/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/libtest23.so.abi: Likewise. * tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise. * tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise. * tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise. * tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. * tests/data/test-read-write/test2.xml: Likewise. * tests/data/test-read-write/test28-without-std-fns-ref.xml: Likewise. * tests/data/test-read-write/test28-without-std-vars-ref.xml: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* origin/master: writer: Sort decls and fix topological sorting for types Bug PR26739 - Handle qualified typedef array types Update test-libandroid.so.abi dwarf-reader: Ignore zero length location expressions from DW_AT_location Improve enum synthetic type names Improve and stabilise sort of member functions update-test-output.py: Update syntax Bug 26770 - Spurious declaration-only-ness induces spurious type changes dwarf-reader: fix lookup for repeated translation unit paths Stabilise sort of canonical types Assume subrange bounds types are unsigned if no underlying type is given. dwarf-reader: get subrange_type bounds signedness from underlying type abg-tools-utils: Fix comment Signed-off-by: Matthias Maennich <maennich@google.com>
This function would misbehave if called for any symbol other than the current main symbol. For safety, we now assert this. Also, when updating the main symbol, the loop condition performed deep equality on symbols which might lead to not all symbols in the chain being updated. * src/abg-ir.cc (elf_symbol::update_main_symbol): Assert we are called at the current main symbol; iterate over the alias chain in a safer fashion. Signed-off-by: Giuliano Procida <gprocida@google.com> Signed-off-by: Matthias Maennich <maennich@google.com>
metti
force-pushed
the
mm-next-stage
branch
2 times, most recently
from
eab7e64 to
2011244
Compare
metti
force-pushed
the
mm-next-stage
branch
2 times, most recently
from
15d161e to
2012074
Compare
added 2 commits
December 7, 2020 15:35
* src/abg-ir.cc (types_have_similar_structure): Arrays are also indirect types, just like pointers and references, for the purpose of the concept of "type similarity". Add that to the introductory comment of the function. Add some more misc comments throughout the code base. Signed-off-by: Dodji Seketeli <dodji@redhat.com> Signed-off-by: Matthias Maennich <maennich@google.com>
As described in the comments of types_have_similar_structure:
"Two indirect types have similar structure if their underlying
types are of the same kind and have the same name. [...] The size
of their underlying type does not matter"
Yet, the size of array elements (a.k.a the underlying type of an array type)
is wrongly considered to matter when assessing the "type similarity"
relationship for arrays.
This patch fixes that.
* src/abg-ir.cc (types_have_similar_structure): When
examining array types, always treat element type changes as
indirect changes.
* tests/data/Makefile.am: Add new test case files.
* tests/data/test-abidiff-exit/test-non-leaf-array-report.txt:
New test case showing correct --leaf-changes-only reporting.
* tests/data/test-abidiff-exit/test-non-leaf-array-v0.c:
Likewise.
* tests/data/test-abidiff-exit/test-non-leaf-array-v0.o:
Likewise.
* tests/data/test-abidiff-exit/test-non-leaf-array-v1.c:
Likewise.
* tests/data/test-abidiff-exit/test-non-leaf-array-v1.o:
Likewise.
* tests/test-abidiff-exit.cc: Run new test case.
Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.