1
0
mirror of git://sourceware.org/git/libabigail.git synced 2025-03-19 18:05:42 +00:00

Bug 25128 - Handle decl-only classes that differ only in size

Because DWARF sometimes emit decl-only classes (real one, with no
members) with a size property, and the rest of the time, would emit
the same decl-only class without a size property, comparing the two
might yield some false positives.

This patch handles those beasts when comparing classes.

	* include/abg-comp-filter.h (is_decl_only_class_with_size_change):
	Declare an overload.
	* include/abg-fwd.h (look_through_decl_only_class): Declare an
	overload.
	* src/abg-comp-filter.cc (is_decl_only_class_with_size_change):
	Define an overload that takes class_or_union& type. Re-write the
	previous overload in terms of this new one.
	* src/abg-ir.cc (look_through_decl_only_class): Define a new
	overload that takes a class_or_union&.  Rewrite the previous
	overload in terms of this one.
	(equals): In the overload for class_or_union&, use
	is_decl_only_class_with_size_change to detect cases of decl-only
	classes that differ only by their size attribute and avoid
	comparing them.
	* tests/data/test-annotate/test21-pr19092.so.abi: Adjust.
	* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
	* tests/data/test-diff-filter/test41-report-0.txt: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This commit is contained in:
Dodji Seketeli 2019-10-28 13:23:39 +01:00
parent 1f4dbe3515
commit 847800d497
5 changed files with 76 additions and 16 deletions

View File

@ -53,6 +53,10 @@ has_harmful_name_change(const diff* dif);
bool
has_virtual_mem_fn_change(const function_decl_diff* diff);
bool
is_decl_only_class_with_size_change(const class_or_union& first,
const class_or_union& second);
bool
is_decl_only_class_with_size_change(const class_or_union_sptr& first,
const class_or_union_sptr& second);
@ -63,6 +67,7 @@ is_decl_only_class_with_size_change(const diff *diff);
bool
has_class_decl_only_def_change(const class_or_union_sptr& first,
const class_or_union_sptr& second);
bool
has_class_decl_only_def_change(const diff *diff);

View File

@ -510,6 +510,9 @@ is_method_type(const type_or_decl_base*);
method_type*
is_method_type(type_or_decl_base*);
class_or_union_sptr
look_through_decl_only_class(const class_or_union&);
class_or_union_sptr
look_through_decl_only_class(class_or_union_sptr);

View File

@ -823,6 +823,36 @@ static bool
base_classes_added_or_removed(const diff* diff)
{return base_classes_added_or_removed(dynamic_cast<const class_diff*>(diff));}
/// Test if two classes that are decl-only (have the decl-only flag
/// and carry no data members) but are different just by their size.
///
/// In some weird DWARF representation, it happens that a decl-only
/// class (with no data member) actually carries a non-zero size.
/// That shouldn't happen, but hey, we need to deal with real life.
/// So we need to detect that case first.
///
/// @param first the first class or union to consider.
///
/// @param seconf the second class or union to consider.
///
/// @return true if the two classes are decl-only and differ in their
/// size.
bool
is_decl_only_class_with_size_change(const class_or_union& first,
const class_or_union& second)
{
if (first.get_qualified_name() != second.get_qualified_name())
return false;
if (!first.get_is_declaration_only() || !second.get_is_declaration_only())
return false;
bool f_is_empty = first.get_data_members().empty();
bool s_is_empty = second.get_data_members().empty();
return f_is_empty && s_is_empty;
}
/// Test if two classes that are decl-only (have the decl-only flag
/// and carry no data members) but are different just by their size.
///
@ -849,16 +879,7 @@ is_decl_only_class_with_size_change(const class_or_union_sptr& first,
class_or_union_sptr s =
look_through_decl_only_class(second);
if (f->get_qualified_name() != s->get_qualified_name())
return false;
if (!f->get_is_declaration_only() || !s->get_is_declaration_only())
return false;
bool f_is_empty = f->get_data_members().empty();
bool s_is_empty = s->get_data_members().empty();
return f_is_empty && s_is_empty;
return is_decl_only_class_with_size_change(*f, *s);
}
/// Test if a diff node is for two classes that are decl-only (have

View File

@ -46,6 +46,7 @@ ABG_END_EXPORT_DECLARATIONS
// </headers defining libabigail's API>
#include "abg-tools-utils.h"
#include "abg-comp-filter.h"
#include "abg-ir-priv.h"
namespace
@ -7844,12 +7845,16 @@ is_method_type(type_or_decl_base* t)
/// If a class (or union) is a decl-only class, get its definition.
/// Otherwise, just return the initial class.
///
/// @param klass the class (or union) to consider.
/// @param the_klass the class (or union) to consider.
///
/// @return either the definition of the class, or the class itself.
class_or_union_sptr
look_through_decl_only_class(class_or_union_sptr klass)
look_through_decl_only_class(const class_or_union& the_class)
{
class_or_union_sptr klass;
if (the_class.get_is_declaration_only())
klass = the_class.get_definition_of_declaration();
if (!klass)
return klass;
@ -7862,6 +7867,25 @@ look_through_decl_only_class(class_or_union_sptr klass)
return klass;
}
/// If a class (or union) is a decl-only class, get its definition.
/// Otherwise, just return the initial class.
///
/// @param klass the class (or union) to consider.
///
/// @return either the definition of the class, or the class itself.
class_or_union_sptr
look_through_decl_only_class(class_or_union_sptr klass)
{
if (!klass)
return klass;
class_or_union_sptr result = look_through_decl_only_class(*klass);
if (!result)
result = klass;
return result;
}
/// Tests if a declaration is a variable declaration.
///
/// @param decl the decl to test.
@ -18637,6 +18661,17 @@ equals(const class_or_union& l, const class_or_union& r, change_kind* k)
if (!def1 || !def2)
{
if (!l.get_is_anonymous()
&& !r.get_is_anonymous()
&& l_is_decl_only && r_is_decl_only
&& comparison::filtering::is_decl_only_class_with_size_change(l, r))
// The two decl-only classes differ from their size. A
// true decl-only class should not have a size property to
// begin with. This comes from a DWARF oddity and can
// results in a false positive, so let's not consider that
// change.
return true;
if (l.get_environment()->decl_only_class_equals_definition()
&& !l.get_is_anonymous()
&& !r.get_is_anonymous())

View File

@ -48,10 +48,6 @@ Variable symbols changes summary: 0 Removed, 0 Added variable symbol not referen
class std::tr1::__shared_ptr<abigail::ir::type_base, __gnu_cxx::_Lock_policy::_S_atomic> at shared_ptr.h:539:1
[C]'method bool abigail::xml_writer::write_context::type_ptr_cmp::operator()(const abigail::ir::type_base*, const abigail::ir::type_base*) const' at abg-writer.cc:359:1 has some indirect sub-type changes:
parameter 1 of type 'const abigail::ir::type_base*' has sub-type changes:
in pointed to type 'const abigail::ir::type_base':
in unqualified underlying type 'class abigail::ir::type_base':
type size changed from 0 to 384 (in bits)