The removed functions section is introduced by the string "Removed
functions", whereas the removed variables section is introduced by the
string "Deleted variables". This patch makes the latter be "Removed
variables" just like for the functions.
* src/abg-comparison.cc (corpus_diff::report): Introduce the
section of removed variables with the string "Removed variable",
rather than with the string "Deleted variable".
* tests/data/test-abicompat/test2-var-removed-report-0.txt: Adjust.
* tests/data/test-diff-suppr/test18-suppr-removed-var-report-0.txt: Likewise.
* tests/data/test-diff-suppr/test18-suppr-removed-var-report-3.txt: Likewise.
* tests/data/test-diff-suppr/test18-suppr-removed-var-report-5.txt: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When computing the set of added function or variable symbols, if a
symbol S with no version symbol was present in a given corpus and that
symbol gained a *DEFAULT* version V in the second corpus, we should
not consider that a new symbol S was added (and that the former S was
removed) because:
1/ S was already present in the first corpus
2/ applications linked to the first corpus and that were using S
(with no version) there, will automatically use the S with version V
in the second corpus, without needing any re-linking; the
power of symbol versioning!
Rather, it's just that S gained a default symbol version.
This patch implements that.
* include/abg-corpus.h (corpus::{lookup_function_symbol,
lookup_variable_symbol}): Take a elf_symbol::version object,
rather than a string representing the version. Add an overload
that takes an elf_symbol.
* src/abg-corpus.cc (find_symbol_by_version): New static function.
(corpus::{lookup_function_symbol, lookup_variable_symbol}): Take a
elf_symbol::version object, rather than a string representing the
version. Add an overload that takes an elf_symbol. If the looked
up symbol has no version and if the corpus contains a symbol with
the same name and with a default version, then return that latter
symbol if the corpus doesn't contain a symbol with the same name
and empty version.
* src/abg-comparison.cc
(class_diff::ensure_lookup_tables_populated): Adjust.
(corpus_diff::priv::ensure_lookup_tables_populated): Before
deciding that a symbol has been added, if the symbol has a default
version, make sure no symbol with the same name and without
version was present in the former corpus. Similarly, before
deciding that a symbol has been removed, if the symbol has no
version, make sure the latter corpus has no symbol with the same
name and with a default version.
* tests/data/test-diff-dwarf/test12-report.txt: Adjust. The
function should not be considered as added, because its symbol
(and version) was already present in the former DSO.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It appears the comparison engine was counting more deleted variables
than necessary, due to a thinko. Fixed thus.
* src/abg-comparison.cc
(corpus_diff::priv::ensure_lookup_tables_populated): Once we have
computed a set of potentially deleted variables that turned out to
contain variables that were actually *NOT* deleted, really take
these into account by removing these false positives from the set
of deleted *variables*. We were trying to delete these from the
set deleted *functions*; woops, I guess this was a copy & paste
error.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-ir.cc (type_has_non_canonicalized_subtype): Once the
type has been traversed, just test if the visitor has accumulated
the 'has_non_canonical_type' property.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* include/abg-ir.h (struct ir_node_visitor): Fix the wording of
the comment of this type.
* src/abg-dwarf-reader.cc (build_ir_node_from_die): Fix the
filling of the text of the comment of the code that chooses to
perform early canonicalizing.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When reading DWARF, a declaration-only class was loosing its
declaration-only-ness just because a member type was being added. Bad
things was then happening next because that (normally
declaration-only) class was then considered as being suitable for
early canonicalizing, while it wasn't (yet), in reality. Its
canonicalizing should have been deferred.
This issue was spotted when comparing
kdebase-workspace-4.3.4-29.el6_6.x86_64.rpm and,
kdebase-workspace-4.3.4-30.el6_6.x86_64.rpm, using their associated
debug info. The issue was happening precisely when comparing their
usr/lib64/kde4/kwin4_effect_builtins.so DSOs, precisely; it was
leading to a crash.
* src/abg-dwarf-reader.cc (build_class_type_and_add_to_ir):
Adding a new member type shouldn't remove the
declaration-only-ness of the class.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This entry point is to test if there are still ABI changes between two
corpora after applying suppression specifications.
* include/abg-comparison.h (corpus_diff::has_net_changes): Declare
new member function.
* src/abg-comparison.cc (corpus_diff::has_net_changes): Define it.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While looking further in the issue Sinny Kumari reported, I realized
that the weak mode wasn't working in that example either.
It turned out that synthesizing qualified types was not working
because we were just looking them up in the binary, rather than
looking up the un-qualified underlying type and then synthezing the
resulting qualified type.
This patch just does that.
* include/abg-fwd.h
(synthesize_type_from_translation_unit): Declare new function.
(synthesize_function_type_from_translation_unit): Make the
translation_unit parameter non-const because the function needs to
bind the life time of the synthesized function to the life time of
the translation unit. Make this function be a friend of
abigail::ir::translation_unit.
(synthesize_function_type_from_translation_unit):
* src/abg-ir.cc (translation_unit::priv::synthesized_types_): New
data member.
(synthesize_type_from_translation_unit): Define new function.
(synthesize_function_type_from_translation_unit): Make the
translation_unit parameter non-const. If the return is void, then
take that in account carefuly. Rather than just looking up the
type of parameters and return value, synthesize them too,
especially when they are qualified types. Bind the life time of
the synthesized function type to the lifetime of the translation
unit.
* tests/data/test-abicompat/test7-fn-changed-report-1.txt: New
test reference output.
* tests/test-abicompat.cc (in_out_spec): Run the harness on the
exisiting test7-fn-changed-app and libtest7-fn-changed-libapp-v1
but in weak mode this time.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Sinny Kumari reported that abicompat is failing to report ABI changes
on a library linked to a small test program. It turned out that the
code that compute if a given exported function is to be kept by
looking at the white list of symbols to keep has a bug in which the
versions of the symbols of the white list were not being reset as they
should. Fixed thus.
* src/abg-ir.cc (elf_symbol::get_name_and_version_from_id): Always
set the version and name of the symbol.
* src/abg-corpus.cc
(corpus::exported_decls_builder::{keep_wrt_id_of_fns_to_keep,
keep_wrt_id_of_vars_to_keep}): Reset the symbol name *and* version
before passing it. This is redundant with the fix in
elf_symbol::get_name_and_version_from_id() that always set the
symbol name and version now, but I felt it makes it easier to
understand the fix overall.
* tests/data/test-abicompat/libtest7-fn-changed-libapp-v{0,1}.so:
New test input binaries.
* tests/data/test-abicompat/test7-fn-changed-app: Likewise.
* tests/data/test-abicompat/test7-fn-changed-{app, libapp-v0,
libapp-v1}.c: Source code of the binary test inputs above.
* * tests/data/test-abicompat/test7-fn-changed-{libapp-v0,
libapp-v1}.h: Likewise.
* tests/data/test-abicompat/test7-fn-changed-report-0.txt: Test
input.
* tests/data/Makefile.am: Add the new test material above to
source distribution.
* tests/test-abicompat.cc (int_out_specs): Add the test inputs
above to the set of inputs this test harness has to run over.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
We were using the unsafe tmpnam function in abilint. This patch
creates a helper type abigail::tools_utils::temp_file that does away
with the use tmpnam in abilint.
* include/abg-tools-utils.h (abigail::tools_utils::temp_file):
Declare new type.
(abigail::tools_utils::temp_file_sptr): New typedef.
* src/abg-tools-utils.cc (temp_file::priv): Define new type.
(temp_file::{temp_file, is_good, get_path, get_stream, create}):
Define new member functions.
* tools/abilint.cc (main): Do not use tmpnam anymore. Use the new
abigail::tools_utils::temp_file type instead.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
abipkgdiff extracts the content of the first package in a directory
named <tmpdir>/package1 and the content second package in
<tmpdir>/package2. If two independant instances of abipkgdiff are
launched at the same time, they are going to walk on each others'
toes, to say the least.
This patch extracts the content of each package in directory named
<tmpdir>/<randomname>/package1, where randomname is supposed to be a
random number, and so should be unique, most of the time.
I guess we should try harder to generate a randomname that is unique
when we see that the directory <tmpdir>/<randomname> exists already,
but for now, what we have is good enough, or at least better than what
we have had so far.
* include/abg-tools-utils.h (get_random_number)
(get_random_number_as_string): Declare new functions.
* src/abg-tools-utils.cc (get_random_number)
(get_random_number_as_string): Define them.
* tools/abipkgdiff.cc
(package::extracted_package_parent_dir_path): New data member.
(package::package): Initialize
package::extracted_package_parent_dir_path to
<tmpdir>/<randomname>, with randomname being a random number
represented as a string.
(extract_rpm): Make sure to create a hierarchy of directories, not
just a directory.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In change reports for function sub-type changes, for the C language,
when the name of the function is different from its linkage name, even
when the function symbol has no aliases, show the symbol information
of the function.
* include/abg-ir.h (translation_unit::language): New enum type.
(translation_unit::{get_language, set_language}): Declare new
accessors.
(translation_unit_language_to_string)
(string_to_translation_unit_language, is_c_language)
(is_cplus_plus_language): Declare new functions.
* src/abg-ir.cc (translation_unit::priv::language_): New data
member.
(translation_unit::priv::language_): Initialize it.
(translation_unit::{set_language, get_language}): Define new
member functions.
(translation_unit_language_to_string)
(string_to_translation_unit_language, is_c_language)
(is_cplus_plus_language): Define new functions.
* src/abg-dwarf-reader.cc (dwarf_language_to_tu_language): New
static function.
(build_translation_unit_and_add_to_ir): Read the language of the
translation unit.
* src/abg-comparison.cc (corpus_diff::report): When reporting a
change in a function sub-type, if we are in C language translation
unit, if the function name is different from its linkage name,
even if the symbol doesn't have any alias, show symbol
information.
* src/abg-reader.cc (read_translation_unit_from_input): Read the
'language' property of the translation unit, if present.
* src/abg-writer.cc (write_translation_unit): Write the 'language'
property to the translation unit, if present.
* tests/data/test-read-dwarf/test0.abi: Adjust for the new
'language' property of the 'abi-instr' element.
* tests/data/test-read-dwarf/test1.abi: Likewise.
* tests/data/test-read-dwarf/test2.so.abi: Likewise.
* tests/data/test-read-dwarf/test3.so.abi: Likewise.
* tests/data/test-read-dwarf/test4.so.abi: Likewise.
* tests/data/test-read-dwarf/test5.o.abi: Likewise.
* tests/data/test-read-dwarf/test6.so.abi: Likewise.
* tests/data/test-read-dwarf/test7.so.abi: Likewise.
* tests/data/test-read-dwarf/test8-qualified-this-pointer.so.abi:
Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
With this patch it's now possible to express the soname or name of the binary
file that contains the ABI artifacts the suppression specifications
should apply to.
* include/abg-comparison.h (suppression_base::priv_): Make this
pimpl member protected.
(suppression_base::set_file_name_regex_str)
(get_file_name_regex_str, get_soname_regex_str)
(set_soname_regex_str): Declare new accessors.
(function_suppression::{suppresses_function,
suppresses_function_symbol}): Take a diff_context_sptr.
(variable_suppression::{suppresses_variable,
suppresses_variable_symbol}): Take a diff_context_sptr.
* src/abg-comparison.cc
(suppression_base::priv::{file_name_regex_str_, file_name_regex_,
soname_regex_str_, soname_regex_}): Define new data members.
(suppression_base::priv::get_file_name_regex_str)
(get_soname_regex_str): Define new member functions.
(suppression_base::set_file_name_regex_str)
(get_file_name_regex_str, get_soname_regex_str)
(set_soname_regex_str): Define new accessors.
(type_suppression::suppresses_diff): Evaluate file_name_regexp and
soname_regexp.
(read_type_suppression): Fix the reading of the "label" property.
Read the file_name_regexp and soname_regexp properties.
(function_suppression::{suppresses_function,
suppresses_function_symbol): Take a diff_context_sptr parameter.
Evaluate file_name_regexp and soname_regexp properties.
(function_suppression::suppresses_diff): Adjust for the api change
of function_suppression::suppresses_function().
(read_function_suppression): Read the file_name_regexp and
soname_regexp properties.
(variable_suppression::suppresses_variable): Take a
diff_context_sptr parameter and evaluate file_name_regexp and
soname_regexp properties.
(variable_suppression::suppresses_variable_symbol): Likewise.
(variable_suppression::suppresses_diff): Adjust for the api change
of variable_suppression::suppresses_variable().
(read_variable_suppression): Read the file_name_regexp and
soname_regexp properties.
(function_is_suppressed, variable_is_suppressed): Take a
diff_context_sptr parameter.
(corpus_diff::priv::apply_suppressions_to_added_removed_fns_vars):
Adjust.
* doc/manuals/libabigail-concepts.rst: Document file_name_regexp
and soname_regexp in the manual.
* tests/data/test-diff-suppr/libtest24-soname-v{0,1}.so: New test
binary input files.
* tests/data/test-diff-suppr/test24-soname-report-{0,4}.txt: New
test input files.
* tests/data/test-diff-suppr/test24-soname-suppr-{0,4}.txt:
Likewise.
* tests/data/test-diff-suppr/test24-soname-v{0,1}.cc: Source code
of the binary test input files above.
* tests/data/Makefile.am: Add the new test material above to
source distribution.
* tests/test-diff-suppr.cc (in_out_spec): Add the new test inputs
to the set of tests this harness has to run over.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When evaluating the way the diff node has been reached, do not crash
on diff nodes that are pointer_diff to something else but a type diff,
because there can be pointers to distinct_diff nodes, for instance.
* src/abg-comparison.cc (type_suppression::suppresses_diff): When
evaluating the reach_kind property, do not crash on diff nodes
that are pointer_diff or reference_diff to something else but a
type diff.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-dwarf-reader.cc (get_soname_of_elf_file): Better wording
of the apidoc of this function.
* tools/abipkgdiff.cc (verbose, elf_file_paths): Add apidoc for
these global variables.
(struct options, ): Add apidoc for these types.
(options::{erase_extraction_directory,
erase_extraction_directories}, display_usage, extract_rpm)
(erase_created_temporary_directories, extract_package)
(file_tree_walker_callback_fn, compare)
(create_maps_of_package_content)
(extract_package_and_map_its_content, prepare_packages, compare)
(parse_command_line): Add apidoc for these functions.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* abipkgdiff.cc (get_soname_of_elf_file): Fix spacing.
* tools/abipkgdiff.cc (elf_file_paths): Make this global variable
static.
(extract_rpm): Rename parameter pkg_path name into package_path.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Directly using elfutils from abipkgdiff.cc feels like a taping into
the wrong abstraction layer from this level. So this patch moves the
determination of the type of elf file into abg-dwarf-reader.cc and
uses it from there. The patch also simplifies the instantiation of
types elf_file and package, from abipkgdiff.cc.
* abg-dwarf-reader.h (enum elf_type): Move this declaration here
from abipkgdiff.cc to here.
(get_type_of_elf_file): Declare this new function.
(get_soname_from_elf): Change this to take a path to the elf file
rather than a Elf* handler. So now to use this, the user doesn't
have to get her hand dirty with elfutils.
* src/abg-dwarf-reader.cc (get_soname_from_elf): Change this to
take a path to the elf file rather than a Elf* handler.
(elf_file_type): Move this static function here, from
abipkgdiff.cc.
(get_type_of_elf_file): New function. This has been factorized
out of create_maps_of_package_content() in abipkgdiff.cc.
* tools/abipkgdiff.cc (class elf_file): Changed struct elf_file
into this. Make the default constructor private.
(elf_file::elf_file): Change the constructor to just take the path
to the elf_file. The base name, soname and elf file type are now
computed from the path file, in the constructor. This makes
instantiation much much easier from the point of view of the user
of the type.
(struct abi_diff): Renamed struct abi_changes into this.
(abi_diff::has_changes): Define new member function.
(abi_diffs): Remove this global variable.
(package::package): Remove the elf file type from the set of
parameters of this constructor. Rather, compute that elf file
type from the path to the elf file, in the constructor. Again,
this eases the use of the type.
(elf_file_type): Remove this from here, as it got moved to
abg-dwarf-reader.cc.
(compare): In the elf_file overload, return true if the comparison
yields ABI changes.
(create_maps_of_package_content): Do not fiddle with elfutils
stuff here. Rather, just instantiate elf_file and the analyzing
of the file magically happens.
(compare): Make the package overload take an abi_diff as output
parameter, rather than populating a global variable in return.
(compare): Add an other overload for package that doesn't take the
abi_diff as output parameter and write it in terms of the previous
one.
(main): Adjust as the instantiation of package is now simpler.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Initially, fetching SONAME from a given DSO file was done in
abipkgdiff.cc. But, this function fits better when defined in
abg-dwarf-reader.cc in order to make it usable by other tools if
needed. For consistancy, get_soname() function has been renamed to
get_soname_from_elf().
* include/abg-dwarf-reader.h (get_soname_from_elf): Declare
new function
* src/abg-dwarf-reader.cc (get_soname_from_elf): Define new
function
* tools/abipkgdiff.cc (get_soname): Remove function
(pkg_diff): Call get_soname_from_elf() instead of get_soname()
Signed-off-by: Sinny Kumari <sinny@redhat.com>
To run abipkgdiff between two pacakges, it should know whether
input files are valid pacakge file or not. This patch detects RPM and SRPM
pacakge file type. abipkgdiff uses it to know whether input files are
valid RPM pacakge file or not.
* include/abg-tools-utils.h (file_type): Added member
FILE_TYPE_RPM and FILE_TYPE_SRPM
(operator<<): New function declaration.
* src/abg-tools-utils.cc (file_type): Detect RPM and
SRPM file type
(operator<<): New function definition
* tools/abidiff.cc (main): Check for RPM and SRPM
file type as well.
* tools/abilint.cci (main): Check for RPM and SRPM file
type as well.
* tools/abipkgdiff.cc (main): Check whether input files
to abipkgdiff are valid RPM files or not.
Signed-off-by: Sinny Kumari <sinny@redhat.com>
* src/abg-dwarf-reader.cc
(read_context::find_symbol_table_section): Allow returning a nil
pointer to symbol table.
(read_context::lookup_elf_symbol_from_index): Return an empty elf
symbol if we got a nil pointer to symbol table.
(read_context::load_symbol_maps): If no symbol table is found then
consider that the symbol maps loading failed.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-comparison.cc (class_diff::report): Do not emit new line
unless the diff is to be reported.
* tests/data/test-diff-filter/test25-cyclic-type-report-0.txt: Adjust.
* tests/data/test-diff-filter/test26-qualified-redundant-node-report-0.txt: Adjust.
* tests/data/test-diff-filter/test27-redundant-and-filtered-children-nodes-report-1.txt:
: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It turned out we need to read all XML ABI files that were still
emitting empty types (nothing) to represent void types, e.g, for
function returning void. So the type comparison code needs to accept
nil types again.
* src/abg-comparison.cc (compute_diff): In the overload of
type_base_sptr accept nil types.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While working on something else, it turned out that we need to cleanup
(de-allocate) the map of canonical types when all the translation
units that own types are de-allocated. Otherwise, when new
translation units are created later, the types in the canonical types
map become unrelated to the types in these new translation units,
leading to memory management issues.
This patch introduces a "usage watchdog" which detects when no
translation unit uses the type system anymore. That usage watchdog is
then used in the destructor of the translation_unit type to
de-allocate the global data that is logically owned by by the type
system.
The patch also changes the API to read translation units and corpora
in a way that forces users to get a handle on the resulting shared
pointer.
* include/abg-ir.h (type_base::canonical_types_map_type): Move
this typedef into abg-ir.cc and out of the type_base namespace.
(type_base::get_canonical_types_map): Likewise.
* src/abg-ir.cc (canonical_types_map_type): New typedef that got
moved here from type_base::canonical_types_map_type.
(get_canonical_types_map): Likewise got moved here from
type_base::get_canonical_types_map. Made static in the process.
(class usage_watchdog): New type.
(usage_watchdog_sptr, usage_watchdog_wptr): New typedefs.
(get_usage_watchdog, get_usage_watchdog_wptr, ref_usage_watchdog)
(maybe_cleanup_type_system_data): New static functions.
(translation_unit::priv::usage_watchdog_): Add new data member.
(translation_unit::priv::priv): Get a reference on the usage
watchdog.
(translation_unit::priv::~priv): If the usage watchdog says that
the type system is not used, then cleanup the global data
logically owned by the type system.
* include/abg-dwarf-reader.h (read_corpus_from_elf): Make this
return a corpus and set the status by reference using a parameter.
* src/abg-dwarf-reader.cc (read_corpus_from_elf): Implement the
above.
* include/abg-reader.h (read_translation_unit_from_file)
(read_translation_unit_from_buffer)
(read_translation_unit_from_istream): Remove the overloads that do
not return a translation_unit_sptr and that pass it as a
parameter. Only keep the overloads that return a
translation_unit_sptr, forcing users of the API to own a proper
reference on the resulting translation_unit pointer. That is
important to handle the life time of the global data of the type
system that need to be cleared when the last translation unit is
de-allocated.
* src/abg-reader.cc (read_translation_unit_from_input): Make this
return a translation_unit_sptr.
(read_translation_unit_from_file)
(read_translation_unit_from_buffer)
(read_translation_unit_from_istream): Remove the overloads that do
not return a translation_unit_sptr and that pass it as a
parameter. Only keep the overloads that return a
translation_unit_sptr.
(read_to_translation_unit): Make this return a
translation_unit_sptr.
* tests/print-diff-tree.cc (main): Adjust.
* tests/test-diff-dwarf.cc (main): Likewise.
* tests/test-ir-walker.cc (main): Likewise.
* tests/test-read-dwarf.cc (main): Likewise.
* tests/test-read-write.cc (main): Likewise.
* tools/abicompat.cc (main): Likewise.
* tools/abidiff.cc (main): Likewise.
* tools/abidw.cc (main): Likewise.
* tools/abilint.cc (main): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In abg-comparison.h, there is no function to test if a given
corpus_diff carries incompatible or subtype (after having applied
suppression specifications) ABI changes. So this patch factorizes the
code of abidiff.cc to provide these features to corpus_diff.
* include/abg-comparison.h (corpus_diff::{has_incompatible_changes,
has_net_subtype_changes}): Declare new member functions.
* src/abg-comparison.cc (corpus_diff::{has_incompatible_changes,
has_net_subtype_changes}): Define them.
* abidiff.cc (main): Use the new member functions above.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It appears there are some missing new lines in the diff report.
Fixed thus.
* src/abg-comparison.cc (class_diff::report): The overload of
represent() for instances of var_decl does not emit new lines. So
the caller must ensure a new line is emitted.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Suppose a function private_foo() has a symbol private_foo and also a
another one (an alias) named public_foo. Then suppose we want to
filter out sub-type changes to private_foo(). But then we still want
to see changes to public_foo.
This patch does add this feature. The [suppress_function] directive
now has a new (hidden) boolean 'allow_other_aliases' property. When
set to 'yes' or 'true', if the function being looked at has an alias
symbol that does *NOT* match the other properties of the directive,
then the directive doesn't suppress reports for the function. This
new property is set to yes by default.
This means that when a function has got multiple aliases, to suppress
the function, one needs to write a regular expression that matches the
names of aliases. Otherwise the function will not be suppressed.
* include/abg-comparison.h (function_suppression::{get,
set}_allow_other_aliases): Declare new member functions.
* src/abg-comparison.cc
(function_suppression::priv::allow_other_aliases_): New data
member.
(function_suppression::priv::priv): Initialize it to 'true'.
(function_suppression::{get, set}_allow_other_aliases): Define new
member functions.
(read_function_suppression): Parse the new "allow_other_aliases"
property.
(function_suppression::suppresses_function): Update to evaluate
the new 'allow_other_aliases' property when there is a property to
match against some a symbol name of the function.
(corpus_diff::report): Fix the printing of function aliases when
printing sub-type changes to properly emit the plural of the word
'symbol' when the function has several aliases.
* include/abg-ir.h (elf_symbol::get_number_of_aliases): Declare
new member function.
* src/abg-ir.cc (elf_symbol::get_number_of_aliases): Define new
member function.
* doc/manuals/libabigail-concepts.rst: Update manual.
* tests/data/test-diff-dwarf/test5-report.txt: Adjust.
* tests/data/test-diff-suppr/libtest23-alias-filter-v0.so: New
test input.
* tests/data/test-diff-suppr/libtest23-alias-filter-v1.so: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-0.suppr: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-1.suppr: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-2.suppr: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-3.suppr: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-4.suppr: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-report-0.txt: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-report-1.txt: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-report-2.txt: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-report-3.txt: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-report-4.txt: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-report-5.txt: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-v0.c: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-v1.c: Likewise.
* tests/data/test-diff-suppr/test23-alias-filter-version-script: Likewise.
* tests/data/Makefile.am: Add the new test stuff to source
distribution.
* tests/test-diff-suppr.cc (in_out_spec): Add the tests inputs
above to the list of input to run over.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-ir.cc (elf_symbol::get_aliases_id_string): Finish the
incomplete apidoc for this member function.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The report emitted by abidiff now tells the user about the aliases of
the current function, when that function has some sub-type changes.
* include/abg-ir.h (elf_symbol::get_aliases_id_string): Declare
new overload.
* src/abg-ir.cc (elf_symbol::get_aliases_id_string): Define new
overload.
* src/abg-comparison.cc (corpus_diff::report): For functions with
sub-type changes report their aliases. Do not do this if the
function is a constructor or destructor because these almost
always have aliases, at least with GCC and the developer most
certainly has not done anything special for that; she would thus
be uselessly surprised by that remote implementation detail.
* tests/data/test-diff-dwarf/test5-report.txt: Adjust test.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The comparison code was too eager in comparing class types because it
was comparing static data members in the process. This was causing
some spurious false positives about functions or variables sub-type
changes. This patch fixes that by not comparing static data members
when comparing class types.
* include/abg-ir.h (class_decl::get_non_static_data_members):
Declare new data members.
* src/abg-comparison.cc
(class_diff::ensure_lookup_tables_populated): Only look at
non-static data members.
(compute_diff): In the overload for class_decl, only compare
non-static data members.
* src/abg-hash.cc (class_decl:#️⃣:operator()): Do not hash
static data members members hashing a class_decl.
* src/abg-ir.cc (class_decl::priv::data_members_): New data
member.
(class_decl::priv::priv): When initializing data members, store
the non-static data members on the side, in the new
class_decl::priv::non_static_data_members_ data member.
(class_decl::get_non_static_data_members): Define member function.
(class_decl::add_data_member): Store the non-static data members
on the side in class_decl::priv::non_static_data_members_.
(equals): In the overload for class_decl, do not take in account
static data members when running the comparison.
* tests/data/test-diff-dwarf/test7-report.txt: Adjust.
* tests/data/test-diff-filter/test12-report.txt: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Reports about added/deleted variables and symbols changes were still
not sorted, causing differences in output for abidiff depending on the
platform it's run on.
This patch fixes that.
* src/abg-comparison.cc (sort_string_var_ptr_map)
(sort_string_elf_symbol_map): Define new static functions.
(var_comp, elf_symbol_comp): Define new comparison functors.
(corpus_diff::report): Sort the deleted variables, added
variables, deleted function symbols, added function symbols,
deleted variable symbols, and added variable symbols before
walking them to emit reports.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When a member function is wrongly considered as being added, then
either the new member function doesn't have a symbol name (linkage
name) or it has one, and it was already present in the first version
of the binary.
What was I thinking ... so I hope this shot is better.
* src/abg-comparison.cc
(class_diff::ensure_lookup_tables_populated): Ensure that when a
member function is wrongly considered as being added, then either
the new member function doesn't have a symbol name (linkage name)
or it has one, and it was already present in the first version of
the binary.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
# Veuillez saisir le message de validation pour vos
modifications. Les lignes # commençant par '#' seront ignorées, et
un message vide abandonne la validation. # Sur la branche
fix-master # Votre branche est à jour avec 'origin/master'. # #
Modifications qui seront validées : # modified:
src/abg-comparison.cc # # Modifications qui ne seront pas validées
: # modified: tools/abidw.cc # # Fichiers non suivis: # abidw.abi
# build/ # depcomp # missing # patch-edited.txt # patch.txt #
prtests/ # test-driver # # ------------------------ >8
------------------------ # Ne touchez pas à la ligne ci-dessus #
Tout se qui suit sera éliminé.
diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 14208f5..ef7c6c9 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -7419,8 +7419,8 @@ class_diff::ensure_lookup_tables_populated(void) const
inserted_member_fns().begin();
i != inserted_member_fns().end();
++i)
- if (i->second->get_symbol()
- && f->lookup_function_symbol(i->second->get_symbol()->get_name(),
+ if (!i->second->get_symbol()
+ || f->lookup_function_symbol(i->second->get_symbol()->get_name(),
i->second->get_symbol()->get_version().str()))
to_delete.push_back(i->first);
It turns out we forget to test for the right exit condition while looping
over symbol aliases, leading to an infinite loop.
* src/abg-ir.cc (elf_symbol::get_alias_from_name)
(elf_symbol::get_alias_which_equals): Test for the next alias
pointing to the main symbol, in the loop exit condition.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Up to now the linkage name of a declaration was set to the name of
it's underlying symbol. This patch changes that to instead honour
what the DW_AT_linkage_name DWARF property says, unless the value of
that property is either missing or wrong.
* include/abg-ir.h (elf_symbol::get_alias_from_name): Declare new
member function.
* src/abg-ir.cc (elf_symbol::get_alias_from_name): Define it.
* src/abg-dwarf-reader.cc (build_var_decl, build_function_decl):
Once the linkage name is supposed to contain the value of the
DW_AT_linkage_name attribute, set it the name of the underlying
symbol only if value of DW_At_linkage_name is missing or different
from the names of all the aliases of the underlying symbol.
* tests/data/test-read-dwarf/test2.so.abi: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
At some point, it appeared that some bogus DWARF wouldn't tie a
function decl with its underlying symbol, but subsequent version of
the DWARF emitter (in the second subject of the diff) would correctly
link the function decl with its underlying symbol.
Comparing the two versions of function decl could then wrongly make
the class_diff code think that the function decl was added to the
binary. I later added code that checks that for every supposedly
added function, its symbol must *NOT* have been present in the first
version of the binary. I added some similar code for the removed
symbols case. And that added code seems to work OK.
But then there is an even more ancient hacky attempt at handling the
same case that is no more warranted. This patch removes it.
* src/abg-comparison.cc
(class_diff::ensure_lookup_tables_populated): Remove the code that
tries to lookup allegedly added functions from the set of deleted
ones, by using the pretty printed name of the function. Handling
the case of a function decl not correctly tied to it symbol is
handled my generically a bit later in this function.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-comparison.cc (function_decl_diff::report): Report
a change in the aliases of the symbols of a function; note that
everything else but have stayed equal in the function.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Up to know we were not reporting vtable changes on top-level function
change reports. This patch adds that feature.
* src/abg-comparison.cc (function_decl_diff::report): Report about
virtual-ness and vtable offset changes.
* tests/data/test-diff-dwarf/test28-vtable-changes-report-0.txt:
New test input file.
* tests/data/test-diff-dwarf/test28-vtable-changes-v{0,1}.o: New
test input binaries.
* tests/data/test-diff-dwarf/test28-vtable-changes-v{0,1}.cc:
Source code of the input binaries above.
* tests/data/Makefile.am: Add the new test input above to source
distribution.
* tests/test-diff-dwarf.cc (in_out_specs): Add the new test input
above to the list of input this test harness has to run over.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It turned out it's important to be able to suppress changes about
types that are reachable from a function parameter only through e.g, a
pointer or a reference, so that only changes types that are reachable
directly from a function parameter are emitted.
This patch adds that feature.
While doing this, I noticed this: Suppose a diff node D2 is marked as
being redundant with a diff node D1 seen previously. So only D1 is
reported; D2 is not, because it's been filtered out, because it's
redundant with D1. But then suppose D1 is filtered out, due to a
suppression specification. At that point, D2 should not be marked
redundant anymore, and should be reported.
Of course, the code before this patch was wrongly filtering D2 *and*
D1 out. So this patch fixes that.
* include/abg-comparison.h (enum type_suppression::reach_kind):
Define new enum.
(type_suppression::{get_consider_reach_kind,
set_consider_reach_kind, get_reach_kind,
mark_last_diff_visited_per_class_of_equivalence,
clear_last_diffs_visited_per_class_of_equivalence,
get_last_visited_diff_of_class_of_equivalence}): Declare new
member functions.
* src/abg-comparison.cc (diff_has_ancestor_filtered_out)
(read_suppression_reach_kind): Define static function.
(type_suppression::priv::{consider_reach_kind_, reach_kind_}):
Define new data members.
(type_suppression::priv::priv): Take a new reach_kind parameter.
(type_suppression::type_suppression): Adjust to new prototype of
priv constructor.
(type_suppression::{get_consider_reach_kind,
set_consider_reach_kind, get_reach_kind, set_reach_kind}): Define
new member functions.
(type_suppression::suppresses_diff): Interpret the result of
type_suppression::get_reach_kind() to determine if the suppression
specification suppresses a given diff node.
(read_type_suppression): Support reading the content of the
"accessed_through" property.
(diff_context::priv::last_visited_diff_node_): New data member.
(diff_context::{mark_last_diff_visited_per_class_of_equivalence,
clear_last_diffs_visited_per_class_of_equivalence,
get_last_visited_diff_of_class_of_equivalence}): Define new data
members.
(redundancy_marking_visitor::visit_begin): So if the current diff
node has already been visited, but if the previously visited node
has been filtered out, then do not mark this node as being
redundant. And mark the current diff node as being the last
visited one in its class of equivalence.
(categorize_redundancy): Clear the map of diff nodes visited per
class of equivalence.
* doc/manuals/libabigail-concepts.rst: Document the new
'accessed_through' property.
* tests/data/test-diff-suppr/test13-suppr-through-pointer-0.suppr:
New test input data.
* tests/data/test-diff-suppr/test13-suppr-through-pointer-report-{0,1}.txt:
Likewise.
* tests/data/test-diff-suppr/libtest13-suppr-through-pointer-v{0,1}.so:
New test input binaries.
* tests/data/test-diff-suppr/test13-suppr-through-pointer-v{0,1}.cc:
Source code of the test input binaries above.
* tests/data/test-diff-suppr/test14-suppr-non-redundant-0.suppr:
New test input data.
* tests/data/test-diff-suppr/test14-suppr-non-redundant-report-0.txt:
Likewise.
* tests/data/test-diff-suppr/test14-suppr-non-redundant-v{0,1}.o:
New test input binaries.
* tests/data/test-diff-suppr/test14-suppr-non-redundant-v{0,1}.cc:
Source code of the binaries above.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
If a type T is used directly (i.e, not through a pointer or reference)
as a function parameter or as a base class, a change in T should never
be marked as redundant in that context. Otherwise, the change in that
context might be filtered out, possibly hiding real ABI incompatible
changes.
This patch implements this policy.
Also, it turned out in some circumstances, we where marking the first
visited diff node of a given class of equivalence of nodes as being
redundant, while we should only mark the *subsequently* visited nodes
of that class of equivalence as visited. The patch also fixes that.
* include/abg-comparison.h (pointer_map): Make this be a map of
{size_t, size_t} pairs, rather than {size_t, bool}, so that each
pointer in the map can be associated to another one.
(diff_context::diff_has_been_visited): Return the pointer to the
first diff node of the equivalence class that has been visited.
* src/abg-comparison.cc (is_pointer_diff, is_reference_diff)
(is_reference_or_pointer_diff, is_fn_parm_diff, is_base_diff)
(is_child_node_of_function_parm_diff, is_child_node_of_base_diff):
Define new static functions.
(diff_context::diff_has_been_visited): Return the pointer to the
first diff node of the equivalence class that has been visited.
(diff_context::mark_diff_as_visited): Save the pointer to the
first diff node of a given class of equivalence that has been
visited.
(redundancy_marking_visitor::visit_begin): If a diff node is a
child node of a function parameter diff or base diff node and if
it's not a pointer or reference diff node, then do not mark it as
redundant. Also, make sure to not mark the first diff node of a
given class of equivalence that has been visited, as redundant;
only the other subsequent nodes should be marked redundant; we
were hitting this case because of an optimization that makes
equivalent class diff nodes to share their private (pimpl) data.
* tests/data/test-diff-filter/test29-finer-redundancy-marking-v{0,1}.o:
New test input binaries.
* tests/data/test-diff-filter/test29-finer-redundancy-marking-v{0,1}.cc:
Source code of the new test input binaries above.
* tests/data/test-diff-filter/test29-finer-redundancy-marking-report-0.txt:
New test input.
* tests/data/Makefile.am: Add the new test material above to the
source distribution.
* tests/test-diff-filter.cc (in_out_specs): Make this test harness
run over the additional test input above.
* tests/data/test-diff-suppr/test5-fn-suppr-report-0.txt: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It appears we were flagging too many base class changes as local.
That was preventing some change category propagation through base
class diff nodes. This patch fixes that.
* abg-ir.cc (equals): In the overload of class_decl::base_spec, if
the underlying class carries changes, then do not flag these
changes as local for the class_decl::base_spec.
* tests/data/test-diff-dwarf/test27-local-base-diff-v{0,1}.o: New
test input binaries.
* tests/data/test-diff-dwarf/test27-local-base-diff-v{0,1}.cc: Source
code for the test input binaries above.
* tests/data/test-diff-dwarf/test27-local-base-diff-report.txt:
New test input.
* tests/data/Makefile.am: Add the test inputs above to source
distribution.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While working on something else, I noticed that the code for handling
copying symbols (and their aliases) was broken, and so comparing two
symbols which main name were different by which had aliases that were
equal was wrongly resulting in the two symbol being different. I think
we shouldn't actually copy symbols and their aliases. Once a symbol
is allocated, interested code should just manipulate that symbol by
address rather than by value an thus do away with the copying.
The patch does that, essentially. In the implementation of a symbol,
the aliases as well as the main symbol are now weak pointers, rather
than naked pointers. Numerous API entry points that were taking
containers of elf_symbol (and were copying elf_symbols over) are not
taking containers of smart pointers to elf_symbol. Copying of
instances of elf_symbol is now thus disabled.
As a result many tests that were exercising elf_symbols (with alias)
comparison have been updated.
As a result, many empty sub-result of PR libabigail/PR17948 are now
fixed.
* include/abg-ir.h (elf_symbol_wptr): New typedef.
(elf_symbol): Make the constructors and assignment operator
private. The type can neither be copied nor created with the new
operator.
(elf_symbol::create): New static member function.
(elf_symbol::{get_main_symbol, get_next_alias, add_alias}):
Adjust.
( compute_aliases_for_elf_symbol): Likewise.
(elf_symbol::operator=): Make this private.
(elf_symbol::get_alias_which_equals): Declare new member function.
* src/abg-comp-filter.cc (function_name_changed_but_not_symbol):
Adjust.
* src/abg-comparison.cc
(class_diff::ensure_lookup_tables_populated): Adjust.
* src/abg-corpus.cc
(corpus::priv::build_unreferenced_symbols_tables): Likewise.
* include/abg-dwarf-reader.h (lookup_symbol_from_elf)
(lookup_public_function_symbol_from_elf): Adjust.
* src/abg-dwarf-reader.cc (lookup_symbol_from_sysv_hash_tab)
(lookup_symbol_from_gnu_hash_tab, lookup_symbol_from_elf_hash_tab)
(lookup_symbol_from_symtab, lookup_symbol_from_elf)
(lookup_public_function_symbol_from_elf)
(lookup_public_variable_symbol_from_elf): Adjust.
(read_context::lookup_elf_symbol_from_index): Likewise.
(read_context::lookup_elf_fn_symbol_from_address): Likewise.
(read_context::lookup_elf_var_symbol_from_address): Likewise.
(read_context::lookup_public_function_symbol_from_elf): Likewise.
(read_context::lookup_public_variable_symbol_from_elf): Likewise.
(read_context::load_symbol_maps): Likewise.
(build_var_decl, build_function_decl): Likewise.
* src/abg-ir.cc (elf_symbol::priv::{main_symbol_, next_alias_}):
Change the type of these from elf_symbol* to elf_symbol_wptr.
(elf_symbol::priv::priv): Adjust.
(elf_symbol::{create, get_alias_which_equals}): Define new functions.
(textually_equals): Likewise.
(elf_symbol::{get_main_symbol, is_main_symbol, get_next_alias,
add_alias}): Adjust to return or take elf_symbol_sptr type, rather
than a elf_symbol* one.
(elf_symbol::{get_aliases_id_string, does_alias}): Adjust.
(compute_alias_for_elf_symbol): Likewise.
(elf_symbol::operator==): Two symbols A and B are now equal if A
has at least one alias that is textually equal to B.
(equals): In the overload for function_decls, in the part where we
compare the decl_base part of the functions without considering
their decl names, we now also omit considering their linkage
names, because we compared they symbols before.
* tools/abisym.cc (main): Adjust.
* tests/data/test-diff-dwarf/test12-report.txt: Adjust.
* tests/data/test-diff-dwarf/test12-report.txt: Adjust.
* tests/data/test-diff-dwarf/test18-alias-sym-report-0.txt: Adjust.
* tests/data/test-diff-dwarf/test8-report.txt: Adjust.
* tests/data/test-diff-filter/test10-report.txt: Adjust.
* tests/data/test-diff-filter/test13-report.txt: Adjust.
* tests/data/test-diff-filter/test2-report.txt: Adjust.
* tests/data/test-diff-filter/test20-inline-report-0.txt: Adjust.
* tests/data/test-diff-filter/test20-inline-report-1.txt: Adjust.
* tests/data/test-diff-filter/test9-report.txt: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch is for supporting this kind of things:
[suppress_type]
name = S
has_data_member_inserted_between = {8, end}
or:
[suppress_type]
name = S
has_data_members_inserted_between = {{8, 31}, {64, end}}
or:
[suppress_type]
name = S
has_data_members_inserted_at = offset_after(member0)
How cool is that, heh?
Anyway, to do this, the patch adds support for tuple values (i.e,
lists of values) in INI files.
Then on top of that the patch adds support for the specific
has_data_member_inserted_between, has_data_members_inserted_between
and has_data_members_inserted_at properties.
* include/abg-comparison.h (type_suppression::insertion_range):
Declare new type.
(type_suppression::insertion_ranges): Declare new typedef.
(type_suppression::{s,g}et_data_member_insertion_ranges): Declare
new member functions.
(is_integer_boundary, is_fn_call_expr_boundary): Declare new
functions.
(type_suppression::insertion_range::{boundary, integer_boundary,
fn_call_expr_boundary}): Define new types.
* src/abg-comparison.cc:
(struct type_suppression::insertion_range::priv): New type.
(type_suppression::insertion_range::{insertion_range, begin,
end}): Define new member functions.
(type_suppression::priv::insertion_ranges_): Add data member.
(type_suppression::{s,g}et_data_member_insertion_ranges): Define
new member functions.
(type_suppression::insertion_range::boundary::priv): Define new
type.
(type_suppression::insertion_range::boundary::{boundary,
~boundary}): Define new member functions.
(type_suppression::insertion_range::integer_boundary::priv):
Define new type.
(type_suppression::insertion_range::integer_boundary::{integer_boundary,
as_integer, operator int, ~integer_boundary}): Define member
functions.
(type_suppression::insertion_range::fn_call_expr_boundary::priv):
Define new type.
(type_suppression::insertion_range::fn_call_expr_boundary::{fn_call_expr_boundary,
as_function_call_expr, operator ini::function_call_expr_sptr}):
Define new member functions.
(type_suppression::insertion_range::{create_integer_boundary,
type_suppression::insertion_range::create_fn_call_expr_boundary,
type_suppression::insertion_range::eval_boundary}): Define new
member functions.
(is_integer_boundary, is_fn_call_expr_boundary): Define new
functions.
(read_type_suppression, read_function_suppression)
(read_variable_suppression): Support the new kinds of
property-related types. Aslo, in read_type_suppression, support
the new properties has_data_member_inserted_at,
has_data_member_inserted_between and
has_data_members_inserted_between.
(type_suppression::suppresses_diff): If we are looking at a type
diff node that has inserted data members, evaluate the insertion
ranges of the current type_suppression and see if they match the
inserted data members.
* include/abg-ini.h (property, simple_property, property_value)
(string_property_value, tuple_property_value, function_call_expr):
Declare new types.
(property_sptr, property_value_sptr, string_property_value_sptr)
(tuple_property_value_sptr): Declare new typedefs.
(is_string_property_value, is_tuple_property_value)
(is_simple_property, is_tuple_property, read_function_call_expr):
Declare new functions.
* src/abg-ini.cc (char_is_white_space, char_is_comment_start)
(char_is_delimiter, char_is_property_value_char)
(char_is_section_name_char, char_is_property_name_char)
(char_is_comment_start, char_is_white_space)
(remove_trailing_white_spaces, is_string_property_value)
(is_tuple_property_value, is_simple_property, is_tuple_property)
(write_property_value, char_is_function_name_char)
(char_is_function_argument_char): Define new functions.
(property::priv, tuple_property_value::priv)
(simple_property::priv, tuple_property::priv): Define new types.
(property::{property, get_name, set_name, ~property}): Define new
member functions.
(struct property_value::priv): Define new type.
(property_value::{property_value, get_kind, operator const
string&(), ~property_value}): Define new member functions.
(struct string_property_value::priv): Define new type.
(string_property_value::{string_property_value, set_content,
as_string, operator string()}, ~string_property_value): Define new
member functions.
(tuple_property_value::{tuple_property_value, get_value_items,
~tuple_property_value, as_string}): Likewise.
(simple_property::{simple_property, get_value, set_value,
~simple_property}): Likewise.
(tuple_property::{tuple_property, set_value, get_value}):
Likewise.
(config::section::find_property): Adjust return type.
(read_context::{char_is_delimiter, char_is_property_value_char,
char_is_section_name_char, char_is_property_name_char,
char_is_comment_start, char_is_white_space}): Remove these from
here as they got moved them to be non-member functions above.
(read_context::read_property_value): Return a property_value_sptr
and do not take any parameter anymore.
(read_context::{read_string_property_value,
read_tuple_property_value, read_function_name,
read_function_argument, read_function_call_expr}): Define new
member functions.
(read_context::read_property): Adjust return type. Also, change to read
the different new kinds of properties values.
(function_call_expr::priv): Define new type.
(function_call_expr::{function_call_expr, get_name,
get_arguments}): New member functions.
(read_context::read_section): Adjust.
(write_property, write_section): Adjust.
* tests/data/test-diff-suppr/libtest{11,12}-add-data-member-v{0,1}.so:
New test input binaries.
* tests/data/test-diff-suppr/test{11,12}-add-data-member-{0,1}.suppr:
New input suppression files.
* tests/data/test-diff-suppr/test11-add-data-member-{2,3,4}.suppr:
Add new test input files.
* tests/data/test-diff-suppr/test{11,12}-add-data-member-report-{0,1}.txt:
New reference output files.
* tests/data/test-diff-suppr/test12-add-data-member-report-2.txt:
Likewise.
* tests/data/test-diff-suppr/test{11,12}-add-data-member-v{0,1}.cc:
Source code for the new binaries above.
* tests/test-diff-suppr.cc (in_out_specs): Add new test inputs.
* tests/data/Makefile.am: Add the new test related files above to
source distribution.
* doc/manuals/libabigail-concepts.rst: Document the new properties
has_data_member_inserted_at, has_data_member_inserted_between and
has_data_members_inserted_between.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Types read from DWARF don't have any alignment information so we
shouldn't try to guess it, especially for structures. So this patch
sets the alignment to zero in that case. This helps remove some
spurious alignment changes detected by abidiff just because in some
cases we fail to guess that.
In the process, I noticed that when calculating the hash value of a
given data member, we were not including the hash value of its
context. This led to mistakenly considering some data member changes
as redundant. So the patch fixes that too.
* src/abg-dwarf-reader.cc (build_type_decl)
(build_class_type_and_add_to_ir, build_pointer_type_def)
(build_reference_type, build_function_decl): Set the alignment for
native types, class, reference and function type to zero,
effectively meaning that they don't have alignment information.
* src/abg-hash.cc (var_decl:#️⃣:operator): Take the hash value
of the data member context in account when computing the hash
value of a given data member.
* tests/data/test-diff-dwarf/test-23-diff-arch-report-0.txt:
Adjust.
* tests/data/test-diff-dwarf/test10-report.txt: Likewise.
* tests/data/test-diff-dwarf/test13-report.txt: Likewise.
* tests/data/test-diff-dwarf/test22-changed-parm-c-report-0.txt: Likewise.
* tests/data/test-diff-dwarf/test26-added-parms-before-variadic-report.txt: Likewise.
* tests/data/test-diff-dwarf/test8-report.txt: Likewise.
* tests/data/test-diff-dwarf/test9-report.txt: Likewise.
* tests/data/test-diff-filter/test13-report.txt: Likewise.
* tests/data/test-diff-filter/test6-report.txt: Likewise.
* tests/data/test-diff-suppr/test9-changed-parm-c-report-0.txt: Likewise.
* tests/data/test-read-dwarf/test0.abi: Likewise.
* tests/data/test-read-dwarf/test1.abi: Likewise.
* tests/data/test-read-dwarf/test2.so.abi: Likewise.
* tests/data/test-read-dwarf/test3.so.abi: Likewise.
* tests/data/test-read-dwarf/test4.so.abi: Likewise.
* tests/data/test-read-dwarf/test5.o.abi: Likewise.
* tests/data/test-read-dwarf/test6.so.abi: Likewise.
* tests/data/test-read-dwarf/test7.so.abi: Likewise.
* tests/data/test-read-dwarf/test8-qualified-this-pointer.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Now the name ID of a function parameter is going to be
"parameter-<index>". I think it's clearer an simpler.
* src/abg-ir.cc (function_decl::parameter::get_name_id): Make this
be "parameter-<index>".
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Just looking at if the name of the changed type hasn't changed is not
enough for detecting a sub-type change; that will be fooled by
compatible changes (changes involving typedefs). So this patch looks
through compatible changes for that matter.
* include/abg-fwd.h (type_has_sub_type_changes): Declare new
function.
* src/abg-ir.cc (type_has_sub_type_changes): Define it.
* src/abg-comparison.cc (fn_parm_diff::report): Use the new
function type_has_sub_type_changes() instead of just looking at
name changes.
* tests/data/test-diff-dwarf/test4-report.txt: Adjust this
reference test output.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* include/abg-fwd.h (get_pretty_representation): Declare new
overload for type_or_decl_base*.
* src/abg-ir.cc (get_pretty_representation): Define it and express
the previous overload for type_or_decl_base_sptr in terms of this
new one.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This is just a small optimization in the passing
* src/abg-ir.h (type_decl::get_void_type_decl): Return a reference
to the smart pointer initially returned.
* src/abg-ir.cc (type_decl::get_void_type_decl): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In the IR built from DWARF, a variadic variadic parameter has an empty
type. Later during type comparison, comparing an empty (NULL) type
with other types proves to be troublesome.
This patch handles the issue by creating a new kind of
abigail::type_decl type specifically for variadic parameters. This is
like what is done for void types.
After that it appears that the categorizing sub-system flags a change
of variadic type to non-variadic type as redundant if that change
appears several times on different functions. We don't want that
because it can hide important changes we want to see. The patch fixes
that too.
* include/abg-fwd.h (is_array_type): New overload for a naked
pointer.
* include/abg-ir.h (type_decl::get_variadic_parameter_type_decl): Declare new
static function.
* src/abg-ir.cc (is_array_type): Define new function overload for
naked pointers
(type_decl::get_variadic_parameter_type_decl): Define new static
function.
* src/abg-dwarf-reader.cc (build_function_decl): The type of
variadic parameter is now a special type_decl.
* include/abg-comparison.h (is_diff_of_variadic_parameter_type)
(is_diff_of_variadic_parameter): New function declarations.
* src/abg-comparison.cc (is_diff_of_variadic_parameter_type)
(is_diff_of_variadic_parameter): Define new functions.
(compute_diff): Refuse to return a NULL
diff for types. Assert that the parameters are non-NULL.
(report_size_and_alignment_changes): We are comparing arrays only
if the two parameters are arrays.
(fn_parm_diff::fn_parm_diff): Refuse that type diff for this diff
node is non empty.
(fn_parm_diff::report): Strengthen an assert. Cleanup a comment.
(redundancy_marking_visitor::visit_begin): Do not mark function
type and variadic parms diff nodes as redundant for local changes.
* tests/data/test-diff-dwarf/libtest26-added-parms-before-variadic-v{0,1}.so:
New test input binaries.
* tests/data/test-diff-dwarf/test26-added-parms-before-variadic-report.txt:
New test output reference.
* tests/data/test-diff-dwarf/test26-added-parms-before-variadic-v{0,1}.c:
Source code of the new test input binaries above.
* tests/data/Makefile.am: Add the new test stuff to source
distribution.
* tests/test-diff-dwarf.cc (in_out_specs): Add the new test inputs
above to the set of input to run this test harness over.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The runtestwritereadarchive test is failing on Rawhide (Fedora 23) for
me now. It appears it was because the content to write to the zip
archive is in a buffer which is stored in a vector of buffers.
When the vector grows, the buffers are potentially copied over, and
the old buffers are destroyed, making the address of the old buffers
being stalled. But then those old address are used by the code that
write stuff in the zip archive. Ooops.
This patch essentially replaces the vector of buffer with a list, so
that growing the list doesn't invalidate the buffers. The patch also
does away with using deprecated APIs of libzip.
* configure.ac: Require libzip 0.10.1 at least.
* src/abg-writer.cc (archive_write_ctxt::serialized_tus): Make
this be a list<string>, rather than a vector<string>.
(create_archive_write_context): Truncate the archive if it exists
already.
(write_translation_unit_to_archive): Do not use the deprecated
zip_add() function anymore. Rather, use zip_file_add().
* tests/test-write-read-archive.cc (main): Double check if the
translation unit we read is empty or not.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In the changed functions/variables section of the abidiff report, when
function parameters were added or removed, they were not properly
sorted. This patch fixes that.
* src/abg-comparison.cc (sort_string_parm_map): Define new static
function.
(struct parm_comp): Define new type.
(function_type_diff::priv::{sorted_deleted_parms_,
sorted_added_parms_}): New data members that hold sorted
deleted/added parameters.
(function_type_diff::ensure_lookup_tables_populated): Initialize
the two new data members above.
(function_type_diff::report): For the report of parameters that
got added/removed, use the sorted set of added/removed parameters
above.
* tests/data/test-diff-dwarf/test24-added-fn-parms-report-0.txt:
New test input.
* tests/data/test-diff-dwarf/libtest24-added-fn-parms-v{0,1}.so:
Likewise.
* tests/data/test-diff-dwarf/test25-removed-fn-parms-report-0.txt:
Likewise.
* tests/data/test-diff-dwarf/libtest25-removed-fn-parms-v{0,1}.so:
Likewise.
* tests/data/test-diff-dwarf/test24-added-fn-parms-v{0,1}.c:
Likewise.
* tests/data/test-diff-dwarf/test25-removed-fn-parms-v{0,1}.c:
Likewise.
* tests/data/Makefile.am: Add the new test material above to the
source distribution.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
As per https://sourceware.org/bugzilla/show_bug.cgi?id=18146, abidiff
the exit code of abidiff and abicompat is now a bit field that can be
inspected to know if the ABI change reported is incompatible for sure,
or if it needs user review of the output to decide.
This patch also updates the documentation.
* doc/manuals/abicompat.rst: Update documentation for abicompat
exit codes.
* doc/manuals/abidiff.rst: Likewise for abidiff exit codes.
* include/abg-tools-utils.h (enum abidiff_status): Declare new
enum.
(operator{|,&,|=}): Declare new operators for the new enum
abidiff_status.
(abidiff_status_has_error, abidiff_status_has_abi_change)
(abidiff_status_has_incompatible_abi_change): Declare new
functions.
* src/abg-tools-utils.cc (operator{|,&,|=}): Define these new
operators.
(abidiff_status_has_error, abidiff_status_has_abi_change)
(abidiff_status_has_incompatible_abi_change): Define new
functions.
* tests/test-diff-filter.cc (main): Adjust for the new exit code
of abidiff.
* tests/test-diff-suppr.cc (main): Likewise.
* tests/test-abicompat.cc (main): Likewise.
* tools/abicompat.cc (enum abicompat_status): Remove.
(operator{|,&,|=}): Remove these operators for enum
abicompat_status.
(perform_compat_check_in_normal_mode)
(perform_compat_check_in_weak_mode): Return abidiff_status instead
of abicompat_status. Adjust therefore.
(main): Adjust to return abidiff_status now, instead of a just
zero for all non-error cases.
* tools/abidiff.cc (main): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch implements the weak mode of abicompat. In this mode, just
the application and the new version of the library are provided. The
types of functions and variables of the library that are consumed by
the application are compared to the types of the functions and
variables expected by the application. The goal is to check if the
types of the declarations consumed by the application and provided by
the library are compatible with what the application expects.
The abicompat first gets the set of symbols undefined in the
application and exported by the library. It then builds the set of
declarations exported by the library that have those symbols. We call
these the set of declarations of the library that are consumed by the
application.
Note that the debug information for the application does not contain
the declarations of the functions/variables whose symbols are
undefined. So we can not just read them to compare them to
declarations exported by the library.
But the *types* of the variables and the *sub-types* of the functions
whose symbols are undefined in the application are present in the
debug information of the application.
So in the weak mode, abicompat compare the *types* of the declarations
consumed by the application as expected by the application (described
by the debug information of the application) with the types of the
declarations exported by the library.
To do this a number of changes were necessary.
The patch builds a representation of all the types found in the
application's debug info. Before that, only the types that are
reachable from exported declarations were represented.
The abidw tool got a new --load-all-types to test this new ability of
loading all types.
The patch also adds support for looking a type, not by name, but by
its internal representation.
In the comparison engine, function_type_diff is introduced to
represent changes between two function types. For this, a new class
type_or_decl_base has been introduced in the IR. It's now the base
class for both decl_base and type_base. And abigail::comparison::diff
now takes two pointers of type_or_decl, not decl_base anymore. So
function_type_diff can take two function_type now; not that a
function_type has no declaration so it doesn't inherit decl_base. A
bunch of changes got made just to adjust to this modification.
A number of fixes were made too, to make this work, like adding
missing comparison operators, removing asserts that too strong, etc..
The patch also adjust the test suite as well as the documentation.
* include/abg-fwd.h (class type_or_decl_base): Forward declare
this.
(is_decl, is_type, is_function_type, get_name, get_type_name)
(get_function_type_name, get_pretty_representation)
(lookup_function_type_in_corpus, lookup_type_in_translation_unit)
(lookup_function_type_in_translation_unit)
(synthesize_function_type_from_translation_unit)
(hash_type_or_decl): New function declarations.
* src/abg-corpus.cc (lookup_type_in_corpus)
(lookup_function_type_in_corpus): Define new functions.
* include/abg-ir.h
(translation_unit::lookup_function_type_in_translation_unit):
Declare new friend function.
(class type_or_decl_base): Declare this.
(operator==(const type_or_decl_base&, const type_or_decl_base&)):
Declare new operator.
(operator==(const type_or_decl_base_sptr&, const
type_or_decl_base_sptr&)): Likewise.
(class {decl_base, type_base}): Make these class inherit
type_or_decl_base.
(decl_base::get_member_scopes): New const overload.
(bool operator==(const function_decl::parameter_sptr&,
const function_decl::parameter_sptr&)): New operator.
(function_type::get_parameters): Remove the non-const overload.
(function_type::get_pretty_representation): Declare new member
function.
(method_type::get_pretty_representation): Likewise.
* src/abg-ir.cc (bool operator==(const type_or_decl_base&, const
type_or_decl_base&)): Define new equality operator.
(bool operator==(const type_or_decl_base_sptr&, const
type_or_decl_base_sptr&)): Likewise.
(strip_typedef): Do not expect canonicalized types anymore. Now
the system accepts (and expects) canonicalized types in certain
cases. For instance, non-complete types and aggregated types that
contain non-complete sub-types.
(get_name, get_function_type_name, get_type_name)
(get_pretty_representation, is_decl, is_type, is_function_type)
(lookup_function_type_in_translation_unit)
(synthesize_function_type_from_translation_unit)
(lookup_type_in_scope, lookup_type_in_translation_unit): Define
new functions or new overloads.
(bool operator==(const function_decl::parameter_sptr&,
const function_decl::parameter_sptr& r)): Define
new operator.
(function_type::get_parameters): Remove non-const overload.
(function_type::get_pretty_representation): Define new function.
(function_type::traverse): Adjust.
(method_type::get_pretty_representation): Likewise.
(function_decl::get_pretty_representation): Avoid emitting the
type of cdtors.
(hash_type_or_decl): Define new function.
* include/abg-dwarf-reader.h (create_read_context)
(read_corpus_from_elf): Take a new 'read_all_types' flag.
* src/abg-dwarf-reader.cc (read_context::load_all_types_): New
flag.
(read_context::read_context): Initialize it.
(read_context::canonical_types_scheduled): If some types still
have non-canonicalized sub-types, then do not canonicalize them.
(read_context::load_all_types): New member functions.
(build_function_decl): Do not represent void return type like
empty type anymore, rather, represent it like a void type node.
(build_ir_node_from_die): When asked, load all types
including those that are not reachable from an exported
declaration.
(create_read_context, read_corpus_from_elf): Take a new
'load_all_types' flag and honour it.
* src/abg-reader.cc (read_context::type_is_from_translation_unit):
Support looking up function types in the current translation unit,
now that we now how to lookup function types.
* include/abg-comparison.h (diff_context::{has_diff_for, add_diff,
set_canonical_diff_for, set_or_get_canonical_diff_for,
get_canonical_diff_for}): Make these take instances of
type_or_decl_base_sptr, instead of decl_base_sptr.
(diff::diff): Likewise.
(diff::{first_subject, second_subject}): Make these return
type_or_decl_base_sptr instead of decl_base_sptr.
(type_diff_base::type_diff_base): Make these take instances of
type_or_decl_base_sptr instead of decl_base_sptr.
(distinct_diff::distinct_diff): Likewise.
(distinct_diff::{first, second}): Make these return
type_or_decl_base_sptr instead of decl_base_sptr.
(distinct_diff::entities_are_of_distinct_kinds): Make these take
instances of type_or_decl_base_sptr instead of decl_base_sptr.
(class function_type_diff): Create this new type. It's a
factorization of the function_decl_diff type.
* src/abg-comparison.cc ():
* src/abg-comp-filter.cc ({harmless, harmful}_filter::visit):
Adjust as diff::{first,second}_subject() now returns a
type_or_decl_base_sptr, no more a decl_base_sptr.
(decls_type, decls_diff_map_type): Remove these typedefs and replace it with ...
(types_or_decls_type, types_or_decls_diff_map_type): ... these.
(struct {decls_hash, decls_equals): Remove these type sand replace them with ...
(struct {types_or_decls_hash, types_or_decls_equals}): ... these.
({type_suppression, variable_suppression}::suppresses_diff):
Adjust.
(diff_context::priv::decls_diff_map): Replace this with ...
(diff_context::priv::types_or_decls_diff_map): ... this.
(diff_context::{has_diff_for, add_diff, get_canonical_diff_for,
set_canonical_diff_for, set_or_get_canonical_diff_for}): Take
type_or_decl_base_sptr instead of decl_base_sptr.
(diff::priv::{first, second}_subject): Make the type of these be
type_or_decl_base_sptr, no more decl_base_sptr.
(diff::priv::priv): Adjust for the subjects of the diff being of
type type_or_decl_sptr now, no more decl_base_sptr.
(diff_less_than_functor::operator()(const diff_sptr, const
diff_sptr) const): Adjust.
(diff::diff): djust for the subjects of the diff being of type
type_or_decl_sptr now, no more decl_base_sptr.
(diff::{first,second}_subject): Make the type of these be
type_or_decl_base_sptr, no more decl_base_sptr.
(report_size_and_alignment_changes): Likewise.
(type_diff_base::type_diff_base): Make the type of this be
type_or_decl_base_sptr instead of type_base_sptr.
(distinct_diff::distinct_diff): Make this take instances of
type_or_decl_base_sptr instead of decl_base_sptr.
(distinct_diff::{first, second, entities_are_of_distinct_kinds}):
Likewise.
(distinct_diff::has_changes): Simplify logic.
(distinct_diff::report): Adjust.
(compute_diff_for_types): Add an additional case to support the
new function_type.
(report_size_and_alignment_changes): Make this take instances of
type_or_decl_base_sptr instead of decl_base_sptr.
(class_diff::priv::member_type_has_changed): Return an instance of
type_or_decl_base_sptr rather than a decl_base_sptr.
(class_diff::report): Adjust.
(diff_comp::operator()(const diff&, diff&) const): Adjust.
(enum function_decl_diff::priv::Flags): Remove.
(function_decl_diff::priv::{first_fn_flags_, second_fn_flags_,
fn_flags_changes_}): Remove.
(function_decl_diff::priv::{fn_is_declared_inline_to_flag,
fn_binding_to_flag}): Remove.
(function_decl_diff::{deleted_parameter_at,
inserted_parameter_at}): Remove.
(function_decl_diff::ensure_lookup_tables_populated): Empty this.
(function_decl_diff::chain_into_hierarchy): Adjust.
(function_decl_diff::function_decl_diff): This now only takes the
subjects. It's body is now empty.
(function_decl_diff::{return_type_diff, subtype_changed_parms,
removed_parms, added_parms, type_diff}): Remove these member
functions.
(function_decl_diff::type_diff): Define new member function.
(function_decl_diff::report): Simplify logic by using the
reporting of the child type diff node.
(compute_diff): Likewise, in the overload for function_decl_sptr
simplify logic by using the child type diff object.
(function_type_diff::priv): Define new type.
(function_type_diff::{function_type_diff,
ensure_lookup_tables_populated, deleted_parameter_at,
inserted_parameter_at, finish_diff_type, first_function_type,
second_function_type, return_type_diff, subtype_changed_parms,
removed_parms, added_parms, get_pretty_representation,
has_changes, has_local_changes, report, chain_into_hierarchy}):
Define new functions.
(compute_diff): Define new overload for function_type_sptr.
* tools/abicompat.cc (options::weak_mode): New data member.
(options::options): Initialize it.
(enum abicompat_status): New enum
(abicompat_status operator|(abicompat_status, abicompat_status))
(abicompat_status& operator|=(abicompat_status &, abicompat_status))
(abicompat_status operator&(abicompat_status, abicompat_status)):
New operators to manipulate the abicompat_status enum.
(display_usage): Add help string for the new --weak-mode option.
(parse_command_line): Add the new --weak-mode command line
argument. If the tool is called with just the application and one
library then assume that we are in the weak mode.
(perform_compat_check_in_normal_mode): Define new function, factorized
from what was in the main function.
(perform_compat_check_in_weak_mode): Define new function.
(struct {fn,var}_change): Define new types.
(main): Use perform_compat_check_in_weak_mode() and
perform_compat_check_in_normal_mode().
* tools/abidiff.cc (main): Adjust.
* tools/abidw.cc: (options::load_all_types): Add new data member.
(options::options): Initialize it.
(display_usage): New help string for --load-all-types.
(parse_command_line): Support the new --load-all-types option.
(main): Adjust and honour the --load-all-types option.
* tools/abilint.cc (main): Adjust.
* doc/manuals/abicompat.rst: Update documentation for the new weak
mode. Also provide stuff that was missing from the examples
provided.
* doc/manuals/abidw.rst: Update documentation for the new
--load-all-types option.
* tests/print-diff-tree.cc (main): Adjust.
* tests/test-diff-dwarf.cc (main): Likewise.
* tests/test-read-dwarf.cc (main): Likewise.
* tests/data/test-abicompat/test0-fn-changed-app: Recompile this.
* tests/data/test-abicompat/libtest5-fn-changed-libapp-v{0,1}.so:
New new test input binaries
* tests/data/test-abicompat/test5-fn-changed-app: Likewise.
* tests/data/test-abicompat/test6-var-changed-app: Likewise.
* tests/data/test-abicompat/libtest6-var-changed-libapp-v{0,1}.so:
Likewise.
* tests/data/test-abicompat/test5-fn-changed-report-0.txt:
Reference output for one test above.
* tests/data/test-abicompat/test6-var-changed-report-0.txt:
Likewise.
* tests/data/test-abicompat/test5-fn-changed-app.cc: Source file
for a binary above.
* tests/data/test-abicompat/test5-fn-changed-libapp-v{0,1}.{h,cc}:
Likewise.
* tests/data/test-abicompat/test6-var-changed-libapp-v{0,1}.{cc,h}:
Likewise.
* tests/data/test-abicompat/test6-var-changed-app.cc: Likewise.
* tests/data/Makefile.am: Add the test related files above to the
source distribution.
* tests/test-abicompat.cc (in_out_spec): Add the new test input
above to the list of inputs to feed to this test harness.
(main): Support taking just the app and one library.
* tests/data/test-read-dwarf/test{0, 1, 2.so, 3.so, 5.o,
8-qualified-this-pointer.so,}.abi: Adjust for void type being
really emitted now, as opposed to just being an empty type.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When the DIE of a class has its 'byte_size' property set, it means the
class is complete, even if the 'declaration' property is set to true
on it. The DWARF reader was not honouring this in all cases and
sometimes some classes were unduly considered as non-complete. The
binary the problem was exhibited on was generated by llvm.
This patch fixes that.
I couldn't produce a binary with similar DWARF output, so this patch
doesn't have a regression test associated to it :( I guess at some
point we should have another git repository with binaries and an
associated test harness, in which we'd stash binaries for which we
haven't wrote the sources ourselves.
* src/abg-dwarf-reader.cc (build_class_type_and_add_to_ir): When
the size of the class is provided then the class is complete, no
matter if this function called to update the class or to build it
for the first time.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Macros EM_AARCH64, EM_TILEPRO and EM_TILEGX were defined in diffrent
commits and release of glibc which is glibc 2.16 and 2.17 in elf/elf.h
file. Compiling libabigail was failing duing to undefined behaviour of
above macros in older glibc release. To solve it, configure.ac checks
whether these macros are defined or not and sets accordinlgy
HAVE_EM_AARCH64_MACRO, HAVE_EM_TILEPRO_MACRO and HAVE_EM_TILEGX_MACRO
macros.
* config.h.in: Generated autoheader by configure.ac
for added macros
* configure.ac: Defining HAVE_EM_AARCH64_MACRO,
HAVE_EM_TILEPRO_MACRO and HAVE_EM_TILEGX_MACRO to check
whether EM_AARCH64, EM_TILEPRO and EM_TILEGX macros are defined
in elf.h or not
* src/abg-dwarf-reader.cc (e_machine_to_string): Look for
EM_AARCH64, EM_TILEPRO and EM_TILEGX macros only
if they are defined in elf.h
Signed-off-by: Sinny Kumari <sinny@redhat.com>
commit eea9ce11eabfda96b2192341b50e9dcc27653369
Author: Dodji Seketeli <dodji@redhat.com>
Date: Mon Mar 30 22:02:52 2015 +0200
18180 - runtestreadwrite fails on i386
The issue here is that unresolved decl-only classes and resolved
decl-only classes have the same hash value (zero), and that changes
the type-id of the resolved decl-only class.
This patch ensures that the hash value of a decl-only class is zero
only for non-resolved classes. The patch also fixes an error in an
input XML file.
* src/abg-hash.cc (class_decl:#️⃣:operator()(const class_decl&)
const): Return zero only for class declarations that are not
resolved.
* tests/data/test-read-write/test20.xml: Fix the output to make a
class definition to reference its declaration, when there was a
forward declaration for it.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
At non-complete class resolution time, it appears that some type
lookup can return non-class type (for instance typedef types, when we
are expecting class types.
This patch implements class type lookup specifically (rather than
broad types lookup) and uses that during the non-complete class
resolution process.
* include/abg-fwd.h (lookup_class_type_in_corpus)
(lookup_class_type_in_translation_unit): Declare new functions.
* src/abg-ir.cc (lookup_class_type_in_translation_unit): Define
new function.
(get_node, convert_node_to_decl): Define new specializations for
the class_decl type.
* src/abg-corpus.cc (lookup_class_type_in_corpus): Define new
function.
* src/abg-dwarf-reader.cc
(read_context::resolve_declaration_only_classes): Lookup class
types specifically.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In the previous commit b9fa3b7, I forgot to initialize the new
read_context::m_exported_decls_builder_ data member in abg-reader.cc. This patch fixes
that.
* src/abg-reader.cc (read_context::read_context): Initialize the
new m_exported_decls_builder_ data member.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It turns out the support for reading the native libabigail XML format
is falling short since the work on (late) type canonicalizing.
The type ID -> XML node map is wrongly considered as being per corpus
data while it should be per translation unit data; type IDs are unique
only for a given translation unit.
The code to walk a XML sub-tree to perform the ID -> node mapping is
wrongly walking all the XML node *after* the sub-tree node too; that
is, it walks the entire corpus file starting from the XML sub-tree
node it's given. It should only walk the sub-tree it's given.
These two issues normally solve the crash reported here. But then
there are other related issues too.
The native XML format reader doesn't populate the set of exported
declarations for the current corpus it's building.
This patch addresses all these issues and makes tests/test-abidiff.cc
supports corpus files for a new regression test.
* src/abg-reader.cc (read_context::m_exported_decls_builder_): New
data member.
(read_context::read_context): Initialize it.
(read_context::{type_is_from_translation_unit,
get_exported_decls_builder, set_exported_decls_builder,
maybe_add_fn_to_exported_decls, maybe_add_fn_to_exported_decls,
type_id_new_in_translation_unit}): New member functions.
(read_context::clear_per_translation_unit_data): Clear id->xml
node map here ...
(read_context::clear_per_corpus_data): ... not here.
(read_context::walk_xml_node_to_map_type_ids): Only walk the
sub-tree we are asked to walk.
(read_translation_unit_from_input): Cleanup.
(read_corpus_from_input): Wire populating of exported declarations
of the current corpus.
(build_function_decl, build_var_decl): Populate exported
declarations of the current corpus here.
(build_type_decl, build_qualified_type_decl)
(build_pointer_type_def, build_reference_type_def)
(build_array_type_def, build_enum_type_decl, build_type_decl)
(build_template_tparameter): Adjust assert on ID to make sure
it's the first type it's being defined in the current translation
unit.
* tests/data/test-abidiff/test-corpus0-report0.txt: New test
reference output.
* tests/data/test-abidiff/test-corpus0-v{0,1}.so.abi: New test
input.
* tests/test-abidiff.cc (specs): Add the test inputs above to the
list of inputs over which to run the test harness.
(main): Support reading corpora too, as this test harness was
reading just translation units before.
(tests/data/Makefile.am): Add test material above to source
distribution.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-ir.cc (type_base::get_canonical_type_for): Cleanup the
logic here. Basically since we are not trying to cache the result
of type hashing anymore, this can be simpler.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Until now, if a diff node N has a local change, even if all of its
children nodes are redundant, N is not considered as being redundant.
This is an issue if the local changes of N are filtered out; in that
case, N should be considered redundant.
This patch fixes that. It introduces a second category bitmap on the
diff node that stores the categorization of the diff node that does
*NOT* take in account the categories inherited from its children
nodes. That way, it's possible to know if the *local changes* of a
given node have been filtered out.
* include/abg-comparison.h (diff::{get_local_category,
add_to_local_category, add_to_local_and_inherited_categories,
remove_from_local_category, set_local_category,
is_filtered_out_wrt_non_inherited_categories,
has_local_changes_to_be_reported}): Declare new member functions.
* src/abg-comp-filter.cc ({harmless, harmful}_filter::{visit,
visit_end}): Update local category too.
* src/abg-comparison.cc (diff::priv::local_category_): Add new
data member.
(diff::priv::priv): Initialize it.
(diff::priv::is_filtered_out): Add new member function. This is
factorized out of diff::is_filtered_out().
(diff::is_filtered_out): Re-write in terms of
diff::priv::is_filtered_out().
(diff::{get_local_category, add_to_local_category,
add_to_local_and_inherited_categories, remove_from_local_category,
set_local_category, is_filtered_out_wrt_non_inherited_categories,
has_local_changes_to_be_reported}): Define new member functions.
(suppression_categorization_visitor::visit_begin): Update local
categories too.
(redundancy_marking_visitor::visit_end): If all of the children
nodes of the a diff node N are redundant and if N has filtered-out
local changes, then N is redundant too.
* tests/data/test-diff-filter/libtest28-redundant-and-filtered-children-nodes-v{1,2}.so:
New binary test inputs.
* tests/data/test-diff-filter/test28-redundant-and-filtered-children-nodes-v{0,1}.cc:
Source code for the binary test inputs above.
* tests/data/test-diff-filter/test28-redundant-and-filtered-children-nodes-report-{0,1}.txt:
New test output references.
* tests/test-diff-filter.cc (in_out_specs): Add the test inputs
above to the set of inputs this test harness has to run over.
* tests/data/Makefile.am: Add the test materials above to the
source distribution.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This helps while debugging from GDB.
* include/abg-fwd.h (is_global_scope): Return a global_scope*.
* src/abg-ir.cc (is_global_scope): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-comparison.cc (qualified_type_diff::report): Assert that
if the qualified type diff node has changes to be reported and no
local change, then its child node must have changes to be
reported.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It appears that the comparison engine reports deletion of *static*
data members in the context of the ABI changes of their parent
class. That is not right because static data member don't have any ABI
impact on their parent class. Fixed thus.
* src/abg-comparison.cc
(class_diff::priv::{get_deleted_non_static_data_members_number,
get_inserted_non_static_data_members_number}): Define new member
functions.
(class_diff::reports): Use the new functions above. Also, add
forgotten new lines where they belong.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
We started doing this earlier, but I forgot to update the code in
decl_base::get_hash().
* src/abg-ir.cc (decl_base::get_hash): Do not cache the hash
value.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-dwarf-reader.cc (build_class_type_and_add_to_ir): A type
that has its size defined is not non-complete. Same if it has a
method or a member type.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
From the DWARF emitted by GCC 4.4.7 for libstdc++ we encountered an
interesting construct.
A non-complete version of std::runtime_error is declared in
libstdc++-v3/src/functexcept.cc and is represented in DWARF as:
[ 37344] class_type
name (strp) "runtime_error"
declaration (flag)
Then a bit later, that *non-complete* class is used as a base class
for a class, *without* being fully defined! This shouldn't happen
but, well, it does:
[ 3b3a1] class_type
specification (ref4) [ 3733e]
byte_size (data1) 16
decl_file (data1) 5
decl_line (data1) 141
containing_type (ref4) [ 3734a]
sibling (ref4) [3b405]
[ 3b3b1] inheritance
type (ref4) [ 37344] <---- here.
The thing is that, later, in another translation unit
(libstdc++-v3/src/stdexcept.cc), that same class is defined fully:
[ 7e9f9] class_type
name (strp) "runtime_error"
declaration (flag)
[...]
[ 80c95] class_type
specification (ref4) [ 7e9f9]
byte_size (data1) 16
decl_file (data1) 4
decl_line (data1) 108
containing_type (ref4) [ 7e9ff]
sibling (ref4) [ 80d2b]
[...] <---------- and the definition goes here.
But then you see that the DIE offset of the "version" of the
runtime_error class that is "defined" libstdc++-v3/src/stdexcept.cc in
is different from the version that is only declared in
libstdc++-v3/src/functexcept.cc. But virtue of the "One Definition
Rule", we can assume that they designate the same type. But still,
runtime_error should have been defined in
libstdc++-v3/src/stdexcept.cc. Anyhow, libabigail needs to be able to
handle this. That is, it needs to wait until the entire ABI corpus is
loaded from DWARF, then lookup the definition of all the non-complete
types we have encountered.
And then only after that non-complete type resolution has taken place,
we can proceed with type canonicalizing, rather than doing it after
the loading of each translation unit like what we were doing
previously.
This is what this patch does.
* include/abg-fwd.h (lookup_type_in_corpus): Declare new function.
* src/abg-corpus.cc (lookup_type_in_corpus): Define new function
here.
* include/abg-ir.h (function_types_type): Declare new typedef.
(translation_unit::get_canonical_function_type): Remove member function.
(translation_unit::bind_function_type_life_time): Declare new
member function.
(classes_type): New typedef.
* src/abg-ir.cc
(translation_unit::priv::canonical_function_types_): Remove data
member.
(translation_unit::priv::function_types): New data member.
(translation_unit::get_canonical_function_type): Remove this
function definition.
(translation_unit::bind_function_type_life_time): New function
definition.
(lookup_node_in_scope): Ensure that the type returned is
complete.
* src/abg-dwarf-reader.cc (string_classes_map): New typedef.
(read_context::decl_only_classes_map_): New data member.
(read_context::declaration_only_classes): New accessor.
(read_context::{maybe_schedule_declaration_only_class_for_resolution,
is_decl_only_class_scheduled_for_resolution,
resolve_declaration_only_classes, current_elf_file_is_executable,
current_elf_file_is_dso}): Define new member functions.
(read_context::clear_per_translation_unit_data): Do not clear the
data structures that associate DIEs to decls/types or that contain
the types to canonicalize here. Rather, clear them ...
(read_context::clear_per_corpus_data): ... here instead.
(read_context::build_translation_unit_and_add_to_ir): Do not
perform late type canonicalizing here. Rather, do it ...
(read_debug_info_into_corpus): ... here instead. And before that,
call read_context::clear_per_corpus_data() and the new
read_context::resolve_declaration_only_classes() here.
(build_class_type_and_add_to_ir): Schedule the non-complete types
for resolution to complete types. Assert that base classes that
are non-complete are scheduled to be completed.
(build_function_decl): Do not try to canonicalize function types
this early, systematically. Now, all the non-complete types needs
to be completed before starting canonicalizing. So let function
types go through the normal processes of deciding when to
canonicalize them. But then, bind the life time of the function
type to the life time of the current translation unit.
(maybe_canonicalize_type): If a class type is non-complete,
schedule it for late canonicalizing.
* src/abg-hash.cc (class_decl:#️⃣:operator()(const class_decl&)
const): During hashing, a base class should be complete.
* src/abg-reader.cc
(read_context::clear_per_translation_unit_data): Do not clear
id/xml node, and type maps here. Rather, clear it ...
(read_context::clear_per_corpus_data): ... here instead.
(read_translation_unit_from_input): Do not perform late
canonicalizing here. Rather, do it ...
(read_corpus_from_input): ... here. Also, call the new
read_context::clear_per_corpus_data() here.
(build_function_decl): Do not canonicalize function types here so
early. Rather, bind the life time of the function type to the
life time of the translation unit.
* src/abg-writer.cc (write_translation_unit): Do not clear the
type/ID map here.
* tests/data/test-read-dwarf/test2.so.abi: Adjust test input.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This is useful to call is_type() under GDB.
* include/abg-fwd.h (is_type): Declare new overload that takes a
naked pointer.
* src/abg-ir.cc (is_type): Define new overload that takes a naked
pointer.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
I am seeing issues related to the fact that a declaration-only class A
would compare different to the full version of class A. This is due
to the fact that that the declaration-only A and the full A have
different hashes, even though they structurally compare equal. So
they have different canonical types, with the current code. This
patch arranges for declaration-only classes to have no canonical type,
forcing it to compare structurally to other types. Then the patch
adjusts strip_typedef() that used to expect that all types it sees
have canonical types. Then the patch changes the type hashing code to
avoid making it cache their hash, because otherwise, in some cases
when we hash a type (too) early, a temporary hash of it gets stored ad
infinitum, even after the type has been later updated. Last but not
least, the patch returns a zero hash for declaration-only classes.
* include/abg-fwd.h (keep_type_alive): Declare new function.
* src/abg-ir.cc (strip_typedef): Simplify logic. Support types
that are not canonicalized.
(type_base::get_canonical_type_for): For declaration-only classes,
return an empty canonical class, forcing the class to be compared
structurally.
(keep_type_alive): Define new function.
* src/abg-hash.cc ({decl_base, type_decl, scope_type_decl,
qualified_type_def, pointer_type_def, reference_type_def,
array_type_def, enum_type_decl, typedef_decl,
class_decl::member_class_template, class_decl, type_tparameter,
template_tparameter, }:#️⃣:operator()): Do not cache the
computed hash.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Propagating redundancy categorization is broken for cases where there
is a diff node that has children nodes carrying changes that are all
filtered out. In that case, if among those children changes there is
a redundant change, normally the parent diff node we are look at
should be marked redundant too. The bug is that it's not marked as
redundant at the moment. This patch fixes that.
* src/abg-comparison.cc (redundancy_marking_visitor::visit_end):
Consider the cases of changes that are a filtered out.
* tests/data/test-diff-filter/libtest27-redundant-and-filtered-children-nodes-v{0,1}.so:
New test binaries to use as test input.
* tests/data/test-diff-filter/test27-redundant-and-filtered-children-nodes-report-{0,1,2}.txt:
New test result baselines.
* tests/data/test-diff-filter/test27-redundant-and-filtered-children-nodes-v{0,1}.cc:
Source code for the test input binaries above.
* tests/test-diff-filter.cc (in_out_spec): Add the binaries to the
test inputs used for this test harness.
* tests/data/Makefile.am: Add the new test material above to the
distribution.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
From inside the comparison engine, I noticed that there were some
discrepancies between some comparison performed there and the
comparison performed from inside the internal representation of
abigail::ir. This can lead to some change reports in which the
reporter thinks there are changes in the IR where there actually are
not. This patch re-uses comparison operators from the generic IR, rather
than re-implementing them in the comparison engine.
* include/abg-ir.h (operator==(scope_decl_sptr, scope_decl_sptr)):
Declare.
(operator==(type_decl_sptr, type_decl_sptr)): Likewise.
(operator==(enum_type_decl_sptr, enum_type_decl_sptr)): Likewise.
* src/abg-comparison.cc (diff_length_of_decl_bases)
(diff_length_of_type_bases): Remove these static functions.
(class_diff::has_changes): Re-use the comparison operator for
class_decl_sptr.
(type_decl_diff::has_changes): Re-use the comparison operator for
type_decl_sptr.
* src/abg-ir.cc (operator==(scope_decl_sptr, scope_decl_sptr)):
Define.
(operator==(type_decl_sptr, type_decl_sptr)): Likewise.
(operator==(enum_type_decl_sptr, enum_type_decl_sptr)): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
While diff-ing libstdc++ against EL 6.5 and EL 7, in the report for
virtual member functions of class types, I noticed that there were
some un-necessary white spaces emitted there. This patch fixes that.
* src/abg-comparison.cc (class_diff::report): When reporting
virtual member functions make sure to emit the newline only if one
report for member function has already been emitted.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This speeds up comparison of function types.
* src/abg-dwarf-reader.cc (build_function_decl): Call
maybe_canonicalize_type to canonicalize the function type.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In the code that compares function types, smart pointers handling showed up
on the hot path of some performance profile. This patch uses naked
pointers there.
* src/abg-ir.cc (equals): In the overload for function types, use
more naked pointers, less smart pointers.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Some smart pointers handling got high on performance profiles. I am
passing those by reference here.
* include/abg-fwd.h (get_member_is_static, is_member_function)
(get_member_function_is_ctor, set_member_function_is_ctor)
(get_member_function_is_dtor, set_member_function_is_dtor)
(get_member_function_is_const, set_member_function_is_const)
(get_member_function_vtable_offset)
(set_member_function_vtable_offset)
(get_member_function_is_virtual): Declare the smart pointer
parameter of these as being passed by reference.
* include/abg-ir.h (get_member_access_specifier)
(get_member_is_static, get_member_access_specifier)
(set_member_function_is_ctor, set_member_function_is_const)
(set_member_function_vtable_offset): Likewise, for these friend
declarations to the decl_base type.
* src/abg-ir.cc (get_member_access_specifier)
(get_member_is_static, is_member_function)
(get_member_function_is_ctor, set_member_function_is_ctor)
(get_member_function_is_dtor, set_member_function_is_dtor)
(get_member_function_is_const, set_member_function_is_const)
(get_member_function_vtable_offset)
(set_member_function_vtable_offset)
(get_member_function_is_virtual): In these definitions, the smart
pointer parameter is passed by reference.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Accessing the context relationship of declarations and setting some
member properties appear to be high in performance profiles due to
shared pointer handling. This patch makes the context relationship
accessors return a naked pointer and also passes a bunch of shared
pointer as references around.
* include/abg-fwd.h (set_member_is_static): Add an overload that
takes the member as a reference to a smart pointer.
(set_member_function_{is_dtor, is_ctor, is_const, vtable_offset,
is_virtual}): Pass the member function as a reference.
(set_member_function_is_const, set_member_function_is_virtual):
Pass the member function as a non-const reference.
* include/abg-ir.h (decl_base::get_context_rel): Return a naked
pointer.
(set_member_is_static, set_member_function_is_virtual): Adjust
this friend declaration.
(set_member_access_specifier): Add an overload that takes a
reference to the member. Pass a reference to smart pointer to the
other overload.
(set_member_function_is_{is_ctor,is_dtor,is_const,is_virtual,vtable_offset}):
Take a non-const reference to function_decl.
* src/abg-ir.cc (decl_base::get_context_rel): Likewise.
(equals(const decl_base&, const decl_base&, change_kind*)):
Adjust.
(equals(const var_decl&, const var_decl&, change_kind*)):
Likewise.
(get_member_access_specifier, get_member_is_static)
(set_data_member_offset, get_data_member_offset)
(set_data_member_is_laid_out, get_data_member_is_laid_out)
(get_member_function_is_ctor, set_member_function_is_ctor)
(get_member_function_is_dtor, set_member_function_is_dtor)
(get_member_function_is_const, set_member_function_is_const)
(get_member_function_vtable_offset)
(set_member_function_vtable_offset)
(get_member_function_is_virtual, set_member_function_is_virtual):
Likewise.
(set_member_access_specifier): Add an overload that takes a
reference to decl_base.
(set_member_is_static, set_member_function_{is_dtor, is_ctor,
is_const, vtable_offset, is_virtual}): Pass the member function as
a reference.): Add an overload that takes the member as a
reference, and write the older overload in terms of the new one.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* incude/abg-ir.h (decl::get_{qualified_name,
qualified_parent_name}): Return a reference to a string rather
than a copy of a string.
(qualified_type_def::get_qualified_name): Likewise.
(reference_type_def::get_qualified_name): Likewise.
(array_type_def::get_qualified_name): Likewise.
(class enum_type_decl::enumerator): Make this is an out-of-line
pimpled class implementation.
(enum_type_decl::enumerator::{get, set}_enum_type): Declare new
method.
(enum_type_decl::enumerator::get_qualified_name): Change this so
that it doesn't take the name of the enum type anymore.
* src/abg-comparison.cc (enum_diff::report): Adjust for
enum_type_decl::enumerator::get_qualified_name() not taking the
name of the enum type anymore.
* src/abg-ir.cc (decl_base::get_qualified_parent_name): Return a
reference to string.
(decl_base::get_qualified_name): Likewise.
(decl_base::get_qualified_name(string&)): Use the new verson of
decl_base::get_qualified_name() that returns a reference.
({qualified_type_def, pointer_type_def, reference_type_def,
array_type_def}::get_qualified_name()): Return a string reference.
({qualified_type_def, pointer_type_def, reference_type_def,
array_type_def}::get_qualified_name(string& qualified_name)
const): Use the new qualified_type_def::get_qualified_name() that
returns a string reference.
(class enum_type_decl::priv): New type.
(enum_type_decl::{get_underlying_type, get_enumerators}): Adjust.
(enum_type_decl::{enumerator::enumerator, enumerator::operator==,
enumerator::get_name, enumerator::get_qualified_name,
enumerator::set_name, enumerator::get_value,
enumerator::set_value, enumerator::get_enum_type,
enumerator::set_enum_type}): Define methodes out-of-line here.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Better support typedefs with void underlying types.
* src/abg-ir.cc (strip_typedef): Consider that the underlying type
can be void.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The DWARF reader assumes that the DIEs for all member types are seen
by build_class_type_and_add_to_ir(), as member type DIEs of the DIE of
the class. Well that assumption is not correct because there can be
errors in the DWARF we are looking at. One of these errors I stumbled
accross is that a DIE for a typedef that should be a member typedef is
actually a child of a *function* DIE. And that function DIE is a
child of the class. Go figure. In any case, get_scope_for_die()
already fixes that up and behaves as if the DIE of the typedef is a
child of the DIE of the class. A side effect of this is that when
build_class_type_and_add_to_ir() reads the DIE of the class, it never
sees the DIE for that typedef.
The takeaway of this state of affairs is that we cannot rely on
build_class_type_and_add_to_ir() to update the member access specifier
for member types because it does not see all member types. Rather
build_ir_node_from_die() detects (reliably) that the type is a member
type and updates the access specifier there.
I also realize that the "is_member_type" flag of
build_ir_node_from_die() and friends is useless now because inside
build_ir_node_from_die() to know that that the type we are building is
a member type, we just need to look at the scope and see if it's a
class type.
So by doing all this, this patch fixes the fact that some types were
not being canonicalized because build_class_type_and_add_to_ir() was
not seeing them. Ahhhh, DWARF.
* include/abg-fwd.h (is_class(decl_base*)): Return a class_decl*
rather than just a bool.
* abg-ir.cc (is_class(decl_base*)): Return a class_decl* rather
than just a bool. Simplify the implementation.
* src/abg-dwarf-reader.cc
(maybe_set_member_type_access_specifier): Define new static
function.
(build_ir_node_from_die): Remove the is_member_type flag. When
building member types set their access specifier. Simplify the
logic of detecting that a type is a member type; basically
delegate taht to the new maybe_set_member_type_access_specifier().
(build_class_type_and_add_to_ir): Do not try to set the member
type access specifiers anymore.
(build_qualified_type, build_pointer_type, build_reference_type)
(build_typedef_type, build_var_decl, build_function_decl): Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Now that we have type canonicalizing, there is no need for trying to
be smart when comparing types; just do the comparison and it should be
fast. Plus in the case of enum_diff, we just getting it wrong as we were
not checking several parts of the enum type, like the member access
specifiers if it was a member type, etc ...
* src/abg-comparison.cc (enum_diff::has_changes): Just use the
normal comparison operator to compare the two enums here. It's
fast now.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Until now, after the ABI corpus was built from DWARF, the translation
units of the corpus were walked and each function was considered for
addition into the set of exported decls. During that walking, a first
version of the set was put into a std::list and then, a set of filters
(user-provided tunables like a list of regular expressions to keep or
remove some functions from the exported decls) is applied to that list
and the final set of exported decls is put in a std::vector.
Profiling has shown that this process of building the set of exported
decls is a hot spot and also that the current use of std::list was a
big memory consumer especially on binaries with large exported symbol
tables.
So this patch builds the set of exported decls "on the fly", during
DWARF reading, as opposed to waiting after the DWARF is read and
having to walk the corpus again. The corpus defines a policy object
that encapsulates the methods for determining if a function or
variable ought to be part of the set of exported decls. The DWARF
reader uses that policy object to determine which functions and
variables among those built during the reading ought be part of the
exported decls; the policy object also has a reference to the final
vector (managed by the corpus) that must hold the exported decls, so
the decls are put in that vector directly without unnecessary copying.
Profiling also showed that the string copying done by
{var_decl,function_decl}::get_id() was a hot spot. So the patch
returns a reference there.
With this patch applied, the peak memory consumption of abidiff on
libabigail.so itself (abidiff libabigail.so libabigail.so) is 54MB of
resident and takes 2 minutes and 16s (on my slow system). Without the
patch the peak consumption was more than 300MB and it was taking
slightly longer.
For the test of bug
https://sourceware.org/bugzilla/show_bug.cgi?id=17948, memory
consumtion and wall clock time spent is down from 3.4GB and 1m59s to
760MB and 0m43s.
* include/abg-ir.h ({var,function}_decl::get_id): Return a
reference.
* src/abg-ir.cc ({var,function}_decl::get_id): Return a reference
to the string rather than copying it over.
* include/abg-corpus.h (class corpus::exported_decls_builder):
Declare new type.
(corpus::{sort_functions, sort_variables,
maybe_drop_some_exported_decls, get_exported_decls_builder}):
Declare new methods.
* src/abg-corpus.h (corpus::exported_decls_builder::priv): Define
new type.
(class symtab_build_visitor_type): Remove this type that is
useless now.
(corpus::exported_decls_builder::{exported_decls_builder,
exported_functions, exported_variables,
maybe_add_fn_to_exported_fns, maybe_add_var_to_exported_vars}):
Define new functions.
(corpus::priv::is_public_decl_table_built): Remove this data
member. It's now useless.
(corpus::priv::priv): Adjust.
(corpus::priv::build_public_decl_table): Remove this member
function. It's now useless.
(corpus::{priv::build_unreferenced_symbols_tables, get_functions,
get_variables}): No need to build the public decls table here.
It's already built by the time the corpus is read from DWARF now.
(corpus::{sort_functions, sort_variables,
maybe_drop_some_exported_decls, get_exported_decls_builder}):
Define new member functions.
* src/abg-dwarf-reader.cc (read_context::exported_decls_builder):
New data member.
(read_context::read_context): Initialize it.
(read_context::{exported_decls_builder,
maybe_add_fn_to_exported_fns, maybe_add_var_to_exported_vars}):
Define new member functions.
(read_debug_info_into_corpus): Get the the new
'exported_decls_builder' object from the corpus and stick it into
the read context so the DWARF reading code can use it to build the
exported decls set. When the DWARF reading is done, sort the set
of exported functions and variables that was built.
(build_ir_node_from_die): When a function or variable is built,
consider putting it into the set of exported decls.
* tools/abicompat.cc (main): Now that the exported decls is built
*before* we had a chance to stick the list of symbol IDs to keep,
call corpus::maybe_drop_some_exported_decls() to update the set of
exported decls we should consider for the corpus.
was applied to that list and the final
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
So abidiff libabigail.so libabigail.so is broken again. Sigh.
It was broken by this wrong commit:
commit 5b33223cb7
Author: Dodji Seketeli <dodji@redhat.com>
Date: Fri Feb 20 13:48:48 2015 +0100
Simplify canonicalizing handling for typedefs
* src/abg-dwarf-reader.cc (build_ir_node_from_die): For typedefs,
we don't need to test that the current scope is a class to know
that we are looking at a member type. Just looking at the
is_member flag is enough.
So the issue arises when for instance, we are reading a class that
defines a member typedef (or enum) and uses that enum as the type of a
data member. When reading that data member (before reading the
definition of the typedef), we read the type of the data member; so we
hit the typedef. But build_ir_node_from_die() cannot fully construct
the scope of the typedef before handing off the typedef because we are
currently building it! So it hands out a non-complete version of the
class that is being built; 'is_member' is not set to 'true' because
we are getting the type of the data member; it's not *necessarily* a
member type. So we need to check !is_class_type(scope) to know if we
are given a member type. I am now thinking that the "is_member" flag
is actually useless. I think I'll remove it in a later patch.
Anyway, this fixes 'abidiff libabigail.so libabigail.so' again. I
have some stashed patches that brings it's time down to ~ 45 seconds.
So we are getting close to being able to include that *ultimate* test in
regression test suite. Oh well.
* src/abg-dwarf-reader.cc (build_ir_node_from_die): When building
typedefs, enum and memeber classes, check that the scope is a
member class to detect if we are building a member type. In which
case the caller is going to handle the canonicalizing of the
member type *after* it's access specification has been adjusted.
Otherwise, that adjustments happens after the type has been
canonicalized and bad things happen at comparison type.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Bug URL: https://sourceware.org/bugzilla/show_bug.cgi?id=17649.
abidiff stumbled accross a diff graph with cycles. And it kept
walking that graph endlessly. Of course.
It turned out on such graphs with cycles, the categorizing code that
uses abigail::comparison::diff::traverse() to walk the graph and
categorize the diff nodes was traversing the same class of equivalence
of certain diff nodes more than once without even noticing.
This patch changes the logic of the diff graph traversing code to make
it always call diff_node_visitor::visit_begin() on the visitor for a
diff node prior to visiting it (visiting means calling
diff_node_visitor::visit()) and diff_node_visitor::visit_end() after
visiting it.
But when the diff node has already been visited and it's reached again
by the traversing code (in case of a cycle) then the
diff_node_visitor::visit_begin() is called, but
diff_node_visitor::visit() is *NOT*. Then
diff_node_visitor::visit_end() is called. In other words, even when
the diff node is not visited (because it's already been visited) the
pair diff_node_visitor::{visit_begin,visit_end}() is called.
This avoids traversing the diff node (or rather the equivalence class
of the diff node) more than once even in presence of cycles, but still
gives a chance to custom visitors to detect that they are seeing a
cycle and act accordingly if need be. This is a kind of cycle
detection feature.
Then the code of the (harmless and harmful categorization) filters has
been adapted to always rely on the cycle detection feature. The code
of the category propagation visitor has also been adapted to propagate
the category of a given diff node to and from its canonical diff node.
* include/abg-comp-filter.h (harm{less,ful}_filter::visit_end):
Declare new methods.
* include/abg-comparison.h (diff_context::maybe_apply_filters):
Remove the traverse_nodes_once flag.
* src/abg-comp-filter.cc (apply_filter): Force the traversing to
operate in cycle avoidance mode.
(harm{less,ful}_filter::visit): Update the category of the
canonical node too.
(harm{less,ful}_filter::visit_end): Define new method.
* src/abg-comparison.cc (diff_context::maybe_apply_filters):
Remove the traverse_nodes_once flag. Adjust. Simplify logic.
(diff::traverse): Always call diff_node_visitor::{begin,end}. If
the node has already been visited previously then do not call
diff_node_visitor::visit() and do not visit the children nodes.
(category_propagation_visitor::visit_end): If the node has
already been visited, then propagate the category from the
canonical nodes of the children nodes.
(propagate_categories): Force the traversing to operate in cycle
avoidance mode.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-comparison.cc (distinct_diff::report): After calling
report_size_and_alignment_changes, one needs to add a new line if
some stuff got emitted out the output stream.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Since we now have proper diff node filtering capabilities, it appears
that there can be distinct diff node that is deemed to be reported
even though it's child node is not; this happens when the distinct
diff node does carry local changes. So remove the assert that was
saying otherwise and enjoy one less abort.
* src/abg-comparison.cc (distinct_diff::report): Remove over-eager
assert.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The late canonicalizing code needed factorizing to increase
maintainability.
* src/abg-dwarf-reader.cc
(read_context::{canonicalize_types_scheduled,
perform_late_type_canonicalizing}): Factorize these from ...
(build_translation_unit_and_add_to_ir): ... here.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
