From: Dodji Seketeli Date: Fri, 31 Mar 2023 12:03:53 +0000 (+0200) Subject: Fix redundancy filtering of range types X-Git-Tag: upstream/2.3~29 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ddb6abd2d3abd7e7d38a71a7ea78bbd9c272e9f1;p=platform%2Fupstream%2Flibabigail.git Fix redundancy filtering of range types After the support for Ada range types was added, it appeared that redundancy propagation was not being correctly handled for range types, especially when those are sub-types of a function parameter type, like a const range. This patch goes through the various problematic spots and addresses the issues. * src/abg-comparison.cc (redundancy_marking_visitor::visit_end): Propagate redundancy category to function parameter diff nodes if they don't carry any local non-type change. * src/abg-default-reporter.cc (default_reporter::report_underlying_changes_of_qualified_type): Define new member function. (default_reporter::report): In the qualified_type_diff overload, use the new report_underlying_changes_of_qualified_type above. * src/abg-ir.cc (types_have_similar_structure): If two arrays are accessed indirectly and if they have size and dimension changes, then the two arrays are considered having a similar structure. Otherwise, if they are accessed directly, having size or dimension change make them considered as having non similar structure. This has an impact on if a change between two array types is considered local or not. * src/abg-leaf-reporter.cc (leaf_reporter::report): Local changes to underlying types of a qualified type are considered local to the qualified type. This change reflects that in the overload for qualified type diff nodes. Otherwise, we won't report what would otherwise be a leaf change to the a qualified type, just because it's actually a leaf change to the underlying type of the qualified type. * tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-{1,2}.txt: New reference output files. * tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.ad{b,s}: Source code for the new binary input below. * tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o: New binary input file. * tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.ad{b,s}: Source code for the new binary input below. * tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o: New binary input file. * tests/data/Makefile.am: Add the new test input files above to source distribution. * tests/test-abidiff-exit.cc (in_out_specs): Add the new input tests above to this test harness. * tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt: Adjust. Signed-off-by: Dodji Seketeli --- diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc index c4596c8e..59530a8a 100644 --- a/src/abg-comparison.cc +++ b/src/abg-comparison.cc @@ -12749,6 +12749,13 @@ struct redundancy_marking_visitor : public diff_node_visitor || (is_var_diff(d) && (!(d->has_local_changes() & LOCAL_NON_TYPE_CHANGE_KIND))) + // A function parameter with non-type local changes + // should not see redundancy propagation either. But + // a function parameter with local type changes can + // definitely be redundant. + || (is_fn_parm_diff(d) + && (!(d->has_local_changes() + & LOCAL_NON_TYPE_CHANGE_KIND))) )) { bool has_non_redundant_child = false; diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc index 31832c3f..b1df9300 100644 --- a/src/abg-default-reporter.cc +++ b/src/abg-default-reporter.cc @@ -348,6 +348,38 @@ default_reporter::report_local_qualified_type_changes(const qualified_type_diff& return false; } +/// For a @ref qualified_type_diff node, report the changes of its +/// underlying type. +/// +/// @param d the @ref qualified_type_diff node to consider. +/// +/// @param out the output stream to emit the report to. +/// +/// @param indent the white string to use for indentation. +/// +/// @return true iff a local change has been emitted. In this case, +/// the local change is a name change. +void +default_reporter::report_underlying_changes_of_qualified_type +(const qualified_type_diff& d, ostream& out, const string& indent) const +{ + if (!d.to_be_reported()) + return; + + diff_sptr dif = d.leaf_underlying_type_diff(); + ABG_ASSERT(dif); + ABG_ASSERT(dif->to_be_reported()); + RETURN_IF_BEING_REPORTED_OR_WAS_REPORTED_EARLIER2(dif, + "unqualified " + "underlying type"); + + string fltname = dif->first_subject()->get_pretty_representation(); + out << indent << "in unqualified underlying type '" << fltname << "'"; + report_loc_info(dif->second_subject(), *d.context(), out); + out << ":\n"; + dif->report(out, indent + " "); +} + /// Report a @ref qualified_type_diff in a serialized form. /// /// @param d the @ref qualified_type_diff node to consider. @@ -372,18 +404,7 @@ default_reporter::report(const qualified_type_diff& d, ostream& out, // It makes a little sense to detail the changes in extenso here. return; - diff_sptr dif = d.leaf_underlying_type_diff(); - ABG_ASSERT(dif); - ABG_ASSERT(dif->to_be_reported()); - RETURN_IF_BEING_REPORTED_OR_WAS_REPORTED_EARLIER2(dif, - "unqualified " - "underlying type"); - - string fltname = dif->first_subject()->get_pretty_representation(); - out << indent << "in unqualified underlying type '" << fltname << "'"; - report_loc_info(dif->second_subject(), *d.context(), out); - out << ":\n"; - dif->report(out, indent + " "); + report_underlying_changes_of_qualified_type(d, out, indent); } /// Report the @ref pointer_diff in a serialized form. diff --git a/src/abg-ir.cc b/src/abg-ir.cc index e0d4f327..ce553653 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -26579,11 +26579,16 @@ types_have_similar_structure(const type_base* first, { const array_type_def* ty2 = is_array_type(second); // TODO: Handle int[5][2] vs int[2][5] better. - if (ty1->get_size_in_bits() != ty2->get_size_in_bits() - || ty1->get_dimension_count() != ty2->get_dimension_count() - || !types_have_similar_structure(ty1->get_element_type(), - ty2->get_element_type(), - /*indirect_type=*/true)) + if (!indirect_type) + { + if (ty1->get_size_in_bits() != ty2->get_size_in_bits() + || ty1->get_dimension_count() != ty2->get_dimension_count()) + return false; + } + + if (!types_have_similar_structure(ty1->get_element_type(), + ty2->get_element_type(), + /*indirect_type=*/true)) return false; return true; diff --git a/src/abg-leaf-reporter.cc b/src/abg-leaf-reporter.cc index 2006d4ff..10d65054 100644 --- a/src/abg-leaf-reporter.cc +++ b/src/abg-leaf-reporter.cc @@ -210,6 +210,12 @@ leaf_reporter::report(const qualified_type_diff& d, ostream& out, return; report_local_qualified_type_changes(d, out, indent); + + // Note that changes that are local to the underlying type of a + // qualified type are considered to be local to the qualified type + // itself. So let's go ahead and report the local changes of the + // underlying type. + report_underlying_changes_of_qualified_type(d, out, indent); } /// Report the changes carried by a @ref pointer_diff node. diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index c91c24fb..6aab630b 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -306,6 +306,14 @@ test-abidiff-exit/ada-subrange/test1-ada-subrange/v0/test1.o \ test-abidiff-exit/ada-subrange/test1-ada-subrange/v1/test1.adb \ test-abidiff-exit/ada-subrange/test1-ada-subrange/v1/test1.ads \ test-abidiff-exit/ada-subrange/test1-ada-subrange/v1/test1.o \ +test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-1.txt \ +test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-2.txt \ +test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.adb \ +test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.ads \ +test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o \ +test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.adb \ +test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.ads \ +test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o \ \ test-diff-dwarf/test0-v0.cc \ test-diff-dwarf/test0-v0.o \ diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-1.txt b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-1.txt new file mode 100644 index 00000000..742dda83 --- /dev/null +++ b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-1.txt @@ -0,0 +1,17 @@ +Functions changes summary: 0 Removed, 2 Changed, 0 Added functions +Variables changes summary: 0 Removed, 0 Changed, 0 Added variable + +2 functions with some indirect sub-type change: + + [C] 'function test__my_int[101] test__first_function(test__my_int[101]&)' at test.adb:6:1 has some indirect sub-type changes: + return type changed: + type name changed from 'test__my_int[101]' to 'test__my_int[201]' + array type size changed from 101000 to 201000 + array type subrange 1 changed length from 101 to 201 + + [C] 'function test__my_index test__second_function(const test__my_index)' at test.adb:12:1 has some indirect sub-type changes: + return type changed: + upper bound of range 'test__my_index' change from '100' to '200' + underlying type of range '[101]' changed: + type size changed from 8 to 16 (in bits) + diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-2.txt b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-2.txt new file mode 100644 index 00000000..45e1c527 --- /dev/null +++ b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-2.txt @@ -0,0 +1,24 @@ +Leaf changes summary: 4 artifacts changed +Changed leaf types summary: 2 leaf types changed +Removed/Changed/Added functions summary: 0 Removed, 2 Changed, 0 Added function +Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable + +2 functions with some sub-type change: + + [C] 'function test__my_int[101] test__first_function(test__my_int[101]&)' at test.adb:6:1 has some sub-type changes: + return type changed: + type name changed from 'test__my_int[101]' to 'test__my_int[201]' + array type size changed from 101000 to 201000 + array type subrange 1 changed length from 101 to 201 + + [C] 'function test__my_index test__second_function(const test__my_index)' at test.adb:12:1 has some sub-type changes: + return type changed: + upper bound of range 'test__my_index' change from '100' to '200' + parameter 1 of type 'const test__my_index' changed: + in unqualified underlying type '[101]': + upper bound of range 'test__my_index' change from '100' to '200' + +'test__Tmy_indexB' changed: + type size changed from 8 to 16 (in bits) +'[101]' changed: + upper bound of range 'test__my_index' change from '100' to '200' diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.adb b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.adb new file mode 100644 index 00000000..98b0315f --- /dev/null +++ b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.adb @@ -0,0 +1,17 @@ +-- Compile this file with: +-- gcc -g -c test.adb + +package body Test is + +function First_Function (A: My_Int_Array) return My_Int_Array is +begin + return A; +end First_Function; + + +function Second_Function (A: My_Index) return My_Index is +begin + return My_Index'Last; +end Second_Function; + +end Test; diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.ads b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.ads new file mode 100644 index 00000000..66a57f8b --- /dev/null +++ b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.ads @@ -0,0 +1,11 @@ +package Test is + +type My_Int is range 0 .. 1000; +type My_Index is range 0 .. 100; +type My_Int_Array is array (My_Index) of My_Int; + +function First_Function (A: My_Int_Array) return My_Int_Array; + +function Second_Function (A: My_Index) return My_Index; + +end Test; \ No newline at end of file diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o new file mode 100644 index 00000000..483ab7c1 Binary files /dev/null and b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o differ diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.adb b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.adb new file mode 100644 index 00000000..98b0315f --- /dev/null +++ b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.adb @@ -0,0 +1,17 @@ +-- Compile this file with: +-- gcc -g -c test.adb + +package body Test is + +function First_Function (A: My_Int_Array) return My_Int_Array is +begin + return A; +end First_Function; + + +function Second_Function (A: My_Index) return My_Index is +begin + return My_Index'Last; +end Second_Function; + +end Test; diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.ads b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.ads new file mode 100644 index 00000000..f2cf0a83 --- /dev/null +++ b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.ads @@ -0,0 +1,11 @@ +package Test is + +type My_Int is range 0 .. 1000; +type My_Index is range 0 .. 200; +type My_Int_Array is array (My_Index) of My_Int; + +function First_Function (A: My_Int_Array) return My_Int_Array; + +function Second_Function (A: My_Index) return My_Index; + +end Test; \ No newline at end of file diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o new file mode 100644 index 00000000..171d6bd0 Binary files /dev/null and b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o differ diff --git a/tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt b/tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt index 3bea50c9..d8d2cfff 100644 --- a/tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt +++ b/tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt @@ -16,6 +16,9 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable [C] 'function const int foo2(S2&)' at test45-basic-type-change-v1.cc:32:1 has some sub-type changes: return type changed: 'const int' changed to 'const char' + in unqualified underlying type 'int': + type name changed from 'int' to 'char' + type size changed from 32 to 8 (in bits) [C] 'function int foo3(S2&)' at test45-basic-type-change-v1.cc:36:1 has some sub-type changes: return type changed: diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc index 22bc9b77..3733dce9 100644 --- a/tests/test-abidiff-exit.cc +++ b/tests/test-abidiff-exit.cc @@ -702,6 +702,28 @@ InOutSpec in_out_specs[] = "data/test-abidiff-exit/ada-subrange/test1-ada-subrange/test1-ada-subrange-report-2.txt", "output/test-abidiff-exit/ada-subrange/test1-ada-subrange/test1-ada-subrange-report-2.txt" }, + { + "data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o", + "data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o", + "", + "", + "", + "--no-default-suppression", + abigail::tools_utils::ABIDIFF_ABI_CHANGE, + "data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-1.txt", + "output/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-1.txt" + }, + { + "data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o", + "data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o", + "", + "", + "", + "--no-default-suppression --leaf-changes-only", + abigail::tools_utils::ABIDIFF_ABI_CHANGE, + "data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-2.txt", + "output/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-2.txt" + }, #ifdef WITH_BTF { "data/test-abidiff-exit/btf/test0-v0.o",