mirror of
git://sourceware.org/git/libabigail.git
synced 2024-12-17 15:34:34 +00:00
2417efb2b7
2099 Commits
Author | SHA1 | Message | Date | |
---|---|---|---|---|
Dodji Seketeli
|
2417efb2b7 |
dwarf-reader: Bug 26908 - don't crash on empty DW_TAG_partial_unit
Sometimes a DW_TAG_partial_unit (imported via a DW_TAG_imported_unit) has no children node. That means the the DW_TAG_partial_unit has no sub-tree. In those cases, the dwarf-reader crashes when ctxt.build_die_parent_relations_under tries to record the point at which the sub-tree (which is non-existent here) of the partial unit is imported by the DW_TAG_imported_unit DIE. This patch avoids crashing in those cases. As this problem is reported on libclang-cpp.so (on Fedora 33) which takes "abidw --abidiff" five hours and ~ 20GB of ram to complete at this point (on a power7 box) this patch doesn't have a regression test attached. * src/abg-dwarf-reader.cc (die_has_children): Define new static function. (read_context::build_die_parent_relations_under): Do not try to instantiate an imported_unit_point type for an imported unit with no children node. (imported_unit_point::imported_unit_point): Assert that the imported die has a sub-tree. (imported_unit_point::imported_unit_point): Remove useless spaces. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
264c8338cc |
reader: Fix off-by-one error in assert
* src/abg-reader.cc (build_subrange_type): Fix off-by-one error. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
bbdf7e0f29 |
writer: fix off-by-one error in assertion
* src/abg-writer.cc (write_array_subrange_type): Fix off-by-one error in assertion. * src/abg-dwarf-reader.cc (build_subrange_type): Assert the length of the array complies with its bounds. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
0a5f056393 |
abipkgdiff: make --self-check to fail on any change against own ABIXML
Now that several subtle causes of spurious ABI change report when comparing a binary against its own ABIXML have been addressed, this patch makes 'abipkgdiff --self-check' to fail on any ABI change reported. That is, harmless changes are not ignored anymore. * tools/abipkgdiff.cc (compare_to_self): Report *any* ABI change. Not just the "net" changes. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
7d5fc6a509 |
abidw: make --abidiff report any change against own ABIXML
Sometimes, "abidw --abidiff <binary>" would pass while "abidw <binary> > abi; abidiff <binary> abi" would fail. This is because "abidw --abidiff" emits an error only when the comparison between the binary and its ABIXML representation yields and incompatible change. Now that many subtle causes of spurious ABI change report emitted when comparing a binary against its own ABIXML have been fixed, this patch makes it so that *any* change would make abidw --abidiff to emit an error. * tools/abidw.cc (load_corpus_and_write_abixml): Emit an error when comparing the binary to its ABIXML representation yields any change. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
95c53c89f3 |
dwarf-reader: Avoid having several functions with the same symbol
In the DWARF debug info, a C++ class can be represented by pieces throughout a given binary. In this picture, a given virtual member function can be represented several times; each time in one different piece of the C++ class. In a given piece of the class, a virtual member function can be represented with its ELF symbol set. In another one, the same virtual member function can be represented without that ELF symbol set. And there can also be pieces of the class that don't have a given virtual function. To handle this, the DWARF reader constructs one class from all its pieces scattered around. It adds each virtual member function to the class as it comes across them while scanning the DWARF. Then there is a pass at the end of the process that sets ELF symbols to the (virtual) member functions that need it. The problem with that pass is that it sometimes sets the same ELF symbol to more than one virtual member function. Those virtual member functions all have the same mangled name that correspond to the ELF symbol; but only one of them is meant to be associated with the ELF symbol. In essence, that one is the one that is exported by the ELF binary. This patch teaches the pass that sets the ELF symbols of function to avoid setting the same ELF symbol to more than one function. The patch also avoids build_function_decl to set symbol to a function if that symbol was already set to an existing function. This patch thus fixes a class of issues what arise when comparing a binary against its own ABIXML representation. Those several functions having the same ELF symbol would cause spurious changes in that context. * src/abg-dwarf-reader.cc (read_context::symbol_already_belongs_to_a_function): Define new member function. (read_context::fixup_functions_with_no_symbols): Use the new symbol_already_belongs_to_a_function function to avoid setting a symbol that already belongs to a function. * tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust. * tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise. * tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
5ae7de0cb6 |
reader: Don't lose anonymous-ness of decl-only classes
When reading an anonymous declaration-only class from ABIXML libabigail forgets to set the is-anonymous class. This leads to spurious change reports when comparing a binary against its ABIXML representation. Fixed thus. Note that this doesn't yet impact any regression test but is useful for a coming patch that will make abidw --abidiff to emit an error for all ABI changes, not just the hard incompatible ones. Without this change, that coming patch will make runtestreaddwarf to fail. * src/abg-reader.cc (build_class_decl): Set the is-anonymous flag when reading a decl-only class. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
a07fce3669 |
ir: Introduce internal pretty representation for anonymous classes
There are two views for internal pretty representation of anonymous classes. 1/ When we look at the anonymous class itself, we use its 'flat representation' i.e: 'class {int blah; char bleh;}' 2/ When we look at a pointer or a reference to the anonymous class we use its generic anonymous internal name, i.e: '__anonymous_struct__*' As a general rule, libabigail always use the keyword 'class' to prefix the name of classes for internal purposes, independent from the fact that the type is a struct or a class. That is a pre-requisite to be able to canonicalize classes and structs together. In other words, if a class and a struct are structurally equal, they are going to be considered equivalent by the canonicalization process. Currently however, in the view 1/ of the pretty representation of anonymous classes, a struct and a class will have different representations. For instance, and empty anonymous struct would be represented as 'struct {}', whereas an empty anonymous class would be represented as 'class {}'. This prevents these two be considered equivalent by the canonicalization process. This leads to spurious change reports later down the road. In the view 2/ we have a similar but different problem: the qualified names of the anonymous classes are not taken into account when representing pointer or references to said anonymous classes. Only their unqualified generic anonymous internal names are taken into account in the representation. This leads to pointers/references to anonymous classes being wrongly considered equivalent even when they belong to different namespaces. This patch corrects the issues related to both views 1/ and 2/. It should make libabigail correctly consider some anonymous classes as equivalent (view 1) and correctly consider pointers/references to anonymous classes as different when they belong to different namespaces (view 2). A number of reference tests are adjusted accordingly. * include/abg-fwd.h (get_class_or_union_flat_representation): Introduce an "internal" parameter. * src/abg-ir.cc (get_class_or_union_flat_representation): Introduce an "internal" parameter. In the flat representation of a class for internal purposes, always use the prefix "class" even if this is a struct. (get_type_name): To build an internal name for a reference or pointer to an anonymous type, consider the namespace name of said type. (equals): In the overload for decl_base, take the namespace name of anonymous decls into account when comparing them. ({var_decl, union_decl}::get_pretty_representation): Adjust calls to get_class_or_union_flat_representation to pass a proper "internal" argument. * src/abg-default-reporter.cc (default_reporter::report): Adjust the call to get_class_or_union_flat_representation to pass an "internal" argument set to 'false'. * tests/data/test-annotate/libtest23.so.abi: Adjust. * tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise. * tests/data/test-read-dwarf/libtest23.so.abi: Likewise. * tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise. * tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise. * tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise. * tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise. * tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
35162bd4e3 |
writer: Emit definitions of declarations when they are present
Libabigail goes a long way to resolve declaration-only classes to their definitions when it's possible. The ABIXML writer however sometimes forgets to emit the definition of such declarations that have been "resolved". Later, when the binary is compared to its own ABIXML representation, the reporting engine thus reports that the definition is lost. This patch fixes that. * src/abg-writer.cc (write_class_decl, write_union_decl): Get the definition of the declaration if it exists and emit that. * tests/data/test-read-dwarf/test13-pr18894.so.abi: Adjust. * tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise. * tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise. * tests/data/test-annotate/test13-pr18894.so.abi: Likewise. * tests/data/test-annotate/test15-pr18892.so.abi: Likewise. * tests/data/test-annotate/test21-pr19092.so.abi: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
b553802af8 |
Bug 26780 - Fix array subrange bounds (de)serialization
When serializing subrange bounds to ABIXML today the writer omits the lower and upper bounds. Only the length is emitted. The reader thus assumes that the lower bound of an array subrange is always 0. In Fortran however, that is not true, as the lower bound is 1. This patch instructs the writer to emits the lower bound whenever it's different from zero. It also emits the upper bound in that case. The reader is updated accordingly to take the lower and upper bounds into account whenever they are present. It they are not, then the lower bound is assumed to be zero and the upper bound is deduced from the length, as was already the case until now. * src/abg-reader.cc (build_subrange_type): Read lower-bound attribute if present. Then try to read upper-bound attribute as well. If this is not an infinite subrange assert that the length must be equal to the difference between the bounds. * src/abg-writer.cc (write_array_subrange_type): Write the lower-bound if it's present and not zero. In that case, write the upper-bound as well. * tests/data/test-diff-pkg/hdf5-1.10.6-2.fc33.x86_64.rpm: Add new binary test input. * tests/data/test-diff-pkg/hdf5-debuginfo-1.10.6-2.fc33.x86_64.rpm: Likewise. * tests/data/test-diff-pkg/hdf5-1.10.6-2.fc33.x86_64.self-check-report-0.txt: Add new reference output. * tests/data/Makefile.am: Add the new test material above to source distribution. * tests/test-diff-pkg.cc (in_out_specs): Add the binary test input to the set of --self-check tests. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
7d5ef25b9c |
reader: Read array subrange length into an uint64_t
We read subrange length like an integer of type 'int" today. We should read it as an uint64_t. Fixed thus. * src/abg-reader.cc (build_subrange_type): Change the type of length to uint64_t and read it using strtoull. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
6ac6cfb3f6 |
abipkgdiff: Avoid uncertainty when sorting worker tasks
Worker tasks that have compared the binaries of two packages are sorted according to the sizes of the binaries they compared. The tasks that compared bigger binaries come first. So the output of these tasks is always sorted the same. When two tasks have sorted binaries of the same size, let's sort them by taking into account the lexicographic order of the binaries names. * elf_size_is_greater: Take the name of the binaries into account when their size is equal. Also, assert that all comparison tasks have compared binaries. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
33223963bd |
tests/data/test-fedabipkgdiff: Update reference output
Instances of self_compare_task don't wrongly shadow task comparison arguments anymore. Instead, they properly use the task comparison arguments of the parent class compare_task. So in the sorting function elf_size_is_greater, we now always have proper elf binaries size. It follows that the sorting can now yield a consistent result, always. It thus appears that the reference output recorded for the "self-comparison" test of runtestfedabipkgdiffpy3.sh was wrong. This patch updates it. * tests/data/test-fedabipkgdiff/test7-self-compare-from-fc23-dbus-glib-report-0.txt: Adjust. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Matthias Maennich
|
18b0693ca1 |
abipkgdiff: minor cleanups
The recent changes to abipkgdiff triggered some minor clang-tidy diagnostics that I address in this patch. - removed unused using statements - cleaned up some mismatches between commented and actual parameter names - removed shadowing data members from self_compare_task and construct with base class constructor instead. * tools/abipkgdiff.cc: remove unused using statements (self_compare_task): remove shadowing members and delegate construction to base class. Signed-off-by: Matthias Maennich <maennich@google.com> |
||
Dodji Seketeli
|
c2ce1a38c6 |
dwarf-reader: support artificially generated translation units
When GCC 10 artificially generates a translation unit, the path/name of the translation unit as given by the DW_AT_name attribute of the translation unit DIE is the string "<artificial>". Libabigail expects that translation units have unique file paths so having several artificially generated translation units like this one with the same name makes hell break loose down the road. This patch suffixes the name of artificial DIE with their die offset number to have a unique path name for artificially generated translation units. * configure.ac: Detect if we are running on RPM >= 4.15. If yes, then define the preprocessor macro RPM_4_15. If that macro is defined then test-diff-pkg.cc can support RPMs from Fedora >= 31 as those are compressed with zstd. Earlier RPM versions don't support that compression scheme. * src/abg-dwarf-reader.cc (build_translation_unit_and_add_to_ir): Suffix the offset of the translation unit to its name when that name is "<artificial>". * tests/data/test-diff-pkg/mesa-libGLU-9.0.1-3.fc33.x86_64.rpm: New binary test input. * tests/data/test-diff-pkg/mesa-libGLU-debuginfo-9.0.1-3.fc33.x86_64.rpm: Likewise. * tests/data/test-diff-pkg/mesa-libGLU-9.0.1-3.fc33.x86_64.self-check-report-0.txt: New reference output for the binary test input above. * tests/data/Makefile.am: Add the new test inputs above to source distribution. * tests/test-diff-pkg.cc (in_out_specs): Add the binary test inputs above to source distribution if we are running on an RPM version >= 4.15. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
ddead98a51 |
fedabipkgdiff: make --self-compare use abipkgdiff --self-check
Now that the abipkgdiff program supports the --self-check option to make it compare the binaries in an RPM against their own ABIXML representation, make the --self-compare option of fedabipkgdiff use the abipkgdiff --self-check. * tools/fedabipkgdiff (abipkgdiff): If the user provides the --self-compare options, generate the abipkgdiff command by using the --self-check option. (run_abipkgdiff): Each return value of the abipkgidiff runs can be negative because they are unsigned values in essence, but as python doesn't seem to have a unsigned integer type. So we need to consider the max of the absolute value of the return codes here. * tests/data/test-fedabipkgdiff/test7-self-compare-from-fc23-dbus-glib-report-0.txt: Adjust. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
edfe5a1dec |
abipkgdiff: Add a new --self-check option
After the report of https://sourceware.org/bugzilla/show_bug.cgi?id=26769 it became apparent that we want to be able compare a binary against its ABIXML representation, including for cases where the binaries are embedded in RPM packages. This patches thus introduces the --self-check option to abipkgdiff so that it can be invoked like this: $ abipkgdiff --self-check --d1 libstdc++-debuginfo-10.2.1-8.fc34.x86_64.rpm --d1 gcc-debuginfo-10.2.1-8.fc34.x86_64.rpm libstdc++-10.2.1-8.fc34.x86_64.rpm ==== SELF CHECK SUCCEEDED for 'libstdc++.so.6.0.28'==== $ With this option, libabigail compares each binary in the RPM against its own ABIXML representation. This should hopefully help to write regression tests which have as sole inputs the links to download the RPMs. It's also useful to ease the process of reproducing the issue raised. This option can now be used, for instance, by the libabigail-selfcheck program over at https://pagure.io/libabigail-selfcheck. * tools/abipkgdiff.cc (options::self_check): Define new data member. (options::options): Initialize it. (display_usage): Add help string for the --self-check option. (parse_command): Parse the new --self-check option. (extract_deb): Add missing newline. (compare): Remove useless white space. (compare_to_self, self_compare_prepared_userspace_package) (self_compare_prepared_package, compare_to_self): Add new static functions. (class self_compare_task): Add new class. (prepare_package): Add a new overload that takes just one parameter. (elf_size_is_greater): Don't crash if the args are empty. (main): If the --self-check option is given, make sure we have just one package in argument. Use the new compare_to_self function to handle the --self-check option. * doc/manuals/abipkgdiff.rst: Add documentation for the new --self-check option. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
cac59a176a |
Bug 26769 - Fix missing types in abixml output
The symptom of the issue at hand is that sometimes there can be types missing from the abixml output. This happens when analysing some C++ code bases. The core of the issue is the following. Support we have a type "struct S" defined somewhere as: struct S // #0 { int dm1; char dm2; }; S s; Suppose that in another translation unit, we have the class 'S' being extended to add a member type to it: struct S // #1 { typedef int dm1_type; }; typedef S::dm1_type Integer; Integer something; When emitting the abixml for the codebase, the definition of the typedef S::dm1_type can be missing. Note that in location #1, struct S is considered declaration-only. It's definition is in another translation unit, in location #0. So the abixml writer emits the 'struct S' defined in location #0, but forgets to emit the 'struct S' in #1, which is indirectly used for the sole purpose of using its member type S::dm1_type. This patch emits the S::dm1_type type that is mistakenly forgotten today. Now that the "struct S" of #1 is also emitted, a tangent problem is uncovered: S in #0 can be wrongly thought to be equivalent to S in #1, for ABI purposes This is because of an ODR-based optimization that is used for C++. That is, the two struct S can be wrongly considered equivalent just because they have the same name. Note that ODR means "One Definition Rule[1]" This patch removes the ODR-based optimization and thus fixes many of the issues uncovered by the previous changes. The patch also uncovered that some non-static variables were sometimes wrongly being added to the set of exported variables, while libabigail reads corpora from abixml. The patch fixes this as well. [1]: One Definition Rule: https://en.wikipedia.org/wiki/One_Definition_Rule * include/abg-corpus.h (corpus::{record_canonical_type, lookup_canonical_type}): Remove function declarations. * src/abg-corpus-priv.h (corpus::priv::canonical_types_): Remove data member. * src/abg-corpus.cc (corpus::{record_canonical_type, lookup_canonical_type}): Remove functions. * src/abg-ir.cc (type_eligible_for_odr_based_comparison): Remove static function. (type_base::get_canonical_type_for): Don't perform the ODR-based optimization for C++ anymore. * src/abg-reader.cc (read_context&::maybe_add_var_to_exported_decls): Don't add a variable that hasn't been added to its scope. Otherwise, it means we added a variable that wasn't yet properly constructed. Also add a new overload for var_decl_sptr&. (build_var_decl): Do not add the var to its the set of exported declaration before we are sure it has been fully constructed and added to the scope it belongs. (build_class_decl): Only add *static* data members to the list of exported declarations. (handle_var_decl): A var decl seen here is a global variable declaration. Add it to the list of exported declarations. * src/abg-writer.cc (write_context::decl_only_type_is_emitted): Constify parameter. (write_translation_unit): Do not forget to emit referenced types that were maybe not canonicalized. Also, avoid using noop_deleter when it's not necessary. (write_namespace_decl): Do not forget to emit canonicalized types that are present in namespaces other than the global namespace. * tests/runtestslowselfcompare.sh.in: New test that compares libabigail.so against its own ABIXML representation. * tests/Makefile.am: Add the new test runtestslowselfcompare.sh to source distribution. This test is too slow to be run during the course of 'make check'. It takes more than 5 minutes on my slow box here. Rather, it can be run using 'make check-self-compare'. I plan to run this before releases now. * tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Adjust. * tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise. * tests/data/test-annotate/test0.abi: Likewise. * tests/data/test-annotate/test13-pr18894.so.abi: Likewise. * tests/data/test-annotate/test14-pr18893.so.abi: Likewise. * tests/data/test-annotate/test15-pr18892.so.abi: Likewise. * tests/data/test-annotate/test17-pr19027.so.abi: Likewise. * tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-annotate/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: Likewise. * tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise. * tests/data/test-read-dwarf/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/libtest24-drop-fns-2.so.abi: Likewise. * tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Likewise. * tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise. * tests/data/test-read-dwarf/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/test14-pr18893.so.abi: Likewise. * tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise. * tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise. * tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise. * tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. * tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise. * tests/data/test-read-write/test28-without-std-fns-ref.xml: Likewise. * tests/data/test-read-write/test28-without-std-vars-ref.xml: Likewise. * tests/data/test-read-write/test6.xml: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
de5de7b447 |
Make sure to canonicalize all types but decl-only classes
While looking at the remaining potential sources of instability in the generation of type ids at abixml generation time, it occurred to me that we still had several non canonicalized types in the system. When a given type T is not canonicalized, hash_as_canonical_type_or_constant later hashes it as an arbitrary constant. So in the ID hash map of the abixml writer, all the non-canonicalized types will tend to end-up in the same hash map bucket, so to speak. More precisely, if T is equivalent to another type T' which is canonicalized, then T and T' will end-up in different buckets (in the same hash map) because they will have different hash values as returned by hash_as_canonical_type_or_constant. They will thus end-up having different type-ids because they are in different buckets. The solution is to make sure that T is *also* canonicalized. That way, if T and T' are equivalent, they'll end-up in the same bucket and they will have the same type-id. In other words, all types should be canonicalized now. The only exception to that rule is declaration-only classes and unions. The reason why a declaration-only type 'struct foo' needs to stay non-canonicalized is that it must equal all the definitions of 'struct foo' that can be found elsewhere. This patch thus canonicalizes all types that were not still not being canonicalized. It also adds an assert in hash_as_canonical_type_or_constant to ensure that only declaration-only class_or_union types are not canonicalized. * include/abg-fwd.h (is_declaration_only_class_or_union_type): Declare new ... * src/abg-ir.cc (is_declaration_only_class_or_union_type): ... function. (clone_array): Add the cloned array subrange to its scope so that it can later be canonicalized. (synthesize_type_from_translation_unit) (synthesize_function_type_from_translation_unit): Canonicalize the synthesized types. (hash_as_canonical_type_or_constant): Ensure that all types are canonicalized. * src/abg-dwarf-reader.cc (maybe_canonicalize_type): Remove useless overload. (build_ir_node_for_variadic_parameter_type) (schedule_array_tree_for_late_canonicalization): Define new static functions. (maybe_strip_qualification): Schedule type canonicalization for types cloned prior to editing. (build_function_type): Use the new build_ir_node_for_variadic_parameter_type. It takes care of canonicalizing variadic parameter types. (build_function_decl): Canonicalize the function type that is created here. (build_ir_node_from_die): Use the overload of maybe_canonicalize_type which canonicalizes class_or_union nodes directly, rather than the one which handles DIE offsets. The latter was used as an optimization to reduce the size of the array of types scheduled for canonicalization, as DIE offsets take less space than pointers to IR types. Now that we have DIE de-duplication, my bet is that we can do away with the former. And that also ensures that we miss no type for canonicalization purposes. * src/abg-reader.cc (build_array_type_def): Canonicalize the subrange types of the array. (build_type): Canonicalize all types. * 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/test13-pr18894.so.abi: Likewise. * tests/data/test-annotate/test15-pr18892.so.abi: Likewise. * tests/data/test-annotate/test17-pr19027.so.abi: Likewise. * tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-annotate/test21-pr19092.so.abi: Likewise. * tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.txt: Likewise. * tests/data/test-diff-filter/test30-pr18904-rvalueref-report1.txt: Likewise. * tests/data/test-diff-filter/test30-pr18904-rvalueref-report2.txt: Likewise. * tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-0.txt: Likewise. * tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: Likewise. * tests/data/test-read-dwarf/PR22122-libftdc.so.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-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/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/test17-pr19027.so.abi: Likewise. * tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. * tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
90eff56227 |
ir: Add equality op to array_type_def::subrange_type::bound_value
* include/abg-ir.h (array_type_def::subrange_type::bound_value::operator==): Declare new ... * src/abg-ir.cc (array_type_def::subrange_type::bound_value::operator==): ... equality operator. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
2cc1ab7ee8 |
writer: Sort decls and fix topological sorting for types
When emitting the declarations of a given translation unit, those declarations are not sorted. Ooops. This patch adds topological sorting for those declarations, making the decls defined first to be emitted first. When the decls are defined at the same location then the pretty representation is used for lexicographic sorting instead. It turns out that during the topological sorting for types there was some uncertainty when the declarations of the types had the same definition location. This patch re-uses the declaration sorting above for the declarations of these types. * include/abg-ir.h (scope_decl::get_sorted_member_decls): Declare new member function. * src/abg-ir.cc (struct decl_topo_comp): New sorting functor. (type_topo_comp::operator()): Re-use the decl_topo_comp to sort type declarations. (scope_decl::priv::sorted_members_): Add new data member. (scope_decl::get_sorted_member_decls): Define new member function. * src/abg-writer.cc (write_translation_unit): Use the new scope_decl::get_sorted_member_decls. * tests/data/test-annotate/libtest23.so.abi: Adjust. * tests/data/test-annotate/test15-pr18892.so.abi: Likewise. * tests/data/test-annotate/test17-pr19027.so.abi: Likewise. * tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-annotate/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/libtest23.so.abi: Likewise. * tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise. * tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise. * tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise. * tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. * tests/data/test-read-write/test2.xml: Likewise. * tests/data/test-read-write/test28-without-std-fns-ref.xml: Likewise. * tests/data/test-read-write/test28-without-std-vars-ref.xml: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
3c87247cb4 |
Bug PR26739 - Handle qualified typedef array types
CV-qualifiers of a typedef of an array type apply to the elements of the array. So this can be transformed into a typedef of array type which element type is similarly CV-qualified. That transformation helps avoiding spurious changes that might occur when comparing the latter form against the former even though both are equivalent. This patch performs that transformation, just like we already do for CV-qualified array types which are transformed into an array of similarly CV-qualified elements. Performing that transformation amounts to editing the type of the array elements. As those types might be used by other parts of the type graph (type node sharing), the patch "clones" the type sub-tree of interest and edits the cloned version. That way, the shared type nodes are not edited. It appears that in the existing version of maybe_strip_qualification, the transformation of CV-qualified arrays into arrays of CV-qualified elements was unfortunately editing shared type nodes. The patch fixes that and re-works the logic of* maybe_strip_qualification to make it handle this new case and the previous one in a somewhat generic manner. * include/abg-fwd.h (is_typedef_of_array, clone_array) (clone_typedef, clone_qualified_type, clone_array_tree): Declare new functions. (peel_qualified_or_typedef_type): Declare new overload. (is_array_of_qualified_element): Constify the parameter. * include/abg-ir.h ({qualified_type, typedef}_def::set_underlying_type): Add new member functions. (array_type_def::subrange_type::subrange_type): Make constify the reference to the underlying type parameter. * src/abg-ir.cc (is_array_of_qualified_element): Constify the parameter. (peel_qualified_or_typedef_type): Define new overload for type_base_sptr. (clone_typedef_array_qualified_type): Define static function. (clone_array clone_typedef, clone_qualified_type) (clone_array_tree, is_typedef_of_array): Define new functions. (qualified_type_def::get_underlying_type): Rename the return type shared_ptr<type_base> into type_base_sptr. ({typedef, qualified_type}_def::set_underlying_type): Define new member function. (array_type_def::subrange_type::priv::priv): Initialize the 'infinite_' data member. * src/abg-dwarf-reader.cc (maybe_strip_qualification): Handle qualified typedef of arrays. Merge this with the handling of qualified arrays. Note that before editing the elements of the array to make the array (or typedef) qualifier apply to the element the sub-tree is cloned to make its type nodes be 'un-shared'. This prevents us from editing type nodes that are shared by other type expressions. * tests/data/test-diff-filter/test-PR26739-report-0.txt: New reference test output. * tests/data/test-diff-filter/test-PR26739-2-report-0.txt: Likewise. * tests/data/test-diff-filter/test-PR26739-v{0,1}.c: Source code of new binary test input. * tests/data/test-diff-filter/test-PR26739-2-v{0,1}.c: Likewise. * tests/data/test-diff-filter/test-PR26739-v{0,1}.o: New binary test inputs. * tests/data/test-diff-filter/test-PR26739-2-v{0,1}.o: Likewise. * tests/data/Makefile.am: Add the new test material above to source distribution. * tests/test-diff-filter.cc (in_out_specs): Add the test inputs above to this harness. * tests/data/test-annotate/test15-pr18892.so.abi: Adjust. * tests/data/test-annotate/test17-pr19027.so.abi: Likewise. * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-annotate/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise. * tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise. * tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise. * tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise. * tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
baee817d3e |
Update test-libandroid.so.abi
Update this test reference output as per the commit below:
|
||
Matthias Maennich
|
38e72b262d |
dwarf-reader: Ignore zero length location expressions from DW_AT_location
Location expressions might occasionally be of length 0. E.g. a reason for that are thread local variables that do not exactly have a location to refer to. Compilers/Linkers may choose an empty location description. E.g. see the dwarfdump output for the added testcase based on libandroid.so (from AOSP). $ dwarfdump libandroid.so|egrep -B9 "gChoreographerE$" LOCAL_SYMBOLS: < 1><0x00000022> DW_TAG_namespace DW_AT_name android < 2><0x00000027> DW_TAG_variable DW_AT_name gChoreographer DW_AT_type <0x00000b65> DW_AT_decl_file 0x00000003 .../choreographer.cpp DW_AT_decl_line 0x00000059 DW_AT_location len 0x0000: : DW_AT_linkage_name _ZN7androidL14gChoreographerE The DW_AT_location is properly read by elfutils' dwarf_location(), but is not useful for us to proceed with. Hence early exit on this. * src/abg-dwarf-reader.cc (die_location_expr): Ignore zero length location expressions. * tests/data/Makefile.am: Add new test files. * tests/data/test-read-dwarf/test-libandroid.so: New test file. * tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise. * tests/test-read-dwarf.cc: Add new test case. Reported-by: Dan Albert <danalbert@google.com> Reviewed-by: Giuliano Procida <gprocida@google.com> Cc: Mark Wielaard <mark@klomp.org> Signed-off-by: Matthias Maennich <maennich@google.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Giuliano Procida
|
8a0825e319 |
Improve enum synthetic type names
The ordering of canonical types (in an abi-instr XML element) appears to be sensitive to the particular C++ library used and the presence of other threads doing heap allocation. This patch forces distinct synthetic enum-underlying types to have distinct names, which ensures deterministic XML output order. * src/abg-dwarf-reader.cc (build_internal_underlying_enum_type_name): Add a size argument (and don't default is_anonymous argument). Append size of type to synthetic type name. (build_enum_underlying_type): Pass type size to build_internal_underlying_enum_type_name. * tests/data/test-abidiff-exit/test-decl-enum-report-3.txt: Update. Note that there may be an issue with leaf-mode reporting of pointer type changes. * tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0.abi: Regenerate this (catching up with various abidw updates). * tests/data/test-annotate/test-anonymous-members-0.o.abi: Refresh with new type names. * tests/data/test-annotate/test0.abi: Likewise. * tests/data/test-annotate/test13-pr18894.so.abi: Likewise. * tests/data/test-annotate/test14-pr18893.so.abi: Likewise. * tests/data/test-annotate/test15-pr18892.so.abi: Likewise. * tests/data/test-annotate/test17-pr19027.so.abi: Likewise. * tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-annotate/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: Likewise. * tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise. * tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise. * tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi: Likewise. * tests/data/test-read-dwarf/test0.abi: Likewise. * tests/data/test-read-dwarf/test0.hash.abi: Likewise. * tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise. * tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise. * tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise. * tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise. * tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise. * tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise. * tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise. * tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise. * tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. * tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise. Signed-off-by: Giuliano Procida <gprocida@google.com> Signed-off-by: Matthias Maennich <maennich@google.com> |
||
Giuliano Procida
|
32343f8091 |
Improve and stabilise sort of member functions
The functor virtual_member_function_less_than did not take into account linkage name which can be the only difference when multiple destructors with differing mangled names are present. This change adds a check for linkage names and also flattens the control flow in the comparison method to make the logic clearer. Lastly, this change also uses std::stable_sort, in case all that remains is insertion order. * src/abg-ir.cc (virtual_member_function_less_than::operator()): Name temporaries like offsets and symbols to reduce repetition; test each pair of elements (including symbol presence) and return immediately if there's a difference; add a comparison of linkage name just after comparing symbol names. (sort_virtual_member_functions): Use stable_sort instead of sort. * tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: Update with new ordering of member functions. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. Signed-off-by: Giuliano Procida <gprocida@google.com> Signed-off-by: Matthias Maennich <maennich@google.com> |
||
Dodji Seketeli
|
b4ec62929c |
update-test-output.py: Update syntax
* tests/update-test-output.py: Update syntax for python3. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
b7ad7f2b2d |
Bug 26770 - Spurious declaration-only-ness induces spurious type changes
Sometimes when amending a C++ class to add new members or properties to it as directed by the DWARF debug information, we can end-up with discrepancies related to declaration-only-ness. That is, an instances of a given type Foo can be wrongly assigned declaration-only-ness that should have been only carried by another instance Foo. Then, later, comparing two pointers to Foo might wrongly lead to spurious reported changes due to the spurious differences of declaration-only-ness in two instances of Foo. By fixing the setting of the declaration-only-ness, especially when amending a C++ class this patch fixes that spurious change detected. * src/abg-dwarf-reader.cc (add_or_update_class_type): When creating a class, set declaration-only-ness unconditionally. When updating the class however, only set the declaration-only-ness when the current one is not consistent with the size of the class. * tests/data/test-annotate/test14-pr18893.so.abi: Adjust. * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise. * tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Matthias Maennich
|
4f93fed261 |
dwarf-reader: fix lookup for repeated translation unit paths
When using relative compilation unit paths in DWARF, the lookup for an existing one was done with an incorrect path. In particular, a '/' was prefixed to the path regardless of whether the compilation_dir is set. That led to the instantiation of an additional translation unit that and failed when adding it to the corpus due to violating translation unit uniqueness. Fix that by correcting the lookup. * src/abg-dwarf-reader.cc(build_translation_unit_and_add_to_ir): Fix lookup for potentially already existing translation units. Reported-by: Dan Albert <danalbert@google.com> Signed-off-by: Matthias Maennich <maennich@google.com> |
||
Giuliano Procida
|
f8ef1958da |
Stabilise sort of canonical types
Some types like unnamed-enum-underlying-type are not distinguished by type_topo_comp. This can result in nondeterministic output and flakey tests. While a more complete ordering from type_topo_comp would be nice, the nondeterminism can reduced by preserving the relative order of identically-named types. * src/abg-ir.cc (scope_decl::get_sorted_canonical_types): Sort canonical types with std::stable_sort(..., type_topo_comp()). Signed-off-by: Giuliano Procida <gprocida@google.com> Reviewed-by: Matthias Maennich <maennich@google.com> Signed-off-by: Matthias Maennich <maennich@google.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Mark Wielaard
|
67db22f50c |
Assume subrange bounds types are unsigned if no underlying type is given.
When running abidiff on test34-libjemalloc.so.2-intel-16.0.3 it would crash in array_type_def::subrange_type::get_length on the ABG_ASSERT get_upper_bound() >= get_lower_bound(). This was because that file contained a subrange upper_bound value encoded with data1 or data2 without an underlying type. In that case we assumed the value was encoded as a signed value which caused some of the upper bounds to be negative (while the lower bound, which wasn't given, was assumed to be zero). * src/abg-dwarf-reader.cc (build_subrange_type): Default is_signed to false. Signed-off-by: Mark Wielaard <mark@klomp.org> Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Mark Wielaard
|
ee059a95cc |
dwarf-reader: get subrange_type bounds signedness from underlying type
This patch was originally submitted by in a comment of a problem report at https://sourceware.org/bugzilla/show_bug.cgi?id=26684#c10. When reading the bounds of a subrange_type, we need to know if the constant value of those bounds is signed or not. For that, we used to just look at the form of those constant and infer the signedness from there. The problem is that the signedness is really determined by the value of the DW_AT_encoding attribute present on the underlying type of the subrange_type; the form of the bound is mere implementation detail that is downstream of the information of carried by the DW_AT_encoding attribute. This patch thus does the right thing by looking at the underlying type of the subrange_type and by doing away with the unnecessary messing with attribute value forms. * src/abg-dwarf-reader.cc (die_attribute_has_form) (die_attribute_is_signed, die_attribute_is_unsigned) (die_attribute_has_no_signedness): Remove static functions. (die_constant_attribute): Add the 'is_signed' parameter. (die_address_attribute): Adjust comment. (build_subrange_type): Determine signedness of the bounds by looking at the DW_AT_encoding attribute of the underlying type. Signed-off-by: Mark Wielaard <mark@klomp.org> Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
247f50dbf3 |
abg-tools-utils: Fix comment
* include/abg-tools-utils.h (enum abidiff_status): Fix a comment. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
8d7ffe3d06 |
configure: Support ABIGAIL_NO_OPTIMIZATION_DEBUG environment variable
When working in development environments with compiler versions that might be very bleeding edge (like the Fedora Rawhide distribution) it might be worthwhile to disable all compiler optimization to have a better debugging experience. In practice, I bumped into this need again and again. So I am adding this ABIGAIL_NO_OPTIMIZATION_DEBUG environment variable to basically allow the "-g -O0" combination, if need be. This patch obviously doesn't change any existing behaviour if the user doesn't set this newly introduced environment variable. * configure.ac: Set the CXXFLAGS and CFLAGS to "-g -O0 -Wall -Wextra -Werror" if the ABIGAIL_NO_OPTIMIZATION_DEBUG is set. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
470e10ff52 |
Structurally compare the few non-canonicalized types in general
In the abixml writer, we recently stopped using type_base::dynamic_hash as a slow path to hash a non-canonicalized type. Rather, we use an arbitrary constant as a hash for those non-canonicalized types. This amounts to using structural comparison for those types. The function that implements this hashing scheme is hash_as_canonical_type_or_constant. A potential concern for that approach was the possible negative impact on speed. As it turns out since the change went in, there was no noticeable speed impact raised after testing. In insight, this is understandable as the number of non-canonicalized types should be extremely reduced now that we canonicalize pretty much all types. As far as I understand it at this point, the only types that would not be canonicalized are unresolved declaration-only types. And, all in all, structurally comparing unresolved declaration-only types should be "fast enough". If we ever stumble across any other non-canonicalized type, I think that would be an artifact of a bug that ought to be fixed. On that basis, I went ahead and used hash_as_canonical_type_or_constant throughout the code base and did away with the use of type_base::dynamic_hash for now, until it's properly audited, regression tested and got ready for the use cases where it might make sense. This patch thus makes hash_type use hash_as_canonical_type_or_constant. The writer is then back to using hash_type again, as it used to; but at the same time, it's still using structural comparison for non-canonilized types. So is hash_type_or_decl, which now uses hash_type to hash types, rather than using type_base::dynamic_hash. Note that the comparison engine heavily uses hash_type_or_decl to hash diff nodes. So with this small patch the comparison engine is now using structural comparison of non-canonicalized types (and diff nodes), just as the abixml writer does. * include/abg-fwd.h (hash_as_canonical_type_or_constant): Remove public declaration of this function. * src/abg-hash.cc (type_base::dynamic_hash::operator()): Add a comment. * src/abg-ir.cc (hash_as_canonical_type_or_constant): Make this function static now. (hash_type_or_decl): Use hash_type for types. * src/abg-writer.cc (type_hasher::operator()): Use hash_type. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
fd0f3ac341 |
Fix redundancy detection in the diff graph
A recent change[1] triggered a variation in the ABI changes reported,
depending on the platform.
It turned out the change made the redundancy detector walk more diff
nodes (like inserted/deleted virtual member functions and their
implicit parameter type) and that uncovered several underlying issues
that has been latent for a long time.
First, we were not walking the inserted/deleted virtual member
functions in a deterministic manner at reporting time. Rather than
walking the unordered maps containing those functions, this patch now
walk them in lexicographic order. The patch also does something
similar for the changed data members, but this time during the diff
graph analysis. That order affects how we consider a given type
change to be redundant.
Second, when looking a diff node named N, if another diff node N'
equivalent to N has already been marked redundant (and thus filtered
out already), we were sometimes wrongly failing to detect and mark N
as redundant. This patch fixes that.
I realized that some code was now unnecessary so I removed it.
A lot of reference output of tests are adjusted by this patch.
Mostly, these were cases we were failing to properly detect (and
filter out) as redundant reports. So the change reports should
hopefully look more concise and to the point now.
[1] the recent change is this one:
|
||
Dodji Seketeli
|
2f92777dc8 |
Consider the implicit 'this' parameter when comparing methods
Since 2013 the implicit 'this' parameter has been excluded from the function parameters taken into account while comparing class member functions. This was an early measure to avoid infinite recursion that would then occur when comparing classes (and thus their member functions that are referenced in their vtable). But since then, we've built descent infrastructure to prevent this kind of recursion in a more generic manner. This patch thus removes that restriction and should therefore lift the concerns expressed in the bug https://sourceware.org/bugzilla/show_bug.cgi?id=26672. Namely, changes to (data members of) a class should now be detected when comparing member functions of that class. With this change, the reference output of several comparison regression tests changed because, obviously, some impacted member functions are now reported along with detecting changes in data membrers of classes. The patch thus adjusts those reference ouputs. The patch also adjust the behaviour of the predicate: "accessed_through = pointer|reference|reference-or-pointer" The idea is to make the predicate work on qualified version of a type. * include/abg-ir.h (function_type::get_first_parm): Declare new accessor. * src/abg-ir.cc (function_type::get_first_parm): Define new accessor. (equals): In the overload for function_type, always take the implicit "this" parameter into account in parameter comparisons. (function_type::get_first_non_implicit_parm): Adjust comment. * src/abg-comp-filter.cc (function_name_changed_but_not_symbol): Avoid potential NULL pointer dereferencing. * src/abg-comparison.cc (function_type_diff::ensure_lookup_tables_populated): Always take the changes to the implicit 'this' parameter into account in the function type diff. (compute_diff): In the overload for function_type, Always compare the implicit 'this' parameter when comparing function parameters. * src/abg-default-reporter.cc (default_reporter::report): Refer to "implicit parameter" when reporting changes on parameters artificially generated by the compiler. * src/abg-suppression.cc (type_suppression::suppresses_diff): Make the 'access_through' predicate work on a qualified version of type 'S', even if it was meant to work on type 'S'. This allows it to work on 'const S', especially when S is accessed through 'pointer to const S', which happens when we consider the implicit 'this' parameter of a const member function. * tests/data/test-abicompat/test5-fn-changed-report-0.txt: Adjust. * tests/data/test-abicompat/test5-fn-changed-report-1.txt: Likewise. * tests/data/test-abidiff-exit/test1-voffset-change-report0.txt: Likewise. * tests/data/test-abidiff/test-PR18791-report0.txt: Likewise. * tests/data/test-abidiff/test-struct1-report.txt: Likewise. * tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test0-report.txt: Likewise. * tests/data/test-diff-dwarf/test28-vtable-changes-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test36-ppc64-aliases-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt: Likewise. * tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt: Likewise. * tests/data/test-diff-dwarf/test5-report.txt: Likewise. * tests/data/test-diff-dwarf/test8-report.txt: Likewise. * tests/data/test-diff-filter/test0-report.txt: Likewise. * tests/data/test-diff-filter/test01-report.txt: Likewise. * tests/data/test-diff-filter/test10-report.txt: Likewise. * tests/data/test-diff-filter/test13-report.txt: Likewise. * tests/data/test-diff-filter/test2-report.txt: Likewise. * tests/data/test-diff-filter/test28-redundant-and-filtered-children-nodes-report-0.txt: Likewise. * tests/data/test-diff-filter/test28-redundant-and-filtered-children-nodes-report-1.txt: Likewise. * tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.txt: Likewise. * tests/data/test-diff-filter/test30-pr18904-rvalueref-report1.txt: Likewise. * tests/data/test-diff-filter/test30-pr18904-rvalueref-report2.txt: Likewise. * tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt: Likewise. * tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt: Likewise. * tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-0.txt: Likewise. * tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-1.txt: Likewise. * tests/data/test-diff-filter/test4-report.txt: Likewise. * tests/data/test-diff-filter/test41-report-0.txt: Likewise. * tests/data/test-diff-filter/test9-report.txt: Likewise. * tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt: Likewise. * tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt: Likewise. * tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-0.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-1.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-10.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-11.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-12.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-13.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-14.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-15.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-16.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-2.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-3.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-4.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-5.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-6.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-7.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-8.txt: Likewise. * tests/data/test-diff-suppr/test24-soname-report-9.txt: Likewise. * tests/data/test-diff-suppr/test31-report-1.txt: Likewise. * tests/data/test-diff-suppr/test33-report-0.txt: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Giuliano Procida
|
b96040e5b3 |
Fix two wrongs in test suppression regex
The suppression specification in test38-char-class-in-ini.abignore was
introduced in commit
|
||
Giuliano Procida
|
1ab36e02e5 |
Add missing newlines to end of test files.
Various test files were missing terminal newlines. * tests/data/test-diff-suppr/test0-type-suppr-2.suppr: Add final new line. * tests/data/test-diff-suppr/test22-suppr-removed-var-sym-4.suppr: Likewise. * tests/data/test-diff-suppr/test23-alias-filter-0.suppr: Likewise. * tests/data/test-diff-suppr/test23-alias-filter-4.suppr: Likewise. * tests/data/test-diff-suppr/test28-add-aliased-function-1.suppr: Likewise. * tests/data/test-diff-suppr/test28-add-aliased-function-2.suppr: Likewise. * tests/data/test-diff-suppr/test28-add-aliased-function-3.suppr: Likewise. * tests/data/test-diff-suppr/test28-add-aliased-function-4.suppr: Likewise. * tests/data/test-diff-suppr/test41-enumerator-changes-0.suppr: Likewise. * tests/data/test-diff-suppr/test7-var-suppr-7.suppr: Likewise. * tests/data/test-ini/test01-equal-in-property-string.abignore: Likewise. Signed-off-by: Giuliano Procida <gprocida@google.com> |
||
Giuliano Procida
|
ebaf3305bc |
abg-corpus.cc: report architecture discrepancies
If there ever is a discrepancy between the architectures of the corpuses of a corpus group, libabigail will just abort with an assertion, if enabled. However, this is a case of invalid input and the cause should be reported to the user. * src/abg-corpus.cc (corpus_group::add_corpus): Report architecture discrepancies. Signed-off-by: Giuliano Procida <gprocida@google.com> |
||
Dodji Seketeli
|
59610d5572 |
Bug 26568 - Union should support more than one anonymous member
When building a union type we try to ensure that each member is present only once. This is because the code to build the union is also used to actually update a partially constructed union. To do so, before adding a member to the union, the member is looked up (among the current members) by name to see if it's already present or not. But then for anonymous members, the name of the member is empty. After the first anonymous member is added to the union, subsequent look-ups with an empty name all succeed. So no more than one anonymous member is added to the union. Oops. A way to fix this is to perform the lookup by taking into account the type of the anonymous data member, not its (empty) name. We already do this for anonymous data members of classes/structs. This patch thus uses that type-based anonymous data member lookup for unions. But then now that unions can have several anonymous members, another issue was uncovered. Array types whose elements are of anonymous type can be wrongly considered different because of canonicalization issues. Let's suppose we have these two arrays whose internal pretty representation are: "__anonymous_struct_1__ foo[5]" and "__anonymous_struct_2__ foo[5]" These are arrays of 5 elements of type anonymous struct. Suppose the anonymous structs "__anonymous_struct_1__" and "__anonymous_struct_2__" are structurally equivalent. Because the internal names of these array element types are different, the internal pretty representations of the arrays are different. And thus the canonical types of the two arrays are different. And that's wrong. In this particular case, they should have the same canonical type and thus be considered equivalent. This patch thus teaches 'get_type_name' to make the internal type name of anonymous types of a given kind be the same. Thus, making all arrays of 5 anonymous struct have the same pretty representation: "__anonymous_struct__ foo[5]" This gives the type canonicalizer a chance to detect that those arrays having the same canonical type. These two changes while being seemingly unrelated need to be bundled together to fix a number of issues in the existing test reference outputs because fixing the first one is needed to uncover the later issue. * src/abg-dwarf-reader.cc (add_or_update_union_type): Don't use the empty name of anonymous members in the lookup to ensure that all data members are unique. Rather, use the whole anonymous member itself for the lookup, just like is done to handle anonymous data member in classes/structs. * src/abg-reader.cc (build_union_decl): Likewise. * src/abg-ir.cc (get_generic_anonymous_internal_type_name): Define new static function. (get_type_name): For internal purposes, make the type name of all anonymous types of a given kind to be the same. This allows the internal representation of anonymous types which are based on type names to all be the same, so that they can be compared among themselves during type canonicalization. * tests/data/test-read-dwarf/test-PR26568-{1,2}.c: Source code of binary test input. * tests/data/test-read-dwarf/test-PR26568-{1,2}.o: New binary test input. * tests/data/test-read-dwarf/test-PR26568-{1,2}.o.abi: New reference test ouput. * tests/data/Makefile.am: Add the new test material above to source distribution. * tests/test-read-dwarf.cc (in_out_specs): Add the new binary test input above to this test harness. * tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0.abi: Adjust. * tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise. * tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
ee2b54ddd9 |
Make abidiff and abidw support several --headers-dir{1,2} options
When handling a binary with abidiff or abidw it can be useful to provide several different header files directories, for the cases where the header files of the binary are scathered in several different directories. It thus becomes possible to invoke abidiff like this: abidiff --headers-dir1 first-header-dir1 \ --headers-dir1 second-header-dir1 \ --headers-dir2 first-header-dir2 \ --headers-dir2 second-header-dir2 \ binary1 binary2 This patch adds support for that. It also modifies the tests/test-abidiff-exit.cc test harness to make it take header directories. With that modification done, a new test is added in that harness to exercise this new feature. This should close the feature request over at https://sourceware.org/bugzilla/show_bug.cgi?id=26565. * doc/manuals/abidiff.rst: Update documentation for the --headers-dir{1,2} options. * doc/manuals/abidw.rst: Likewise for the --header-dir option. * include/abg-tools-utils.h (gen_suppr_spec_from_headers): Add new overload that takes a vector of headers root dirs. * src/abg-tools-utils.cc (gen_suppr_spec_from_headers_root_dir): Define new function. (gen_suppr_spec_from_headers): Define a new overload that takes a vector of head_root_dir strings; it uses the new gen_suppr_spec_from_headers function. Use the new overload in the previous one that takes just one head_root_dir string. * tools/abidiff.cc (options::headers_dirs{1,2}): Rename option::headers_dir{1,2} into this one and make it be a vector of strings rather than just a string. (parse_command_line): Support several --headers-dir{1,2} on the command line. (set_diff_context_from_opts, set_suppressions): Adjust. * tools/abidw.cc (options::headers_dirs): Renamed options::headers_dir into this and make it be a vector of strings rather than just a string. (parse_command_line): Support several --headers-dir on the command line. (set_suppressions): Adjust. * tests/data/test-abidiff-exit/test-headers-dirs/headers-a/header-a-v{0,1}.h: Header files of new binary test input. * tests/data/test-abidiff-exit/test-headers-dirs/headers-b/header-b-v{0,1}.h: Likewise. * tests/data/test-abidiff-exit/test-headers-dirs/test-headers-dir-v{0,1}.c: Source code of new binary test input. * tests/data/test-abidiff-exit/test-headers-dirs/test-headers-dir-report-{1,2}.txt: Reference output of new binary test input. * tests/data/test-abidiff-exit/test-headers-dirs/test-headers-dir-v{0,1}.o: New binary test input. * tests/data/Makefile.am: Add the new files above to source distribution. * tests/test-abidiff-exit.cc (InOutSpec::in_elfv{0,1}_path): Add new data members. (in_out_specs): Adjust the content of this array as its type changed. Also, add two new entries to run the test over the new binary test inputs above. (do_prefix_strings): Define new static function. (main): Use it the new do_prefix_strings here. Make abidiff use the --header-dir{1,2} option whenever header directories are specified in an entry of the in_out_specs array. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
3d7916caa4 |
Bug 26309 - Wrong leaf reporting of changes to typedef underlying type
In leaf mode, libabigail fails to report changes to the underlying type of a typedef. At its core, this is due to the fact that changes to the underlying type of a typedef are not considered local. As the leaf reporter only reports local changes (as opposed to non-local changes which are changes to sub-types) it doesn't detect those non-local typedef changes. To handle this, this patch makes changes to the underlying type of a typedef be considered as local changes. This is like what we already do for pointer and qualified types. Now that we have another set of changes to report in the leaf reporter, we need to handle how to propagate the category and redundancy status of those changes. The patch does this too. Also, just like we do pointer and qualified type changes, the patch avoids marking the diff node carrying the typedef change as being a leaf change. That way, only existing leaf changes carrying that typedef diff node will be reported. For instance, a function whose parameter has a typedef change will be reported because that change to the function is considered a leaf change. Otherwise, reporting the typedef (or the pointer or qualified) type change on its own is not useful unless it impacts those leaf changes that we deem useful. The patch adds the example given in problem report to the testsuite. * src/abg-ir.cc (equals): In the overload for typedef_decls, report changes to the underlying type as being local of kind LOCAL_TYPE_CHANGE_KIND. * src/abg-comparison.cc (leaf_diff_node_marker_visitor::visit_begin): Do not mark typedef diff node as leaf node. (suppression_categorization_visitor::visit_end): Propagate the 'suppressed' category of the underlying type to the parent typedef unless the later has a local non-type change. (redundancy_marking_visitor::visit_end): Likewise for the 'redundant' category. * include/abg-reporter.h (report_non_type_typedef_changes): Rename ... * src/abg-default-reporter.cc (report_non_type_typedef_changes): ... report_local_typedef_changes into this. * src/abg-leaf-reporter.cc (leaf_reporter::report): Make the leaf reporter invoke the reporting method of the default reporter for typedefs as all typedef changes are now local. * tests/data/test-diff-filter/test-PR26309-report-0.txt: Add new test reference output. * tests/data/test-diff-filter/test-PR26309-v{0,1}.o: Add new test binary input. * tests/data/test-diff-filter/test-PR26309-v{0,1}.c: Add source code for new test binary input. * tests/data/Makefile.am: Add the new text material above to source distribution. * tests/test-diff-filter.cc (in_out_specs): Add the new test input above to this test harness. * tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt: Adjust. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
a434894e4d |
Fix thinko in get_vmlinux_path_from_kernel_dist
* src/abg-tools-utils.cc (get_vmlinux_path_from_kernel_dist): Fix thinko. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
5cf2473d3c |
writer: Avoid using dynamic hashing in type maps
When using type maps the hash used for a given type T is usually the pointer value of the canonical type of T. In the rare cases where T doesn't have a canonical type, we were using a 'dynamic hash' computed by recursively walking the components of T and progressively building a hash that way. The dynamic hashing code hasn't been updated in a while and would need some overhaul to support hashing with schemes like MD5 or maybe sha even. It might be useful for various use cases that have been proposed by some users over the years but nobody was motivated enough to implement it. In the mean time, rather than trying to come up with a fully beefed up dynamic hashing code, we'd rather just return a constant number for non canonicalized types. In practise that amounts to forcing the code of the maps to always use structural comparison for those non canonicalized types. Note that the amount of non-canonicalized types should be fairly small. For now, the only non-canonicalized types should be declaration-only types and those are quite fast to compare anyway. This patch thus introduces a new hashing scheme for maps in the writer which just uses a numerical constant as the hash for non-canonicalized types. * include/abg-fwd.h (hash_as_canonical_type_or_constant): Declare ... * src/abg-ir.cc (hash_as_canonical_type_or_constant): ... new function. * src/abg-writer.cc (type_hasher::operator()): Use the new hash_as_canonical_type_or_constant. * tests/data/test-read-dwarf/test16-pr18904.so.abi: Adjust. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Dodji Seketeli
|
005ab5c9bd |
Use flat representation to canonicalize anonymous classes and unions
During the canonicalization of a type T, the algorithm uses the internal pretty representation of T to limit the number of types to compare T to. That internal pretty representation is based on the type name. For anonymous types, the type name is not unique; it's constructed just for internal purposes. So using that in the pretty representation might negatively impact the accuracy of the canonicalization; it might make it so that two types might wrongly be considered canonicaly different. To fix that, this change makes the internal pretty representation of anonymous classes (and unions) use their flat representation. For the record, the flat representation of an anonymous struct with a an integer and a char data members is the string: 'struct {int i; char c;}' * src/abg-ir.cc ({class, union}_decl::get_pretty_representation): Use the flat representation of the class or union even for internal purposes. * tests/data/test-annotate/libtest23.so.abi: Adjust. * tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt: Likewise. * tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.txt: Likewise. * tests/data/test-diff-filter/test30-pr18904-rvalueref-report1.txt: Likewise. * tests/data/test-diff-filter/test30-pr18904-rvalueref-report2.txt: Likewise. * tests/data/test-diff-filter/test35-pr18754-no-added-syms-report-0.txt: Likewise. * tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt: Likewise. * tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise. * tests/data/test-read-dwarf/libtest23.so.abi: Likewise. * tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise. Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Giuliano Procida
|
0bd4b93517 |
DWARF: track chained DIE declaration-only status
Bug 26297: Possible misinterpretation of DW_AT_declaration via DW_AT_specification A DIE representing a definition can refer to the DIE that represents the matching declaration by using the DW_AT_specification attribute. A similar situation exists for a cloned entity (like a cloned variable or function) where the DW_AT_abstract_origin points to the original entity. Usually, to get the union of the attributes for a given definition DIE, we also need to gather the attributes carried by the declaration (or original) DIE accompanying the current definition DIE. For the "is_declaration" attribute however, we should only look at the current (definition) DIE at hand. Said otherwise, we should not follow declaration/origin link when we want to know if a given entity is declaration-only. Thus, this commit causes the DWARF reader to only consider a DIE to be "declaration only" if all DIEs in the chain leading to it have the DW_AT_declaration attribute set. It is a follow-up commit to a change making die_is_declaration_only examine just the immediate DIE. The responsibility of tracking the cumulative declaration-only status of DIEs falls on build_ir_node_from_die which is the function that makes recursive calls to itself on encountering a DW_AT_specification or DW_AT_abstract_origin link. Various other functions that would have previously called die_is_declaration_only are modified so that get the cumulative value for this flag, rather than just examing the DIE they are given. This change eliminates a lot of spurious declaration-only types in ABI output and may also prevent the same, particularly when anonymous, from confusing libabigail's type equality and canonicalisation logic. * src/abg-dwarf-reader.cc (add_or_update_class_type): Add an is_declaration_only argument. Use this in favour of the die_is_declaration_only helper function. (add_or_update_union_type): Ditto. (function_is_suppressed): Ditto. (build_or_get_fn_decl_if_not_suppressed): Ditto. (build_enum_type): Ditto. (build_ir_node_from_die): To the main overload, add is_declaration_only argument and default this to true. Update this to false if the given DIE is not declaration only and pass this on in recusrive calls and calls to build_enum_type, add_or_update_union_type, add_or_update_class_type and build_or_get_fn_decl_if_not_suppressed. * tests/data/test-annotate/test17-pr19027.so.abi: Update test. This is mostly the removal of is-declaration-only attributes, removal of unreachable parts of the type graph and type id renumbering. * tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. * tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1-report-0.txt: Likewise. * tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise. * tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise. * tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise. * tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise. Signed-off-by: Giuliano Procida <gprocida@google.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com> |
||
Giuliano Procida
|
386fede9ce |
DWARF: look up DW_AT_declaration non-recursively
Bug 26297: Possible misinterpretation of DW_AT_declaration via DW_AT_specification The DWARF attribute DW_AT_declaration indicates that a DIE is "declaration only". DWARF DIEs can be linked with DW_AT_specification and DW_AT_abstract_origin attributes, effectively combining them. A lone DW_AT_declaration in a chain of such DIEs should not render the whole chain declaration only. The function die_is_declaration_only currently traverses such links in search of the attribute which precludes being able to check for the attribute at each DIE in the chain and some DIEs are mistakenly treated as declaration-only by the DWARF reader. This commit changes die_is_declaration_only to examine the given DIE only. It extends the die_flag_attribute function so that it can perform a direct as well as a recursive attribute search. The function die_die_attribute's 'look_thru_abstract_origin' argument is renamed to 'recursively' to match. A following commit will change the DWARF reader to ensure it takes note of the declaration-only status of all DIEs in a chain. * src/abg-dwarf-reader.cc (die_die_attribute): Rename 'look_thru_abstract_origin' argument to 'recursively' and mention DW_AT_specification in its doc comment. Remove stale comment for non-existent argument. Simplify code with the help of the ternary operator. (die_flag_attribute): Add recursively argument, defaulted to true. If this is false, look for attribute using dwarf_attr rather than dwarf_attr_integrate. (die_is_declaration_only): Call die_flag_attribute specifying non-recursive attribute search. * tests/data/test-annotate/test15-pr18892.so.abi: Update tests. This is mostly the removal of unreachable parts of the type graph and type id renumbering. * 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-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1-report-0.txt: Likewise. * tests/data/test-read-dwarf/test15-pr18892.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/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. Signed-off-by: Giuliano Procida <gprocida@google.com> |
||
Giuliano Procida
|
f18a192aa6 |
Improve documentation of abidiff --type-id-style
* tools/abidw.cc (display_usage): In documentation of "--type-id-style" option, add a missing closing ')', spell "type id" without a '-', split overly long string over two lines, use "<...>" to indicate mandatory argument and improve description of formats. * doc/manuals/abidw.rst: In documentation of "--type-id-style" option, use "<...>" to indicate mandatory argument. Signed-off-by: Giuliano Procida <gprocida@google.com> |
||
Giuliano Procida
|
dfd410c585 |
Fix maybe_report_data_members_replaced_by_anon_dm
The function maybe_report_data_members_replaced_by_anon_dm has a bug and some minor issues. This commit tidies these up. The function came to my attention when a broken equality test triggered an infinite loop. The issues were: - dms_replaced_by_same_anon_dm declared outside loop, gets reused - self-comparison of first decl, potential infinite loop - anonymous_data_member, assigned but not used - two iterators i, j used, when one would suffice The first issue results in incorrect diff reports if data members in a structure are replaced by more than one anonymous data member. This commit adds additional test cases for this, following the pattern used for the existing PR25661 ones. The second issue only affects behaviour if equality is defined inconsistently with object identity. * src/abg-reporter-priv.cc (maybe_report_data_members_replaced_by_anon_dm): Move declarations of anonymous_data_member and dms_replaced_by_same_anon_dm into inner loop. Use anonymous_data_member for testing and reporting, allowing iterators i and j to be replaced by just iterator i. Push first decl onto dms_replaced_by_same_anon_dm unconditionally and move control flow logic into loop condition. * tests/data/Makefile.am: Add new test cases. * tests/data/test-diff-filter/test-PR25661-7-report-1.txt: New test case file. * tests/data/test-diff-filter/test-PR25661-7-report-2.txt: Likewise. * tests/data/test-diff-filter/test-PR25661-7-report-3.txt: Likewise. * tests/data/test-diff-filter/test-PR25661-7-report-4.txt: Likewise. * tests/data/test-diff-filter/test-PR25661-7-v0.c: Likewise. * tests/data/test-diff-filter/test-PR25661-7-v0.o: Likewise. * tests/data/test-diff-filter/test-PR25661-7-v1.c: Likewise. * tests/data/test-diff-filter/test-PR25661-7-v1.o: Likewise. * tests/test-diff-filter.cc: Call new test cases. Signed-off-by: Giuliano Procida <gprocida@google.com> |