Re: [PATCH 2/2] abidiff: Fix anonymous member size change reports.

[Matthias Maennich <maennich@google.com>]

On Fri, Apr 03, 2020 at 10:53:55PM +0100, Android Kernel Team wrote:
>Data members have names (unless anonymous), types, sizes and
>offsets (if a member of a class or struct). It is the responsibility
>of the represent function to detail all differences of a changed
>member. The size of a member is the same as the size of its type.
>
>A recent change to the function focused on cleaning up the formatting
>logic and added a catch-all case to the end of it in case no diff
>information had been emitted by then.
>
>This catch-all triggered for anonymous structs and union diffs under
>certain circumstances when these anonymous members had changed in
>size.
>
>This patch ensures size change information for a member is emitted
>exactly once (unless the type change has been or is being reported
>already). It also ensures anonymous members are identified
>appropriately and includes a test case would otherwise reach the
>catch-all case in both default and --leaf-changes-only modes.
>
>	* src/abg-reporter-priv.cc (represent): In the var_diff_sptr
>	overload, factor out some expressions as local variables, rely
>	on diff_to_be_reported to decide whether to emit a change,
>	fold together local/non-local change reporting using
>	local_changes to preserve current name formatting differences,
>	keep track explicitly of whether size information has been
>	emitted and ensure it happens if needed, make offset and size
>	change reporting for anonymous data members more meaningful.
>	* tests/test-abidiff-exit.cc: Run new test cases.
>	* tests/data/Makefile.am: Add new test files.
>	* tests/data/test-abidiff-exit/test-member-size-v0.cc: New
>	test.
>	* tests/data/test-abidiff-exit/test-member-size-v0.o: Ditto.
>	* tests/data/test-abidiff-exit/test-member-size-v1.cc: Ditto.
>	* tests/data/test-abidiff-exit/test-member-size-v1.o: Ditto.
>	* tests/data/test-abidiff-exit/test-member-size-report0.txt:
>	New test, default mode.
>	* tests/data/test-abidiff-exit/test-member-size-report1.txt:
>	New test, --leaf-changes-only mode.
>	* tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt:
>	Eliminate duplicate reporting of member sizes.
>	* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
>	Ditto.
>	* tests/data/test-abidiff-exit/test-leaf1-report.txt: Ditto.
>	* tests/data/test-abidiff-exit/test-no-stray-comma-report.txt:
>	Ditto.
>	* tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt:
>	Add size report for anonymous data member.
>	* tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt:
>	Ditto.
>	* tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt:
>	Add missing size change report.
>	* tests/data/test-diff-suppr/test36-leaf-report-0.txt: Remove
>	size change report for previously reported type.
>	* tests/data/test-diff-suppr/test46-PR25128-report-1.txt:
>	Eliminate duplicate reporting of member size change.
>	* tests/data/test-diff-suppr/test46-PR25128-report-2.txt:
>	Ditto.
>

This broke one test case on my machine, that needs trivial adjusting:

test-leaf-peeling-report-indirect.txt:

   | @@ -9,7 +9,6 @@
   |      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-v0.cc:5:1' changed:
   |    type size hasn't changed
   | FAIL runtestabidiffexit (exit status: 1)


