There are rare use cases where we do not want to compare the SONAME when
testing libraries for compatiblity or diffing libraries. This adds an
option to ignore the SONAME when doing the comparison. In these cases,
we will edit the application's DT_NEEDED to point to the other library.
This reuses the show_soname_change() function and slightly changes its
meaning to not only control if the sonames are printed but also if
they are compared. There didn't seem to be any other users of this
function and slight semantic change seemed harmless.
* doc/manuals/abicompat.rst - added new option
* doc/manuals/abidiff.rst - added new option to manpage
* src/abg-comparison.cc (compute_diff): don't bother comparing the
sonames if you aren't going to print them.
* tools/abicompat.cc (options::ignore_soname): Add new data
member.
(parse_command_line): Support the new --ignore-soname command line
option.
(display_usage): Add a description string for the new
--ignore-soname command line option.
(create_diff_context): Set the diff_context::show_soname_change
from the new options::ignore_soname data member.
* tools/abidiff.cc (options::ignore_soname): Add new data member.
(display_usage): Add a description string for the new
--ignore-soname command line option.
(parse_command_line): Support the new --ignore-soname command line
option.
(set_diff_context_from_opts): Set the
diff_context::show_soname_change from the new
options::ignore_soname.
Signed-off-by: Ben Woodard <woodard@redhat.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
64-bit ARM addresses normally have bits 47 to 63 as either all 0 or
all 1. If tagging is used, bits 56 to 63 can vary, but the
interpretation of such values is as if the bits were all the same as
bit 55.
Such tagging is used for HWASAN and this affects the ELF symbol values
seen in shared libraries.
This commit changes the interpretation of 64-bit ARM symbol values by
unconditionally extending bit 55 into bits 56 to 63.
This fixes missing types for symbols in HWASAN-compiled libraries.
* src/abg-elf-helpers.cc: (architecture_is_arm64): Add helper.
* src/abg-elf-helpers.h: Likewise.
* src/abg-symtab-reader.cc: (get_symbol_value): Adjust 64-bit
ARM symbol values by extending bit 55 into bits 56 to 63.
Signed-off-by: Giuliano Procida <gprocida@google.com>
A previous changes duplicated some logic for tweaking ELF symbol
values (and possibly updating some bookkeeping information).
This change refactors the code so the logic is in one place, in
symtab::get_symbol_value.
* src/abg-symtab-reader.cc (symtab::load_): Replace address
tweaking logic with a call to get_symbol_value.
(symtab::add_alternative_address_lookups): Likewise.
(symtab::get_symbol_value): New function containing address
tweaking logic for PPC and ARM.
Signed-off-by: Giuliano Procida <gprocida@google.com>
This patch avoids multiple inclusion of `var-decl' node for the same
field in anonymous struct/union declaration in the abixml, e.g:
struct uprobe_task {
union {
struct {
unsigned long vaddr;
};
struct {
int dup_xol_work;
};
};
};
Three `var-decl' nodes are written in abixml file, expected a single
one:
<var-decl name='dup_xol_work' .../>
* src/abg-ctf-reader.cc (process_ctf_sou_members): Remove
CTF_MN_RECURSE flag.
* tests/data/Makefile.am: Add new input test files.
* tests/data/test-read-ctf/test-anonymous-fields.c: New
test file.
* tests/data/test-read-ctf/test-anonymous-fields.o: New
expected test output.
* tests/data/test-read-ctf/test-anonymous-fields.o.abi:
Likewise.
* tests/test-read-ctf.cc: Add new test.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
entry_of_file_with_name uses `string_ends_with' to test if the
filename given in the `fts_path' member of `FTSENT' matches the
`fname' argument. This result is not correct when in the current
directory there are file with names: "./rmdir-xyx", "./dir-xyz" it
returns true for both files, this patch fixes this ambiguity by using
`basename' instead of `string_ends_with'.
* src/abg-tools-utils.cc (entry_of_file_with_name): Replace
call `string_ends_with' by `basename'.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Add double dash to option name in man page. A minor typo.
* doc/manuals/abipkgdiff.rst: add missing `--`
Signed-off-by: Ben Woodard <woodard@redhat.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Undefined struct/union forward declaration types are not serialized in
abixml representation using `class-decl' node.
For instance, consider:
struct key_type;
typedef void (*key_restrict_link_func_t)(struct key_type *type);
Expected node:
<class-decl name='key_type' ... is-declaration-only='yes'.../>
* src/abg-ctf-reader.cc (process_ctf_forward_type): New
function.
(process_ctf_type): New CTF_K_FORWARD case.
* tests/data/test-read-ctf/test-forward-undefine-type-decl.c:
New testcase.
* tests/data/test-read-ctf/test-forward-undefine-type-decl.abi:
New expected result.
* tests/data/test-read-ctf/test-forward-undefine-type-decl.o
New test input.
* tests/test-read-ctf.cc: Add new testcase to test harness.
* tests/data/Makefile.am: Add new test input files to test harness.
Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This should address PR28939, reported at
https://sourceware.org/bugzilla/show_bug.cgi?id=28939
In function_type_diff::chain_into_hierarchy, the diff details
represented by the diff nodes of the function parameters are already
sorted in the vector priv->sorted_changed_parms_by_id_. Note that the
sorting of function parameter diff nodes was done using a criterion
that takes into account the position of each function parameter.
Members of the vector priv->sorted_changed_parms_by_id_ are then added
to the diff graph using diff::append_child_node.
The problem is that diff::append_child_node sorts the children nodes
/again/, this time using a criterion that is different from the one
used in function_type_diff::chain_into_hierarchy. This is wrong.
This patch prevents diff::append_child_node from sorting the children
node because they have been sorted already.
* src/abg-comparison.cc (diff::append_child_node): Do not sort
children nodes here because they must have been sorted already.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.txt: Adjust.
* 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-filter/test41-report-0.txt: Likewise.
* tests/data/test-diff-pkg/libICE-1.0.6-1.el6.x86_64.rpm--libICE-1.0.9-2.el7.x86_64.rpm-report-0.txt:
Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
During the traversal of the graph of diff nodes, diff::traverse does
two things:
1/ If the generic view of the diff node of is not yet connected to
the diff graph, then connect it. Note that the typed view of the
diff node is always connected to the diff graph.
2/ Visit the diff node using its generic view and visit its
children nodes.
Looking at the part 1/, I realized that the code connecting the
generic view of the diff node to the diff graph was duplicated in
every single type of diff node.
This patch put that code into diff::finish_diff_type and makes all the
different kinds diff node use that (virtual) member function,
effectively factorizing that code.
* include/abg-comparison.h ({distinct, var, pointer, reference,
array, qualified_type, enum, class_or_union, class, union, base,
scope, fn_parm, function_type, function_decl, type_decl,
typedef}_diff::finish_diff_type): Remove these declarations.
* src/abg-comparison.cc ({distinct, var, pointer, reference,
array, qualified_type, enum, class_or_union, class, union, base,
scope, fn_parm, function_type, function_decl, type_decl,
typedef}_diff::finish_diff_type): Remove these definitions.
(diff::finish_diff_type): Add the code to connect the children
nodes to the current node, making the generic view of the diff
node usable to walk the graph.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
To add a node to the diff graph, what is used is the "generic view"
(or generic API) of the diff node.
To understand the difference between the generic view and the typed
view of diff nodes, this patch adds comments to the diff node API.
* include/abg-comparison.h (class diff): Add comments to this class.
(diff::chain_into_hierarchy): Add comment to this method.
* src/abg-comparison-priv.h (diff::priv): Add comment to this class.
* src/abg-comparison.cc (diff::finish_diff_type): Add comment to
this method.
(diff::traverse): Add comment.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Variadic parameter types are one of the rare types that should not be
canonicalized, just like the void type. These types are created by
the system (compiler/libabigail) and thus can be compared in O(1)
time. So far, I forgot to prevent the canonicalization of variadic
parameter type everywhere, as should have been the case since the
recent introduction of the is_non_canonicalized_type function. This
patch fixes that.
* src/abg-ir.cc (is_non_canonicalized_type): Recognize variadic
parameter types.
* tests/data/test-diff-filter/test-PR28013-fn-variadic.c.{0,1}.abi: New test inputs.
* tests/data/test-diff-filter/test-PR28013-fn-variadic.c.report.txt: Likewise.
* tests/data/Makefile.am: Add the new test files to source distribution.
* tests/test-diff-filter.cc (in_out_specs): Add the new tests to
this harness.
* 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/test0.abi: Likewise.
* tests/data/test-annotate/test13-pr18894.so.abi: Likewise.
* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-diff-filter/test-PR28013-fn-variadic.c.0.abi: Likewise.
* tests/data/test-diff-filter/test-PR28013-fn-variadic.c.1.abi: Likewise.
* tests/data/test-diff-filter/test-PR28013-fn-variadic.c.report.txt: Likewise.
* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi: Likewise.
* tests/data/test-read-dwarf/libtest23.so.abi: Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libaaudio.so.abi: Likewise.
* tests/data/test-read-dwarf/test-libandroid.so.abi: Likewise.
* tests/data/test-read-dwarf/test0.abi: Likewise.
* tests/data/test-read-dwarf/test0.hash.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This comes from trying to fix
https://sourceware.org/bugzilla/show_bug.cgi?id=26646.
During DIE comparison for the purpose of DIE canonicalization, we need
to detect a loop due to a recurring aggregate comparison. Thus, the
compare_dies function returns true in when it detects that it's
comparing two aggregate that are already being compared. In that
situation of "detected aggregate redundancy", even though the
comparison seemingly succeeds, no canonical type propagation should
happen.
This patch prevents canonical type propagation when compare_dies
return true to signal aggregate redundancy detection.
This addresses https://sourceware.org/bugzilla/show_bug.cgi?id=26646#c21.
* src/abg-dwarf-reader.cc (compare_dies): Do not propagate
canonical type when aggregate redundancy is detected.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch addresses the comment
https://sourceware.org/bugzilla/show_bug.cgi?id=26646#c15 of the
reported problem.
The core of the problem seems to be that when compare_dies compares
two DW_TAG_{structure,union}_type, it "gives up" when the comparison
stack contains more than 5 such DIEs being compared. Giving up means
that it considers the two DIEs to be equivalent if they have the same
size and kind, basically. This is to avoid infinite loops or too long
loops that we've seen with artificial DWARF input generated by
fuzzers.
This patch fixes that by better using the "aggregates_being_compared"
data structure that is meant to prevent looping infinitely over
aggregate DIEs that might be recursively defined. That data structure
now contains the DWARF offset pairs of the DIEs being compared, rather
than their "string representation". Lookups in that data structure
should now be faster and more precise.
The artificial limit of the comparison stack not having more than 5
DIEs is now lifted.
* src/abg-dwarf-reader.cc (struct dwarf_offset_pair_hash)
(dwarf_offset_pair_set_type): Define new type.
(die_offset, has_offset_pair, insert_offset_pair)
(erase_offset_pair): Define new static helper functions.
(compare_dies): Use a set of DWARF offsets for the
'aggregates_being_compared' data structure, rather than a set of
string representation of the DIEs. Always look at the size of the
types being compared first so that we can get out quickly if they
differ. For DIEs of DW_TAG_{union,struct}_type kind, don't limit
the depth of the stack of DIEs being compared to 5; so we don't
consider two types as being equal just because the depth of the
stack being compared is 5 and the two types have the same size and
are equal. Hopefully things don't take too long.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While looking at something else, I realized that some parts of the
comment of the equal operator of enumerator are obsolete. I am
removing it.
* src/abg-ir.cc (enum_type_decl::enumerator::operator==): Remove
the obsolete parts from the comment.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
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>
