Dodji Seketeli [Mon, 7 Feb 2022 09:42:32 +0000 (10:42 +0100)]
symtab-reader: Remove an over-agressive assertion
In symtab::load, the symtab reader walks the symbol table and records
each relation "symbol <-> address".
So, the relation "foo <-> address-of-foo" is going to be recorded.
The relation "foo.cfi <-> address-of-foo.cfi" is going to be recorded
as well.
But then, because the symbol foo.cfi has a special meaning, in the
realm of "control flow integrity", the relation "foo.cfi <->
address-of-foo.cfi" (as well as all the *.cfi <-> address-of*.cfi
relations) is going to be recorded (again but) in a particular way by
calling symtab::add_alternative_address_lookups.
The problem is that in, symtab::add_alternative_address_lookups there
is an assert that (wrongly) assumes that the relation foo.cfi <->
address-of-foo.cfi is being seen for the first time. This is wrong
because the loop in symtab::load that records all the "symbol <->
address" relations has seen and recorded this foo.cfi <->
address-of-foo.cfi relation once already.
This patch removes that assert so that the kernel referred to in the bug
report of PR26646, as mentioned in
https://sourceware.org/bugzilla/show_bug.cgi?id=26646#c5, can be
processed by abidw without crashing.
David Seifert [Sat, 29 Jan 2022 19:31:07 +0000 (20:31 +0100)]
Find fts-standalone on musl
When using the musl C library fts is optional. So we need to detect
its presence by looking at the fts-standalone pkgconfig module.
This patch does that.
This comes from Gentoo bug https://bugs.gentoo.org/831571
* configure.ac: Invoke AC_CANONICAL_HOST to compute the host_cpu,
host_vendor, host_os parts of the 'host" variable. Then if the
host_os ends up with "musl" then, check for the fts-standalone
pkgconfig module and record the fts library into
FTS_{LIBS,CFLAGS}.
* src/Makefile.am: Link to $FTS_LIBS and use $FTS_CFLAGS for
compilation.
* tools/Makefile.am: Likewise.
* tools/abisym.cc: Include libgen.h
* tools/kmidiff.cc: Remove useless fts.h header file.
Signed-off-by: David Seifert <soap@gentoo.org> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
abidiff: improve whitespace generation in symbol diff report
maybe_report_diff_for_symbol() has a few issues:
1. The responsibility for newline emission is somewhat unclear and
indeed it would emit spurious blank lines before most of the
sub-diffs it reports.
2. Different sub-diff text and terminal commas are emitted according to
whether or not there had been a previous sub-diff - making the output
harder to grep and post-process.
3. The function also returns a bool but that return value is never used.
Hence, change the function to return void, the function stanzas to
always emit newline-terminated lines and ensure the wording and
punctuation of each sub-diff do not vary. This also tweaks (shortens)
the wording used for CRC diffs.
This is a performance and safety improvement made possible by previous
changes which ensure that the same pointers are used for insertion and
look-up.
This change affects two test cases. In more detail:
The test case test-read-dwarf/PR22122-libftdc.so.abi has many
duplicate type-id-60 which appear to all be types defined with a DWARF
DW_AT_signature attribute. These are made into separate types by this
change, but remain incomplete.
The test case test-read-dwarf/PR25007-sdhci.ko.abi has duplicate
declarations and these get split into duplicate declarations with new
type ids following this change. The test suite runs with an implicit
--no-linux-kernel-mode so the duplicates are treated separately. They
presumably had the same type ids before this change due to deep
equality considering them equal.
* src/abg-writer.cc (type_ptr_map): use default equality on
type_base pointer.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Refresh
test case, as described above.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Likewise.
This is a performance and safety improvement made possible by the
previous changes which ensure that the same pointers are inserted and
looked up.
This essentially removes the now unnecessary deep comparison.
* src/abg-writer.cc (type_ptr_set_type): Change typedef
container type to use default equality and hashing for pointer
keys.
(fn_type_ptr_set_type): Likewise.
Giuliano Procida [Tue, 18 Jan 2022 09:03:15 +0000 (09:03 +0000)]
abidiff: include ABI XML versions when reporting a mismatch
In the rare event of an XML version mismatch it would be helpful to
have the versions in the error message, particularly if abidiff is
being run from automation.
* tools/abidiff.cc
(emit_incompatible_format_version_error_message): Add version1
and version2 arguments. Add versions to error message.
(main): Pass emit_incompatible_format_version_error_message
mismatching versions.
Mark Wielaard [Tue, 18 Jan 2022 11:56:45 +0000 (11:56 +0000)]
DWARF reader: use size_t for DWARF expression length cont.
A recent change broke 32-bit builds due to an implicit assumption that
size_t == uint64_t. Note that size_t is part of the elfutils
dwarf_getlocation* functions' types.
The previous fix omitted some instances of uint64_t. This commit
updates further functions to consistently use size_t for DWARF
expression lengths and indexes.
* src/abg-dwarf-reader.cc (eval_last_constant_dwarf_sub_expr):
Change expr_len argument type to size_t.
(op_pushes_constant_value): Update ops_len and index argument
types to size_t. Update next_index argument type to size_t&.
(op_pushes_non_constant_value): Likewise.
(op_is_arith_logic): Update expr_len and index argument types
to size_t. Update next_index argument type to size_t&.
(op_is_control_flow): Likewise.
Giuliano Procida [Mon, 17 Jan 2022 14:51:15 +0000 (14:51 +0000)]
DWARF reader: use size_t for DWARF expression length
A recent change broke 32-bit builds due to an implicit assumption that
size_t == uint64_t. This appears in the elfutils dwarf_getlocation*
functions' types.
This commit updates callers and other functions to use size_t
consistently for such expression lengths and indexes.
* src/abg-dwarf-reader.cc (die_location_expr): Change expr_len
argument type to size_t*.
(op_manipulates_stack): Change expr_len and index argument
types to size_t; change next_index argument type to size_t&.
(eval_last_constant_dwarf_sub_expr): Change expr_len argument
and local variables index and next_index types to size_t.
(die_member_offset): Change local variable expr_len type to
size_t.
(die_location_address): Likewise.
(die_virtual_function_index): Likewise.
Giuliano Procida [Thu, 13 Jan 2022 08:56:48 +0000 (08:56 +0000)]
Bug 28191 - Interpret DWARF 5 addrx locations
This change uses libdw facilities to interpret location expressions
instead of using libabigail's own mini-interpreter. With the fix for
elfutils https://sourceware.org/bugzilla/show_bug.cgi?id=28220 in
elfutils-0.186, abidw will correctly interpret Clang DWARF 5 symbol
addresses. Without that fix many declarations will not be linked to
their corresponding symbols due to the incorrect interpretation of
location attribute data.
* src/abg-dwarf-reader.cc (die_location_address): Use
dwarf_attr_integrate, dwarf_getlocation and
dwarf_getlocation_attr to decode addreses, instead of
die_location_expr and eval_last_constant_dwarf_sub_expr.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Refresh test reference output; two more symbols have types.
This is a fix for bug https://bugzilla.redhat.com/show_bug.cgi?id=1951526.
Basically, this commits makes is so that two enums below are
considered equal by libabigail:
enum foo // This is foo #1
{
e0 = 0;
e1 = 1;
e2 = 2;
};
enum foo // This is foo #2
{
e0 = 0;
e1 = 1;
e2 = 2;
e_added = 1; // This enumerator is considered redundant
// with the enumerator e1 because their values
// are the same.
};
With this patch, foo #1 and foo #2 are considered equal, just like in
the original commit 1cfbff1b. In the original commit however, this
was achieved by comparing the enums without considering their
enumerator names. This was named "binary-only enum comparison". In
reality, that approach was too big of a hammer and was causing the
issues raised in the bug. Namely, type canonicalization would
conflate anonymous enums that were unrelated (precisely because their
enumerator names were different), leading to spurious type change
reports when comparing abixml files pre-dating commit 1cfbff1b with
posterior abixml files.
If I refer to the example above with foo #1 and #2, this patch detects
that the value of the enumerator 'e_added' is redundant with the value
of the enumerator e1. As such, the two foo #1 and #2 are considered
equal. Enumerator names are now fully taken into account.
With this precise approach, it now seems we can do away with the
careful dance of using "binary-only enum comparison" at some precise
times of the libabigail pipeline. Now, we can just use the new enum
comparison scheme all the time. Leading to less (complicated) code
and a hopefully accurate representation.
* include/abg-ir.h (environment::use_enum_binary_only_equality):
Remove.
* src/abg-comparison.cc (compute_diff): In the overload for
enum_type_decl, stop using binary-only-equality for enums.
* src/abg-dwarf-reader.cc
(read_context::compare_before_canonicalisation): Likewise.
* src/abg-ir.cc (environment::use_enum_binary_only_equality):
Remove.
(enumerators_values_are_equal)
(is_enumerator_value_present_in_enum)
(is_enumerator_value_redundant): Define new static functions.
(equals): In the overload for enum_type_decl, use the new
is_enumerator_value_redundant to detect if two enums are equal
modulo a redundant enumerator value. In that case, consider they
are equal.
* tests/data/test-abidiff/test-enum0-report.txt: Adjust.
* tests/data/test-annotate/test-anonymous-members-0.o.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-diff-dwarf/PR25058-liblttng-ctl-report-1.txt: 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.
ctf-reader: Assert on ir::hash_as_canonical_type_or_constant
In some scenarios where we declare same data types `recursively' such
as: like linked list, functions that accept the same pointer to function
as arguments, forward types declarations to build structures with member
fields with mutual dependencies, etc., an assertion is trigger:
It is happening because the recursively behavior of `process_ctf_type'
and `process_ctf_*' used to register ctf types doesn't verify when a
ctf_type was processed and registered before by their subsequence
_recursive_ calls, so `type_base' object is built more than once and the
second time when it is inserted in `types_maps', it refuses in a silent
way, being that the key was already inserted, however
`add_decl_to_scope', `bind_function_type_life_time' successfully
registered the ctf type object. In this patch `process_ctf_type'
delegates register types task to `process_ctf_*' functions guaranteeing
a single ctf type registration, also it improves the performance looking
for the type before start to build it again.
* src/abg-ctf-reader.cc (process_ctf_base_type): Add new
`translation_unit_sptr' parameter. Add condition to validate
success 'base_type' construction and register type object.
(process_ctf_typedef): Add `lookup_type' to get a `type_base'
object when this was previously created, if this is not the
case, register ctf type. Add condition to validate success
'base_type' construction and register type object.
(process_ctf_function_type): Likewise.
(process_ctf_array_type): Likewise.
(process_ctf_qualified_type): Likewise.
(process_ctf_pointer_type): Likewise.
(process_ctf_struct_type): Add `add_decl_to_scope'.
(process_ctf_union_type): Likewise.
(process_ctf_type): Add `lookup_type' to get a `type_base'
object when this was previously created. Delegate register
type object to `process_ctf_*'.
* tests/data/Makefile.am: Add tests I/O and expected files.
* tests/data/test-read-ctf/test-array-of-pointers.[co]: New
testcase.
* tests/data/test-read-ctf/test-list-struct.[co]: Likewise.
* tests/data/test-read-ctf/test-callback.[co]: Likewise.
* tests/data/test-read-ctf/test-callback2.[co]: Likewise.
* tests/data/test-read-ctf/test-functions-declaration.[co]: Likewise.
* tests/data/test-read-ctf/test-forward-type-decl.[co]: Likewise.
* tests/data/test-read-ctf/test-array-of-pointers.abi:
Expected test output.
* tests/data/test-read-ctf/test-callback.abi: Likewise.
* tests/data/test-read-ctf/test-callback2.abi: Likewise.
* tests/data/test-read-ctf/test-forward-type-decl.abi: Likewise.
* tests/data/test-read-ctf/test-functions-declaration.abi: Likewise.
* tests/data/test-read-ctf/test-list-struct.abi: Likewise.
* tests/test-read-ctf.cc: Add testcases to CTF test harness.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
File "/srv/buildbot/worker/libabigail-centos-x86_64/build/tests/mockfedabipkgdiff", line 73, in <module>
fedabipkgdiff_mod = importlib.machinery.SourceFileLoader('fedabipkgdiff', FEDABIPKGDIFF).load_module()
AttributeError: 'module' object has no attribute 'machinery'
Again, I've asked The Internet what to do about that, and this commit is the
result. But beware: I'm still not a Python wizard.
* tests/mockfedabipkgdiff.in: Replace Python 'import importlib'
with 'import importlib.machinery'.
Otherwise, 'is_rpm_file' fails, resulting in a few test cases failing with an
unhelpful 'Unknown arguments. Please refer to --help.' message (similar to what
had been observed in PR22077 "runtestfedabipkgdiff.py fails on centos-x86_64").
* tools/fedabipkgdiff (is_rpm_file): Also accept MIME type
'application/x-redhat-package-manager' for RPM files.
CC: Chenxiong Qi <cqi@redhat.com> Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
Thomas Schwinge [Fri, 17 Dec 2021 22:26:30 +0000 (23:26 +0100)]
Replace use of deprecated Python 'imp' module with 'importlib'
In the test logs, I've found a number of:
[...]/tests/mockfedabipkgdiff:42: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
import imp
I've asked The Internet what to do about that, and this commit is the result.
But beware: I'm not a Python wizard.
* tests/mockfedabipkgdiff.in: Replace use of deprecated Python
'imp' module with 'importlib'.
CC: Chenxiong Qi <cqi@redhat.com> Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
Documenting/providing a way to enable such testing, this commit can be
considered a sequel to commit 90d236a03343d75ea504d53c73270e50b9000e75
"Bug 22076 - Disable fedabipkgdiff for old koji clients", for Mark Wielaard's
PR22076 "runtestfedabipkgdiff.py fails on debian-amd64".
Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
Thread 4 "abipkgdiff" hit Breakpoint 1, package::create_abi_file_path (this=0x5555555a7990,
elf_file_path="/media/[...]/home/thomas/.cache/libabigail/abipkgdiff-tmp-dir-upGgLK/package1/usr/lib64/libGLU.so.1.3.1", abi_file_path="") at [...]/tools/abipkgdiff.cc:668
668 create_abi_file_path(const string &elf_file_path,
(gdb) n
671 string abi_path, dir, parent;
(gdb) n
672 if (!abigail::tools_utils::string_suffix(elf_file_path,
(gdb) n
675 return false;
So we unexpectedly 'return false' here. That's because of 'elf_file_path' as
above ('realpath'ed) vs. 'extracted_dir_path()' as follows (not 'realpath'ed):
[...]
abipkgdiff: Could not create the directory tree to store the abi for '[...]'
abipkgdiff: Writting ABIXML file '' ...
abipkgdiff: Wrote ABIXML file '' OK
abipkgdiff: Reading ABIXML file '' ...
abipkgdiff: Could not read temporary ABIXML file ''
==== Error happened during self check of '[...]' ====
[...]
That is, after a failed 'create_abi_file_path', we proceed with an empty
'abi_file_path' -- because that one only gets set "iff the function return
true". So we ought to 'return abigail::tools_utils::ABIDIFF_ERROR' in that
case.
(It's likewise strange why 'create_write_context'/'write_corpus' succeed with
an empty 'abi_file_path', but that's for another day...)
Thomas Schwinge [Fri, 10 Dec 2021 16:52:46 +0000 (17:52 +0100)]
configure: Instead of for rpm 4.15+ version, test actual rpm/zstd support
If no 'rpm' is available, we currently get:
[...]
checking for rpm... no
../git/configure: line 13119: rpm: command not found
configure: detected rpm version:
configure: rpm support in abipkgdiff is disabled
[...]
Here is the configuration of the package:
[...]
Enable rpm support in abipkgdiff : no
Enable rpm 4.15 support in abipkgdiff tests : no
[...]
Notice intermixed error output: 'rpm: command not found'.
If Ubuntu focal 'rpm' 4.14.2.1+dfsg1-1build2 is available, we currently get:
[...]
checking for rpm... yes
configure: detected rpm version: 4.14.2.1
configure: rpm support in abipkgdiff is enabled
configure: rpm 4.15 support in abipkgdiff tests is enabled
[...]
Here is the configuration of the package:
[...]
Enable rpm support in abipkgdiff : yes
Enable rpm 4.15 support in abipkgdiff tests : yes
[...]
Notice wrong 4.15+ version detection (due to '[[ "$rpmversion" > "4.14.0" ]]'),
which is satisfied by '4.14.2.1'. (Comparing versions with shell '[['
generally is fragile; instead use 'autoconf-archive/ax_compare_version.m4'
or similar?)
Also, 'configure'ing with '--disable-rpm415' doesn't work; same output as
before. That's due to commit 26c41c060bf30750fe2cded87edaf1ae47027523
"Fix thinko in configure.ac", where either there was no thinko in fact (the
original idea, I suppose, was only if 'test x$ENABLE_RPM = xyes' to do the
4.15+ 'auto' checking?), and/or a typo: instead of 'test x$ENABLE_RPM = xyes',
the first conditional should 'test x$ENABLE_RPM415 = xyes'?
And, 'configure'ing with '--enable-rpm415' doesn't raise a hard error if 'rpm'
actually isn't 4.15+.
But all that said, we don't actually need to check for rpm 4.15+ version, but
instead may simply check for the rpm/zstd support that we need: 'rpm2cpio'.
* configure.ac: Instead of for rpm 4.15+ version, test actual
rpm/zstd support.
* tests/test-diff-pkg.cc: Adjust.
Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
Thomas Schwinge [Tue, 14 Dec 2021 16:44:53 +0000 (17:44 +0100)]
Further update 'make distcheck-fast'
What got pushed in recent commit 497357cfd5793e5364a92080d9fae40cf94dc7dd
"Better highlight 'make distcheck-fast'" was the initial submission, before the
changes I made after Matthias Maennich's review. So here they are again.
* CONTRIBUTING: Further update 'make distcheck-fast'.
Suggested-by: Matthias Maennich <maennich@google.com> Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
If 'configure' finds some Python koji module, but it's "insufficient" per
the testing once added in commit 90d236a03343d75ea504d53c73270e50b9000e75
"Bug 22076 - Disable fedabipkgdiff for old koji clients", we currently get,
for example:
[...]
checking python3 module: koji... yes
[...]
checking checking if koji client is recent enough ...... Traceback (most recent call last):
File "<string>", line 3, in <module>
File "[...]/koji/__init__.py", line 2016, in read_config
raise ConfigurationError("no configuration for profile name: %s"
koji.ConfigurationError: no configuration for profile name: koji
no, disabling fedpkgdiff
[...]
Here is the configuration of the package:
[...]
Enable fedabipkgdiff : auto
[...]
Note repeated 'checking' and '...', intermixed error output, 'fedpkgdiff'
typo, final 'auto' result.
Changing that to:
[...]
checking if koji client is recent enough... no
configure: WARNING: disabling fedabipkgdiff
[...]
Here is the configuration of the package:
[...]
Enable fedabipkgdiff : no
[...]
... with 'config.log':
[...]
configure:13774: checking if koji client is recent enough
configure:13784: result: no
Traceback (most recent call last):
File "<string>", line 3, in <module>
File "[...]/koji/__init__.py", line 2016, in read_config
raise ConfigurationError("no configuration for profile name: %s"
koji.ConfigurationError: no configuration for profile name: koji
configure:13792: WARNING: disabling fedabipkgdiff
[...]
Similarly, with explicit '--enable-fedabipkgdiff', we currently get:
[...]
checking checking if koji client is recent enough ...... Traceback (most recent call last):
File "<string>", line 3, in <module>
File "[...]/koji/__init__.py", line 2016, in read_config
raise ConfigurationError("no configuration for profile name: %s"
koji.ConfigurationError: no configuration for profile name: koji
no, disabling fedpkgdiff
[...]
Here is the configuration of the package:
[...]
Enable fedabipkgdiff : yes
[...]
... instead of a fatal error.
Changing that to:
[...]
checking if koji client is recent enough... no
configure: error: unsuitable koji client
Thomas Schwinge [Sat, 11 Dec 2021 17:58:22 +0000 (18:58 +0100)]
CONTRIBUTING: Move "Coding language and style" section
This section got added in commit 4f8c9b170d80b920d13e8f291b347df74db34307
"Use C++11 for the code base", but unfortunately got placed in the middle
of the "Regression tests" section. Move it after that one.
* CONTRIBUTING: Move "Coding language and style" section.
Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
elfutils libdw before 0.184 would not correctly handle a
DW_AT_data_member_location when encoded as a DW_FORM_implicit const
in dwarf_location_expression.
Work around this by first trying to read a data_member_location as a
constant value and only try to get it as a DWARF expression if that
* src/abg-dwarf-reader.cc (die_constant_data_member_location):
New function.
(die_member_offset): Use die_constant_data_member_location
before calling die_location_expr and eval_quickly.
This patch implements some regression tests for ctf reading.
Since the code shares a lot of functionalities already used
in the readi-dwarf test, a library was built and test common
harness were moved to a common location. So input files for
test-read-{dwarf,ctf}.cc now are located in:
tests/data/test-read-common directory, ABIs description are
stored in the same location but in a separate file, one for
each binary debugging information: (e.g, test4-ctf.so.abi
and test4-dwarf.so.abi)
* tests/test-read-ctf.cc: New ctf reading regression test.
* tests/test-read-common.cc: New library to be used with
test-read-{ctf,dwarf}.cc.
* tests/test-read-common.h: Likewise.
* tests/test-annotate.cc (in_out_specs): Adjust path for input files.
* tests/Makefile.am: Build new tests/test-read-ctf.cc file.
* tests/data/Makefile.am: Add test inputs and expected files.
Add libtestreadcommon.a test library and use it for test-read-{ctf,dwarf}.
* tests/test-read-dwarf.cc: Adapt test to use libtestreadcommon.a in
test-read-common.{cc,h}.
* tests/data/test-annotate/test3.so.abi: Adjust ELF input path file
location to ./tests/data/test-read-common.
* tests/data/test-annotate/test4.so.abi: Likewise.
* tests/data/test-read-common/PR26261: Move test harness to
test-read-common directory.
* tests/data/test-read-common/PR27700: Likewise.
* tests/data/test-read-common/test-PR26568-*: Likewise.
* tests/data/test-read-common/test3.{c,so}: Likewise.
* tests/data/test-read-common/test4.{c,so}: Likewise.
* tests/data/test-read-common/crti*: Helper object to export
_init and _fini sysmbols.
* tests/data/test-read-ctf/test-ambiguous-struct-A.c: New testcase.
* tests/data/test-read-ctf/test-ambiguous-struct-B.c: Likewise.
* tests/data/test-read-ctf/test-conflicting-type-syms-a.c: Likewise.
* tests/data/test-read-ctf/test-conflicting-type-syms-b.c: Likewise.
* tests/data/test-read-ctf/test-enum.c: Likewise.
* tests/data/test-read-ctf/test-enum-many.c: Likewise.
* tests/data/test-read-ctf/test-enum-symbol.c: Likewise.
* tests/data/test-read-ctf/test-struct-iteration.c: Likewise.
* tests/data/test-read-ctf/test-dynamic-array.c: Likewise.
* tests/data/test-read-ctf/test0.c: Likewise.
* tests/data/test-read-ctf/test1.c: Likewise.
* tests/data/test-read-ctf/test2.c: Likewise.
* tests/data/test-read-ctf/test5.c: Likewise.
* tests/data/test-read-ctf/test7.{c,h}: Likewise.
* tests/data/test-read-ctf/test8.c: Likewise.
* tests/data/test-read-ctf/test9.c: Likewise.
* tests/data/test-read-ctf/test-ambiguous-struct-A.o.hash.abi: Testcase
expected result.
* tests/data/test-read-ctf/PR26261/PR26261-exe.abi: Likewise.
* tests/data/test-read-ctf/PR27700/test-PR27700.abi: Likewise.
* tests/data/test-read-ctf/test-ambiguous-struct-A.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-ambiguous-struct-B.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-conflicting-type-syms-a.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-conflicting-type-syms-b.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-enum.o.abi: Likewise.
* tests/data/test-read-ctf/test-enum-many.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-enum-symbol.o.hash.abi: Likewise.
* tests/data/test-read-ctf/test-struct-iteration.o.abi: Likewise.
* tests/data/test-read-ctf/test-dynamic-array.o.abi: Likewise.
* tests/data/test-read-ctf/test0: Likewise.
* tests/data/test-read-ctf/test0*.abi: Likewise.
* tests/data/test-read-ctf/test1.so: Likewise.
* tests/data/test-read-ctf/test1*.abi: Likewise.
* tests/data/test-read-ctf/test2.so: Likewise.
* tests/data/test-read-ctf/test2*.abi: Likewise.
* tests/data/test-read-ctf/test3.so.abi: Likewise.
* tests/data/test-read-ctf/test4*.abi: Likewise.
* tests/data/test-read-ctf/test5.o.abi: Likewise.
* tests/data/test-read-ctf/test7.o.abi: Likewise.
* tests/data/test-read-ctf/test8.o.abi: Likewise.
* tests/data/test-read-ctf/test9.o.abi: Likewise.
* tests/data/test-read-dwarf/PR26261/PR26261-exe.abi: Update
expected abixml file.
* tests/data/test-read-dwarf/PR27700/test-PR27700.abi: Likewise.
* tests/data/test-read-dwarf/test-PR26568-1.*.abi: Likewise.
* tests/data/test-read-dwarf/test3*.abi: Likewise.
* tests/data/test-read-dwarf/test4*.abi: Likewise.
* doc/api/libabigail.doxy: Add tests/test-read-common.{cc,h} to
doxygen.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
ctf-reader: Fix length in dynamic array definition
Defining an array type with dynamic length, node `subrange'
in the abixml file doesn't write the accurate `length'
property `infinite', instead `1' is written:
<subrange length='1' .../>
So, member function `array_type_def::subrange_type::is_infinite'
is set when `upper_bound' value is equal to `0'.
* src/abg-ctf-reader.cc (process_ctf_array_type):
set subrange_type::is_infinite when `upper_bound' value
is equal to `0'.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
XML writer: use exemplar types for tracking referenced types
The emitted type sets are used with some referenced type sets (which use
bare type pointers). To keep consistency between what is being recorded
in each set, switch to storing exemplar type pointers in the referenced
type sets.
This change results in the omission of a small number of duplicate
types from various test cases. In each case the duplicates were
previously caused by a referenced type being emitted for one
translation unit and then the same type being emitted as a canonical
type for a later translation unit.
It also causes the movement of some function types in some test cases.
Some of those types are now considered referenced and appear earlier as
a result.
* src/abg-writer.cc (record_type_as_referenced): Use exemplar
type with referenced type sets.
(type_is_referenced): Likewise.
(tests/data/test-annotate/test14-pr18893.so.abi): Duplicate
type(s) removed, as described above.
(tests/data/test-read-dwarf/test14-pr18893.so.abi): Likewise.
(tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi):
Likewise.
(tests/data/test-read-dwarf/test16-pr18904.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/PR25007-sdhci.ko.abi): Some
function type(s) reordered, as described above.
(tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi):
Likewise.
(tests/data/test-annotate/test15-pr18892.so.abi):: Duplicate
type(s) removed and some function type(s) reordered, as
described above.
(tests/data/test-read-dwarf/test15-pr18892.so.abi): Likewise.
(tests/data/test-annotate/test21-pr19092.so.abi): Likewise.
(tests/data/test-read-dwarf/test21-pr19092.so.abi): Likewise
XML writer: use consistent type pointers for type ids and emission tracking
Insertion uses the canonical type, if available, but look-up did
not. Given that type id insertion and look-up also use canonical
types, it makes sense to adjust the remaining code accordingly.
Neither decl_only_type_is_emitted nor record_decl_only_type_as_emitted
do the check, but very few types end up being recorded this way.
The functions write_class_decl and write_union_decl (but not
write_class_decl_opening_tag and write_union_decl_opening_tag which
can be called in other contexts) resolve declaration-only types to a
definition where possible.
To ensure type ids consistently refer to the same canonical type we
should use canonical types and definitions-of-declarations more
consistently.
This change introduces get_exemplar_type to return the exemplar type
that should be used for type id and emitted checks. That exemplar
type is the canonical type of a given type, or the canonical type of
the definition-of-declaration-only-type when applicable.
However, it does not also change all the write functions to write out
the exemplar types.
* include/abg-fwd.h (get_exemplar_type): Declare new function.
* src/abg-ir.cc (get_exemplar_type): Define new function.
* src/abg-writer.cc (type_has_existing_id): use get_exemplar_type
for resolution.
(get_id_for_type): Likewise.
(record_type_as_emitted): Likewise.
(type_is_emitted): Likewise.
Dodji Seketeli [Mon, 6 Dec 2021 16:45:46 +0000 (17:45 +0100)]
reader: Build array types with their element type "a priori"
In some ancient settings, we were building array types with no element
type set, then, when we have the element type built, we'd set it to
the array type. This was to avoid looping indefinitely in cases where
the element type would indirectly depend on the array type itself.
Since then, we've built infrastructure to avoid those loops.
So we don't need those 'temporarily empty arrays' anymore. Besides,
trying get the pretty representation of a variable declaration which
type is one of those temporarily empty arrays crashes because that
code doesn't expect empty arrays.
This patch sets the element type at array type building type now. The
code also asserts that element types of arrays are not empty, in the
pretty representation of variable declarations.
* src/abg-ir.cc (var_decl::get_pretty_representation): Assert that
array element types are not empty.
* src/abg-reader.cc (build_array_type_def): Set array element
types a priori.
This should fix bug https://sourceware.org/bugzilla/show_bug.cgi?id=28073
There is at least a case where the evaluation of the suppression
specification rule incarnated by the property
has_data_member_inserted_between doesn't work. This is in the context
of the following suppression specification:
[suppress_type]
name = struct_foo
has_data_member_inserted_between = {offset_of(dm1), offset_of(dm2)}
The evaluation of the rule incarnated by
has_data_member_inserted_between fails in the context of a type change
where the data member "dm1" is removed from the type struct_foo. In
that case, the evaluation of the suppression should ALWAYS yield to
the suppression specification NOT suppressing the change. But in some
cases the change is suppressed nonetheless.
This patch fixes that.
The idea of the patch is that if the class has a removed data member
or if its size shrinks then no type change on that class can be
suppressed. This is because those two kinds of change are
incompatible ABI (or at least API) changes. So they should be
reported.
The patch also fixes the evaluation of the boundaries of the insertion
range expressed as an "offset_after" expression.
* doc/manuals/libabigail-concepts.rst: Update the documentation to
reflect that has_data_member* properties will never suppress any
type change if the change carries a data member suppression or a
type size reduction.
* include/abg-fwd.h (get_last_data_member)
(get_next_data_member_offset): Declare new functions.
* include/abg-suppression.h
(insertion_range::boundary_value_is_end): Declare new static
member function.
(type_supression::insertion_range::eval_boundary): Make this
static function take an uint64_t rather than ssize_t.
(type_suppression::insertion_range::integer_boundary::{integer_boundary,
as_integer, operator int}): Make these member functions and
operator take or return uint64_t rather than int.
* src/abg-ir.cc (get_last_data_member)
(get_next_data_member_offset): Define new functions.
* src/abg-suppression.cc
(type_suppression::suppresses_diff): Rework logic to better handle
"has_data_member_inserted_*" properties in the context of class
diffs. If the diff object carries data member removal or size
reduction, the diff object is not suppressed by the current type
suppression. Also, the property "has_data_member_inserted_at =
end", is now represented by an insertion range where the beginning
and the end of the range are both the max possible value of
insertion range boundaries; the code is made to recognize that.
(type_suppression::insertion_range::eval_boundary): Make this
static function take an uint64_t rather than ssize_t. If the
boundary is expressed as a "offset_after" expression, make sure
the offset of the next data member is considered if it's present.
(type_suppression::insertion_range::integer_boundary::{integer_boundary,
as_integer, operator int}): Make these take or return uint64_t
rather than int.
(type_suppression::insertion_range::boundary_value_is_end): Define
new member function.
(type_suppression::insertion_range::integer_boundary::priv::value_):
Turn the type of this into uint64_t, from int.
(type_suppression::insertion_range::integer_boundary::priv::priv):
The parameter of this is now uint64_t, from int.
* tests/data/test-diff-suppr/PR28073/PR28073-bitfield-removed.c:
New test source code.
* tests/data/test-diff-suppr/PR28073/PR28073-bitfield-removed.o:
New test binary.
* tests/data/test-diff-suppr/PR28073/PR28073-bitfield-removed.o.abi:
New test input.
* tests/data/test-diff-suppr/PR28073/PR28073-output-{1,2}.txt: New
test reference output.
* tests/data/test-diff-suppr/PR28073/PR28073.after.o: New test
binary.
* tests/data/test-diff-suppr/PR28073/PR28073.after.o.abi: New test
input.
* tests/data/test-diff-suppr/PR28073/PR28073.before.o: New test
binary.
* tests/data/test-diff-suppr/PR28073/PR28073.before.o.abi: New
test input.
* tests/data/test-diff-suppr/PR28073/PR28073.c: New test source
code.
* tests/data/test-diff-suppr/PR28073/bitfield.suppr: New test
input.
* tests/data/Makefile.am: Add the new test material to source
distribution.
* tests/test-diff-suppr.cc: Add the new test input to this test
harness.
XML writer: adjust tracking of emitted declarations
Replace the std::unordered_map used to track emitted declarations with a
std::unordered_set as the map only ever held "true". The container
itself does not need to be marked mutable because record_decl_as_emitted
is called in a non-const context and can itself be made non-const. In
addition, the method decl_is_emitted calls a helper which no longer used
anywhere else and can be inlined. The remaining two methods are always
called on non-type declarations, so the test that existed in
decl_is_emitted can be dropped.
* abg-writer.cc (write_context): Replace mutable
m_emitted_decls_map with plain m_emitted_decls_set.
(decl_name_is_emitted): Inlined into decl_is_emitted; dropped.
(decl_is_emitted): Turn the is_type check into an assert and
inline decl_name_is_emitted. Look up in set instead of map.
(record_decl_as_emitted): Make non-const. Insert into set
instead of map.
runtestreadctf fails executed by make check-valgrind, a memory leak is
reported:
at 0x48397B5: malloc (vg_replace_malloc.c:381)
by 0x56E6316: ctf_bufopen_internal (ctf-open.c:1388)
by 0x56E5FD3: ctf_bufopen (ctf-open.c:1297)
by 0x56CD688: ctf_arc_bufopen (ctf-archive.c:444)
by 0x4EB31FD: abigail::ctf_reader::read_corpus(abigail::ctf_reader::read_context*,\
abigail::elf_reader::status&) (abg-ctf-reader.cc:1111)
by 0x10C6BB: test_task_ctf::perform() (test-read-ctf.cc:236)
by 0x4E8E9AF: abigail::workers::worker::wait_to_execute_a_task\
(abigail::workers::queue::priv*) (abg-workers.cc:400)
by 0x543BEAD: start_thread (pthread_create.c:463)
by 0x5369A5E: clone (clone.S:95)
This is happening because `ctf_variable_next' bumps the dictionary
reference counter, so when `ctf_close' is called it doesn't free the
memory allocated by the dict, because `ctf_refcnt' is greater than one.
Therefore `ctf_dict_close' is required.
ctf-reader: Make create_read_context return a smart pointer.
* include/abg-ctf-reader.h (read_context_sptr): New typedef.
(create_read_context): Use read_context_sptr as return value.
(read_corpus): New overload that takes a read_context_sptr.
* src/abg-ctf-reader.cc (create_read_context): Use
read_context_sptr as return value.
(read_corpus): New overload that takes a read_context_sptr.
* tools/abidiff.cc (main): Use read_context_sptr.
* tools/abidw.cc (load_corpus_and_write_abixml): Adjust call to
create_read_context.
* tools/abilint.cc: Likewise.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Dodji Seketeli [Mon, 22 Nov 2021 15:16:22 +0000 (16:16 +0100)]
ctf-reader: Remove useless parameter from fill_ctf_section
While looking at something else, I noticed fill_ctf_section now has a
useless parameter that was making compiling with some "tight" options
cringe.
Fixed thus.
* src/abg-ctf-reader.cc (fill_ctf_section): Remove the now useless
context parameter. Adjust the comment accordingly.
(slurp_elf_info): Adjust the invocation of fill_ctf_section.
ctf-reader: Use argument by reference reading the context
* include/abg-ctf-reader.h (create_read_context): Pass the string
by reference.
* src/abg-ctf-reader.cc (ctf_reader::read_context): Likewise.
(create_read_context): Likewise.
This patch adds support for CTF to the abidw utility. It depends on
the previous patch that makes abigail::ctf_reader::read_corpus to
return a status code.
* tools/abidw.cc: Conditionally include abg-ctf-reader.h.
(load_corpus_and_write_abixml): Do not get a
dwarf_reader::read_context as an argument.
(main): Adjust call to load_corpus_and_write_abixml accordingly.
(struct options): New option use_ctf.
(options): ... and initialize it.
(display_usage): Document --ctf.
(parse_command_line): Handle --ctf.
* doc/manuals/abidw.rst: Document --ctf.
Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch makes ctf_reader::read_corpus to get a reference to a
`status' variable as an argument, and set it to reflect the result of
the read operation. The utilities calling to ctf_reader::read_corpus
are updated accordingly.
* include/abg-ctf-reader.h: Include abg-elf-reader-common.h.
read_corpus now gets an extra argument `status'.
* src/abg-ctf-reader.cc (read_corpus): Likewise, and set `status'
accordingly when the debug info is not found.
* tools/abilint.cc (main): Pass a status argument to
ctf_reader::read_corpus.
* tools/abidiff.cc (main): Likewise.
Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
tangmeng [Thu, 11 Nov 2021 05:53:42 +0000 (13:53 +0800)]
Standardize and improve the output of several tests
This patch updates several test harnesses to make their output show
the command line of the failing tests, a brief informative summary
about the number of unit tests executed, failed and executed with
success.
These tests now used the
abigail::tests::emit_test_{summary,status_and_update_counters}
functions provided in tests/test-utils.cc.
* tests/test-abidiff-exit.cc (main): Use
abigail::tests::emit_test_{summary, status_and_update_counters}
functions to ameliorate and standardize test output.
* tests/test-alt-dwarf-file.cc (main): Likewise.
* tests/test-annotate.cc (main): Likewise.
* tests/test-diff-dwarf-abixml.cc (main): Likewise.
* tests/test-ini.cc (main): Likewise.
* tests/test-lookup-syms.cc (main): Likewise.
Dodji Seketeli [Mon, 15 Nov 2021 16:08:47 +0000 (17:08 +0100)]
Bug 28584 - Don't drop global variables that lack DW_AT_external
Clang doesn't always emit the DW_AT_external property that flags a
decl as being external. In those cases, the DWARF reader just drops
the variable on the floor as it considers it as being "non-exported".
This patch considers that a variable decl that is at named namespace
scope is essentially "external". Then if the variable has an ELF
symbol associated to it, then an IR node will be created for it.
The other changes are just needed adaptations due to the core change.
* src/abg-dwarf-reader.cc (die_is_effectively_public_decl): Define
new static function.
(die_flag_attribute, die_is_public_decl): Adjust const-ness.
(build_ir_node_from_die): When building an IR for a variable,
consider the variable as being external if the variable is at
namespace scope, even if its DIE doesn't have the DW_AT_external
attribute.
* tests/data/test-read-dwarf/PR28584/PR28584-smv.cc: New source
code for a new clang-built binary.
* tests/data/test-read-dwarf/PR28584/PR28584-smv.clang.o: New
clang-built input binary for testing purposes.
* tests/data/test-read-dwarf/PR28584/PR28584-smv.clang.o.abi: The
reference output abixml.
* tests/data/Makefile.am: Add the new test material above to
source distribution.
* tests/test-read-dwarf.cc (in_out_specs): Add the new test input
to this test harness.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
Move dwarf_reader::status facilities to an abigail::elf_reader namespace
The DWARF reader is no longer the only ELF-based reader in libabigail:
the CTF reader also operates on ELF files. Other ELF-based formats
(such as BTF) may also join in the future. These readers share a lot
of similarities: they all operate on object files, they fetch
debugging information from certain sections, they rely on the symtab
of the read object, the debugging info may be in a separated file (for
certain formats) and so on.
It follows that a lot of logic can be shared among all the ELF-based
readers. This patch is oriented to that direction.
A new namespace, abigail::elf_reader, is introduced with the goal of
holding features and definitions useful for any ELF-based abigail
reader. Then all the definitions related to the status resulting from
extracting a corpus from an object file (the dwarf_reader::status) are
moved to abigail::elf_reader. The utilities and tests are adjusted
accordingly.
Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
* include/abg-reader-common.h: New file.
* include/abg-dwarf-reader.h (enum status): Move to
abg-reader-status.h.
(status_to_diagnostic_string): Likewise.
(operator|): Likewise.
(operator&): Likewise.
(operator|=): Likewise.
(operator&=): Likewise.
Include abg-reader-common.h.
* include/Makefile.am (pkginclude_HEADERS): Add
abg-elf-reader-common.h.
* src/abg-elf-reader-status.cc: New file.
* src/abg-dwarf-reader.cc (operator|): Move to
abg-elf-reader-common.cc.
(operator&): Likewise.
(operator|): Likewise.
(operator|=): Likewise.
(operator&=): Likewise.
(status_to_diagnostic_string): Likewise.
* src/Makefile.am (libabigail_la_SOURCES): Add
elf-reader-common.cc.
* src/abg-tools-utils.cc: Use abigail::elf_reader instead of
abigail::dwarf_reader for the status definitions.
* tools/abicompat.cc: Likewise.
* tools/abidiff.cc: Likewise.
* tools/abidw.cc: Likewise.
* tools/abilint.cc: Likewise.
* tools/abipkgdiff.cc: Likewise.
* tests/print-diff-tree.cc: Likewise.
* tests/test-diff-dwarf.cc: Likewise.
* tests/test-read-dwarf.cc: Likewise.
* tests/test-symtab.cc: Likewise.
* tests/test-ir-walker.cc: Likewise.
Dodji Seketeli [Fri, 12 Nov 2021 17:28:43 +0000 (18:28 +0100)]
abg-config.{cc,h}: Misc comment cleanups
* include/abg-config.h (abigail_get_library_version): Remove the
comment from the header file ...
* src/abg-config.cc (abigail_get_library_version): ... to put it
in the definition.
Dodji Seketeli [Fri, 12 Nov 2021 08:47:03 +0000 (09:47 +0100)]
abidw: Add --abixml-version
Add a command line option to display the version number of the ABIXML
output format.
* doc/manuals/abidw.rst: Document the --abixml-version command
line option.
* configure.ac (ABIXML_VERSION_MAJOR, ABIXML_VERSION_MINOR):
Define these two new autoconf variables.
* include/abg-config.h (abigail_get_abixml_version): Declare new
function.
* include/abg-tools-utils.h (get_abixml_version_string): Declare
new function.
* include/abg-version.h.in (ABIGAIL_ABIXML_VERSION_MAJOR)
(ABIGAIL_ABIXML_VERSION_MINOR): Define new preprocessor macros.
* src/abg-config.cc (config::config): Initialize
config::m_format_{minor,major} using the newly defined
preprocessor macros ABIGAIL_ABIXML_VERSION_M{IN,AJ}OR.
* src/abg-tools-utils.cc (get_abixml_version_string): Define new
function.
* tools/abidw.cc (options::display_abixml_version): Define new
data member.
(options::options): Initialize it.
(display_usage): Emit a help string for the new --abixml-version
option.
(parse_command_line): Parse the --abixml-version string.
(main): Emit the abixml version when asked.
Dodji Seketeli [Thu, 4 Nov 2021 09:59:32 +0000 (10:59 +0100)]
Bug 28450 - Fix cloned member function handling in DWARF
When the DWARF reader encounters a function DIE 'f' that has a
DW_AT_specification that points to a member function, the current
implementation creates a function IR for the member function. The
problem is that the member function has no ELF symbol associated to
it. The ELF symbol is associated to 'f', not to the member function.
The DWARF reader then wrongly drops the member function on the floor
because it has no ELF symbol function associated. So that member
function specification never gets its concrete function represented in
the IR.
This patch fixes the issue by detecting that the member function is
the "specification" for 'f' and that the ELF symbol associated to it
might not be there. In that case, if the ELF symbol is on the 'f' DIE
itself, we get it from there.
The patch makes more member functions to be represented so it uncovers
a latent issue which is explained below.
Today, some compilers can emit redundant DWARF constructs like "const
reference" or "const void". A reference is always const so the const
is superfluous. A similar thing can be said about "const void".
maybe_strip_qualification detects those constructs and rewrites the IR
into a "no-op qualified reference", or a "no-op qualified void". The
no-op was needed in the previous incarnations of the DWARF reader
because it was expecting a 'qualified type' IR to be associated to a
qualified type DIE. Note, however, that that expectation has been
generally relaxed since then.
The problem is that the comparison engine, when building the diff IR
needs to strip those no-op qualified types off, to avoid having
spurious change diagnostics. That stripping introduces some
challenges because the tree is more or less un-mutable at that point
(after type canonicalization) so the stripping can only be partial.
This patch removes the no-op qualified types altogether, rather than
trying harder to handle them down the line. In other words, a const
reference is now represented as a reference and a const void as a
void. This makes things much simpler.
The problem however is that the in-memory IR (and thus the emitted
ABIXML) doesn't have any no-op qualified type anymore. So comparing
an old ABIXML that contains those no-op qualified types against its
origin ELF corpus can yield some spurious diagnostics. To fix it, one
needs to re-generate the ABIXML file.
This patch bumps the ABIXML version to 2.1 and introduces a new
ABIXML-FORMAT-VERSIONS file that documents the format changes.
* ABIXML-FORMAT-VERSIONS: New file that documents the version
changes of the ABIXML-FORMAT-VERSIONS.
* include/abg-fwd.h (look_through_no_op_qualified_type): Remove
this function declaration.
(strip_useless_const_qualification): Declare new function
declaration.
* src/abg-comparison.cc (compute_diff_for_types): Avoid stripping
off no-op-qualified types as these beasts don't exist anymore.
(redundancy_marking_visitor::visit_end): Allow a variable which
type has local changes to be considered redundant if its type is
itself redundant.
* src/abg-config.cc (config::config): Bump the abixml version from
2.0 to 2.1.
* src/abg-dwarf-reader.cc (maybe_strip_qualification): Factorize
out the new strip_useless_const_qualification function from here.
(build_or_get_fn_decl_if_not_suppressed): If the function is
created but doesn't have an ELF symbol associated to it, then
update it so that the ELF symbol can be associated. Otherwise,
potential_member_fn_should_be_dropped might later drop that
function on the floor because it doesn't have any ELF symbol
associated.
* src/abg-ir.cc (strip_useless_const_qualification): Define this,
which is has been factorized out of maybe_strip_qualification.
(look_through_no_op_qualified_type): Remove this definition.
(equals): In the overload for reference_type_def, do not peel
typedefs off from the reference before comparison. This is now
useless as the comparison infrastructure got a lot better. In the
overload for references, stop using
look_through_no_op_qualified_type as this function doesn't exist
anymore.
* src/abg-tools-utils.cc (abidiff_status_has_error): Detect when
there is a usage error as well.
* tests/data/test-types-stability/PR28450-libepetra.so.13.0: Add
new testing binary to the repository.
* tests/data/Makefile.am: Add the new testing binary above to
source distribution.
* tests/test-types-stability.cc (elf_paths): Add the new test to
this test harness.
* tests/test-diff-dwarf-abixml.cc (main): Add better error messages.
* tests/data/test-abidiff-exit/test-member-size-report0.txt: Adjust.
* tests/data/test-annotate/libtest23.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-annotate/test-anonymous-members-0.o.abi: Likewise.
* tests/data/test-annotate/test0.abi: Likewise.
* tests/data/test-annotate/test1.abi: Likewise.
* tests/data/test-annotate/test13-pr18894.so.abi: Likewise.
* tests/data/test-annotate/test14-pr18893.so.abi: Likewise.
* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise.
* tests/data/test-annotate/test6.so.abi: Likewise.
* tests/data/test-annotate/test7.so.abi: Likewise.
* tests/data/test-annotate/test8-qualified-this-pointer.so.abi: Likewise.
* tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1.abi: 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/test42-PR21296-clanggcc-report0.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/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/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-1.txt: Likewise.
* tests/data/test-diff-suppr/test24-soname-report-10.txt: Likewise.
* tests/data/test-diff-suppr/test24-soname-report-12.txt: Likewise.
* tests/data/test-diff-suppr/test24-soname-report-14.txt: Likewise.
* tests/data/test-diff-suppr/test24-soname-report-16.txt: Likewise.
* tests/data/test-diff-suppr/test24-soname-report-4.txt: Likewise.
* tests/data/test-diff-suppr/test31-report-1.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/libtest23.so.abi: Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libaaudio.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/test0.abi: Likewise.
* tests/data/test-read-dwarf/test0.hash.abi: Likewise.
* tests/data/test-read-dwarf/test1.abi: Likewise.
* tests/data/test-read-dwarf/test1.hash.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise.
* tests/data/test-read-dwarf/test2.so.abi: Likewise.
* tests/data/test-read-dwarf/test2.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise.
* tests/data/test-read-dwarf/test3-alias-1.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test3-alias-2.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test3-alias-3.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test3-alias-4.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test3.so.abi: Likewise.
* tests/data/test-read-dwarf/test3.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test4.so.abi: Likewise.
* tests/data/test-read-dwarf/test4.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test5.o.abi: Likewise.
* tests/data/test-read-dwarf/test5.o.hash.abi: Likewise.
* tests/data/test-read-dwarf/test6.so.abi: Likewise.
* tests/data/test-read-dwarf/test6.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test7.so.abi: Likewise.
* tests/data/test-read-dwarf/test7.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test8-qualified-this-pointer.so.abi: Likewise.
* tests/data/test-read-dwarf/test8-qualified-this-pointer.so.hash.abi: Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
* tests/data/test-read-write/test-crc.xml: Likewise.
* tests/data/test-read-write/test26.xml: Likewise.
* tests/data/test-read-write/test27.xml: Likewise.
* tests/data/test-read-write/test28-without-std-fns-ref.xml: Likewise.
* tests/data/test-read-write/test28-without-std-vars-ref.xml: Likewise.
abg-ctf-reader: use the right string table for CTF data
The CTF library needs the string table associated with the symbol
table of the ELF file. This patch makes the CTF reader to use the
right string table.
Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
* src/abg-ctf-reader.cc (slurp_elf_info): Use
find_strtab_for_symtab_section.
elf_helpers: new utility function find_strtab_for_symtab_section
This patch adds a new utility function that, given a section
containing a symbol table, returns the corresponding string table
section.
Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
* src/abg-elf-helpers.h: Prototype for find_strtab_for_symtab_section.
* src/abg-elf-helpers.cc (find_strtab_for_symtab_section): New function.
tangmeng [Tue, 9 Nov 2021 12:27:59 +0000 (20:27 +0800)]
test-abicompat: Make the test output more pleasant
When testing with runtestabicompat, the following problems were found:
1. abicompat tested multiple scenarios, but the last result was used
as the basis for the return value of the command.
2. For multiple test scenarios, the execution results cannot be known
after the test, which is easy to cause confusion.
* test/test-abicompat.cc (main): make test output more pleasant.
tangmeng [Fri, 5 Nov 2021 09:38:28 +0000 (17:38 +0800)]
abicompat: Add prompt message for abnormal operation
When using abicompat, if the --redundant option and --no-redundant
option are used at the same time, no error is prompted and none of the
options have an impact.
This patch emits an error message in that case.
* tools/abicompat.cc (parse_command_line): Notify the user
when --redundant and --no-redundant are used at the same time
ctf: make libabigail::ctf_reader::read_corpus reentrant
The libctf call ctf_open is not reentrant. This is because it uses
bfd_open (and other BFD calls) internally in order to fetch the
different bits of CTF from the ELF file.
This is unfortunate, as it makes libabigail::ctf_reader::read_corpus
non-reentrant. We detected this problem thanks to one of the
libabigail test driver, that exercises tests in parallel using
threads.
Fortunately libctf provides an alternate way to decode CTF data, that
involves the user to provide the raw contents of the relevant ELF
sections (.ctf, the symtab, the string table) to ctf_arc_bufopen
call.
This patch changes the CTF reader in libabigail to use this
mechanism. libelf is used in order to extract the contents of these
sections.
* src/abg-ctf-reader.cc (class read_context): New attributes
elf_handler, elf_fd, ctf_sect, symtab_sec and strtab_sect.
(read_context): Do not read the CTF archive here.
(slurp_elf_info): Adjust to use attributes instead of locals, and
fetch the raw ELF section contents for libctf.
(close_elf_handler): New function.
(fill_ctf_section): Likewise.
(read_corpus): Call open_elf_handler, close_elf_handler and build
the CTF archive using ctf_arc_bufopen instead of ctf_open.
Signed-by: Jose E. Marchesi <jose.marchesi@oracle.com> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Add support for the CTF debug format to libabigail.
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 :)
tangmeng [Fri, 29 Oct 2021 09:23:43 +0000 (17:23 +0800)]
abicompat: Add prompt message for abnormal operation
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.
tangmeng [Fri, 29 Oct 2021 01:41:29 +0000 (09:41 +0800)]
abilint: fix trivial typo when using abilint
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.
Dodji Seketeli [Wed, 20 Oct 2021 12:42:05 +0000 (14:42 +0200)]
PR28365 - Assert on empty typedef on webkit2gtk3-jsc-2.32.3-1.fc34.x86_64
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.
Giuliano Procida [Mon, 11 Oct 2021 13:17:09 +0000 (14:17 +0100)]
Tweak clang-format configuration
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.
Dodji Seketeli [Mon, 18 Oct 2021 10:53:40 +0000 (12:53 +0200)]
Add debug info package for wireshark-cli-3.4.9-1.fc36.x86_64.rpm
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.
Dodji Seketeli [Thu, 14 Oct 2021 17:02:59 +0000 (19:02 +0200)]
Bug 28364 - libwiretap fails self comparison
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.
Dodji Seketeli [Thu, 14 Oct 2021 14:56:23 +0000 (16:56 +0200)]
writer: Don't forget that a naming typedef is referenced
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.
Dodji Seketeli [Thu, 14 Oct 2021 14:38:18 +0000 (16:38 +0200)]
writer: Don't forget to emit types referenced by function types
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.
Dodji Seketeli [Thu, 14 Oct 2021 13:38:34 +0000 (15:38 +0200)]
ir: Avoid canonicalizing types that are not meant to
hash_as_canonical_type_or_constant asserts that a certain number of
types are not meant to be canonicalized. We ought to make sure that
type_base::get_canonical_type_for always agrees with
hash_as_canonical_type_or_constant. This patch enforces that for
good measure.
Dodji Seketeli [Thu, 14 Oct 2021 10:12:01 +0000 (12:12 +0200)]
Add --enable-debug-type-canonicalization to configure
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.
Dodji Seketeli [Wed, 13 Oct 2021 11:52:22 +0000 (13:52 +0200)]
Improve type (de)serialization instability debugging
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.
Bug 27086 - Consider all C++ virtual destructors when there are many
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.
Bug 27970 - Duplicated member functions cause spurious self comparison changes
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.
abipkgdiff: Do not erase working dirs before we are done using them
* 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.
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.
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.
RHBZ1944102 - self comparing ABI of protobuf-3.14.0-2.el9 failed
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.
RHBZ1951496 - ir: Acknowledge that "void type" is not canonicalized
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.
xml-reader: Get back to original way of building qualified types
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
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.
ir: Avoid infinite loop during type canonicalization
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.
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
RHBZ-1944096 - assertion failure during self comparison of systemd
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.
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.
Giuliano Procida [Fri, 27 Aug 2021 15:24:39 +0000 (16:24 +0100)]
abg-writer: faster referenced type emission tests
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.