D3 #50
Open
D3 #50
Conversation
This file contains 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
The problem this patch addresses is visible when doing: $ fedabidiff --debug --self-compare -a --from fc36 wildmagic5 More specifically: tools/abidw --noout -d usr/lib/debug/ usr/lib64/libWm5Mathematics.so.5.17 This is reported at https://sourceware.org/bugzilla/show_bug.cgi?id=29302#c2: Functions changes summary: 0 Removed, 9 Changed, 0 Added functions Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 9 functions with some indirect sub-type change: [C] 'method virtual Wm5::ConvexHull<double>::~ConvexHull(int)' at Wm5ConvexHull.h:24:1 has some indirect sub-type changes: implicit parameter 0 of type 'Wm5::ConvexHull<double>*' has sub-type changes: in pointed to type 'class Wm5::ConvexHull<double>': type size changed from 384 to 0 (in bits) 1 member function deletion: 'method virtual Wm5::ConvexHull<double>::~ConvexHull(int)' at Wm5ConvexHull.cpp:32:1 no member function changes (2 filtered); [C] 'method virtual Wm5::ConvexHull<float>::~ConvexHull(int)' at Wm5ConvexHull.h:24:1 has some indirect sub-type changes: implicit parameter 0 of type 'Wm5::ConvexHull<float>*' has sub-type changes: in pointed to type 'class Wm5::ConvexHull<float>': type size changed from 320 to 0 (in bits) 1 member function deletion: 'method virtual Wm5::ConvexHull<float>::~ConvexHull(int)' at Wm5ConvexHull.cpp:32:1 no member function changes (2 filtered); [...] It appears that some class type DIEs don't have any size information and are marked as being declarations. These have an incomplete set of data member / virtual member functions compared to DIEs for the SAME types described elsewhere in the DWARF. About these, the specification says: 5.7.1 Structure, Union and Class Type Entries [...] An incomplete structure, union or class type is represented by a structure, union or class entry that does not have a byte size attribute and that has a DW_AT_declaration attribute. But then the DWARF reader doesn't know about these so it creates several types from these incomplete DIEs. Those types are thus incorrect. And that leads to spurious self comparison changes down the road. This patch thus detects that the DIEs is for an incomplete type and does not build an IR for its sub-types. It rather builds a declaration-only type. Then later, that declaration-only type is resolved to its corresponding definition using the existing read_context::resolve_declaration_only_classes function. The patch also removes the temporary speed hack that was introduced into read_context::resolve_declaration_only_classes() by the commit: b9af1f3 dwarf-reader: Avoid long comparisons when resolving C++ decl-only classes This is no longer a problem thanks to the great speed improvement we have from the commit: 69795d1 Bug 29303 - Cache the result of structural aggregate comparison * src/abg-dwarf-reader.cc (read_context::resolve_declaration_only_classes): Do not avoid comparing ODR-relevant classes. (add_or_update_class_type): Detect incomplete classes and treat them as decl-only types. They'll be resolved to their proper complete types later, by read_context::resolve_declaration_only_classes. * tests/data/test-annotate/libtest23.so.abi: Adjust. * 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/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-diff-filter/test41-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-read-dwarf/PR22122-libftdc.so.abi: Likewise. * tests/data/test-read-dwarf/libtest23.so.abi: Likewise. * tests/data/test-read-dwarf/test-libaaudio.so.abi: Likewise. * tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise. * tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise. * tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise. * tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise. * tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise. * tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise. * tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise. * tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. * tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise. Signed-off-by: Dodji Seketeli <[email protected]>
* tests/data/test-diff-filter/test41-report-0.txt: Adjust. * tests/data/test-diff-pkg-ctf/gmp-6.x.x86_64-report-0.txt: Adjust. * tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt: Adjust. * tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt: Adjust. Signed-off-by: Dodji Seketeli <[email protected]>
When emitting the pretty representation of pointers from the IR, it can so happen that the pointed type is a decl-only type which has been resolved to a definition. In that case, the decl-only type might be anonymous while the definition has a naming typedef, effectively giving it a name. Then, using the decl-only type to construct the name of the type might yield a different type name, more precisely, the internal "anonymous" name of that type. This can lead to several types having the same anonymous name, leading to instability with respect to sorting. The patch just looks through the decl-only pointed-to types before using their name to construct the name of the pointer type. The patch does that for reference types as well. * src/abg-ir.cc (look_through_decl_only): New overload for type_base. * tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust. Signed-off-by: Dodji Seketeli <[email protected]>
When using non-internal pretty representation of array types (e.g, for sorting of types in a given scope for the purpose of serialization), some array element types might have the same name, even though they don't have the same qualified name. In those cases, the serialized abixml output is not stable. This patches uses qualified names for array element names for type sorting purposes. However, this patch uncovers a problem that shows up in the tests outputs for test-abidiff-exit and test-diff-filter, where emitting qualified names of qualified types shows that there can be redundant qualifiers in the serialized output. This issue will be fixed separately in a later commit. For now, the output of these tests is temporarily updated to have the tests pass. * src/abg-ir.cc (get_type_representation): In the overload for array_type_def, use qualified names for element types. * tests/data/test-abidiff-exit/qualifier-typedef-array-report-1.txt: Adjust. * tests/data/test-annotate/test15-pr18892.so.abi: Adjust. * tests/data/test-annotate/test17-pr19027.so.abi: Adjust. * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Adjust. * tests/data/test-diff-filter/test-PR26739-2-report-0.txt: Adjust. Signed-off-by: Dodji Seketeli <[email protected]>
While looking at something else, I noticed that there are some
qualified types built from the DWARF debug info that read:
const volatile const int m[5]
That's a tree of (chained) qualified types that end up having some
redundant qualifiers. That IR tree might look like:
[C] --> [C|V] --> [int]
We want to edit that IR tree to make it look like:
[C] --> [V] --> [int]
And that would serialize as
const volatile int m[5]
This patch introduces the editing of the qualified type IR tree to
remove the redundant qualifiers, right after the IR is built from
DWARF.
* include/abg-fwd.h (strip_redundant_quals_from_underyling_types):
Declare new function.
* include/abg-ir.h (operator&=): Declare new binary operator for
qualified_type_def::CV and ...
* src/abg-ir.cc (+operator&=): ... define it here.
(strip_redundant_quals_from_underyling_types): Define new function
and ...
* src/abg-dwarf-reader.cc (maybe_strip_qualification): ... Use it
here.
* tests/data/test-abidiff-exit/qualifier-typedef-array-report-1.txt:
Adjust.
Signed-off-by: Dodji Seketeli <[email protected]>
On some platforms, "long int" and "long long int" can have the same
size. In that case, we want those two types to be equivalent from ABI
standpoint. Otherwise, through the use of typedefs and pointers, two
structs "C" defined in different translation units where one uses
"long int" in a translation unit and "long long int" in another should
be considered ABI compatible if long int and long long int have the
same size on that platform.
Otherwise, that causes spurious type changes that lead to self
comparison change down the road. For instance, the following command
fails:
$ tools/fedabipkgdiff --debug --self-compare -a --from fc36 btrfs-progs
This patch thus changes the comparison engine of the IR so that the
"short, long and long long" modifiers don't change the result of
comparing integral types that share the same base type when they have
the same size.
* include/abg-fwd.h (is_integral_type): Declare new function.
* include/abg-ir.h (type_decl::get_qualified_name): Add a
declaration of an implementation of the virtual interface
get_qualified_name.
* src/abg-ir-priv.h (integral_type::set_modifiers): Define a new
setter.
(integral_type::to_string): Add an "internal" flag.
* src/abg-ir.cc (operator~, operator&=): Declare
new operators.
(get_internal_integral_type_name): Define new static function.
(decl_base::priv::{temporary_internal_qualified_name_,
internal_qualified_name_}): Define two new data members.
(get_type_name): For internal name of integral types, use the new
get_internal_integral_type_name function.
(is_integral_type): Define new function.
(integral_type::set_modifiers): Define new member function.
(operator|, operator&): Fix some indentation.
(operator~, operator&=): Define new operators.
(parse_integral_type): Fix the logic of this function. Namely, it
wasn't handling parsing "long long" correctly.
(integral_type::to_string): Add an "internal" flag.
(equals): In the overload for type_decl, do not take the short,
long and long long into account when comparing integral types of
the same size.
(type_decl::get_qualified_name): Define new method.
(type_decl::get_pretty_representation): For internal name of
integral types, use the new get_internal_integral_type_name
function.
({decl,type}_topo_comp::operator()): Use the non-internal pretty
representation of decls/types for sorting purpose.
* src/abg-reader.cc (build_type_decl): We don't expect the
integral type name from abixml to the same as the name of the
parsed integral type, as the abixml file can be old and have an
old format.
* tests/data/test-annotate/libtest23.so.abi: Adjust.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Adjust.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Adjust.
* tests/data/test-annotate/test0.abi: Adjust.
* tests/data/test-annotate/test15-pr18892.so.abi: Adjust.
* tests/data/test-annotate/test17-pr19027.so.abi: Adjust.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Adjust.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Adjust.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Adjust.
* tests/data/test-annotate/test21-pr19092.so.abi: Adjust.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
Adjust.
* tests/data/test-diff-filter/test41-report-0.txt: Adjust.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt:
Adjust.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt:
Adjust.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
Adjust.
* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
Adjust.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Adjust.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Adjust.
* tests/data/test-read-dwarf/libtest23.so.abi: Adjust.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi: Adjust.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Adjust.
* tests/data/test-read-dwarf/test-PR26568-1.o.abi: Adjust.
* tests/data/test-read-dwarf/test-PR26568-2.o.abi: Adjust.
* tests/data/test-read-dwarf/test-libaaudio.so.abi: Adjust.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Adjust.
* tests/data/test-read-dwarf/test0.abi: Adjust.
* tests/data/test-read-dwarf/test0.hash.abi: Adjust.
* tests/data/test-read-dwarf/test1.hash.abi: Adjust.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Adjust.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Adjust.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Adjust.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Adjust.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Adjust.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Adjust.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Adjust.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Adjust.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Adjust.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust.
* tests/data/test-read-write/test22.xml: Adjust.
* tests/data/test-read-write/test23.xml: Adjust.
* tests/data/test-read-write/test28-without-std-fns-ref.xml: Adjust.
* tests/data/test-read-write/test28-without-std-vars-ref.xml: Adjust.
Signed-off-by: Dodji Seketeli <[email protected]>
After the latest commits, it apears that sorting typedefs referenced by by other types that are emitted in the abixml file is not stable when the underlying types are integral types. This happens after the commit: commit 7cd8374 Author: Dodji Seketeli <[email protected]> Date: Fri Jul 22 22:45:04 2022 +0200 ir: Consider integral types of same kind and size as equivalent This patch fixes that by using the non-internal pretty representation (which disambiguates integral types) of types for sorting purposes. * src/abg-writer.cc (read_context::type_ptr_cmp::operator()): In the less-than operator the type pointer comparison functor, use the non-internal pretty representation of types for sorting purposes. * tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Adjust * tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. Signed-off-by: Dodji Seketeli <[email protected]>
…alent
This patch is a revert of my previous patch referenced below:
commit 7cd8374
Author: Dodji Seketeli <[email protected]>
Date: Fri Jul 22 22:45:04 2022 +0200
ir: Consider integral types of same kind and size as equivalent
On some platforms, "long int" and "long long int" can have the same
size. In that case, we want those two types to be equivalent from ABI
standpoint. Otherwise, through the use of typedefs and pointers, two
structs "C" defined in different translation units where one uses
"long int" in a translation unit and "long long int" in another should
be considered ABI compatible if long int and long long int have the
same size on that platform.
That patch was doing several things, including fixing the parsing of
integral modifiers in parse_integral_type, fixing the sorting of types
for serialization purposes based on their pretty representation as
well as considering int types with the same size as equivalent,
independently from the short and long modifiers.
This patch just reverts the last item of the list above from the 'equals'
function that handle type_decls and updates the impacted tests
accordingly.
This is because there seems to be a consensus in the community about
the fact that libabigail should do as much as possible to detect API
incompatibilities when it's possible, rather than just focusing on ABI
incompatibilities. Point taken.
Please note that the following command will still fail now:
$ tools/fedabipkgdiff --debug --self-compare -a --from fc36 btrfs-progs
This is because the self-comparison check of the 'btrfs' program
from that package fails:
$ abidw --abidiff -d usr/lib/debug usr/sbin/btrfs
That issue will need to be addressed in a different manner, I guess.
* src/abg-ir.cc (equals): In the overload for type_decl, do not
consider int types of the same type as being equivalent by
overlooking their long and short modifiers.
* tests/data/test-annotate/libtest23.so.abi: Adjust.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-annotate/test0.abi: Likewise.
* tests/data/test-annotate/test15-pr18892.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-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/libtest23.so.abi: Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi:
Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libaaudio.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/test0.abi: Likewise.
* tests/data/test-read-dwarf/test0.hash.abi: Likewise.
* tests/data/test-read-dwarf/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/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.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/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
* tests/data/test-read-write/test28-without-std-fns-ref.xml:
Likewise.
* tests/data/test-read-write/test28-without-std-vars-ref.xml:
Likewise.
Signed-off-by: Dodji Seketeli <[email protected]>
It seems like some recent changes have changed the reference expected output of the test-read-ctf tests. I believe it's this change that is the culprit: df28c22 writer: Make sorting referenced typedefs types stable in abixml This patch updates the expected reference output accordingly. * tests/data/test-read-ctf/test-PR26568-1.o.abi: Adjust. * tests/data/test-read-ctf/test-PR26568-2.o.abi: Likewise. * tests/data/test-read-ctf/test-ambiguous-struct-A.o.hash.abi: Likewise. * tests/data/test-read-ctf/test-ambiguous-struct-B.o.hash.abi: Likewise. * tests/data/test-read-ctf/test-anonymous-fields.o.abi: Likewise. * tests/data/test-read-ctf/test-array-of-pointers.abi: Likewise. * tests/data/test-read-ctf/test-callback2.abi: Likewise. * tests/data/test-read-ctf/test-conflicting-type-syms-a.o.hash.abi: Likewise. * tests/data/test-read-ctf/test-conflicting-type-syms-b.o.hash.abi: Likewise. * tests/data/test-read-ctf/test-list-struct.abi: Likewise. * tests/data/test-read-ctf/test0.abi: Likewise. * tests/data/test-read-ctf/test0.hash.abi: Likewise. * tests/data/test-read-ctf/test1.so.abi: Likewise. * tests/data/test-read-ctf/test1.so.hash.abi: Likewise. * tests/data/test-read-ctf/test7.o.abi: Likewise. * tests/data/test-read-ctf/test9.o.abi: Likewise. Signed-off-by: Dodji Seketeli <[email protected]>
When analysing the ld.so binary, the DWARF reader drops the description of the rtld_global_ro global variable on the floor. This is because it fails to get the decl that the rtld_global_ro variable belongs to. So it doesn't know where the put the description of rtld_global_ro, in the IR. The root cause is that the DWARF partial_unit that contains the description of the type of the rtld_global_ro variable doesn't contain any description of the language its types originate from. The DWARF reader needs to know the type, to know if it needs to construct a map that associates each type/decl DIE to their parent DIE. This is useful to determine which namespace a given DIE belongs to. But then this is needed only for languages that support naming organising devices like namespaces, namely, C++, Ada, Java etc. For C, the DWARF reader knows that by default, all decls/types belong to the global namespace. But then in this particular case, the partial_unit doesn't carry any language information, so libabigail just drops the ball on the floor, so there is no information for the rtld_global_ro variable. The fix is thus to consider the absence of a DIE->PARENT map as implying that all decls are defined in the global namespace. * src/abg-dwarf-reader.cc (get_scope_for_die): Consider that in the absence of the DIE->PARENT map, all decls are in the global. namespace. * tests/data/test-abidiff-exit/ld-2.28-210.so: New test. * tests/data/test-abidiff-exit/ld-2.28-211.so: Likewise. * tests/data/test-abidiff-exit/test-ld-2.28-210.so--ld-2.28-211.so.txt: New reference test output. * tests/test-abidiff-exit.cc (in_out_specs): Add the test above to the harness. Signed-off-by: Dodji Seketeli <[email protected]>
The archive `vmlinux.ctfa' contain CTF debug information for
all the types used by more than one module, CTF for the core
kernel and CTF for each module compiled in Linux tree directory.
CTF information for out-of-tree module is not present in
`vmlinux.ctfa' file, even so, the compiler can emit the `.ctf'
section into the out-of-tree modules and it can be extracted
by the libabigail tools.
* src/abg-ctf-reader.cc (process_ctf_archive, read_corpus
slurp_elf_info): Avoid looking for `vmlinux.ctfa' when we aren't
processing a `cur_corpus_group_'. So CTF info is embedded in the
`.ko' file.
* tests/data/Makefile.am: Add test inputs and expected files.
* tests/data/test-read-ctf/test-linux-module.{ko,c,abi}: Add new
test input and reference kABI.
* tests/test-read-ctf.cc: Add new testcase.
Signed-off-by: Guillermo E. Martinez <[email protected]>
Signed-off-by: Dodji Seketeli <[email protected]>
The code to print out the remaining declaration-only types unintentionally omitted the first such type. This change fixes the logic and uses a single test to decide whether or not to print the stats header line. * src/abg-dwarf-reader.cc (read_context::resolve_declaration_only_classes): Fix conditional logic so that showing stats includes the first unresolved type. (read_context::resolve_declaration_only_enums): Likewise. Signed-off-by: Giuliano Procida <[email protected]>
The code that makes the last attempt to resolve declaration-only types was protected by a conditional checking that the number of TUs for a given type was more than 1. The previous branch checked for exactly 1. However, the entire block is inside a conditional where the number of TUs is guaranteed to be greater than 0. Removing the conditional makes it clear that this branch handles all remaining cases. * src/abg-dwarf-reader.cc (read_context::resolve_declaration_only_classes): Remove tautological conditional. Signed-off-by: Giuliano Procida <[email protected]>
The logic for resolving declaration-only enums and classes was almost the same. However, the class code had a couple of extra improvements that were missing from the enum code. One of these caused resolution failures with Linux kernel ABIs, resulting in duplicate (declared / defined) enums in ABI XML. This change adds the improvements to the enum resolution code. * src/abg-dwarf-reader.cc (read_context::resolve_declaration_only_enums): Use an ordered map to ensure TUs are always considered in the same order and so improve ABI XML stability. Given multiple possible definitions for a enum declaration, check to see if they are equal and resolve the declaration to the first definition if so. Signed-off-by: Giuliano Procida <[email protected]>
I forgot to update tests/data/Makefile.am when I added the test input
files test-abidiff-exit/ld-2.28-21{0,1}.so and
test-abidiff-exit/test-ld-2.28-210.so--ld-2.28-211.so.txt. Fixed thus.
Also, I am adding a new file that describes how to get the source code
of those binaries.
* tests/data/Makefile.am: Add the new files to source
distribution. Also, add the new
test-abidiff-exit/ld-2.28-21x.so.sources.txt.
* tests/data/test-abidiff-exit/ld-2.28-21x.so.sources.txt: New
file describing where the sources are.
Signed-off-by: Dodji Seketeli <[email protected]>
…asses While looking at something else, I stumbled upon this problem. When comparing a class, equals first calls the overload for class_or_union to compare data members. If we are in the process of type canonicalization, the right hand side operand might be "canonical-type-propagated", during that call to the overload. In other words, it can inherit the canonical type of the left-hand-side operand. The problem is that that canonical type propagation, if it happens, is too early because this equals function still needs to compare other things like virtual member functions etc. So, the original intend of the code was to erase the canonical type that might have been propagated. This is all and well. The problem however is that the code /always/ erases the canonical type of the right hand side operand, even if it was the result of the propagation optimization long before it entered this equals function. Oops. This patch fixes that issue. * src/abg-ir.cc (equals): In the overload for const class_decl&, do not cancel the propagated canonical type if the propagation is not the result of invoking the equals overload for class_or_union. Signed-off-by: Dodji Seketeli <[email protected]>
I noticed that some code failed trying to call translation_unit::is_empty in the absence of the instance of the abigail::ir::environment type used to create the IR. This is because translation_unit::is_empty was trying to create something. That's odd. It shouldn't need to create anything to test for its emptiness. Fixed thus. * src/abg-ir.cc (translation_unit::is_empty): If there is no global scope, then we know its empty. No need to create one. Signed-off-by: Dodji Seketeli <[email protected]>
…o IR
This is a long overdue clean-up.
Back in the day when we had no DWARF type DIE canonicalization, there
could be a LOT of IR types to be canonicalized. To store set of those
types to be canonicalized, we the used the already existing
association type DIE offset -> IR types that we had, and so we only
had to store the DIE offsets of the IR types that we wanted to
canonicalize. At canonicalization time, we'd walk the set of DIE
offsets we stored on the side, use the association type DIE offset ->
IR types to retrieve the IR type to canonicalize and we'd canonicalize
it. This is somewhat complicated.
Now that we have DWARF DIEs canonicalization, the number of IR types
dropped significantly so this complicated scheme is no more warranted.
So this patch drops that indirection and stores the IR types to be
canonicalize into a vector of IR types and that's it. Yay, less code.
* src/abg-dwarf-reader.cc (maybe_canonicalize_type): Remove
the overload that takes a Dwarf_Die* parameter.
(operator++(die_source& source)): Likewise.
(read_context::{types_to_canonicalize_,
alt_types_to_canonicalize_,
type_unit_types_to_canonicalize_}): Remove these data members
of type vector<Dwarf_Off>. (read_context::initialize): Remove
invocations to alt_types_to_canonicalize_.clear(),
type_unit_types_to_canonicalize_.clear(), and
extra_types_to_canonicalize_.clear().
(read_context::extra_types_to_canonicalize_): Rename this into
read_context::types_to_canonicalize_, of type
vector<type_base_sptr>.
(read_context::types_to_canonicalize): Remove these member
functions that take a parameter of type die_source.
(read_context::extra_types_to_canonicalize): Rename this
function into types_to_canonicalize. It returns a type const
vector<type_base_sptr>&.
(read_context::schedule_type_for_late_canonicalization):
Remove this overload that takes a type const Dwarf_Die*. In
the overload that takes a parameter const type_base_sptr
however, rename the invocation to
extra_types_to_canonicalize_.push_back(t) into
types_to_canonicalize_.push_back(t).
(read_context::canonicalize_types_scheduled): This doesn't
take a die_source parameter anymore. It now only cycles
through the types retrieved by types_to_canonicalize() to
canonicalize them.
(read_context::add_late_canonicalized_types_stats): Remove the
die_source parameter. Use types_to_canonicalize().
(read_context::perform_late_type_canonicalizing): Just call
read_context::canonicalize_types_scheduled().
(build_ir_node_from_die): Adjust calls to maybe_canonicalize_type.
Also, really canonicalize the function type when a function decl is
constructed.
* tests/data/test-annotate/test13-pr18894.so.abi: Adjust.
* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.txt:
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/test13-pr18894.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/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <[email protected]>
Add libtool versioning to libabigail.so starting from 2.1 onward to
not cause problems with older utilities due to library version
incompatibility. This will also stop tools that look for libraries
which have a different ABI but the same version from complaining.
* configure.ac: Define the variables libabigail_so_{current,
revision, age}. These are to be adjusted after each releases
depending on how the compatibility status of the libabigail's
code.
* src/Makefile.am: Add -version-info
$(LIBABIGAIL_SO_CURRENT):$(LIBABIGAIL_SO_REVISION):$(LIBABIGAIL_SO_AGE)
to LDFLAGS.
Signed-off-by: Ben Woodard <[email protected]>
Signed-off-by: Dodji Seketeli <[email protected]>
…mber
The current mechanism used by the ctf reader for looking for debug
information given a specific Linux symbol is the following: it opens
the dictionary (default) which name matches the binary name being
processed in the current corpus, e.g. `vmlinux' or
`module-name`.ko. However there are symbols and information that are
not located in the default dictionary; this is evident comparing the
symbols in `Module.symvers' file with ABI XML file, so for example,
the ctf reader is expecting to find the information for
`LZ4_decompress_fast' symbol in the CTF `vmlinux' archive member,
because this symbols is defined in `vmlinux' binary:
0x4c416eb9 LZ4_decompress_fast vmlinux EXPORT_SYMBOL
But, it figures out that it is missing. The correct location is
`vmlinux#0' dictionary:
CTF archive member: vmlinux:
...
Function objects:
...
CTF archive member: vmlinux#0:
Function objects:
...
LZ4_decompress_fast -> 0x80037400: (kind 5) int (*) (const char *, char *, int) (aligned at 0x8)
...
Therefore, ctf reader must be looking for debug information in the
whole archive; fortunately `libctf' provides a fast lookup mechanism
using cache, dictionary references, etc., so the penalty performance
is ~10%.
Now, it make use of `ctf_lookup_by_symbol_name' at first instance
which is in charge to locate symbol information given a symbol name on
either CTF Function or Variable sections; if the symbol isn't found it
tries using `ctf_lookup_variable' to look into the CTF Variable
section; this could happens due to `ld' operating with the
`--ctf-variables' option which makes function types information to
reside in the CTF Variable section.
* src/abg-ctf-reader.cc (lookup_symbol_in_ctf_archive): New function.
(process_ctf_archive): Use `lookup_symbol_in_ctf_archive'.
Signed-off-by: Guillermo E. Martinez <[email protected]>
Signed-off-by: Dodji Seketeli <[email protected]>
When a global variable named V has the same name as member variable
that appears in an anonymous scope and if the the abixml writer emits
the member variable V first, it gets confused when comes the time to
emit the global V as it wrongly thinks it's been already emitted.
This is because when emitting the "internal" pretty representation of
the member variable, libabigail fails to consider printing a qualified
name. So the two 'V' wrongly have names that can't be told apart.
For instance consider the testcase example:
struct A {
struct {
int xx; // #0
};
};
The qualified name of xx, for internal purposes (to name things
internally for the purpose of book keeping) would be:
'A::__anonymous_struct__::xx'.
Libabigail wrongly names it 'xx', hence the confusion with the global variable.
Fixed thus.
* src/abg-ir.cc (var_decl::get_pretty_representation): Always use
qualified name of vars in an anonymous scope for internal
purposes.
* tests/data/test-annotate/PR29443-missing-xx.o.annotated.abi: New
reference test output.
* tests/test-annotate.cc (in_out_specs): Add the above to the test
harness.
* tests/data/test-read-dwarf/PR29443-missing-xx.cc: New source
code for the test.
* tests/data/test-read-dwarf/PR29443-missing-xx.o: New test input
binary.
* tests/data/test-read-dwarf/PR29443-missing-xx.o.abi: New test
reference output.
* tests/data/Makefile.am: Add the new test material to source
distribution.
* tests/test-read-dwarf.cc (in_out_specs): Add the above to the
test harness.
Signed-off-by: Dodji Seketeli <[email protected]>
When running runtestannotate on different architectures, some spurious test failures can happen because the resulting abixml contains the architecture of the original binary. This can be a problem for tests where the binary is compiled on the fly. We don't yet have those, but I was playing with some of these while debugging something else and stumbled across that issue. This patch thus removes mentions of the architecture of the binary, just like what runtestreaddwarf does. * tests/test-annotate.cc (main): Add the --no-architecture option to abidw. * tests/data/test-annotate/PR29443-missing-xx.o.annotated.abi: Adjust. * tests/data/test-annotate/libtest23.so.abi: Likewise. * tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Likewise. * tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise. * tests/data/test-annotate/test-anonymous-members-0.o.abi: Likewise. * tests/data/test-annotate/test0.abi: Likewise. * tests/data/test-annotate/test1.abi: Likewise. * tests/data/test-annotate/test13-pr18894.so.abi: Likewise. * tests/data/test-annotate/test14-pr18893.so.abi: Likewise. * tests/data/test-annotate/test15-pr18892.so.abi: Likewise. * tests/data/test-annotate/test17-pr19027.so.abi: Likewise. * tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-annotate/test2.so.abi: Likewise. * tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-annotate/test21-pr19092.so.abi: Likewise. * tests/data/test-annotate/test3.so.abi: Likewise. * tests/data/test-annotate/test4.so.abi: Likewise. * tests/data/test-annotate/test5.o.abi: Likewise. * tests/data/test-annotate/test6.so.abi: Likewise. * tests/data/test-annotate/test7.so.abi: Likewise. * tests/data/test-annotate/test8-qualified-this-pointer.so.abi: Likewise. Signed-off-by: Dodji Seketeli <[email protected]>
In a previous patch, some file paths were butchered. Ooops. Fixed thus. * tests/data/Makefile.am: Fix test file paths. Signed-off-by: Dodji Seketeli <[email protected]>
In 'src/abg-reader.cc', the function
'walk_xml_node_to_map_type_ids(read_context& ctxt, xmlNodePtr node)'
finds all of the child nodes of 'node' that has the 'id' attribute,
and then put the child node into map where the 'id' of the child node
as key and the child node itself as the value.
But the comment for this function writes:
/* src/abg-reader.cc begin */
/// Walk an entire XML sub-tree to build a map where the key is the
/// the value of the 'id' attribute (for type definitions) and the key
/// is the xml node containing the 'id' attribute.
...
static void
walk_xml_node_to_map_type_ids(read_context& ctxt,
xmlNodePtr node)
...
/* src/abg-reader.cc end */
The second and third lines of the comment above says the the child node
as the key of the map, but it should be the value of the map.
This patch fix the problematic comment described above, from
'and the key is the xml node' to 'and the value is the xml node'.
* src/abg-reader.cc (walk_xml_node_to_map_type_ids): fix comment
Signed-off-by: Xiaole He <[email protected]>
Signed-off-by: Dodji Seketeli <[email protected]>
Apparently guile produced ELF files don't set sh_entsize for the dynamic section. Which would cause a divide by zero. Luckily we do know how big an dynamic entry should be. So use gelf_fsize for ELF_T_DYN if sh_entsize is zero. * src/abg-dwarf-reader.cc (get_soname_of_elf_file): Make sure entsize is non-zero before use. https://sourceware.org/bugzilla/show_bug.cgi?id=29346 Signed-off-by: Mark Wielaard <[email protected]>
During the type DIEs comparison for canonicalization we need to
perform the optimization called "canonical type propagation". This is
very similar to what we do for type canonicalization at the IR level.
It's described in-extenso in abg-ir.cc, in the comment that starts
with "@defgroup OnTheFlyCanonicalization".
Basically during canonicalization, suppose we end-up comparing a
sub-type pair {Sl, Sr}. Suppose Sr does already have a canonical
type. Suppose also, that {Sl, Sr} ends up comparing equal. We can
thus deduce that Sl would have the same canonical type as Sr. So we
'propagate' the canonical type of Sr onto Sl. Sl is said to have been
canonical-type-propagated from Sr.
Now, I need to introduce the concept of redundant type.
Consider an aggregate type T and its sub-types ST0, ST1 and ST2 which
looks like this:
T
+-- ST0
|
+-- ST1
| +
| |
| +-- T
|
+-- ST2
Notice how the sub-type ST1 itself has a sub-type T.
Now, consider the type T', similar to T, which looks like:
T'
+-- ST0'
|
+-- ST1'
| +
| |
| +-- T'
|
+-- ST2'
|
+-- ST'3
Now consider how libabigail compares those two types T and T'
member-wise, aka structurally:
T T'
+-- ST0 +-- ST0'
| |
+-- ST1 +-- ST1' <--- Let's call this point #0.
| + | +
| | | |
| +-- T | +-- T' <--- Let's call this point #1.
| | Note that the sub-types of
+-- ST2 +-- ST2' T and T' are not
| represented at this point
+-- ST'3 but they do exist!
Representing them would lead
to a never-ending graph, right?
The properties of T are compared against the properties of T', but to
complete that comparison, the sub-type ST0 is compared against ST0',
etc. A comparison stack is thus maintained. Each member of the stack
is the pair of sub-types being compared. That content changes as
different sub-types are compared. Let's consider the content of the
stack when we reach the point #0 in the graph above. That stack
content would look like:
[{T,T'} | {ST0, ST0'}]
If we keep going and reach the point #1, the content of the stack
becomes:
[{T,T'} |{ST0, ST0'} | {T, T'}]
At this point, we see that {T,T'} appears twice in the stack. If we
keep going and explore again the sub-types of {T,T'}, an infinite loop
appears. The pair {T,T'} is said to be redundant.
To avoid those infinite loops, redundancy detection is done by
compare_dies in abg-dwarf-reader.cc. When compare_dies detects at #1 that
{T,T'} was already present in the stack then it returns "true" early,
as if in #1, T compares equal to T'.
But then what does that mean for the value of comparing {ST0,ST0'}?
The sub-type {T,T'} in #1 compared equal, so compare_dies assumes that
{ST0,ST0'} compares equal as well, right? That is what libabigail
used to assume before the commit
commit 7ecef63
Author: Dodji Seketeli <[email protected]>
Date: Mon Apr 4 15:35:48 2022 +0200
Canonicalize DIEs w/o assuming ODR & handle typedefs transparently
Well, it turns out we need to compare the entire tree of sub-types of
{T,T'} until we reach {ST2, ST2'}. At that point, compare_dies sees
that ST2' has a sub-type ST3' where ST2 has none. So {ST2,ST2'}
compares /different/. So {T,T'} compares different. So back to #0,
because {ST1,ST2'} has {T,T'} as sub-type (or said differently,
{ST1,ST2'} depends on the redundant pair {T,T'}) then {ST1,ST1'}
compares different.
So {ST1,ST1'} compares different even though it were initially thought to
compare equal because compare_dies had to get out early in #1,
considering that {T,T'} compared equal at that point.
Now, there are two ways to operate when we reach #1:
1/ Avoid performing canonical type propagation as soon as we detect
that {T,T'} is redundant. That avoidance lasts until we finish
comparing {ST2,ST2'}, that is, until the complete comparison of
{T,T'}. That means hat comparing every single sub-type is almost
assured to be done structurally, rather than canonically.
2/ Speculate that {T,T'} compare equal, unless proved otherwise. If
{T,T'} proves to be equal, then things are well and fast. If they
prove different, then {ST0,ST0'} must be edited afterwards to cancel
the canonical type propagation that might have taken place. In other
words, sub-types that depends on redundant types pairs must be tracked
until the comparison of those redundant type pairs is done. If a
redundant type pair compares different then all the sub-types that
depend on it must be walked to have their propagated canonical types
erase as if no canonical type propagation ever took place.
The first approach is the one I initially put in place with the patch
referred to above. It proved to be super slow. Analyzing the kernel
was taking literally hours. Oops.
This patch implements the second approach. It's more involved than
the first approach. But it brings the time down to around 2 minutes
on a reasonably fast machine. This is still slower than what I would
like to see, but it's way better than what had with the first
approach.
A subsequent patch will bring the analysis time for the kernel further
down. But in the mean time, this one is really needed, I think.
So the patch introduces a new type named offset_pairs_stack_type to
track the pairs of type DIEs being compared. Each DIE is now
identified by a new offset_type type. That type contains the
traditional DIE offset using the Dwarf_Off type from elfutils, but it
also contains the source of that DIE. The offset_pairs_stack_type
also tracks redundant type pairs and their dependant type pairs.
compare_dies is modified to return a value that is an instance of the
new enum comparison_result. The results can be
COMPARISON_RESULT_EQUAL when a type pair compares equal,
COMPARISON_RESULT_DIFFERENT when a type pair compares different,
COMPARISON_RESULT_CYCLE_DETECTED when a redundant type is detected,
leading to a comparison cycle, and COMPARISON_RESULT_UNKNOWN when the
outcome of the comparison cannot be known because we are comparing a
pair that depends on a redundant pair.
A new function return_comparison_result is introduced. It's intended
to be invoked right before returning from compare_dies. It looks at
the actual comparison result and depending on the state of the stack
of type pairs being compared, handles the book keeping of redundant
types and their dependant types. It also handles when to propagate
canonical types if necessary and when to cancel the canonical types
that might have been propagated.
The ABG_RETURN macro has been adapted to invoke
return_comparison_result before returning out from compare_dies.
* src/abg-ir-priv.h (enum comparison_result): Define new enum.
* src/abg-dwarf-reader.cc (type_comparison_result_to_be_cached)
(maybe_cache_type_comparison_result)
(get_cached_type_comparison_result)
(maybe_get_cached_type_comparison_result)
(maybe_propagate_canonical_type, propagate_canonical_type)
(return_comparison_result): Define new static functions.
(has_offset_pair, insert_offset_pair, erase_offset_pair)
(have_offset_pair_in_common): Remove static functions.
(read_context::die_comparison_visits_): Remove data member. The
concept supported by this data member is now replaced by caching
the results of comparing aggregate types, especially those that
are not yet canonicalized. This essentially prevents the same
aggregate type pair to be compared again and again.
(read_context::{die_comparison_results_, compare_count_,
canonical_propagated_count_, cancelled_propagation_count_}): New
data members.
(read_context::initialize): Initialize the new data members
compare_count_, canonical_propagated_count_,
cancelled_propagation_count_ of integral type.
(read_context::{erase_canonical_die_offset}): New member
functions.
(struct offset_pairs_stack_type): Define new type.
(die_offset): Remove.
(is_canon_type_to_be_propagated_tag): Add union types to the set
of types for which canonical type propagation might occur.
(is_type_die_to_be_canonicalized): Add function types and array
types to the types to be canonicalized.
(ABG_RETURN): Change this macro to consider
COMPARISON_RESULT_DIFFERENT rather than the "false" boolean.
Also, it uses the new return_comparison_result function.
(ABG_RETURN_FALSE): Likewise, use the new return_comparison_result
function.
(SET_RESULT_TO_FALSE): Make this return
COMPARISON_RESULT_DIFFERENT.
(SET_RESULT_TO, RETURN_IF_COMPARISON_CYCLE_DETECTED): Define new
macros.
(compare_dies): Make this return comparison_result rather than
just a bool. This is also the core of the overhaul of the
canonical DIE propagation algorithm. The algorithm is now similar
to the one implemented in the equals function for class_or_union
types in abg-ir.cc. It's described in the comment that starts
with '@defgroup OnTheFlyCanonicalization' in abg-ir.cc. The type
of the aggregates_being_compared parameter is now
offset_pairs_stack_type in parameter. No more need for the
redundant_aggregates_being_compared parameter. The new
offset_type that also encapsulates the source of the offset is now
used in lieu of the Dwarf_Off type. Results of comparing
aggregates being compared are now cached. When comparing
aggregates, use the RETURN_IF_COMPARISON_CYCLE_DETECTED to return
early if a cycle is detected. The invocation of the ABG_RETURN
macro (especially the call to return_comparison_result) is where
the book keeping for canonical types propagation takes place, so
remove the explicit code that was handling that from the end of
this function.
(read_debug_info_into_corpus): Print statistics about the number
of aggregates compared and canonical-type-propagated.
* tests/data/test-annotate/test14-pr18893.so.abi: Adjust.
* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-diff-pkg/GtkAda-gl-2.24.2-29.fc29.x86_64--2.24.2-30.fc30.x86_64-report-0.txt:
Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/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/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.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <[email protected]>
Profiling showed that the DWARF reader scans too much data.
Basically, in build_translation_unit_and_add_to_ir,
build_ir_node_from_die is called on every single DIE that is seen, for
a given translation unit.
There are interfaces (function and variable decls) that are not
associated with exported ELF symbols and that are analyzed by
build_ir_node_from_die nonetheless. For instance, interfaces that are
visible outside of their translation units are analyzed and the types
that are reachable from those interfaces are analyzed as well.
Once that is done, an ABI corpus is built with the subset of
interfaces that have exported ELF symbol (strictly those that are part
of the ABI), but types that are not necessarily reachable from those
ABI interfaces can also be put into the ABI corpus.
Some tools make use of this "lose" behaviour of libabigail. For
instance, abicompat precisely wants to analyze interfaces with
undefined symbols. For an application, those interfaces represents
the interfaces that the application expects to be provided by some
shared library.
When analyzing the exported interface of the Linux Kernel (or any
other huge application) however, analyzing more types than necessary
appears to incur a huge time penalty.
So, this patch introduces an optional behaviour whereby
build_translation_unit_and_add_to_ir is restricted to analyzing
interfaces that have exported ELF symbols only. So only the types
reachable from those interfaces are analyzed. This more than halves
the time spent by "abidw --noout vmlinux".
Strictly speaking, this new behaviour is triggered by a new option named
--exported-interfaces-only, supported by the tools abidw, abidiff,
abipkgdiff and kmidiff.
When looking at the Linux Kernel however, this option is enabled by
default.
Note that an option --allow-non-exported-interfaces is also introduce
to function under the previous model of operations. This option is
enabled by default on all the tools when they are not looking at the
Linux Kernel.
With this enabled, analyzing the Linux Kernel is back to taking less
than a minute on a reasonable machine.
* doc/manuals/tools-use-libabigail.txt: New doc text.
* doc/manuals/Makefile.am: Add the new tools-use-libabigail.rst
tool to the source distribution.
* doc/manuals/abidiff.rst: Include the new
tools-use-libabigail.rst. Document the --exported-interfaces-only
and --allow-non-exported-interfaces.
* doc/manuals/abidw.rst: Likewise.
* doc/manuals/abipkgdiff.rst: Likewise.
* doc/manuals/kmidiff.rst: Likewise.
* include/abg-ir.h
(environment::{user_set_analyze_exported_interfaces_only,
analyze_exported_interfaces_only}): Declare new accessors.
* src/abg-ir.cc
(environment::{user_set_analyze_exported_interfaces_only,
analyze_exported_interfaces_only}): Define new accessors.
* src/abg-dwarf-reader.cc (die_is_variable_decl)
(die_is_function_decl): Define new static functions.
(read_context::is_decl_die_with_exported_symbol): Define new
member function.
(read_context::get_{function,variable}_address): Const-ify the
Dwarf_Die* parameter.
(build_translation_unit_and_add_to_ir): If the user asks to
analyze exported interfaces only, the analyze only interfaces
that have exported ELF symbols.
(read_debug_info_into_corpus): If we are looking at the Linux
Kernel, then only analyze exported interfaces unless the user asks
otherwise.
* src/abg-ir-priv.h
(environment::priv::analyze_exported_interfaces_only_): Define new
data member.
* tools/abidiff.cc (options::exported_interfaces_only): Define new
data member.
(display_usage): Add new help strings for
--exported-interfaces-only and --allow-non-exported-interfaces.
(parse_command_line): Parse the new options
--exported-interfaces-only and --allow-non-exported-interfaces.
(main): Pass the value of opts.exported_interfaces_only to the
environment.
* tools/abidw.cc (options::exported_interfaces_only): Define new
data member.
(display_usage): Add new help strings for
--exported-interfaces-only and --allow-non-exported-interfaces.
(parse_command_line): Parse the new options
(load_corpus_and_write_abixml)
(load_kernel_corpus_group_and_write_abixml): Pass the value of
opts.exported_interfaces_only onto the environment.
* tools/abipkgdiff.cc (options::exported_interfaces_only): Define new
data member.
(display_usage): Add new help strings for
--exported-interfaces-only and --allow-non-exported-interfaces.
(parse_command_line): Parse the new options
(compare_task::perform, self_compare_task::perform): Pass the
value of opts.exported_interfaces_only onto the environment.
(compare_prepared_linux_kernel_packages): Likewise.
* tools/kmidiff.cc(options::exported_interfaces_only): Define new
data member.
(display_usage): Add new help strings for
--exported-interfaces-only and --allow-non-exported-interfaces.
(parse_command_line): Parse the new options
(main): Pass the value of opts.exported_interfaces_only onto the
environment.
Signed-off-by: Dodji Seketeli <[email protected]>
Caching the result of IR comparison cannot happen on IR nodes that are
being compared in the context of type canonicalization if one of the
nodes is the target of canonical type propagation. This is especially
true if the recursive IR node which comparison "temporarily" did yield
true (to avoid an infinite loop) at least until the comparison of the
rest of the sub-tree of the recursive type is done. In that case, we
should not cache the result the comparison as it might change later.
The patch adds a way to track recursive types so that we know when not
to cache their comparison result.
As we now have a facility to track recursive types during canonical
type propagation (rather than just types that depend on recursive
types) we can be a bit more precise when confirming or cancelling
types that have been subject to canonical type propagation.
Also the patch cleans up the detection of comparison cycle to make it
more typesafe so that the macro
RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED can be used for class_decl
and union_decl, not just class_or_union. It makes the code more
readable/maintainable is the equals overload for class_decl and
union_decl all have their proper call to
RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED. The same is true for
mark_types_as_being_compared.
Doing that cleans up the detection of recursive types as well as the
types that depends on those recursive types.
* src/abg-ir-priv.h (environment::priv::recursive_types_): Define
new data member.
(environment::priv::cache_type_comparison_result): Cache results
only non-recursive and not dependant types, or when the result is
"false". In all these cases, the result, once cached, will not
change.
(environment::priv::mark_dependant_types_compared_until): Mark
types as recursive at the same time others are marked as
dependant.
(environment::priv::{is_recursive_type, set_is_not_recursive}):
Define new member functions.
(environment::priv::{confirm_ct_propagation,
cancel_ct_propagation}): Confirming canonical type propagation
should happen for recursive types as well as their dependant
types.
* src/abg-ir.cc (return_comparison_result): Keep up with the
book-keeping at all time when type canonicalization process is
on-going. Whenever we expect types that depends on recursive
types, expect recursive types too, obviously.
(type_base::get_canonical_type_for): Do not erase the comparison
result cache between the canonicalization of two different types.
(is_comparison_cycle_detected)
(mark_types_as_being_compared, unmark_types_as_being_compared):
Define new overloads for class_decl;
(equals): In the overloads for class_decl and union_decl, use
RETURN_COMPARISON_RESULT and mark_types_as_being_compared without
casting it to class_or_union.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
Adjust.
* tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.txt:
Adjust.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Adjust.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt:
Adjust.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt:
Adjust.
Signed-off-by: Dodji Seketeli <[email protected]>
Now that DIE and IR canonicalizing has progressed in precision,
especially due to fixing the canonical type propagation it seems like
we can go back to canonicalizing typedefs again. This makes the
writer not needing to handle non-canonicalized types because only a
short number and types of types are still non-canonicalized.
Then I ran the test suite and hell broke lose. Things would work on a
given platform but won't on others, etc.
So a great deal of the patch actually fixes issues uncovered by the
fact that we are back to canonicalizing typedefs.
The symptoms of many of the issues are related to emitting abixml in a
stable manner across toolchains. But in reality those issues are
real, deeper ones.
This patch is thus ALSO an attempt at fixing all the issues at once
because they can't be separated. Either the abixml is stable and
"make check" passes on all platforms, or it is not and testing fails
on some platforms.
I believe that at the core of the problems lies the observation that a
given typedef (of a given name) could appear several times (the number
of times not being quite constant) in a given class or namespace.
That would lead to some instabilities in the sorting and ordering of
type-ids depending on the toolchain being used, for instance.
To fix this add_or_update_class_type in the DWARF reader is taught to
avoid adding a typedef to a class if a member type of the same name
already exists at that class scope. This handles the fact that a
given class can be built piece-wise in the DWARF. Also, when handling
a typedef type, build_ir_node_from_die is taught to avoid creating a
typedef IR for a given scope (namespace, union or class) if said scope
already contains a type of the same name. To be able to do that, the
handling of member types is moved from the ir::class_or_union type to
the ir::scope_decl type. That way, all scopes (not just classes or
unions) can handle looking up for the (member) types they contain.
Another issue that was uncovered is that when emitting decl-only class
named A (in a namespace for instance) there can be some instability as
to which decl-only class A is emitted. This is due to the fact that
libabigail considers all decl-only classes named A to be equal. The
problem however is that a decl-only class A might have member types.
And a decl-only A declared in a translation unit somewhere might have
some member types that are different from the same decl-only A
declared in another translation unit. This doesn't necessarily
violate the ODR, but rather, can be the result of compiler
optimization. For instance, in a given translation unit, only a given
member type of the decl-only A is used by the code, whereas in another
one, another member type of the same decl-only A is used. So only
partial views of A are emitted in the translation unit depending on
what is used. Anyway, to handle that case, when comes the time to
emit the decl-only A in the ABIXML writer, write_class_decl now emits
all the member types of all the instances A known to the system. This
hopefully removes the instability I was seeing. To do that,
maybe_update_types_lookup_map<class_decl> has been taught to also keep
track of decl-only classes. A new lookup_decl_only_class_types has
been defined to gather all the instances of a given decl-only class
known to the system.
Last but not least, the topological type sorting facilities carried by
the types {decl,type}_topo_comp has been fixed to ensure a more stable
sorting and also to ensure that "abilint foo.abi" emits the decls and
types in the same order as the one defined in foo.abi. This fixes
another abixml instability I was seeing in the test suite across
different toolchains. While doing that, I noticed that the ABIXML
reader was forgetting to properly mark the corpus as originating from
ABIXML. So I fixed that too.
* include/abg-fwd.h (lookup_decl_only_class_types): Declare new
function.
* src/abg-ir-priv.h (class_or_union::priv::member_types_): Move
this data member into scope_decl::priv.
(class_or_union::priv::priv): Do not take member types.
* include/abg-ir.h (sort_type): Likewise.
(namespaces_type): Add new typedefs.
(scope_decl::insert_member_decl): Make this be non-virtual.
(scope_decl::{insert_member_type, add_member_type,
remove_member_type, get_member_types, get_sorted_member_types,
find_member_type}): Move these methods here from ...
(class_or_union::{insert_member_type, add_member_type,
remove_member_type, get_member_types, get_sorted_member_types,
find_member_type}): ... here.
(class_or_union::insert_member_decl): Make this be non-virtual.
(class_decl::insert_member_decl): Make this be non-virtual.
(class_or_union::get_sorted_member_types): Declare ...
* src/abg-dwarf-reader.cc (add_or_update_class_type): Do not add a
member type to the class type being built if it already exists
there.
(build_ir_node_from_die): If a scope (namespace, union or class)
already has a typedef, do not create a new one to add it there
again.
* src/abg-ir.cc (equals): In the overload for typedefs take into
account the name of typedefs again.
(lookup_decl_only_class_types): Define new function.
(compare_using_locations): Define new static function that has
been factorized out of ...
(decl_topo_comp::operator()): ... here. Also, two decls originate
from an abixml corpus, compare them only using their artificial
locations, meaning make them keep the same order as the order of
the original abixml file.
(type_top_comp::operator()): Likewise.
(sort_types): Define new function.
(scope_decl::priv::member_types_): Move this here from
class_or_union::priv.
(scope_decl::priv::sorted_member_types_): Define new data member.
(scope_decl::{get_member_types, find_member_type,
insert_member_type, add_member_type, remove_member_type}): Move
these methods here from class_or_union.
(scope_decl::get_sorted_member_types): Define new method.
(is_non_canonicalized_type): Canonicalize typedefs.
(lookup_type_in_map): Allow this to look through decl-only types
to get their definition. Otherwise, if only a decl-only was
found, return it.
(maybe_update_types_lookup_map<class_decl>): Allow adding
decl-only class types to the map of classes held per TU and per
corpus.
(class_or_union::{class_or_union, add_member_decl,
has_no_member}): Adjust.
(class_or_union::{insert_member_type, add_member_type,
add_member_type, remove_member_type, get_member_types,
get_sorted_member_types, find_member_type}): Move these methods to
scope_decl.
({class_decl, class_or_union}::insert_member_decl): Remove the
"before" parameter as it was not used anymore due to the re-use of
the scope_decl::add_member that handles member types.
* src/abg-reader.cc (read_corpus_group_from_input)
(create_native_xml_read_context): Set the corpus origin.
* src/abg-writer.cc (write_context::{m_nc_type_id_map,
m_emitted_non_canonicalized_type_set,
m_referenced_non_canonicalized_types_set}): Remove these maps that
hold stuff for non-canonicalized types.
(write_context::{type_has_existing_id, get_id_for_type,
clear_type_id_map, has_non_emitted_referenced_types,
record_type_as_referenced, type_is_referenced,
record_type_as_emitted, type_is_emitted, clear_referenced_types,
write_referenced_types, write_canonical_type_ids}): Adjust these
as the maps for non-canonicalized types are gone.
(write_context::{get_referenced_non_canonicalized_types,
get_emitted_non_canonicalized_type_set}): Remove these methods.
(write_decl_in_scope)
(write_class_decl_opening_tag, write_union_decl_opening_tag)
(write_union_decl): Adjust comments.
(write_class_decl): Sort member types. Also, When emitting a
decl-only class, get all of the decl-only classes of the same name
declared in several other TUs and emit all their member types into
this decl-only class just once. This reduces redundancies and
sorting instabilities because for libabigail, all decl-only
classes of a given name are equal, even if they have member types.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
* tests/data/test-annotate/libtest23.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-annotate/test-anonymous-members-0.o.abi:
Likewise.
* tests/data/test-annotate/test0.abi: Likewise.
* tests/data/test-annotate/test1.abi: Likewise.
* tests/data/test-annotate/test13-pr18894.so.abi: Likewise.
* tests/data/test-annotate/test14-pr18893.so.abi: Likewise.
* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-annotate/test2.so.abi: Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt:
Likewise.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt:
Likewise.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt:
Likewise.
* tests/data/test-diff-filter/test41-report-0.txt: Likewise.
* tests/data/test-diff-pkg/PR24690/PR24690-report-0.txt: Likewise.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Likewise.
* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Likewise.
* tests/data/test-read-dwarf/PR26261/PR26261-exe.abi: Likewise.
* tests/data/test-read-dwarf/PR27700/test-PR27700.abi: Likewise.
* tests/data/test-read-dwarf/libtest23.so.abi: Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi:
Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-read-dwarf/test-PR26568-1.o.abi: Likewise.
* tests/data/test-read-dwarf/test-PR26568-2.o.abi: Likewise.
* tests/data/test-read-dwarf/test-libaaudio.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/test0.abi: Likewise.
* tests/data/test-read-dwarf/test0.hash.abi: Likewise.
* tests/data/test-read-dwarf/test1.abi: Likewise.
* tests/data/test-read-dwarf/test1.hash.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-read-dwarf/test2.so.abi: Likewise.
* tests/data/test-read-dwarf/test2.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
* tests/data/test-read-write/test18.xml: Likewise.
* tests/data/test-read-write/test28-without-std-fns-ref.xml:
Likewise.
* tests/data/test-read-write/test28-without-std-vars-ref.xml:
Likewise.
Signed-off-by: Dodji Seketeli <[email protected]>
In the [supress_type] directive, this patch adds support for two new
properties:
* has_data_data_member = {foo, bar, blah}
Suppresses change reports involving a type which has data members
with names specified by the value of this property.
* has_data_member_regexp = some-regexp
Suppresses change reports involving a type which has data members
with names specified by the regular expression given as a value of
this property.
* include/abg-fwd.h (string_set_type): Define new typedef.
* src/abg-suppression-priv.h
* include/abg-suppression.h
(type_suppression::{get,set}_potential_data_member_names[_regex_str]):
Declare new data member.
(type_suppression::priv::{potential_data_members_,
potential_data_members_regex_str_,
potential_data_members_regex_}): Define new data members.
(type_suppression::priv::{get,set}_potential_data_member_names_regex):
Define new member functions.
* src/abg-suppression.cc
(type_suppression::{get,set}_potential_data_member_names): Define new
member functions.
(type_suppression::{get,set}_potential_data_member_names_regex_str):
Likewise.
(type_suppression::suppresses_diff): Implement suppression using
the new "has_data_member" and "has_data_member_regexp" properties.
(read_type_suppression): Support parsing the new "has_data_member"
and "has_data_member_regexp" properties of the type suppression
directive.
* tests/data/test-diff-suppr/has-data-member-[1-7].suppr: New
suppression specifications for test purposes.
* tests/data/test-diff-suppr/test-has-data-member-output-{1,2}.txt:
New reference test outputs.
* tests/data/test-diff-suppr/test-has-data-member-v{0,1}.cc:
Source code of new input binary tests.
* tests/data/test-diff-suppr/test-has-data-member-v{0,1}.o: New
binary test inputs.
* tests/data/Makefile.am: Add the test inputs below to source
distribution.
* tests/test-diff-suppr.cc (in_out_specs): Add the new test inputs
above to this test harness.
Signed-off-by: Dodji Seketeli <[email protected]>
In preparation of subsequent changes, this patch factorizes a function is_data_member_offset_in_range() out of type_suppression::suppression(). This is useful to determine if a data member offset is within an "offset range" expressed by the type_suppression::insertion_range type. This function is useful to implement the offset_of_first_data_member_regexp and offset_of_last_data_member_regexp properties to come in subsequent patches. Please note that is_data_member_offset_in_range works on data members of unions and classes, not just on classes like what the original code of inside type_suppression::suppresses_diff was doing. This patch should not have any functional impact on the code. * include/abg-fwd.h (get_last_data_member) (get_next_data_member_offset): Declare functions. * src/abg-ir.cc (get_next_data_member): Add an overload for class_or_union and write the overload for class_or_union_sptr in term of the former. (get_last_data_member): Add overloads form class_or_union& and class_or_union*. Write the overload for class_or_union_sptr in terms of the one for class_or_union*. (get_next_data_member_offset): Add an overload for class_or_union* and write the overload for class_or_union_sptr in terms of the former. * include/abg-suppression.h (type_suppression::insertion_range::eval_boundary): Take a class_or_union* for the context, as opposed to a class_decl_sptr. This makes this static function work for unions as well. (is_data_member_offset_in_range): Declare new function. * src/abg-suppression.cc (type_suppression::suppression_diff): Factorize ... (is_data_member_offset_in_range): ... this function out. (type_suppression::insertion_range::eval_boundary): Adjust this to make it take a class_or_union* rather than a class_decl_sptr. Signed-off-by: Dodji Seketeli <[email protected]>
The "has_data_member_inserted_between" and
"has_data_members_inserted_between" properties of the [suppress_type]
directive allows the suppression of type changes when a data member is
inserted in a given range. It turns out that suppressing type changes
that incur a change in the size of the type might not be what the user
wants by default, because the type size in itself might actually be an
incompatible ABI change that would then fly under the radar because of
this suppression specification.
An arguably better default behavior in this case would be to NOT
suppress the type change if the data member insertion does incur a
change in the size of the type.
But then, there would be cases where the user would really want to
suppress the type change due to data member insertion in a given range
even if it incurs a change in the type size. This is where this patch
enters into play.
The patch introduces the "has_size_change" property of the
[suppress_type] directive. In the presence of
"has_data_members_inserted_between" or
"has_data_member_inserted_between" properties, if the
"has_size_change" property is set to "yes", then the type change would
be suppressed if data members are inserted in the given range even if
the insertion incurs a type size change.
Otherwise, with this patch, in the absence of the "has_size_change"
property, the "has_data_member_inserted_between" and
"has_data_members_inserted_between" properties won't trigger the type
change suppression if the data member insertion incurs a type size
change.
* doc/manuals/libabigail-concepts.rst: Document the new
has_size_change property.
* include/abg-suppression.h
(type_suppression::{g,s}et_has_size_change): Declare new accessors.
* src/abg-suppression-priv.h
(type_suppression::priv::has_size_change_): Define new data
member.
(type_suppression::priv::priv): Initialize the new data member.
* src/abg-suppression.cc
(type_suppression::{g,s}et_has_size_change): Define new accessors.
(type_suppression::suppresses_diff): Make the
has_data_member_inserted_* clauses have effect only if the class
size hasn't changed, unless the class has as the "has_size_change"
property. Also, allow members to be deleted in the right
insertion range if the resulting size stays the same or if the
has_size_change property is present. This allows some custom
behaviours where "padding" data members would be removed while
some new data members would be added, resulting in a type which
size would not change.
(read_type_suppression): Support parsing the "has_size_change"
property.
* tests/data/test-diff-suppr/test11-add-data-member-0.1.suppr: New
test suppression specification.
* tests/data/test-diff-suppr/test11-add-data-member-1.1.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-2.1.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-3.1.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-4.1.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-report-1.1.txt:
Likewise.
* tests/data/test-diff-suppr/test12-add-data-member-0.1.suppr:
Likewise.
* tests/data/test-diff-suppr/test12-add-data-member-report-1.1.txt:
New test reference output.
* tests/data/test-diff-suppr/test13-suppr-through-pointer-0.1.suppr:
New test suppression specification.
* tests/data/test-diff-suppr/test13-suppr-through-pointer-report-1.1.txt:
New test reference output.
* tests/data/test-diff-suppr/test35-leaf-report-0.1.txt: Likewise.
* tests/data/test-diff-suppr/test35-leaf.1.suppr: New test
suppression specification.
* tests/data/Makefile.am: Add the new testing material to source
distribution.
* tests/data/test-diff-suppr/test11-add-data-member-1.suppr: Add
the has_size_change property to explicitly allow suppressing type
changes involving data member insertion even when the type size
changes.
* tests/data/test-diff-suppr/test11-add-data-member-0.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-2.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-3.suppr:
Likewise.
* tests/data/test-diff-suppr/test11-add-data-member-4.suppr:
Likewise.
* tests/data/test-diff-suppr/test12-add-data-member-0.suppr:
Likewise.
* tests/data/test-diff-suppr/test13-suppr-through-pointer-0.suppr:
Likewise.
* tests/data/test-diff-suppr/test35-leaf.suppr: Likewise.
* tests/test-diff-suppr.cc (in_out_specs): Add the new test input
to the test harness.
Signed-off-by: Dodji Seketeli <[email protected]>
… selectors
This patch adds support for two data member offset selector
expressions for properties of the [suppress_type] directive:
offset_of_first_data_member_regexp() and offset_of_last_data_member_regexp().
These function-call expressions take a regular expression argument and
evaluate to the offset of the first (resp. last) data member matching
the regular expression argument.
An example of their use would be be:
[suppress_type]
type_kind = struct
has_data_member_inserted_between =
{
offset_of_first_data_member_regexp(^__special_padding_space),
offset_of_last_data_member_regexp(^__special_padding_space)
}
This would be useful to suppress change reports involving a struct
which has "padding" data members added on-purpose like:
struct S
{
int member0;
char member1;
unsigned __special_padding_space1;
unsigned __special_padding_space2;
unsigned __special_padding_space3;
};
* doc/manuals/libabigail-concepts.rst: Document the new
properties.
* include/abg-fwd.h: Forward declare comparison::{diff_context,
diff_context_sptr, diff_context_wptr, diff, diff_wptr} and
regex::regex_t_sptr.
(find_first_data_member_matching_regexp)
(find_last_data_member_matching_regexp): Declare new functions.
* include/abg-suppression.h: Inject std::{string, shared_ptr,
vector} and comparison::{diff, diff_context_sptr} into the suppr
namespace. Remove the "abg-comparison.h" header.
* src/abg-elf-helpers.cc: Include sstream.
* src/abg-ir.cc (find_first_data_member_matching_regexp)
(find_last_data_member_matching_regexp): Define new functions.
* src/abg-suppression.cc
(type_suppression::insertion_range::eval_boundary): Support
evaluating "offset_of_first_data_member_regexp" and
"offset_of_first_data_member_regexp".
* src/abg-ctf-reader.cc: Include sstream.
* tests/data/test-diff-suppr/test-has-data-member-inserted-between-1-report-[1-4].txt:
New test reference outputs.
* tests/data/test-diff-suppr/test-has-data-member-inserted-between-1-v[0-4].c:
Source code of new test input.
* tests/data/test-diff-suppr/test-has-data-member-inserted-between-1-v[0-4].o:
New binary test input.
* tests/data/test-diff-suppr/test-has-data-member-inserted-between-1.suppr:
New suppression specification.
* tests/data/Makefile.am: Add the new test input files to source
distribution.
* tests/test-diff-suppr.cc (in_out_specs): Add the new test input
to this test harness.
Signed-off-by: Dodji Seketeli <[email protected]>
This patch adds support for a new 'allow_type' suppression directive.
It suppresses all the changes that are NOT matched by the directive.
In other words, this directive determines the set of type changes that
are NOT suppressed. Any other change is suppressed. This thus called
a "negated suppression directive".
The way these negated suppression directives interact with the direct
suppression directives that already exist is the following.
The suppression evaluation pass visits every single diff node
(carrying a type change) of the diff graph. Negated suppressions are
evaluated first, in order of occurrence.
There are thus, two alternatives:
1/ At least one negated suppression matches the current diff node.
or
2/ No negated suppression matches the current diff node.
In case of 1/ then direct suppression specifications are
considered. There are two alternatives:
1.1/ At least one direct suppression matches the current diff node.
The diff node is suppressed: categorized as being in the
SUPPRESSED_CATEGORY category)
or
1.2/ No direct suppression matches the current diff node.
The diff node is not suppressed: categorized as being in the
HAS_ALLOWED_CHANGE_CATEGORY category.
In case of 2/ then direct suppression specifications are
considered. There are two alternatives:
2.1 At least one direct suppression matches the current diff node.
The diff node is categorized as being in the
SUPPRESSED_CATEGORY category, just like in 1.1.
2.2 No direct suppression matches the current diff node.
The diff node is not suppressed and not categorized.
As a result of the category propagation pass, a node which has a
parent node categorized as HAS_ALLOWED_CHANGE_CATEGORY is itself
categorized as HAS_PARENT_WITH_ALLOWED_CHANGE_CATEGORY. A node which
has a descendant categorized as HAS_ALLOWED_CHANGE_CATEGORY will
itself be categorized as HAS_DESCENDANT_WITH_ALLOWED_CHANGE_CATEGORY.
Nodes that are categorized as HAS_ALLOWED_CHANGE_CATEGORY,
HAS_DESCENDANT_WITH_ALLOWED_CHANGE_CATEGORY and
HAS_PARENT_WITH_ALLOWED_CHANGE_CATEGORY are not suppressed by the
reporting passes. This is needed for the reporting passes to emit the
impact sub-tree up to the diff node which carry the change that was
actually categorized as HAS_ALLOWED_CHANGE_CATEGORY.
* include/abg-comparison.h: Include abg-suppression.h
(diff, diff_context, diff_sptr, diff_context_sptr): Remove these
forward decls from here.
(enum diff_category::{HAS_ALLOWED_CHANGE_CATEGORY,
HAS_DESCENDANT_WITH_ALLOWED_CHANGE_CATEGORY,
HAS_PARENT_WITH_ALLOWED_CHANGE_CATEGORY}): Add new enumerators.
(enum diff_category::EVERYTHING_CATEGORY): Update enumerator.
(diff_context::{negated_suppressions, direct_suppressions}): Declare
new member functions.
(diff_context::suppressions): Add overload.
(diff::{is_filtered_out_without_looking_at_allowed_changes,
is_allowed_by_specific_negated_suppression,
has_descendant_allowed_by_specific_negated_suppression,
has_parent_allowed_by_specific_negated_suppression}): Declare new
member functions.
* include/abg-suppression.h (class negated_suppression_base, class
negated_type_suppression): Declare new classes.
(negated_suppression_sptr, negated_suppression_type): Define new
typedefs.
(is_negated_suppression): Declare new functions.
* src/abg-suppression.cc
(negated_suppression_base::{negated_suppression_base,
~negated_suppression_base}): Define member functions.
(negated_type_suppression::{negated_type_suppression,
suppresses_diff, ~negated_type_suppression}): Likewise.
(is_negated_suppression): Define functions.
(read_type_suppression): Allow parsing the "allow_type" directive
and instantiate a negated_type_suppression.
* src/abg-comparison-priv.h
(diff_context::priv::{negated_suppression_type_,
direct_suppressions}): Define new data members.
(diff::priv::is_filtered_out): A node categorized as
HAS_DESCENDANT_ALLOWED_BY_SPECIFIC_NEGATED_SUPPRESSION,
HAS_PARENT_ALLOWED_BY_SPECIFIC_NEGATED_SUPPRESSION and
HAS_ALLOWED_CHANGE_CATEGORY is not filtered out.
* src/abg-comparison.cc (diff_context::suppressions): Add a
non-const overload.
(diff_context::{negated,direct}_suppressions): Define new member
function.
(diff_context::add_suppression): Invalidate the cache data members
diff_context::priv::{negated,direct}_suppressions_.
(diff::is_filtered_out): A node categorized as
HAS_DESCENDANT_ALLOWED_BY_SPECIFIC_NEGATED_SUPPRESSION,
HAS_PARENT_ALLOWED_BY_SPECIFIC_NEGATED_SUPPRESSION and
HAS_ALLOWED_CHANGE_CATEGORY is not filtered out.
(diff::is_filtered_out_without_looking_at_allowed_changes): Define
new member function.
(diff::is_suppressed): If there is at least one negated
suppression that match the diff node, then it's not suppressed,
unless it's matched by a direct suppression.
(diff::{is_allowed_by_specific_negated_suppression,
has_descendant_allowed_by_specific_negated_suppression,
has_parent_allowed_by_specific_negated_suppression}): Define new
member functions.
(operator<<(ostream& o, diff_category c)): Serialize
HAS_{DESCENDANT_WITH,PARENT_WITH}_ALLOWED_CHANGE_CATEGORY
enumerators.
(category_propagation_visitor::visit_end): Do not propagate
HAS_ALLOWED_CHANGE_CATEGORY,
HAS_DESCENDANT_WITH_ALLOWED_CHANGE_CATEGORY and
HAS_PARENT_WITH_ALLOWED_CHANGE_CATEGORY categories.
(suppression_categorization_visitor::visit_begin): Categorize a
node that is not suppressed by a direct suppression and is
suppressed by a negated one as
HAS_ALLOWED_CHANGE_CATEGORY. Propagate it to descendant nodes as
HAS_PARENT_WITH_ALLOWED_CHANGE_CATEGORY ...
(suppression_categorization_visitor::visit_end): ... and to parent
node as HAS_DESCENDANT_WITH_ALLOWED_CHANGE_CATEGORY.
* src/abg-default-reporter.cc (default::reporter): In the overload
for typedef_diff, qualified_type_diff, reference_diff,
fn_parm_diff, function_type_diff, array_diff, base_diff,
function_decl_diff, report local changes only
on node that are not filtered out wrt allowed changed.
* tests/data/test-abidiff-exit/test-allow-type-array-suppr.txt:
New test input.
* tests/data/test-abidiff-exit/test-allow-type-array-v0--v1-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-array-v0--v2-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-array-v0--v3-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-array-v{0,1,2,3}.c:
Source code of new binary test inputs.
* tests/data/test-abidiff-exit/test-allow-type-array-v{0,1,2,3}.o:
New binary test inputs.
* tests/data/test-abidiff-exit/test-allow-type-region-suppr.txt:
New test input.
* tests/data/test-abidiff-exit/test-allow-type-region-v0--v1-report-{1,2}.txt:
* tests/data/test-abidiff-exit/test-allow-type-region-v0--v2-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-region-v0--v3-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-region-v0--v4-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-region-v0--v5-report-{1,2}.txt:
Likewise.
* tests/data/test-abidiff-exit/test-allow-type-region-v{0,1,2,3,4,5}.c:
Source code of new binary test input.
* tests/data/test-abidiff-exit/test-allow-type-region-v{0,1,2,3,4,5}.o:
New binary test inputs.
* tests/data/test-abidiff-exit/test-allow-type-suppr{1,2}.txt: New
test inputs.
* tests/data/Makefile.am: Add the new testing files above to
source distribution.
Signed-off-by: Dodji Seketeli <[email protected]>
* include/abg-suppression.h (class type_suppression::insertion_range::end): Fix indentation. * src/abg-default-reporter.cc (default_reporter::report): Fix indentation in the overload for corpus_diff. * src/abg-suppression-priv.h (type_suppression::priv::source_locations_to_keep_): Fix alignment. * src/abg-suppression.cc (read_type_suppression): Fix alignment of comment. Signed-off-by: Dodji Seketeli <[email protected]>
While looking at something else I felt the need for having
"abidiff --verbose" emit more timing information. I have thus added
a lot more logging around.
* include/abg-comparison.h ({diff, corpus_diff,
diff_context}::do_log): Declare member functions.
* include/abg-corpus.h (corpus::do_log): Likewise.
* src/abg-comparison-priv.h (diff_context::priv::do_log_): Add new
data member.
(diff_context::priv::priv): Initialize the new data member.
* src/abg-comparison.cc ({diff, corpus_diff,
diff_context}::do_log): Define member functions.
(diff_context::maybe_apply_filters): Add timing logs to applying
filters and propagating categories.
(corpus_diff::priv::apply_filters_and_compute_diff_stats): Add
timing logs to applying and propagating filters to changed
functions, variables, unreachable & leaf type changes,
suppressions application.
* src/abg-corpus-priv.h (corpus::priv::do_log): Add new data
member.
(corpus::priv::priv): Initialize it.
* src/abg-corpus.cc (corpus::do_log): Define member functions.
* src/abg-reader.cc (reader::do_log): Likewise.
(reader::read_corpus): Add timing log around the invocation of
perform_late_type_canonicalizing.
* tools/abidiff.cc (set_diff_context_from_opts): Set logging.
(main): Add timing logging for diff computing, changes analysis &
report generation.
Signed-off-by: Dodji Seketeli <[email protected]>
Up until now, a kernel whitelist was expected to be a ini file with a section having a name ending with the word "whitelist". Nowadays, they are called "stablelist", so the name of the section ends up with "stablelist". This patch makes gen_suppr_spec_from_kernel_abi_whitelists support that. * src/abg-tools-utils.cc (gen_suppr_spec_from_kernel_abi_whitelists): Support section name that ends with the word 'stablelist'. Signed-off-by: Dodji Seketeli <[email protected]>
When applying a filter to a corpus_diff node, visit each diff node only once. This can have some serious performance impact when there are a lot of diff nodes to visit. * src/abg-comp-filter.cc (apply_filter): In the overload for corpus_diff, visit each diff node only once. Signed-off-by: Dodji Seketeli <[email protected]>
This patch allows to avoid applying filters on interface diff node
sub-graphs because those filters are useful for interface impact
analysis only, which is not needed in the leaf-node report, for
instance. When using the leaf-node report, this capability speeds up
corpus_diff::apply_filters_and_suppressions_before_reporting, hence
the functions like corpus_diff::{has_incompatible_changes,
has_net_subtype_changes} are sped up too.
That patch thus adds a --no-change-categorization option to abidiff to
avoid doing that change categorization (A.K.A applying filters).
* doc/manuals/abidiff.rst: Document the new
--no-change-categorization option.
* doc/manuals/kmidiff.rst: Likewise.
* include/abg-comparison.h
(diff_context::perform_change_categorization): Declare new
accessor member functions.
* src/abg-comparison-priv.h
(diff_context::priv::perform_change_categorization_): Add new data
member.
(diff_context::priv::priv): Initialize the new data member.
* src/abg-comparison.cc
(diff_context::perform_change_categorization): Define new accessor
member functions.
(corpus_diff::priv::apply_filters_and_compute_diff_stats):
Don't apply filters on the diff node sub-graphs of interfaces when
the user requested "no change categorization".
* tools/abidiff.cc (options::perform_change_categorization): New
data member.
(options::options): Initialize the new data member.
(display_usage): Add a help string for the new
--no-change-categorization.
(parse_command_line): Parse the new --no-change-categorization
option.
(set_diff_context_from_opts): Set the option on the diff context
here.
* tools/kmidiff.cc(options::perform_change_categorization): New
data member.
(options::options): Initialize the new data member.
(display_usage): Add a help string for the new
--no-change-categorization.
(parse_command_line): Parse the new --no-change-categorization
option.
(set_diff_context_from_opts): Set the option on the diff context
here.
Signed-off-by: Dodji Seketeli <[email protected]>
When marking leaf nodes, if interface impact analysis is not required, then avoid visiting the same diff node twice when walking the diff graph. Allowing to visit the same diff node twice would be useful so that the sub-graph of each interface diff node would be walked in full to determine the impact of each leaf diff node on each interface. But if such impact analysis is not required, then we can just visit each diff graph node once and speed up things greatly in leaf node reporting mode. * src/abg-comparison.cc (corpus_diff::mark_leaf_diff_nodes): If impact analysis is not required, visit each node just once. Signed-off-by: Dodji Seketeli <[email protected]>
Categorizing a diff graph with a thousands of function diff nodes takes a lot of time. At that point, categorizing each node has harmful/harmless is showing up in the profile, particularly because has_var_type_cv_qual_change and to a lesser extend class_diff_has_harmless_odr_violation_change perform some structural comparison still, oops. This patch avoids doing that. On my machine, the categorizing of each node goes from around 130ms to 92 ms. * src/abg-comp-filter.cc (class_diff_has_harmless_odr_violation_change) (has_var_type_cv_qual_change): Avoid doing structural comparison here. Signed-off-by: Dodji Seketeli <[email protected]>
When abipkgdiff is invoked on a `kernel' package, compare_prepared_linux_kernel_packages fails to look for the `vmlinux' file from the debuginfo package, because of a thinko. Then, build_corpus_group_from_kernel_dist_under, also fails to find `vmlinux' from the debuginfo package because of another thinko. This patch fixes the two thinkos. * src/abg-tools-utils.cc (get_binary_paths_from_kernel_dist): Fix a thinko and really use the kernel_modules_root variable. Look for modules under kernel_modules_root and look for vmlinux (if necessary) under debug_info_root. Add comments. (compare_prepared_linux_kernel_packages): Fix another thinko. Now we do have the path to vmlinux, from debuginfo packages before getting into get_binary_paths_from_kernel_dist. Signed-off-by: Guillermo E. Martinez <[email protected]> Signed-off-by: Dodji Seketeli <[email protected]>
…nder * src/abg-tools-utils.cc (build_corpus_group_from_kernel_dist_under): Improve logging. Signed-off-by: Dodji Seketeli <[email protected]>
It seems the commit below (in libabigail 2.1) broke the cycle detection during union types comparison. commit 4bc513a Author: Dodji Seketeli <[email protected]> Date: Thu Sep 8 19:09:33 2022 +0200 Fix IR comparison result caching and canonical type propagation tracking This patch fixes that. * src/abg-ir.cc (CACHE_COMPARISON_RESULT_AND_RETURN): Define new macro. (equals): In the overload for unions, detect cycles right away. Also, do not use mark_types_as_being_compared and return_comparison_result as that would indirectly call environment::priv::unmark_as_being_compared one too many, thus breaking the cycle detection machinery. Rather, just cache the result of comparing the type as a class_or_union and return, using CACHE_COMPARISON_RESULT_AND_RETURN. Signed-off-by: Dodji Seketeli <[email protected]>
When canceling canonical DIE propagation, we wrongly assume that the pair of DIEs being compared yield a COMPARE_RESULT_UNKNOWN result. The reality is that it can also yield a COMPARE_RESULT_DIFFERENT result, especially when we are looking at the first sub-type that compares different and that triggered the canonical DIE propagation canceling to begin with. This can be reproduced by the command: $ fedabipkgdiff --self-compare -a --from fc37 xorg-x11-server-Xvfb Fixed thus. * src/abg-dwarf-reader.cc (offset_pairs_stack_type::cancel_canonical_propagated_type): The result of comparing the canonical-propagated types being canceled is either COMPARISON_RESULT_UNKNOWN or COMPARISON_RESULT_DIFFERENT. Also, do not forget to update the cached value for the comparison of the depend types too. Signed-off-by: Dodji Seketeli <[email protected]>
…calization When looking at something else, I configured the package with --enable-debug-type-canonicalization and surprise, there were some compilation errors. Fixed thus. * src/abg-dwarf-reader.cc (reader::initialize): Use env(). rather than environment->. * tools/abidiff.cc (options::options): Initialize do_debug_type_canonicalization() before use_btf. Signed-off-by: Dodji Seketeli <[email protected]>
…l functions
When an ELF symbol alias designates two different functions,
Libabigail can be confused as to which function to consider.
This confusion indirectly leads to showing spurious changes in the
return type of some functions when an ELF symbol designates more than
one function.
In other words, when an ELF symbol designates two (or more) functions,
the comparison engine needs a way to tell the two functions apart. It
needs an other way to identify the functions.
This patch fixes the confusion by using the pretty representation of
the functions in those cases.
Please note that to replicate the issue reported in this bug, here is
the command I used:
$ fedabipkgdiff --debug --self-compare -a --from fc37 smesh
* include/abg-corpus.h (corpus::lookup_functions): Return a set of
functions rather than a vector of functions where a function can
be present more than once. This allows to determine if a symbol
designates more than one function.
(corpus::exported_decls_builder::priv_): Make this public so that
some outside code can access it.
(corpus::exported_decls_builder::fn_id_maps_to_several_fns):
Declare new function.
(corpus::exported_decls_builder::maybe_add_fn_to_exported_fns):
Remove useless const here.
* include/abg-fwd.h (get_function_id_or_pretty_representation):
Declare new function.
* include/abg-ir.h
(elf_symbol::get_alias_with_default_symbol_version): Declare new
member function.
* src/abg-comparison.cc
(corpus_diff::priv::ensure_lookup_tables_populated): Use the new
get_function_id_or_pretty_representation rather than
function_decl::get_id() to identify a function.
* src/abg-corpus-priv.h (str_fn_ptr_set_map_type): Define this new
typedef of unordered_map<string, std::unordered_set<function_decl*> >.
(corpus::exported_decls_builder::priv::id_fns_map_): Change the
type of this to the new str_fn_ptr_set_map_type.
(corpus::exported_decls_builder::priv::{id_fns_map, fn_id_is_in_id_fns_map,
fn_is_in_fns, fn_is_in_id_fns_map}): Adjust to using a set of
functions rather than a vector.
(corpus::exported_decls_builder::fn_is_in_fns_by_repr): Define new
static function.
(corpus::exported_decls_builder::add_fn_to_exported): Remove
useless const.
* src/abg-corpus.cc
(corpus::exported_decls_builder::fn_id_maps_to_several_fns):
Define new function.
(corpus::exported_decls_builder::maybe_add_fn_to_exported_fns):
Remove useless const.
(corpus::lookup_functions): Return a set of functions rather than
a vector of functions where a function can be present more than
once. This allows to determine if a symbol designates more than
one function.
* src/abg-dwarf-reader.cc
(reader::symbol_already_belongs_to_a_function): Adjust.
* src/abg-fe-iface.cc (fe_iface::maybe_add_fn_to_exported_decls):
Adjust.
* src/abg-ir.cc (get_function_id_or_pretty_representation): Define
new function.
Signed-off-by: Dodji Seketeli <[email protected]>
…bipkgdiff Some packages like foo-utils.rpm can be associated with two debuginfo RPMs: One foo-utils-debuginfo.rpm and foo-debuginfo.rpm. This is because the foo-debuginfo.rpm contains debug info that has been factorized out of all the sub-packages of foo, foo-utils being one of those sub-packages. In those cases, fedabipkgdiff needs to provide foo-debuginfo.rpm and foo-utils-debuginfo.rpm to abipkgdiff so that it can find all the necessary debuginfo. This patch fixes fedabipkgdiff accordingly and adds some more logging to abipkgdiff to make it emit an explicit message for cases like this. * tools/abipkgdiff.cc (compare_to_self): Emit an error message when in verbose mode, for cases where we fail to find the alternate debug info. * tools/fedabipkgdiff (generate_comparison_halves): Always provide all associated debuginfo packages to abipkgdiff. Signed-off-by: Dodji Seketeli <[email protected]>
Some binaries can have a symbol table in which no symbol is actually defined and exported. That binary would thus have an empty ABI corpus. Interestingly, Libabigail's DWARF reader is emits an error when it encounters such binaries. This patch adds the support for those binaries. * src/abg-dwarf-reader.cc (reader::read_debug_info_into_corpus): Get out early also upon having an empty symbol table. * src/abg-elf-reader.cc (reader::read_corpus): Error out only if there is no symbol table for the binary. If an empty symbol table is found however, that is not an error. * tests/data/test-read-dwarf/PR29692-kdelibs3-libkjava.so.1.0.0: New binary test input. * tests/data/test-read-dwarf/PR29692-kdelibs3-libkjava.so.1.0.0.abi: New expected abixml file. * tests/data/Makefile.am: Add the new input test files to source distribution. * tests/test-read-dwarf.cc (in_out_specs): Add the new test inputs above to this test harness. Signed-off-by: Dodji Seketeli <[email protected]>
* tools/abipkgdiff.cc (compare_to_self): Fix a typo. Signed-off-by: Dodji Seketeli <[email protected]>
Now that we support empty symtabs test-symtab needs to be updated so
that it expects to have a corpus now, upon a binary with no exported
symbols, albeit an empty one.
* tests/test-symtab.cc (TEST_CASE("Symtab::Empty", "[symtab,
basic]")): Adjust.
(TEST_CASE("Symtab::NoDebugInfo", "[symtab, basic]")): Likewise.
Signed-off-by: Dodji Seketeli <[email protected]>
This was triggered by doing:
$ fedabipkgdiff --self-compare -a --from fc37 amg4psblas-mpich
* src/abg-dwarf-reader.cc (add_or_update_class_type): Make sure
the array is big enough.
Signed-off-by: Dodji Seketeli <[email protected]>
…ruct * include/abg-fwd.h (anonymous_data_member_to_class_or_union): Add a new overload. (anonymous_data_member_exists_in_class): Declare new function. * src/abg-dwarf-reader.cc (add_or_update_class_type): Use the new anonymous_data_member_exists_in_class function. * src/abg-ir.cc (anonymous_data_member_to_class_or_union): Define new function. * tests/data/test-read-dwarf/test-libandroid.so.abi: Adjust. Signed-off-by: Dodji Seketeli <[email protected]>
Note that this issue can be reproduced by doing:
$ fedabipkgdiff --self-compare -a --from fc37 dx
When a binary is a symlink to another one,
create_maps_of_package_content doesn't realize it and considers the
two binaries to be different.
Later, when comes time to self compare both binaries, their abixml
files (which are the same, by virtue of symlinks) might be written at
the same time in different threads, creating a race condition, leading
to corruption of the abixml.
This patch fixes this by teaching create_maps_of_package_content to
resolve symlinks so that it can detect when two files actually point
to the same file.
* tools/abipkgdiff.cc (create_maps_of_package_content): Resolve
symlinks when mapping binaries. Don't map a binary that has
already been seen.
Signed-off-by: Dodji Seketeli <[email protected]>
While running tests, I noticed that python was consuming a huge amount of CPU. Frank Eigler located the problem and pointed out that python was continiously polling for abipkgdiff's completion. For small packages, the time to completion can be less than a second but some packages can take literally hours to analyze. Having python spinning in such a tight loop is unnecessary. I added a small sleep to this loop with a bit of backoff. Vanessa Sochat helped with examples of how to fix the python code. * tools/fedabipkgdiff (abipkgdiff): add sleep while waiting for subprocess completion. Also, update copyright year notice to 2023. Signed-off-by: Ben Woodard <[email protected]> Signed-off-by: Dodji Seketeli <[email protected]>
Libabigail doesn't yet support generic range types like in Ada.
Ranges are supported as a kind of implementation detail of array
types. But then its use like in the construct below is not supported:
type My_Int is range 0 .. 5;
function My_Function return My_Int;
Here, the integer type "My_Int" can take the discrete values that go
from 0 to 5. It's represented in the DWARF debug info as being a
range type of length 6 and whose underlying type has a size of 8 bits.
This patch adds support for a range type to be (de-)serialized from
and to abixml, diffed, diff-analyzed and diff-reported.
The ABIXML version number has been bumped from 2.1 to 2.2 accordingly.
* configure.ac: Bump the abixml version to 2.2 from 2.1
* include/abg-comparison.h (diff_maps::get_subrange_diff_map):
Declare new member functions.
(class subrange_diff): Define new class.
(subrange_diff_sptr): Define new typedef.
(compute_diff): New overload for subrange_diff.
(is_subrange_diff): Declare new function.
* include/abg-ir.h (equals): Declare an overload for
subrange_type.
* include/abg-reporter.h (reporter_base::report): Declare an
overload for subrange_diff.
(default_reporter::report_underlying_changes_of_qualified_type):
Declare member function.
(leaf_reporter::report): Declare and overload for subrange_diff.
Declare new member function.
* include/abg-tools-utils.h
(get_anonymous_subrange_internal_name_prefix): Declare new function.
* src/abg-comparison-priv.h (struct subrange_diff::priv): Define
new type.
* src/abg-comparison.cc (diff_maps::priv::subrange_diff_map_):
Define data member.
(diff_maps::get_subrange_diff_map): Define member function.
(is_subrange_diff, compute_diff): Define new functions.
(compute_diff_for_types): Handle array_type::subrange_type types.
(subrange_diff::{subrange_diff, first_subrange, second_subrange,
get_pretty_representation, has_changes, has_local_changes, report,
chain_into_hierarchy}): Define member functions.
(diff_maps::insert_diff_node): Handle subrange diff nodes.
(corpus_diff::priv::count_leaf_type_changes): Count subranges diff
nodes.
* src/abg-default-reporter.cc (default_reporter::report): Define
an overload for subrange_diff.
* src/abg-ir.cc (has_generic_anonymous_internal_type_name):
Support subrange types.
* src/abg-leaf-reporter.cc (report_type_changes_from_diff_maps):
Report about subrange types.
(leaf_reporter::report): Define and overload for subrange_diff
nodes.
* src/abg-reader.cc (build_subrange_type): Add a boolean to add
the subrange type to the current scope.
(build_array_type_def): Adjust when calling build_subrange_type.
(build_type): Support building subrange types.
* src/abg-reporter-priv.cc (represent): Define a new overload for
the subrange_diff type.
* src/abg-reporter-priv.h (represent): Declare a new overload for
the subrange_diff type.
* src/abg-tools-utils.cc (ANONYMOUS_SUBRANGE_INTERNAL_NAME)
(ANONYMOUS_SUBRANGE_INTERNAL_NAME_LEN): Define new static const
variables.
(get_anonymous_subrange_internal_name_prefix): Define new
function.
* src/abg-writer.cc (write_array_subrange_type): Define new static
function.
(write_decl): Support emitting subrange_types.
* tests/data/test-abidiff-exit/ada-subrange/test1-ada-subrange/test1-ada-subrange-report-1.txt:
New reference output.
* tests/data/test-abidiff-exit/ada-subrange/test1-ada-subrange/test1-ada-subrange-report-2.txt:
Likewise.
* tests/data/test-abidiff-exit/ada-subrange/test1-ada-subrange/v0/test1.ad{b,s}:
Source code of the input binary below.
* tests/data/test-abidiff-exit/ada-subrange/test1-ada-subrange/v0/test1.o:
New input test.
* tests/data/test-abidiff-exit/ada-subrange/test1-ada-subrange/v1/test1.ad{b,s}:
Source code of the input binary below.
* tests/data/test-abidiff-exit/ada-subrange/test1-ada-subrange/v1/test1.o:
New input test.
* tests/data/Makefile.am: Add the new test files to source
distributions.
* tests/test-abidiff-exit.cc (in_out_specs): Add the new tests
input above to this test harness.
* tests/data/test-annotate/PR29443-missing-xx.o.annotated.abi:
Adjust.
* tests/data/test-annotate/libtest23.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-annotate/test-anonymous-members-0.o.abi:
Likewise.
* tests/data/test-annotate/test0.abi: Likewise.
* tests/data/test-annotate/test1.abi: Likewise.
* tests/data/test-annotate/test13-pr18894.so.abi: Likewise.
* tests/data/test-annotate/test14-pr18893.so.abi: Likewise.
* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-annotate/test2.so.abi: Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-annotate/test3.so.abi: Likewise.
* tests/data/test-annotate/test4.so.abi: Likewise.
* tests/data/test-annotate/test5.o.abi: Likewise.
* tests/data/test-annotate/test6.so.abi: Likewise.
* tests/data/test-annotate/test7.so.abi: Likewise.
* tests/data/test-annotate/test8-qualified-this-pointer.so.abi:
Likewise.
* tests/data/test-read-btf/test0.o.abi: Likewise.
* tests/data/test-read-btf/test1.o.abi: Likewise.
* tests/data/test-read-ctf/PR27700/test-PR27700.abi: Likewise.
* tests/data/test-read-ctf/test-PR26568-1.o.abi: Likewise.
* tests/data/test-read-ctf/test-PR26568-2.o.abi: Likewise.
* tests/data/test-read-ctf/test-alias.o.abi: Likewise.
* tests/data/test-read-ctf/test-ambiguous-struct-A.o.hash.abi:
Likewise.
* tests/data/test-read-ctf/test-ambiguous-struct-B.o.hash.abi:
Likewise.
* tests/data/test-read-ctf/test-anonymous-fields.o.abi: Likewise.
* tests/data/test-read-ctf/test-array-mdimension.abi: Likewise.
* tests/data/test-read-ctf/test-array-of-pointers.abi: Likewise.
* tests/data/test-read-ctf/test-array-size.abi: Likewise.
* tests/data/test-read-ctf/test-bitfield-enum.abi: Likewise.
* tests/data/test-read-ctf/test-bitfield.abi: Likewise.
* tests/data/test-read-ctf/test-callback.abi: Likewise.
* tests/data/test-read-ctf/test-callback2.abi: Likewise.
* tests/data/test-read-ctf/test-conflicting-type-syms-a.o.hash.abi:
Likewise.
* tests/data/test-read-ctf/test-conflicting-type-syms-b.o.hash.abi:
Likewise.
* tests/data/test-read-ctf/test-const-array.abi: Likewise.
* tests/data/test-read-ctf/test-dynamic-array.o.abi: Likewise.
* tests/data/test-read-ctf/test-enum-many.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-enum-symbol.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-enum.o.abi: Likewise.
* tests/data/test-read-ctf/test-fallback.abi: Likewise.
* tests/data/test-read-ctf/test-forward-type-decl.abi: Likewise.
* tests/data/test-read-ctf/test-functions-declaration.abi:
Likewise.
* tests/data/test-read-ctf/test-linux-module.abi: Likewise.
* tests/data/test-read-ctf/test-list-struct.abi: Likewise.
* tests/data/test-read-ctf/test0.abi: Likewise.
* tests/data/test-read-ctf/test0.hash.abi: Likewise.
* tests/data/test-read-ctf/test1.so.abi: Likewise.
* tests/data/test-read-ctf/test1.so.hash.abi: Likewise.
* tests/data/test-read-ctf/test2.so.abi: Likewise.
* tests/data/test-read-ctf/test2.so.hash.abi: Likewise.
* tests/data/test-read-ctf/test3.so.abi: Likewise.
* tests/data/test-read-ctf/test3.so.hash.abi: Likewise.
* tests/data/test-read-ctf/test4.so.abi: Likewise.
* tests/data/test-read-ctf/test4.so.hash.abi: Likewise.
* tests/data/test-read-ctf/test5.o.abi: Likewise.
* tests/data/test-read-ctf/test7.o.abi: Likewise.
* tests/data/test-read-ctf/test8.o.abi: Likewise.
* tests/data/test-read-ctf/test9.o.abi: Likewise.
* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/PR24378-fn-is-not-scope.abi:
Likewise.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Likewise.
* tests/data/test-read-dwarf/PR26261/PR26261-exe.abi: Likewise.
* tests/data/test-read-dwarf/PR27700/test-PR27700.abi: Likewise.
* tests/data/test-read-dwarf/PR28584/PR28584-smv.clang.o.abi:
Likewise.
* tests/data/test-read-dwarf/PR29443-missing-xx.o.abi: Likewise.
* tests/data/test-read-dwarf/PR29692-kdelibs3-libkjava.so.1.0.0.abi:
Likewise.
* tests/data/test-read-dwarf/libtest23.so.abi: Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi:
Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-read-dwarf/test-PR26568-1.o.abi: Likewise.
* tests/data/test-read-dwarf/test-PR26568-2.o.abi: Likewise.
* tests/data/test-read-dwarf/test-fallback.abi: Likewise.
* tests/data/test-read-dwarf/test-libaaudio.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/test-suppressed-alias.o.abi:
Likewise.
* tests/data/test-read-dwarf/test0.abi: Likewise.
* tests/data/test-read-dwarf/test0.hash.abi: Likewise.
* tests/data/test-read-dwarf/test1.abi: Likewise.
* tests/data/test-read-dwarf/test1.hash.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.
* tests/data/test-read-dwarf/test2.so.abi: Likewise.
* tests/data/test-read-dwarf/test2.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Likewise.
* tests/data/test-read-dwarf/test3-alias-1.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test3-alias-2.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test3-alias-3.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test3-alias-4.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test3.so.abi: Likewise.
* tests/data/test-read-dwarf/test3.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test4.so.abi: Likewise.
* tests/data/test-read-dwarf/test4.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test5.o.abi: Likewise.
* tests/data/test-read-dwarf/test5.o.hash.abi: Likewise.
* tests/data/test-read-dwarf/test6.so.abi: Likewise.
* tests/data/test-read-dwarf/test6.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test7.so.abi: Likewise.
* tests/data/test-read-dwarf/test7.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test8-qualified-this-pointer.so.abi: Likewise.
* tests/data/test-read-dwarf/test8-qualified-this-pointer.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
* tests/data/test-read-write/test-crc.xml: Likewise.
* tests/data/test-read-write/test26.xml: Likewise.
* tests/data/test-read-write/test27.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 <[email protected]>
After the support for Ada range types was added, it appeared that
redundancy propagation was not being correctly handled for range
types, especially when those are sub-types of a function parameter
type, like a const range.
This patch goes through the various problematic spots and addresses
the issues.
* src/abg-comparison.cc (redundancy_marking_visitor::visit_end):
Propagate redundancy category to function parameter diff nodes if
they don't carry any local non-type change.
* src/abg-default-reporter.cc
(default_reporter::report_underlying_changes_of_qualified_type):
Define new member function.
(default_reporter::report): In the qualified_type_diff overload,
use the new report_underlying_changes_of_qualified_type above.
* src/abg-ir.cc (types_have_similar_structure): If two arrays are
accessed indirectly and if they have size and dimension changes,
then the two arrays are considered having a similar structure.
Otherwise, if they are accessed directly, having size or dimension
change make them considered as having non similar structure. This
has an impact on if a change between two array types is considered
local or not.
* src/abg-leaf-reporter.cc (leaf_reporter::report): Local changes
to underlying types of a qualified type are considered local to
the qualified type. This change reflects that in the overload for
qualified type diff nodes. Otherwise, we won't report what would
otherwise be a leaf change to the a qualified type, just because
it's actually a leaf change to the underlying type of the
qualified type.
* tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-{1,2}.txt:
New reference output files.
* tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.ad{b,s}:
Source code for the new binary input below.
* tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o:
New binary input file.
* tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.ad{b,s}:
Source code for the new binary input below.
* tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o:
New binary input file.
* tests/data/Makefile.am: Add the new test input files above to
source distribution.
* tests/test-abidiff-exit.cc (in_out_specs): Add the new input
tests above to this test harness.
* tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt:
Adjust.
Signed-off-by: Dodji Seketeli <[email protected]>
When abipkgdiff is called with a debug info package that references an
alternate debug info file that is not found -- because debug info
package is missing from the command line -- the program aborts. This
is because the libabigail library is further invoked by the tool with
debuginfo in an inconsistent state (missing alternate debug info).
Note however that abipkgdiff only emits an explanatory message when
invoked with the --verbose option.
This patch teaches abipkgdiff to emit explanatory messages when an
alternate debug info file is not found. The message suggests that the
user adds the missing RPM package (which contains the alternate
missing debuginfo file) to the command line using the --d1/--d2
switches.
* src/abg-fe-iface.cc (status_to_diagnostic_string): Remove the
newline from the end of the returned diagnostic string.
* tools/abipkgdiff.cc (get_pretty_printed_list_of_packages)
(emit_alt_debug_info_not_found_error): Define new static
functions.
(compare, compare_to_self): Add an ostream& parameter as a
destination of diagnostic messages. If the
abigail::fe_iface::STATUS_ALT_DEBUG_INFO_NOT_FOUND bit is set in
the status code, emit the explanatory message to output stream
associated to the current comparison task. Remove the previous
handling of this case.
(compare_task::maybe_emit_pretty_error_message_to_output): Define
new member function to get the output stream associated to the
current task massage it and stick to result into the pretty output
string to be emitted to the user in the end, namely the
compare_task::pretty_output string data member.
({compares, self_compare}_task::perform): Adjust this to call the
new maybe_emit_pretty_error_message_to_output in case of error.
* tests/data/test-diff-pkg/libxfce4ui-devel-4.12.1-8.fc27.ppc64-self-report-0.txt:
Adjust.
* tests/data/test-diff-pkg/test-dbus-glib-0.80-3.fc12.x86_64-report-0.txt:
Likewise.
Signed-off-by: Dodji Seketeli <[email protected]>
Signed-off-by: Ben Woodard <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.