In some abixml file, there can be global decls that don't have ELF
symbols. We still want to see how those decls use the type that is
being used, as analyzed by abilint --show-type-use <type-id>.
* include/abg-fwd.h (is_at_global_scope): Declare ...
* src/abg-ir.cc (is_at_global_scope): ... new overload.
* tools/abilint.cc (emit_artifact_use_trace): Emit the trace also
when the decl is at global scope or has a linkage name.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
"abilint --show-type-use <type-id> <abixml-file>" is a facility that
shows how a type defined in an abixml file is used. That is, it emits
a textual representation of how the use a type is used up until the
function or global variable that constitutes an entry point in the API
corpus. Here is an example of its use:
test-read-write$ abilint --noout --show-type-use type-id-5 test17.xml
Type ID 'type-id-5' is for type 'enum E'
The usage graph for that type is:
| -> enum E -> E S::m2 -> class S -> S* -> method void S::S()
| -> enum E -> E S::m2 -> class S -> S* -> method void S::__base_ctor ()
| -> enum E -> E S::m2 -> class S -> S* -> method void S::__comp_ctor ()
| -> enum E -> E S::m2 -> class S -> S* -> method void S::S(S&)
| -> enum E -> E S::m2 -> class S -> S* -> S& -> method void S::S(S&)
| -> enum E -> E S::m2 -> class S -> S* -> S& -> S var
| -> enum E -> E S::m2 -> class S -> S* -> S& -> E S::m2 -> class S -> S* -> method void S::S()
| -> enum E -> E S::m2 -> class S -> S* -> S& -> E S::m2 -> class S -> S* -> method void S::__base_ctor ()
| -> enum E -> E S::m2 -> class S -> S* -> S& -> E S::m2 -> class S -> S* -> method void S::__comp_ctor ()
| -> enum E -> E S::m2 -> class S -> S* -> S& -> E S::m2 -> class S -> S* -> method void S::S(S&)
| -> enum E -> E S::m2 -> class S -> S* -> S& -> E S::m2 -> class S -> S* -> S& -> method void S::S(S&)
| -> enum E -> E S::m2 -> class S -> S* -> S& -> E S::m2 -> class S -> S* -> S& -> S var
$
The screenshot above should be self explanatory.
This facility is useful to analyse type usage and find potential
issues in how libabigail represents some types.
To activate this feature, one needs to configure the package with the
configure option "--enable-show-type-use-in-abilint".
* configure.ac: Define the --enable-show-type-use-in-abilint
configure option. It defines the WITH_SHOW_TYPE_USE_IN_ABILINT
macro.
* include/abg-reader.h (read_translation_unit): Add an overload
that takes the read context.
(get_types_from_type_id, get_artifact_used_by_relation_map):
Declare new functions.
* src/abg-reader.cc (get_types_from_type_id)
(get_artifact_used_by_relation_map): Declare these functions as
friend of the read_context type.
(read_context::m_artifact_used_by_map):
(read_context::key_type_decl): Replace the shared_ptr<type_base>
type of the first parm by the equivalent type_base_sptr type.
(read_context::{record_artifact_as_used_by,
record_artifacts_as_used_in_fn_decl,
record_artifacts_as_used_in_fn_type}): Add new methods guarded by
the WITH_SHOW_TYPE_USE_IN_ABILINT macro.
(get_types_from_type_id, get_artifact_used_by_relation_map): Define
new functions guarded by the WITH_SHOW_TYPE_USE_IN_ABILINT macro.
(read_translation_unit): Define new overload.
(RECORD_ARTIFACT_AS_USED_BY, RECORD_ARTIFACTS_AS_USED_IN_FN_DECL)
(RECORD_ARTIFACTS_AS_USED_IN_FN_TYPE): Define new macros.
(build_function_decl, build_var_decl, build_qualified_type_decl)
(build_pointer_type_def, build_reference_type_def)
(build_function_type, build_array_type_def, build_enum_type_decl)
(build_typedef_decl, build_class_decl, build_union_decl): Use the
macros above to mark the relevant sub-types as used by the
artifact being built.
* tools/abilint.cc (struct artifact_use_relation_tree): Define new
type, guarded by the WITH_SHOW_TYPE_USE_IN_ABILINT macro.
(fill_artifact_use_tree, build_type_use_tree, emit_trace)
(emit_artifact_use_trace, emit_artifact_use_trace)
(show_how_type_is_used): Define static functions guarded by the
WITH_SHOW_TYPE_USE_IN_ABILINT macro.
(display_usage): Add doc string for the --show-type-use option,
guarded by the WITH_SHOW_TYPE_USE_IN_ABILINT macro.
(parse_command_line): Parse the --show-type-use option, guarded by
the WITH_SHOW_TYPE_USE_IN_ABILINT macro.
(main): Slight re-organisation to make the abixml file reading use
a read_context. That read context is then used to analyze how a
given type is used whenever the --show-type-use option is used.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Fix typos and try to improve some grammar in the files
in the top level directory.
* COMPILING: Improve grammar
* CONTRIBUTING: Improve grammar
* README: Improve grammar
* VISIBILITY: Improve grammar
Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
In the loop of write_referenced_types temporary shared_ptr objects are
created. In the case of a declaration, two shared_ptrs are created. It
is possible to code this so only one object is created.
This is a small optimisation with no change to tests or behaviour.
* src/abg-writer.cc (write_referenced_types): Create temporary
shared_ptr objects within each conditional branch instead of
outside the conditionals and within one of the branches.
Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
In the loop that emits declarations, the iterator already points to a
decl_base_sptr, there is no need to do another dynamic_cast. It is
also possible to simplify the loop and its conditionals.
There is no change to tests or behaviour.
* src/abg-writer.cc (decl_is_emitted): Make decl_base_sptr
argument a const reference.
(write_translation_unit): Eliminate a typedef and just use a
range-for loop without the extra dynamic cast for the non-type
case. Use else instead of continue to make it clear there are
only two possibilities.
Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
This was introduced in commit e2d45017 and was unused then.
* src/abg-writer.cc (write_elf_symbols_table): Drop unused
local variable emitted_syms.
Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
The type_hasher functor is no longer used.
* src/abg-writer.cc (type_hasher): Remove this unused functor.
Remove a following comment referencing it.
Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
In a version of the kernel binary referred to in this problem report,
the parameter 'skb' of the udp4_hwcsum function, which is of type
"pointer to struct sk_buff", indirectly refers to a pointer to a
declaration-only struct ip_mc_list.
In another version of that kernel binary, the same parameter skb of
the udp4_hwcsum function is still of type "pointer to struct sk_buff",
but in that case, the sk_buff indirectly refers to a pointer to a
fully defined struct ip_mc_list.
The first kernel only contains a decl-only struct ip_mc_list whereas
the second one contains a fully defined struct ip_mc_list.
This problem comes from the fact that in add_or_update_class_type, we
"reuse" the "struct sk_buff" that we've already seen in the same
binary, if any. Depending on the order in which types are defined in
the debug information, if the DIE for struct sk_buff that refers to a
decl-only struct ip_mc_list has already been "seen" by the
DWARF-reader, then add_or_update_class_type re-uses the IR of that DIE
that's been constructed already; otherwise, the IR for the struct
sk_buff represented by the current DIE is constructed.
This patch fixes the problem by always constructing an IR for the
DIE that is being seen, in add_or_update_class_type.
* src/abg-dwarf-reader.cc (add_or_update_class_type): Do not reuse
the IR for a DIE with the same textual representation as the one
we are seeing now.
* tests/data/test-annotate/test21-pr19092.so.abi: Adjust.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
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.
* src/abg-symtab-reader.cc
(symtab::add_alternative_address_lookups): Remove over-aggressive
assert.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Reviewed-by: Giuliano Procida <gprocida@google.com>
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>
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.
* src/abg-reporter-priv.cc (maybe_report_diff_for_symbol):
Make return void. Simplify and fix new-line emission. Remove
comma emission. Tweak CRC wording.
* src/abg-reporter-priv.h (maybe_report_diff_for_symbol):
Return void.
* tests/data/test-abidiff-exit/test-crc-report.txt: Shorten CRC
wording.
* tests/data/test-abidiff/test-crc-report.txt: Likewise.
* tests/data/test-diff-filter/test-PR27569-report-0.txt:
Likewise.
Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
This patch is based on a patch submitted to this bug report by George
Rawlinson <grawlinson@archlinux.org>.
It generates a man page for the kmidiff tool.
* doc/manuals/conf.py: Update copyright year. Generate man page
for the kmidiff tool.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Let the writer look through declaration-only enums for definitions.
This matches what get_preferred_type, write_class_decl and
write_union_decl do. No current test cases are affected.
* src/abg-writer.cc (write_enum_type_decl): Look through
declaration-only types the same as for structs and unions.
Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
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.
Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
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.
Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This fixes a copy/paste error in function documentation.
* tools/abidiff.cc
(emit_incompatible_format_version_error_message): Fix
parameter documentation.
Fixes: b251bc611e ("abidiff: include ABI XML versions when reporting a mismatch")
Signed-off-by: Giuliano Procida <gprocida@google.com>
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.
Signed-off-by: Giuliano Procida <gprocida@google.com>
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.
Fixes: 16207c4af7 ("Bug 28191 - Interpret DWARF 5 addrx locations")
Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
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.
Fixes: 16207c4af7 ("Bug 28191 - Interpret DWARF 5 addrx locations")
Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
CFI symbols need special interpretation and this work is performed by
the add_alternative_address_lookups method.
Some symbol addresses need to be "tweaked" to be correctly interpreted
and this must also happen in add_alternative_address_lookups.
In particular, this commit fixes ARM32 CFI symbol interpretation.
* src/abg-symtab-reader.cc
(symtab::add_alternative_address_lookups): Tweak function
addresses in the same manner as done in symtab::load_.
Signed-off-by: Giuliano Procida <gprocida@google.com>
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.
Signed-off-by: Giuliano Procida <gprocida@google.com>
This commit re-visit the commit below:
commit 1cfbff1b30
Author: Dodji Seketeli <dodji@redhat.com>
Date: Mon Jun 7 16:07:50 2021 +0200
rhbz1951526 - SELF CHECK FAILED for 'gimp-2.10'
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.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Fix-up for recent commit f0582fdbf1
"Replace use of deprecated Python 'imp' module with 'importlib'", and
commit cc1f38ffed
"Replace Python 'import importlib' with 'import importlib.machinery'",
because compatibility...
* tests/mockfedabipkgdiff.in: Handle several variants of Python
'imp', 'importlib' modules.
Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
Tested-by: Mark Wielaard <mark@klomp.org> (CentOS 7)
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
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:
abidw: ../../../libabigail-upstream/src/abg-ir.cc:25251: size_t
abigail::ir::hash_as_canonical_type_or_constant(const
abigail::ir::type_base*): Assertion `__abg_cond__' failed.
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>
Fix-up for recent commit f0582fdbf1
"Replace use of deprecated Python 'imp' module with 'importlib'", which...
[...] seems to have broken something on centos7 x86_64:
https://builder.wildebeest.org/buildbot/#/changes/7273
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'.
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>
... as is now documented in 'CONTRIBUTING'.
* tools/fedabipkgdiff: Handle 'koji.ConfigurationError'.
* configure.ac: Likewise.
* CONTRIBUTING: Document "fedabipkgdiff testing".
Documenting/providing a way to enable such testing, this commit can be
considered a sequel to commit 90d236a033
"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>
... instead of open-coding it, insufficiently.
On one system, I have a '/home' -> 'media/[...]/home' symlink, so:
$ readlink -f /home/thomas/.cache
/media/[...]/home/thomas/.cache
Now:
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):
(gdb) print extracted_dir_path()
$1 = "/home/thomas/.cache/libabigail/abipkgdiff-tmp-dir-upGgLK/package1"
Avoid that by just using 'convert_path_to_relative' here.
(I did not generally review the code for other such problems...)
* tools/abipkgdiff.cc (create_abi_file_path): Use
'convert_path_to_relative'.
Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
Currently we may run into this:
[...]
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...)
* tools/abipkgdiff.cc (compare_to_self): Respect
'create_abi_file_path' interface.
Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
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 26c41c060b
"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>
What got pushed in recent commit 497357cfd5
"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 90d236a033
"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
* configure.ac: Tune fedabipkgdiff dependencies detection.
Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
This section got added in commit 4f8c9b170d
"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.
Signed-off-by: Mark Wielaard <mark@klomp.org>
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>
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>
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
Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
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.
Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
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.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
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.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
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.
Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