>Signed-off-by: Giuliano Procida <gprocida@google.com>
>---
> src/abg-reporter-priv.cc                      |  98 +++++++-----------
> tests/data/Makefile.am                        |   6 ++
> .../test-leaf-cxx-members-report.txt          |   4 +-
> .../test-leaf-peeling-report.txt              |   1 -
> .../test-abidiff-exit/test-leaf1-report.txt   |   1 -
> .../test-member-size-report0.txt              |  27 +++++
> .../test-member-size-report1.txt              |  21 ++++
> .../test-abidiff-exit/test-member-size-v0.cc  |  26 +++++
> .../test-abidiff-exit/test-member-size-v0.o   | Bin 0 -> 3184 bytes
> .../test-abidiff-exit/test-member-size-v1.cc  |  27 +++++
> .../test-abidiff-exit/test-member-size-v1.o   | Bin 0 -> 3192 bytes
> .../test-no-stray-comma-report.txt            |   1 -
> .../test45-anon-dm-change-report-0.txt        |   1 +
> .../test44-anonymous-data-member-report-0.txt |   1 +
> ...-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt |   2 +-
> .../test-diff-suppr/test36-leaf-report-0.txt  |   1 -
> .../test46-PR25128-report-1.txt               |   1 -
> .../test46-PR25128-report-2.txt               |   1 -
> tests/test-abidiff-exit.cc                    |  18 ++++
> 19 files changed, 167 insertions(+), 70 deletions(-)
> create mode 100644 tests/data/test-abidiff-exit/test-member-size-report0.txt
> create mode 100644 tests/data/test-abidiff-exit/test-member-size-report1.txt
> create mode 100644 tests/data/test-abidiff-exit/test-member-size-v0.cc
> create mode 100644 tests/data/test-abidiff-exit/test-member-size-v0.o
> create mode 100644 tests/data/test-abidiff-exit/test-member-size-v1.cc
> create mode 100644 tests/data/test-abidiff-exit/test-member-size-v1.o
>
>diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
>index 323de503..72181367 100644
>--- a/src/abg-reporter-priv.cc
>+++ b/src/abg-reporter-priv.cc
>@@ -411,16 +411,26 @@ represent(const var_diff_sptr	&diff,
>
>   var_decl_sptr o = diff->first_var();
>   var_decl_sptr n = diff->second_var();
>+  string name1 = o->get_qualified_name();
>+  string name2 = n->get_qualified_name();
>+  uint64_t size1 = get_var_size_in_bits(o);
>+  uint64_t size2 = get_var_size_in_bits(n);
>+  uint64_t offset1 = get_data_member_offset(o);
>+  uint64_t offset2 = get_data_member_offset(n);

Those are only used const, hence can be made const.
Also, further down, offset1 feels like to much of an abbreviation.

>
>   // Has the main diff text been output?
>   bool emitted = false;
>   // Are we continuing on a new line? (implies emitted)
>   bool begin_with_and = false;
>-  string name1 = o->get_qualified_name();
>-  string name2 = n->get_qualified_name();
>-  string pretty_representation = o->get_pretty_representation();
>+  // Have we reported a size change already?
>+  bool size_reported = false;
>+  // Are we reporting a change to an anonymous member?
>   bool is_strict_anonymous_data_member_change = false;
>
>+  string pretty_representation = o->get_pretty_representation();
>+  bool show_size_offset_changes = ctxt->get_allowed_category()
>+				  & SIZE_OR_OFFSET_CHANGE_CATEGORY;
>+
>   if (is_anonymous_data_member(o) && is_anonymous_data_member(n))
>     {
>       is_strict_anonymous_data_member_change = true;
>@@ -430,8 +440,7 @@ represent(const var_diff_sptr	&diff,
>       if (tr1 != tr2)
> 	{
> 	  show_offset_or_size(indent + "anonymous data member at offset",
>-			      get_data_member_offset(o),
>-			      *ctxt, out);
>+			      offset1, *ctxt, out);
>
> 	  out << " changed from:\n"
> 	      << indent << "  " << tr1 << "\n"
>@@ -460,7 +469,6 @@ represent(const var_diff_sptr	&diff,
> 	  begin_with_and = true;
> 	  emitted = true;
> 	}
>-      // TODO: else ...?
>     }
>   else if (filtering::has_anonymous_data_member_change(diff))
>     {
>@@ -472,9 +480,7 @@ represent(const var_diff_sptr	&diff,
> 	    + o->get_pretty_representation()
> 	    + " at offset";
>
>-	  show_offset_or_size(indent + repr,
>-			      get_data_member_offset(o),
>-			      *ctxt, out);
>+	  show_offset_or_size(indent + repr, offset1, *ctxt, out);
> 	  out << " became data member '"
> 	      << n->get_pretty_representation()
> 	      << "'\n";
>@@ -486,9 +492,7 @@ represent(const var_diff_sptr	&diff,
> 	    + o->get_pretty_representation()
> 	    + " at offset";
>
>-	  show_offset_or_size(indent + repr,
>-			      get_data_member_offset(o),
>-			      *ctxt, out);
>+	  show_offset_or_size(indent + repr, offset1, *ctxt, out);
> 	  out << " became anonymous data member '"
> 	      << n->get_pretty_representation()
> 	      << "'\n";
>@@ -499,11 +503,15 @@ represent(const var_diff_sptr	&diff,
>     }
>   else if (diff_sptr d = diff->type_diff())
>     {
>-      if (local_only && d->has_local_changes())
>+      if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
> 	{
>-	  out << indent << "type '" << get_pretty_representation(o->get_type())
>-	      << "' of '" << o->get_qualified_name()
>-	      << "' changed";
>+	  if (local_only)
>+	    out << indent << "type '" << get_pretty_representation(o->get_type())
>+		<< "' of '" << o->get_qualified_name()
>+		<< "' changed";
>+	  else
>+	    out << indent
>+		<< "type of '" << pretty_representation << "' changed";
>
> 	  if (d->currently_reporting())
> 	    {
>@@ -521,30 +529,7 @@ represent(const var_diff_sptr	&diff,
>
> 	  begin_with_and = true;
> 	  emitted = true;
>-	}
>-      else
>-	{
>-	  if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
>-	    {
>-	      out << indent
>-		  << "type of '" << pretty_representation << "' changed";
>-	      if (d->currently_reporting())
>-		{
>-		  out << ", as being reported\n";
>-		}
>-	      else if (d->reported_once())
>-		{
>-		  out << ", as reported earlier\n";
>-		}
>-	      else
>-		{
>-		  out << ":\n";
>-		  d->report(out, indent + "  ");
>-		}
>-
>-	      begin_with_and = true;
>-	      emitted = true;
>-	    }
>+	  size_reported = true;
> 	}
>     }
>
>@@ -589,9 +574,9 @@ represent(const var_diff_sptr	&diff,
> 	out << "now becomes laid out";
>       emitted = true;
>     }
>-  if ((ctxt->get_allowed_category() & SIZE_OR_OFFSET_CHANGE_CATEGORY))
>+  if (show_size_offset_changes)
>     {
>-      if (get_data_member_offset(o) != get_data_member_offset(n))
>+      if (offset1 != offset2)
> 	{
> 	  if (begin_with_and)
> 	    {
>@@ -600,26 +585,20 @@ represent(const var_diff_sptr	&diff,
> 	    }
> 	  else if (!emitted)
> 	    {
>+	      out << indent;
> 	      if (is_strict_anonymous_data_member_change)
>-		out << indent;
>-	      else
>-		out << indent << "'" << pretty_representation << "' ";
>+		out << "anonymous data member ";
>+	      out << "'" << pretty_representation << "' ";
> 	    }
> 	  else
> 	    out << ", ";
>
>-	  show_numerical_change("offset",
>-				get_data_member_offset(o),
>-				get_data_member_offset(n),
>-				*ctxt, out);
>+	  show_numerical_change("offset", offset1, offset2, *ctxt, out);
> 	  maybe_show_relative_offset_change(diff, *ctxt, out);
>-
> 	  emitted = true;
> 	}
>-      if (// If we are not displaying only local changes, we must
>-	  // have indicated the type size change already.
>-	  local_only
>-	  && (get_var_size_in_bits(o) != get_var_size_in_bits(n)))
>+
>+      if (!size_reported && size1 != size2)
> 	{
> 	  if (begin_with_and)
> 	    {
>@@ -628,18 +607,15 @@ represent(const var_diff_sptr	&diff,
> 	    }
> 	  else if (!emitted)
> 	    {
>+	      out << indent;
> 	      if (is_strict_anonymous_data_member_change)
>-		out << indent;
>-	      else
>-		out << indent << "'" << pretty_representation << "' ";
>+		out << "anonymous data member ";
>+	      out << "'" << pretty_representation << "' ";
> 	    }
> 	  else
> 	    out << ", ";
>
>-	  show_numerical_change("size",
>-				get_var_size_in_bits(o),
>-				get_var_size_in_bits(n),
>-				*ctxt, out);
>+	  show_numerical_change("size", size1, size2, *ctxt, out);
> 	  maybe_show_relative_size_change(diff, *ctxt, out);
> 	  emitted = true;
> 	}
>diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
>index bb0fd83b..64e36b1a 100644
>--- a/tests/data/Makefile.am
>+++ b/tests/data/Makefile.am
>@@ -140,6 +140,12 @@ test-abidiff-exit/test-leaf-cxx-members-v0.o \
> test-abidiff-exit/test-leaf-cxx-members-v1.cc \
> test-abidiff-exit/test-leaf-cxx-members-v1.o \
> test-abidiff-exit/test-leaf-cxx-members-report.txt \
>+test-abidiff-exit/test-member-size-v0.cc \
>+test-abidiff-exit/test-member-size-v0.o \
>+test-abidiff-exit/test-member-size-v1.cc \
>+test-abidiff-exit/test-member-size-v1.o \
>+test-abidiff-exit/test-member-size-report0.txt \
>+test-abidiff-exit/test-member-size-report1.txt \
> \
> test-diff-dwarf/test0-v0.cc		\
> test-diff-dwarf/test0-v0.o			\
>diff --git a/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt b/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
>index 082db83e..eae74a3b 100644
>--- a/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
>+++ b/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
>@@ -35,8 +35,8 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
>     type 'int' of 'ops::deleted_var' changed:
>       type name changed from 'int' to 'long int'
>       type size changed from 32 to 64 (in bits)
>-    and name of 'ops::deleted_var' changed to 'ops::added_var' at test-leaf-cxx-members-v1.cc:2:1, size changed from 32 to 64 (in bits) (by +32 bits)
>+    and name of 'ops::deleted_var' changed to 'ops::added_var' at test-leaf-cxx-members-v1.cc:2:1
>     type 'int' of 'ops::changed_var' changed:
>       type name changed from 'int' to 'long int'
>       type size changed from 32 to 64 (in bits)
>-    and offset changed from 96 to 128 (in bits) (by +32 bits), size changed from 32 to 64 (in bits) (by +32 bits)
>+    and offset changed from 96 to 128 (in bits) (by +32 bits)
>diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>index bd5f36f8..7ab4dd6e 100644
>--- a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>@@ -9,7 +9,6 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
>     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)

As above, the same patch needs to be applied to
test-leaf-peeling-report-indirect.txt.

>
> 'struct ops1 at test-leaf-peeling-v0.cc:5:1' changed:
>   type size hasn't changed
>diff --git a/tests/data/test-abidiff-exit/test-leaf1-report.txt b/tests/data/test-abidiff-exit/test-leaf1-report.txt
>index 9af6ccaa..aea08b81 100644
>--- a/tests/data/test-abidiff-exit/test-leaf1-report.txt
>+++ b/tests/data/test-abidiff-exit/test-leaf1-report.txt
>@@ -40,4 +40,3 @@ Removed/Changed/Added variables summary: 1 Removed, 1 Changed, 1 Added variable
>     type 'int' of 'changed::foo' 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)
>diff --git a/tests/data/test-abidiff-exit/test-member-size-report0.txt b/tests/data/test-abidiff-exit/test-member-size-report0.txt
>new file mode 100644
>index 00000000..ff93dd50
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-member-size-report0.txt
>@@ -0,0 +1,27 @@
>+Functions changes summary: 0 Removed, 2 Changed, 0 Added functions
>+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>+
>+2 functions with some indirect sub-type change:
>+
>+  [C] 'function void reg1(S*, T*, T*)' at test-member-size-v1.cc:26:1 has some indirect sub-type changes:
>+    parameter 1 of type 'S*' has sub-type changes:
>+      in pointed to type 'struct S' at test-member-size-v1.cc:3:1:
>+        type size changed from 128 to 192 (in bits)
>+        1 data member insertion:
>+          'int S::y', at offset 128 (in bits) at test-member-size-v1.cc:6:1
>+        no data member change (1 filtered);
>+    parameter 2 of type 'T*' has sub-type changes:
>+      in pointed to type 'struct T' at test-member-size-v1.cc:14:1:
>+        type size changed from 192 to 256 (in bits)
>+        2 data member changes:
>+          'S T::s' size changed from 128 to 192 (in bits) (by +64 bits)
>+          'int T::a' offset changed from 128 to 192 (in bits) (by +64 bits)
>+
>+  [C] 'function void reg2(U*)' at test-member-size-v1.cc:27:1 has some indirect sub-type changes:
>+    parameter 1 of type 'U*' has sub-type changes:
>+      in pointed to type 'struct U' at test-member-size-v1.cc:19:1:
>+        type size changed from 192 to 256 (in bits)
>+        2 data member changes:
>+          anonymous data member 'struct {S s;}' size changed from 128 to 192 (in bits) (by +64 bits)
>+          'int U::r' offset changed from 128 to 192 (in bits) (by +64 bits)
>+
>diff --git a/tests/data/test-abidiff-exit/test-member-size-report1.txt b/tests/data/test-abidiff-exit/test-member-size-report1.txt
>new file mode 100644
>index 00000000..dbad76be
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-member-size-report1.txt
>@@ -0,0 +1,21 @@
>+Leaf changes summary: 3 artifacts changed
>+Changed leaf types summary: 3 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 S at test-member-size-v0.cc:3:1' changed:
>+  type size changed from 128 to 192 (in bits)
>+  1 data member insertion:
>+    'int S::y', at offset 128 (in bits) at test-member-size-v1.cc:6:1
>+
>+'struct T at test-member-size-v0.cc:13:1' changed:
>+  type size changed from 192 to 256 (in bits)
>+  there are data member changes:
>+    type 'struct S' of 'T::s' changed, as reported earlier
>+    'int T::a' offset changed from 128 to 192 (in bits) (by +64 bits)
>+
>+'struct U at test-member-size-v0.cc:18:1' changed:
>+  type size changed from 192 to 256 (in bits)
>+  there are data member changes:
>+    anonymous data member 'struct {S s;}' size changed from 128 to 192 (in bits) (by +64 bits)
>+    'int U::r' offset changed from 128 to 192 (in bits) (by +64 bits)
>diff --git a/tests/data/test-abidiff-exit/test-member-size-v0.cc b/tests/data/test-abidiff-exit/test-member-size-v0.cc
>new file mode 100644
>index 00000000..f974a37f
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-member-size-v0.cc
>@@ -0,0 +1,26 @@
>+struct M;
>+
>+struct S {
>+  int x;
>+  M * m;
>+};
>+
>+struct M {
>+  int a;
>+  S * s;
>+};
>+
>+struct T {
>+  S s;
>+  int a;
>+};
>+
>+struct U {
>+  struct {
>+    S s;
>+  };
>+  int r;
>+};
>+
>+void reg1(S*, T*, T*) { }
>+void reg2(U*) { }
>diff --git a/tests/data/test-abidiff-exit/test-member-size-v0.o b/tests/data/test-abidiff-exit/test-member-size-v0.o
>new file mode 100644
>index 0000000000000000000000000000000000000000..e020bef77d57d601420f969061707bbf1486ac99
>GIT binary patch
>literal 3184
>zcmbtWL2MgU5S{P!#_PCtoVcY4LC91fsLC!`E2`o`ifKxds<Z@BqY4SqwX=yW;&rs%
>z6t@VFPzA~@m5>058xj(?a^Q}H#JR_E;{Xyz;Dp2hX8!(hHs8(#M)J=5H}hux|2zM$
>zKYaO>ODQ3MMZkGD7%>WPKXYK$q*;R!OhJ8R@7I;R_y1bi+x>lIum0QqTR$*_<}#*A
>z%V<j`jqQU>@OZ*btwB*3)~g5^eGuskSzvTPn3+CW7NpZ=JBT246$34cFvA0pRe?Pa
>z<Cza+plQ8<+O%wZrmn-Z$Xj1iG=>yR0wr-m5{r<kW5ks85aF?+D2@N9LXT%;C}h5s
>zj1a^lieV8T9vvgal2eBG(!jN5>5;nE!bwo&m@spQP#O>}5`kupge(nG_iieE0~6e(
>zKrtzvDwJ|YVYV<SrZUBo7*)dU%H_`Gp3hZt0@E3JFsyiZxwL>2N<Rs?-1(e=<;@8+
>zFJsVHNhCR$JjAh<BVL>_3Nby9MQYQZU6`T7Srda&(>RT1X_hpfcuCTtNAg7uKr~*%
>zp8>RwLi}CG<5M;pu2Deb_aeLFcN%_Z_u6-S`_`P(Y)ZCy60&g_;!g{6=1+5C(VYA!
>zWnhXUi0g08hyGS&)vW_+mmU>(F6!(+>~UA!wN>}}T4f!Uuhq*-XU@3u<*HM0+;U~k
>zt-9`9wQP5y-N0Ym@&i9?H*x5N&DLUn;jDdjzHD!m?M<(DJ7~7TZqVKB*}K8J?O?->
>zyvDZQ16W>KI#+(?qTgtHL7drktEaUZa)HO?6L;ax>md3?F?%dy?c=UIhB*C5aN_}9
>zDil9XE#=3qVE_)v_X6Q4vg6o)DfY*n;U}iXllaF!!ybA+<Q*16y=mQJAY+sm>a6QN
>z0}l25z->U{QW>B$pKM}uU>fm?4*+EukRQSbFraV*eryP*-kwVAgWP+>PAi<pe9Dh%
>z-l0xQG$j?~ofuEpg!7CDr`2B|5M?q}z_=R12@R)nNiq{Fxst&FLX6fyLV4t6Bu4;0
>zDb4wxR^y*F9IqQGzbH)|L-~1M##IY(o>MArQo~(^pHMjGQShPtF&RW%RJI<^XF<oy
>zDxT(;+=rTm^Zdc--R|HqMjeH*Zm|{l{Ro^5FY>@?^m^cg{<i0A_>JAI^>(n?MW3|2
>zMkDlZ9ddi27i{6pkfdz41D`2a0-R>I)A8}U<^<iyckpVmdy&_CN0I#2`ex{Ld^Pd^
>zsb-N+d&Pao>1xW*?{SJv$`d#sOuqr%P6j1xolpHm{r6w_)IWMoD#!J&XijY?zhCq(
>zq(-EK?5JidD+2W!#qn98`+;N;e|k>Bzfs^PG)RaZyNW&?qwqP^q;m-_nR@IM#7513
>zU%gP~)BNdn)#tx4A}2y{Q`N82AR&7Fb@Yv@|Glcu`$YBW9@FdZAVxWPo@h$>PSyXM
>z0j0S<>-P{J#lNZ=itjH@!#vh^6`%dABfTM&gLxeH1sa5@=`sv`#lJB?VuRPu@TX{u
>z+P{Y*-XDFvn9ut<sy5|*pz62Ogz39MUqziy?_|_c7gYbxqdzJ7`YG=yzD3Q3@>Q$`
>HPo4iS_W$I+
>
>literal 0
>HcmV?d00001
>
>diff --git a/tests/data/test-abidiff-exit/test-member-size-v1.cc b/tests/data/test-abidiff-exit/test-member-size-v1.cc
>new file mode 100644
>index 00000000..b3bd5988
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-member-size-v1.cc
>@@ -0,0 +1,27 @@
>+struct M;
>+
>+struct S {
>+  int x;
>+  M * m;
>+  int y;
>+};
>+
>+struct M {
>+  int a;
>+  S * s;
>+};
>+
>+struct T {
>+  S s;
>+  int a;
>+};
>+
>+struct U {
>+  struct {
>+    S s;
>+  };
>+  int r;
>+};
>+
>+void reg1(S*, T*, T*) { }
>+void reg2(U*) { }

While these are only test cases, I would like for them to have a bit
more descriptive names or a comment for each of them to make it easier
to read the test case.

Other than those little nits, this is a very nice patch:


Reviewed-by: Matthias Maennich <maennich@google.com>
Tested-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>diff --git a/tests/data/test-abidiff-exit/test-member-size-v1.o b/tests/data/test-abidiff-exit/test-member-size-v1.o
>new file mode 100644
>index 0000000000000000000000000000000000000000..eaec9ebead662bc23c00130c537133a5be5c2019
>GIT binary patch
>literal 3192
>zcmbtW&u<%55T57F#_Me3IB}ySq(VjlK~;9iMp4zYq?iWUR6q$$jiO2r*3QPZ8gHug
>zx;RCs2oT68S_z3FapNyQ;?M&JBraUJaYO<nBo6#IA#s42w{Psti*tdIyfg2c`DWgG
>zJMXRUJpbawj1a&g;5-aQi~>B!4(zft%TR<VsI2vVUhBR6*IKXh`&zH^%iY($VF=Bu
>zm}*2uTQX^44>G~SDLb<Pv%;{hBV=?zjAY3IqXojuzK51&?1LDUXaPn>%w`xv<|QbI
>zoJDTK2Qj8NZ4h?0M;tIMAGK*Y_{>~`lE_<MP&9^2m;_2<K@w*nQ^AO$^&`Tgv*P&J
>zcPex`D?=gsm1IO9W)#CBK+KPlVzHGWKFi~>bM(mEYv3fPazc3M0HN3?yi5d|ITCU-
>z$lQBF=}k=V5e14#@mRiS8~OSCq?pPUW-zLVJ7?Qx?Q{0BEij!i4u+KsFBi|?ghn0(
>z+dgj_Sl*mK^D+jF7Dbwq9fu^=a>X-qMn0j3vPgOMlM8c{IA>x|W)`RMB+ZiMlPpPE
>z^hm$cK8VIE_%nd^QHa0u<M@<)hieoN2kqEt1+8ijIql}{z`5nQwVGs`GmuNl5PwdX
>zbAOr>E9T_883R)sL5zaB2V2*dP%o}~6(9{y#jTqP^wzzNb?@57;uc)GQYo#TI^`{u
>zmfc0yD=jW~%bvHeTyk1*Ck$5VK^R2M8V>!a)>!GDIqjTYDmnF%v*Wk-!&)QS3->y0
>zrxU)}47Z)wukHqIfJ>{ZXG>2!7gU>mm}GXm<&#<sfa7wFxCOVbg6I~+{IRTc7q{dD
>z;`AfIT?cqJU-%%iIzD;@18_*brwK=q?Z*C#i9hiieqw4ok$?Pi*h4Reyobe5uUYpP
>z$QUMuI`g{EfCEk6cN>tpT>2=@Czl!xOe0zG08o|z<A*Q;49FjWA3KCoV^5{_LGC?b
>zCl$_PKH^7pZqj?HHcK@l73G~6OWB0;j0mUIUmy@=GEu;|8p5=O)1f4pOO#wm|MVb6
>z>mZ>#@+OiafS;7+{Ew^guNuxh`J2M2V<<oG%d~1C&VzT8loJ~6DZHd`&ZFQ1`(rYQ
>zx}a=5p3j1gmsC8>F})9E4d?lT+um>C8AcsPiEgkJ2i+LlZ9n$Gt+w0XM!~M{ZU@y)
>zeXAMn?4eKEezh6}w+^`d$PepyEu<;C%`jjJmH@Z5*J=g$QFFt+IB@Y^a@w(9yP-%y
>zV{0e!TY;MR|5WqHr@i7n<aA|a=$AOfrsWYF5T@S%ucw1jw$7*iqW=4@eCi)PCza#+
>zuWC+hC_i5GZ=_75lyp_IabHruQ5>HYx*teZ@Tcb_{7VIXNQ0E<vFqs5F$zDYnsgz>
>zB~y=m0kL88zpq{>^J)HcyXy1b9+DFwxS{G-Xpj=U{ucU%)&Ex2=Y693bdTxvZz4uH
>zd7fxW`C8S#&w$cgpY>aa594pDhT{8+(=d<q`-;zg)_3%VR1W5G+^1*|rl!j<yrcM=
>zeIzk>{S1GM#<2bSVaWTVuNU)qUx(GE+~2GE4K-o<ZqQdz=hHhGw$!rf|0VRNMPEPV
>R9mX$Ev!Q&RsQy#u{|i5f=e7U<
>
>literal 0
>HcmV?d00001
>
>diff --git a/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt b/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
>index 1583a40b..52c48b3d 100644
>--- a/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
>+++ b/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
>@@ -10,4 +10,3 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
>       type name changed from 'int[1]' to 'int[2]'
>       array type size changed from 32 to 64
>       array type subrange 1 changed length from 1 to 2
>-    and size changed from 32 to 64 (in bits) (by +32 bits)
>diff --git a/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt b/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
>index 12af287b..2f8f31d0 100644
>--- a/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
>+++ b/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
>@@ -13,6 +13,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>           'char S1::m1' offset changed from 64 to 40 (in bits) (by -24 bits)
>         1 data member change:
>           anonymous data member struct {int m0; char m01;} at offset 0 (in bits) became data member 'int S1::m0'
>+          and size changed from 64 to 32 (in bits) (by -32 bits)
>
>   [C] 'function void foo(S0&)' has some indirect sub-type changes:
>     parameter 1 of type 'S0&' has sub-type changes:
>diff --git a/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt b/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
>index b8bb951c..ed68b550 100644
>--- a/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
>+++ b/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
>@@ -12,5 +12,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>             struct {int b; float c;}
>           to:
>             struct {int b; float c; char e;}
>+          and size changed from 64 to 96 (in bits) (by +32 bits)
>           'int S2::d' offset changed from 96 to 128 (in bits) (by +32 bits)
>
>diff --git a/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt b/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
>index b1611303..2a679d60 100644
>--- a/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
>+++ b/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
>@@ -78,7 +78,7 @@
>                       type name changed from 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl' to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl'
>                       type size changed from 128 to 192 (in bits)
>                       1 data member change:
>-                        name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node'
>+                        name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node', size changed from 128 to 192 (in bits) (by +64 bits)
>                     and name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl'
>
> ================ end of changes of 'libsigc-2.0.so.0.0.0'===============
>diff --git a/tests/data/test-diff-suppr/test36-leaf-report-0.txt b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
>index 9caa9428..fcbb9232 100644
>--- a/tests/data/test-diff-suppr/test36-leaf-report-0.txt
>+++ b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
>@@ -16,7 +16,6 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
>   type size changed from 64 to 96 (in bits)
>   there are data member changes:
>     type 'struct leaf1' of 'leaf2::member0' changed, as reported earlier
>-    and size changed from 32 to 64 (in bits) (by +32 bits)
>     'char leaf2::member1' offset changed from 32 to 64 (in bits) (by +32 bits)
>   3 impacted interfaces:
>     function void interface1(struct_type*)
>diff --git a/tests/data/test-diff-suppr/test46-PR25128-report-1.txt b/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
>index 36439ce7..acff860c 100644
>--- a/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
>+++ b/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
>@@ -13,5 +13,4 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
>     type 'unsigned long int' of 'root_domain::max_cpu_capacity' changed:
>       entity changed from 'unsigned long int' to 'struct max_cpu_capacity' at sched.h:722:1
>       type size changed from 64 to 192 (in bits)
>-    and size changed from 64 to 192 (in bits) (by +128 bits)
>     'perf_domain* root_domain::pd' offset changed from 14528 to 14656 (in bits) (by +128 bits)
>diff --git a/tests/data/test-diff-suppr/test46-PR25128-report-2.txt b/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
>index 67dbf58f..79827103 100644
>--- a/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
>+++ b/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
>@@ -9,5 +9,4 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
>     type 'unsigned long int' of 'root_domain::max_cpu_capacity' changed:
>       entity changed from 'unsigned long int' to 'struct max_cpu_capacity' at sched.h:722:1
>       type size changed from 64 to 192 (in bits)
>-    and size changed from 64 to 192 (in bits) (by +128 bits)
>     'perf_domain* root_domain::pd' offset changed from 14528 to 14656 (in bits) (by +128 bits)
>diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
>index 17523afa..97981a69 100644
>--- a/tests/test-abidiff-exit.cc
>+++ b/tests/test-abidiff-exit.cc
>@@ -177,6 +177,24 @@ InOutSpec in_out_specs[] =
>     "data/test-abidiff-exit/test-leaf-cxx-members-report.txt",
>     "output/test-abidiff-exit/test-leaf-cxx-members-report.txt"
>   },
>+  {
>+    "data/test-abidiff-exit/test-member-size-v0.o",
>+    "data/test-abidiff-exit/test-member-size-v1.o",
>+    "",
>+    "",
>+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
>+    "data/test-abidiff-exit/test-member-size-report0.txt",
>+    "output/test-abidiff-exit/test-member-size-report0.txt"
>+  },
>+  {
>+    "data/test-abidiff-exit/test-member-size-v0.o",
>+    "data/test-abidiff-exit/test-member-size-v1.o",
>+    "",
>+    "--leaf-changes-only",
>+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
>+    "data/test-abidiff-exit/test-member-size-report1.txt",
>+    "output/test-abidiff-exit/test-member-size-report1.txt"
>+  },
>   {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
> };
>
>-- 
>2.26.0.292.g33ef6b2f38-goog
>
>