From: Dodji Seketeli Date: Mon, 28 Oct 2019 12:23:39 +0000 (+0100) Subject: Bug 25128 - Handle decl-only classes that differ only in size X-Git-Tag: upstream/1.7~38 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=847800d4972b30b2e0a0fdd9d7c668dfcfc6fac2;p=platform%2Fupstream%2Flibabigail.git 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 --- diff --git a/include/abg-comp-filter.h b/include/abg-comp-filter.h index ca693464..752047b0 100644 --- a/include/abg-comp-filter.h +++ b/include/abg-comp-filter.h @@ -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); diff --git a/include/abg-fwd.h b/include/abg-fwd.h index 24cfe947..823f0b96 100644 --- a/include/abg-fwd.h +++ b/include/abg-fwd.h @@ -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); diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc index 3f94e5ef..8ca40d6f 100644 --- a/src/abg-comp-filter.cc +++ b/src/abg-comp-filter.cc @@ -823,6 +823,36 @@ static bool base_classes_added_or_removed(const diff* diff) {return base_classes_added_or_removed(dynamic_cast(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 diff --git a/src/abg-ir.cc b/src/abg-ir.cc index ebe2e3fa..a6ae7ae6 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -46,6 +46,7 @@ ABG_END_EXPORT_DECLARATIONS // #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()) diff --git a/tests/data/test-diff-filter/test41-report-0.txt b/tests/data/test-diff-filter/test41-report-0.txt index edeb24ea..3a87adb5 100644 --- a/tests/data/test-diff-filter/test41-report-0.txt +++ b/tests/data/test-diff-filter/test41-report-0.txt @@ -48,10 +48,6 @@ Variable symbols changes summary: 0 Removed, 0 Added variable symbol not referen class std::tr1::__shared_ptr 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)