[v2] abg-ir.cc: Improve types_have_similar_structure.
Commit Message
This function is used to determine whether or not type differences are
"local" and is primarily used in --leaf-changes-only mode. The logic
has some issues which are addressed by this patch:
- Any number of points-to (*) and refers-to (& and &&) components
are peeled off the types being compared, rather than checking
these match in number and kind.
- This peeling is done with peel_typedef_pointer_or_reference_type
which also peels any number of CV qualifiers (OK) and
array-of ([N]) type components (not OK).
- The function sets a state variable (was_indirect_type) to modify
the behaviour of downstream comparisons, but this cannot be
passed through recursive calls.
The effect of the indirect_type flag is to switch to comparisons by
name: identically named structs don't get introspected. Arguably, a
more useful behaviour for --leaf-changes-only mode would be to treat
any change to a named type as non-local, except in the context of the
definition of that type itself. This would be a more significant
change in behaviour.
* include/abg-fwd.h (types_have_similar_structure): In both
overloads, add an indirect_type argument, defaulting to
false.
* src/abg-ir.cc (reference_type_def constructor): Tabify.
(types_have_similar_structure): In both overloads, add an
indirect_type argument and update documentation text. In the
type_base_sptr overload, pass indirect_type in the tail
call. In the type_base* overload, replace was_indirect_type
with indirect_type; peel CV qualifiers and typedefs without
testing as the peel function does this; replace the
indiscriminate peeling of qualifier/pointer/reference/array
type components with code that checks the same
pointer/reference/array type component is used on each side
and makes recursive calls with indirect_type set to true; pass
the indirect_type argument recursively when comparing other
subtypes; move the typeid check earlier, document its purpose
and remove unneccessary checks after later dynamic casts;
remove an always-true conditional; add a TODO for comparing
array types more accurately.
* tests/data/Makefile.am: Add new test case files.
* tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc: New
test case.
* tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc: Ditto.
* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
Ditto.
* tests/test-abidiff-exit.cc: Run new test case.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
include/abg-fwd.h | 6 +-
src/abg-ir.cc | 147 ++++++++++--------
tests/data/Makefile.am | 5 +
.../test-leaf-peeling-report.txt | 38 +++++
.../test-abidiff-exit/test-leaf-peeling-v0.cc | 24 +++
.../test-abidiff-exit/test-leaf-peeling-v0.o | Bin 0 -> 3704 bytes
.../test-abidiff-exit/test-leaf-peeling-v1.cc | 24 +++
.../test-abidiff-exit/test-leaf-peeling-v1.o | Bin 0 -> 3752 bytes
tests/test-abidiff-exit.cc | 9 ++
9 files changed, 182 insertions(+), 71 deletions(-)
create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v0.o
create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v1.o
Comments
Hi!
On Wed, Mar 25, 2020 at 02:18:59PM +0000, Android Kernel Team wrote:
>This function is used to determine whether or not type differences are
>"local" and is primarily used in --leaf-changes-only mode. The logic
>has some issues which are addressed by this patch:
>
> - Any number of points-to (*) and refers-to (& and &&) components
> are peeled off the types being compared, rather than checking
> these match in number and kind.
> - This peeling is done with peel_typedef_pointer_or_reference_type
> which also peels any number of CV qualifiers (OK) and
> array-of ([N]) type components (not OK).
> - The function sets a state variable (was_indirect_type) to modify
> the behaviour of downstream comparisons, but this cannot be
> passed through recursive calls.
>
>The effect of the indirect_type flag is to switch to comparisons by
>name: identically named structs don't get introspected. Arguably, a
>more useful behaviour for --leaf-changes-only mode would be to treat
>any change to a named type as non-local, except in the context of the
>definition of that type itself. This would be a more significant
>change in behaviour.
>
> * include/abg-fwd.h (types_have_similar_structure): In both
> overloads, add an indirect_type argument, defaulting to
> false.
> * src/abg-ir.cc (reference_type_def constructor): Tabify.
> (types_have_similar_structure): In both overloads, add an
> indirect_type argument and update documentation text. In the
> type_base_sptr overload, pass indirect_type in the tail
> call. In the type_base* overload, replace was_indirect_type
> with indirect_type; peel CV qualifiers and typedefs without
> testing as the peel function does this; replace the
> indiscriminate peeling of qualifier/pointer/reference/array
> type components with code that checks the same
> pointer/reference/array type component is used on each side
> and makes recursive calls with indirect_type set to true; pass
> the indirect_type argument recursively when comparing other
> subtypes; move the typeid check earlier, document its purpose
> and remove unneccessary checks after later dynamic casts;
> remove an always-true conditional; add a TODO for comparing
> array types more accurately.
> * tests/data/Makefile.am: Add new test case files.
> * tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc: New
> test case.
> * tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc: Ditto.
> * tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
> Ditto.
> * tests/test-abidiff-exit.cc: Run new test case.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>
>---
> include/abg-fwd.h | 6 +-
> src/abg-ir.cc | 147 ++++++++++--------
> tests/data/Makefile.am | 5 +
> .../test-leaf-peeling-report.txt | 38 +++++
> .../test-abidiff-exit/test-leaf-peeling-v0.cc | 24 +++
> .../test-abidiff-exit/test-leaf-peeling-v0.o | Bin 0 -> 3704 bytes
> .../test-abidiff-exit/test-leaf-peeling-v1.cc | 24 +++
> .../test-abidiff-exit/test-leaf-peeling-v1.o | Bin 0 -> 3752 bytes
> tests/test-abidiff-exit.cc | 9 ++
> 9 files changed, 182 insertions(+), 71 deletions(-)
> create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
> create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
> create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v0.o
> create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
> create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v1.o
>
>diff --git a/include/abg-fwd.h b/include/abg-fwd.h
>index 6f2a862c..1aab70a6 100644
>--- a/include/abg-fwd.h
>+++ b/include/abg-fwd.h
>@@ -1293,11 +1293,13 @@ function_decl_is_less_than(const function_decl&f, const function_decl &s);
>
> bool
> types_have_similar_structure(const type_base_sptr& first,
>- const type_base_sptr& second);
>+ const type_base_sptr& second,
>+ bool indirect_type = false);
>
> bool
> types_have_similar_structure(const type_base* first,
>- const type_base* second);
>+ const type_base* second,
>+ bool indirect_type = false);
>
> } // end namespace ir
>
>diff --git a/src/abg-ir.cc b/src/abg-ir.cc
>index 2853fe6c..a10b0bb7 100644
>--- a/src/abg-ir.cc
>+++ b/src/abg-ir.cc
>@@ -13573,10 +13573,10 @@ reference_type_def::reference_type_def(const type_base_sptr pointed_to,
> decl_base_sptr pto = dynamic_pointer_cast<decl_base>(pointed_to);
> string name;
> if (pto)
>- {
>- set_visibility(pto->get_visibility());
>- name = string(pto->get_name()) + "&";
>- }
>+ {
>+ set_visibility(pto->get_visibility());
>+ name = string(pto->get_name()) + "&";
>+ }
Correct, but unrelated. No complaints. :-)
> else
> name = string(get_type_name(is_function_type(pointed_to),
> /*qualified_name=*/true)) + "&";
>@@ -22563,33 +22563,44 @@ function_decl_is_less_than(const function_decl &f, const function_decl &s)
> /// Test if two types have similar structures, even though they are
> /// (or can be) different.
> ///
>-/// Two indirect types (pointers, references, qualified, typedefs)
>-/// have similar structure if their underlying types are of the same
>-/// kind and have the same name. In this indirect types case, the
>-/// size of the underlying type does not matter.
>+/// const and volatile qualifiers are completely ignored.
>+///
>+/// typedef are resolved to their definitions; their names are ignored.
>+///
>+/// Two indirect types (pointers or references) have similar structure
>+/// if their underlying types are of the same kind and have the same
>+/// name. In the indirect types case, the size of the underlying type
>+/// does not matter.
> ///
> /// Two direct types (i.e, non indirect) have a similar structure if
> /// they have the same kind, name and size. Two class types have
>-/// similar structure if they have the same name, size, and if their
>-/// data members have similar types.
>+/// similar structure if they have the same name, size, and if the
>+/// types of their data members have similar types.
> ///
> /// @param first the first type to consider.
> ///
> /// @param second the second type to consider.
> ///
>+/// @param indirect_type whether to do an indirect comparison
>+///
> /// @return true iff @p first and @p second have similar structures.
> bool
> types_have_similar_structure(const type_base_sptr& first,
>- const type_base_sptr& second)
>-{return types_have_similar_structure(first.get(), second.get());}
>+ const type_base_sptr& second,
>+ bool indirect_type)
>+{return types_have_similar_structure(first.get(), second.get(), indirect_type);}
>
> /// Test if two types have similar structures, even though they are
> /// (or can be) different.
> ///
>-/// Two indirect types (pointers, references, qualified, typedefs)
>-/// have similar structure if their underlying types are of the same
>-/// kind and have the same name. In this indirect types case, the
>-/// size of the underlying type does not matter.
>+/// const and volatile qualifiers are completely ignored.
>+///
>+/// typedef are resolved to their definitions; their names are ignored.
>+///
>+/// Two indirect types (pointers or references) have similar structure
>+/// if their underlying types are of the same kind and have the same
>+/// name. In the indirect types case, the size of the underlying type
>+/// does not matter.
> ///
> /// Two direct types (i.e, non indirect) have a similar structure if
> /// they have the same kind, name and size. Two class types have
>@@ -22600,9 +22611,13 @@ types_have_similar_structure(const type_base_sptr& first,
> ///
> /// @param second the second type to consider.
> ///
>+/// @param indirect_type whether to do an indirect comparison
>+///
> /// @return true iff @p first and @p second have similar structures.
> bool
>-types_have_similar_structure(const type_base* first, const type_base* second)
>+types_have_similar_structure(const type_base* first,
>+ const type_base* second,
>+ bool indirect_type)
> {
> if (!!first != !!second)
> return false;
>@@ -22610,33 +22625,39 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> if (!first)
> return false;
>
>- if (is_typedef(first) || is_qualified_type(first))
>- first = peel_qualified_or_typedef_type(first);
>-
>- if (is_typedef(second) || is_qualified_type(second))
>- second = peel_qualified_or_typedef_type(second);
>+ // Treat typedefs purely as type aliases and ignore CV-qualifiers.
>+ first = peel_qualified_or_typedef_type(first);
>+ second = peel_qualified_or_typedef_type(second);
>
>- bool was_indirect_type = (is_pointer_type(first)
>- || is_pointer_type(second)
>- || is_reference_type(first)
>- || is_reference_type(second));
>+ // Eliminate all but N of the N^2 comparison cases. This also guarantees the
>+ // various ty2 below cannot be null.
>+ if (typeid(*first) != typeid(*second))
>+ return false;
Just curious (because I think it is correct): Does it make a measurable
difference?
>
>- if (was_indirect_type)
>+ // Peel off matching pointers.
>+ if (const pointer_type_def* ty1 = is_pointer_type(first))
A picky compiler might ask you to add some parentheses here. [-Wparentheses]
> {
>- first = peel_typedef_pointer_or_reference_type(first);
>- second = peel_typedef_pointer_or_reference_type(second);
>+ const pointer_type_def* ty2 = is_pointer_type(second);
>+ return types_have_similar_structure(ty1->get_pointed_to_type(),
>+ ty2->get_pointed_to_type(),
>+ true);
> }
>
>- if (typeid(*first) != typeid(*second))
>- return false;
>+ // Peel off matching references.
>+ if (const reference_type_def* ty1 = is_reference_type(first))
As above.
>+ {
>+ const reference_type_def* ty2 = is_reference_type(second);
>+ if (ty1->is_lvalue() != ty2->is_lvalue())
>+ return false;
>+ return types_have_similar_structure(ty1->get_pointed_to_type(),
>+ ty2->get_pointed_to_type(),
>+ true);
>+ }
>
> if (const type_decl* ty1 = is_type_decl(first))
> {
> const type_decl* ty2 = is_type_decl(second);
>- if (ty2 == 0)
>- return false;
>-
>- if (!was_indirect_type)
>+ if (!indirect_type)
> if (ty1->get_size_in_bits() != ty2->get_size_in_bits())
> return false;
>
>@@ -22646,10 +22667,7 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> if (const enum_type_decl* ty1 = is_enum_type(first))
> {
> const enum_type_decl* ty2 = is_enum_type(second);
>- if (ty2 == 0)
>- return false;
>-
>- if (!was_indirect_type)
>+ if (!indirect_type)
> if (ty1->get_size_in_bits() != ty2->get_size_in_bits())
> return false;
>
>@@ -22660,20 +22678,16 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> if (const class_decl* ty1 = is_class_type(first))
> {
> const class_decl* ty2 = is_class_type(second);
>- if (ty2 == 0)
>- return false;
>-
> if (!ty1->get_is_anonymous() && !ty2->get_is_anonymous()
> && ty1->get_name() != ty2->get_name())
> return false;
>
>- if (!was_indirect_type)
>+ if (!indirect_type)
> {
>- if (!was_indirect_type)
>- if ((ty1->get_size_in_bits() != ty2->get_size_in_bits())
>- || (ty1->get_non_static_data_members().size()
>- != ty2->get_non_static_data_members().size()))
>- return false;
>+ if ((ty1->get_size_in_bits() != ty2->get_size_in_bits())
>+ || (ty1->get_non_static_data_members().size()
>+ != ty2->get_non_static_data_members().size()))
>+ return false;
>
> for (class_or_union::data_members::const_iterator
> i = ty1->get_non_static_data_members().begin(),
>@@ -22685,7 +22699,8 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> var_decl_sptr dm1 = *i;
> var_decl_sptr dm2 = *j;
> if (!types_have_similar_structure(dm1->get_type().get(),
>- dm2->get_type().get()))
>+ dm2->get_type().get(),
>+ indirect_type))
> return false;
> }
> }
>@@ -22696,14 +22711,11 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> if (const union_decl* ty1 = is_union_type(first))
> {
> const union_decl* ty2 = is_union_type(second);
>- if (ty2 == 0)
>- return false;
>-
> if (!ty1->get_is_anonymous() && !ty2->get_is_anonymous()
> && ty1->get_name() != ty2->get_name())
> return false;
>
>- if (!was_indirect_type)
>+ if (!indirect_type)
> return ty1->get_size_in_bits() == ty2->get_size_in_bits();
>
> return true;
>@@ -22712,13 +22724,13 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> if (const array_type_def* ty1 = is_array_type(first))
> {
> const array_type_def* ty2 = is_array_type(second);
>-
>- if (!was_indirect_type)
>- 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()))
>- return false;
>+ // TODO: Handle uint32_t[10] vs uint64_t[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))
>+ return false;
>
> return true;
> }
>@@ -22726,14 +22738,12 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> if (const array_type_def::subrange_type *ty1 = is_subrange_type(first))
> {
> const array_type_def::subrange_type *ty2 = is_subrange_type(second);
>- if (!ty2)
>- return false;
>-
> if (ty1->get_upper_bound() != ty2->get_upper_bound()
> || ty1->get_lower_bound() != ty2->get_lower_bound()
> || ty1->get_language() != ty2->get_language()
> || !types_have_similar_structure(ty1->get_underlying_type(),
>- ty2->get_underlying_type()))
>+ ty2->get_underlying_type(),
>+ indirect_type))
> return false;
>
> return true;
>@@ -22742,11 +22752,9 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> if (const function_type* ty1 = is_function_type(first))
> {
> const function_type* ty2 = is_function_type(second);
>- if (!ty2)
>- return false;
>-
> if (!types_have_similar_structure(ty1->get_return_type(),
>- ty2->get_return_type()))
>+ ty2->get_return_type(),
>+ indirect_type))
> return false;
>
> if (ty1->get_parameters().size() != ty2->get_parameters().size())
>@@ -22759,7 +22767,8 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> && j != ty2->get_parameters().end());
> ++i, ++j)
> if (!types_have_similar_structure((*i)->get_type(),
>- (*j)->get_type()))
>+ (*j)->get_type(),
>+ indirect_type))
> return false;
>
> return true;
>diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
>index cf7792f1..6d4e2faa 100644
>--- a/tests/data/Makefile.am
>+++ b/tests/data/Makefile.am
>@@ -130,6 +130,11 @@ test-abidiff-exit/test-leaf3-v0.o \
> test-abidiff-exit/test-leaf3-v1.c \
> test-abidiff-exit/test-leaf3-v1.o \
> test-abidiff-exit/test-leaf3-report.txt \
>+test-abidiff-exit/test-leaf-peeling-v0.cc \
>+test-abidiff-exit/test-leaf-peeling-v0.o \
>+test-abidiff-exit/test-leaf-peeling-v1.cc \
>+test-abidiff-exit/test-leaf-peeling-v1.o \
>+test-abidiff-exit/test-leaf-peeling-report.txt \
> \
> test-diff-dwarf/test0-v0.cc \
> test-diff-dwarf/test0-v0.o \
>diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>new file mode 100644
>index 00000000..54a26869
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>@@ -0,0 +1,38 @@
>+Leaf changes summary: 4 artifacts changed
>+Changed leaf types summary: 4 leaf types changed
>+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
>+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
>+
>+'struct foo at test-leaf-peeling.0.cc:1:1' changed:
>+ type size changed from 32 to 64 (in bits)
>+ there are data member changes:
>+ type 'int' of 'foo::z' changed:
>+ type name changed from 'int' to 'long int'
>+ type size changed from 32 to 64 (in bits)
>+ and size changed from 32 to 64 (in bits) (by +32 bits)
>+
>+
>+
>+'struct ops1 at test-leaf-peeling.0.cc:5:1' changed:
>+ type size hasn't changed
>+ there are data member changes:
>+ type 'int*' of 'ops1::x' changed:
>+ pointer type changed from: 'int*' to: 'int**'
>+
>+
>+
>+
>+'struct ops2 at test-leaf-peeling.0.cc:9:1' changed:
>+ type size changed from 320 to 640 (in bits)
>+ there are data member changes:
>+ 'foo ops2::y[10]' size changed from 320 to 640 (in bits) (by +320 bits)
>+
>+
>+
>+'struct ops3 at test-leaf-peeling.0.cc:13:1' changed:
>+ type size hasn't changed
>+ there are data member changes:
>+ type 'void (int&)*' of 'ops3::spong' changed:
>+ pointer type changed from: 'void (int&)*' to: 'void (int&&)*'
>+
>+
>diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
>new file mode 100644
>index 00000000..b927d15e
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
>@@ -0,0 +1,24 @@
>+struct foo {
>+ int z;
>+};
>+
>+struct ops1 {
>+ int * x;
>+};
>+
>+struct ops2 {
>+ foo y[10];
>+};
>+
>+struct ops3 {
>+ void (*spong)(int & wibble);
>+};
>+
>+void register_ops1(ops1*) {
>+}
>+
>+void register_ops2(ops2*) {
>+}
>+
>+void register_ops3(ops3*) {
>+}
>diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v0.o b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.o
>new file mode 100644
>index 0000000000000000000000000000000000000000..a556a63560122a5e926ab708bb13f0fd40714638
>GIT binary patch
>literal 3704
>zcmbtXO>7%g5T56a?R7SGoVcZpf{-JD=nuP%ov0+G4TjJ(5v8h-Dx?-j*Us9pu-B1y
>z14$7g5<jB1B7p#@NJ!jz;zo~%3aK}495`{|iiEg;IKa&A8|V4izQ9QPX6Bop_ujm>
>zv-Z0eFRvwp044z|aHuf~a6fY>S0b?j1(=3w>$iVhzx~dicV2riTznm~jYR=wlsA?S
>zlfj%KC+eLJj0s_`A!ytJkw}w-8OD-LKZGR7rbwD~<i;L|RE8vgR1^`JPl$~90h#wf
>zjHgKgNZUAkijPS{1~O(EIj75--=Z>3h{%mUBo5a1I84w0*(i4^a>@}Pj-`k(SyB*p
>z|3Yb!hQ!C)xTL!%#Q{e~1f0T>Owtesw0Z$kBLZ$v01a`#OBeuRN<5J(SVnF(HzlUi
>z`D17;2*b3jbJjD~vLy;wj4T*ttZo#RaK&SfgJrE)hKW@`=bY-Cnnc?MMx}+=C{VaL
>z2OEVm7?zP?xUF$anNw3*M+Ny#B+(bEmX}F#j(B#4J^g5jmE)g1KSREg<eNB-n|KPJ
>z1nmZJMGDO+vMRoUiUD-2Lj0Y};uu|Gtc-xr3qrf)xpjNT^IA>6;mkX=S`?j}g-kpR
>z#pgv*yqBC^>5T~k6SX>iqqyq_&4%x773tb-UN0?ly+$(#z3ygbCn#NAq!EH0bcH@k
>zpH}YE79a}3p-j@ksC8bta;>;}`gCcrxa^djQn5T=S}v95%SF2#?)u(^hUa_TW(`%h
>zTid>{w{+G%yI8avMZ4|>`+jY^+wnWQfxYX$)%3UQ(5<$-U|<jBXb)vL6Sj8-j0=%*
>z0pQZ=>bc^H7rbiI_2cE(rKeA73!};&iOP%Bzx6VRy}X#6NSk-?G#*8q9-`<)c`ld#
>zFtM6Vt)T;AWIInrq*3-)WAjkEGz~DE8r#nA{|p!qz(XDeBi_3jeRcya!^n`0O}81)
>z_xAS821H$_r?X&SM;b;(yyF3r0oehJ03+@;12zLD1~7W(rj<HGwMT4T;;b`Ym-r*d
>zi5MSsiATLMI?mTn$N2_wC8;~JO!M=O2&d{D;6pSOv#6UB-jw=L4gXE*Gm_JdP*4A)
>z;;(k3D+*5euS>nH;hR$bT*IXbUuyW4)bDBdDXIUc;g_ZUlZO8x^`9l@eet>Rk#pTv
>zq+eRT5{z?ROX8e|&%LDi)4d^D((vgRhVxn;3HQ#F=8Znz(vX*&=VgEH6*^v&4s?d<
>zIaV~B*MC#Pd-ZXG{Wi8taDuQK^KF(wZ!ZLA%MD#{szCrw*K4`XmRH?vY&QLR2W^q;
>zR;yj_%|5f+b^Qi*VKoH5Odf~e6yVf4?Y4)X8^`a2o`atOI|$v{8#2h--mG`swkH?<
>zKiMqCQw`O3ic(f+K$?qLZzNxp$WEBv8C}&$k@ff|(MIppe~m9;te%sm<N2$aQB$Me
>z8}w7OLO}|BB@6P444`{Re%u47ZxUU=N6$(40}1{>ibA^YRkZ0Cxql=tFTYQmLic?U
>z4a3%dAQ#H<w0`>P=<BC`qvu2j?#uaakfM<8OZ{!w{NKolxK1=b^+|pH9mFUnuM@?H
>z3i3j5-(j5RXZ|)ChQ&Wdi$bFNr4q+6e;<u3t9}<{yo!;m`+gxYS+1V@x{TlJA+f;w
>zXZRBo$hE?N<PW9ey40zD^dExbxvsPxeQpZ*PR@TI7fjzn`n90P)0yb+9zsx&_x~m-
>V3hDc&yu;%2lo;vjnDs_I{$J;aGn)Va
>
>literal 0
>HcmV?d00001
>
>diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
>new file mode 100644
>index 00000000..7c88df9f
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
>@@ -0,0 +1,24 @@
>+struct foo {
>+ long z;
>+};
>+
>+struct ops1 {
>+ int ** x;
>+};
>+
>+struct ops2 {
>+ foo y[10];
>+};
There is no apparent difference in ops2 between the test files. What are
we testing?
>+
>+struct ops3 {
>+ void (*spong)(int && wibble);
>+};
>+
>+void register_ops1(ops1*) {
>+}
>+
>+void register_ops2(ops2*) {
>+}
>+
>+void register_ops3(ops3*) {
>+}
Maybe we can add some more test cases:
- compare reference types and pointer types
- compare int*** and int* (two levels of peeling)
- compare uint32_t[10] and uint64_t[5] (with the same TODO as in the code)
- nested types, e.g. struct ops1_v0* vs. struct ops1_v1*
Nice work. I like it!
Cheers,
Matthias
>diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v1.o b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.o
>new file mode 100644
>index 0000000000000000000000000000000000000000..e71ade4431f80a78ee73292d60808a2ee5404640
>GIT binary patch
>literal 3752
>zcmbtXO=x3P6h1evNnV>K{Yl4Zsf!f+8^*k8(lRq0ZH=wdX;6nE6lF$fU(%N}O!89l
>zUbU@=f+Ear6h#DAiVI!25#8%7MK^*A7lMjz-4zt{ocqpAZ*Ni%J&=3O`Oe?H=l;C*
>z)vHgf#)JSx0+!%dQz*digU517iY3UxIBcxl{&ns4D@S);_^Q8n3$qQ#09)pb<l|&8
>zqsg&ayA8Gw_9}wbA&8iiFoPW@H#yiMZr8}Z4<a!{5<tRk%CUoEav<LD!^n4C4BBra
>zW*vYSN|FR%=W+NDe}i(s+)08tciR3Ml_5ezX6Sq3kop#fVHzN9Q;CFh$`ByVCWt9Y
>zu^>K3;BrT3h`qIg0iU2D3Ya)0;1UJU5Ct4`1IA7X*r5O#qJXEVv=|fjXR;|PGo2X|
>z<H^ypXw3@CPNlA-9!V{vL>8No2Fs4ra`rOLc;G%rrIs)VyMWFa-8nIWwlYQ~*~!KZ
>z3Q^8drokB~XD?ueAxxRkQ>M-cau|-I19okhlVlw6*p%}0<rGVk?=4S}?+E$ECUFrD
>z<1a?50W7A`j3(>iv#3}=+bYDLnKX{_L}O<JgkBKlo1R<C?|NRd;n$slQ?1JA_%sYg
>zWhj1B#Kr6J>80)%voKM!?bmaAe$c4<-gb`8l|)Fm5Vm$9*hL5Ec=bjQdY#SoZcsR(
>z6;Eh$ARTZhk~9a`u5aX4&Yv&L=N6ozQ^*x(3k!w9Y%!N_g?qlYSoeId)2O2AcB(sz
>z`<E}~FV5%k^<2K@1_yq1r_=V^dqIBBf2rYb=R>#B^a9wtQJB-GR9v5zB~R&v>0XfT
>zg`PBX5}yOOwz6_1_s|nwrQ!O~#Pfwq=Zq!D7Q;luKpZ{;Vt-Uj4=3$AxH0!4PCp`f
>zXCBXtz8PCdCsxq`F|s{MMx@j1&qwCIb~g1eERE06AAk245WxNJ2P57+B`53_TKbX2
>z51whW;KW7lnk|qIP*+!i1=^#lK;%}mVhxEz3(`Fp0ao<z^w=yI?!oB#jca|7y+>@8
>zaiuH1#`rzRi4=a?z5BOkO&wX&U!6hKNqVT2O!d`@2&e9C5{NVwG5N3){tfGA41AjP
>zDduz{^xZ$F`Rk8US;Hy+IP2F9e4X_}1K(i%V*_UuJ~i+gtbbwPIo5wP@MYG2Ht@e#
>z|Bbm?uiDQvUkTOEdG?zy@Xr~aWL)J@=V8h4e}Mh38u%}amyJ9OclVm+Bfr>e7-g>N
>zDu4Bcn0SsIXm9oXEg86)|CWKP{(uu4wD8$Q9(E$WqomN=55d`XLl>M%5P;M1ny$0$
>zRrcze4Zqe#o0Q#3rQ^ML!t8cjzm6}t9)ce)PlaF!aH{QA%fk<m<F`Z4!B0s(2;J%}
>z4)S(3YaO@c@x=cpqkBO;)bARlEYW~86S3|{zB-YeF!RN9btg?W;~zvD-M9Z5U%*&1
>zCzVt6+lJ9l<)0+<r)!CVH2RVoa>N024#`iw4fLK8E#lA2N%(gR-X%pN)0f^26C?L`
>z`S8wau_T(lPa@WD{-5zgRXoj~zDDN!xBKKo2<~$Io1|!D)~9#4U;UrBkm?iFr}xvW
>zzl#{<RP#hpQjQOVdXE&Y>MQ;-8v4c0)1;8-eCed(6n_JaO4j{ua=ea_Z2G>>7`LnI
>zzQytTT_h6J`W5^R3glW+0P`buTw|U3NB?H1c-2>$k6D{Se&G7A@r3C+Nq<F{@w6xA
>e-9reT<MY2oibm%8DQ~~{5+z3ZC}Q2wjQ<DlLOH<z
>
>literal 0
>HcmV?d00001
>
>diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
>index 6ecbfd24..ebac7a00 100644
>--- a/tests/test-abidiff-exit.cc
>+++ b/tests/test-abidiff-exit.cc
>@@ -158,6 +158,15 @@ InOutSpec in_out_specs[] =
> "data/test-abidiff-exit/test-leaf3-report.txt",
> "output/test-abidiff-exit/test-leaf3-report.txt"
> },
>+ {
>+ "data/test-abidiff-exit/test-leaf-peeling-v0.o",
>+ "data/test-abidiff-exit/test-leaf-peeling-v1.o",
>+ "",
>+ "--leaf-changes-only",
>+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
>+ "data/test-abidiff-exit/test-leaf-peeling-report.txt",
>+ "output/test-abidiff-exit/test-leaf-peeling-report.txt"
>+ },
> {0, 0, 0 ,0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
> };
>
>--
>2.25.1.696.g5e7596f4ac-goog
>
>
Hi.
On Thu, 26 Mar 2020 at 11:38, Matthias Maennich <maennich@google.com> wrote:
>
> Hi!
>
> On Wed, Mar 25, 2020 at 02:18:59PM +0000, Android Kernel Team wrote:
> >This function is used to determine whether or not type differences are
> >"local" and is primarily used in --leaf-changes-only mode. The logic
> >has some issues which are addressed by this patch:
> >
> > - Any number of points-to (*) and refers-to (& and &&) components
> > are peeled off the types being compared, rather than checking
> > these match in number and kind.
> > - This peeling is done with peel_typedef_pointer_or_reference_type
> > which also peels any number of CV qualifiers (OK) and
> > array-of ([N]) type components (not OK).
> > - The function sets a state variable (was_indirect_type) to modify
> > the behaviour of downstream comparisons, but this cannot be
> > passed through recursive calls.
> >
> >The effect of the indirect_type flag is to switch to comparisons by
> >name: identically named structs don't get introspected. Arguably, a
> >more useful behaviour for --leaf-changes-only mode would be to treat
> >any change to a named type as non-local, except in the context of the
> >definition of that type itself. This would be a more significant
> >change in behaviour.
> >
> > * include/abg-fwd.h (types_have_similar_structure): In both
> > overloads, add an indirect_type argument, defaulting to
> > false.
> > * src/abg-ir.cc (reference_type_def constructor): Tabify.
> > (types_have_similar_structure): In both overloads, add an
> > indirect_type argument and update documentation text. In the
> > type_base_sptr overload, pass indirect_type in the tail
> > call. In the type_base* overload, replace was_indirect_type
> > with indirect_type; peel CV qualifiers and typedefs without
> > testing as the peel function does this; replace the
> > indiscriminate peeling of qualifier/pointer/reference/array
> > type components with code that checks the same
> > pointer/reference/array type component is used on each side
> > and makes recursive calls with indirect_type set to true; pass
> > the indirect_type argument recursively when comparing other
> > subtypes; move the typeid check earlier, document its purpose
> > and remove unneccessary checks after later dynamic casts;
> > remove an always-true conditional; add a TODO for comparing
> > array types more accurately.
> > * tests/data/Makefile.am: Add new test case files.
> > * tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc: New
> > test case.
> > * tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc: Ditto.
> > * tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
> > Ditto.
> > * tests/test-abidiff-exit.cc: Run new test case.
> >
> >Signed-off-by: Giuliano Procida <gprocida@google.com>
> >---
> > include/abg-fwd.h | 6 +-
> > src/abg-ir.cc | 147 ++++++++++--------
> > tests/data/Makefile.am | 5 +
> > .../test-leaf-peeling-report.txt | 38 +++++
> > .../test-abidiff-exit/test-leaf-peeling-v0.cc | 24 +++
> > .../test-abidiff-exit/test-leaf-peeling-v0.o | Bin 0 -> 3704 bytes
> > .../test-abidiff-exit/test-leaf-peeling-v1.cc | 24 +++
> > .../test-abidiff-exit/test-leaf-peeling-v1.o | Bin 0 -> 3752 bytes
> > tests/test-abidiff-exit.cc | 9 ++
> > 9 files changed, 182 insertions(+), 71 deletions(-)
> > create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
> > create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
> > create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v0.o
> > create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
> > create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v1.o
> >
> >diff --git a/include/abg-fwd.h b/include/abg-fwd.h
> >index 6f2a862c..1aab70a6 100644
> >--- a/include/abg-fwd.h
> >+++ b/include/abg-fwd.h
> >@@ -1293,11 +1293,13 @@ function_decl_is_less_than(const function_decl&f, const function_decl &s);
> >
> > bool
> > types_have_similar_structure(const type_base_sptr& first,
> >- const type_base_sptr& second);
> >+ const type_base_sptr& second,
> >+ bool indirect_type = false);
> >
> > bool
> > types_have_similar_structure(const type_base* first,
> >- const type_base* second);
> >+ const type_base* second,
> >+ bool indirect_type = false);
> >
> > } // end namespace ir
> >
> >diff --git a/src/abg-ir.cc b/src/abg-ir.cc
> >index 2853fe6c..a10b0bb7 100644
> >--- a/src/abg-ir.cc
> >+++ b/src/abg-ir.cc
> >@@ -13573,10 +13573,10 @@ reference_type_def::reference_type_def(const type_base_sptr pointed_to,
> > decl_base_sptr pto = dynamic_pointer_cast<decl_base>(pointed_to);
> > string name;
> > if (pto)
> >- {
> >- set_visibility(pto->get_visibility());
> >- name = string(pto->get_name()) + "&";
> >- }
> >+ {
> >+ set_visibility(pto->get_visibility());
> >+ name = string(pto->get_name()) + "&";
> >+ }
>
> Correct, but unrelated. No complaints. :-)
>
> > else
> > name = string(get_type_name(is_function_type(pointed_to),
> > /*qualified_name=*/true)) + "&";
> >@@ -22563,33 +22563,44 @@ function_decl_is_less_than(const function_decl &f, const function_decl &s)
> > /// Test if two types have similar structures, even though they are
> > /// (or can be) different.
> > ///
> >-/// Two indirect types (pointers, references, qualified, typedefs)
> >-/// have similar structure if their underlying types are of the same
> >-/// kind and have the same name. In this indirect types case, the
> >-/// size of the underlying type does not matter.
> >+/// const and volatile qualifiers are completely ignored.
> >+///
> >+/// typedef are resolved to their definitions; their names are ignored.
> >+///
> >+/// Two indirect types (pointers or references) have similar structure
> >+/// if their underlying types are of the same kind and have the same
> >+/// name. In the indirect types case, the size of the underlying type
> >+/// does not matter.
> > ///
> > /// Two direct types (i.e, non indirect) have a similar structure if
> > /// they have the same kind, name and size. Two class types have
> >-/// similar structure if they have the same name, size, and if their
> >-/// data members have similar types.
> >+/// similar structure if they have the same name, size, and if the
> >+/// types of their data members have similar types.
> > ///
> > /// @param first the first type to consider.
> > ///
> > /// @param second the second type to consider.
> > ///
> >+/// @param indirect_type whether to do an indirect comparison
> >+///
> > /// @return true iff @p first and @p second have similar structures.
> > bool
> > types_have_similar_structure(const type_base_sptr& first,
> >- const type_base_sptr& second)
> >-{return types_have_similar_structure(first.get(), second.get());}
> >+ const type_base_sptr& second,
> >+ bool indirect_type)
> >+{return types_have_similar_structure(first.get(), second.get(), indirect_type);}
> >
> > /// Test if two types have similar structures, even though they are
> > /// (or can be) different.
> > ///
> >-/// Two indirect types (pointers, references, qualified, typedefs)
> >-/// have similar structure if their underlying types are of the same
> >-/// kind and have the same name. In this indirect types case, the
> >-/// size of the underlying type does not matter.
> >+/// const and volatile qualifiers are completely ignored.
> >+///
> >+/// typedef are resolved to their definitions; their names are ignored.
> >+///
> >+/// Two indirect types (pointers or references) have similar structure
> >+/// if their underlying types are of the same kind and have the same
> >+/// name. In the indirect types case, the size of the underlying type
> >+/// does not matter.
> > ///
> > /// Two direct types (i.e, non indirect) have a similar structure if
> > /// they have the same kind, name and size. Two class types have
> >@@ -22600,9 +22611,13 @@ types_have_similar_structure(const type_base_sptr& first,
> > ///
> > /// @param second the second type to consider.
> > ///
> >+/// @param indirect_type whether to do an indirect comparison
> >+///
> > /// @return true iff @p first and @p second have similar structures.
> > bool
> >-types_have_similar_structure(const type_base* first, const type_base* second)
> >+types_have_similar_structure(const type_base* first,
> >+ const type_base* second,
> >+ bool indirect_type)
> > {
> > if (!!first != !!second)
> > return false;
> >@@ -22610,33 +22625,39 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> > if (!first)
> > return false;
> >
> >- if (is_typedef(first) || is_qualified_type(first))
> >- first = peel_qualified_or_typedef_type(first);
> >-
> >- if (is_typedef(second) || is_qualified_type(second))
> >- second = peel_qualified_or_typedef_type(second);
> >+ // Treat typedefs purely as type aliases and ignore CV-qualifiers.
> >+ first = peel_qualified_or_typedef_type(first);
> >+ second = peel_qualified_or_typedef_type(second);
> >
> >- bool was_indirect_type = (is_pointer_type(first)
> >- || is_pointer_type(second)
> >- || is_reference_type(first)
> >- || is_reference_type(second));
> >+ // Eliminate all but N of the N^2 comparison cases. This also guarantees the
> >+ // various ty2 below cannot be null.
> >+ if (typeid(*first) != typeid(*second))
> >+ return false;
>
> Just curious (because I think it is correct): Does it make a measurable
> difference?
The code was there before, after the peeling. However, various ty2
checks were still there, including ones I'd added. Once I went and
reminded myself what typeid meant, the linear code made a lot more
sense.
> >
> >- if (was_indirect_type)
> >+ // Peel off matching pointers.
> >+ if (const pointer_type_def* ty1 = is_pointer_type(first))
>
> A picky compiler might ask you to add some parentheses here. [-Wparentheses]
-Wparentheses is already enabled by -Wall which is enabled by ABIGAIL_DEVEL=1.
In any case, there is no ambiguity here.
> > {
> >- first = peel_typedef_pointer_or_reference_type(first);
> >- second = peel_typedef_pointer_or_reference_type(second);
> >+ const pointer_type_def* ty2 = is_pointer_type(second);
> >+ return types_have_similar_structure(ty1->get_pointed_to_type(),
> >+ ty2->get_pointed_to_type(),
> >+ true);
> > }
> >
> >- if (typeid(*first) != typeid(*second))
> >- return false;
> >+ // Peel off matching references.
> >+ if (const reference_type_def* ty1 = is_reference_type(first))
>
> As above.
>
> >+ {
> >+ const reference_type_def* ty2 = is_reference_type(second);
> >+ if (ty1->is_lvalue() != ty2->is_lvalue())
> >+ return false;
> >+ return types_have_similar_structure(ty1->get_pointed_to_type(),
> >+ ty2->get_pointed_to_type(),
> >+ true);
> >+ }
> >
> > if (const type_decl* ty1 = is_type_decl(first))
> > {
> > const type_decl* ty2 = is_type_decl(second);
> >- if (ty2 == 0)
> >- return false;
> >-
> >- if (!was_indirect_type)
> >+ if (!indirect_type)
> > if (ty1->get_size_in_bits() != ty2->get_size_in_bits())
> > return false;
> >
> >@@ -22646,10 +22667,7 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> > if (const enum_type_decl* ty1 = is_enum_type(first))
> > {
> > const enum_type_decl* ty2 = is_enum_type(second);
> >- if (ty2 == 0)
> >- return false;
> >-
> >- if (!was_indirect_type)
> >+ if (!indirect_type)
> > if (ty1->get_size_in_bits() != ty2->get_size_in_bits())
> > return false;
> >
> >@@ -22660,20 +22678,16 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> > if (const class_decl* ty1 = is_class_type(first))
> > {
> > const class_decl* ty2 = is_class_type(second);
> >- if (ty2 == 0)
> >- return false;
> >-
> > if (!ty1->get_is_anonymous() && !ty2->get_is_anonymous()
> > && ty1->get_name() != ty2->get_name())
> > return false;
> >
> >- if (!was_indirect_type)
> >+ if (!indirect_type)
> > {
> >- if (!was_indirect_type)
> >- if ((ty1->get_size_in_bits() != ty2->get_size_in_bits())
> >- || (ty1->get_non_static_data_members().size()
> >- != ty2->get_non_static_data_members().size()))
> >- return false;
> >+ if ((ty1->get_size_in_bits() != ty2->get_size_in_bits())
> >+ || (ty1->get_non_static_data_members().size()
> >+ != ty2->get_non_static_data_members().size()))
> >+ return false;
> >
> > for (class_or_union::data_members::const_iterator
> > i = ty1->get_non_static_data_members().begin(),
> >@@ -22685,7 +22699,8 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> > var_decl_sptr dm1 = *i;
> > var_decl_sptr dm2 = *j;
> > if (!types_have_similar_structure(dm1->get_type().get(),
> >- dm2->get_type().get()))
> >+ dm2->get_type().get(),
> >+ indirect_type))
> > return false;
> > }
> > }
> >@@ -22696,14 +22711,11 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> > if (const union_decl* ty1 = is_union_type(first))
> > {
> > const union_decl* ty2 = is_union_type(second);
> >- if (ty2 == 0)
> >- return false;
> >-
> > if (!ty1->get_is_anonymous() && !ty2->get_is_anonymous()
> > && ty1->get_name() != ty2->get_name())
> > return false;
> >
> >- if (!was_indirect_type)
> >+ if (!indirect_type)
> > return ty1->get_size_in_bits() == ty2->get_size_in_bits();
> >
> > return true;
> >@@ -22712,13 +22724,13 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> > if (const array_type_def* ty1 = is_array_type(first))
> > {
> > const array_type_def* ty2 = is_array_type(second);
> >-
> >- if (!was_indirect_type)
> >- 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()))
> >- return false;
> >+ // TODO: Handle uint32_t[10] vs uint64_t[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))
> >+ return false;
> >
> > return true;
> > }
> >@@ -22726,14 +22738,12 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> > if (const array_type_def::subrange_type *ty1 = is_subrange_type(first))
> > {
> > const array_type_def::subrange_type *ty2 = is_subrange_type(second);
> >- if (!ty2)
> >- return false;
> >-
> > if (ty1->get_upper_bound() != ty2->get_upper_bound()
> > || ty1->get_lower_bound() != ty2->get_lower_bound()
> > || ty1->get_language() != ty2->get_language()
> > || !types_have_similar_structure(ty1->get_underlying_type(),
> >- ty2->get_underlying_type()))
> >+ ty2->get_underlying_type(),
> >+ indirect_type))
> > return false;
> >
> > return true;
> >@@ -22742,11 +22752,9 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> > if (const function_type* ty1 = is_function_type(first))
> > {
> > const function_type* ty2 = is_function_type(second);
> >- if (!ty2)
> >- return false;
> >-
> > if (!types_have_similar_structure(ty1->get_return_type(),
> >- ty2->get_return_type()))
> >+ ty2->get_return_type(),
> >+ indirect_type))
> > return false;
> >
> > if (ty1->get_parameters().size() != ty2->get_parameters().size())
> >@@ -22759,7 +22767,8 @@ types_have_similar_structure(const type_base* first, const type_base* second)
> > && j != ty2->get_parameters().end());
> > ++i, ++j)
> > if (!types_have_similar_structure((*i)->get_type(),
> >- (*j)->get_type()))
> >+ (*j)->get_type(),
> >+ indirect_type))
> > return false;
> >
> > return true;
> >diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
> >index cf7792f1..6d4e2faa 100644
> >--- a/tests/data/Makefile.am
> >+++ b/tests/data/Makefile.am
> >@@ -130,6 +130,11 @@ test-abidiff-exit/test-leaf3-v0.o \
> > test-abidiff-exit/test-leaf3-v1.c \
> > test-abidiff-exit/test-leaf3-v1.o \
> > test-abidiff-exit/test-leaf3-report.txt \
> >+test-abidiff-exit/test-leaf-peeling-v0.cc \
> >+test-abidiff-exit/test-leaf-peeling-v0.o \
> >+test-abidiff-exit/test-leaf-peeling-v1.cc \
> >+test-abidiff-exit/test-leaf-peeling-v1.o \
> >+test-abidiff-exit/test-leaf-peeling-report.txt \
> > \
> > test-diff-dwarf/test0-v0.cc \
> > test-diff-dwarf/test0-v0.o \
> >diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
> >new file mode 100644
> >index 00000000..54a26869
> >--- /dev/null
> >+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
> >@@ -0,0 +1,38 @@
> >+Leaf changes summary: 4 artifacts changed
> >+Changed leaf types summary: 4 leaf types changed
> >+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
> >+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> >+
> >+'struct foo at test-leaf-peeling.0.cc:1:1' changed:
> >+ type size changed from 32 to 64 (in bits)
> >+ there are data member changes:
> >+ type 'int' of 'foo::z' changed:
> >+ type name changed from 'int' to 'long int'
> >+ type size changed from 32 to 64 (in bits)
> >+ and size changed from 32 to 64 (in bits) (by +32 bits)
> >+
> >+
> >+
> >+'struct ops1 at test-leaf-peeling.0.cc:5:1' changed:
> >+ type size hasn't changed
> >+ there are data member changes:
> >+ type 'int*' of 'ops1::x' changed:
> >+ pointer type changed from: 'int*' to: 'int**'
> >+
> >+
> >+
> >+
> >+'struct ops2 at test-leaf-peeling.0.cc:9:1' changed:
> >+ type size changed from 320 to 640 (in bits)
> >+ there are data member changes:
> >+ 'foo ops2::y[10]' size changed from 320 to 640 (in bits) (by +320 bits)
> >+
> >+
> >+
> >+'struct ops3 at test-leaf-peeling.0.cc:13:1' changed:
> >+ type size hasn't changed
> >+ there are data member changes:
> >+ type 'void (int&)*' of 'ops3::spong' changed:
> >+ pointer type changed from: 'void (int&)*' to: 'void (int&&)*'
> >+
> >+
> >diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
> >new file mode 100644
> >index 00000000..b927d15e
> >--- /dev/null
> >+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
> >@@ -0,0 +1,24 @@
> >+struct foo {
> >+ int z;
> >+};
> >+
> >+struct ops1 {
> >+ int * x;
> >+};
> >+
> >+struct ops2 {
> >+ foo y[10];
> >+};
> >+
> >+struct ops3 {
> >+ void (*spong)(int & wibble);
> >+};
> >+
> >+void register_ops1(ops1*) {
> >+}
> >+
> >+void register_ops2(ops2*) {
> >+}
> >+
> >+void register_ops3(ops3*) {
> >+}
> >diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v0.o b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.o
> >new file mode 100644
> >index 0000000000000000000000000000000000000000..a556a63560122a5e926ab708bb13f0fd40714638
> >GIT binary patch
> >literal 3704
> >zcmbtXO>7%g5T56a?R7SGoVcZpf{-JD=nuP%ov0+G4TjJ(5v8h-Dx?-j*Us9pu-B1y
> >z14$7g5<jB1B7p#@NJ!jz;zo~%3aK}495`{|iiEg;IKa&A8|V4izQ9QPX6Bop_ujm>
> >zv-Z0eFRvwp044z|aHuf~a6fY>S0b?j1(=3w>$iVhzx~dicV2riTznm~jYR=wlsA?S
> >zlfj%KC+eLJj0s_`A!ytJkw}w-8OD-LKZGR7rbwD~<i;L|RE8vgR1^`JPl$~90h#wf
> >zjHgKgNZUAkijPS{1~O(EIj75--=Z>3h{%mUBo5a1I84w0*(i4^a>@}Pj-`k(SyB*p
> >z|3Yb!hQ!C)xTL!%#Q{e~1f0T>Owtesw0Z$kBLZ$v01a`#OBeuRN<5J(SVnF(HzlUi
> >z`D17;2*b3jbJjD~vLy;wj4T*ttZo#RaK&SfgJrE)hKW@`=bY-Cnnc?MMx}+=C{VaL
> >z2OEVm7?zP?xUF$anNw3*M+Ny#B+(bEmX}F#j(B#4J^g5jmE)g1KSREg<eNB-n|KPJ
> >z1nmZJMGDO+vMRoUiUD-2Lj0Y};uu|Gtc-xr3qrf)xpjNT^IA>6;mkX=S`?j}g-kpR
> >z#pgv*yqBC^>5T~k6SX>iqqyq_&4%x773tb-UN0?ly+$(#z3ygbCn#NAq!EH0bcH@k
> >zpH}YE79a}3p-j@ksC8bta;>;}`gCcrxa^djQn5T=S}v95%SF2#?)u(^hUa_TW(`%h
> >zTid>{w{+G%yI8avMZ4|>`+jY^+wnWQfxYX$)%3UQ(5<$-U|<jBXb)vL6Sj8-j0=%*
> >z0pQZ=>bc^H7rbiI_2cE(rKeA73!};&iOP%Bzx6VRy}X#6NSk-?G#*8q9-`<)c`ld#
> >zFtM6Vt)T;AWIInrq*3-)WAjkEGz~DE8r#nA{|p!qz(XDeBi_3jeRcya!^n`0O}81)
> >z_xAS821H$_r?X&SM;b;(yyF3r0oehJ03+@;12zLD1~7W(rj<HGwMT4T;;b`Ym-r*d
> >zi5MSsiATLMI?mTn$N2_wC8;~JO!M=O2&d{D;6pSOv#6UB-jw=L4gXE*Gm_JdP*4A)
> >z;;(k3D+*5euS>nH;hR$bT*IXbUuyW4)bDBdDXIUc;g_ZUlZO8x^`9l@eet>Rk#pTv
> >zq+eRT5{z?ROX8e|&%LDi)4d^D((vgRhVxn;3HQ#F=8Znz(vX*&=VgEH6*^v&4s?d<
> >zIaV~B*MC#Pd-ZXG{Wi8taDuQK^KF(wZ!ZLA%MD#{szCrw*K4`XmRH?vY&QLR2W^q;
> >zR;yj_%|5f+b^Qi*VKoH5Odf~e6yVf4?Y4)X8^`a2o`atOI|$v{8#2h--mG`swkH?<
> >zKiMqCQw`O3ic(f+K$?qLZzNxp$WEBv8C}&$k@ff|(MIppe~m9;te%sm<N2$aQB$Me
> >z8}w7OLO}|BB@6P444`{Re%u47ZxUU=N6$(40}1{>ibA^YRkZ0Cxql=tFTYQmLic?U
> >z4a3%dAQ#H<w0`>P=<BC`qvu2j?#uaakfM<8OZ{!w{NKolxK1=b^+|pH9mFUnuM@?H
> >z3i3j5-(j5RXZ|)ChQ&Wdi$bFNr4q+6e;<u3t9}<{yo!;m`+gxYS+1V@x{TlJA+f;w
> >zXZRBo$hE?N<PW9ey40zD^dExbxvsPxeQpZ*PR@TI7fjzn`n90P)0yb+9zsx&_x~m-
> >V3hDc&yu;%2lo;vjnDs_I{$J;aGn)Va
> >
> >literal 0
> >HcmV?d00001
> >
> >diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
> >new file mode 100644
> >index 00000000..7c88df9f
> >--- /dev/null
> >+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
> >@@ -0,0 +1,24 @@
> >+struct foo {
> >+ long z;
> >+};
> >+
> >+struct ops1 {
> >+ int ** x;
> >+};
> >+
> >+struct ops2 {
> >+ foo y[10];
> >+};
>
> There is no apparent difference in ops2 between the test files. What are
> we testing?
We are testing the reporting of an indirect change via its dependency
on struct foo (which is reported already).
We could (and I would) argue that the change to ops2 is not local and
people doing kernel ABI diffs might agree as well. However, at the
moment, size changes are considered as local. If y had been a pointer
something something foo, the size would not have changed and this
would not have been reported. I'll add a comment to the test code.
> >+
> >+struct ops3 {
> >+ void (*spong)(int && wibble);
> >+};
> >+
> >+void register_ops1(ops1*) {
> >+}
> >+
> >+void register_ops2(ops2*) {
> >+}
> >+
> >+void register_ops3(ops3*) {
> >+}
>
> Maybe we can add some more test cases:
>
> - compare reference types and pointer types
> - compare int*** and int* (two levels of peeling)
> - compare uint32_t[10] and uint64_t[5] (with the same TODO as in the code)
> - nested types, e.g. struct ops1_v0* vs. struct ops1_v1*
Agreed. Will do.
> Nice work. I like it!
>
> Cheers,
> Matthias
Thank you,
Giuliano.
> >diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v1.o b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.o
> >new file mode 100644
> >index 0000000000000000000000000000000000000000..e71ade4431f80a78ee73292d60808a2ee5404640
> >GIT binary patch
> >literal 3752
> >zcmbtXO=x3P6h1evNnV>K{Yl4Zsf!f+8^*k8(lRq0ZH=wdX;6nE6lF$fU(%N}O!89l
> >zUbU@=f+Ear6h#DAiVI!25#8%7MK^*A7lMjz-4zt{ocqpAZ*Ni%J&=3O`Oe?H=l;C*
> >z)vHgf#)JSx0+!%dQz*digU517iY3UxIBcxl{&ns4D@S);_^Q8n3$qQ#09)pb<l|&8
> >zqsg&ayA8Gw_9}wbA&8iiFoPW@H#yiMZr8}Z4<a!{5<tRk%CUoEav<LD!^n4C4BBra
> >zW*vYSN|FR%=W+NDe}i(s+)08tciR3Ml_5ezX6Sq3kop#fVHzN9Q;CFh$`ByVCWt9Y
> >zu^>K3;BrT3h`qIg0iU2D3Ya)0;1UJU5Ct4`1IA7X*r5O#qJXEVv=|fjXR;|PGo2X|
> >z<H^ypXw3@CPNlA-9!V{vL>8No2Fs4ra`rOLc;G%rrIs)VyMWFa-8nIWwlYQ~*~!KZ
> >z3Q^8drokB~XD?ueAxxRkQ>M-cau|-I19okhlVlw6*p%}0<rGVk?=4S}?+E$ECUFrD
> >z<1a?50W7A`j3(>iv#3}=+bYDLnKX{_L}O<JgkBKlo1R<C?|NRd;n$slQ?1JA_%sYg
> >zWhj1B#Kr6J>80)%voKM!?bmaAe$c4<-gb`8l|)Fm5Vm$9*hL5Ec=bjQdY#SoZcsR(
> >z6;Eh$ARTZhk~9a`u5aX4&Yv&L=N6ozQ^*x(3k!w9Y%!N_g?qlYSoeId)2O2AcB(sz
> >z`<E}~FV5%k^<2K@1_yq1r_=V^dqIBBf2rYb=R>#B^a9wtQJB-GR9v5zB~R&v>0XfT
> >zg`PBX5}yOOwz6_1_s|nwrQ!O~#Pfwq=Zq!D7Q;luKpZ{;Vt-Uj4=3$AxH0!4PCp`f
> >zXCBXtz8PCdCsxq`F|s{MMx@j1&qwCIb~g1eERE06AAk245WxNJ2P57+B`53_TKbX2
> >z51whW;KW7lnk|qIP*+!i1=^#lK;%}mVhxEz3(`Fp0ao<z^w=yI?!oB#jca|7y+>@8
> >zaiuH1#`rzRi4=a?z5BOkO&wX&U!6hKNqVT2O!d`@2&e9C5{NVwG5N3){tfGA41AjP
> >zDduz{^xZ$F`Rk8US;Hy+IP2F9e4X_}1K(i%V*_UuJ~i+gtbbwPIo5wP@MYG2Ht@e#
> >z|Bbm?uiDQvUkTOEdG?zy@Xr~aWL)J@=V8h4e}Mh38u%}amyJ9OclVm+Bfr>e7-g>N
> >zDu4Bcn0SsIXm9oXEg86)|CWKP{(uu4wD8$Q9(E$WqomN=55d`XLl>M%5P;M1ny$0$
> >zRrcze4Zqe#o0Q#3rQ^ML!t8cjzm6}t9)ce)PlaF!aH{QA%fk<m<F`Z4!B0s(2;J%}
> >z4)S(3YaO@c@x=cpqkBO;)bARlEYW~86S3|{zB-YeF!RN9btg?W;~zvD-M9Z5U%*&1
> >zCzVt6+lJ9l<)0+<r)!CVH2RVoa>N024#`iw4fLK8E#lA2N%(gR-X%pN)0f^26C?L`
> >z`S8wau_T(lPa@WD{-5zgRXoj~zDDN!xBKKo2<~$Io1|!D)~9#4U;UrBkm?iFr}xvW
> >zzl#{<RP#hpQjQOVdXE&Y>MQ;-8v4c0)1;8-eCed(6n_JaO4j{ua=ea_Z2G>>7`LnI
> >zzQytTT_h6J`W5^R3glW+0P`buTw|U3NB?H1c-2>$k6D{Se&G7A@r3C+Nq<F{@w6xA
> >e-9reT<MY2oibm%8DQ~~{5+z3ZC}Q2wjQ<DlLOH<z
> >
> >literal 0
> >HcmV?d00001
> >
> >diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
> >index 6ecbfd24..ebac7a00 100644
> >--- a/tests/test-abidiff-exit.cc
> >+++ b/tests/test-abidiff-exit.cc
> >@@ -158,6 +158,15 @@ InOutSpec in_out_specs[] =
> > "data/test-abidiff-exit/test-leaf3-report.txt",
> > "output/test-abidiff-exit/test-leaf3-report.txt"
> > },
> >+ {
> >+ "data/test-abidiff-exit/test-leaf-peeling-v0.o",
> >+ "data/test-abidiff-exit/test-leaf-peeling-v1.o",
> >+ "",
> >+ "--leaf-changes-only",
> >+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
> >+ "data/test-abidiff-exit/test-leaf-peeling-report.txt",
> >+ "output/test-abidiff-exit/test-leaf-peeling-report.txt"
> >+ },
> > {0, 0, 0 ,0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
> > };
> >
> >--
> >2.25.1.696.g5e7596f4ac-goog
> >
> >
On Thu, Mar 26, 2020 at 12:14:20PM +0000, Giuliano Procida wrote:
>Hi.
>
>On Thu, 26 Mar 2020 at 11:38, Matthias Maennich <maennich@google.com> wrote:
>>
>> Hi!
>>
>> On Wed, Mar 25, 2020 at 02:18:59PM +0000, Android Kernel Team wrote:
>> >This function is used to determine whether or not type differences are
>> >"local" and is primarily used in --leaf-changes-only mode. The logic
>> >has some issues which are addressed by this patch:
>> >
>> > - Any number of points-to (*) and refers-to (& and &&) components
>> > are peeled off the types being compared, rather than checking
>> > these match in number and kind.
>> > - This peeling is done with peel_typedef_pointer_or_reference_type
>> > which also peels any number of CV qualifiers (OK) and
>> > array-of ([N]) type components (not OK).
>> > - The function sets a state variable (was_indirect_type) to modify
>> > the behaviour of downstream comparisons, but this cannot be
>> > passed through recursive calls.
>> >
>> >The effect of the indirect_type flag is to switch to comparisons by
>> >name: identically named structs don't get introspected. Arguably, a
>> >more useful behaviour for --leaf-changes-only mode would be to treat
>> >any change to a named type as non-local, except in the context of the
>> >definition of that type itself. This would be a more significant
>> >change in behaviour.
>> >
>> > * include/abg-fwd.h (types_have_similar_structure): In both
>> > overloads, add an indirect_type argument, defaulting to
>> > false.
>> > * src/abg-ir.cc (reference_type_def constructor): Tabify.
>> > (types_have_similar_structure): In both overloads, add an
>> > indirect_type argument and update documentation text. In the
>> > type_base_sptr overload, pass indirect_type in the tail
>> > call. In the type_base* overload, replace was_indirect_type
>> > with indirect_type; peel CV qualifiers and typedefs without
>> > testing as the peel function does this; replace the
>> > indiscriminate peeling of qualifier/pointer/reference/array
>> > type components with code that checks the same
>> > pointer/reference/array type component is used on each side
>> > and makes recursive calls with indirect_type set to true; pass
>> > the indirect_type argument recursively when comparing other
>> > subtypes; move the typeid check earlier, document its purpose
>> > and remove unneccessary checks after later dynamic casts;
>> > remove an always-true conditional; add a TODO for comparing
>> > array types more accurately.
>> > * tests/data/Makefile.am: Add new test case files.
>> > * tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc: New
>> > test case.
>> > * tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc: Ditto.
>> > * tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
>> > Ditto.
>> > * tests/test-abidiff-exit.cc: Run new test case.
>> >
>> >Signed-off-by: Giuliano Procida <gprocida@google.com>
>> >---
>> > include/abg-fwd.h | 6 +-
>> > src/abg-ir.cc | 147 ++++++++++--------
>> > tests/data/Makefile.am | 5 +
>> > .../test-leaf-peeling-report.txt | 38 +++++
>> > .../test-abidiff-exit/test-leaf-peeling-v0.cc | 24 +++
>> > .../test-abidiff-exit/test-leaf-peeling-v0.o | Bin 0 -> 3704 bytes
>> > .../test-abidiff-exit/test-leaf-peeling-v1.cc | 24 +++
>> > .../test-abidiff-exit/test-leaf-peeling-v1.o | Bin 0 -> 3752 bytes
>> > tests/test-abidiff-exit.cc | 9 ++
>> > 9 files changed, 182 insertions(+), 71 deletions(-)
>> > create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>> > create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
>> > create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v0.o
>> > create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
>> > create mode 100644 tests/data/test-abidiff-exit/test-leaf-peeling-v1.o
>> >
>> >diff --git a/include/abg-fwd.h b/include/abg-fwd.h
>> >index 6f2a862c..1aab70a6 100644
>> >--- a/include/abg-fwd.h
>> >+++ b/include/abg-fwd.h
>> >@@ -1293,11 +1293,13 @@ function_decl_is_less_than(const function_decl&f, const function_decl &s);
>> >
>> > bool
>> > types_have_similar_structure(const type_base_sptr& first,
>> >- const type_base_sptr& second);
>> >+ const type_base_sptr& second,
>> >+ bool indirect_type = false);
>> >
>> > bool
>> > types_have_similar_structure(const type_base* first,
>> >- const type_base* second);
>> >+ const type_base* second,
>> >+ bool indirect_type = false);
>> >
>> > } // end namespace ir
>> >
>> >diff --git a/src/abg-ir.cc b/src/abg-ir.cc
>> >index 2853fe6c..a10b0bb7 100644
>> >--- a/src/abg-ir.cc
>> >+++ b/src/abg-ir.cc
>> >@@ -13573,10 +13573,10 @@ reference_type_def::reference_type_def(const type_base_sptr pointed_to,
>> > decl_base_sptr pto = dynamic_pointer_cast<decl_base>(pointed_to);
>> > string name;
>> > if (pto)
>> >- {
>> >- set_visibility(pto->get_visibility());
>> >- name = string(pto->get_name()) + "&";
>> >- }
>> >+ {
>> >+ set_visibility(pto->get_visibility());
>> >+ name = string(pto->get_name()) + "&";
>> >+ }
>>
>> Correct, but unrelated. No complaints. :-)
>>
>> > else
>> > name = string(get_type_name(is_function_type(pointed_to),
>> > /*qualified_name=*/true)) + "&";
>> >@@ -22563,33 +22563,44 @@ function_decl_is_less_than(const function_decl &f, const function_decl &s)
>> > /// Test if two types have similar structures, even though they are
>> > /// (or can be) different.
>> > ///
>> >-/// Two indirect types (pointers, references, qualified, typedefs)
>> >-/// have similar structure if their underlying types are of the same
>> >-/// kind and have the same name. In this indirect types case, the
>> >-/// size of the underlying type does not matter.
>> >+/// const and volatile qualifiers are completely ignored.
>> >+///
>> >+/// typedef are resolved to their definitions; their names are ignored.
>> >+///
>> >+/// Two indirect types (pointers or references) have similar structure
>> >+/// if their underlying types are of the same kind and have the same
>> >+/// name. In the indirect types case, the size of the underlying type
>> >+/// does not matter.
>> > ///
>> > /// Two direct types (i.e, non indirect) have a similar structure if
>> > /// they have the same kind, name and size. Two class types have
>> >-/// similar structure if they have the same name, size, and if their
>> >-/// data members have similar types.
>> >+/// similar structure if they have the same name, size, and if the
>> >+/// types of their data members have similar types.
>> > ///
>> > /// @param first the first type to consider.
>> > ///
>> > /// @param second the second type to consider.
>> > ///
>> >+/// @param indirect_type whether to do an indirect comparison
>> >+///
>> > /// @return true iff @p first and @p second have similar structures.
>> > bool
>> > types_have_similar_structure(const type_base_sptr& first,
>> >- const type_base_sptr& second)
>> >-{return types_have_similar_structure(first.get(), second.get());}
>> >+ const type_base_sptr& second,
>> >+ bool indirect_type)
>> >+{return types_have_similar_structure(first.get(), second.get(), indirect_type);}
>> >
>> > /// Test if two types have similar structures, even though they are
>> > /// (or can be) different.
>> > ///
>> >-/// Two indirect types (pointers, references, qualified, typedefs)
>> >-/// have similar structure if their underlying types are of the same
>> >-/// kind and have the same name. In this indirect types case, the
>> >-/// size of the underlying type does not matter.
>> >+/// const and volatile qualifiers are completely ignored.
>> >+///
>> >+/// typedef are resolved to their definitions; their names are ignored.
>> >+///
>> >+/// Two indirect types (pointers or references) have similar structure
>> >+/// if their underlying types are of the same kind and have the same
>> >+/// name. In the indirect types case, the size of the underlying type
>> >+/// does not matter.
>> > ///
>> > /// Two direct types (i.e, non indirect) have a similar structure if
>> > /// they have the same kind, name and size. Two class types have
>> >@@ -22600,9 +22611,13 @@ types_have_similar_structure(const type_base_sptr& first,
>> > ///
>> > /// @param second the second type to consider.
>> > ///
>> >+/// @param indirect_type whether to do an indirect comparison
>> >+///
>> > /// @return true iff @p first and @p second have similar structures.
>> > bool
>> >-types_have_similar_structure(const type_base* first, const type_base* second)
>> >+types_have_similar_structure(const type_base* first,
>> >+ const type_base* second,
>> >+ bool indirect_type)
>> > {
>> > if (!!first != !!second)
>> > return false;
>> >@@ -22610,33 +22625,39 @@ types_have_similar_structure(const type_base* first, const type_base* second)
>> > if (!first)
>> > return false;
>> >
>> >- if (is_typedef(first) || is_qualified_type(first))
>> >- first = peel_qualified_or_typedef_type(first);
>> >-
>> >- if (is_typedef(second) || is_qualified_type(second))
>> >- second = peel_qualified_or_typedef_type(second);
>> >+ // Treat typedefs purely as type aliases and ignore CV-qualifiers.
>> >+ first = peel_qualified_or_typedef_type(first);
>> >+ second = peel_qualified_or_typedef_type(second);
>> >
>> >- bool was_indirect_type = (is_pointer_type(first)
>> >- || is_pointer_type(second)
>> >- || is_reference_type(first)
>> >- || is_reference_type(second));
>> >+ // Eliminate all but N of the N^2 comparison cases. This also guarantees the
>> >+ // various ty2 below cannot be null.
>> >+ if (typeid(*first) != typeid(*second))
>> >+ return false;
>>
>> Just curious (because I think it is correct): Does it make a measurable
>> difference?
>
>The code was there before, after the peeling. However, various ty2
>checks were still there, including ones I'd added. Once I went and
>reminded myself what typeid meant, the linear code made a lot more
>sense.
>
>> >
>> >- if (was_indirect_type)
>> >+ // Peel off matching pointers.
>> >+ if (const pointer_type_def* ty1 = is_pointer_type(first))
>>
>> A picky compiler might ask you to add some parentheses here. [-Wparentheses]
>
>-Wparentheses is already enabled by -Wall which is enabled by ABIGAIL_DEVEL=1.
>
>In any case, there is no ambiguity here.
>
>> > {
>> >- first = peel_typedef_pointer_or_reference_type(first);
>> >- second = peel_typedef_pointer_or_reference_type(second);
>> >+ const pointer_type_def* ty2 = is_pointer_type(second);
>> >+ return types_have_similar_structure(ty1->get_pointed_to_type(),
>> >+ ty2->get_pointed_to_type(),
>> >+ true);
>> > }
>> >
>> >- if (typeid(*first) != typeid(*second))
>> >- return false;
>> >+ // Peel off matching references.
>> >+ if (const reference_type_def* ty1 = is_reference_type(first))
>>
>> As above.
>>
>> >+ {
>> >+ const reference_type_def* ty2 = is_reference_type(second);
>> >+ if (ty1->is_lvalue() != ty2->is_lvalue())
>> >+ return false;
>> >+ return types_have_similar_structure(ty1->get_pointed_to_type(),
>> >+ ty2->get_pointed_to_type(),
>> >+ true);
>> >+ }
>> >
>> > if (const type_decl* ty1 = is_type_decl(first))
>> > {
>> > const type_decl* ty2 = is_type_decl(second);
>> >- if (ty2 == 0)
>> >- return false;
>> >-
>> >- if (!was_indirect_type)
>> >+ if (!indirect_type)
>> > if (ty1->get_size_in_bits() != ty2->get_size_in_bits())
>> > return false;
>> >
>> >@@ -22646,10 +22667,7 @@ types_have_similar_structure(const type_base* first, const type_base* second)
>> > if (const enum_type_decl* ty1 = is_enum_type(first))
>> > {
>> > const enum_type_decl* ty2 = is_enum_type(second);
>> >- if (ty2 == 0)
>> >- return false;
>> >-
>> >- if (!was_indirect_type)
>> >+ if (!indirect_type)
>> > if (ty1->get_size_in_bits() != ty2->get_size_in_bits())
>> > return false;
>> >
>> >@@ -22660,20 +22678,16 @@ types_have_similar_structure(const type_base* first, const type_base* second)
>> > if (const class_decl* ty1 = is_class_type(first))
>> > {
>> > const class_decl* ty2 = is_class_type(second);
>> >- if (ty2 == 0)
>> >- return false;
>> >-
>> > if (!ty1->get_is_anonymous() && !ty2->get_is_anonymous()
>> > && ty1->get_name() != ty2->get_name())
>> > return false;
>> >
>> >- if (!was_indirect_type)
>> >+ if (!indirect_type)
>> > {
>> >- if (!was_indirect_type)
>> >- if ((ty1->get_size_in_bits() != ty2->get_size_in_bits())
>> >- || (ty1->get_non_static_data_members().size()
>> >- != ty2->get_non_static_data_members().size()))
>> >- return false;
>> >+ if ((ty1->get_size_in_bits() != ty2->get_size_in_bits())
>> >+ || (ty1->get_non_static_data_members().size()
>> >+ != ty2->get_non_static_data_members().size()))
>> >+ return false;
>> >
>> > for (class_or_union::data_members::const_iterator
>> > i = ty1->get_non_static_data_members().begin(),
>> >@@ -22685,7 +22699,8 @@ types_have_similar_structure(const type_base* first, const type_base* second)
>> > var_decl_sptr dm1 = *i;
>> > var_decl_sptr dm2 = *j;
>> > if (!types_have_similar_structure(dm1->get_type().get(),
>> >- dm2->get_type().get()))
>> >+ dm2->get_type().get(),
>> >+ indirect_type))
>> > return false;
>> > }
>> > }
>> >@@ -22696,14 +22711,11 @@ types_have_similar_structure(const type_base* first, const type_base* second)
>> > if (const union_decl* ty1 = is_union_type(first))
>> > {
>> > const union_decl* ty2 = is_union_type(second);
>> >- if (ty2 == 0)
>> >- return false;
>> >-
>> > if (!ty1->get_is_anonymous() && !ty2->get_is_anonymous()
>> > && ty1->get_name() != ty2->get_name())
>> > return false;
>> >
>> >- if (!was_indirect_type)
>> >+ if (!indirect_type)
>> > return ty1->get_size_in_bits() == ty2->get_size_in_bits();
>> >
>> > return true;
>> >@@ -22712,13 +22724,13 @@ types_have_similar_structure(const type_base* first, const type_base* second)
>> > if (const array_type_def* ty1 = is_array_type(first))
>> > {
>> > const array_type_def* ty2 = is_array_type(second);
>> >-
>> >- if (!was_indirect_type)
>> >- 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()))
>> >- return false;
>> >+ // TODO: Handle uint32_t[10] vs uint64_t[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))
>> >+ return false;
>> >
>> > return true;
>> > }
>> >@@ -22726,14 +22738,12 @@ types_have_similar_structure(const type_base* first, const type_base* second)
>> > if (const array_type_def::subrange_type *ty1 = is_subrange_type(first))
>> > {
>> > const array_type_def::subrange_type *ty2 = is_subrange_type(second);
>> >- if (!ty2)
>> >- return false;
>> >-
>> > if (ty1->get_upper_bound() != ty2->get_upper_bound()
>> > || ty1->get_lower_bound() != ty2->get_lower_bound()
>> > || ty1->get_language() != ty2->get_language()
>> > || !types_have_similar_structure(ty1->get_underlying_type(),
>> >- ty2->get_underlying_type()))
>> >+ ty2->get_underlying_type(),
>> >+ indirect_type))
>> > return false;
>> >
>> > return true;
>> >@@ -22742,11 +22752,9 @@ types_have_similar_structure(const type_base* first, const type_base* second)
>> > if (const function_type* ty1 = is_function_type(first))
>> > {
>> > const function_type* ty2 = is_function_type(second);
>> >- if (!ty2)
>> >- return false;
>> >-
>> > if (!types_have_similar_structure(ty1->get_return_type(),
>> >- ty2->get_return_type()))
>> >+ ty2->get_return_type(),
>> >+ indirect_type))
>> > return false;
>> >
>> > if (ty1->get_parameters().size() != ty2->get_parameters().size())
>> >@@ -22759,7 +22767,8 @@ types_have_similar_structure(const type_base* first, const type_base* second)
>> > && j != ty2->get_parameters().end());
>> > ++i, ++j)
>> > if (!types_have_similar_structure((*i)->get_type(),
>> >- (*j)->get_type()))
>> >+ (*j)->get_type(),
>> >+ indirect_type))
>> > return false;
>> >
>> > return true;
>> >diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
>> >index cf7792f1..6d4e2faa 100644
>> >--- a/tests/data/Makefile.am
>> >+++ b/tests/data/Makefile.am
>> >@@ -130,6 +130,11 @@ test-abidiff-exit/test-leaf3-v0.o \
>> > test-abidiff-exit/test-leaf3-v1.c \
>> > test-abidiff-exit/test-leaf3-v1.o \
>> > test-abidiff-exit/test-leaf3-report.txt \
>> >+test-abidiff-exit/test-leaf-peeling-v0.cc \
>> >+test-abidiff-exit/test-leaf-peeling-v0.o \
>> >+test-abidiff-exit/test-leaf-peeling-v1.cc \
>> >+test-abidiff-exit/test-leaf-peeling-v1.o \
>> >+test-abidiff-exit/test-leaf-peeling-report.txt \
>> > \
>> > test-diff-dwarf/test0-v0.cc \
>> > test-diff-dwarf/test0-v0.o \
>> >diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>> >new file mode 100644
>> >index 00000000..54a26869
>> >--- /dev/null
>> >+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>> >@@ -0,0 +1,38 @@
>> >+Leaf changes summary: 4 artifacts changed
>> >+Changed leaf types summary: 4 leaf types changed
>> >+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
>> >+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
>> >+
>> >+'struct foo at test-leaf-peeling.0.cc:1:1' changed:
>> >+ type size changed from 32 to 64 (in bits)
>> >+ there are data member changes:
>> >+ type 'int' of 'foo::z' changed:
>> >+ type name changed from 'int' to 'long int'
>> >+ type size changed from 32 to 64 (in bits)
>> >+ and size changed from 32 to 64 (in bits) (by +32 bits)
>> >+
>> >+
>> >+
>> >+'struct ops1 at test-leaf-peeling.0.cc:5:1' changed:
>> >+ type size hasn't changed
>> >+ there are data member changes:
>> >+ type 'int*' of 'ops1::x' changed:
>> >+ pointer type changed from: 'int*' to: 'int**'
>> >+
>> >+
>> >+
>> >+
>> >+'struct ops2 at test-leaf-peeling.0.cc:9:1' changed:
>> >+ type size changed from 320 to 640 (in bits)
>> >+ there are data member changes:
>> >+ 'foo ops2::y[10]' size changed from 320 to 640 (in bits) (by +320 bits)
>> >+
>> >+
>> >+
>> >+'struct ops3 at test-leaf-peeling.0.cc:13:1' changed:
>> >+ type size hasn't changed
>> >+ there are data member changes:
>> >+ type 'void (int&)*' of 'ops3::spong' changed:
>> >+ pointer type changed from: 'void (int&)*' to: 'void (int&&)*'
>> >+
>> >+
>> >diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
>> >new file mode 100644
>> >index 00000000..b927d15e
>> >--- /dev/null
>> >+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
>> >@@ -0,0 +1,24 @@
>> >+struct foo {
>> >+ int z;
>> >+};
>> >+
>> >+struct ops1 {
>> >+ int * x;
>> >+};
>> >+
>> >+struct ops2 {
>> >+ foo y[10];
>> >+};
>> >+
>> >+struct ops3 {
>> >+ void (*spong)(int & wibble);
>> >+};
>> >+
>> >+void register_ops1(ops1*) {
>> >+}
>> >+
>> >+void register_ops2(ops2*) {
>> >+}
>> >+
>> >+void register_ops3(ops3*) {
>> >+}
>> >diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v0.o b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.o
>> >new file mode 100644
>> >index 0000000000000000000000000000000000000000..a556a63560122a5e926ab708bb13f0fd40714638
>> >GIT binary patch
>> >literal 3704
>> >zcmbtXO>7%g5T56a?R7SGoVcZpf{-JD=nuP%ov0+G4TjJ(5v8h-Dx?-j*Us9pu-B1y
>> >z14$7g5<jB1B7p#@NJ!jz;zo~%3aK}495`{|iiEg;IKa&A8|V4izQ9QPX6Bop_ujm>
>> >zv-Z0eFRvwp044z|aHuf~a6fY>S0b?j1(=3w>$iVhzx~dicV2riTznm~jYR=wlsA?S
>> >zlfj%KC+eLJj0s_`A!ytJkw}w-8OD-LKZGR7rbwD~<i;L|RE8vgR1^`JPl$~90h#wf
>> >zjHgKgNZUAkijPS{1~O(EIj75--=Z>3h{%mUBo5a1I84w0*(i4^a>@}Pj-`k(SyB*p
>> >z|3Yb!hQ!C)xTL!%#Q{e~1f0T>Owtesw0Z$kBLZ$v01a`#OBeuRN<5J(SVnF(HzlUi
>> >z`D17;2*b3jbJjD~vLy;wj4T*ttZo#RaK&SfgJrE)hKW@`=bY-Cnnc?MMx}+=C{VaL
>> >z2OEVm7?zP?xUF$anNw3*M+Ny#B+(bEmX}F#j(B#4J^g5jmE)g1KSREg<eNB-n|KPJ
>> >z1nmZJMGDO+vMRoUiUD-2Lj0Y};uu|Gtc-xr3qrf)xpjNT^IA>6;mkX=S`?j}g-kpR
>> >z#pgv*yqBC^>5T~k6SX>iqqyq_&4%x773tb-UN0?ly+$(#z3ygbCn#NAq!EH0bcH@k
>> >zpH}YE79a}3p-j@ksC8bta;>;}`gCcrxa^djQn5T=S}v95%SF2#?)u(^hUa_TW(`%h
>> >zTid>{w{+G%yI8avMZ4|>`+jY^+wnWQfxYX$)%3UQ(5<$-U|<jBXb)vL6Sj8-j0=%*
>> >z0pQZ=>bc^H7rbiI_2cE(rKeA73!};&iOP%Bzx6VRy}X#6NSk-?G#*8q9-`<)c`ld#
>> >zFtM6Vt)T;AWIInrq*3-)WAjkEGz~DE8r#nA{|p!qz(XDeBi_3jeRcya!^n`0O}81)
>> >z_xAS821H$_r?X&SM;b;(yyF3r0oehJ03+@;12zLD1~7W(rj<HGwMT4T;;b`Ym-r*d
>> >zi5MSsiATLMI?mTn$N2_wC8;~JO!M=O2&d{D;6pSOv#6UB-jw=L4gXE*Gm_JdP*4A)
>> >z;;(k3D+*5euS>nH;hR$bT*IXbUuyW4)bDBdDXIUc;g_ZUlZO8x^`9l@eet>Rk#pTv
>> >zq+eRT5{z?ROX8e|&%LDi)4d^D((vgRhVxn;3HQ#F=8Znz(vX*&=VgEH6*^v&4s?d<
>> >zIaV~B*MC#Pd-ZXG{Wi8taDuQK^KF(wZ!ZLA%MD#{szCrw*K4`XmRH?vY&QLR2W^q;
>> >zR;yj_%|5f+b^Qi*VKoH5Odf~e6yVf4?Y4)X8^`a2o`atOI|$v{8#2h--mG`swkH?<
>> >zKiMqCQw`O3ic(f+K$?qLZzNxp$WEBv8C}&$k@ff|(MIppe~m9;te%sm<N2$aQB$Me
>> >z8}w7OLO}|BB@6P444`{Re%u47ZxUU=N6$(40}1{>ibA^YRkZ0Cxql=tFTYQmLic?U
>> >z4a3%dAQ#H<w0`>P=<BC`qvu2j?#uaakfM<8OZ{!w{NKolxK1=b^+|pH9mFUnuM@?H
>> >z3i3j5-(j5RXZ|)ChQ&Wdi$bFNr4q+6e;<u3t9}<{yo!;m`+gxYS+1V@x{TlJA+f;w
>> >zXZRBo$hE?N<PW9ey40zD^dExbxvsPxeQpZ*PR@TI7fjzn`n90P)0yb+9zsx&_x~m-
>> >V3hDc&yu;%2lo;vjnDs_I{$J;aGn)Va
>> >
>> >literal 0
>> >HcmV?d00001
>> >
>> >diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
>> >new file mode 100644
>> >index 00000000..7c88df9f
>> >--- /dev/null
>> >+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
>> >@@ -0,0 +1,24 @@
>> >+struct foo {
>> >+ long z;
>> >+};
>> >+
>> >+struct ops1 {
>> >+ int ** x;
>> >+};
>> >+
>> >+struct ops2 {
>> >+ foo y[10];
>> >+};
>>
>> There is no apparent difference in ops2 between the test files. What are
>> we testing?
>
>We are testing the reporting of an indirect change via its dependency
>on struct foo (which is reported already).
>
>We could (and I would) argue that the change to ops2 is not local and
>people doing kernel ABI diffs might agree as well. However, at the
>moment, size changes are considered as local. If y had been a pointer
>something something foo, the size would not have changed and this
>would not have been reported. I'll add a comment to the test code.
I agree that this should not be considered local then. Dodji, what do
you think?
Cheers,
Matthias
>
>> >+
>> >+struct ops3 {
>> >+ void (*spong)(int && wibble);
>> >+};
>> >+
>> >+void register_ops1(ops1*) {
>> >+}
>> >+
>> >+void register_ops2(ops2*) {
>> >+}
>> >+
>> >+void register_ops3(ops3*) {
>> >+}
>>
>> Maybe we can add some more test cases:
>>
>> - compare reference types and pointer types
>> - compare int*** and int* (two levels of peeling)
>> - compare uint32_t[10] and uint64_t[5] (with the same TODO as in the code)
>> - nested types, e.g. struct ops1_v0* vs. struct ops1_v1*
>
>Agreed. Will do.
>
>> Nice work. I like it!
>>
>> Cheers,
>> Matthias
>
>Thank you,
>Giuliano.
>
>> >diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v1.o b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.o
>> >new file mode 100644
>> >index 0000000000000000000000000000000000000000..e71ade4431f80a78ee73292d60808a2ee5404640
>> >GIT binary patch
>> >literal 3752
>> >zcmbtXO=x3P6h1evNnV>K{Yl4Zsf!f+8^*k8(lRq0ZH=wdX;6nE6lF$fU(%N}O!89l
>> >zUbU@=f+Ear6h#DAiVI!25#8%7MK^*A7lMjz-4zt{ocqpAZ*Ni%J&=3O`Oe?H=l;C*
>> >z)vHgf#)JSx0+!%dQz*digU517iY3UxIBcxl{&ns4D@S);_^Q8n3$qQ#09)pb<l|&8
>> >zqsg&ayA8Gw_9}wbA&8iiFoPW@H#yiMZr8}Z4<a!{5<tRk%CUoEav<LD!^n4C4BBra
>> >zW*vYSN|FR%=W+NDe}i(s+)08tciR3Ml_5ezX6Sq3kop#fVHzN9Q;CFh$`ByVCWt9Y
>> >zu^>K3;BrT3h`qIg0iU2D3Ya)0;1UJU5Ct4`1IA7X*r5O#qJXEVv=|fjXR;|PGo2X|
>> >z<H^ypXw3@CPNlA-9!V{vL>8No2Fs4ra`rOLc;G%rrIs)VyMWFa-8nIWwlYQ~*~!KZ
>> >z3Q^8drokB~XD?ueAxxRkQ>M-cau|-I19okhlVlw6*p%}0<rGVk?=4S}?+E$ECUFrD
>> >z<1a?50W7A`j3(>iv#3}=+bYDLnKX{_L}O<JgkBKlo1R<C?|NRd;n$slQ?1JA_%sYg
>> >zWhj1B#Kr6J>80)%voKM!?bmaAe$c4<-gb`8l|)Fm5Vm$9*hL5Ec=bjQdY#SoZcsR(
>> >z6;Eh$ARTZhk~9a`u5aX4&Yv&L=N6ozQ^*x(3k!w9Y%!N_g?qlYSoeId)2O2AcB(sz
>> >z`<E}~FV5%k^<2K@1_yq1r_=V^dqIBBf2rYb=R>#B^a9wtQJB-GR9v5zB~R&v>0XfT
>> >zg`PBX5}yOOwz6_1_s|nwrQ!O~#Pfwq=Zq!D7Q;luKpZ{;Vt-Uj4=3$AxH0!4PCp`f
>> >zXCBXtz8PCdCsxq`F|s{MMx@j1&qwCIb~g1eERE06AAk245WxNJ2P57+B`53_TKbX2
>> >z51whW;KW7lnk|qIP*+!i1=^#lK;%}mVhxEz3(`Fp0ao<z^w=yI?!oB#jca|7y+>@8
>> >zaiuH1#`rzRi4=a?z5BOkO&wX&U!6hKNqVT2O!d`@2&e9C5{NVwG5N3){tfGA41AjP
>> >zDduz{^xZ$F`Rk8US;Hy+IP2F9e4X_}1K(i%V*_UuJ~i+gtbbwPIo5wP@MYG2Ht@e#
>> >z|Bbm?uiDQvUkTOEdG?zy@Xr~aWL)J@=V8h4e}Mh38u%}amyJ9OclVm+Bfr>e7-g>N
>> >zDu4Bcn0SsIXm9oXEg86)|CWKP{(uu4wD8$Q9(E$WqomN=55d`XLl>M%5P;M1ny$0$
>> >zRrcze4Zqe#o0Q#3rQ^ML!t8cjzm6}t9)ce)PlaF!aH{QA%fk<m<F`Z4!B0s(2;J%}
>> >z4)S(3YaO@c@x=cpqkBO;)bARlEYW~86S3|{zB-YeF!RN9btg?W;~zvD-M9Z5U%*&1
>> >zCzVt6+lJ9l<)0+<r)!CVH2RVoa>N024#`iw4fLK8E#lA2N%(gR-X%pN)0f^26C?L`
>> >z`S8wau_T(lPa@WD{-5zgRXoj~zDDN!xBKKo2<~$Io1|!D)~9#4U;UrBkm?iFr}xvW
>> >zzl#{<RP#hpQjQOVdXE&Y>MQ;-8v4c0)1;8-eCed(6n_JaO4j{ua=ea_Z2G>>7`LnI
>> >zzQytTT_h6J`W5^R3glW+0P`buTw|U3NB?H1c-2>$k6D{Se&G7A@r3C+Nq<F{@w6xA
>> >e-9reT<MY2oibm%8DQ~~{5+z3ZC}Q2wjQ<DlLOH<z
>> >
>> >literal 0
>> >HcmV?d00001
>> >
>> >diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
>> >index 6ecbfd24..ebac7a00 100644
>> >--- a/tests/test-abidiff-exit.cc
>> >+++ b/tests/test-abidiff-exit.cc
>> >@@ -158,6 +158,15 @@ InOutSpec in_out_specs[] =
>> > "data/test-abidiff-exit/test-leaf3-report.txt",
>> > "output/test-abidiff-exit/test-leaf3-report.txt"
>> > },
>> >+ {
>> >+ "data/test-abidiff-exit/test-leaf-peeling-v0.o",
>> >+ "data/test-abidiff-exit/test-leaf-peeling-v1.o",
>> >+ "",
>> >+ "--leaf-changes-only",
>> >+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
>> >+ "data/test-abidiff-exit/test-leaf-peeling-report.txt",
>> >+ "output/test-abidiff-exit/test-leaf-peeling-report.txt"
>> >+ },
>> > {0, 0, 0 ,0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
>> > };
>> >
>> >--
>> >2.25.1.696.g5e7596f4ac-goog
>> >
>> >
Hello Giuliano,
Giuliano Procida <gprocida@google.com> a ?crit:
> This function is used to determine whether or not type differences are
> "local" and is primarily used in --leaf-changes-only mode. The logic
> has some issues which are addressed by this patch:
>
> - Any number of points-to (*) and refers-to (& and &&) components
> are peeled off the types being compared, rather than checking
> these match in number and kind.
> - This peeling is done with peel_typedef_pointer_or_reference_type
> which also peels any number of CV qualifiers (OK) and
> array-of ([N]) type components (not OK).
> - The function sets a state variable (was_indirect_type) to modify
> the behaviour of downstream comparisons, but this cannot be
> passed through recursive calls.
>
> The effect of the indirect_type flag is to switch to comparisons by
> name: identically named structs don't get introspected. Arguably, a
> more useful behaviour for --leaf-changes-only mode would be to treat
> any change to a named type as non-local, except in the context of the
> definition of that type itself. This would be a more significant
> change in behaviour.
>
> * include/abg-fwd.h (types_have_similar_structure): In both
> overloads, add an indirect_type argument, defaulting to
> false.
> * src/abg-ir.cc (reference_type_def constructor): Tabify.
> (types_have_similar_structure): In both overloads, add an
> indirect_type argument and update documentation text. In the
> type_base_sptr overload, pass indirect_type in the tail
> call. In the type_base* overload, replace was_indirect_type
> with indirect_type; peel CV qualifiers and typedefs without
> testing as the peel function does this; replace the
> indiscriminate peeling of qualifier/pointer/reference/array
> type components with code that checks the same
> pointer/reference/array type component is used on each side
> and makes recursive calls with indirect_type set to true; pass
> the indirect_type argument recursively when comparing other
> subtypes; move the typeid check earlier, document its purpose
> and remove unneccessary checks after later dynamic casts;
> remove an always-true conditional; add a TODO for comparing
> array types more accurately.
> * tests/data/Makefile.am: Add new test case files.
> * tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc: New
> test case.
> * tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc: Ditto.
> * tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
> Ditto.
> * tests/test-abidiff-exit.cc: Run new test case.
Applied to master.
Thanks!
@@ -1293,11 +1293,13 @@ function_decl_is_less_than(const function_decl&f, const function_decl &s);
bool
types_have_similar_structure(const type_base_sptr& first,
- const type_base_sptr& second);
+ const type_base_sptr& second,
+ bool indirect_type = false);
bool
types_have_similar_structure(const type_base* first,
- const type_base* second);
+ const type_base* second,
+ bool indirect_type = false);
} // end namespace ir
@@ -13573,10 +13573,10 @@ reference_type_def::reference_type_def(const type_base_sptr pointed_to,
decl_base_sptr pto = dynamic_pointer_cast<decl_base>(pointed_to);
string name;
if (pto)
- {
- set_visibility(pto->get_visibility());
- name = string(pto->get_name()) + "&";
- }
+ {
+ set_visibility(pto->get_visibility());
+ name = string(pto->get_name()) + "&";
+ }
else
name = string(get_type_name(is_function_type(pointed_to),
/*qualified_name=*/true)) + "&";
@@ -22563,33 +22563,44 @@ function_decl_is_less_than(const function_decl &f, const function_decl &s)
/// Test if two types have similar structures, even though they are
/// (or can be) different.
///
-/// Two indirect types (pointers, references, qualified, typedefs)
-/// have similar structure if their underlying types are of the same
-/// kind and have the same name. In this indirect types case, the
-/// size of the underlying type does not matter.
+/// const and volatile qualifiers are completely ignored.
+///
+/// typedef are resolved to their definitions; their names are ignored.
+///
+/// Two indirect types (pointers or references) have similar structure
+/// if their underlying types are of the same kind and have the same
+/// name. In the indirect types case, the size of the underlying type
+/// does not matter.
///
/// Two direct types (i.e, non indirect) have a similar structure if
/// they have the same kind, name and size. Two class types have
-/// similar structure if they have the same name, size, and if their
-/// data members have similar types.
+/// similar structure if they have the same name, size, and if the
+/// types of their data members have similar types.
///
/// @param first the first type to consider.
///
/// @param second the second type to consider.
///
+/// @param indirect_type whether to do an indirect comparison
+///
/// @return true iff @p first and @p second have similar structures.
bool
types_have_similar_structure(const type_base_sptr& first,
- const type_base_sptr& second)
-{return types_have_similar_structure(first.get(), second.get());}
+ const type_base_sptr& second,
+ bool indirect_type)
+{return types_have_similar_structure(first.get(), second.get(), indirect_type);}
/// Test if two types have similar structures, even though they are
/// (or can be) different.
///
-/// Two indirect types (pointers, references, qualified, typedefs)
-/// have similar structure if their underlying types are of the same
-/// kind and have the same name. In this indirect types case, the
-/// size of the underlying type does not matter.
+/// const and volatile qualifiers are completely ignored.
+///
+/// typedef are resolved to their definitions; their names are ignored.
+///
+/// Two indirect types (pointers or references) have similar structure
+/// if their underlying types are of the same kind and have the same
+/// name. In the indirect types case, the size of the underlying type
+/// does not matter.
///
/// Two direct types (i.e, non indirect) have a similar structure if
/// they have the same kind, name and size. Two class types have
@@ -22600,9 +22611,13 @@ types_have_similar_structure(const type_base_sptr& first,
///
/// @param second the second type to consider.
///
+/// @param indirect_type whether to do an indirect comparison
+///
/// @return true iff @p first and @p second have similar structures.
bool
-types_have_similar_structure(const type_base* first, const type_base* second)
+types_have_similar_structure(const type_base* first,
+ const type_base* second,
+ bool indirect_type)
{
if (!!first != !!second)
return false;
@@ -22610,33 +22625,39 @@ types_have_similar_structure(const type_base* first, const type_base* second)
if (!first)
return false;
- if (is_typedef(first) || is_qualified_type(first))
- first = peel_qualified_or_typedef_type(first);
-
- if (is_typedef(second) || is_qualified_type(second))
- second = peel_qualified_or_typedef_type(second);
+ // Treat typedefs purely as type aliases and ignore CV-qualifiers.
+ first = peel_qualified_or_typedef_type(first);
+ second = peel_qualified_or_typedef_type(second);
- bool was_indirect_type = (is_pointer_type(first)
- || is_pointer_type(second)
- || is_reference_type(first)
- || is_reference_type(second));
+ // Eliminate all but N of the N^2 comparison cases. This also guarantees the
+ // various ty2 below cannot be null.
+ if (typeid(*first) != typeid(*second))
+ return false;
- if (was_indirect_type)
+ // Peel off matching pointers.
+ if (const pointer_type_def* ty1 = is_pointer_type(first))
{
- first = peel_typedef_pointer_or_reference_type(first);
- second = peel_typedef_pointer_or_reference_type(second);
+ const pointer_type_def* ty2 = is_pointer_type(second);
+ return types_have_similar_structure(ty1->get_pointed_to_type(),
+ ty2->get_pointed_to_type(),
+ true);
}
- if (typeid(*first) != typeid(*second))
- return false;
+ // Peel off matching references.
+ if (const reference_type_def* ty1 = is_reference_type(first))
+ {
+ const reference_type_def* ty2 = is_reference_type(second);
+ if (ty1->is_lvalue() != ty2->is_lvalue())
+ return false;
+ return types_have_similar_structure(ty1->get_pointed_to_type(),
+ ty2->get_pointed_to_type(),
+ true);
+ }
if (const type_decl* ty1 = is_type_decl(first))
{
const type_decl* ty2 = is_type_decl(second);
- if (ty2 == 0)
- return false;
-
- if (!was_indirect_type)
+ if (!indirect_type)
if (ty1->get_size_in_bits() != ty2->get_size_in_bits())
return false;
@@ -22646,10 +22667,7 @@ types_have_similar_structure(const type_base* first, const type_base* second)
if (const enum_type_decl* ty1 = is_enum_type(first))
{
const enum_type_decl* ty2 = is_enum_type(second);
- if (ty2 == 0)
- return false;
-
- if (!was_indirect_type)
+ if (!indirect_type)
if (ty1->get_size_in_bits() != ty2->get_size_in_bits())
return false;
@@ -22660,20 +22678,16 @@ types_have_similar_structure(const type_base* first, const type_base* second)
if (const class_decl* ty1 = is_class_type(first))
{
const class_decl* ty2 = is_class_type(second);
- if (ty2 == 0)
- return false;
-
if (!ty1->get_is_anonymous() && !ty2->get_is_anonymous()
&& ty1->get_name() != ty2->get_name())
return false;
- if (!was_indirect_type)
+ if (!indirect_type)
{
- if (!was_indirect_type)
- if ((ty1->get_size_in_bits() != ty2->get_size_in_bits())
- || (ty1->get_non_static_data_members().size()
- != ty2->get_non_static_data_members().size()))
- return false;
+ if ((ty1->get_size_in_bits() != ty2->get_size_in_bits())
+ || (ty1->get_non_static_data_members().size()
+ != ty2->get_non_static_data_members().size()))
+ return false;
for (class_or_union::data_members::const_iterator
i = ty1->get_non_static_data_members().begin(),
@@ -22685,7 +22699,8 @@ types_have_similar_structure(const type_base* first, const type_base* second)
var_decl_sptr dm1 = *i;
var_decl_sptr dm2 = *j;
if (!types_have_similar_structure(dm1->get_type().get(),
- dm2->get_type().get()))
+ dm2->get_type().get(),
+ indirect_type))
return false;
}
}
@@ -22696,14 +22711,11 @@ types_have_similar_structure(const type_base* first, const type_base* second)
if (const union_decl* ty1 = is_union_type(first))
{
const union_decl* ty2 = is_union_type(second);
- if (ty2 == 0)
- return false;
-
if (!ty1->get_is_anonymous() && !ty2->get_is_anonymous()
&& ty1->get_name() != ty2->get_name())
return false;
- if (!was_indirect_type)
+ if (!indirect_type)
return ty1->get_size_in_bits() == ty2->get_size_in_bits();
return true;
@@ -22712,13 +22724,13 @@ types_have_similar_structure(const type_base* first, const type_base* second)
if (const array_type_def* ty1 = is_array_type(first))
{
const array_type_def* ty2 = is_array_type(second);
-
- if (!was_indirect_type)
- 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()))
- return false;
+ // TODO: Handle uint32_t[10] vs uint64_t[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))
+ return false;
return true;
}
@@ -22726,14 +22738,12 @@ types_have_similar_structure(const type_base* first, const type_base* second)
if (const array_type_def::subrange_type *ty1 = is_subrange_type(first))
{
const array_type_def::subrange_type *ty2 = is_subrange_type(second);
- if (!ty2)
- return false;
-
if (ty1->get_upper_bound() != ty2->get_upper_bound()
|| ty1->get_lower_bound() != ty2->get_lower_bound()
|| ty1->get_language() != ty2->get_language()
|| !types_have_similar_structure(ty1->get_underlying_type(),
- ty2->get_underlying_type()))
+ ty2->get_underlying_type(),
+ indirect_type))
return false;
return true;
@@ -22742,11 +22752,9 @@ types_have_similar_structure(const type_base* first, const type_base* second)
if (const function_type* ty1 = is_function_type(first))
{
const function_type* ty2 = is_function_type(second);
- if (!ty2)
- return false;
-
if (!types_have_similar_structure(ty1->get_return_type(),
- ty2->get_return_type()))
+ ty2->get_return_type(),
+ indirect_type))
return false;
if (ty1->get_parameters().size() != ty2->get_parameters().size())
@@ -22759,7 +22767,8 @@ types_have_similar_structure(const type_base* first, const type_base* second)
&& j != ty2->get_parameters().end());
++i, ++j)
if (!types_have_similar_structure((*i)->get_type(),
- (*j)->get_type()))
+ (*j)->get_type(),
+ indirect_type))
return false;
return true;
@@ -130,6 +130,11 @@ test-abidiff-exit/test-leaf3-v0.o \
test-abidiff-exit/test-leaf3-v1.c \
test-abidiff-exit/test-leaf3-v1.o \
test-abidiff-exit/test-leaf3-report.txt \
+test-abidiff-exit/test-leaf-peeling-v0.cc \
+test-abidiff-exit/test-leaf-peeling-v0.o \
+test-abidiff-exit/test-leaf-peeling-v1.cc \
+test-abidiff-exit/test-leaf-peeling-v1.o \
+test-abidiff-exit/test-leaf-peeling-report.txt \
\
test-diff-dwarf/test0-v0.cc \
test-diff-dwarf/test0-v0.o \
new file mode 100644
@@ -0,0 +1,38 @@
+Leaf changes summary: 4 artifacts changed
+Changed leaf types summary: 4 leaf types changed
+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
+
+'struct foo at test-leaf-peeling.0.cc:1:1' changed:
+ type size changed from 32 to 64 (in bits)
+ there are data member changes:
+ type 'int' of 'foo::z' changed:
+ type name changed from 'int' to 'long int'
+ type size changed from 32 to 64 (in bits)
+ and size changed from 32 to 64 (in bits) (by +32 bits)
+
+
+
+'struct ops1 at test-leaf-peeling.0.cc:5:1' changed:
+ type size hasn't changed
+ there are data member changes:
+ type 'int*' of 'ops1::x' changed:
+ pointer type changed from: 'int*' to: 'int**'
+
+
+
+
+'struct ops2 at test-leaf-peeling.0.cc:9:1' changed:
+ type size changed from 320 to 640 (in bits)
+ there are data member changes:
+ 'foo ops2::y[10]' size changed from 320 to 640 (in bits) (by +320 bits)
+
+
+
+'struct ops3 at test-leaf-peeling.0.cc:13:1' changed:
+ type size hasn't changed
+ there are data member changes:
+ type 'void (int&)*' of 'ops3::spong' changed:
+ pointer type changed from: 'void (int&)*' to: 'void (int&&)*'
+
+
new file mode 100644
@@ -0,0 +1,24 @@
+struct foo {
+ int z;
+};
+
+struct ops1 {
+ int * x;
+};
+
+struct ops2 {
+ foo y[10];
+};
+
+struct ops3 {
+ void (*spong)(int & wibble);
+};
+
+void register_ops1(ops1*) {
+}
+
+void register_ops2(ops2*) {
+}
+
+void register_ops3(ops3*) {
+}
new file mode 100644
GIT binary patch
literal 3704
zcmbtXO>7%g5T56a?R7SGoVcZpf{-JD=nuP%ov0+G4TjJ(5v8h-Dx?-j*Us9pu-B1y
z14$7g5<jB1B7p#@NJ!jz;zo~%3aK}495`{|iiEg;IKa&A8|V4izQ9QPX6Bop_ujm>
zv-Z0eFRvwp044z|aHuf~a6fY>S0b?j1(=3w>$iVhzx~dicV2riTznm~jYR=wlsA?S
zlfj%KC+eLJj0s_`A!ytJkw}w-8OD-LKZGR7rbwD~<i;L|RE8vgR1^`JPl$~90h#wf
zjHgKgNZUAkijPS{1~O(EIj75--=Z>3h{%mUBo5a1I84w0*(i4^a>@}Pj-`k(SyB*p
z|3Yb!hQ!C)xTL!%#Q{e~1f0T>Owtesw0Z$kBLZ$v01a`#OBeuRN<5J(SVnF(HzlUi
z`D17;2*b3jbJjD~vLy;wj4T*ttZo#RaK&SfgJrE)hKW@`=bY-Cnnc?MMx}+=C{VaL
z2OEVm7?zP?xUF$anNw3*M+Ny#B+(bEmX}F#j(B#4J^g5jmE)g1KSREg<eNB-n|KPJ
z1nmZJMGDO+vMRoUiUD-2Lj0Y};uu|Gtc-xr3qrf)xpjNT^IA>6;mkX=S`?j}g-kpR
z#pgv*yqBC^>5T~k6SX>iqqyq_&4%x773tb-UN0?ly+$(#z3ygbCn#NAq!EH0bcH@k
zpH}YE79a}3p-j@ksC8bta;>;}`gCcrxa^djQn5T=S}v95%SF2#?)u(^hUa_TW(`%h
zTid>{w{+G%yI8avMZ4|>`+jY^+wnWQfxYX$)%3UQ(5<$-U|<jBXb)vL6Sj8-j0=%*
z0pQZ=>bc^H7rbiI_2cE(rKeA73!};&iOP%Bzx6VRy}X#6NSk-?G#*8q9-`<)c`ld#
zFtM6Vt)T;AWIInrq*3-)WAjkEGz~DE8r#nA{|p!qz(XDeBi_3jeRcya!^n`0O}81)
z_xAS821H$_r?X&SM;b;(yyF3r0oehJ03+@;12zLD1~7W(rj<HGwMT4T;;b`Ym-r*d
zi5MSsiATLMI?mTn$N2_wC8;~JO!M=O2&d{D;6pSOv#6UB-jw=L4gXE*Gm_JdP*4A)
z;;(k3D+*5euS>nH;hR$bT*IXbUuyW4)bDBdDXIUc;g_ZUlZO8x^`9l@eet>Rk#pTv
zq+eRT5{z?ROX8e|&%LDi)4d^D((vgRhVxn;3HQ#F=8Znz(vX*&=VgEH6*^v&4s?d<
zIaV~B*MC#Pd-ZXG{Wi8taDuQK^KF(wZ!ZLA%MD#{szCrw*K4`XmRH?vY&QLR2W^q;
zR;yj_%|5f+b^Qi*VKoH5Odf~e6yVf4?Y4)X8^`a2o`atOI|$v{8#2h--mG`swkH?<
zKiMqCQw`O3ic(f+K$?qLZzNxp$WEBv8C}&$k@ff|(MIppe~m9;te%sm<N2$aQB$Me
z8}w7OLO}|BB@6P444`{Re%u47ZxUU=N6$(40}1{>ibA^YRkZ0Cxql=tFTYQmLic?U
z4a3%dAQ#H<w0`>P=<BC`qvu2j?#uaakfM<8OZ{!w{NKolxK1=b^+|pH9mFUnuM@?H
z3i3j5-(j5RXZ|)ChQ&Wdi$bFNr4q+6e;<u3t9}<{yo!;m`+gxYS+1V@x{TlJA+f;w
zXZRBo$hE?N<PW9ey40zD^dExbxvsPxeQpZ*PR@TI7fjzn`n90P)0yb+9zsx&_x~m-
V3hDc&yu;%2lo;vjnDs_I{$J;aGn)Va
literal 0
HcmV?d00001
new file mode 100644
@@ -0,0 +1,24 @@
+struct foo {
+ long z;
+};
+
+struct ops1 {
+ int ** x;
+};
+
+struct ops2 {
+ foo y[10];
+};
+
+struct ops3 {
+ void (*spong)(int && wibble);
+};
+
+void register_ops1(ops1*) {
+}
+
+void register_ops2(ops2*) {
+}
+
+void register_ops3(ops3*) {
+}
new file mode 100644
GIT binary patch
literal 3752
zcmbtXO=x3P6h1evNnV>K{Yl4Zsf!f+8^*k8(lRq0ZH=wdX;6nE6lF$fU(%N}O!89l
zUbU@=f+Ear6h#DAiVI!25#8%7MK^*A7lMjz-4zt{ocqpAZ*Ni%J&=3O`Oe?H=l;C*
z)vHgf#)JSx0+!%dQz*digU517iY3UxIBcxl{&ns4D@S);_^Q8n3$qQ#09)pb<l|&8
zqsg&ayA8Gw_9}wbA&8iiFoPW@H#yiMZr8}Z4<a!{5<tRk%CUoEav<LD!^n4C4BBra
zW*vYSN|FR%=W+NDe}i(s+)08tciR3Ml_5ezX6Sq3kop#fVHzN9Q;CFh$`ByVCWt9Y
zu^>K3;BrT3h`qIg0iU2D3Ya)0;1UJU5Ct4`1IA7X*r5O#qJXEVv=|fjXR;|PGo2X|
z<H^ypXw3@CPNlA-9!V{vL>8No2Fs4ra`rOLc;G%rrIs)VyMWFa-8nIWwlYQ~*~!KZ
z3Q^8drokB~XD?ueAxxRkQ>M-cau|-I19okhlVlw6*p%}0<rGVk?=4S}?+E$ECUFrD
z<1a?50W7A`j3(>iv#3}=+bYDLnKX{_L}O<JgkBKlo1R<C?|NRd;n$slQ?1JA_%sYg
zWhj1B#Kr6J>80)%voKM!?bmaAe$c4<-gb`8l|)Fm5Vm$9*hL5Ec=bjQdY#SoZcsR(
z6;Eh$ARTZhk~9a`u5aX4&Yv&L=N6ozQ^*x(3k!w9Y%!N_g?qlYSoeId)2O2AcB(sz
z`<E}~FV5%k^<2K@1_yq1r_=V^dqIBBf2rYb=R>#B^a9wtQJB-GR9v5zB~R&v>0XfT
zg`PBX5}yOOwz6_1_s|nwrQ!O~#Pfwq=Zq!D7Q;luKpZ{;Vt-Uj4=3$AxH0!4PCp`f
zXCBXtz8PCdCsxq`F|s{MMx@j1&qwCIb~g1eERE06AAk245WxNJ2P57+B`53_TKbX2
z51whW;KW7lnk|qIP*+!i1=^#lK;%}mVhxEz3(`Fp0ao<z^w=yI?!oB#jca|7y+>@8
zaiuH1#`rzRi4=a?z5BOkO&wX&U!6hKNqVT2O!d`@2&e9C5{NVwG5N3){tfGA41AjP
zDduz{^xZ$F`Rk8US;Hy+IP2F9e4X_}1K(i%V*_UuJ~i+gtbbwPIo5wP@MYG2Ht@e#
z|Bbm?uiDQvUkTOEdG?zy@Xr~aWL)J@=V8h4e}Mh38u%}amyJ9OclVm+Bfr>e7-g>N
zDu4Bcn0SsIXm9oXEg86)|CWKP{(uu4wD8$Q9(E$WqomN=55d`XLl>M%5P;M1ny$0$
zRrcze4Zqe#o0Q#3rQ^ML!t8cjzm6}t9)ce)PlaF!aH{QA%fk<m<F`Z4!B0s(2;J%}
z4)S(3YaO@c@x=cpqkBO;)bARlEYW~86S3|{zB-YeF!RN9btg?W;~zvD-M9Z5U%*&1
zCzVt6+lJ9l<)0+<r)!CVH2RVoa>N024#`iw4fLK8E#lA2N%(gR-X%pN)0f^26C?L`
z`S8wau_T(lPa@WD{-5zgRXoj~zDDN!xBKKo2<~$Io1|!D)~9#4U;UrBkm?iFr}xvW
zzl#{<RP#hpQjQOVdXE&Y>MQ;-8v4c0)1;8-eCed(6n_JaO4j{ua=ea_Z2G>>7`LnI
zzQytTT_h6J`W5^R3glW+0P`buTw|U3NB?H1c-2>$k6D{Se&G7A@r3C+Nq<F{@w6xA
e-9reT<MY2oibm%8DQ~~{5+z3ZC}Q2wjQ<DlLOH<z
literal 0
HcmV?d00001
@@ -158,6 +158,15 @@ InOutSpec in_out_specs[] =
"data/test-abidiff-exit/test-leaf3-report.txt",
"output/test-abidiff-exit/test-leaf3-report.txt"
},
+ {
+ "data/test-abidiff-exit/test-leaf-peeling-v0.o",
+ "data/test-abidiff-exit/test-leaf-peeling-v1.o",
+ "",
+ "--leaf-changes-only",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+ "data/test-abidiff-exit/test-leaf-peeling-report.txt",
+ "output/test-abidiff-exit/test-leaf-peeling-report.txt"
+ },
{0, 0, 0 ,0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
};