When the DWARF reader of libabigail sees a data member for a given
class flagged as being a declaration, it considers the declaration as
being a definition. The reason why it doesn't strictly trust the
"is_declaration" flag of DWARF is that some DWARF producers sometimes
wrongly emit that flag.
But then, it turns out that a class declaration can have a *static*
data member without loosing its declaration-only property. This patch
thus changes the behaviour of the DWARF reader to make it consider the
class declaration as being a definition when the class has a
*non-static* data member; a static data member only is not enough to
make the class declaration become a definition.
* src/abg-dwarf-reader.cc (build_class_type_and_add_to_ir): The
presence of a data member shouldn't make a declaration-only class
loose its declaration-only-ness; the presence of a enon-static*
data member should.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Adjust.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-4.8.3.so:
New binary test input.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-4.9.2.so:
Likewise.
* tests/data/test-diff-filter/test31-pr18535-libstdc++-report-0.txt:
New test reference output.
* tests/data/Makefile.am: Add the new test material to the build
system.
* tests/test-diff-filter.cc (in_out_specs): Add the new test
inputs to the set of inputs to consider.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This is a debugging and sanity check option. It saves the abi of the
ELF binary to a temporary file, reads it back and compares the abis of
the temporary file against the abi of the input ELF binary.
* tools/abidw.cc (options::abidiff): New data member.
(options::options): Initialize it.
(display_usage): Add a usage string for the new --abidiff option.
(parse_command): Parse the new --abidiff options.
(main): Save the abi of the input elf in a temporary abixml file;
read it back and compare both.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-hash.cc (class_decl:#️⃣:operator()): Use a temporary
variable to ease debugging.
* src/abg-reader.cc (read_context::is_wip_type): Make this
function const.
* src/abg-writer.cc (write_context): Move data members at the top.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Recursive type hashing was showing up as the major hot spot of
performance profiles. After spending a few days on trying to speed it
up, I have officially declared recursive tree node hashing as a slow
process and I am giving up.
I have thus decided to not use that at type canonicalization time.
Rather, I am proposing a new type canonicalization routine where types
are first hashed by hashing their pretty representation string.
Basically, if T is the total number of types in the system and C the
number of classes of equivalences (or the number of canonical types),
the number of type comparisons done by a naive type canonicalization
routine is N x C. With the worse C being equal to N itself, that
worse number of comparisons is N*N.
By using a hash table to store the canonical types, keyed by a hash of
their pretty representation string, the number of type comparisons can
be brought down to N*P, where P is a the greater number of which
pretty representation string hash collide. That number P is usually
small; my measurements show that N usually goes from 1 to 3. And
moreover, computing the hash of the pretty representation string of
the function is way faster than using the recursive type hash!
As a result, running abidw on the libcilkrts.so library, from GCC goes
from 12 minutes to 0.4 seconds!
Incidentally, now that we are not trying to speed up the recursive
type hashing process, all the complicated business we had around
caching the result of the hashing is gone! I was thinking that hash
cashing was inherently a bad idea, especially for recursive types --
that refer to themselves directly or indirectly, because in those
case, depending on when you cached the hash value, the value of the
hashing can be different.
The abixml writer's code doesn't use the recursive type hash anymore
either; it uses the pointer value of the canonical type as hash.
Super fast too!
The patch had to fix pieces here and there to comply with the fact
that canonical types are now used across the board in a mandatory
fashion.
* include/abg-ir.h (canonical_types_map_type): Adjust this typedef
to make it point to an unordered_map which the key is now a string
and the value is a vector of types.
(type_or_decl_base::{get_cached_hash_value, set_cached_hash_value,
cached_hash}): Remove these member functions and type.
(struct type_base::cached_hash): Remove.
* src/abg-ir.cc (struct type_or_decl_base::priv::hash_): Remove.
(type_or_decl_base::priv::priv): Adjust.
(type_or_decl_base::{g,s}et_cached_hash_value): Remove.
(type_base::get_canonical_type_for): For declaration-only classes,
look at their definition for the canonical_type. Do not use
recursive type hashing anymore. Rather, use the pretty
representation string, and hash that.
(class_decl::base_spec::get_hash): Do away with hash value caching
here.
(class_decl::operator==): For decl-only classes, look at their
definitions for canonical types.
(hash_type_or_decl): Adjust comment. Use the canonical type
pointer value for type hash. That's the fast path. Otherwise, if
not available, fall back to a slow path which is the recursive
type hash we were using before.
* src/abg-dwarf-reader.cc (maybe_canonicalize_type): Schedule all
classes and typedef to classes for late canonicalization.
* src/abg-hash.cc (type_base::dynamic_hash::operator()): There is
no hash value cashing anymore.
(type_base::cached_hash::operator()): Remove.
* src/abg-reader.cc (read_context::get_type): Slight style
adjustment.
(read_translation_unit_from_file)
(read_translation_unit_from_buffer): Do not forget to canonicalize
types when reading just one translation unit.
(build_type_tparameter, build_template_tparameter): Canonicalize
the type.
* src/abg-writer.cc (struct type_hasher): New hasher type.
(type_ptr_map): Use a deep pointer comparison equal operator
functor, and canonical types as type hash values.
(write_class_decl): Do not write size and alignment on decl-only
classes. Do not record decl-only classes as being emitted. Their
definition must be emitted before.
* tests/test-read-write.cc (main): Do not do abi testing on
translation units (as opposed to doing it on abi corpora) as that
code is not wet yet. We need to know how to diff namespaces.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The building of the qualified name of a declaration is showing up in
performance profile as a hot spot. This patch addresses that
performance issue by updating the qualified name of a declaration
whenever the declaration is added to its context and saving the
result. Getting the qualified name later is just a matter of a string
copy. I guess we can do something about those string copies later as
they don't show up high performance profiles at the moment.
* include/abg-ir.h (decl_base::priv_): Make this be public, so
that the qualified name updater function can access it.
(class class_decl): Make set_member_is_static() a friend function.
* src/abg-ir.cc (class ::qualified_name_setter): New tree walking
type.
(decl_base::get_qualified_parent_name): Do not do any computation
here. Just return the pre-computed qualified parent name string.
(decl_base::get_qualified_name): Likewise, for qualified name.
(scope_decl::{add,insert}_member_decl): Update the qualified name of the
newly added member. Set the scope of the member here. It's not
going to be set elsewhere, from now on.
(add_decl_to_scope): Do not set the scope here anymore. Just call
scope_decl::add_member_decl and let it do the work.
(insert_decl_into_scope): Likewise, just call
scope_decl::insert_member_decl and let it do the work.
(class_decl::{add_data_member, add_member_function}): Do not
handle details of context setting at this point. Let
scope_decl::add_member_decl do it. Adjust the properties of the
context relation afterwards. In add_data_member, when a data
member changes its static-ness, move the data member into the
class_decl::priv::non_static_data_members_ or out of it, as
necessary.
(class_decl::insert_member_decl): By default, a data member is
considered static.
(set_member_is_static): Move this definition after the definitions
of class_decl, so that this function can see those. Also, when a
data member changes its static-ness, move the data member into the
class_decl::priv::non_static_data_members_ or out of it, as
necessary.
(class_decl::add_member_function_template): As we the
underlying function template decl to the context, do not do any
scope adding for it here.
(::qualified_name_setter::{do_update, visit_begin}): Define new
member functions.
(update_qualified_name): Define new static function.
* src/abg-reader.cc (build_class_decl): Make build_function_decl,
build_var_decl, build_function_tdecl and build_class_tdecl
automatically add the created decl to their context, and then
update the properties of the resulting member decl later, just
like what we do in the DWARF reader.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch adds a new set of test functions that are going to be used
in subsequent patches to come.
* include/abg-fwd.h (is_function_decl, is_decl, is_namespace)
(is_scope_decl): Declare new function overloads.
* src/abg-ir.cc (is_function_decl, is_decl, is_namespace)
(is_scope_decl): Define them.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch adds support for properties source_location_not_in and
source_location_not_regexp in the [suppress_type] section of
suppression specifications. So the suppression specification:
[suppress_type]
source_location_not_in = foo1.h, foo2.h bar1.h bar2.h
suppresses ABI change reports about types that are *NOT* defined in
files foo{1,2}.h and bar{1,2}.h. The intended use of this construct is to
constrain abi change reports to types that are part of the API of a
given shared library. The API of the library is supposed to be
defined in foo.h and bar.h only.
Similarly, the suppression specification:
[suppress_type]
source_location_not_regexp = (foo|bar){1,2}\\.h
suppresses ABI change reports about types that are not defined in the
same set of files foo1.h, foo2.h, bar1.h and bar2.h.
* include/abg-ini.h (enum property_value::value_kind): Add a
LIST_PROPERTY_VALUE kind.
(class {list_property_value, list_property}): Declare new types.
(is_list_property, is_list_property_value): Declare new functions.
* src/abg-ini.cc (struct list_property_value::priv): Define new
type.
(list_property_value::{list_property_value, get_content,
set_content, as_string}): Define new member functions.
(is_list_property_value): Define new function.
(struct list_property::priv): Define new type.
(list_property::{list_property, get_value, set_value,
handle_escape}): Define new member functions.
(is_list_property): Define new function.
(read_context::buf_): New data member.
(read_context::{peek, get, put_back, good, eof, read_string,
read_list_property_value}): New member functions.
(read_context::read_next_char): Use the new read_context::{get,
good, eof} member function, rather than using the input stream
directly.
(read_context::{skip_white_spaces, skip_comments,
skip_white_spaces_or_comments, read_property_name,
read_function_name, read_function_argument,
read_function_call_expr, read_property_value,
read_tuple_property_value, read_section_name, read_section}):
Adjust to use the new member functions of read_context rather than
using the input stream directly.
(read_context::read_string_property_value): Likewise. Use the new
read_context::read_string() method.
(read_context::{read, write}_property): Support reading list_property.
* include/abg-comparison.h
(type_suppression::{get_source_locations_to_keep,
set_source_locations_to_keep,
set_source_location_to_keep_regex_str,
get_source_location_to_keep_regex_str}): Add new member functions.
* src/abg-comparison.cc
(type_suppression::priv::{source_location_to_keep_,
source_location_to_keep_regex_str_,
source_location_to_keep_regex_}): Add new data members.
(type_suppression::priv::{g,s}et_source_location_to_keep_regex):
Define new member functions.
(type_suppression::{g,s}et_source_locations_to_keep): Define new
member functions.
(type_suppression::{g,s}et_source_location_to_keep_regex_str):
Likewise.
(type_suppression::suppresses_type): Support
"source_location_not_regexp" and "source_location_not_in"
properties of suppression specifications.
(read_type_suppression): Likewise. Also adjust to the fact that
ta tuple property value that is a list of strings is not a list
property value.
* doc/manuals/libabigail-concepts.rst: Add documentation for
source_location_not_in and source_location_not_regexp.
* tests/data/test-diff-suppr/libtest26-loc-suppr-v{0,1}.so: New
binary test inputs.
* tests/data/test-diff-suppr/test26-loc-suppr-{0,1,2}.suppr: New
suppression specification test inputs.
* tests/data/test-diff-suppr/test26-loc-suppr-report-{0,1,2,3}.txt:
New test reference reports.
* tests/data/test-diff-suppr/test26-loc-suppr-v{0,1}.cc: Source
code of the test binary input above.
* tests/data/test-diff-suppr/test26-loc-suppr.h: Likewise.
* tests/data/Makefile.am: Add the new test material to source
distribution.
* tests/test-diff-suppr.cc (in_out_specs): Add the new test inputs above.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* include/abg-ini.h (simple_property::{simple_property,
set_value}): Pass the value shared pointer by reference.
* src/abg-ini.cc (simple_property::{simple_property, set_value):
Pass the value shared pointer by reference.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Make string_ends_with() allow suffixes that are as long as the string we are
looking at.
* src/abg-tools-utils.cc (string_ends_with): Allow suffixes of the
same size as the input string.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* tools/abipkgdiff.cc (package::erase_extraction_directory)
(extract_rpm, extract_deb, extract_tar)
(erase_created_temporary_directories_parent): Check the return
value of the system() function.
(extract_package): Add necessary white space.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* doc/manuals/Makefile.am (install-man-and-info-doc): Don't check
for abidiff.info, that thing doesn't exist. Rather, check for
abigail.info. Use gzip -f in case the zipped file exists already.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-ir.cc (equals): In the overload for decl_base, compare
qualified names, not just names.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-comparison.cc (function_comp::operator()): Fix a typo
preventing the proper sorting of function name when their
declarator names are equal. Oops.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.txt: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Look at this code:
struct list;
struct payload
{
int value;
list* parent_list; //<-- the hash value of struct list when looking
// through this pointer is the non-zero
// value as computed on the struct list
// type below.
};
struct list
{
payload* p; // <-- While walking the struct list type, the hash
// value of the 'struct list' sub-tree node when
// looking through this pointer is zero, because we
// are still computing the hash value of struct list.
// we do it this way to break the otherwise infinite
// recursion that might occur here.
list* next; // <-- likewise here.
list* prev; // <-- likewise here.
}; // <-- when we reach this point the hash value of struct list
// is computed and is different from zero.
Basically, when a type refers to itself in one of its sub-type (like
struct list here, where list::p refers to struct list, because its
type contains a pointer to struct list), then we need to devise a way
to break the infinite recursion we might fall into when computing its
hash value.
So, when computing the hash value of struct list, when we look at the
type of list::prev, which is "list*", we say that the hash value of the type
pointed to by the type of list::next (which is struct list itself) is
zero. This allows us to break the possibly infinite recursion here.
But then, this means that the hash value of "struct list" depends on
*when* we request that hash value. If we are computing the hash value
of struct list itself, then the temporary value of "struct list" is
zero. But then once we are done computing the hash value of "struct
list", that value becomes non-zero.
Hence, the hash value of a type depends on when that value is
computed.
But then if we want to cache that hash value and re-use it later,
which value should we cache? Definitely not the zero value!
So in other words, we can use (and thus cache) the hash value of a
given type T only after the hash values of all types which use T have
been computed.
To satisfy that condition, we decide to use the (cached) hash value of
each type only after we've computed all the hash values of all types
of the system.
So, during type canonicalization, when a type T is canonicalized, this
patch stores the hash value of T. But then it's only when all types
are canonicalized that the hashing code is allowed to re-use the
cached value of types.
This fixes the issues of spurious type differences introduced when the
same type was read either from DWARF or from abixml. Those
differences where introduced by differences in the order of hashing
types which sub-types refer to themselves. The patch also updates
regression tests accordingly.
* src/abg-dwarf-reader.cc (read_debug_info_into_corpus): Before we
read debug info and build the IR, set a flag in the environment
saying that type canonicalization isn't finished yet. But then,
after type canonicalization is done, flip that flag to say that
type canonicalization is done.
* src/abg-reader.cc (read_corpus_from_input): Likewise.
* src/abg-ir.cc (type_base::get_canonical_type_for): Once a type
has been canonicalized, cache its hash value.
* src/abg-hash.cc (type_base::dynamic_hash::operator()): If type
canonicalization has been done and if the type has a cached value,
use that one.
* tests/data/test-read-dwarf/test2.so.abi: Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
There are resources needed by the type system and other artifacts of
libabigail. Today, when the life time of those resources need to be
greater than all of artifacts of Abigail, then said resources are made
global.
But then global resources are not great, if anything because they
complicate the future use of the library in concurrent computing
setups.
As I was in the need to add one resource to be used by the type
system, I decided to sit down and first overhaul how these long lived
resources needed to be handled.
And here comes the concept of "environment". An environment is a
place where one can put resources that need to live longer than all
the other artifacts of the Abigail system. And so, the code that
creates Abigail artifacts needs and environment of for said artifacts
to use. In other words, artifacts now use an environment.
This has interesting and strong implications. We can only compare two
artifacts if they use the same environment. This is quite a strong
requirement.
But then when this requirement is fulfilled, comparing two types
amounts to just comparing two pointer values; hash values for types
can also be cached. Now *that* is great for speed of comparison, is
it not?
This patch introduce the concept environment (which is basically a new
abigail::ir::environment type), removes the global variables and uses
the environment instead. Each ABI artifact (either type or decl) now
has a ::get_environment() member function to get its environment.
This patch also disables the caching of hash values because the
caching must happen only *after* all types have been canonicalized.
We were not respecting that requirement until now, and that introduces
wrong hash values. A subsequent patch is going to re-introduce hash
value caching again, once the infrastructure is in place to set a flag
in the environment (hah!) once type canonicalization is done, and then
later read that flag when some client code requests a hash value, to
know if we should look in the hash value cache or not.
The patch obviously changes the output of numerous regression tests
(if anything b/c it disables hash value caching) so 'make check'
yields regressions. But then, it's only the subsequent patch that
updates the tests.
* include/abg-ir.h: Adjust note about memory management.
(class environment): Declare new class.
(translation_unit::translation_unit): Take an environment in
parameter.
(translation_unit::{g,s}et_environment): Declare new member
functions.
(type_or_decl_base::{g,s}et_environment): Likewise.
(type_or_decl_base::{get_cached_hash_value,
set_cached_hash_value}): Change the name of
decl_base::peek_hash_value() and decl_base::set_hash() here into
these and move them here.
(type_or_decl_base::hashing_started): Move
decl_base::hashing_started() here.
({g,s}et_environment_for_artifact): Declare new functions.
(class decl_base): Move member functions hashing_started(),
peek_hash_value() and set_hash() on to the type_or_decl_base base
class.
(scope_decl::scope_decl): Initialize the virtual member
type_or_decl_base().
(type_decl::{get_void_type_decl,
get_variadic_parameter_type_decl}): Remove these static member
functions. They are now non-static member functions of the new
environment type.
* src/abg-ir.cc (class environment_setter): New internal class.
(get_canonical_types_map): Remove. This now becomes a member
function of the environment type.
(class usage_watchdog): Remove.
(usage_watchdog_{s,w}ptr): Remove these typedefs.
(get_usage_watchdog_wptr, ref_usage_watchdog)
(maybe_cleanup_type_system_data): Remove these functions.
(translation_unit::priv::usage_watchdog_): Remove data member.
(translation_unit::priv::env_): New data member.
(translation_unit::priv::priv): Take an environment and initialize
the new env_ data member. Do not initialize the removed
usage_watchdog_.
(translation_unit::translation_unit): Take an environment
parameter.
(translation_unit::get_global_scope): Set the environment of a new
global scope.
(translation_unit::{g,s}et_environment): New accessors.
(translation_unit::bind_function_type_life_time): Set the
environment of the function type.
(struct environment::priv): New class.
(environment::{environment, ~environment, get_canonical_types_map,
get_variadic_parameter_type_decl, canonicalization_is_done}): New
member functions.
(struct type_or_decl_base::priv): New class.
(type_or_decl_base::{type_or_decl_base, hashing_started,
get_cached_hash_value, set_cached_hash_value, set_environment,
get_environment, traverse}): New member functions.
({s,g}get_environment_for_artifact): New functions.
(decl_base::priv::{hash_, hashing_started}): Remove.
(decl_base::priv::priv): Adjust.
(decl_base::decl_base): In the copy constructor, initialize the
virtual base type_or_decl_base. Do not initialize hash_ and
hashing_started data member that got removed.
(decl_base::{hashing_started, peek_hash_value, set_hash}): Remove
member functions.
(strip_typedef): Set the environment of the new type which has its
typedefs stripped off. Adjust the call to type_or_void().
(scope_decl::{add, insert}_member_decl): Set the environment of
the new member decl to the environment of its scope.
(synthesize_type_from_translation_unit)
(synthesize_function_type_from_translation_unit): Set the
environment for the newly synthesized type. Adjust calls to
type_or_void().
(type_or_void): Take an environment in parameter. Get the void
type from the environment.
(get_canonical_types_map): Remove.
(type_base::get_canonical_type_for): Get the canonical types map
from the environment, not from a global variable.
(type_decl::{get_void_type_decl,
get_variadic_parameter_type_decl}): Remove.
(pointer_type_def::pointer_type_def): Adjust call to type_or_void.
(reference_type_def::reference_type_def): Likewise.
(function_decl::parameter::get_pretty_representation): Get the
variadic parameter type decl from the environment.
(class_decl::priv::classes_being_compared_): Remove static data
member.
(class_decl::priv::{mark_as_being_compared,
unmark_as_being_compared, comparison_started): Use the "classes
being compared" map from the environment.
(class_decl::base_spec::get_hash): Adjust.
(keep_type_alive): Get the alive types array from the environment)
not from a global variable anymore.
(get_next_string): Put the counter in thread-local storage.
* src/abg-hash.cc (scope_decl:#️⃣:operator())
(function_decl:#️⃣:operator()): Do not handle caching (here).
* include/abg-corpus.h (corpus::{g,s}et_environment): Declare new
accessors.
* src/abg-corpus.cc (corpus::priv::env): New data member.
(corpus::priv::priv): Initialize it.
(corpus::corpus): Take an environment in parameter.
(corpus::{g,s}et_environment): Define new member functions
(corpus::add): Set the environment of the newly added translation
unit, if it's not set already set. In any case, assert that the
translation unit must use the same environment as the corpus.
* include/abg-dwarf-reader.h (create_read_context)
(read_corpus_from_elf): Take an environment parameter.
({s,g}et_debug_info_root_path, {s,g}et_environment): Declare new
functions.
* src/abg-dwarf-reader.cc (read_context::{env_,
offline_callbacks_}): New data members.
(read_context::read_context): Initialize them.
(read_context::clear_per_translation_unit_data): Do not touch the
void type declaration, it doesn't belong to the translation unit.
(read_context::{env, offline_callbacks}): New accessors.
(read_context::{create_default_dwfl}): New member function.
(read_context::dwfl_handle): Add a setter overload.
({s,g}et_debug_info_root_path): Define new accessors.
(create_default_dwfl, create_dwfl_sptr, create_default_dwfl_sptr):
Remove these.
(build_translation_unit_and_add_to_ir): Adjust to pass the
environment to the newly created translation unit.
(build_function_decl): Adjust to pass the environment to the
created function and parameter types. Get variadic parameter type
node from the current environment, not from a global variable.
And do not try to canonicalize function types here.
(read_debug_info_into_corpus): Set the environment of the newly
created corpus.
(build_ir_node_for_void_type): Get the void type node from the
current environment, rather than from a global variable.
(create_read_context): Take the environment in parameter.
Create the default dwarf front end library handle using the new
member function of the read context. Set the current environment
used by the reader.
(read_corpus_from_elf): Take an environment in
parameter. Overhaul. This is now simpler.
(has_alt_debug_info): Adjust the call to create_read_context() to
make it pass an empty environment.
* include/abg-fwd.h (class environment): Forward declare.
* include/abg-reader.h (read_translation_unit_from_file)
(read_translation_unit_from_buffer)
(read_translation_unit_from_istream)
(read_corpus_from_native_xml): Take an environment in parameter.
* src/abg-reader.cc (read_context::m_env): New data member.
(read_context::read_context): Initialize it.
(read_context::{get_environment, set_environment}): New data
member.
(read_translation_unit): Set environment of the new translation
unit.
(read_corpus_from_input): Set the environment of the new corpus.
(read_translation_unit_from_file)
(read_translation_unit_from_buffer)
(read_translation_unit_from_istream, read_corpus_from_native_xml):
Take an environment in parameter.
(build_function_parameter): Get variadic parameter type from the environment.
* src/abg-comparison.cc (compute_diff): Add asserts in all the
overloads to ensure that the artifact being compared come from the
same environment.
* tests/print-diff-tree.cc (main): Create an env for the ABI
artifacts to use.
* tests/test-abidiff.cc (main): Likewise.
* 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.
* tools/abipkgdiff.cc (main): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
In the DWARF reader, we strip the const qualifier when it applies to
reference types because a reference is always const. Those redundant
const qualifiers can later introduce spurious changes in type
comparison.
But then we were forgetting to add the stripped type to the IR, in
some cases. This patch fixes that.
* include/abg-ir.h (operator&, operator~): Add overloaded bitwise
operators for qualified_type_def::CV.
* src/abg-ir.cc (operator&, operator~): Define them.
* src/abg-dwarf-reader.cc (maybe_strip_qualification): Fix
comment. If there are multiple qualifiers, only strip the const
one.
(build_ir_node_from_die): Once we've built a qualified type, if
the 'const' qualifier is stripped, then add the new (stripped)
type to the set of new types.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-hash.cc (class_decl:#️⃣:operator()): Remove some dead
code.
* src/abg-ir.cc (equals): In the overload for class_decl,
re-indent.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
runtestreaddwarf and runtestcanonicalizetypes.sh are now the tests
which takes the most time. Run them at the beginning.
* tests/Makefile.am: Run runtestreaddwarf and
runtestcanonicalizetypes at the beginning.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-comparison.cc (reference_diff::has_changes): Just
compare the references, rather than assuming that the change can
only be on underlying types.
(reference_diff::report): Describe lvalue/rvalue changes for
references.
* src/abg-ir.cc (reference_type_def::reference_type_def): Properly
set the name for an rvalue reference.
(equals): For references, compare lvalue-ness too.
(reference_type_def::get_qualified_name): Properly set rvalue
reference names.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-liba.so:
New test input.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-libb.so:
New test input.
* tests/data/test-diff-filter/test30-pr18904-rvalueref-report0.txt:
New test reference output.
* tests/data/Makefile.am: Add the new files to source
distribution.
* tests/test-diff-filter.cc (in_out_specs): Run the new tests.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
The series of fixes to make
"abidw foo > foo.abi && abidiff foo foo.abi"
work continues.
On a binary submitted as part of bug 18904, I am still seeing type
degradation. This patch addresses the different cases of degradation
that are happening.
* include/abg-fwd.h (get_type_scope): Declare new function.
* src/abg-hash.cc (var_decl:#️⃣:operator()): Do not cache the
hash because that can alter the hash computing of a larger type
which embeds a var decl as a member declaration. This is
especially true if the var decl indirectly references the larger
type. The only way to cache the value of a var decl would be to
wait after all canonical types have been computed. We'd then seal
all types. After that sealing happens, we can cache var decls
starting from the top-level ones.
(function_decl:#️⃣:operator()): Likewise.
* src/abg-ir.cc (get_type_scope): Define new functions.
* src/abg-reader.cc (read_is_declaration_only): Declare this
function earlier.
(typedef const_types_map_it): Adjust this to make it point to a
map of string and vector of types, as opposed to a map to string
and type as it was before.
(typedef types_map_it): New typedef.
(read_context::map_id_and_node): Map a type id to the last
xmlNodePtr that represent a *declaration*. That gives more leeway
to the declaration resolution code to choose the right definition
later. Otherwise, there are cases where the wrong definition. By
wrong definition, I mean a definition that is different from the
one chosen by the DWARF reading code, for a given declaration.
Basically for a given ABI corpus, a type declaration resolve to
the first definition seen in the corpus.
(read_context::get_all_type_decls): Define new member function.
(read_context::types_equal): Use qualified names only if both
types have a scope.
(read_context::key_type_decl): Now a given ID is associated to
*all* the declarations and definition that have that ID.
(read_translation_unit_from_input): Make sure the current corpus
node points to the right node.
(build_class_decl): Resolve class declarations to the first
definition seen in the corpus. Key a type decl before reading its
members as a reading a member can request the current decl. No
need to try and canonicalize a member type, as build_class_decl()
does that already.
* tests/data/test-read-dwarf/test16-pr18904.so: New test binary
input.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: New test
output reference.
* tests/test-read-dwarf.cc: Run the test above.
* tests/data/Makefile.am: Add the new test input to source
distribution.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
abidiff-ing libtsan.so again the output of abidw libtsan.so does not
yield the empty set. This is because some types, especially an enum (in
certain cases) when read (de-serialized) from DWARF doesn't hash the
same as when de-serialized from abixml.
This is because an enum type can have a linkage name, referred to by
the DW_AT_linkage_name DWARF attribute. This linkage_name was being
read from DWARF but wasn't serialized to abixml. At de-serialization
time, well, the linkage_name information was lost. Oops.
Also, I have seen that in some case we can canonicalize enum types too
early, when we de-serialize them from abixml, before we are done
building them.
This patch addresses these issues.
* src/abg-reader.cc (read_context::maybe_canonicalize_type): Late
canonicalize enum types.
(build_enum_type_decl): Read the linkage name of the enum type.
* src/abg-writer.cc (write_enum_type_decl): Emit the linkage name
of the enum type.
* tests/data/test-read-dwarf/test15-pr18892.so: New binary test
input.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: New test
output reference.
* tests/data/Makefile.am: Add the new test inputs above to source
distribution.
* tests/test-read-dwarf.cc (in_out_specs): Run the two tests above.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
abidiff-ing libGLU.so against the result of 'abidw libGLU.so' does not
yield the empty set. This is because hashing certain types when they
are read (de-serialized) from DWARF doesn't give the same result as
when they are de-serialized from abixml. I call this type
degradation. And it leads to spurious comparison differences.
This is due to several issues.
1/ The logical link between a class declaration and its definition
-- that is built when reading types from DWARF is not preserved in
abixml. So, for example, when a class S refers to itself via a
pointer to its declaration, that type might hash differently when
read from DWARF and when read from abixml. When read from abixml
it's a pointer to S itself. But then that 'self' can be a copy of S
that is defined in another file because abixml doesn't enforce the
One Definition Rule from C++ either.
2/ As the result of hashing is kept in a cache for var_decl and
function_decl, hashing those decl before their types are completely
built caches a value that becomes wrong when their type become
completely built.
3/ In DWARF, a class which has a virtual member function can still
be considered as being declaration-only. And its definition can
come later in the DWARF info. Our DWARF reader removes the
"declaration-only" flag from a class as soon as it sees virtual
member functions in that class; that makes us consider that class as
a definition. And then later when we read the real definition of
the class we have two classes of the same name, with different
layouts/size in the system. This leads to spurious comparison
differences too.
This patch addresses issues 1, 2 and 3.
* src/abg-dwarf-reader.cc (build_class_type_and_add_to_ir): Do not
consider that virtual member functions disqualify a class from
being declaration-only.
* src/abg-hash.cc (var_decl:#️⃣:operator()): Do not cache the
result of hashing before we are done building the type of the
var_decl.
(function_decl:#️⃣:operator()): Likewise, do not cache the
result of hashing before we are done building the type of the
function_decl.
* src/abg-reader.cc (build_class_decl): Build the link between a
class declaration and its definition. If there are several
definitions of a class in the corpus, keep just one.
* src/abg-writer.cc (write_class_is_declaration_only): Emit the
link between a class declaration and its definition.
(write_class_decl): Emit a class declaration even if it has a
definition. The definition is going to be emitted
separately.
* tests/data/test-read-dwarf/test14-pr18893.so: New binary test
input.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: New test
reference output.
* tests/data/Makefile.am: Add the new test input files to source
distribution.
* tests/test-read-dwarf.cc (in_out_specs): Run the new tests.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When building a class type from DWARF, we were locally trying to
figure out if we should early canonicalize the resulting class type or
not. We should rather use the common code that knows how to decide
that. And this is what this patch does.
* src/abg-dwarf-reader.cc (build_ir_node_from_die):
(maybe_canonicalize_type): Move the specific logic that was in
build_ir_node_from_die (for class types) here.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-tools-utils.cc (string_ends_with): Handle the case where
the string suffix is longer than the string itself.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It turns out that using a size_t to serialize an enumerator is not
enough to represent things like enum foo {value = -3}; We need to
represent it using ssize_t.
Also, the patch avoids early canonicalization (when reading DWARF) of
types that refer to themselves. This was leading to type degradation
(serializing the type from IR to abixml and de-serializing it back to
IR leads to a different type).
* include/abg-ir.h (enum_type_decl::enumerator::get_value()):
Change the type of this from size_t to ssize_t.
* src/abg-ir.cc (enum_type_decl::enumerator::get_value): Do the
same on the definition side.
(non_canonicalized_subtype_detector::visit_begin): If a type
refers to itself, late canonicalize it to have a similar hashing
result as what the abixml reader does.
* src/abg-reader.cc (build_enum_type_decl): Use ssize_t to read
the value of enumerators.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: New test input.
* tests/data/Makefile.am: Add the new test inputs above to source
distribution.
* tests/test-read-dwarf.cc (in_out_specs): Add new test inputs.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This patch adds vtable changes detection based on the detection of
virtual member function changes. That is, when a member function
changes, if that member function is virtual, then infer if the change
implies changes to the vtable of the containing class.
Before that patch, we were doing the vtable change detection when we
were comparing two classes; we were then comparing their virtual
member functions. But as for a given class all its virtual member
functions are not necessarily emitted in the DWARF debug info (only
the virtual member functions that are used in a given translation unit
are emitted in that translation unit) it's not reliable to compare
virtual member functions as part of comparing a given class. We thus
decided some patches ago to stop comparing virtual member functions
when we compare two classes.
So with this patch now, we still detect changes to the vtable and emit
an appropriate message to the user.
* include/abg-ir.h (class_decl::{has_virtual_base, has_vtable}):
Declare new member functions.
* src/abg-comp-filter.cc (has_virtual_mem_fn_change): New overload
for function_decl_diff.
(has_virtual_mem_fn_change): In the overload for diff*, support
virtual member function changes detection for function_decl_diff*.
* src/abg-comparison.cc (function_decl_diff::report): Detect and
report changes to a vtable by looking a changes that can happen to
a given member function.
(corpus_diff::report): Detect and report changes to vtables by
looking at changes change to member functions.
* tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt:
New text input.
* tests/data/test-diff-dwarf/test29-vtable-changes-v{0,1}.cc: Source
code of new test input binaries.
* tests/data/test-diff-dwarf/test29-vtable-changes-v{0,1}.o: New
test input binaries.
* tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt:
New text input.
* tests/data/test-diff-dwarf/test30-vtable-changes-v{0,1}.cc: New
test input.
* tests/data/test-diff-dwarf/test30-vtable-changes-v{0,1}.o: New
test input binaries.
* tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt:
New test input.
* tests/data/test-diff-dwarf/test31-vtable-changes-v{0,1}.cc:
Source code of new test input binary.
* tests/data/test-diff-dwarf/test31-vtable-changes-v{0,1}.o: New
test input binary.
* tests/data/Makefile.am: Add the new test input files above to
source distribution.
* tests/test-diff-dwarf.cc (in_out_specs): Consume the new test
inputs above.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
When comparing two classes, do not compare their virtual member
functions anymore, because DWARF might not represent all the virtual
member functions of a class, in a given translation unit.
We still detect changes to virtual member functions (adding or
removing) because the index of a given member function in a vtable is
a property of the member function itself. So if a vtable index
changes on a function, we detect it as part of comparing the exported
member functions themselves. Likewise, if a member function is added
or removed, we detect it; and so if it's a virtual member function
then we detect it too. In a subsequent patch, we'll add a dedicated
section to the report emitted by abidiff for changes to the vtable of
classes, I guess.
For now, this patch fixes some crashes we were having due to
discrepancies in hash values of classes, due to the fact that not all
of their virtual member functions were present in the debug info,
depending on the translation unit of the classes in question.
* src/abg-ir.cc (equals): When comparing two classes, do not
compare their virtual member functions.
* src/abg-hash.cc (class_decl:#️⃣:operator()): Do not hash
virtual member functions when hashing a class.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
So the last series of patches have changed the test output a lot.
This patch adjusts the reference output to have "make check" work
again. There is still one test that fails:
./build/tests/runtestreaddwarf. It'll be addressed in subsequent
patches from now.
* tests/data/test-abidiff/test-enum0-report.txt: Adjust.
* tests/data/test-abidiff/test-enum1-report.txt: Adjust.
* tests/data/test-abidiff/test-qual-type0-report.txt: Adjust.
* tests/data/test-abidiff/test-struct0-report.txt: Adjust.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Adjust.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust.
* tests/data/test-read-write/test17.xml: Adjust.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
use abidiff to compare the ABI of the input ABI against the result of
writing that input back into an XML format. It should yield the empty
set. I wonder why I haven't done this since the beginning. It turned
out we had to fix many things to make it work now. Just using "GNU
diff" to compare the output against a reference output is definitely
not enough.
* tests/test-read-dwarf.cc (main): Use abidiff to compare the
input elf file with the XML emitted. That should yield the empty
set.
* tests/test-read-write.cc (main): Likewise, use abidiff to
compare the input abixml file with the one that is emitted.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* tools/abidw.cc (options::noout): New data member.
(options::options): Initialize it.
(display_usage): Add a usage string for the new option.
(parse_command_line): Parse the new option.
(main): If --noout is provided, do not emit the XML form.
* doc/manuals/abidw.rst: Document the new option.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This new option omits architectures when comparing ABIs.
* tools/abidiff.cc (options::no_arch): New data member.
(options::options): Initialize it.
(display_usage): Display a help string for the new options.
(parse_command_line): Parse the new options.
(main): If --no-architecture is provided, set the corpus
architecture to "".
* doc/manuals/abidiff.rst: Document the new options.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Forcing each TU to be read in a self contained way was a mistake
because it introduces differences with how DWARF is represented. In
DWARF, types need to be reconciled at the DSO level. I.e, some types
that are only declared in some TUs are to be defined later in other
TUs. So abixml needs to reflect that, otherwise, some types read from
abixml might wrongly appear to be different from the same type read
from DWARF.
But then we need to be able to use a type (refer to its type id)
before defining it later. That means, we need to read the abixml file
in, as a whole. Rather than walking it with a cursor like we used to
do.
This patch does that. That implies to be able to build (on-demand)
an entire translation unit, just because we refer to a type that is
inside that TU.
The patch also fixes some ancillary issues that are related or
uncovered by this "whole-corpus" way of seeing things; these issues were
causing type hashing differences with what the DWARF reader does.
* src/abg-reader.cc (class read_context): Move data member at
the top of the class like what is done elsewhere in the code
base.
(read_context::m_corp_node): New data member.
(read_context::read_context): Initialize it.
(read_context::{get,set}_corpus_node): New accessors.
(read_context::map_id_and_node): Accept that a node id previously
defined is defined again. In that case we just remember the first
mapping id -> xml-node. That seems to work for now.
(read_context::get_translation_unit): Fix the logic.
(read_context::m_wip_types_map): Rename
read_context::m_wip_classes_map into this.
(read_context::clear_wip_types_map): Rename
read_context::clear_wip_classes into this.
(read_context::mark_type_as_wip): Rename
read_context::mark_class_as_wip into this.
(read_context::unmark_type_as_wip): Rename
read_context::unmark_type_as_wip into this.
(read_context::is_wip_type): Rename read_context::is_wip_class
into this.
(read_context::types_equal): New member function.
(read_context::clear_per_translation_unit_data): Do not clear
anything anymore as the previous data that were per-tu are now
per-corpus.
(read_context::clear_per_corpus_data): Clear here the previous
data that were per-tu.
(read_context::maybe_canonicalize_type): Add a new force_delay
flag that forces the type to be late-canonicalized. Also force
late-canonicalize references, pointers, qualified-type and typedef
because they must be canonicalized once they've been added to
their context; but then this function might be called too early,
before they are added to their context.
(read_context::type_id_new_in_translation_unit): Remove this
member function.
(read_translation_unit_from_input): Be able to either use the
xmlTextReader interface, or get the current 'abi-instr' xml
element node. If using the xmlTextReader interface, use it to
move to the 'abi-instr' node, expand it and then use that. In
either case, call read_translation_unit() with the 'abi-instr' xml
element node.
(read_translation_unit): Take an 'abi-instr' XML element in
argument now, use that to read the translation unit, as opposed to
using the xmlTextReader interface we where using before to walk
the sub-tree of the abi-instr xml node.
(read_context::get_scope_for_node): If the scope is a new
translation unit, then build the new translation unit.
(read_symbol_db_from_input): Take the function and variable symbol
data bases, and read the current xml element node (do not use the
xmlTextReader interface anymore) to populate the function and
variable symbols.
(read_elf_needed_from_input): Do not use the xmlTextReader
interface anymore. Rather, use the current xml element node, look
for the 'elf-needed' xml element node and use it to populate the
set of elf dependencies.
(read_corpus_from_input): Rework to expand the contents of the
corpus node and use the result, rather than just exclusively
relying on the xmlTextReader interface.
(build_function_parameter): Build a proper IR node for variadic
parameters. Build function type node *after* having built all the
parameters IR, so that parameter indexing is the same as what is
done in the DWARF reader. Also, if the function is not being
added to its context yet, then delay the canonicalizing of its
type, just like what is done by the DWARF reader.
(build_qualified_type_decl, build_pointer_type_def)
(build_reference_type_def, build_enum_type_decl, build_type_decl):
Adjust. Do not enforce anymore that the ID of this type be new in
the current TU. Delay canonicalizing if the type is not being
added to its context. For typedefs, use an adapted way of
checking the consistency of the underlying type.
(build_array_type_def): Do not enforce anymore that the ID of this
type be new in the current TU. Support the fact that the array
might not have any DW_AT_byte_size attribute. Force late
canonicalizing if the array is not being added to its context.
(build_class_decl): Adjust. Reuse the
read_context::maybe_canonicalize_type() function rather than
trying to determine locally when to canonicalize.
(build_template_tparameter): Adjust
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
It appears now that forcing unresolved class declarations to be
declared is not a good idea. It's better to just leave them as is,
and they'll have a hash value of zero. We were forcing them to be
defined (with a size of 1) because they were used as base classes. It
appears that GCC and Clang (at least) allow base classes to be
non-complete, in case the base class has a vtable; in that case, the
full debug info of the base class would be emitted in another DSO,
where the vtable is emitted, making the base class be complete from a
debug info standpoint. So it's better for us to be in par with that
vision.
Furthermore, one of the reasons why they were not resolved, most of
the time, was that the resolution code was buggy; and that has been
fixed in a patch applied very recently.
So this patch removes the forcing code.
The patch also fixes the handling of class declaration during the
parsing. Basically, bugs in some versions of Clang are so that we
cannot completely trust the DW_AT_declaration property on a class.
What we do is that when we see that property, we flag the class as
being a declaration. But then if there is a DW_AT_byte_size property,
the class is considered as being defined. We were being over-zealous
in considering the class as being defined, because having a member
function was enough; this patch now only considers the presence of a
*virtual* member functions, data members, base classes or a
DW_AT_byte_size as being conditions for being defined.
* src/abg-dwarf-reader.cc (read_context::decl_only_classes_map_):
Remove this data member.
(read_context::{declaration_only_classes_to_force_defined,
schedule_declaration_only_class_for_forced_resolution}): Remove
these member functions.
(read_context::resolve_declaration_only_classes): Do not force
resolution of class declaration.
(build_class_type_and_add_to_ir): Do not schedule classes for
forced-resolution when they are used as base classes. The
presence of a member function is not enough to make the class be
defined. It needs to be a virtual member function.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-hash.cc (class_decl:#️⃣:operator()): Do not force
base classes to have definitions anymore. GCC and Clang (at
least) some time emits debug info in which the definition of some
base classes are missing, especially when those base classes have
vtables. In that case, the definition of the class might it's in
the binary where the vtable is emitted, which might not be the
binary we are looking at. So let's relax the assertion we had
here for base classes. For hashing virtual member functions,
directly walk the virtual member functions by looking at
class_decl::get_virtual_mem_fns() rather than walking all
member functions and looking for the virtual ones. This is a
speed optimization but it also helps during debugging.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
There are two issues in comparing templates currently.
One is that comparing member class template recurses for ever (oops).
The other is that the logic of comparing function templates is wrong
and leads to false comparisons.
* include/abg-ir.h (function_tdecl::operator==): Introduce a new
virtual member operator that takes a function_tdecl&.
* src/abg-ir.cc
(class_decl::member_function_template::operator==): Avoid the
static cast in the overload for member_base. In the overload for
member_class_template, avoid infinite recursion.
(function_tdecl::operator==): In the overload for decl_base, do
not do the real work here in the overload for decl_base Rather,
the real work is done in the new overload for function_tdecl, and
all other overloads call that one.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Sometimes during hashing the "type sub-object" of a class can be
queried for its size or alignment. In those case, if the class is a
declaration that happens to be accompanied with a definition, its the
size/alignment of the definition that we want, not the one of the
declaration, that is zero. Otherwise, this can cause spurious hashing
changes between two class types that are otherwise equivalent modulo
the use of a class declaration.
This patch being part of a series that aims at fixing a number of type
hashing issues, the regression tests are adjusted at the end of the
series, not here.
* include/abg-ir.h (type_base::{set_size_in_bits,
set_alignment_in_bits}): Make these member functions virtual.
(class_decl::{set_size_in_bits, get_size_in_bits,
get_alignment_in_bits, set_alignment_in_bits}): Declare these
virtual member functions.
* src/abg-ir.cc (class_decl::{set_size_in_bits, get_size_in_bits,
get_alignment_in_bits, set_alignment_in_bits}): Define these
virtual functions.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Until now, the type lookup algorithm was broken for c++. For two
reasons:
1/ The algorithm to break a fully qualified type name into name
components is buggy. When given the type name:
foo<ns1::t1, ns1::t2>::t3
the components making up the name are: "foo<ns1", "t1, ns1", "t2>"
and "t3. That is wrong. The components should be:
"foo<ns1::t1, ns2::t2>" and "t3".
2/ When a type is found, if it's a declaration, it's skipped. This
is wrong because if the declaration is accompanied with a
definition, it should be returned.
This patch addresses the two issues above. It allows more
declaration-only classes to be resolved and so reduces the number of
spurious hashing differences between two instances of the same type
which should otherwise have the same hash.
There is no regression test update with this patch because we really
need the full series this patch is part of, to fix the type hashing
correctness issues we have. So the regression test updates are coming
at the end of the series.
* src/abg-ir.cc (find_next_delim_in_cplus_type): Define new static
function.
(fqn_to_components): Use the new function above to break up a
fully qualified name into components, rather than the too simple
string::find_first_of() we were using previously.
(lookup_node_in_scope): If the found type (class) is a
declaration-only and if it has a definition, then return it.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
* src/abg-hash.cc (decl_base:#️⃣:operator()(const decl_base&)):
Always hash the qualified name of the decl.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Until now, a base class had to be a class itself. It couldn't be a
typedef to a class, for instance. Clang's debug info does allow base
classes which are compatible with classes (e.g, typedefs of classes),
which is correct. We ought to accept that. Hence this patch.
* include/abg-fwd.h (is_compatible_with_class_type): Declare a new
overload.
* src/abg-dwarf-reader.cc (build_class_type_and_add_to_ir): Rather
than requiring that base classes be of class type, just require
that they be compatible with class types.
* src/abg-ir.cc (is_compatible_with_class_type): Define a new
overload.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This function can abort when called on a function_decl that is not a
member function. This patch addresses that issue.
* src/abg-ir.cc (function_decl::get_pretty_representation): Make
sure the function type is a member function before calling
get_member_function_is_{virtual,ctor,dtor,const}.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Caching the qualified name of a given type has always been subject to
subtle bugs. If the qualified name is queried (so it's computed)
before the type is added into its final content, then what is cached
is a non-qualified type name. Later when the type is finally added to
its context, querying its qualified name will just yield the cached
non-qualified name. And that has impact on hashing and comparison.
We needed a way to signal that the type is "fully built and added to
its final context". When the type is fully built then we can cache
its qualified name.
This patch uses the presence of the canonical type as the signal; if
the canonical type is present then the type is fully built and added
to its final context. And then at that point the cached qualified
name is used.
Note that this patch is the first of a series fixing several things
that influence hashing, comparison, the reading and writing of abixml.
It's only at the end of the series that an update to regression tests
is provided. In between, some patches of the series are going to
"break" the regression tests. That is fine.
* src/abg-ir.cc (decl_base::{get_qualified_parent_name,
get_qualified_name}): Use the qualified name cache only if the
type is fully built, i.e, when its canonical type is present.
(qualified_type_def::get_qualified_name): Likewise.
(pointer_type_def::get_qualified_name): Likewise.
(reference_type_def::get_qualified_name): Likewise.
(array_type_def::get_qualified_name): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
