From: Dodji Seketeli Date: Fri, 19 Jul 2019 15:45:27 +0000 (+0200) Subject: Bug 24787 - Filter out enum changes into compatible integer types X-Git-Tag: upstream/1.7~62 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=f2437aabad085e9f9c99d28e94ac0f046e224763;p=platform%2Fupstream%2Flibabigail.git Bug 24787 - Filter out enum changes into compatible integer types Libabigail's filtering engine fails to recognize an enum changing into a compatible integer (or vice versa) as a harmless change. This patch fixes that. * include/abg-comparison.h (peel_typedef_or_qualified_type_diff): Declare new function. (peel_pointer_or_qualified_type_diff): Rename peel_pointer_or_qualified_type into this. * include/abg-fwd.h (is_enum_type): Declare a new overload for type_or_decl_base*. * src/abg-comp-filter.cc (has_harmless_enum_to_int_change): Define new static function. * src/abg-comparison.cc (categorize_harmless_diff_node): Use the new has_harmless_enum_to_int_change here. (peel_pointer_or_qualified_type_diff): Renamed peel_pointer_or_qualified_type into this. (is_diff_of_basic_type): Adjust. (peel_typedef_or_qualified_type_diff): Define new function. * test-diff-filter/PR24787-lib{one, two}.so: New test input binaries. * test-diff-filter/PR24787-{one, two}.c: Source files of the test input binaries above. * test-diff-filter/PR24787-report-0.txt: Test output reference. * tests/data/Makefile.am: Add the new testing material to source distribution. * tests/test-diff-filter.cc (in_out_specs): Add the new test to the test harness. Signed-off-by: Dodji Seketeli --- diff --git a/include/abg-comparison.h b/include/abg-comparison.h index 66392b97..8537a435 100644 --- a/include/abg-comparison.h +++ b/include/abg-comparison.h @@ -2799,7 +2799,10 @@ const diff* peel_qualified_diff(const diff* dif); const diff* -peel_pointer_or_qualified_type(const diff*dif); +peel_pointer_or_qualified_type_diff(const diff* dif); + +const diff* +peel_typedef_or_qualified_type_diff(const diff* dif); }// end namespace comparison }// end namespace abigail diff --git a/include/abg-fwd.h b/include/abg-fwd.h index 3a717436..ec37af62 100644 --- a/include/abg-fwd.h +++ b/include/abg-fwd.h @@ -423,6 +423,9 @@ is_typedef(type_base*); enum_type_decl_sptr is_enum_type(const type_or_decl_base_sptr&); +const enum_type_decl* +is_enum_type(const type_or_decl_base*); + bool is_class_type(const type_or_decl_base&); diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc index 5362b536..d412bca7 100644 --- a/src/abg-comp-filter.cc +++ b/src/abg-comp-filter.cc @@ -1035,6 +1035,52 @@ has_harmful_enum_change(const diff* diff) return false; } +/// Test if a diff node carries a harmless change of an enum into an +/// integer (or vice-versa). +/// +/// The test takes into account the fact change we care about might be +/// wrapped into a typedef or qualified type diff. +/// +/// @param diff the diff node to consider. +/// +/// @return true if @p diff is a harmless enum to integer change. +static bool +has_harmless_enum_to_int_change(const diff* diff) +{ + if (!diff) + return false; + + diff = peel_typedef_or_qualified_type_diff(diff); + + if (const distinct_diff *d = is_distinct_diff(diff)) + { + const enum_type_decl *enum_type = 0; + const type_base *integer_type = 0; + + type_base *first_type = + peel_qualified_or_typedef_type(is_type(d->first().get())); + type_base *second_type = + peel_qualified_or_typedef_type(is_type(d->second().get())); + + if (const enum_type_decl *e = is_enum_type(first_type)) + enum_type = e; + else if (const enum_type_decl *e = is_enum_type(second_type)) + enum_type = e; + + if (const type_base * i = is_type_decl(first_type)) + integer_type = i; + else if (const type_base *i = is_type_decl(second_type)) + integer_type = i; + + if (enum_type + && integer_type + && enum_type->get_size_in_bits() == integer_type->get_size_in_bits()) + return true; + } + + return false; +} + /// Test if an @ref fn_parm_diff node has a top cv qualifier change on /// the type of the function parameter. /// @@ -1385,8 +1431,9 @@ categorize_harmless_diff_node(diff *d, bool pre) || static_data_member_type_size_changed(f, s)) category |= STATIC_DATA_MEMBER_CHANGE_CATEGORY; - if (has_enumerator_insertion(d) - && !has_harmful_enum_change(d)) + if ((has_enumerator_insertion(d) + && !has_harmful_enum_change(d)) + || has_harmless_enum_to_int_change(d)) category |= HARMLESS_ENUM_CHANGE_CATEGORY; if (function_name_changed_but_not_symbol(d)) diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc index feae5f96..55033b09 100644 --- a/src/abg-comparison.cc +++ b/src/abg-comparison.cc @@ -11551,7 +11551,7 @@ const type_decl_diff* is_diff_of_basic_type(const diff* diff, bool allow_indirect_type) { if (allow_indirect_type) - diff = peel_pointer_or_qualified_type(diff); + diff = peel_pointer_or_qualified_type_diff(diff); return is_diff_of_basic_type(diff); } @@ -11646,7 +11646,7 @@ peel_qualified_diff(const diff* dif) /// @return the underlying diff node of @p dif, or just return @p dif /// if it's not a pointer, reference or qualified diff node. const diff* -peel_pointer_or_qualified_type(const diff*dif) +peel_pointer_or_qualified_type_diff(const diff*dif) { while (true) { @@ -11662,6 +11662,33 @@ peel_pointer_or_qualified_type(const diff*dif) return dif; } +/// If a diff node is about changes between two typedefs or qualified +/// types, get the diff node about changes between the underlying +/// types. +/// +/// Note that this function walks the tree of underlying diff nodes +/// returns the first diff node about types that are not typedef or +/// qualified types. +/// +/// @param dif the dif node to consider. +/// +/// @return the underlying diff node of @p dif, or just return @p dif +/// if it's not typedef or qualified diff node. +const diff* +peel_typedef_or_qualified_type_diff(const diff *dif) +{ + while (true) + { + if (const typedef_diff *d = is_typedef_diff(dif)) + dif = peel_typedef_diff(d); + else if (const qualified_type_diff *d = is_qualified_type_diff(dif)) + dif = peel_qualified_diff(d); + else + break; + } + return dif; +} + /// Test if a diff node represents a diff between two class or union /// types. /// diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 982d32e5..3474b2eb 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -704,6 +704,11 @@ test-diff-filter/test-PR24731-v0.c \ test-diff-filter/test-PR24731-v0.o \ test-diff-filter/test-PR24731-v1.c \ test-diff-filter/test-PR24731-v1.o \ +test-diff-filter/PR24787-libone.so \ +test-diff-filter/PR24787-libtwo.so \ +test-diff-filter/PR24787-one.c \ +test-diff-filter/PR24787-two.c \ +test-diff-filter/PR24787-report-0.txt \ \ test-diff-suppr/test0-type-suppr-v0.cc \ test-diff-suppr/test0-type-suppr-v1.cc \ diff --git a/tests/data/test-diff-filter/PR24787-libone.so b/tests/data/test-diff-filter/PR24787-libone.so new file mode 100755 index 00000000..d86352be Binary files /dev/null and b/tests/data/test-diff-filter/PR24787-libone.so differ diff --git a/tests/data/test-diff-filter/PR24787-libtwo.so b/tests/data/test-diff-filter/PR24787-libtwo.so new file mode 100755 index 00000000..c18d26a2 Binary files /dev/null and b/tests/data/test-diff-filter/PR24787-libtwo.so differ diff --git a/tests/data/test-diff-filter/PR24787-one.c b/tests/data/test-diff-filter/PR24787-one.c new file mode 100644 index 00000000..b37712d6 --- /dev/null +++ b/tests/data/test-diff-filter/PR24787-one.c @@ -0,0 +1,13 @@ +#include + + +typedef enum FooFilter { + FOO_FILTER_ONE = 1 << 0, + FOO_FILTER_TWO = 1 << 1, + FOO_FILTER_THREE = 1 << 2, +} FooFilter; + + +void do_something(FooFilter filter_flags) { + printf("%d\n", filter_flags); +} diff --git a/tests/data/test-diff-filter/PR24787-report-0.txt b/tests/data/test-diff-filter/PR24787-report-0.txt new file mode 100644 index 00000000..9666a8fd --- /dev/null +++ b/tests/data/test-diff-filter/PR24787-report-0.txt @@ -0,0 +1,3 @@ +Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function +Variables changes summary: 0 Removed, 0 Changed, 0 Added variable + diff --git a/tests/data/test-diff-filter/PR24787-two.c b/tests/data/test-diff-filter/PR24787-two.c new file mode 100644 index 00000000..4308181e --- /dev/null +++ b/tests/data/test-diff-filter/PR24787-two.c @@ -0,0 +1,13 @@ +#include + + +typedef enum FooFilter { + FOO_FILTER_ONE = 1 << 0, + FOO_FILTER_TWO = 1 << 1, + FOO_FILTER_THREE = 1 << 2, +} FooFilter; + + +void do_something(unsigned int filter_flags) { + printf("%d\n", filter_flags); +} diff --git a/tests/test-diff-filter.cc b/tests/test-diff-filter.cc index 04a1f677..f8879d3c 100644 --- a/tests/test-diff-filter.cc +++ b/tests/test-diff-filter.cc @@ -563,6 +563,13 @@ InOutSpec in_out_specs[] = "data/test-diff-filter/test-PR24731-report-1.txt", "output/test-diff-filter/test-PR24731-report-1.txt", }, + { + "data/test-diff-filter/PR24787-libone.so", + "data/test-diff-filter/PR24787-libtwo.so", + "--no-default-suppression", + "data/test-diff-filter/PR24787-report-0.txt", + "output/test-diff-filter/PR24787-report-0.txt", + }, // This should be the last entry {NULL, NULL, NULL, NULL, NULL} };