CTF (C Type Format) is a lightweight debugging format that provides
information about C types and the association between functions and
data symbols and types. It is designed to be very compact and
simple. More can be learned about it at https://ctfstd.org.
This patch introduces support in libabigail to extract ABI information
from CTF stored in ELF files.
A few notes on this implementation:
- The implementation is complete in terms of CTF support. Every CTF
feature is processed and handled to generate libabigail IR. This
includes basic types, typedefs, pointer, array and struct types.
The CTF record of data objects (variables) and functions are also
used in order to generate the corresponding libabigail IR artifacts.
- The decoding of CTF data is done using the libctf library which is
part of binutils. In order to link with it, binutils shall be built
with --enable-shared for libctf.so to become available.
- This initial implementation is aimed to simplicity. We have not
tried to resolve any and every corner case that may require special
handling. We have observed that the DWARF front-end (which is
naturally way more complex as the scope is way bigger) is plagued
with hacks to handle such situations. However, for the CTF support
we prefer to proceed in a simpler and more modest way: we will
handle these problems if/when we find them. The fact that CTF only
supports C (currently) certainly helps there.
- Likewise, in this basic support we are not handling symbol
suppressions or other goodies that libabigail provides. We are new
to libabigail and ABI analysis, and at this point we simply don't
have a clear picture about what is most useful/relevant to support
or not. With the maintainer's blesssing, we will tackle that
functionaly after this basic support is applied upstream.
- The implementation in abg-ctf-reader.{cc,h} is pretty much
self-contained. As a result there is some duplication in terms of
ELF handling with the DWARF reader, but since that logic is very
simple and can be easily implemented, we don't consider this to be a
big deal (for now.) Hopefully the maintainers agree.
- The libabigail tools assume that ELF means to always use DWARF to
generate the ABI IR. We added a new command-line option --ctf to
the tools in order to make them to use the CTF debug info instead.
We are definitely not sure whether this is the best user interface.
In fact I would be suprised if it was ;)
- We added support for --ctf to both abilint and abidiff. We are not
sure whether it would make sense to add support for CTF to the other
tools. Feedback welcome.
- We are pondering about what to do in terms of testing. We have
cursory tested this implementation using abilint and abidiff. We
know we are generating IR corpus that seem to be ok. It would be
good however to be able to run the libabigail testsuites using CTF.
However the testsuites may need some non-trivial changes in order to
make this possible. Let's talk about that :)
* configure.ac: Check for libctf.
* src/abg-ctf-reader.cc: New file.
* include/abg-ctf-reader.h: Likewise.
* src/Makefile.am (libabigail_la_SOURCES): Add abg-ctf-reader.cc
conditionally.
* include/Makefile.am (pkginclude_HEADERS): Add abg-ctf-reader.h
conditionally.
* tools/abilint.cc (struct options): New option `use_ctf'.
(display_usage): Documentation for --ctf.
(parse_command_line): Handle --ctf.
(main): Honour --ctf.
* tools/abidiff.cc (struct options): New option `use_ctf'.
(display_usage): Documentation for --ctf.
(parse_command_line): Handle --ctf.
(main): Honour --ctf.
* doc/manuals/abidiff.rst: Document --ctf.
* doc/manuals/abilint.rst: Likewise.
Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When using abicompat, if the uses the --weak-mode option and also
provides a lib2 path on the command line, the lib2 path is silently
ignored.
This patch provides a warning to notify the user that the lib2 path is
ignored in that case.
* tools/abicompat.cc (main): Notify the user when the path to
the second library is ignored because the --weak-mode option
was provided. Also, fix comment.
Signed-off-by: tangmeng <tangmeng@uniontech.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When using the abilint command, several problems were found:
1.When abilint prints its version information, it does not terminate
it with a newline.
2.There is a spelling error, the path is mistakenly written as patch.
3.There are extra fields in the help option.
4.Inappropriate and confusing option description.
* tools/abilint.cc (display_usage): Correct the errors and
redundant content in the help information.
(main): Add a newline after version string.
Signed-off-by: tangmeng <tangmeng@uniontech.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When abicompat prints its help information, it does not terminate
it with a newline and option format is not aligned the way that
other commands do.
* tools/abicompat.cc (display_usage): End the usage message with a
newline and properly indent it.
Signed-off-by: tangmeng <tangmeng@uniontech.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When doing self-comparison check of
/usr/lib64/libjavascriptcoregtk-4.0.so.18.18.7 from
webkit2gtk3-jsc-2.32.3-1.fc34.x86_64, reading back the abixml file
fails because an empty typedef is used as the element type of an
array.
The empty typedef is there (in a transient manner) because the typedef
is being built. First an empty typedef is built and then its
underlying type is built. During the construction of the underlying
type however (an enum), the empty typedef itself is used (as the
naming typedef of the enum). But because its empty, an assert is
violated during the construction of an array which element type is the
(empty) typedef. A snake eating its own (half-baked) tail, so to
speak.
The patch fixes the issue by constructing the underlying (enum) type
first. Once its constructed, then it's used to construct the typedef
which is thus never empty, even in a transient manner.
The patch adjusts the building of enums so that the naming typedef is
built only once the enum itself is fully constructed. This breaks the
vicious cycle exposed above.
The offending RPM is too big to be added to the test suite. Which
argues (yet again) for the implementation of a separate test suite
that runs libabigail tests on a huge pile of RPMs without having to
embed them in the tarball. We really ought to start that project.
* src/abg-reader.cc (build_enum_type_decl): Set the naming typedef
only after the enum is created and keyed.
(build_typedef_decl): Build the underlying type of the typedef
first.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
These are the updates:
AlignConsecutiveDeclarations: false
- the dominant style in libabigail is not to align
AllowShortBlocksOnASingleLine: Always
AllowShortEnumsOnASingleLine: true
AllowShortFunctionsOnASingleLine: All
AllowShortLambdasOnASingleLine: All
- the libabigail style favours short things on a single line
Cpp11BracedListStyle: true
- this seems to improve some initialiser syntax
BinPackArguments: false
- we already turn this off for parameters
SpaceAfterCStyleCast: true
- this is the libabigail style
* .clang-format: Various tweaks to Clang format configuration.
Signed-off-by: Giuliano Procida <gprocida@google.com>
I forgot to add the wireshark-cli-3.4.9-1.fc36.x86_64.rpm debug info
package for the test entry that uses it in tests/test-diff-pkg.cc.
It's not necessary on the x86-64 platform, but on many others, it
seems the alternate debug info contained in that package is needed.
So I am adding it in here.
* tests/data/test-diff-pkg/wireshark/wireshark-debuginfo-3.4.9-1.fc36.x86_64.rpm:
Add new debug info package.
* tests/data/Makefile.am: Add it to the source distribution.
* tests/test-diff-pkg.cc: Use the new debug info package in the
test harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In this case, thanks to all the debugging infrastructure in place,
especially the canonical type debugging infrastructure, I was able to
notice that there was a canonicalization error on a function type when
reading the libwiretap binary as in:
$ build/tools/abidw --debug-tc /usr/lib64/libwiretap.so.11.0.8
structural & canonical equality different for type: function type void (wtap*)
in compare_types_during_canonicalization at: /home/dodji/git/libabigail/PR28364/src/abg-ir.cc:13575: execution should not have reached this point!
Abandon (core dumped)
$
When digging deeper, I noticed that, in the DWARF reader, when
building a function type, we are associating a "textual
representation" of the function type 'void (wtap*)' to its DIE (inside
the current translation) way too early.
By too early, I mean, the association was done before the function
type was fully 'built'. Its parameters were not 'collected', for
instance. So that means that a 'pointer to that function type' could
be formed, with a wrong representation, during the time where the
function type wasn't fully formed. Just moving the association to
after the type was fully constructed solved the issue.
This one was hard to spot!
Later, this uncovered the fact that we could now have (and thus
serialize) member functions of unions. And it turned out the abixml
reader didn't expect those. Oops. I fixed that one as well.
* src/abg-dwarf-reader.cc (build_function_type): Associate the DIE
representation to the constructed type once it's fully built.
* src/abg-reader.cc (build_function_type): Support member function of unions.
* tests/data/Makefile.am: Add the new test input files to the
source distribution.
* tests/data/test-diff-pkg/wireshark/wireshark-cli-3.4.9-1.fc36.x86_64-self-check-report.txt:
Add new test input file.
* tests/data/test-diff-pkg/wireshark/wireshark-cli-3.4.9-1.fc36.x86_64.rpm:
Likewise.
* tests/data/test-diff-pkg/wireshark/wireshark-cli-debuginfo-3.4.9-1.fc36.x86_64.rpm:
Likewise.
* tests/test-diff-pkg.cc (in_out_specs): Add these new test input
files to this test harness.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When looking into something else, I noticed that when emitting the
'naming-typedef' property of class, the typedef wasn't categorized as
a referenced type. So sometimes the writer could forget to emit the
naming typedef itself later. Fixed thus.
* src/abg-writer.cc (write_naming_typedef): Notice that the naming
typedef is referenced.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When looking into something else, I noticed that we were forgetting
when array subranges are emitted. Fixed thus.
* src/abg-writer.cc (write_array_subrange_type): Record the type
as emitted.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While looking into something else, I noticed that sometimes the writer
would forget to emit types referenced by function types. Fixed thus.
* src/abg-writer.cc (write_referenced_types): Factorize out of ...
(write_translation_unit): ... here. Also, use it to write the
types referenced by emitted function types.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Fix a nagging warning coming from referencing test binaries that are
no more present.
* tests/Makefile.am: Stop referring to test-dot.cc which is no
more.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This configure option adds the possibility to debug the type
canonicalization process specifically.
When this new configure option is turned on, in
ir::get_canonical_type_for, when the type T, candidate for
canonicalization is compared to a given canonical type C, the
comparison is done twice; once using structural equality and once
using canonical equality whenever it's possible. For all the
sub-types of T and C, structural equality and canonical equality must
yield the same result. Otherwise, an error message is emitted and the
process aborts.
This all happens when using the abidw program with the --enable-tc or
--enable-type-canonicalization option.
This has proven to be very helpful to detect type canonicalization issues.
For instance, here is a trace of canonicalization issue that was
detected thanks to this patch:
$ build/tools/abidw --debug-tc /usr/lib64/libwiretap.so.11.0.8
structural & canonical equality different for type: function type void (wtap*)
in compare_types_during_canonicalization at: /home/dodji/git/libabigail/PR28364/src/abg-ir.cc:13575: execution should not have reached this point!
Abandon (core dumped)
This means that right after canonicalizing the type "void (wtap*)",
structural and canonical equality yield different results. So it
means there is a problem with that type specifically that makes its
canonicalization "go wrong". This requires further debugging to
understand, but at least, we are super close to the root cause of the
canonicalization problem.
* configure.ac: Support the new
--enable-debug-type-canonicalization option. Define macro
WITH_DEBUG_TYPE_CANONICALIZATION accordingly.
* doc/manuals/abidw.rst: Update documentation.
* include/abg-ir.h
(environment::debug_type_canonicalization_is_on): Declare new
member function if WITH_DEBUG_TYPE_CANONICALIZATION is defined.
* src/abg-ir-priv.h
(environment::priv::{use_canonical_type_comparison_,
debug_type_canonicalization_}): Define new data members if
WITH_DEBUG_TYPE_CANONICALIZATION is defined.
(environment::priv::priv): Initialize them.
* src/abg-ir.cc (try_canonical_compare): When
WITH_DEBUG_TYPE_CANONICALIZATION is defined, perform comparison
using either structural or canonical equality depending on the
environment::priv::use_canonical_type_comparison_ flag.
(environment::debug_type_canonicalization_is_on): Define member
function when WITH_DEBUG_TYPE_CANONICALIZATION is defined.
(compare_types_during_canonicalization): Define new function.
(type_base::get_canonical_type_for): Use the new function
compare_types_during_canonicalization.
* tools/abidw.cc (options::debug_type_canonicalization): Define
new data member.
(option::option): Initialize it.
(display_usage): Add help string for --debug-tc.
(parse_command_line): Support new option --debug-tc or
--debug-type-canonicalization.
(load_corpus_and_write_abixml): Turn type canonicalization
debugging on if --enable-tc is provided.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When debugging an issue uncovered by performing self comparison (abidw
--abidiff <binary>) I realized that I needed a stronger verification
of canonical types changing between type serialization and type
de-serialization. Namely, when a type T with canonical type C is
serialized, its de-serialized type should still have the same
canonical type C. Otherwise, it means some "type instability" took
place during serialization and de-serialization.
This patch implements that verification and also cleans up things
that came across while working on adding this debugging check.
* include/abg-fwd.h (is_non_canonicalized_type): Declare new
function.
* src/abg-ir-priv.h: Include abg-corpus.h
(environment::priv::pointer_type_id_map_): Fix comment.
(environment::priv::check_canonical_type_from_abixml_during_self_comp):
Define new member function.
* src/abg-ir.cc (unmark_types_as_being_compared): Factorize this
from ...
(return_comparison_result): ... here. Also, add a parameter to
control whether this function should perform the "canonical type
propagation optimization" or not. By default the optimization is
performed. This can be changed for debugging purposes later.
(type_base::get_canonical_type_for): Re-organise the self
comparison debugging process to invoke the new function
environment::priv::check_canonical_type_from_abixml_during_self_comp
each time a canonical type is computed, in addition to doing the
previous verification that was done when no canonical type was
found. Emit better error messages.
(is_non_canonicalized_type): Rename the static function
is_allowed_non_canonicalized_type into this and make it
non-static.
(hash_as_canonical_type_or_constant): Adjust.
* src/abg-reader.cc (maybe_map_type_with_type_id): Define new
static function.
(read_context::maybe_check_abixml_canonical_type_stability):
Ignore types that were not canonicalized.
(read_corpus_from_input): Set the origin of the corpus early
enough so that it's available to the canonicalizer even for types
being canonicalized early.
(MAYBE_MAP_TYPE_WITH_TYPE_ID): Factorize this macro out of ...
(build_type): ... this. That macro is defined only when debugging
self comparison.
(build_array_type_def): Map the read subrange type with its
type-id.
(handle_{type_decl, qualified_type, pointer_type_def,
reference_type_def, function_type, array_type_def,enum_type_decl,
typedef_decl, class_decl, union_decl}): Map the read type with its
type-id.
(load_canonical_type_ids): Ignore non-canonicalized types that
which ids were saved in the type-id file.
* src/abg-writer.cc (write_type_record): Factorize from ...
(write_canonical_type_ids): ... here. Don't forget to write the
type-ids of decl-only types. This can be useful for eye
inspection.
* tools/abidw.cc (load_corpus_and_write_abixml): Wait until the
end of the function before removing the type-id file. This can be
useful for eye inspection.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The complete and deleting C++ destructors have the same signature.
Because the dwarf-reader re-uses the IR of functions that have the
same signature, it can happen that one of the two destructors of a
class is missed and thus not represented in the IR. When these
destructors are virtual, that can have an impact on class comparison,
because virtual member functions are take part in class comparison,
just like data member and unlike non-virtual member functions.
This patch fixes the build_or_get_fn_decl_if_not_suppressed to avoid
"reusing" virtual destructors, based on their signature when several
are present. Instead an IR is built for all virtual destructors that
are seen.
* src/abg-dwarf-reader.c (build_or_get_fn_decl_if_not_suppressed):
Do not try to re-use a virtual destructor of a class, based on its
signature. Several different of these can have the same
signature, inside a given class.
* tests/data/test-types-stability/PR27086-libstdc++.so.6.0.26:
Add new binary test input.
* tests/data/Makefile.am: Add the new test input to source
distribution.
* tests/test-types-stability.cc (elf_paths): Add the test input
above to this harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
I stumbled upon some mis-indented function declaration while looking
at something else. Fixed thus.
* src/abg-dwarf-reader.cc (finish_member_function_reading): Fix
indentation.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Sometimes, in DWARF, a given class can even be defined piece-wise
across several DIEs. The first DIE would defined some properties and
subsequent DIEs would define others. dwarf-reader already supports
this for most properties. Some properties however can be duplicated
across two DIES.
For instance, a DIE describing a class 'C' can define a virtual member
function, and then a subsequent different DIE further describing other
properties of the same class C would define the same virtual member
function again. In that case, we should not define the virtual member
function twice in the IR of C that is being built.
Libabigail is failing to do exactly that. It's representing the
virtual member function of C twice in this case.
* src/abg-dwarf-reader.cc (fixup_functions_with_no_symbols): When
the function decl is finally associated to its (publicly defined)
ELF symbol, mark it as being exported.
(finish_member_function_reading): Don't risk marking a virtual
function as being non-virtual when updating its properties.
(build_or_get_fn_decl_if_not_suppressed): Update comment. If the
member function is already present in the class, do not create a
new one; rather, reuse the existing one. It's going to be later
updated by finish_member_function_reading.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Adjust.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: 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/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/PR22015-libboost_iostreams.so.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.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.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* tools/abipkgdiff.cc (compare_prepared_userspace_packages):
Removing working directories "early" prevents e.g,
dwarf_reader::get_soname_of_elf_file from accessing those files.
So do not remove them until the very end.
* tests/data/test-diff-pkg/libxcrypt-4.1.1-6.el8.x86_64--libxcrypt-4.1.1-6.el8.x86_64-output-1.txt:
Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Consider these two anonymous enum declarations in these different contexts:
enum {A0 = 0; A1 = 1;} global0; // 1/: anonymous enum
typedef enum {E0 = 0; E1 = 1;} E; // 2/: anonymous enum named by a typedef.
E global0;
In the first context "1/", the type of the global variable is an
anonymous enum that is used as such.
In the second context "2/", the type of the global variable is an
anonymous type that is named by the typedef 'E'. So then, it's E that
is used to designate the enum. The anonymous type is thus never used
directly. In essence, it's the same thing as if it was declared as
enum E {E0 = 0; E1 = 1;};
Right now, libabigail canonicalizes the enum 1/ and 2/ together and
that results in 1/ being canonically equal to 2/.
So, when saving the corpus into abixml, because enum 1/ and enum 2/ can be
used interchangeably, either 1/ or 2/ is going to be saved. That
can result in spurious change reports in which the reporter refer to
the enum 1/ where it should refer to enum 2/ or vice versa.
Intrinsically, the enum 1/ and enum 2/ are different because one
essentially has a name (provided by a typedef) and the second does
not. One is anonymous whereas the second is not, essentially.
At the moment, libabigail supports typedef-named anonymous classes.
But it doesn't support this concept for enums.
This patch extends that concept to enums as well. It makes it so that
any anonymous type can now by typedef-named. In that case, the type
now looks like it has a name which is the typedef name. The
information about the typedef naming a given type is kept and
serialized into abixml.
Thus with this patch, the enum in 1/ is now considered (canonically)
different from enum 2/. So there is no possible confusion once the
type is serialized into abixml.
* include/abg-fwd.h (scope_anonymous_or_typedef_named)
(is_anonymous_or_typedef_named): Declare new functions.
* include/abg-ir.h (decl_base::set_has_anonymous_parent): Remove
declaration.
(decl_base::{get,set}_naming_typedef): Declare new member
functions.
* src/abg-ir.cc (update_qualified_name): Define static function.
(decl_base::priv::naming_typedef_): Define new data member.
(decl_base::priv::has_anonymous_parent_): Remove data member.
(decl_base::priv::priv): Adjust constructor.
(decl_base::get_has_anonymous_parent): Rather than storing a flag
for this, dynamically look at if the scope is anonymous.
(decl_base::set_has_anonymous_parent): Remove definition.
(decl_base::{get,set}_naming_typedef): Define new member
functions.
(scope_anonymous_or_typedef_named)
(is_anonymous_or_typedef_named): Define new functions.
(get_decl_name_for_comparison): Define new sub-routine for the
decl_base overload of equals.
(equals): In the overload for decl_base, use the new
get_decl_name_for_comparison. It helps to ensure that all
anonymous decls of the same kind have the same name for the
purpose of comparison. It also ensures that non anonymous decls
that are part of anonymous scopes should be compared only by
looking at their non-qualified names. In the overload for
class_or_union, adjust.
(scope_decl::add_member_decl): No more need to flag the fast that
the parent scope is anonymous here.
(get_debug_representation): Fix a thinko.
(class_or_union::get_naming_typedef): Remove member function as
it's now handled by decl_base::get_naming_typedef.
* src/abg-dwarf-reader.cc (build_typedef_type): When a typedef is
a naming typedef, then mark the named decl as being typedef-named.
(maybe_canonicalize_type): Delay canonicalization of anonymous
types because they can be typedef-named later.
* src/abg-reader.cc (read_naming_typedef_id_string)
(maybe_set_naming_typedef): Define new static function.
(build_class_decl): Use it here, rather than reading the
"naming-typedef-id" by hand.
(build_enum_type_decl, build_union_decl): Read the
"naming-typedef-id" property.
* src/abg-writer.cc (write_naming_typedef): Make this accept
decl_base_sptr, rather than just class_decl_sptr.
(write_enum_type_decl): Write the naming-typedef-id property if
needed.
* tests/data/test-abidiff-exit/test-PR28316-report.txt: New test
reference output.
* tests/data/test-abidiff-exit/test-PR28316-v{0,1}.cc: Source code
of new binary test input.
* tests/data/test-abidiff-exit/test-PR28316-v{0,1}.o: New binary
test input files.
* tests/data/Makefile.am: Add the new test files to the source
distribution.
* tests/test-abidiff-exit.cc: Add the new test files above to this
harness.
* tests/data/test-annotate/libtest23.so.abi: Adjust.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-annotate/test13-pr18894.so.abi: Likewise.
* tests/data/test-annotate/test14-pr18893.so.abi: Likewise.
* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-diff-dwarf/test15-enum-report.txt: Likewise.
* tests/data/test-diff-filter/test19-enum-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/PR24690/PR24690-report-0.txt: Likewise.
* tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.txt:
Likewise.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-0.txt:
Likewise.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-1.txt:
Likewise.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Likewise.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt:
Likewise.
* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi:
Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/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/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When two packages are different just because one adds or removes
binaries -- and no binary have any ABI change otherwise, abipkgdiff
quits early and doesn't report the added and removed binaries.
This patch fixes the issue by reporting added/removed binaries even
when no ABI comparison took place.
* tools/abipkgdiff.cc (compare_prepared_userspace_packages): Do
not return early if there are no binaries to compare. Also add
more verbose messages.
* tests/data/test-diff-pkg/libxcrypt-4.1.1-6.el8.x86_64--libxcrypt-4.1.1-6.el8.x86_64-output-1.txt:
New reference output file.
* tests/data/test-diff-pkg/libxcrypt-4.1.1-6.el8.x86_64--libxcrypt-compat-4.4.18-3.el9.x86_64-report-1.txt:
New reference output file.
* tests/data/test-diff-pkg/libxcrypt-4.1.1-6.el8.x86_64.rpm: New
binary input file.
* tests/data/test-diff-pkg/libxcrypt-4.4.18-3.el9.x86_64.rpm: Likewise.
* tests/data/test-diff-pkg/libxcrypt-compat-4.4.18-3.el9.x86_64.rpm: Likewise.
* tests/data/test-diff-pkg/libxcrypt-compat-debuginfo-4.4.18-3.el9.x86_64.rpm: Likewise.
* tests/data/test-diff-pkg/libxcrypt-debuginfo-4.1.1-6.el8.x86_64.rpm: Likewise.
* tests/data/test-diff-pkg/libxcrypt-debuginfo-4.4.18-3.el9.x86_64.rpm: Likewise.
* tests/data/Makefile.am: Add the new testing files to source
distribution.
* tests/test-diff-pkg.cc (in_out_specs): Add these binary packages
to this testing harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Reading size and alignment from abixml can lead to loss of precision
that surfaced when self comparing the protobuf package as described in
bug https://bugzilla.redhat.com/show_bug.cgi?id=1944102.
Fixed thus.
* src/abg-reader.cc (read_size_and_alignment): Use atoll to read
long long values, not atoi.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In the libabigail type system, the void type is a synthetic type and
is thus not canonicalized.
We forgot to mention this "void type" to
hash_as_canonical_type_or_constant as being one of the types that are
allowed to be non canonicalized in the system. This omission violates
an assert in that function.
The patch introduces a new is_allowed_non_canonicalized_type
subroutine that defines the types that are allowed to be non
canonicalized in the system and make it recognize "void type" as such.
hash_as_canonical_type_or_constant uses the new
is_allowed_non_canonicalized_type.
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1951496
* src/abg-ir.cc (is_allowed_non_canonicalized_type): Define new
static function.
(hash_as_canonical_type_or_constant): Use it.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
referenced_type_should_be_emitted should not crash on types with
associated translation unit, like e.g, "void type".
Fixed thus.
* src/abg-writer.cc (referenced_type_should_be_emitted): Don't
crash on types with no associated translation unit.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
We build qualified types today by building a temporary qualified type
of "void" (one that has no underlying type), then the underlying type
is built, and once it's built, it's set to the qualified type.
We do this so that if during the construction of the underlying type,
the proper qualified type is needed (by recursion) then the temporary
type is found and used. We used this strategy recently as a temporary
measure until the canonical type propagation algorithm is fixed. And
now it seems fixed.
The problem with that temporary approach is that in some rare cases,
the temporary qualified void type can be used elsewhere and violate
assertions that expect a proper qualified type.
The patch thus creates the underlying type first. If the qualified
type is created (by recursion) in the process, we use it. Otherwise,
the qualified type is later created with the proper underlying type
that is already available.
This helps to fix https://bugzilla.redhat.com/show_bug.cgi?id=1951501.
* src/abg-reader.cc (build_qualified_type_decl): Create the
underlying type first, then create the qualified type.
This helps fix bug
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
During the canonical type propagation optimization, when the
comparison of two type sub-objects fails, we need to cancel the
(potential) propagation of the canonical type of the current type
sub-object being compared.
We were not doing that in return_comparison_result, but were expecting
it. Oops.
Fixed thus.
This helps to fix bug https://bugzilla.redhat.com/show_bug.cgi?id=1951501.
* src/abg-ir.cc (return_comparison_result): When the comparison of
the current type sub-object fails, clear the potentially
propagated canonical type and remove it from the set of types with
non confirmed propagated canonical types.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While looking at something else, I noticed an occurrence of infinite
loop during type canonicalization, especially when cancelling
canonical type propagation on some types.
Fixed thus.
This helps address https://bugzilla.redhat.com/show_bug.cgi?id=1951501
* src/abg-ir-priv.h
(environment::priv::collect_types_that_depends_on): Don't try to
collect a type that has already been collected.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While looking at something else, I stumbled across this bug where the
linkage name of enum are not escaped in abixml. So "forbidden"
characters like '<' can snick in.
Fixed thus.
This helps address https://bugzilla.redhat.com/show_bug.cgi?id=1951501
* src/abg-writer.cc (write_enum_type_decl): Escape linkage name.
When reading the abixml representing an enumerator which value is
exactly either LLONG_MIN or LLONG_MAX, build_enum_type_decl fails
because we wrongly think that an underflow or overflow happened, while
using strtoll.
This patch fixes the condition used to detect {under,over}flow
whenusing strtoll.
* src/abg-reader.cc (build_enum_type_decl): When strtoll detects
an underflow or overflo, it sets errno to ERANGE. So take that
into account.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Reporting the change in array type exhibits a glitch in the type name.
As the bug report says:
The resulting abidiff output contains:
type of 'int numbers[2]' changed:
type name changed from 'void[2]' to 'void[3]'
array type size changed from 64 to 96
array type subrange 1 changed length from 2 to 3
instead of
type of 'int numbers[2]' changed:
type name changed from 'int[2]' to 'int[3]'
array type size changed from 64 to 96
array type subrange 1 changed length from 2 to 3
The problem comes from array_type_def::get_qualified_name() where we
fail to generate a "new" qualified name once the type of the array is
canonicalized.
Fixed thus.
* src/abg-ir.cc (array_type_def::get_qualified_name): Use the
cache for temporary qualified names when the type is not yet
canonicalized. That way, the cache for (non-temporary) qualified
names is used only for canonicalized types.
* tests/data/test-abidiff/test-PR27985-report.txt: Reference
output for the new test.
* tests/data/test-abidiff/test-PR27985-v{0,1}.c: Source code for
the new test binary inputs.
* tests/data/test-abidiff/test-PR27985-v{0,1}.o: New test binary inputs.
* tests/data/test-abidiff/test-PR27985-v{0,1}.o.abi: New test
abixml input.
* tests/data/Makefile.am: Add the new test materials above to
source distribution.
* tests/test-abidiff.cc (specs): Add the tests above to the harness.
* tests/data/test-diff-pkg/nss-3.23.0-1.0.fc23.x86_64-report-0.txt:
Adjust.
* tests/data/test-abidiff-exit/qualifier-typedef-array-report-1.txt:
Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When determining whether a referenced type should be emitted, various
tests are done:
- has the type been emitted already? hash table lookup
- does the translation unit match? string comparison
- is this the last translation unit? read bool variable
The translation unit tests were added in recent commits and followed
the hash table lookups. This resulted in a performance regression
affecting Android continuous integration tests.
The lookups require a hash calculation and an equality check if the
hash is present. The equality checks are expensive deep equalities
rather than pointer comparisons.
This change reorders the tests so that the lookups happen last. This
speeds up abidw by more than a factor of 10 for one Android library.
* src/abg-writer.cc (write_translation_unit): Reorder
referenced type emission tests for efficiency. Consolidate
related comments.
Signed-off-by: Giuliano Procida <gprocida@google.com>
Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
An anonymous struct/union is, by definition an entity that is not
named (unless a naming typedef is provided for it).
It turns out that in C++ binaries, there are anonymous types that are
logically equivalent (as far as ABI is concerned) because they have
the same members and layout, but turn out to be evaluated as being
different because they are defined in different name spaces. And
because they are not named, showing them as being different just
because of their name space doesn't bring anything but spurious error
reporting.
Consider the DWARF representing this:
struct S
{
union
{
int a;
int b;
} member;
};
where the 'member' is of type S::<anonymous-union>.
Probably due to LTO, we see some DWARF that represents the type of
'member' as just <anonymous-union>, in some translation units.
I could not generate that DWARF from a small test case, myself. But
it comes from the binary 'usr/bin/lto-dump', from the
https://bugzilla.redhat.com/show_bug.cgi?id=1925886 problem report.
So in that case, we want the S::<anonymous-union> to compare equal to
the <anonymous-union>, otherwise, this produces spurious type changes,
especially when doing self comparison.
This is what this patch does.
* include/abg-fwd.h (is_anonymous_type): Constify this function.
* src/abg-ir.cc (equals): In the overload for decl_base, do not
take scope of anonymous types into account. In the overload for
array_type_def do not peel of typedefs. This is not directly
related to anonymous types, but it make comparison more robust
against naming typedefs used for anonymous types in array
elements.
(get_type_name): Do not take into account the scope of anonymous
types when building internal representation of types. Note that
the internal representation is what is used for canonicalization.
This means that all anonymous types are compared against each
others during type canonicalization.
* src/abg-reader.cc (build_class_decl): Do not try to re-use
anonymous types, just like we already do for DWARF.
* tests/data/test-annotate/test17-pr19027.so.abi: Adjust.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
Likewise.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
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-read-dwarf/PR22122-libftdc.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/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/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/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>
Since we arranged to only emit referenced types in translation units
where they belong, it appears that in some cases we forget to emit
some referenced types.
This is because some referenced types might belong to a translation
unit that is *already* emitted by the time we detect that a type is
referenced.
To fix this correctly, we should probably have a pass that walks the
corpus to detect referenced types, so that we have their set even
before we start emitting translation units.
But for now, the patch just detects when we are emitting the last
translation unit. In that case all the non-emitted referenced types
are emitted. It doesn't seem to be an issue if those don't belong to
that translation unit, compared to their original (from the DWARF)
type.
* include/abg-writer.h (write_translation_unit): Add a new
parameter that says if we are emitting the last TU.
* src/abg-writer.cc (write_translation_unit::{type_is_emitted,
decl_only_type_is_emitted}): Constify these methods.
(write_context::has_non_emitted_referenced_types): Define new
member function using the const methods above.
(write_translation_unit): When emitting the last TU, emit all the
referenced types.
(write_corpus): Set signal when emitting the last translation
unit.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Some classes can be defined piece-wise, in some rare cases in the
abixml. build_class_decl is currently preventing that to happen,
leading to some spurious self comparison errors.
Fixed thus.
* src/abg-reader.cc (build_class_decl): Keep going when the class
has already been built. The rest of the code knows how to add new
stuff.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While working on another bug, it turned out the initial fix for the
bug https://sourceware.org/bugzilla/show_bug.cgi?id=27236 was just
papering over the real issue.
I think the real issue is that "canonical type propagation"
optimization was being done even in cases where it shouldn't have been
done. This patch recognizes the limits of that optimization and avoid
performing it when we are off limits.
So here is what that optimization is. The text below is also present
in the comments in the source code. I am putting it here to explain
the context.
During the canonicalization of a type T (which doesn't yet have a
canonical type), T is compared structurally (member-wise) against a
type C which already has a canonical type. The comparison expression
is C == T.
During that structural comparison, if a subtype of C (which also
already has a canonical type) is structurally compared to a subtype of
T (which doesn't yet have a canonical type) and if they are equal,
then we can deduce that the canonical type of the subtype of C is the
canonical type of the subtype of C.
Thus, we can canonicalize the sub-type of the T, during the
canonicalization of T itself. That canonicalization of the sub-type
of T is what we call "propagating the canonical type of the sub-type
of C onto the sub-type of T". It's also called "on-the-fly
canonicalization". It's on the fly because it happens during a
comparison -- which itself happens during the canonicalization of T.
So this is the general description of the "canonical type propagation
optimization".
Now we must recognize the limits of that optimization. Said
otherwise, there is a case when a type is *NOT* eligible to this
canonical type propagation optimization.
The reason why a type is deemed NON-eligible to the canonical type
propagation optimization is that it "depends" on a recursively present
type. Let me explain.
Suppose we have a type T that has sub-types named ST0 and ST1.
Suppose ST1 itself has a sub-type that is T itself. In this case, we
say that T is a recursive type, because it has T (itself) as one of
its sub-types:
T
+-- ST0
|
+-- ST1
| +
| |
| +-- T
|
+-- ST2
ST1 is said to "depend" on T because it has T as a sub-type. But
because T is recursive, then ST1 is said to depend on a recursive
type. Notice however that ST0 does not depend on any recursive type.
Now suppose we are comparing T to a type T' that has the same
structure with sub-types ST0', ST1' and ST2'. During the
comparison of ST1 against ST1', their sub-type T is compared
against T'. Because T (resp. T') is a recursive type that is
already being compared, the comparison of T against T' (as a
subtypes of ST1 and ST1') returns true, meaning they are
considered equal. This is done so that we don't enter an infinite
recursion.
That means ST1 is also deemed equal to ST1'. If we are in the
course of the canonicalization of T' and thus if T (as well as as
all of its sub-types) is already canonicalized, then the canonical
type propagation optimization will make us propagate the canonical
type of ST1 onto ST1'. So the canonical type of ST1' will be
equal to the canonical type of ST1 as a result of that
optmization.
But then, later down the road, when ST2 is compared against ST2',
let's suppose that we find out that they are different. Meaning
that ST2 != ST2'. This means that T != T', i.e, the
canonicalization of T' failed for now. But most importantly, it
means that the propagation of the canonical type of ST1 to ST1'
must now be invalidated. Meaning, ST1' must now be considered as
not having any canonical type.
In other words, during type canonicalization, if ST1' depends on a
recursive type T', its propagated canonical type must be
invalidated (set to nullptr) if T' appears to be different from T,
a.k.a, the canonicalization of T' temporarily failed.
This means that any sub-type that depends on recursive types and
that has been the target of the canonical type propagation
optimization must be tracked. If the dependant recursive type
fails its canonicalization, then the sub-type being compared must
have its propagated canonical type cleared. In other words, its
propagated canonical type must be cancelled.
This concept of cancelling the propagated canonical type when needed
is what this patch introduces.
New data members have been introduced to the environment::priv private
structure. Those are to keep track of the stack of sub-types being
compared so that we can detect if a candidate to the canonical type
propagation optimization depends on a recursive type.
There is also a data structure in there to track the targets of the
canonical type propagation optimization that "might" need to see their
propagated canonical types be cancelled.
Then new functions have been introduced to detect when a type depends
on a recursive type, to cancel or confirm propagated canonical types
etc.
In abg-ir.cc, The RETURN* macros used in the equals() overloads have
been factorized using the newly introduced function templates
return_comparison_result(). This now contains the book keeping that
was previously done (in the RETURN* macros) to detect recursive cycles
in the comparison, as well as triggering the canonical type
propagation. This i also where the logic of properly limiting the
optimization is implemented now.
* include/abg-ir.h (pointer_set): This typedef is now for an
unordered_set<uintptr_t> rather than an unordered_set<size_t>.
(environment::priv_): Make this public so that code in free form
function from abg-ir.cc can access it.
* src/abg-ir-priv.h (struct type_base::priv): Move this private
structure here, from abg-ir.cc.
(type_base::priv::{depends_on_recursive_type_,
canonical_type_propagated_}): Added these two new data members.
(type_base::priv::priv): Initialize the two new data members.
(type_base::priv::{depends_on_recursive_type,
set_depends_on_recursive_type,
set_does_not_depend_on_recursive_type, canonical_type_propagated,
set_canonical_type_propagated, clear_propagated_canonical_type}):
Define new member functions.
(struct environment::priv): Move this struct here, from abg-ir.cc.
(environment::priv::{types_with_non_confirmed_propagated_ct_,
left_type_comp_operands_, right_type_comp_operands_}): New data
members.
(environment::priv::{mark_dependant_types,
mark_dependant_types_compared_until, confirm_ct_propagation,
collect_types_that_depends_on, cancel_ct_propagation,
remove_from_types_with_non_confirmed_propagated_ct}): New member
functions.
* src/abg-ir.cc (struct environment::priv, struct)
(type_base::priv, struct class_or_union::priv): Move these struct
to include/abg-ir-priv.h.
(push_composite_type_comparison_operands)
(pop_composite_type_comparison_operands)
(mark_dependant_types_compared_until)
(maybe_cancel_propagated_canonical_type): Define new functions.
(notify_equality_failed, mark_types_as_being_compared): Re-indent.
(is_comparison_cycle_detected, return_comparison_result): Define
new function templates.
(RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED): Define new macro.
(equals(const function_type& l, const function_type& r)): Redefine
the RETURN macro using the new return_comparison_result function
template. Use the new RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED
and mark_types_as_being_compared functions.
(equals(const class_or_union& l, const class_or_union&, change_kind*)):
Likewise.
(equals(const class_decl& l, const class_decl&, change_kind*)):
Likewise. Because this uses another equal() function to compare
the class_or_union part the type, ensure that no canonical type
propagation occurs at that point.
(types_are_being_compared): Remove as it's not used anymore.
(maybe_propagate_canonical_type): Use the new
environment::priv::propagate_ct() function here.
(method_matches_at_least_one_in_vector): Ensure the
right-hand-side operand of the equality stays on the right. This
is important because the equals() functions expect that.
* src/abg-reader.cc (build_type): Ensure all types are
canonicalized.
* 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:
Likewise.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Likewise.
* tests/data/test-diff-pkg/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/test-libaaudio.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
There are several self comparison issues uncovered by comparing the
file test-PR27995.abi (provided in the bug report) against itself.
This patch address them all as well as the regressions induced on some
of the test suite and then and updates the other reference test suite
output that need it.
In the equals overload for decl_base, we compare the non-internal
versions of qualified decl names. For var_decls of anonymous class or
union types, the non-internal version is the flat-representation of
the type. Thus a benign change in a data member name of the anonymous
type might cause the equals function to consider the var_decls to be
wrongly different. The internal version of the qualified decl name
should return a name that is stable for types, irrespective of these
benign variations. The patch thus makes the equals overload for
decl_base to compare internal versions of qualified decl names instead.
The patch ensures that enum_type_decl::get_pretty_representation
return and internal pretty representation that is "stable" for
anonymous types. Basically, all anonymous enums will have the same of
name that looks like "__anonymous_enum__". This is to ensure two
things: first, that all anonymous enums are compared against each
other during type canonicalization, ensuring that when two anonymous
enums are canonically different, it really is because of changes in
their enumerators or basic type, not because of anything having to do
with their artificial names. Second, that in the equals overload for
decl_base, their internal qualified name always compare equal. This
nullifies the risk of having anonymous types compare different because
of their (non existent) name. This is because libabigail's dwarf
reader assigns artificial unique names to anonymous types, so we don't
want to use these during actual type comparison.
We do something similar for class_decl::get_pretty_representation and
union_decl::get_pretty_representation where the pretty internal
representation for class/union decl would now be
__anonymous_{struct,union}__.
The patch scouts the uses of get_pretty_representation() to make sure
to use avoid using the internal-form of the pretty representations
when it's not warranted. It also updates the doxygen comments of the
overloads of that function.
In the abixml reader, we were wrongly canonicalizing array types
early, even before they were fully created. The was leading to
spurious type chances down the road.
The patch also fixes the caching of the name of function types by
making it consistent with caching of the names of the other types of
the system. The idea is that we don't cache the name of a function
type until it's canonicalize. This is because the type might be
edited during its pre-canonicalization life time; and that editing
might change its name. However once the type is canonicalized, it
becomes immutable. At that point we can cache its name, for
performance purposes. Note that we need to do that both for the
"internal version" of the type name (used for canonilization purposes)
and the "non-internal version" one, which is used for other purposes.
This caching scheme wasn't respected for function types, so we were
caching a potentially wrong name for the type after its
canonicalization.
Last but not least, there is a problem that makes canonical type
comparison different from structural type comparison.
Let's consider these two declarations:
typedef int FirstInt;
typedef int SecondInt;
Now, consider these two pointer types: FirstInt* and SecondInt*;
These two pointer types are canonically different because they have
different type names. This is because during type canonicalization,
types with the same "pretty representation" are compared against each
other. So types with different type names will certainly have
different pretty representations and won't be compared; they are thus
going to have different canonical types.
However, FirstInt* and SecondInt* do compare equal, structurally,
because the equals overload for pointer_type_def compares the
pointed-to types of pointers by peeling off typedefs. So, here, as
both pointed-to types are 'int' when the typedefs are peeled off, the
two pointers structurally compare equal. This discrepancy between
structural and canonical equality introduces subtle and spurious type
changes depending on the order in which types are canonicalized. For
instance:
struct {FirstInt* m0;}; /* First type. */
struct {SecondInt* m0;}; /* Second type. */
If FirstInt* and SecondInt* are canonicalized before their containing
anonymous types, then the two anonymous types will compare different
(because FirstInt* and SecondInt* compare different) and have
different canonical types. If, however, the anonymous types are
canonicalized before FirstInt* and SecondInt*, then will compare equal
because FirstInt* and SecondInt* are structurally equivalent.
FirstInt* and SecondInt* will be canonicalized latter and have
different canonical types (because they have different type names)
despite being structurally equivalent.
The change in the order of canonicalization can happen when
canonicalizing types from a corpus coming from DWARF as opposed to
canonicalizing types from a corpus coming from abixml.
The patch fixes this discrepancy by not peeling off typedefs from the
pointed-to types when comparing pointers. Note that this makes us
regress on bug https://sourceware.org/bugzilla/show_bug.cgi?id=27236,
where the typedef peeling was introduced. In hindsight, introducing
that typedef peeling was a mistake. I'll try to address that bug
again in a subsequent patch.
* doc/manuals/abidiff.rst: Add documentation for the --debug
option.
* src/abg-ir.cc (equals): In the overload for decl_base consider
the internal version of qualified decl name. In the overload for
pointer_type_def do not peel typedefs off from the compared
pointed-to types. In the overload for typedef_decl compare the
typedef as a decl as well. In the overload for var_decl, compare
variables that have the same ELF symbols without taking into
account their qualified name, rather than their name. Stop
comparing data member without considering their names.
In the overload for class_or_union, when a decl-only class that is
ODR-relevant is compared against another type, assume that
equality if names are equal. This is useful in environments where
some TUs are ODR-relevant and others aren't.
(*::get_pretty_representation): Update doxygen comments.
(enum_type_decl::get_pretty_representation): Return an internal
pretty representation that is stable across all anonymous enums.
(var_decl::get_anon_dm_reliable_name): Use the non-internal pretty
representation for anonymous data members.
(function_type::priv::temp_internal_cached_name_): New data
member.
(function_type::get_cached_name): Cache the internal name after
the function type is canonicalized. Make sure internal name and
non-internal name are cached separately.
(class_or_union::find_anonymous_data_member): Look for the anonymous
data member by looking at its non-internal name.
({class, union}_decl::get_pretty_representation): Use something like "class
__anonymous_{union,struct}__" for all anonymous classes, so that they can
all be compared against each other during type canonicalization.
(type_has_sub_type_changes): Use non-internal pretty
representation.
(hash_type_or_decl, function_decl_is_less_than:): Use internal
pretty representation for comparison here.
* src/abg-reader.cc (read_context::maybe_canonicalize_type): Don't
early canonicalize array types.
* src/abg-writer.cc (annotate): Use non-internal pretty
representation.
* tests/data/test-diff-filter/test-PR27995-report-0.txt: New
reference report.
* tests/data/test-diff-filter/test-PR27995.abi: New test input
abixml file.
* tests/data/Makefile.am: Add test-PR27995.abi,
test-PR27995-report-0.txt to the source distribution.
* tests/data/test-annotate/libtest23.so.abi: Adjust.
* tests/data/test-diff-dwarf/test6-report.txt: Adjust.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt: Adjust.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt: Adjust.
* tests/data/test-diff-filter/test41-report-0.txt: Adjust.
* tests/data/test-diff-filter/test43-decl-only-def-change-leaf-report-0.txt: Adjust.
* tests/data/test-diff-filter/test8-report.txt: Adjust.
* tests/data/test-diff-pkg/libICE-1.0.6-1.el6.x86_64.rpm--libICE-1.0.9-2.el7.x86_64.rpm-report-0.txt:
Adjust.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-0.txt:
Adjust.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Adjust.
* 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.
* 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-suppr/test39-opaque-type-report-0.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/libtest23.so.abi: Adjust.
* tests/data/test-read-dwarf/test-libandroid.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/test9-pr18818-clang.so.abi: Adjust.
* tests/test-diff-filter.cc (in_out_specs): Add the
test-PR27995.abi to the test harness.
* tools/abidiff.cc (options::do_debug): New data member.
(options::options): Initialize it.
(parse_command_line): Parse --debug.
(main): Activate self comparison debug if the user provided
--debug.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
types_defined_same_linux_kernel_corpus_public() performs an
optimization while comparing two types in the context of the Linux
kernel. If two types of the same kind and name are defined in the
same corpus and in the same file, then they ought to be equal.
For two anonymous classes that have naming typedefs, the function
forgets to ensure that the naming typedefs have the same name.
I have no binary that exhibits the potential issue, but I stumbled
upon the problem while looking at something else that uncovered
the problem. This change doesn't impact any of the binaries of the
regression suite at the moment, though.
Fixed thus.
* src/abg-ir.cc (types_defined_same_linux_kernel_corpus_public):
Ensure that anonymous classes with naming typedefs have identical
typedef names.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In is_anonymous_data_member(), we only test that the name of the data
member is empty; we forget to test that decl_base::get_is_anonymous()
is true. This might make us wrongly think that a data member is
anonymous in cases like in the equals() function for var_decl, where
we temporarily set the name of the compared var_decl to "" before
invoking the decl_base::operator==. We do this to perform the
comparison by not taking into account the name of the variable.
This hasn't yet happened on the binaries of the regression test suite,
but it's definitely wrong so I am fixing it here.
* src/abg-ir.cc: (is_anonymous_data_member): Consider
decl_base::get_is_anonymous as well.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While looking at something else, I stumbled across some minor issues
in the debugging facilities I use to track self comparison problems.
I added a missing ABG_RETURN macro in the stack of equals() function
to better detect when there is a change, under the debugger.
I also fixed get_debug_representation() to properly display the
class/enum name (as expected) rather their pretty representation.
* src/abg-ir.cc (maybe_compare_as_member_decls): Add a missing
ABG_RETURN
(get_debug_representation): Display the name of class and enums,
not their pretty representation.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Bitfield and other member offsets can be specified in DWARF using:
- DW_AT_data_bit_offset, or
- DW_AT_data_member_location and optionally DW_AT_bit_offset.
The code would only use the value DW_AT_data_member_location if there
was no DW_AT_bit_offset. This commit fixes this and adjusts
documentation and affected tests.
* src/abg-dwarf-reader.cc (read_and_convert_DW_at_bit_offset):
Update documentation.
(die_member_offset): Treat DW_AT_bit_offset as an optional
adjustment to DW_AT_data_member_location.
* tests/data/test-annotate/test13-pr18894.so.abi: Update.
* tests/data/test-annotate/test15-pr18892.so.abi: Update.
* tests/data/test-annotate/test17-pr19027.so.abi: Update.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Update.
* tests/data/test-annotate/test21-pr19092.so.abi: Update.
* tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0.abi:
Regenerate.
* tests/data/test-diff-pkg/libcdio-0.94-1.fc26.x86_64--libcdio-0.94-2.fc26.x86_64-report.1.txt:
Report now empty.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Update.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Update.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Update.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Update.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Update.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Update.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Update.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi:
Update.
Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
There is a formatted output operator for elf_symbol::visibility in
abg-ir.cc. However, it had no visibile declaration and was not usable
by library users. This commit adds the declaration.
* include/abg-ir.h (operator<<(elf_symbol::visibility): Add
declaration.
Signed-off-by: Giuliano Procida <gprocida@google.com>
This boolean property was obsoleted by the new symtab reader
implementation. It has no users.
Following this change, the find_ksymtab_strings_section function joins
find_ksymtab_section and find_ksymtab_gpl_section in having no users.
* include/abg-ir.h (elf_symbol::elf_symbol): Drop
is_linux_string_cst argument.
(elf_symbol::create): Likewise.
(elf_symbol::get_is_linux_string_cst): Drop method.
* src/abg-dwarf-reader.cc (lookup_symbol_from_sysv_hash_tab):
Remove code that gets the index of the __ksymtab_strings
section. Drop corresponding elf_symbol::create argument.
(lookup_symbol_from_gnu_hash_tab): Likewise.
(lookup_symbol_from_symtab): Likewise.
(create_default_fn_sym): Drop false is_linux_string_cst
argument to elf_symbol::create.
* src/abg-ir.cc (elf_symbol::priv::is_linux_string_cst_): Drop
member variable.
(elf_symbol::priv default ctor): Drop initialisation of
is_linux_string_cst_.
(elf_symbol::priv normal ctor): Drop is_linux_string_cst
argument and corresponding is_linux_string_cst_
initialisation.
(elf_symbol::elf_symbol ctor): Drop is_linux_string_cst
argument and corresponding forwarding to priv ctor.
(elf_symbol::create): Drop is_linux_string_cst argument and
corresponding forwarding to ctor.
(elf_symbol::get_is_linux_string_cst): Drop method.
* src/abg-reader.cc (build_elf_symbol): Drop false
is_linux_string_cst argument to elf_symbol::create.
* src/abg-symtab-reader.cc (symtab::load): Likewise.
Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
