Re: [PATCH v2] Fix the reporting of leaf change statistics.

public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed

* Re: [PATCH v2] Fix the reporting of leaf change statistics.
  2020-01-01  0:00     ` Matthias Maennich via libabigail
@ 2020-01-01  0:00       ` Dodji Seketeli
  0 siblings, 0 replies; 14+ messages in thread
From: Dodji Seketeli @ 2020-01-01  0:00 UTC (permalink / raw)
  To: Matthias Maennich; +Cc: Giuliano Procida, libabigail, kernel-team

Hello Giuliano, Matthias,

Matthias Maennich <maennich@google.com> a écrit:

> On Mon, Feb 17, 2020 at 05:54:31PM +0000, Matthias Maennich wrote:
>>Hi Giuliano!
>>
>>On Thu, Feb 13, 2020 at 04:19:26PM +0000, Android Kernel Team wrote:
>>>Leaf changes (as reported with --leaf-changes-only) to variables were
>>>miscounted as changes to functions.
>>>
>>>	* src/abg-comparison.cc
>>>	(apply_filters_and_compute_diff_stats):	Increment the correct
>>>	counter for leaf variable changes.

Woops, looks like a copy/paste thinko.  Thanks for catching that!

>>>	* tests/data/Makefile.am: Add test case files.
>>>	* tests/data/test-abidiff-exit/test-leaf0-*: New test case.
>>>	* tests/test-abidiff-exit.cc: Run new test case.
>>>
>>>Signed-off-by: Giuliano Procida <gprocida@google.com>
>>>---
>>>src/abg-comparison.cc                            |   4 ++--
>>>tests/data/Makefile.am                           |   3 +++
>>>.../data/test-abidiff-exit/test-leaf0-report.txt |  13 +++++++++++++
>>>tests/data/test-abidiff-exit/test-leaf0-v0.o     | Bin 0 -> 2784 bytes
>>>tests/data/test-abidiff-exit/test-leaf0-v1.o     | Bin 0 -> 2824 bytes
>>
>>I assume the sources of those are rather small, so can you add the
>>source snippets of the test files to the commit message?

Right.  I try to always add the source of the binaries to the commit
when we have them.

Thanks for pointing that out.

[...]

>>>  stat.num_func_syms_added(added_unrefed_fn_syms_.size());
>>>diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
>>>index 1f574d2e..2a928ef2 100644
>>>--- a/tests/data/Makefile.am
>>>+++ b/tests/data/Makefile.am
>>>@@ -107,6 +107,9 @@ test-abidiff-exit/test-loc-v0.bi \
>>>test-abidiff-exit/test-loc-v1.bi \
>>>test-abidiff-exit/test-loc-with-locs-report.txt \
>>>test-abidiff-exit/test-loc-without-locs-report.txt \
>>>+test-abidiff-exit/test-leaf0-v0.o \
>>>+test-abidiff-exit/test-leaf0-v1.o \
>>>+test-abidiff-exit/test-leaf0.report.txt \
>                              ^^^
> The file is named    test-leaf0-report.txt
>
> With that, `make distcheck` runs successfully as well.

Agreed.

Cheers,

-- 
		Dodji

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] Fix the reporting of leaf change statistics.
  2020-01-01  0:00 [PATCH] Fix the reporting of leaf change statistics Giuliano Procida via libabigail
@ 2020-01-01  0:00 ` Giuliano Procida via libabigail
  2020-01-01  0:00   ` Matthias Maennich via libabigail
  2020-01-01  0:00   ` [PATCH v3] " Giuliano Procida via libabigail
  0 siblings, 2 replies; 14+ messages in thread
From: Giuliano Procida via libabigail @ 2020-01-01  0:00 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida

Leaf changes (as reported with --leaf-changes-only) to variables were
miscounted as changes to functions.

	* src/abg-comparison.cc
	(apply_filters_and_compute_diff_stats):	Increment the correct
	counter for leaf variable changes.
	* tests/data/Makefile.am: Add test case files.
	* tests/data/test-abidiff-exit/test-leaf0-*: New test case.
	* tests/test-abidiff-exit.cc: Run new test case.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-comparison.cc                            |   4 ++--
 tests/data/Makefile.am                           |   3 +++
 .../data/test-abidiff-exit/test-leaf0-report.txt |  13 +++++++++++++
 tests/data/test-abidiff-exit/test-leaf0-v0.o     | Bin 0 -> 2784 bytes
 tests/data/test-abidiff-exit/test-leaf0-v1.o     | Bin 0 -> 2824 bytes
 tests/test-abidiff-exit.cc                       |   9 +++++++++
 6 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-report.txt
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.o
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.o

diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index af4cd5e7..61aea47f 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -9767,8 +9767,8 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat)
 	      (stat.num_leaf_var_changes_filtered_out() + 1);
 	}
       if ((*i)->has_local_changes())
-	stat.num_leaf_func_changes
-	  (stat.num_leaf_func_changes() + 1);
+	stat.num_leaf_var_changes
+	  (stat.num_leaf_var_changes() + 1);
     }
 
   stat.num_func_syms_added(added_unrefed_fn_syms_.size());
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 1f574d2e..2a928ef2 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -107,6 +107,9 @@ test-abidiff-exit/test-loc-v0.bi \
 test-abidiff-exit/test-loc-v1.bi \
 test-abidiff-exit/test-loc-with-locs-report.txt \
 test-abidiff-exit/test-loc-without-locs-report.txt \
+test-abidiff-exit/test-leaf0-v0.o \
+test-abidiff-exit/test-leaf0-v1.o \
+test-abidiff-exit/test-leaf0.report.txt \
 \
 test-diff-dwarf/test0-v0.cc		\
 test-diff-dwarf/test0-v0.o			\
diff --git a/tests/data/test-abidiff-exit/test-leaf0-report.txt b/tests/data/test-abidiff-exit/test-leaf0-report.txt
new file mode 100644
index 00000000..46f39976
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf0-report.txt
@@ -0,0 +1,13 @@
+Leaf changes summary: 2 artifacts changed
+Changed leaf types summary: 0 leaf type changed
+Removed/Changed/Added functions summary: 0 Removed, 1 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 1 Changed, 0 Added variable
+
+1 function with some sub-type change:
+
+  [C]'function int changed_fun()' has some sub-type changes:
+    return type changed:
+      type name changed from 'int' to 'long int'
+      type size changed from 32 to 64 (in bits)
+
+
diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.o b/tests/data/test-abidiff-exit/test-leaf0-v0.o
new file mode 100644
index 0000000000000000000000000000000000000000..a79511cc3c6db039aaa1f5528db19b55f75c7a2d
GIT binary patch
literal 2784
zcmbtVOKTHR6h4#N*w)yPHa=P^GJ;QZ#zabO)oL_a>jM$Ng@VLMa?=ExiA*Ln>Ouq&
z+__LhaOa=UwSUBoKftvMJ!j^g<o0HY3s1<s=X~dR&wb3xtG8BDmIX`}T!5j(D1g0V
z$d@9v1bLW&duwlhekJ+gcWB`>7U?FU5HTMC@sQE@6!DgoF5@&QSRuyJEkBH-SRzd@
z88C4G>oZu>*5PbEBeL_^X=`RYw+|q-03v%fb1t)-xt_l`mzo6-hai)=kP#DDnF|-^
zSnp@A=gF-!`|<J|Mn8%Fs3Jg1jX0qOMeHh$0*>NnS--PWI7U6hWm^!+Zs=@CuTgL|
z3vR6zIi&M287Dw-cY5c|v~@p~Lcdzm^Vem)(&+iX{H7O-M!9zTo?Sk1qPSo$yQkfv
zeY#LwF5u60+F{R^E9=siL92$M7u1?7{iQ``alv-hZKvUNpZT?B(D6IHuG90Mw*0yi
zdesftg;Qbs2~-{wi<}$l*aWy%E}ylJU6Iw6=g0Y+rQ=58XkELb5tse^E?E7XHNR_o
z;vKeOKjQQ!ikf&Sn|qTgPwiMmgBVU6>EMjPuRlpjk$5s`BL<z_;wDg&L_(cXQzt+>
z<eE~zxF!u*0h2>P<3zk-4T)I+QzIAw0<zoSyGHOCM5eWM@f|yW_>9({!NDCljl9ux
z$VNAwJFpYuF;7KsC4|$ej}wSA9Z$eG-noeO82C#i(`6%~eb#r4<53*_e7Izc#PPX~
zb1@vn(Ytz6;cTx&xNG3AmHa@>W<9)iiW5CU$}YE!{o*#bt#DeazFu&<&)RqgaSnrc
zZZa3jeh6;e3q5eF-7dI+-0<AGtoGI`Ex*x0UBr9UY9KeasJ(z*ldUcrEnl(=rU19r
zX}2Z5vaa6=rHk*1(+$1aV--j?D~-TwOI7%PPV?wbz2$y#xRPQ9=TymLtJ6Ug2s3Xo
z@3U@8yy;I*G@U2Y{}ve|@%@RVQMrf;GbY)xf14fEY}%egoN|<uo~N8j9DgYR=v)!R
zXP@uU3SwqVit&vGq}%Z{7){$ds4+1z9#>#t;2ATfE%iC6{#kV?*q`dBSJJG%j+hw}
zAvmMJ8zg9C+E!4Plz&a-=RQ$>dY8=nPY|P+Tqm*-y;6xjFra46&+`k!ll=Fp%Y5V?
z{;!q4VFu^7JO)E8=Vj!R@;A*(3Rq%8HS_uzeveF2{~jgukG?|e&wZtO%-j^aufV1%
mn7#w_4x9e8C+6Kl2!1H=6bTxc>!-L${(V*d7tNPg)Bi7zRJKt7

literal 0
HcmV?d00001

diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.o b/tests/data/test-abidiff-exit/test-leaf0-v1.o
new file mode 100644
index 0000000000000000000000000000000000000000..4a433b9572abb46799391619f21538b54fb3204c
GIT binary patch
literal 2824
zcmbtVOOF#r5UzH!FU(r|0KsYl8k84^%&<nQ)h??AgOh~~5kg2Df`qKG-F_(ccr_kx
zmVlH?K;(eL0SO@?gp@y#3nzX{?#Vd^Qq|LCx7))hWx1>B>w0zdV}AYahuxeIz$D-b
zj5J08ZcdNnk`_y_3`=nD=I?*~N&15i8g1ivL`$<eOe$+29rBi(BUy;)GO{_z6LNNn
zMC~c0&PxVN96-DQu_(SQEazomwXi6bW)@BY<kmqJF6X!NTlu}^t1G!v0P+jS=dYj@
z#th)<3eQJ{y=8JMoqBbB1*3n1e_avGaxTp|;TVX@F8(EajiV6%7Upr(eZ*x8NK`-Z
zJ1T6f`3J?IR@1&y$6+o_fa>w7qqnBSkGUNB)mmZHRP{<@5CO{vVGI|O?ml#SQPXQj
z3Ea4S&nsWJP+a%6f=fZsyR=r^TEm~`cauS+cA6?ual3|U7}r`mhnpMz#=7S>J--q5
zA4auS+>3gHzCVb5Zbx-L39B8|hsyn8k#l1HgN)K|l*^aBbKk0JJB-rY{^oh72)#gB
zmJ!4+cR?I3h}B~=voEkEpCL|vBHhqyg@vcN^8EBJ8l-UMNQdYM{PQtuQf8i1+LU3J
z$J`{&WRWx{)}51J9eQh0!i=SjL<w^v#gb&YVhc${3G)*e0TS@2W{`wq6X+5mi`Kfh
zmuC=PvidVT#1kizH=hpN#9<hA;3&qkyTO$ZPOJV3|3r%^1IC{l{e*+#z0^b(kBIi!
z-nH`@1s#XiL{MPil$Yz+HaNwxyGoaoi0e6>s&LD2#*GSh9sIG;?;Gyc$vvSs`q?se
z3!kvx_yq15oO)pQ0fPR+E?!mSNu2T)ONly6AgG5)2tl>qhags+FsQ5Qpjl~0jULWv
zIjmM=bucy^#`NNhyL8%-Vi!ySL9N&ADtvf?sF$b!pC7-UgtebcAl0fg;;^es;s2AZ
zqCfSP`^n)-Mi`!7qm!*oXHg-{z2Usiwk^r7KRx4g&RqXHWQ=5=!7ROqpc|9y*uP7P
zMXoJ<eCUf;vTR87%m>sn4s@;v;(GWV?I7mHL<ru~fOI=$!_l>+I$ey6^9EcSdZvPF
zOMT9&zho{2`&0e&db;)35p!cA1ltDuo)n8*+X~KQ<=;2?xlfdz-VHbZK4KJ;>qJr0
z6O-sI1BP>c=8q81^8egiW_}+z4Er(v&G_>?^S!ahfYHfby*B=?`5gmnj$F0G>u2~S
z3R(SomeD`@I<Y_ZmDb_rrr6I6*fI;H?*qNhu0QRGd-o86zYTbi6pOfT3{l)H|97VT
KKiJT4*Z)8NIl2)5

literal 0
HcmV?d00001

diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index 4cef727e..bafa7400 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -111,6 +111,15 @@ InOutSpec in_out_specs[] =
     "data/test-abidiff-exit/test-loc-without-locs-report.txt",
     "output/test-abidiff-exit/test-loc-without-locs-report.txt"
   },
+  {
+    "data/test-abidiff-exit/test-leaf0-v0.o",
+    "data/test-abidiff-exit/test-leaf0-v1.o",
+    "",
+    "--no-show-locs --leaf-changes-only",
+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+    "data/test-abidiff-exit/test-leaf0-report.txt",
+    "output/test-abidiff-exit/test-leaf0-report.txt"
+  },
   {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
 };
 
-- 
2.25.0.265.gbab2e86ba0-goog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Fix the reporting of leaf change statistics.
  2020-01-01  0:00 ` [PATCH v2] " Giuliano Procida via libabigail
  2020-01-01  0:00   ` Matthias Maennich via libabigail
@ 2020-01-01  0:00   ` Giuliano Procida via libabigail
  2020-01-01  0:00     ` Matthias Maennich via libabigail
  2020-01-01  0:00     ` [PATCH v4] " Giuliano Procida via libabigail
  1 sibling, 2 replies; 14+ messages in thread
From: Giuliano Procida via libabigail @ 2020-01-01  0:00 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida

Leaf changes (as reported with --leaf-changes-only) to variables were
miscounted as changes to functions.

	* src/abg-comparison.cc
	(apply_filters_and_compute_diff_stats):	Increment the correct
	counter for leaf variable changes.
	* tests/data/Makefile.am: Add test case files.
	* tests/data/test-abidiff-exit/test-leaf0-*: New test case.
	* tests/test-abidiff-exit.cc: Run new test case.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-comparison.cc                            |   4 ++--
 tests/data/Makefile.am                           |   5 +++++
 .../data/test-abidiff-exit/test-leaf0-report.txt |  13 +++++++++++++
 tests/data/test-abidiff-exit/test-leaf0-v0.cc    |   5 +++++
 tests/data/test-abidiff-exit/test-leaf0-v0.o     | Bin 0 -> 2784 bytes
 tests/data/test-abidiff-exit/test-leaf0-v1.cc    |   5 +++++
 tests/data/test-abidiff-exit/test-leaf0-v1.o     | Bin 0 -> 2824 bytes
 tests/test-abidiff-exit.cc                       |   9 +++++++++
 8 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-report.txt
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.cc
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.o
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.cc
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.o

diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 5371e843..ef753e6d 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -9767,8 +9767,8 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat)
 	      (stat.num_leaf_var_changes_filtered_out() + 1);
 	}
       if ((*i)->has_local_changes())
-	stat.num_leaf_func_changes
-	  (stat.num_leaf_func_changes() + 1);
+	stat.num_leaf_var_changes
+	  (stat.num_leaf_var_changes() + 1);
     }
 
   stat.num_func_syms_added(added_unrefed_fn_syms_.size());
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 5031e6d3..07077608 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -110,6 +110,11 @@ test-abidiff-exit/test-loc-without-locs-report.txt \
 test-abidiff-exit/test-no-stray-comma-report.txt \
 test-abidiff-exit/test-no-stray-comma-v0.o \
 test-abidiff-exit/test-no-stray-comma-v1.o \
+test-abidiff-exit/test-leaf0-v0.cc \
+test-abidiff-exit/test-leaf0-v0.o \
+test-abidiff-exit/test-leaf0-v1.cc \
+test-abidiff-exit/test-leaf0-v1.o \
+test-abidiff-exit/test-leaf0-report.txt \
 \
 test-diff-dwarf/test0-v0.cc		\
 test-diff-dwarf/test0-v0.o			\
diff --git a/tests/data/test-abidiff-exit/test-leaf0-report.txt b/tests/data/test-abidiff-exit/test-leaf0-report.txt
new file mode 100644
index 00000000..46f39976
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf0-report.txt
@@ -0,0 +1,13 @@
+Leaf changes summary: 2 artifacts changed
+Changed leaf types summary: 0 leaf type changed
+Removed/Changed/Added functions summary: 0 Removed, 1 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 1 Changed, 0 Added variable
+
+1 function with some sub-type change:
+
+  [C]'function int changed_fun()' has some sub-type changes:
+    return type changed:
+      type name changed from 'int' to 'long int'
+      type size changed from 32 to 64 (in bits)
+
+
diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.cc b/tests/data/test-abidiff-exit/test-leaf0-v0.cc
new file mode 100644
index 00000000..27ba39c9
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf0-v0.cc
@@ -0,0 +1,5 @@
+int changed_var = 0;
+
+int changed_fun() {
+  return 0;
+}
diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.o b/tests/data/test-abidiff-exit/test-leaf0-v0.o
new file mode 100644
index 0000000000000000000000000000000000000000..a79511cc3c6db039aaa1f5528db19b55f75c7a2d
GIT binary patch
literal 2784
zcmbtVOKTHR6h4#N*w)yPHa=P^GJ;QZ#zabO)oL_a>jM$Ng@VLMa?=ExiA*Ln>Ouq&
z+__LhaOa=UwSUBoKftvMJ!j^g<o0HY3s1<s=X~dR&wb3xtG8BDmIX`}T!5j(D1g0V
z$d@9v1bLW&duwlhekJ+gcWB`>7U?FU5HTMC@sQE@6!DgoF5@&QSRuyJEkBH-SRzd@
z88C4G>oZu>*5PbEBeL_^X=`RYw+|q-03v%fb1t)-xt_l`mzo6-hai)=kP#DDnF|-^
zSnp@A=gF-!`|<J|Mn8%Fs3Jg1jX0qOMeHh$0*>NnS--PWI7U6hWm^!+Zs=@CuTgL|
z3vR6zIi&M287Dw-cY5c|v~@p~Lcdzm^Vem)(&+iX{H7O-M!9zTo?Sk1qPSo$yQkfv
zeY#LwF5u60+F{R^E9=siL92$M7u1?7{iQ``alv-hZKvUNpZT?B(D6IHuG90Mw*0yi
zdesftg;Qbs2~-{wi<}$l*aWy%E}ylJU6Iw6=g0Y+rQ=58XkELb5tse^E?E7XHNR_o
z;vKeOKjQQ!ikf&Sn|qTgPwiMmgBVU6>EMjPuRlpjk$5s`BL<z_;wDg&L_(cXQzt+>
z<eE~zxF!u*0h2>P<3zk-4T)I+QzIAw0<zoSyGHOCM5eWM@f|yW_>9({!NDCljl9ux
z$VNAwJFpYuF;7KsC4|$ej}wSA9Z$eG-noeO82C#i(`6%~eb#r4<53*_e7Izc#PPX~
zb1@vn(Ytz6;cTx&xNG3AmHa@>W<9)iiW5CU$}YE!{o*#bt#DeazFu&<&)RqgaSnrc
zZZa3jeh6;e3q5eF-7dI+-0<AGtoGI`Ex*x0UBr9UY9KeasJ(z*ldUcrEnl(=rU19r
zX}2Z5vaa6=rHk*1(+$1aV--j?D~-TwOI7%PPV?wbz2$y#xRPQ9=TymLtJ6Ug2s3Xo
z@3U@8yy;I*G@U2Y{}ve|@%@RVQMrf;GbY)xf14fEY}%egoN|<uo~N8j9DgYR=v)!R
zXP@uU3SwqVit&vGq}%Z{7){$ds4+1z9#>#t;2ATfE%iC6{#kV?*q`dBSJJG%j+hw}
zAvmMJ8zg9C+E!4Plz&a-=RQ$>dY8=nPY|P+Tqm*-y;6xjFra46&+`k!ll=Fp%Y5V?
z{;!q4VFu^7JO)E8=Vj!R@;A*(3Rq%8HS_uzeveF2{~jgukG?|e&wZtO%-j^aufV1%
mn7#w_4x9e8C+6Kl2!1H=6bTxc>!-L${(V*d7tNPg)Bi7zRJKt7

literal 0
HcmV?d00001

diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.cc b/tests/data/test-abidiff-exit/test-leaf0-v1.cc
new file mode 100644
index 00000000..020cb761
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf0-v1.cc
@@ -0,0 +1,5 @@
+long changed_var = 0;
+
+long changed_fun() {
+  return 0;
+}
diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.o b/tests/data/test-abidiff-exit/test-leaf0-v1.o
new file mode 100644
index 0000000000000000000000000000000000000000..4a433b9572abb46799391619f21538b54fb3204c
GIT binary patch
literal 2824
zcmbtVOOF#r5UzH!FU(r|0KsYl8k84^%&<nQ)h??AgOh~~5kg2Df`qKG-F_(ccr_kx
zmVlH?K;(eL0SO@?gp@y#3nzX{?#Vd^Qq|LCx7))hWx1>B>w0zdV}AYahuxeIz$D-b
zj5J08ZcdNnk`_y_3`=nD=I?*~N&15i8g1ivL`$<eOe$+29rBi(BUy;)GO{_z6LNNn
zMC~c0&PxVN96-DQu_(SQEazomwXi6bW)@BY<kmqJF6X!NTlu}^t1G!v0P+jS=dYj@
z#th)<3eQJ{y=8JMoqBbB1*3n1e_avGaxTp|;TVX@F8(EajiV6%7Upr(eZ*x8NK`-Z
zJ1T6f`3J?IR@1&y$6+o_fa>w7qqnBSkGUNB)mmZHRP{<@5CO{vVGI|O?ml#SQPXQj
z3Ea4S&nsWJP+a%6f=fZsyR=r^TEm~`cauS+cA6?ual3|U7}r`mhnpMz#=7S>J--q5
zA4auS+>3gHzCVb5Zbx-L39B8|hsyn8k#l1HgN)K|l*^aBbKk0JJB-rY{^oh72)#gB
zmJ!4+cR?I3h}B~=voEkEpCL|vBHhqyg@vcN^8EBJ8l-UMNQdYM{PQtuQf8i1+LU3J
z$J`{&WRWx{)}51J9eQh0!i=SjL<w^v#gb&YVhc${3G)*e0TS@2W{`wq6X+5mi`Kfh
zmuC=PvidVT#1kizH=hpN#9<hA;3&qkyTO$ZPOJV3|3r%^1IC{l{e*+#z0^b(kBIi!
z-nH`@1s#XiL{MPil$Yz+HaNwxyGoaoi0e6>s&LD2#*GSh9sIG;?;Gyc$vvSs`q?se
z3!kvx_yq15oO)pQ0fPR+E?!mSNu2T)ONly6AgG5)2tl>qhags+FsQ5Qpjl~0jULWv
zIjmM=bucy^#`NNhyL8%-Vi!ySL9N&ADtvf?sF$b!pC7-UgtebcAl0fg;;^es;s2AZ
zqCfSP`^n)-Mi`!7qm!*oXHg-{z2Usiwk^r7KRx4g&RqXHWQ=5=!7ROqpc|9y*uP7P
zMXoJ<eCUf;vTR87%m>sn4s@;v;(GWV?I7mHL<ru~fOI=$!_l>+I$ey6^9EcSdZvPF
zOMT9&zho{2`&0e&db;)35p!cA1ltDuo)n8*+X~KQ<=;2?xlfdz-VHbZK4KJ;>qJr0
z6O-sI1BP>c=8q81^8egiW_}+z4Er(v&G_>?^S!ahfYHfby*B=?`5gmnj$F0G>u2~S
z3R(SomeD`@I<Y_ZmDb_rrr6I6*fI;H?*qNhu0QRGd-o86zYTbi6pOfT3{l)H|97VT
KKiJT4*Z)8NIl2)5

literal 0
HcmV?d00001

diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index aea57c32..5372b3fe 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -120,6 +120,15 @@ InOutSpec in_out_specs[] =
     "data/test-abidiff-exit/test-no-stray-comma-report.txt",
     "output/test-abidiff-exit/test-no-stray-comma-report.txt"
   },
+  {
+    "data/test-abidiff-exit/test-leaf0-v0.o",
+    "data/test-abidiff-exit/test-leaf0-v1.o",
+    "",
+    "--no-show-locs --leaf-changes-only",
+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+    "data/test-abidiff-exit/test-leaf0-report.txt",
+    "output/test-abidiff-exit/test-leaf0-report.txt"
+  },
   {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
 };
 
-- 
2.25.0.265.gbab2e86ba0-goog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] Fix the reporting of leaf change statistics.
  2020-01-01  0:00       ` Giuliano Procida via libabigail
@ 2020-01-01  0:00         ` Dodji Seketeli
  2020-01-01  0:00           ` Giuliano Procida via libabigail
  0 siblings, 1 reply; 14+ messages in thread
From: Dodji Seketeli @ 2020-01-01  0:00 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: Matthias Maennich, libabigail, kernel-team

Hello,

Giuliano Procida <gprocida@google.com> a écrit:

[...]


>> >diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
>> >index 5031e6d3..07077608 100644
>> >--- a/tests/data/Makefile.am
>> >+++ b/tests/data/Makefile.am
>> >@@ -110,6 +110,11 @@ test-abidiff-exit/test-loc-without-locs-report.txt \
>> > test-abidiff-exit/test-no-stray-comma-report.txt \
>> > test-abidiff-exit/test-no-stray-comma-v0.o \
>> > test-abidiff-exit/test-no-stray-comma-v1.o \
>> >+test-abidiff-exit/test-leaf0-v0.cc \
>>
>> I do not think we need to 'distribute' the source files here. I mean, it
>> will not harm, but they are mostly there for reference and not needed
>> during `make distcheck`.
>
> I was following someone else's example. Agreed, they are not needed in
> the Makefile.

Matthias is right in saying that we don't *need* to distribute the
source files.

However, I tend to always distribute them when we have them around, so
that even people who only have access to the tarball (yes that can still
happen even in this day and age) can tinker around with the source code,
make changes and try out stuff.

I thought I'd just throw this point of view in.

Thanks!

-- 
		Dodji

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] Fix the reporting of leaf change statistics.
  2020-01-01  0:00 ` [PATCH v2] " Giuliano Procida via libabigail
@ 2020-01-01  0:00   ` Matthias Maennich via libabigail
  2020-01-01  0:00     ` Matthias Maennich via libabigail
  2020-01-01  0:00   ` [PATCH v3] " Giuliano Procida via libabigail
  1 sibling, 1 reply; 14+ messages in thread
From: Matthias Maennich via libabigail @ 2020-01-01  0:00 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: libabigail, dodji, kernel-team

Hi Giuliano!

On Thu, Feb 13, 2020 at 04:19:26PM +0000, Android Kernel Team wrote:
>Leaf changes (as reported with --leaf-changes-only) to variables were
>miscounted as changes to functions.
>
>	* src/abg-comparison.cc
>	(apply_filters_and_compute_diff_stats):	Increment the correct
>	counter for leaf variable changes.
>	* tests/data/Makefile.am: Add test case files.
>	* tests/data/test-abidiff-exit/test-leaf0-*: New test case.
>	* tests/test-abidiff-exit.cc: Run new test case.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>
>---
> src/abg-comparison.cc                            |   4 ++--
> tests/data/Makefile.am                           |   3 +++
> .../data/test-abidiff-exit/test-leaf0-report.txt |  13 +++++++++++++
> tests/data/test-abidiff-exit/test-leaf0-v0.o     | Bin 0 -> 2784 bytes
> tests/data/test-abidiff-exit/test-leaf0-v1.o     | Bin 0 -> 2824 bytes

I assume the sources of those are rather small, so can you add the
source snippets of the test files to the commit message?

> tests/test-abidiff-exit.cc                       |   9 +++++++++
> 6 files changed, 27 insertions(+), 2 deletions(-)
> create mode 100644 tests/data/test-abidiff-exit/test-leaf0-report.txt
> create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.o
> create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.o
>
>diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
>index af4cd5e7..61aea47f 100644
>--- a/src/abg-comparison.cc
>+++ b/src/abg-comparison.cc
>@@ -9767,8 +9767,8 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat)
> 	      (stat.num_leaf_var_changes_filtered_out() + 1);
> 	}
>       if ((*i)->has_local_changes())
>-	stat.num_leaf_func_changes
>-	  (stat.num_leaf_func_changes() + 1);
>+	stat.num_leaf_var_changes
>+	  (stat.num_leaf_var_changes() + 1);
>     }
>
>   stat.num_func_syms_added(added_unrefed_fn_syms_.size());
>diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
>index 1f574d2e..2a928ef2 100644
>--- a/tests/data/Makefile.am
>+++ b/tests/data/Makefile.am
>@@ -107,6 +107,9 @@ test-abidiff-exit/test-loc-v0.bi \
> test-abidiff-exit/test-loc-v1.bi \
> test-abidiff-exit/test-loc-with-locs-report.txt \
> test-abidiff-exit/test-loc-without-locs-report.txt \
>+test-abidiff-exit/test-leaf0-v0.o \
>+test-abidiff-exit/test-leaf0-v1.o \
>+test-abidiff-exit/test-leaf0.report.txt \
> \
> test-diff-dwarf/test0-v0.cc		\
> test-diff-dwarf/test0-v0.o			\
>diff --git a/tests/data/test-abidiff-exit/test-leaf0-report.txt b/tests/data/test-abidiff-exit/test-leaf0-report.txt
>new file mode 100644
>index 00000000..46f39976
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-leaf0-report.txt
>@@ -0,0 +1,13 @@
>+Leaf changes summary: 2 artifacts changed
>+Changed leaf types summary: 0 leaf type changed
>+Removed/Changed/Added functions summary: 0 Removed, 1 Changed, 0 Added function
>+Removed/Changed/Added variables summary: 0 Removed, 1 Changed, 0 Added variable
>+
>+1 function with some sub-type change:
>+
>+  [C]'function int changed_fun()' has some sub-type changes:
>+    return type changed:
>+      type name changed from 'int' to 'long int'
>+      type size changed from 32 to 64 (in bits)
>+
>+

Should the variable change not be reported as well? Is this a different
issue or are variables changes are not supposed to be reported in this
mode?

>diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.o b/tests/data/test-abidiff-exit/test-leaf0-v0.o
>new file mode 100644
>index 0000000000000000000000000000000000000000..a79511cc3c6db039aaa1f5528db19b55f75c7a2d
>GIT binary patch
>literal 2784
>zcmbtVOKTHR6h4#N*w)yPHa=P^GJ;QZ#zabO)oL_a>jM$Ng@VLMa?=ExiA*Ln>Ouq&
>z+__LhaOa=UwSUBoKftvMJ!j^g<o0HY3s1<s=X~dR&wb3xtG8BDmIX`}T!5j(D1g0V
>z$d@9v1bLW&duwlhekJ+gcWB`>7U?FU5HTMC@sQE@6!DgoF5@&QSRuyJEkBH-SRzd@
>z88C4G>oZu>*5PbEBeL_^X=`RYw+|q-03v%fb1t)-xt_l`mzo6-hai)=kP#DDnF|-^
>zSnp@A=gF-!`|<J|Mn8%Fs3Jg1jX0qOMeHh$0*>NnS--PWI7U6hWm^!+Zs=@CuTgL|
>z3vR6zIi&M287Dw-cY5c|v~@p~Lcdzm^Vem)(&+iX{H7O-M!9zTo?Sk1qPSo$yQkfv
>zeY#LwF5u60+F{R^E9=siL92$M7u1?7{iQ``alv-hZKvUNpZT?B(D6IHuG90Mw*0yi
>zdesftg;Qbs2~-{wi<}$l*aWy%E}ylJU6Iw6=g0Y+rQ=58XkELb5tse^E?E7XHNR_o
>z;vKeOKjQQ!ikf&Sn|qTgPwiMmgBVU6>EMjPuRlpjk$5s`BL<z_;wDg&L_(cXQzt+>
>z<eE~zxF!u*0h2>P<3zk-4T)I+QzIAw0<zoSyGHOCM5eWM@f|yW_>9({!NDCljl9ux
>z$VNAwJFpYuF;7KsC4|$ej}wSA9Z$eG-noeO82C#i(`6%~eb#r4<53*_e7Izc#PPX~
>zb1@vn(Ytz6;cTx&xNG3AmHa@>W<9)iiW5CU$}YE!{o*#bt#DeazFu&<&)RqgaSnrc
>zZZa3jeh6;e3q5eF-7dI+-0<AGtoGI`Ex*x0UBr9UY9KeasJ(z*ldUcrEnl(=rU19r
>zX}2Z5vaa6=rHk*1(+$1aV--j?D~-TwOI7%PPV?wbz2$y#xRPQ9=TymLtJ6Ug2s3Xo
>z@3U@8yy;I*G@U2Y{}ve|@%@RVQMrf;GbY)xf14fEY}%egoN|<uo~N8j9DgYR=v)!R
>zXP@uU3SwqVit&vGq}%Z{7){$ds4+1z9#>#t;2ATfE%iC6{#kV?*q`dBSJJG%j+hw}
>zAvmMJ8zg9C+E!4Plz&a-=RQ$>dY8=nPY|P+Tqm*-y;6xjFra46&+`k!ll=Fp%Y5V?
>z{;!q4VFu^7JO)E8=Vj!R@;A*(3Rq%8HS_uzeveF2{~jgukG?|e&wZtO%-j^aufV1%
>mn7#w_4x9e8C+6Kl2!1H=6bTxc>!-L${(V*d7tNPg)Bi7zRJKt7
>
>literal 0
>HcmV?d00001
>
>diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.o b/tests/data/test-abidiff-exit/test-leaf0-v1.o
>new file mode 100644
>index 0000000000000000000000000000000000000000..4a433b9572abb46799391619f21538b54fb3204c
>GIT binary patch
>literal 2824
>zcmbtVOOF#r5UzH!FU(r|0KsYl8k84^%&<nQ)h??AgOh~~5kg2Df`qKG-F_(ccr_kx
>zmVlH?K;(eL0SO@?gp@y#3nzX{?#Vd^Qq|LCx7))hWx1>B>w0zdV}AYahuxeIz$D-b
>zj5J08ZcdNnk`_y_3`=nD=I?*~N&15i8g1ivL`$<eOe$+29rBi(BUy;)GO{_z6LNNn
>zMC~c0&PxVN96-DQu_(SQEazomwXi6bW)@BY<kmqJF6X!NTlu}^t1G!v0P+jS=dYj@
>z#th)<3eQJ{y=8JMoqBbB1*3n1e_avGaxTp|;TVX@F8(EajiV6%7Upr(eZ*x8NK`-Z
>zJ1T6f`3J?IR@1&y$6+o_fa>w7qqnBSkGUNB)mmZHRP{<@5CO{vVGI|O?ml#SQPXQj
>z3Ea4S&nsWJP+a%6f=fZsyR=r^TEm~`cauS+cA6?ual3|U7}r`mhnpMz#=7S>J--q5
>zA4auS+>3gHzCVb5Zbx-L39B8|hsyn8k#l1HgN)K|l*^aBbKk0JJB-rY{^oh72)#gB
>zmJ!4+cR?I3h}B~=voEkEpCL|vBHhqyg@vcN^8EBJ8l-UMNQdYM{PQtuQf8i1+LU3J
>z$J`{&WRWx{)}51J9eQh0!i=SjL<w^v#gb&YVhc${3G)*e0TS@2W{`wq6X+5mi`Kfh
>zmuC=PvidVT#1kizH=hpN#9<hA;3&qkyTO$ZPOJV3|3r%^1IC{l{e*+#z0^b(kBIi!
>z-nH`@1s#XiL{MPil$Yz+HaNwxyGoaoi0e6>s&LD2#*GSh9sIG;?;Gyc$vvSs`q?se
>z3!kvx_yq15oO)pQ0fPR+E?!mSNu2T)ONly6AgG5)2tl>qhags+FsQ5Qpjl~0jULWv
>zIjmM=bucy^#`NNhyL8%-Vi!ySL9N&ADtvf?sF$b!pC7-UgtebcAl0fg;;^es;s2AZ
>zqCfSP`^n)-Mi`!7qm!*oXHg-{z2Usiwk^r7KRx4g&RqXHWQ=5=!7ROqpc|9y*uP7P
>zMXoJ<eCUf;vTR87%m>sn4s@;v;(GWV?I7mHL<ru~fOI=$!_l>+I$ey6^9EcSdZvPF
>zOMT9&zho{2`&0e&db;)35p!cA1ltDuo)n8*+X~KQ<=;2?xlfdz-VHbZK4KJ;>qJr0
>z6O-sI1BP>c=8q81^8egiW_}+z4Er(v&G_>?^S!ahfYHfby*B=?`5gmnj$F0G>u2~S
>z3R(SomeD`@I<Y_ZmDb_rrr6I6*fI;H?*qNhu0QRGd-o86zYTbi6pOfT3{l)H|97VT
>KKiJT4*Z)8NIl2)5
>
>literal 0
>HcmV?d00001
>
>diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
>index 4cef727e..bafa7400 100644
>--- a/tests/test-abidiff-exit.cc
>+++ b/tests/test-abidiff-exit.cc
>@@ -111,6 +111,15 @@ InOutSpec in_out_specs[] =
>     "data/test-abidiff-exit/test-loc-without-locs-report.txt",
>     "output/test-abidiff-exit/test-loc-without-locs-report.txt"
>   },
>+  {
>+    "data/test-abidiff-exit/test-leaf0-v0.o",
>+    "data/test-abidiff-exit/test-leaf0-v1.o",
>+    "",
>+    "--no-show-locs --leaf-changes-only",

Should we also add a test case to the default mode (not leaf changes),
to ensure consistency?

Thanks!

Cheers,
Matthias

>+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
>+    "data/test-abidiff-exit/test-leaf0-report.txt",
>+    "output/test-abidiff-exit/test-leaf0-report.txt"
>+  },
>   {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
> };
>
>-- 
>2.25.0.265.gbab2e86ba0-goog
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] Fix the reporting of leaf change statistics.
@ 2020-01-01  0:00 Giuliano Procida via libabigail
  2020-01-01  0:00 ` [PATCH v2] " Giuliano Procida via libabigail
  0 siblings, 1 reply; 14+ messages in thread
From: Giuliano Procida via libabigail @ 2020-01-01  0:00 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida

	* src/abg-comparison.cc
	(apply_filters_and_compute_diff_stats):	Increment the correct
        counter for leaf variable changes.
	* tests/data/Makefile.am: Add test case files.
	* tests/data/test-abidiff-exit/test-leaf0-*: New test case.
	* tests/test-abidiff-exit.cc: Run new test case.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-comparison.cc                            |   4 ++--
 tests/data/Makefile.am                           |   3 +++
 .../data/test-abidiff-exit/test-leaf0-report.txt |  13 +++++++++++++
 tests/data/test-abidiff-exit/test-leaf0-v0.o     | Bin 0 -> 2784 bytes
 tests/data/test-abidiff-exit/test-leaf0-v1.o     | Bin 0 -> 2824 bytes
 tests/test-abidiff-exit.cc                       |   9 +++++++++
 6 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-report.txt
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.o
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.o

diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index af4cd5e7..61aea47f 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -9767,8 +9767,8 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat)
 	      (stat.num_leaf_var_changes_filtered_out() + 1);
 	}
       if ((*i)->has_local_changes())
-	stat.num_leaf_func_changes
-	  (stat.num_leaf_func_changes() + 1);
+	stat.num_leaf_var_changes
+	  (stat.num_leaf_var_changes() + 1);
     }
 
   stat.num_func_syms_added(added_unrefed_fn_syms_.size());
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 1f574d2e..2a928ef2 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -107,6 +107,9 @@ test-abidiff-exit/test-loc-v0.bi \
 test-abidiff-exit/test-loc-v1.bi \
 test-abidiff-exit/test-loc-with-locs-report.txt \
 test-abidiff-exit/test-loc-without-locs-report.txt \
+test-abidiff-exit/test-leaf0-v0.o \
+test-abidiff-exit/test-leaf0-v1.o \
+test-abidiff-exit/test-leaf0.report.txt \
 \
 test-diff-dwarf/test0-v0.cc		\
 test-diff-dwarf/test0-v0.o			\
diff --git a/tests/data/test-abidiff-exit/test-leaf0-report.txt b/tests/data/test-abidiff-exit/test-leaf0-report.txt
new file mode 100644
index 00000000..46f39976
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf0-report.txt
@@ -0,0 +1,13 @@
+Leaf changes summary: 2 artifacts changed
+Changed leaf types summary: 0 leaf type changed
+Removed/Changed/Added functions summary: 0 Removed, 1 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 1 Changed, 0 Added variable
+
+1 function with some sub-type change:
+
+  [C]'function int changed_fun()' has some sub-type changes:
+    return type changed:
+      type name changed from 'int' to 'long int'
+      type size changed from 32 to 64 (in bits)
+
+
diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.o b/tests/data/test-abidiff-exit/test-leaf0-v0.o
new file mode 100644
index 0000000000000000000000000000000000000000..a79511cc3c6db039aaa1f5528db19b55f75c7a2d
GIT binary patch
literal 2784
zcmbtVOKTHR6h4#N*w)yPHa=P^GJ;QZ#zabO)oL_a>jM$Ng@VLMa?=ExiA*Ln>Ouq&
z+__LhaOa=UwSUBoKftvMJ!j^g<o0HY3s1<s=X~dR&wb3xtG8BDmIX`}T!5j(D1g0V
z$d@9v1bLW&duwlhekJ+gcWB`>7U?FU5HTMC@sQE@6!DgoF5@&QSRuyJEkBH-SRzd@
z88C4G>oZu>*5PbEBeL_^X=`RYw+|q-03v%fb1t)-xt_l`mzo6-hai)=kP#DDnF|-^
zSnp@A=gF-!`|<J|Mn8%Fs3Jg1jX0qOMeHh$0*>NnS--PWI7U6hWm^!+Zs=@CuTgL|
z3vR6zIi&M287Dw-cY5c|v~@p~Lcdzm^Vem)(&+iX{H7O-M!9zTo?Sk1qPSo$yQkfv
zeY#LwF5u60+F{R^E9=siL92$M7u1?7{iQ``alv-hZKvUNpZT?B(D6IHuG90Mw*0yi
zdesftg;Qbs2~-{wi<}$l*aWy%E}ylJU6Iw6=g0Y+rQ=58XkELb5tse^E?E7XHNR_o
z;vKeOKjQQ!ikf&Sn|qTgPwiMmgBVU6>EMjPuRlpjk$5s`BL<z_;wDg&L_(cXQzt+>
z<eE~zxF!u*0h2>P<3zk-4T)I+QzIAw0<zoSyGHOCM5eWM@f|yW_>9({!NDCljl9ux
z$VNAwJFpYuF;7KsC4|$ej}wSA9Z$eG-noeO82C#i(`6%~eb#r4<53*_e7Izc#PPX~
zb1@vn(Ytz6;cTx&xNG3AmHa@>W<9)iiW5CU$}YE!{o*#bt#DeazFu&<&)RqgaSnrc
zZZa3jeh6;e3q5eF-7dI+-0<AGtoGI`Ex*x0UBr9UY9KeasJ(z*ldUcrEnl(=rU19r
zX}2Z5vaa6=rHk*1(+$1aV--j?D~-TwOI7%PPV?wbz2$y#xRPQ9=TymLtJ6Ug2s3Xo
z@3U@8yy;I*G@U2Y{}ve|@%@RVQMrf;GbY)xf14fEY}%egoN|<uo~N8j9DgYR=v)!R
zXP@uU3SwqVit&vGq}%Z{7){$ds4+1z9#>#t;2ATfE%iC6{#kV?*q`dBSJJG%j+hw}
zAvmMJ8zg9C+E!4Plz&a-=RQ$>dY8=nPY|P+Tqm*-y;6xjFra46&+`k!ll=Fp%Y5V?
z{;!q4VFu^7JO)E8=Vj!R@;A*(3Rq%8HS_uzeveF2{~jgukG?|e&wZtO%-j^aufV1%
mn7#w_4x9e8C+6Kl2!1H=6bTxc>!-L${(V*d7tNPg)Bi7zRJKt7

literal 0
HcmV?d00001

diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.o b/tests/data/test-abidiff-exit/test-leaf0-v1.o
new file mode 100644
index 0000000000000000000000000000000000000000..4a433b9572abb46799391619f21538b54fb3204c
GIT binary patch
literal 2824
zcmbtVOOF#r5UzH!FU(r|0KsYl8k84^%&<nQ)h??AgOh~~5kg2Df`qKG-F_(ccr_kx
zmVlH?K;(eL0SO@?gp@y#3nzX{?#Vd^Qq|LCx7))hWx1>B>w0zdV}AYahuxeIz$D-b
zj5J08ZcdNnk`_y_3`=nD=I?*~N&15i8g1ivL`$<eOe$+29rBi(BUy;)GO{_z6LNNn
zMC~c0&PxVN96-DQu_(SQEazomwXi6bW)@BY<kmqJF6X!NTlu}^t1G!v0P+jS=dYj@
z#th)<3eQJ{y=8JMoqBbB1*3n1e_avGaxTp|;TVX@F8(EajiV6%7Upr(eZ*x8NK`-Z
zJ1T6f`3J?IR@1&y$6+o_fa>w7qqnBSkGUNB)mmZHRP{<@5CO{vVGI|O?ml#SQPXQj
z3Ea4S&nsWJP+a%6f=fZsyR=r^TEm~`cauS+cA6?ual3|U7}r`mhnpMz#=7S>J--q5
zA4auS+>3gHzCVb5Zbx-L39B8|hsyn8k#l1HgN)K|l*^aBbKk0JJB-rY{^oh72)#gB
zmJ!4+cR?I3h}B~=voEkEpCL|vBHhqyg@vcN^8EBJ8l-UMNQdYM{PQtuQf8i1+LU3J
z$J`{&WRWx{)}51J9eQh0!i=SjL<w^v#gb&YVhc${3G)*e0TS@2W{`wq6X+5mi`Kfh
zmuC=PvidVT#1kizH=hpN#9<hA;3&qkyTO$ZPOJV3|3r%^1IC{l{e*+#z0^b(kBIi!
z-nH`@1s#XiL{MPil$Yz+HaNwxyGoaoi0e6>s&LD2#*GSh9sIG;?;Gyc$vvSs`q?se
z3!kvx_yq15oO)pQ0fPR+E?!mSNu2T)ONly6AgG5)2tl>qhags+FsQ5Qpjl~0jULWv
zIjmM=bucy^#`NNhyL8%-Vi!ySL9N&ADtvf?sF$b!pC7-UgtebcAl0fg;;^es;s2AZ
zqCfSP`^n)-Mi`!7qm!*oXHg-{z2Usiwk^r7KRx4g&RqXHWQ=5=!7ROqpc|9y*uP7P
zMXoJ<eCUf;vTR87%m>sn4s@;v;(GWV?I7mHL<ru~fOI=$!_l>+I$ey6^9EcSdZvPF
zOMT9&zho{2`&0e&db;)35p!cA1ltDuo)n8*+X~KQ<=;2?xlfdz-VHbZK4KJ;>qJr0
z6O-sI1BP>c=8q81^8egiW_}+z4Er(v&G_>?^S!ahfYHfby*B=?`5gmnj$F0G>u2~S
z3R(SomeD`@I<Y_ZmDb_rrr6I6*fI;H?*qNhu0QRGd-o86zYTbi6pOfT3{l)H|97VT
KKiJT4*Z)8NIl2)5

literal 0
HcmV?d00001

diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index 4cef727e..bafa7400 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -111,6 +111,15 @@ InOutSpec in_out_specs[] =
     "data/test-abidiff-exit/test-loc-without-locs-report.txt",
     "output/test-abidiff-exit/test-loc-without-locs-report.txt"
   },
+  {
+    "data/test-abidiff-exit/test-leaf0-v0.o",
+    "data/test-abidiff-exit/test-leaf0-v1.o",
+    "",
+    "--no-show-locs --leaf-changes-only",
+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+    "data/test-abidiff-exit/test-leaf0-report.txt",
+    "output/test-abidiff-exit/test-leaf0-report.txt"
+  },
   {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
 };
 
-- 
2.25.0.265.gbab2e86ba0-goog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] Fix the reporting of leaf change statistics.
  2020-01-01  0:00   ` Matthias Maennich via libabigail
@ 2020-01-01  0:00     ` Matthias Maennich via libabigail
  2020-01-01  0:00       ` Dodji Seketeli
  0 siblings, 1 reply; 14+ messages in thread
From: Matthias Maennich via libabigail @ 2020-01-01  0:00 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: libabigail, dodji, kernel-team

On Mon, Feb 17, 2020 at 05:54:31PM +0000, Matthias Maennich wrote:
>Hi Giuliano!
>
>On Thu, Feb 13, 2020 at 04:19:26PM +0000, Android Kernel Team wrote:
>>Leaf changes (as reported with --leaf-changes-only) to variables were
>>miscounted as changes to functions.
>>
>>	* src/abg-comparison.cc
>>	(apply_filters_and_compute_diff_stats):	Increment the correct
>>	counter for leaf variable changes.
>>	* tests/data/Makefile.am: Add test case files.
>>	* tests/data/test-abidiff-exit/test-leaf0-*: New test case.
>>	* tests/test-abidiff-exit.cc: Run new test case.
>>
>>Signed-off-by: Giuliano Procida <gprocida@google.com>
>>---
>>src/abg-comparison.cc                            |   4 ++--
>>tests/data/Makefile.am                           |   3 +++
>>.../data/test-abidiff-exit/test-leaf0-report.txt |  13 +++++++++++++
>>tests/data/test-abidiff-exit/test-leaf0-v0.o     | Bin 0 -> 2784 bytes
>>tests/data/test-abidiff-exit/test-leaf0-v1.o     | Bin 0 -> 2824 bytes
>
>I assume the sources of those are rather small, so can you add the
>source snippets of the test files to the commit message?
>
>>tests/test-abidiff-exit.cc                       |   9 +++++++++
>>6 files changed, 27 insertions(+), 2 deletions(-)
>>create mode 100644 tests/data/test-abidiff-exit/test-leaf0-report.txt
>>create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.o
>>create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.o
>>
>>diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
>>index af4cd5e7..61aea47f 100644
>>--- a/src/abg-comparison.cc
>>+++ b/src/abg-comparison.cc
>>@@ -9767,8 +9767,8 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat)
>>	      (stat.num_leaf_var_changes_filtered_out() + 1);
>>	}
>>      if ((*i)->has_local_changes())
>>-	stat.num_leaf_func_changes
>>-	  (stat.num_leaf_func_changes() + 1);
>>+	stat.num_leaf_var_changes
>>+	  (stat.num_leaf_var_changes() + 1);
>>    }
>>
>>  stat.num_func_syms_added(added_unrefed_fn_syms_.size());
>>diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
>>index 1f574d2e..2a928ef2 100644
>>--- a/tests/data/Makefile.am
>>+++ b/tests/data/Makefile.am
>>@@ -107,6 +107,9 @@ test-abidiff-exit/test-loc-v0.bi \
>>test-abidiff-exit/test-loc-v1.bi \
>>test-abidiff-exit/test-loc-with-locs-report.txt \
>>test-abidiff-exit/test-loc-without-locs-report.txt \
>>+test-abidiff-exit/test-leaf0-v0.o \
>>+test-abidiff-exit/test-leaf0-v1.o \
>>+test-abidiff-exit/test-leaf0.report.txt \
                              ^^^
The file is named    test-leaf0-report.txt

With that, `make distcheck` runs successfully as well.

Cheers,
Matthias

>>\
>>test-diff-dwarf/test0-v0.cc		\
>>test-diff-dwarf/test0-v0.o			\
>>diff --git a/tests/data/test-abidiff-exit/test-leaf0-report.txt b/tests/data/test-abidiff-exit/test-leaf0-report.txt
>>new file mode 100644
>>index 00000000..46f39976
>>--- /dev/null
>>+++ b/tests/data/test-abidiff-exit/test-leaf0-report.txt
>>@@ -0,0 +1,13 @@
>>+Leaf changes summary: 2 artifacts changed
>>+Changed leaf types summary: 0 leaf type changed
>>+Removed/Changed/Added functions summary: 0 Removed, 1 Changed, 0 Added function
>>+Removed/Changed/Added variables summary: 0 Removed, 1 Changed, 0 Added variable
>>+
>>+1 function with some sub-type change:
>>+
>>+  [C]'function int changed_fun()' has some sub-type changes:
>>+    return type changed:
>>+      type name changed from 'int' to 'long int'
>>+      type size changed from 32 to 64 (in bits)
>>+
>>+
>
>Should the variable change not be reported as well? Is this a different
>issue or are variables changes are not supposed to be reported in this
>mode?
>
>>diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.o b/tests/data/test-abidiff-exit/test-leaf0-v0.o
>>new file mode 100644
>>index 0000000000000000000000000000000000000000..a79511cc3c6db039aaa1f5528db19b55f75c7a2d
>>GIT binary patch
>>literal 2784
>>zcmbtVOKTHR6h4#N*w)yPHa=P^GJ;QZ#zabO)oL_a>jM$Ng@VLMa?=ExiA*Ln>Ouq&
>>z+__LhaOa=UwSUBoKftvMJ!j^g<o0HY3s1<s=X~dR&wb3xtG8BDmIX`}T!5j(D1g0V
>>z$d@9v1bLW&duwlhekJ+gcWB`>7U?FU5HTMC@sQE@6!DgoF5@&QSRuyJEkBH-SRzd@
>>z88C4G>oZu>*5PbEBeL_^X=`RYw+|q-03v%fb1t)-xt_l`mzo6-hai)=kP#DDnF|-^
>>zSnp@A=gF-!`|<J|Mn8%Fs3Jg1jX0qOMeHh$0*>NnS--PWI7U6hWm^!+Zs=@CuTgL|
>>z3vR6zIi&M287Dw-cY5c|v~@p~Lcdzm^Vem)(&+iX{H7O-M!9zTo?Sk1qPSo$yQkfv
>>zeY#LwF5u60+F{R^E9=siL92$M7u1?7{iQ``alv-hZKvUNpZT?B(D6IHuG90Mw*0yi
>>zdesftg;Qbs2~-{wi<}$l*aWy%E}ylJU6Iw6=g0Y+rQ=58XkELb5tse^E?E7XHNR_o
>>z;vKeOKjQQ!ikf&Sn|qTgPwiMmgBVU6>EMjPuRlpjk$5s`BL<z_;wDg&L_(cXQzt+>
>>z<eE~zxF!u*0h2>P<3zk-4T)I+QzIAw0<zoSyGHOCM5eWM@f|yW_>9({!NDCljl9ux
>>z$VNAwJFpYuF;7KsC4|$ej}wSA9Z$eG-noeO82C#i(`6%~eb#r4<53*_e7Izc#PPX~
>>zb1@vn(Ytz6;cTx&xNG3AmHa@>W<9)iiW5CU$}YE!{o*#bt#DeazFu&<&)RqgaSnrc
>>zZZa3jeh6;e3q5eF-7dI+-0<AGtoGI`Ex*x0UBr9UY9KeasJ(z*ldUcrEnl(=rU19r
>>zX}2Z5vaa6=rHk*1(+$1aV--j?D~-TwOI7%PPV?wbz2$y#xRPQ9=TymLtJ6Ug2s3Xo
>>z@3U@8yy;I*G@U2Y{}ve|@%@RVQMrf;GbY)xf14fEY}%egoN|<uo~N8j9DgYR=v)!R
>>zXP@uU3SwqVit&vGq}%Z{7){$ds4+1z9#>#t;2ATfE%iC6{#kV?*q`dBSJJG%j+hw}
>>zAvmMJ8zg9C+E!4Plz&a-=RQ$>dY8=nPY|P+Tqm*-y;6xjFra46&+`k!ll=Fp%Y5V?
>>z{;!q4VFu^7JO)E8=Vj!R@;A*(3Rq%8HS_uzeveF2{~jgukG?|e&wZtO%-j^aufV1%
>>mn7#w_4x9e8C+6Kl2!1H=6bTxc>!-L${(V*d7tNPg)Bi7zRJKt7
>>
>>literal 0
>>HcmV?d00001
>>
>>diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.o b/tests/data/test-abidiff-exit/test-leaf0-v1.o
>>new file mode 100644
>>index 0000000000000000000000000000000000000000..4a433b9572abb46799391619f21538b54fb3204c
>>GIT binary patch
>>literal 2824
>>zcmbtVOOF#r5UzH!FU(r|0KsYl8k84^%&<nQ)h??AgOh~~5kg2Df`qKG-F_(ccr_kx
>>zmVlH?K;(eL0SO@?gp@y#3nzX{?#Vd^Qq|LCx7))hWx1>B>w0zdV}AYahuxeIz$D-b
>>zj5J08ZcdNnk`_y_3`=nD=I?*~N&15i8g1ivL`$<eOe$+29rBi(BUy;)GO{_z6LNNn
>>zMC~c0&PxVN96-DQu_(SQEazomwXi6bW)@BY<kmqJF6X!NTlu}^t1G!v0P+jS=dYj@
>>z#th)<3eQJ{y=8JMoqBbB1*3n1e_avGaxTp|;TVX@F8(EajiV6%7Upr(eZ*x8NK`-Z
>>zJ1T6f`3J?IR@1&y$6+o_fa>w7qqnBSkGUNB)mmZHRP{<@5CO{vVGI|O?ml#SQPXQj
>>z3Ea4S&nsWJP+a%6f=fZsyR=r^TEm~`cauS+cA6?ual3|U7}r`mhnpMz#=7S>J--q5
>>zA4auS+>3gHzCVb5Zbx-L39B8|hsyn8k#l1HgN)K|l*^aBbKk0JJB-rY{^oh72)#gB
>>zmJ!4+cR?I3h}B~=voEkEpCL|vBHhqyg@vcN^8EBJ8l-UMNQdYM{PQtuQf8i1+LU3J
>>z$J`{&WRWx{)}51J9eQh0!i=SjL<w^v#gb&YVhc${3G)*e0TS@2W{`wq6X+5mi`Kfh
>>zmuC=PvidVT#1kizH=hpN#9<hA;3&qkyTO$ZPOJV3|3r%^1IC{l{e*+#z0^b(kBIi!
>>z-nH`@1s#XiL{MPil$Yz+HaNwxyGoaoi0e6>s&LD2#*GSh9sIG;?;Gyc$vvSs`q?se
>>z3!kvx_yq15oO)pQ0fPR+E?!mSNu2T)ONly6AgG5)2tl>qhags+FsQ5Qpjl~0jULWv
>>zIjmM=bucy^#`NNhyL8%-Vi!ySL9N&ADtvf?sF$b!pC7-UgtebcAl0fg;;^es;s2AZ
>>zqCfSP`^n)-Mi`!7qm!*oXHg-{z2Usiwk^r7KRx4g&RqXHWQ=5=!7ROqpc|9y*uP7P
>>zMXoJ<eCUf;vTR87%m>sn4s@;v;(GWV?I7mHL<ru~fOI=$!_l>+I$ey6^9EcSdZvPF
>>zOMT9&zho{2`&0e&db;)35p!cA1ltDuo)n8*+X~KQ<=;2?xlfdz-VHbZK4KJ;>qJr0
>>z6O-sI1BP>c=8q81^8egiW_}+z4Er(v&G_>?^S!ahfYHfby*B=?`5gmnj$F0G>u2~S
>>z3R(SomeD`@I<Y_ZmDb_rrr6I6*fI;H?*qNhu0QRGd-o86zYTbi6pOfT3{l)H|97VT
>>KKiJT4*Z)8NIl2)5
>>
>>literal 0
>>HcmV?d00001
>>
>>diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
>>index 4cef727e..bafa7400 100644
>>--- a/tests/test-abidiff-exit.cc
>>+++ b/tests/test-abidiff-exit.cc
>>@@ -111,6 +111,15 @@ InOutSpec in_out_specs[] =
>>    "data/test-abidiff-exit/test-loc-without-locs-report.txt",
>>    "output/test-abidiff-exit/test-loc-without-locs-report.txt"
>>  },
>>+  {
>>+    "data/test-abidiff-exit/test-leaf0-v0.o",
>>+    "data/test-abidiff-exit/test-leaf0-v1.o",
>>+    "",
>>+    "--no-show-locs --leaf-changes-only",
>
>Should we also add a test case to the default mode (not leaf changes),
>to ensure consistency?
>
>Thanks!
>
>Cheers,
>Matthias
>
>>+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
>>+    "data/test-abidiff-exit/test-leaf0-report.txt",
>>+    "output/test-abidiff-exit/test-leaf0-report.txt"
>>+  },
>>  {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
>>};
>>
>>-- 
>>2.25.0.265.gbab2e86ba0-goog
>>
>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] Fix the reporting of leaf change statistics.
  2020-01-01  0:00   ` [PATCH v3] " Giuliano Procida via libabigail
@ 2020-01-01  0:00     ` Matthias Maennich via libabigail
  2020-01-01  0:00       ` Giuliano Procida via libabigail
  2020-01-01  0:00     ` [PATCH v4] " Giuliano Procida via libabigail
  1 sibling, 1 reply; 14+ messages in thread
From: Matthias Maennich via libabigail @ 2020-01-01  0:00 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: libabigail, dodji, kernel-team

On Mon, Mar 02, 2020 at 12:58:53PM +0000, Android Kernel Team wrote:
>Leaf changes (as reported with --leaf-changes-only) to variables were
>miscounted as changes to functions.
>
>	* src/abg-comparison.cc
>	(apply_filters_and_compute_diff_stats):	Increment the correct
>	counter for leaf variable changes.
>	* tests/data/Makefile.am: Add test case files.
>	* tests/data/test-abidiff-exit/test-leaf0-*: New test case.

Please spell out the single files.
    * tests/data/test-abidiff-exit/test-leaf0-report.txt: New test case.
    * tests/data/test-abidiff-exit/test-leaf0-v0.cc: Likewise.
    * tests/data/test-abidiff-exit/test-leaf0-v0.o: Likewise.
    * tests/data/test-abidiff-exit/test-leaf0-v1.cc: Likewise.
    * tests/data/test-abidiff-exit/test-leaf0-v1.o: Likewise.

>	* tests/test-abidiff-exit.cc: Run new test case.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>
>---
> src/abg-comparison.cc                            |   4 ++--
> tests/data/Makefile.am                           |   5 +++++
> .../data/test-abidiff-exit/test-leaf0-report.txt |  13 +++++++++++++
> tests/data/test-abidiff-exit/test-leaf0-v0.cc    |   5 +++++
> tests/data/test-abidiff-exit/test-leaf0-v0.o     | Bin 0 -> 2784 bytes
> tests/data/test-abidiff-exit/test-leaf0-v1.cc    |   5 +++++
> tests/data/test-abidiff-exit/test-leaf0-v1.o     | Bin 0 -> 2824 bytes
> tests/test-abidiff-exit.cc                       |   9 +++++++++
> 8 files changed, 39 insertions(+), 2 deletions(-)
> create mode 100644 tests/data/test-abidiff-exit/test-leaf0-report.txt
> create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.cc
> create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.o
> create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.cc
> create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.o
>
>diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
>index 5371e843..ef753e6d 100644
>--- a/src/abg-comparison.cc
>+++ b/src/abg-comparison.cc
>@@ -9767,8 +9767,8 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat)
> 	      (stat.num_leaf_var_changes_filtered_out() + 1);
> 	}
>       if ((*i)->has_local_changes())
>-	stat.num_leaf_func_changes
>-	  (stat.num_leaf_func_changes() + 1);
>+	stat.num_leaf_var_changes
>+	  (stat.num_leaf_var_changes() + 1);
>     }
>
>   stat.num_func_syms_added(added_unrefed_fn_syms_.size());
>diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
>index 5031e6d3..07077608 100644
>--- a/tests/data/Makefile.am
>+++ b/tests/data/Makefile.am
>@@ -110,6 +110,11 @@ test-abidiff-exit/test-loc-without-locs-report.txt \
> test-abidiff-exit/test-no-stray-comma-report.txt \
> test-abidiff-exit/test-no-stray-comma-v0.o \
> test-abidiff-exit/test-no-stray-comma-v1.o \
>+test-abidiff-exit/test-leaf0-v0.cc \

I do not think we need to 'distribute' the source files here. I mean, it
will not harm, but they are mostly there for reference and not needed
during `make distcheck`.

Other than these little nits.

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

Cheers,
Matthias

>+test-abidiff-exit/test-leaf0-v0.o \
>+test-abidiff-exit/test-leaf0-v1.cc \
>+test-abidiff-exit/test-leaf0-v1.o \
>+test-abidiff-exit/test-leaf0-report.txt \
> \
> test-diff-dwarf/test0-v0.cc		\
> test-diff-dwarf/test0-v0.o			\
>diff --git a/tests/data/test-abidiff-exit/test-leaf0-report.txt b/tests/data/test-abidiff-exit/test-leaf0-report.txt
>new file mode 100644
>index 00000000..46f39976
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-leaf0-report.txt
>@@ -0,0 +1,13 @@
>+Leaf changes summary: 2 artifacts changed
>+Changed leaf types summary: 0 leaf type changed
>+Removed/Changed/Added functions summary: 0 Removed, 1 Changed, 0 Added function
>+Removed/Changed/Added variables summary: 0 Removed, 1 Changed, 0 Added variable
>+
>+1 function with some sub-type change:
>+
>+  [C]'function int changed_fun()' has some sub-type changes:
>+    return type changed:
>+      type name changed from 'int' to 'long int'
>+      type size changed from 32 to 64 (in bits)
>+
>+
>diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.cc b/tests/data/test-abidiff-exit/test-leaf0-v0.cc
>new file mode 100644
>index 00000000..27ba39c9
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-leaf0-v0.cc
>@@ -0,0 +1,5 @@
>+int changed_var = 0;
>+
>+int changed_fun() {
>+  return 0;
>+}
>diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.o b/tests/data/test-abidiff-exit/test-leaf0-v0.o
>new file mode 100644
>index 0000000000000000000000000000000000000000..a79511cc3c6db039aaa1f5528db19b55f75c7a2d
>GIT binary patch
>literal 2784
>zcmbtVOKTHR6h4#N*w)yPHa=P^GJ;QZ#zabO)oL_a>jM$Ng@VLMa?=ExiA*Ln>Ouq&
>z+__LhaOa=UwSUBoKftvMJ!j^g<o0HY3s1<s=X~dR&wb3xtG8BDmIX`}T!5j(D1g0V
>z$d@9v1bLW&duwlhekJ+gcWB`>7U?FU5HTMC@sQE@6!DgoF5@&QSRuyJEkBH-SRzd@
>z88C4G>oZu>*5PbEBeL_^X=`RYw+|q-03v%fb1t)-xt_l`mzo6-hai)=kP#DDnF|-^
>zSnp@A=gF-!`|<J|Mn8%Fs3Jg1jX0qOMeHh$0*>NnS--PWI7U6hWm^!+Zs=@CuTgL|
>z3vR6zIi&M287Dw-cY5c|v~@p~Lcdzm^Vem)(&+iX{H7O-M!9zTo?Sk1qPSo$yQkfv
>zeY#LwF5u60+F{R^E9=siL92$M7u1?7{iQ``alv-hZKvUNpZT?B(D6IHuG90Mw*0yi
>zdesftg;Qbs2~-{wi<}$l*aWy%E}ylJU6Iw6=g0Y+rQ=58XkELb5tse^E?E7XHNR_o
>z;vKeOKjQQ!ikf&Sn|qTgPwiMmgBVU6>EMjPuRlpjk$5s`BL<z_;wDg&L_(cXQzt+>
>z<eE~zxF!u*0h2>P<3zk-4T)I+QzIAw0<zoSyGHOCM5eWM@f|yW_>9({!NDCljl9ux
>z$VNAwJFpYuF;7KsC4|$ej}wSA9Z$eG-noeO82C#i(`6%~eb#r4<53*_e7Izc#PPX~
>zb1@vn(Ytz6;cTx&xNG3AmHa@>W<9)iiW5CU$}YE!{o*#bt#DeazFu&<&)RqgaSnrc
>zZZa3jeh6;e3q5eF-7dI+-0<AGtoGI`Ex*x0UBr9UY9KeasJ(z*ldUcrEnl(=rU19r
>zX}2Z5vaa6=rHk*1(+$1aV--j?D~-TwOI7%PPV?wbz2$y#xRPQ9=TymLtJ6Ug2s3Xo
>z@3U@8yy;I*G@U2Y{}ve|@%@RVQMrf;GbY)xf14fEY}%egoN|<uo~N8j9DgYR=v)!R
>zXP@uU3SwqVit&vGq}%Z{7){$ds4+1z9#>#t;2ATfE%iC6{#kV?*q`dBSJJG%j+hw}
>zAvmMJ8zg9C+E!4Plz&a-=RQ$>dY8=nPY|P+Tqm*-y;6xjFra46&+`k!ll=Fp%Y5V?
>z{;!q4VFu^7JO)E8=Vj!R@;A*(3Rq%8HS_uzeveF2{~jgukG?|e&wZtO%-j^aufV1%
>mn7#w_4x9e8C+6Kl2!1H=6bTxc>!-L${(V*d7tNPg)Bi7zRJKt7
>
>literal 0
>HcmV?d00001
>
>diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.cc b/tests/data/test-abidiff-exit/test-leaf0-v1.cc
>new file mode 100644
>index 00000000..020cb761
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-leaf0-v1.cc
>@@ -0,0 +1,5 @@
>+long changed_var = 0;
>+
>+long changed_fun() {
>+  return 0;
>+}
>diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.o b/tests/data/test-abidiff-exit/test-leaf0-v1.o
>new file mode 100644
>index 0000000000000000000000000000000000000000..4a433b9572abb46799391619f21538b54fb3204c
>GIT binary patch
>literal 2824
>zcmbtVOOF#r5UzH!FU(r|0KsYl8k84^%&<nQ)h??AgOh~~5kg2Df`qKG-F_(ccr_kx
>zmVlH?K;(eL0SO@?gp@y#3nzX{?#Vd^Qq|LCx7))hWx1>B>w0zdV}AYahuxeIz$D-b
>zj5J08ZcdNnk`_y_3`=nD=I?*~N&15i8g1ivL`$<eOe$+29rBi(BUy;)GO{_z6LNNn
>zMC~c0&PxVN96-DQu_(SQEazomwXi6bW)@BY<kmqJF6X!NTlu}^t1G!v0P+jS=dYj@
>z#th)<3eQJ{y=8JMoqBbB1*3n1e_avGaxTp|;TVX@F8(EajiV6%7Upr(eZ*x8NK`-Z
>zJ1T6f`3J?IR@1&y$6+o_fa>w7qqnBSkGUNB)mmZHRP{<@5CO{vVGI|O?ml#SQPXQj
>z3Ea4S&nsWJP+a%6f=fZsyR=r^TEm~`cauS+cA6?ual3|U7}r`mhnpMz#=7S>J--q5
>zA4auS+>3gHzCVb5Zbx-L39B8|hsyn8k#l1HgN)K|l*^aBbKk0JJB-rY{^oh72)#gB
>zmJ!4+cR?I3h}B~=voEkEpCL|vBHhqyg@vcN^8EBJ8l-UMNQdYM{PQtuQf8i1+LU3J
>z$J`{&WRWx{)}51J9eQh0!i=SjL<w^v#gb&YVhc${3G)*e0TS@2W{`wq6X+5mi`Kfh
>zmuC=PvidVT#1kizH=hpN#9<hA;3&qkyTO$ZPOJV3|3r%^1IC{l{e*+#z0^b(kBIi!
>z-nH`@1s#XiL{MPil$Yz+HaNwxyGoaoi0e6>s&LD2#*GSh9sIG;?;Gyc$vvSs`q?se
>z3!kvx_yq15oO)pQ0fPR+E?!mSNu2T)ONly6AgG5)2tl>qhags+FsQ5Qpjl~0jULWv
>zIjmM=bucy^#`NNhyL8%-Vi!ySL9N&ADtvf?sF$b!pC7-UgtebcAl0fg;;^es;s2AZ
>zqCfSP`^n)-Mi`!7qm!*oXHg-{z2Usiwk^r7KRx4g&RqXHWQ=5=!7ROqpc|9y*uP7P
>zMXoJ<eCUf;vTR87%m>sn4s@;v;(GWV?I7mHL<ru~fOI=$!_l>+I$ey6^9EcSdZvPF
>zOMT9&zho{2`&0e&db;)35p!cA1ltDuo)n8*+X~KQ<=;2?xlfdz-VHbZK4KJ;>qJr0
>z6O-sI1BP>c=8q81^8egiW_}+z4Er(v&G_>?^S!ahfYHfby*B=?`5gmnj$F0G>u2~S
>z3R(SomeD`@I<Y_ZmDb_rrr6I6*fI;H?*qNhu0QRGd-o86zYTbi6pOfT3{l)H|97VT
>KKiJT4*Z)8NIl2)5
>
>literal 0
>HcmV?d00001
>
>diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
>index aea57c32..5372b3fe 100644
>--- a/tests/test-abidiff-exit.cc
>+++ b/tests/test-abidiff-exit.cc
>@@ -120,6 +120,15 @@ InOutSpec in_out_specs[] =
>     "data/test-abidiff-exit/test-no-stray-comma-report.txt",
>     "output/test-abidiff-exit/test-no-stray-comma-report.txt"
>   },
>+  {
>+    "data/test-abidiff-exit/test-leaf0-v0.o",
>+    "data/test-abidiff-exit/test-leaf0-v1.o",
>+    "",
>+    "--no-show-locs --leaf-changes-only",
>+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
>+    "data/test-abidiff-exit/test-leaf0-report.txt",
>+    "output/test-abidiff-exit/test-leaf0-report.txt"
>+  },
>   {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
> };
>
>-- 
>2.25.0.265.gbab2e86ba0-goog
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] Fix the reporting of leaf change statistics.
  2020-01-01  0:00     ` Matthias Maennich via libabigail
@ 2020-01-01  0:00       ` Giuliano Procida via libabigail
  2020-01-01  0:00         ` Dodji Seketeli
  0 siblings, 1 reply; 14+ messages in thread
From: Giuliano Procida via libabigail @ 2020-01-01  0:00 UTC (permalink / raw)
  To: Matthias Maennich; +Cc: libabigail, Dodji Seketeli, kernel-team

HI there.

On Tue, 3 Mar 2020 at 16:43, Matthias Maennich <maennich@google.com> wrote:
>
> On Mon, Mar 02, 2020 at 12:58:53PM +0000, Android Kernel Team wrote:
> >Leaf changes (as reported with --leaf-changes-only) to variables were
> >miscounted as changes to functions.
> >
> >       * src/abg-comparison.cc
> >       (apply_filters_and_compute_diff_stats): Increment the correct
> >       counter for leaf variable changes.
> >       * tests/data/Makefile.am: Add test case files.
> >       * tests/data/test-abidiff-exit/test-leaf0-*: New test case.
>
> Please spell out the single files.
>     * tests/data/test-abidiff-exit/test-leaf0-report.txt: New test case.
>     * tests/data/test-abidiff-exit/test-leaf0-v0.cc: Likewise.
>     * tests/data/test-abidiff-exit/test-leaf0-v0.o: Likewise.
>     * tests/data/test-abidiff-exit/test-leaf0-v1.cc: Likewise.
>     * tests/data/test-abidiff-exit/test-leaf0-v1.o: Likewise.

OK.

> >       * tests/test-abidiff-exit.cc: Run new test case.
> >
> >Signed-off-by: Giuliano Procida <gprocida@google.com>
> >---
> > src/abg-comparison.cc                            |   4 ++--
> > tests/data/Makefile.am                           |   5 +++++
> > .../data/test-abidiff-exit/test-leaf0-report.txt |  13 +++++++++++++
> > tests/data/test-abidiff-exit/test-leaf0-v0.cc    |   5 +++++
> > tests/data/test-abidiff-exit/test-leaf0-v0.o     | Bin 0 -> 2784 bytes
> > tests/data/test-abidiff-exit/test-leaf0-v1.cc    |   5 +++++
> > tests/data/test-abidiff-exit/test-leaf0-v1.o     | Bin 0 -> 2824 bytes
> > tests/test-abidiff-exit.cc                       |   9 +++++++++
> > 8 files changed, 39 insertions(+), 2 deletions(-)
> > create mode 100644 tests/data/test-abidiff-exit/test-leaf0-report.txt
> > create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.cc
> > create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.o
> > create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.cc
> > create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.o
> >
> >diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
> >index 5371e843..ef753e6d 100644
> >--- a/src/abg-comparison.cc
> >+++ b/src/abg-comparison.cc
> >@@ -9767,8 +9767,8 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat)
> >             (stat.num_leaf_var_changes_filtered_out() + 1);
> >       }
> >       if ((*i)->has_local_changes())
> >-      stat.num_leaf_func_changes
> >-        (stat.num_leaf_func_changes() + 1);
> >+      stat.num_leaf_var_changes
> >+        (stat.num_leaf_var_changes() + 1);
> >     }
> >
> >   stat.num_func_syms_added(added_unrefed_fn_syms_.size());
> >diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
> >index 5031e6d3..07077608 100644
> >--- a/tests/data/Makefile.am
> >+++ b/tests/data/Makefile.am
> >@@ -110,6 +110,11 @@ test-abidiff-exit/test-loc-without-locs-report.txt \
> > test-abidiff-exit/test-no-stray-comma-report.txt \
> > test-abidiff-exit/test-no-stray-comma-v0.o \
> > test-abidiff-exit/test-no-stray-comma-v1.o \
> >+test-abidiff-exit/test-leaf0-v0.cc \
>
> I do not think we need to 'distribute' the source files here. I mean, it
> will not harm, but they are mostly there for reference and not needed
> during `make distcheck`.

I was following someone else's example. Agreed, they are not needed in
the Makefile.

> Other than these little nits.
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
>
> Cheers,
> Matthias
>
> >+test-abidiff-exit/test-leaf0-v0.o \
> >+test-abidiff-exit/test-leaf0-v1.cc \
> >+test-abidiff-exit/test-leaf0-v1.o \
> >+test-abidiff-exit/test-leaf0-report.txt \
> > \
> > test-diff-dwarf/test0-v0.cc           \
> > test-diff-dwarf/test0-v0.o                    \
> >diff --git a/tests/data/test-abidiff-exit/test-leaf0-report.txt b/tests/data/test-abidiff-exit/test-leaf0-report.txt
> >new file mode 100644
> >index 00000000..46f39976
> >--- /dev/null
> >+++ b/tests/data/test-abidiff-exit/test-leaf0-report.txt
> >@@ -0,0 +1,13 @@
> >+Leaf changes summary: 2 artifacts changed
> >+Changed leaf types summary: 0 leaf type changed
> >+Removed/Changed/Added functions summary: 0 Removed, 1 Changed, 0 Added function
> >+Removed/Changed/Added variables summary: 0 Removed, 1 Changed, 0 Added variable
> >+
> >+1 function with some sub-type change:
> >+
> >+  [C]'function int changed_fun()' has some sub-type changes:
> >+    return type changed:
> >+      type name changed from 'int' to 'long int'
> >+      type size changed from 32 to 64 (in bits)
> >+
> >+
> >diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.cc b/tests/data/test-abidiff-exit/test-leaf0-v0.cc
> >new file mode 100644
> >index 00000000..27ba39c9
> >--- /dev/null
> >+++ b/tests/data/test-abidiff-exit/test-leaf0-v0.cc
> >@@ -0,0 +1,5 @@
> >+int changed_var = 0;
> >+
> >+int changed_fun() {
> >+  return 0;
> >+}
> >diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.o b/tests/data/test-abidiff-exit/test-leaf0-v0.o
> >new file mode 100644
> >index 0000000000000000000000000000000000000000..a79511cc3c6db039aaa1f5528db19b55f75c7a2d
> >GIT binary patch
> >literal 2784
> >zcmbtVOKTHR6h4#N*w)yPHa=P^GJ;QZ#zabO)oL_a>jM$Ng@VLMa?=ExiA*Ln>Ouq&
> >z+__LhaOa=UwSUBoKftvMJ!j^g<o0HY3s1<s=X~dR&wb3xtG8BDmIX`}T!5j(D1g0V
> >z$d@9v1bLW&duwlhekJ+gcWB`>7U?FU5HTMC@sQE@6!DgoF5@&QSRuyJEkBH-SRzd@
> >z88C4G>oZu>*5PbEBeL_^X=`RYw+|q-03v%fb1t)-xt_l`mzo6-hai)=kP#DDnF|-^
> >zSnp@A=gF-!`|<J|Mn8%Fs3Jg1jX0qOMeHh$0*>NnS--PWI7U6hWm^!+Zs=@CuTgL|
> >z3vR6zIi&M287Dw-cY5c|v~@p~Lcdzm^Vem)(&+iX{H7O-M!9zTo?Sk1qPSo$yQkfv
> >zeY#LwF5u60+F{R^E9=siL92$M7u1?7{iQ``alv-hZKvUNpZT?B(D6IHuG90Mw*0yi
> >zdesftg;Qbs2~-{wi<}$l*aWy%E}ylJU6Iw6=g0Y+rQ=58XkELb5tse^E?E7XHNR_o
> >z;vKeOKjQQ!ikf&Sn|qTgPwiMmgBVU6>EMjPuRlpjk$5s`BL<z_;wDg&L_(cXQzt+>
> >z<eE~zxF!u*0h2>P<3zk-4T)I+QzIAw0<zoSyGHOCM5eWM@f|yW_>9({!NDCljl9ux
> >z$VNAwJFpYuF;7KsC4|$ej}wSA9Z$eG-noeO82C#i(`6%~eb#r4<53*_e7Izc#PPX~
> >zb1@vn(Ytz6;cTx&xNG3AmHa@>W<9)iiW5CU$}YE!{o*#bt#DeazFu&<&)RqgaSnrc
> >zZZa3jeh6;e3q5eF-7dI+-0<AGtoGI`Ex*x0UBr9UY9KeasJ(z*ldUcrEnl(=rU19r
> >zX}2Z5vaa6=rHk*1(+$1aV--j?D~-TwOI7%PPV?wbz2$y#xRPQ9=TymLtJ6Ug2s3Xo
> >z@3U@8yy;I*G@U2Y{}ve|@%@RVQMrf;GbY)xf14fEY}%egoN|<uo~N8j9DgYR=v)!R
> >zXP@uU3SwqVit&vGq}%Z{7){$ds4+1z9#>#t;2ATfE%iC6{#kV?*q`dBSJJG%j+hw}
> >zAvmMJ8zg9C+E!4Plz&a-=RQ$>dY8=nPY|P+Tqm*-y;6xjFra46&+`k!ll=Fp%Y5V?
> >z{;!q4VFu^7JO)E8=Vj!R@;A*(3Rq%8HS_uzeveF2{~jgukG?|e&wZtO%-j^aufV1%
> >mn7#w_4x9e8C+6Kl2!1H=6bTxc>!-L${(V*d7tNPg)Bi7zRJKt7
> >
> >literal 0
> >HcmV?d00001
> >
> >diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.cc b/tests/data/test-abidiff-exit/test-leaf0-v1.cc
> >new file mode 100644
> >index 00000000..020cb761
> >--- /dev/null
> >+++ b/tests/data/test-abidiff-exit/test-leaf0-v1.cc
> >@@ -0,0 +1,5 @@
> >+long changed_var = 0;
> >+
> >+long changed_fun() {
> >+  return 0;
> >+}
> >diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.o b/tests/data/test-abidiff-exit/test-leaf0-v1.o
> >new file mode 100644
> >index 0000000000000000000000000000000000000000..4a433b9572abb46799391619f21538b54fb3204c
> >GIT binary patch
> >literal 2824
> >zcmbtVOOF#r5UzH!FU(r|0KsYl8k84^%&<nQ)h??AgOh~~5kg2Df`qKG-F_(ccr_kx
> >zmVlH?K;(eL0SO@?gp@y#3nzX{?#Vd^Qq|LCx7))hWx1>B>w0zdV}AYahuxeIz$D-b
> >zj5J08ZcdNnk`_y_3`=nD=I?*~N&15i8g1ivL`$<eOe$+29rBi(BUy;)GO{_z6LNNn
> >zMC~c0&PxVN96-DQu_(SQEazomwXi6bW)@BY<kmqJF6X!NTlu}^t1G!v0P+jS=dYj@
> >z#th)<3eQJ{y=8JMoqBbB1*3n1e_avGaxTp|;TVX@F8(EajiV6%7Upr(eZ*x8NK`-Z
> >zJ1T6f`3J?IR@1&y$6+o_fa>w7qqnBSkGUNB)mmZHRP{<@5CO{vVGI|O?ml#SQPXQj
> >z3Ea4S&nsWJP+a%6f=fZsyR=r^TEm~`cauS+cA6?ual3|U7}r`mhnpMz#=7S>J--q5
> >zA4auS+>3gHzCVb5Zbx-L39B8|hsyn8k#l1HgN)K|l*^aBbKk0JJB-rY{^oh72)#gB
> >zmJ!4+cR?I3h}B~=voEkEpCL|vBHhqyg@vcN^8EBJ8l-UMNQdYM{PQtuQf8i1+LU3J
> >z$J`{&WRWx{)}51J9eQh0!i=SjL<w^v#gb&YVhc${3G)*e0TS@2W{`wq6X+5mi`Kfh
> >zmuC=PvidVT#1kizH=hpN#9<hA;3&qkyTO$ZPOJV3|3r%^1IC{l{e*+#z0^b(kBIi!
> >z-nH`@1s#XiL{MPil$Yz+HaNwxyGoaoi0e6>s&LD2#*GSh9sIG;?;Gyc$vvSs`q?se
> >z3!kvx_yq15oO)pQ0fPR+E?!mSNu2T)ONly6AgG5)2tl>qhags+FsQ5Qpjl~0jULWv
> >zIjmM=bucy^#`NNhyL8%-Vi!ySL9N&ADtvf?sF$b!pC7-UgtebcAl0fg;;^es;s2AZ
> >zqCfSP`^n)-Mi`!7qm!*oXHg-{z2Usiwk^r7KRx4g&RqXHWQ=5=!7ROqpc|9y*uP7P
> >zMXoJ<eCUf;vTR87%m>sn4s@;v;(GWV?I7mHL<ru~fOI=$!_l>+I$ey6^9EcSdZvPF
> >zOMT9&zho{2`&0e&db;)35p!cA1ltDuo)n8*+X~KQ<=;2?xlfdz-VHbZK4KJ;>qJr0
> >z6O-sI1BP>c=8q81^8egiW_}+z4Er(v&G_>?^S!ahfYHfby*B=?`5gmnj$F0G>u2~S
> >z3R(SomeD`@I<Y_ZmDb_rrr6I6*fI;H?*qNhu0QRGd-o86zYTbi6pOfT3{l)H|97VT
> >KKiJT4*Z)8NIl2)5
> >
> >literal 0
> >HcmV?d00001
> >
> >diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
> >index aea57c32..5372b3fe 100644
> >--- a/tests/test-abidiff-exit.cc
> >+++ b/tests/test-abidiff-exit.cc
> >@@ -120,6 +120,15 @@ InOutSpec in_out_specs[] =
> >     "data/test-abidiff-exit/test-no-stray-comma-report.txt",
> >     "output/test-abidiff-exit/test-no-stray-comma-report.txt"
> >   },
> >+  {
> >+    "data/test-abidiff-exit/test-leaf0-v0.o",
> >+    "data/test-abidiff-exit/test-leaf0-v1.o",
> >+    "",
> >+    "--no-show-locs --leaf-changes-only",
> >+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
> >+    "data/test-abidiff-exit/test-leaf0-report.txt",
> >+    "output/test-abidiff-exit/test-leaf0-report.txt"
> >+  },
> >   {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
> > };
> >
> >--
> >2.25.0.265.gbab2e86ba0-goog
> >
> >

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] Fix the reporting of leaf change statistics.
  2020-01-01  0:00         ` Dodji Seketeli
@ 2020-01-01  0:00           ` Giuliano Procida via libabigail
  0 siblings, 0 replies; 14+ messages in thread
From: Giuliano Procida via libabigail @ 2020-01-01  0:00 UTC (permalink / raw)
  To: Dodji Seketeli; +Cc: Matthias Maennich, libabigail, kernel-team

I'll happily add the source files to the Makefile. Patch to follow.

Regards,
Giuliano.

On Fri, 6 Mar 2020 at 10:09, Dodji Seketeli <dodji@seketeli.org> wrote:
>
> Hello,
>
> Giuliano Procida <gprocida@google.com> a écrit:
>
> [...]
>
>
> >> >diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
> >> >index 5031e6d3..07077608 100644
> >> >--- a/tests/data/Makefile.am
> >> >+++ b/tests/data/Makefile.am
> >> >@@ -110,6 +110,11 @@ test-abidiff-exit/test-loc-without-locs-report.txt \
> >> > test-abidiff-exit/test-no-stray-comma-report.txt \
> >> > test-abidiff-exit/test-no-stray-comma-v0.o \
> >> > test-abidiff-exit/test-no-stray-comma-v1.o \
> >> >+test-abidiff-exit/test-leaf0-v0.cc \
> >>
> >> I do not think we need to 'distribute' the source files here. I mean, it
> >> will not harm, but they are mostly there for reference and not needed
> >> during `make distcheck`.
> >
> > I was following someone else's example. Agreed, they are not needed in
> > the Makefile.
>
> Matthias is right in saying that we don't *need* to distribute the
> source files.
>
> However, I tend to always distribute them when we have them around, so
> that even people who only have access to the tarball (yes that can still
> happen even in this day and age) can tinker around with the source code,
> make changes and try out stuff.
>
> I thought I'd just throw this point of view in.
>
> Thanks!
>
> --
>                 Dodji
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4] Fix the reporting of leaf change statistics.
  2020-01-01  0:00   ` [PATCH v3] " Giuliano Procida via libabigail
  2020-01-01  0:00     ` Matthias Maennich via libabigail
@ 2020-01-01  0:00     ` Giuliano Procida via libabigail
  2020-03-09 15:10       ` [PATCH v5] " Giuliano Procida
  1 sibling, 1 reply; 14+ messages in thread
From: Giuliano Procida via libabigail @ 2020-01-01  0:00 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida, Matthias Maennich

Leaf changes (as reported with --leaf-changes-only) to variables were
miscounted as changes to functions.

	* src/abg-comparison.cc
	(apply_filters_and_compute_diff_stats):	Increment the correct
	counter for leaf variable changes.
	* tests/data/Makefile.am: Add new test case files.
	* tests/data/test-abidiff-exit/test-leaf0-report.txt: New test
	case.
	* tests/data/test-abidiff-exit/test-leaf0-v0.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf0-v0.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf0-v1.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf0-v1.o: Ditto.
	* tests/test-abidiff-exit.cc: Run new test case.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-comparison.cc                            |   4 ++--
 tests/data/Makefile.am                           |   3 +++
 .../data/test-abidiff-exit/test-leaf0-report.txt |  13 +++++++++++++
 tests/data/test-abidiff-exit/test-leaf0-v0.cc    |   5 +++++
 tests/data/test-abidiff-exit/test-leaf0-v0.o     | Bin 0 -> 2784 bytes
 tests/data/test-abidiff-exit/test-leaf0-v1.cc    |   5 +++++
 tests/data/test-abidiff-exit/test-leaf0-v1.o     | Bin 0 -> 2824 bytes
 tests/test-abidiff-exit.cc                       |   9 +++++++++
 8 files changed, 37 insertions(+), 2 deletions(-)
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-report.txt
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.cc
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.o
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.cc
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.o

diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 5371e843..ef753e6d 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -9767,8 +9767,8 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat)
 	      (stat.num_leaf_var_changes_filtered_out() + 1);
 	}
       if ((*i)->has_local_changes())
-	stat.num_leaf_func_changes
-	  (stat.num_leaf_func_changes() + 1);
+	stat.num_leaf_var_changes
+	  (stat.num_leaf_var_changes() + 1);
     }
 
   stat.num_func_syms_added(added_unrefed_fn_syms_.size());
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 5031e6d3..04653e86 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -110,6 +110,9 @@ test-abidiff-exit/test-loc-without-locs-report.txt \
 test-abidiff-exit/test-no-stray-comma-report.txt \
 test-abidiff-exit/test-no-stray-comma-v0.o \
 test-abidiff-exit/test-no-stray-comma-v1.o \
+test-abidiff-exit/test-leaf0-v0.o \
+test-abidiff-exit/test-leaf0-v1.o \
+test-abidiff-exit/test-leaf0-report.txt \
 \
 test-diff-dwarf/test0-v0.cc		\
 test-diff-dwarf/test0-v0.o			\
diff --git a/tests/data/test-abidiff-exit/test-leaf0-report.txt b/tests/data/test-abidiff-exit/test-leaf0-report.txt
new file mode 100644
index 00000000..46f39976
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf0-report.txt
@@ -0,0 +1,13 @@
+Leaf changes summary: 2 artifacts changed
+Changed leaf types summary: 0 leaf type changed
+Removed/Changed/Added functions summary: 0 Removed, 1 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 1 Changed, 0 Added variable
+
+1 function with some sub-type change:
+
+  [C]'function int changed_fun()' has some sub-type changes:
+    return type changed:
+      type name changed from 'int' to 'long int'
+      type size changed from 32 to 64 (in bits)
+
+
diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.cc b/tests/data/test-abidiff-exit/test-leaf0-v0.cc
new file mode 100644
index 00000000..27ba39c9
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf0-v0.cc
@@ -0,0 +1,5 @@
+int changed_var = 0;
+
+int changed_fun() {
+  return 0;
+}
diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.o b/tests/data/test-abidiff-exit/test-leaf0-v0.o
new file mode 100644
index 0000000000000000000000000000000000000000..a79511cc3c6db039aaa1f5528db19b55f75c7a2d
GIT binary patch
literal 2784
zcmbtVOKTHR6h4#N*w)yPHa=P^GJ;QZ#zabO)oL_a>jM$Ng@VLMa?=ExiA*Ln>Ouq&
z+__LhaOa=UwSUBoKftvMJ!j^g<o0HY3s1<s=X~dR&wb3xtG8BDmIX`}T!5j(D1g0V
z$d@9v1bLW&duwlhekJ+gcWB`>7U?FU5HTMC@sQE@6!DgoF5@&QSRuyJEkBH-SRzd@
z88C4G>oZu>*5PbEBeL_^X=`RYw+|q-03v%fb1t)-xt_l`mzo6-hai)=kP#DDnF|-^
zSnp@A=gF-!`|<J|Mn8%Fs3Jg1jX0qOMeHh$0*>NnS--PWI7U6hWm^!+Zs=@CuTgL|
z3vR6zIi&M287Dw-cY5c|v~@p~Lcdzm^Vem)(&+iX{H7O-M!9zTo?Sk1qPSo$yQkfv
zeY#LwF5u60+F{R^E9=siL92$M7u1?7{iQ``alv-hZKvUNpZT?B(D6IHuG90Mw*0yi
zdesftg;Qbs2~-{wi<}$l*aWy%E}ylJU6Iw6=g0Y+rQ=58XkELb5tse^E?E7XHNR_o
z;vKeOKjQQ!ikf&Sn|qTgPwiMmgBVU6>EMjPuRlpjk$5s`BL<z_;wDg&L_(cXQzt+>
z<eE~zxF!u*0h2>P<3zk-4T)I+QzIAw0<zoSyGHOCM5eWM@f|yW_>9({!NDCljl9ux
z$VNAwJFpYuF;7KsC4|$ej}wSA9Z$eG-noeO82C#i(`6%~eb#r4<53*_e7Izc#PPX~
zb1@vn(Ytz6;cTx&xNG3AmHa@>W<9)iiW5CU$}YE!{o*#bt#DeazFu&<&)RqgaSnrc
zZZa3jeh6;e3q5eF-7dI+-0<AGtoGI`Ex*x0UBr9UY9KeasJ(z*ldUcrEnl(=rU19r
zX}2Z5vaa6=rHk*1(+$1aV--j?D~-TwOI7%PPV?wbz2$y#xRPQ9=TymLtJ6Ug2s3Xo
z@3U@8yy;I*G@U2Y{}ve|@%@RVQMrf;GbY)xf14fEY}%egoN|<uo~N8j9DgYR=v)!R
zXP@uU3SwqVit&vGq}%Z{7){$ds4+1z9#>#t;2ATfE%iC6{#kV?*q`dBSJJG%j+hw}
zAvmMJ8zg9C+E!4Plz&a-=RQ$>dY8=nPY|P+Tqm*-y;6xjFra46&+`k!ll=Fp%Y5V?
z{;!q4VFu^7JO)E8=Vj!R@;A*(3Rq%8HS_uzeveF2{~jgukG?|e&wZtO%-j^aufV1%
mn7#w_4x9e8C+6Kl2!1H=6bTxc>!-L${(V*d7tNPg)Bi7zRJKt7

literal 0
HcmV?d00001

diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.cc b/tests/data/test-abidiff-exit/test-leaf0-v1.cc
new file mode 100644
index 00000000..020cb761
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf0-v1.cc
@@ -0,0 +1,5 @@
+long changed_var = 0;
+
+long changed_fun() {
+  return 0;
+}
diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.o b/tests/data/test-abidiff-exit/test-leaf0-v1.o
new file mode 100644
index 0000000000000000000000000000000000000000..4a433b9572abb46799391619f21538b54fb3204c
GIT binary patch
literal 2824
zcmbtVOOF#r5UzH!FU(r|0KsYl8k84^%&<nQ)h??AgOh~~5kg2Df`qKG-F_(ccr_kx
zmVlH?K;(eL0SO@?gp@y#3nzX{?#Vd^Qq|LCx7))hWx1>B>w0zdV}AYahuxeIz$D-b
zj5J08ZcdNnk`_y_3`=nD=I?*~N&15i8g1ivL`$<eOe$+29rBi(BUy;)GO{_z6LNNn
zMC~c0&PxVN96-DQu_(SQEazomwXi6bW)@BY<kmqJF6X!NTlu}^t1G!v0P+jS=dYj@
z#th)<3eQJ{y=8JMoqBbB1*3n1e_avGaxTp|;TVX@F8(EajiV6%7Upr(eZ*x8NK`-Z
zJ1T6f`3J?IR@1&y$6+o_fa>w7qqnBSkGUNB)mmZHRP{<@5CO{vVGI|O?ml#SQPXQj
z3Ea4S&nsWJP+a%6f=fZsyR=r^TEm~`cauS+cA6?ual3|U7}r`mhnpMz#=7S>J--q5
zA4auS+>3gHzCVb5Zbx-L39B8|hsyn8k#l1HgN)K|l*^aBbKk0JJB-rY{^oh72)#gB
zmJ!4+cR?I3h}B~=voEkEpCL|vBHhqyg@vcN^8EBJ8l-UMNQdYM{PQtuQf8i1+LU3J
z$J`{&WRWx{)}51J9eQh0!i=SjL<w^v#gb&YVhc${3G)*e0TS@2W{`wq6X+5mi`Kfh
zmuC=PvidVT#1kizH=hpN#9<hA;3&qkyTO$ZPOJV3|3r%^1IC{l{e*+#z0^b(kBIi!
z-nH`@1s#XiL{MPil$Yz+HaNwxyGoaoi0e6>s&LD2#*GSh9sIG;?;Gyc$vvSs`q?se
z3!kvx_yq15oO)pQ0fPR+E?!mSNu2T)ONly6AgG5)2tl>qhags+FsQ5Qpjl~0jULWv
zIjmM=bucy^#`NNhyL8%-Vi!ySL9N&ADtvf?sF$b!pC7-UgtebcAl0fg;;^es;s2AZ
zqCfSP`^n)-Mi`!7qm!*oXHg-{z2Usiwk^r7KRx4g&RqXHWQ=5=!7ROqpc|9y*uP7P
zMXoJ<eCUf;vTR87%m>sn4s@;v;(GWV?I7mHL<ru~fOI=$!_l>+I$ey6^9EcSdZvPF
zOMT9&zho{2`&0e&db;)35p!cA1ltDuo)n8*+X~KQ<=;2?xlfdz-VHbZK4KJ;>qJr0
z6O-sI1BP>c=8q81^8egiW_}+z4Er(v&G_>?^S!ahfYHfby*B=?`5gmnj$F0G>u2~S
z3R(SomeD`@I<Y_ZmDb_rrr6I6*fI;H?*qNhu0QRGd-o86zYTbi6pOfT3{l)H|97VT
KKiJT4*Z)8NIl2)5

literal 0
HcmV?d00001

diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index aea57c32..5372b3fe 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -120,6 +120,15 @@ InOutSpec in_out_specs[] =
     "data/test-abidiff-exit/test-no-stray-comma-report.txt",
     "output/test-abidiff-exit/test-no-stray-comma-report.txt"
   },
+  {
+    "data/test-abidiff-exit/test-leaf0-v0.o",
+    "data/test-abidiff-exit/test-leaf0-v1.o",
+    "",
+    "--no-show-locs --leaf-changes-only",
+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+    "data/test-abidiff-exit/test-leaf0-report.txt",
+    "output/test-abidiff-exit/test-leaf0-report.txt"
+  },
   {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
 };
 
-- 
2.25.0.265.gbab2e86ba0-goog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v5] Fix the reporting of leaf change statistics.
  2020-01-01  0:00     ` [PATCH v4] " Giuliano Procida via libabigail
@ 2020-03-09 15:10       ` Giuliano Procida
  2020-03-09 15:12         ` Giuliano Procida
  2020-03-10 16:47         ` Dodji Seketeli
  0 siblings, 2 replies; 14+ messages in thread
From: Giuliano Procida @ 2020-03-09 15:10 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida, Matthias Maennich

Leaf changes (as reported with --leaf-changes-only) to variables were
miscounted as changes to functions.

	* src/abg-comparison.cc
	(apply_filters_and_compute_diff_stats):	Increment the correct
	counter for leaf variable changes.
	* tests/data/Makefile.am: Add new test case files.
	* tests/data/test-abidiff-exit/test-leaf0-report.txt: New test
	case.
	* tests/data/test-abidiff-exit/test-leaf0-v0.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf0-v0.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf0-v1.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf0-v1.o: Ditto.
	* tests/test-abidiff-exit.cc: Run new test case.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-comparison.cc                            |   4 ++--
 tests/data/Makefile.am                           |   5 +++++
 .../data/test-abidiff-exit/test-leaf0-report.txt |  13 +++++++++++++
 tests/data/test-abidiff-exit/test-leaf0-v0.cc    |   5 +++++
 tests/data/test-abidiff-exit/test-leaf0-v0.o     | Bin 0 -> 2784 bytes
 tests/data/test-abidiff-exit/test-leaf0-v1.cc    |   5 +++++
 tests/data/test-abidiff-exit/test-leaf0-v1.o     | Bin 0 -> 2824 bytes
 tests/test-abidiff-exit.cc                       |   9 +++++++++
 8 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-report.txt
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.cc
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.o
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.cc
 create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.o

diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 5371e843..ef753e6d 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -9767,8 +9767,8 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat)
 	      (stat.num_leaf_var_changes_filtered_out() + 1);
 	}
       if ((*i)->has_local_changes())
-	stat.num_leaf_func_changes
-	  (stat.num_leaf_func_changes() + 1);
+	stat.num_leaf_var_changes
+	  (stat.num_leaf_var_changes() + 1);
     }
 
   stat.num_func_syms_added(added_unrefed_fn_syms_.size());
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 5031e6d3..07077608 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -110,6 +110,11 @@ test-abidiff-exit/test-loc-without-locs-report.txt \
 test-abidiff-exit/test-no-stray-comma-report.txt \
 test-abidiff-exit/test-no-stray-comma-v0.o \
 test-abidiff-exit/test-no-stray-comma-v1.o \
+test-abidiff-exit/test-leaf0-v0.cc \
+test-abidiff-exit/test-leaf0-v0.o \
+test-abidiff-exit/test-leaf0-v1.cc \
+test-abidiff-exit/test-leaf0-v1.o \
+test-abidiff-exit/test-leaf0-report.txt \
 \
 test-diff-dwarf/test0-v0.cc		\
 test-diff-dwarf/test0-v0.o			\
diff --git a/tests/data/test-abidiff-exit/test-leaf0-report.txt b/tests/data/test-abidiff-exit/test-leaf0-report.txt
new file mode 100644
index 00000000..f823789d
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf0-report.txt
@@ -0,0 +1,13 @@
+Leaf changes summary: 2 artifacts changed
+Changed leaf types summary: 0 leaf type changed
+Removed/Changed/Added functions summary: 0 Removed, 1 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 1 Changed, 0 Added variable
+
+1 function with some sub-type change:
+
+  [C] 'function int changed_fun()' has some sub-type changes:
+    return type changed:
+      type name changed from 'int' to 'long int'
+      type size changed from 32 to 64 (in bits)
+
+
diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.cc b/tests/data/test-abidiff-exit/test-leaf0-v0.cc
new file mode 100644
index 00000000..27ba39c9
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf0-v0.cc
@@ -0,0 +1,5 @@
+int changed_var = 0;
+
+int changed_fun() {
+  return 0;
+}
diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.o b/tests/data/test-abidiff-exit/test-leaf0-v0.o
new file mode 100644
index 0000000000000000000000000000000000000000..a79511cc3c6db039aaa1f5528db19b55f75c7a2d
GIT binary patch
literal 2784
zcmbtVOKTHR6h4#N*w)yPHa=P^GJ;QZ#zabO)oL_a>jM$Ng@VLMa?=ExiA*Ln>Ouq&
z+__LhaOa=UwSUBoKftvMJ!j^g<o0HY3s1<s=X~dR&wb3xtG8BDmIX`}T!5j(D1g0V
z$d@9v1bLW&duwlhekJ+gcWB`>7U?FU5HTMC@sQE@6!DgoF5@&QSRuyJEkBH-SRzd@
z88C4G>oZu>*5PbEBeL_^X=`RYw+|q-03v%fb1t)-xt_l`mzo6-hai)=kP#DDnF|-^
zSnp@A=gF-!`|<J|Mn8%Fs3Jg1jX0qOMeHh$0*>NnS--PWI7U6hWm^!+Zs=@CuTgL|
z3vR6zIi&M287Dw-cY5c|v~@p~Lcdzm^Vem)(&+iX{H7O-M!9zTo?Sk1qPSo$yQkfv
zeY#LwF5u60+F{R^E9=siL92$M7u1?7{iQ``alv-hZKvUNpZT?B(D6IHuG90Mw*0yi
zdesftg;Qbs2~-{wi<}$l*aWy%E}ylJU6Iw6=g0Y+rQ=58XkELb5tse^E?E7XHNR_o
z;vKeOKjQQ!ikf&Sn|qTgPwiMmgBVU6>EMjPuRlpjk$5s`BL<z_;wDg&L_(cXQzt+>
z<eE~zxF!u*0h2>P<3zk-4T)I+QzIAw0<zoSyGHOCM5eWM@f|yW_>9({!NDCljl9ux
z$VNAwJFpYuF;7KsC4|$ej}wSA9Z$eG-noeO82C#i(`6%~eb#r4<53*_e7Izc#PPX~
zb1@vn(Ytz6;cTx&xNG3AmHa@>W<9)iiW5CU$}YE!{o*#bt#DeazFu&<&)RqgaSnrc
zZZa3jeh6;e3q5eF-7dI+-0<AGtoGI`Ex*x0UBr9UY9KeasJ(z*ldUcrEnl(=rU19r
zX}2Z5vaa6=rHk*1(+$1aV--j?D~-TwOI7%PPV?wbz2$y#xRPQ9=TymLtJ6Ug2s3Xo
z@3U@8yy;I*G@U2Y{}ve|@%@RVQMrf;GbY)xf14fEY}%egoN|<uo~N8j9DgYR=v)!R
zXP@uU3SwqVit&vGq}%Z{7){$ds4+1z9#>#t;2ATfE%iC6{#kV?*q`dBSJJG%j+hw}
zAvmMJ8zg9C+E!4Plz&a-=RQ$>dY8=nPY|P+Tqm*-y;6xjFra46&+`k!ll=Fp%Y5V?
z{;!q4VFu^7JO)E8=Vj!R@;A*(3Rq%8HS_uzeveF2{~jgukG?|e&wZtO%-j^aufV1%
mn7#w_4x9e8C+6Kl2!1H=6bTxc>!-L${(V*d7tNPg)Bi7zRJKt7

literal 0
HcmV?d00001

diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.cc b/tests/data/test-abidiff-exit/test-leaf0-v1.cc
new file mode 100644
index 00000000..020cb761
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf0-v1.cc
@@ -0,0 +1,5 @@
+long changed_var = 0;
+
+long changed_fun() {
+  return 0;
+}
diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.o b/tests/data/test-abidiff-exit/test-leaf0-v1.o
new file mode 100644
index 0000000000000000000000000000000000000000..4a433b9572abb46799391619f21538b54fb3204c
GIT binary patch
literal 2824
zcmbtVOOF#r5UzH!FU(r|0KsYl8k84^%&<nQ)h??AgOh~~5kg2Df`qKG-F_(ccr_kx
zmVlH?K;(eL0SO@?gp@y#3nzX{?#Vd^Qq|LCx7))hWx1>B>w0zdV}AYahuxeIz$D-b
zj5J08ZcdNnk`_y_3`=nD=I?*~N&15i8g1ivL`$<eOe$+29rBi(BUy;)GO{_z6LNNn
zMC~c0&PxVN96-DQu_(SQEazomwXi6bW)@BY<kmqJF6X!NTlu}^t1G!v0P+jS=dYj@
z#th)<3eQJ{y=8JMoqBbB1*3n1e_avGaxTp|;TVX@F8(EajiV6%7Upr(eZ*x8NK`-Z
zJ1T6f`3J?IR@1&y$6+o_fa>w7qqnBSkGUNB)mmZHRP{<@5CO{vVGI|O?ml#SQPXQj
z3Ea4S&nsWJP+a%6f=fZsyR=r^TEm~`cauS+cA6?ual3|U7}r`mhnpMz#=7S>J--q5
zA4auS+>3gHzCVb5Zbx-L39B8|hsyn8k#l1HgN)K|l*^aBbKk0JJB-rY{^oh72)#gB
zmJ!4+cR?I3h}B~=voEkEpCL|vBHhqyg@vcN^8EBJ8l-UMNQdYM{PQtuQf8i1+LU3J
z$J`{&WRWx{)}51J9eQh0!i=SjL<w^v#gb&YVhc${3G)*e0TS@2W{`wq6X+5mi`Kfh
zmuC=PvidVT#1kizH=hpN#9<hA;3&qkyTO$ZPOJV3|3r%^1IC{l{e*+#z0^b(kBIi!
z-nH`@1s#XiL{MPil$Yz+HaNwxyGoaoi0e6>s&LD2#*GSh9sIG;?;Gyc$vvSs`q?se
z3!kvx_yq15oO)pQ0fPR+E?!mSNu2T)ONly6AgG5)2tl>qhags+FsQ5Qpjl~0jULWv
zIjmM=bucy^#`NNhyL8%-Vi!ySL9N&ADtvf?sF$b!pC7-UgtebcAl0fg;;^es;s2AZ
zqCfSP`^n)-Mi`!7qm!*oXHg-{z2Usiwk^r7KRx4g&RqXHWQ=5=!7ROqpc|9y*uP7P
zMXoJ<eCUf;vTR87%m>sn4s@;v;(GWV?I7mHL<ru~fOI=$!_l>+I$ey6^9EcSdZvPF
zOMT9&zho{2`&0e&db;)35p!cA1ltDuo)n8*+X~KQ<=;2?xlfdz-VHbZK4KJ;>qJr0
z6O-sI1BP>c=8q81^8egiW_}+z4Er(v&G_>?^S!ahfYHfby*B=?`5gmnj$F0G>u2~S
z3R(SomeD`@I<Y_ZmDb_rrr6I6*fI;H?*qNhu0QRGd-o86zYTbi6pOfT3{l)H|97VT
KKiJT4*Z)8NIl2)5

literal 0
HcmV?d00001

diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index aea57c32..5372b3fe 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -120,6 +120,15 @@ InOutSpec in_out_specs[] =
     "data/test-abidiff-exit/test-no-stray-comma-report.txt",
     "output/test-abidiff-exit/test-no-stray-comma-report.txt"
   },
+  {
+    "data/test-abidiff-exit/test-leaf0-v0.o",
+    "data/test-abidiff-exit/test-leaf0-v1.o",
+    "",
+    "--no-show-locs --leaf-changes-only",
+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+    "data/test-abidiff-exit/test-leaf0-report.txt",
+    "output/test-abidiff-exit/test-leaf0-report.txt"
+  },
   {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
 };
 
-- 
2.25.1.481.gfbce0eb801-goog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5] Fix the reporting of leaf change statistics.
  2020-03-09 15:10       ` [PATCH v5] " Giuliano Procida
@ 2020-03-09 15:12         ` Giuliano Procida
  2020-03-10 16:47         ` Dodji Seketeli
  1 sibling, 0 replies; 14+ messages in thread
From: Giuliano Procida @ 2020-03-09 15:12 UTC (permalink / raw)
  To: libabigail; +Cc: Dodji Seketeli, kernel-team, Matthias Maennich

This has also been updated to include the extra spacing after [C]. I
don't think I've posted a version with that before now.

Regards,
Giuliano.


On Mon, 9 Mar 2020 at 15:10, Giuliano Procida <gprocida@google.com> wrote:
>
> Leaf changes (as reported with --leaf-changes-only) to variables were
> miscounted as changes to functions.
>
>         * src/abg-comparison.cc
>         (apply_filters_and_compute_diff_stats): Increment the correct
>         counter for leaf variable changes.
>         * tests/data/Makefile.am: Add new test case files.
>         * tests/data/test-abidiff-exit/test-leaf0-report.txt: New test
>         case.
>         * tests/data/test-abidiff-exit/test-leaf0-v0.cc: Ditto.
>         * tests/data/test-abidiff-exit/test-leaf0-v0.o: Ditto.
>         * tests/data/test-abidiff-exit/test-leaf0-v1.cc: Ditto.
>         * tests/data/test-abidiff-exit/test-leaf0-v1.o: Ditto.
>         * tests/test-abidiff-exit.cc: Run new test case.
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
> Signed-off-by: Giuliano Procida <gprocida@google.com>
> ---
>  src/abg-comparison.cc                            |   4 ++--
>  tests/data/Makefile.am                           |   5 +++++
>  .../data/test-abidiff-exit/test-leaf0-report.txt |  13 +++++++++++++
>  tests/data/test-abidiff-exit/test-leaf0-v0.cc    |   5 +++++
>  tests/data/test-abidiff-exit/test-leaf0-v0.o     | Bin 0 -> 2784 bytes
>  tests/data/test-abidiff-exit/test-leaf0-v1.cc    |   5 +++++
>  tests/data/test-abidiff-exit/test-leaf0-v1.o     | Bin 0 -> 2824 bytes
>  tests/test-abidiff-exit.cc                       |   9 +++++++++
>  8 files changed, 39 insertions(+), 2 deletions(-)
>  create mode 100644 tests/data/test-abidiff-exit/test-leaf0-report.txt
>  create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.cc
>  create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v0.o
>  create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.cc
>  create mode 100644 tests/data/test-abidiff-exit/test-leaf0-v1.o
>
> diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
> index 5371e843..ef753e6d 100644
> --- a/src/abg-comparison.cc
> +++ b/src/abg-comparison.cc
> @@ -9767,8 +9767,8 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat)
>               (stat.num_leaf_var_changes_filtered_out() + 1);
>         }
>        if ((*i)->has_local_changes())
> -       stat.num_leaf_func_changes
> -         (stat.num_leaf_func_changes() + 1);
> +       stat.num_leaf_var_changes
> +         (stat.num_leaf_var_changes() + 1);
>      }
>
>    stat.num_func_syms_added(added_unrefed_fn_syms_.size());
> diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
> index 5031e6d3..07077608 100644
> --- a/tests/data/Makefile.am
> +++ b/tests/data/Makefile.am
> @@ -110,6 +110,11 @@ test-abidiff-exit/test-loc-without-locs-report.txt \
>  test-abidiff-exit/test-no-stray-comma-report.txt \
>  test-abidiff-exit/test-no-stray-comma-v0.o \
>  test-abidiff-exit/test-no-stray-comma-v1.o \
> +test-abidiff-exit/test-leaf0-v0.cc \
> +test-abidiff-exit/test-leaf0-v0.o \
> +test-abidiff-exit/test-leaf0-v1.cc \
> +test-abidiff-exit/test-leaf0-v1.o \
> +test-abidiff-exit/test-leaf0-report.txt \
>  \
>  test-diff-dwarf/test0-v0.cc            \
>  test-diff-dwarf/test0-v0.o                     \
> diff --git a/tests/data/test-abidiff-exit/test-leaf0-report.txt b/tests/data/test-abidiff-exit/test-leaf0-report.txt
> new file mode 100644
> index 00000000..f823789d
> --- /dev/null
> +++ b/tests/data/test-abidiff-exit/test-leaf0-report.txt
> @@ -0,0 +1,13 @@
> +Leaf changes summary: 2 artifacts changed
> +Changed leaf types summary: 0 leaf type changed
> +Removed/Changed/Added functions summary: 0 Removed, 1 Changed, 0 Added function
> +Removed/Changed/Added variables summary: 0 Removed, 1 Changed, 0 Added variable
> +
> +1 function with some sub-type change:
> +
> +  [C] 'function int changed_fun()' has some sub-type changes:
> +    return type changed:
> +      type name changed from 'int' to 'long int'
> +      type size changed from 32 to 64 (in bits)
> +
> +
> diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.cc b/tests/data/test-abidiff-exit/test-leaf0-v0.cc
> new file mode 100644
> index 00000000..27ba39c9
> --- /dev/null
> +++ b/tests/data/test-abidiff-exit/test-leaf0-v0.cc
> @@ -0,0 +1,5 @@
> +int changed_var = 0;
> +
> +int changed_fun() {
> +  return 0;
> +}
> diff --git a/tests/data/test-abidiff-exit/test-leaf0-v0.o b/tests/data/test-abidiff-exit/test-leaf0-v0.o
> new file mode 100644
> index 0000000000000000000000000000000000000000..a79511cc3c6db039aaa1f5528db19b55f75c7a2d
> GIT binary patch
> literal 2784
> zcmbtVOKTHR6h4#N*w)yPHa=P^GJ;QZ#zabO)oL_a>jM$Ng@VLMa?=ExiA*Ln>Ouq&
> z+__LhaOa=UwSUBoKftvMJ!j^g<o0HY3s1<s=X~dR&wb3xtG8BDmIX`}T!5j(D1g0V
> z$d@9v1bLW&duwlhekJ+gcWB`>7U?FU5HTMC@sQE@6!DgoF5@&QSRuyJEkBH-SRzd@
> z88C4G>oZu>*5PbEBeL_^X=`RYw+|q-03v%fb1t)-xt_l`mzo6-hai)=kP#DDnF|-^
> zSnp@A=gF-!`|<J|Mn8%Fs3Jg1jX0qOMeHh$0*>NnS--PWI7U6hWm^!+Zs=@CuTgL|
> z3vR6zIi&M287Dw-cY5c|v~@p~Lcdzm^Vem)(&+iX{H7O-M!9zTo?Sk1qPSo$yQkfv
> zeY#LwF5u60+F{R^E9=siL92$M7u1?7{iQ``alv-hZKvUNpZT?B(D6IHuG90Mw*0yi
> zdesftg;Qbs2~-{wi<}$l*aWy%E}ylJU6Iw6=g0Y+rQ=58XkELb5tse^E?E7XHNR_o
> z;vKeOKjQQ!ikf&Sn|qTgPwiMmgBVU6>EMjPuRlpjk$5s`BL<z_;wDg&L_(cXQzt+>
> z<eE~zxF!u*0h2>P<3zk-4T)I+QzIAw0<zoSyGHOCM5eWM@f|yW_>9({!NDCljl9ux
> z$VNAwJFpYuF;7KsC4|$ej}wSA9Z$eG-noeO82C#i(`6%~eb#r4<53*_e7Izc#PPX~
> zb1@vn(Ytz6;cTx&xNG3AmHa@>W<9)iiW5CU$}YE!{o*#bt#DeazFu&<&)RqgaSnrc
> zZZa3jeh6;e3q5eF-7dI+-0<AGtoGI`Ex*x0UBr9UY9KeasJ(z*ldUcrEnl(=rU19r
> zX}2Z5vaa6=rHk*1(+$1aV--j?D~-TwOI7%PPV?wbz2$y#xRPQ9=TymLtJ6Ug2s3Xo
> z@3U@8yy;I*G@U2Y{}ve|@%@RVQMrf;GbY)xf14fEY}%egoN|<uo~N8j9DgYR=v)!R
> zXP@uU3SwqVit&vGq}%Z{7){$ds4+1z9#>#t;2ATfE%iC6{#kV?*q`dBSJJG%j+hw}
> zAvmMJ8zg9C+E!4Plz&a-=RQ$>dY8=nPY|P+Tqm*-y;6xjFra46&+`k!ll=Fp%Y5V?
> z{;!q4VFu^7JO)E8=Vj!R@;A*(3Rq%8HS_uzeveF2{~jgukG?|e&wZtO%-j^aufV1%
> mn7#w_4x9e8C+6Kl2!1H=6bTxc>!-L${(V*d7tNPg)Bi7zRJKt7
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.cc b/tests/data/test-abidiff-exit/test-leaf0-v1.cc
> new file mode 100644
> index 00000000..020cb761
> --- /dev/null
> +++ b/tests/data/test-abidiff-exit/test-leaf0-v1.cc
> @@ -0,0 +1,5 @@
> +long changed_var = 0;
> +
> +long changed_fun() {
> +  return 0;
> +}
> diff --git a/tests/data/test-abidiff-exit/test-leaf0-v1.o b/tests/data/test-abidiff-exit/test-leaf0-v1.o
> new file mode 100644
> index 0000000000000000000000000000000000000000..4a433b9572abb46799391619f21538b54fb3204c
> GIT binary patch
> literal 2824
> zcmbtVOOF#r5UzH!FU(r|0KsYl8k84^%&<nQ)h??AgOh~~5kg2Df`qKG-F_(ccr_kx
> zmVlH?K;(eL0SO@?gp@y#3nzX{?#Vd^Qq|LCx7))hWx1>B>w0zdV}AYahuxeIz$D-b
> zj5J08ZcdNnk`_y_3`=nD=I?*~N&15i8g1ivL`$<eOe$+29rBi(BUy;)GO{_z6LNNn
> zMC~c0&PxVN96-DQu_(SQEazomwXi6bW)@BY<kmqJF6X!NTlu}^t1G!v0P+jS=dYj@
> z#th)<3eQJ{y=8JMoqBbB1*3n1e_avGaxTp|;TVX@F8(EajiV6%7Upr(eZ*x8NK`-Z
> zJ1T6f`3J?IR@1&y$6+o_fa>w7qqnBSkGUNB)mmZHRP{<@5CO{vVGI|O?ml#SQPXQj
> z3Ea4S&nsWJP+a%6f=fZsyR=r^TEm~`cauS+cA6?ual3|U7}r`mhnpMz#=7S>J--q5
> zA4auS+>3gHzCVb5Zbx-L39B8|hsyn8k#l1HgN)K|l*^aBbKk0JJB-rY{^oh72)#gB
> zmJ!4+cR?I3h}B~=voEkEpCL|vBHhqyg@vcN^8EBJ8l-UMNQdYM{PQtuQf8i1+LU3J
> z$J`{&WRWx{)}51J9eQh0!i=SjL<w^v#gb&YVhc${3G)*e0TS@2W{`wq6X+5mi`Kfh
> zmuC=PvidVT#1kizH=hpN#9<hA;3&qkyTO$ZPOJV3|3r%^1IC{l{e*+#z0^b(kBIi!
> z-nH`@1s#XiL{MPil$Yz+HaNwxyGoaoi0e6>s&LD2#*GSh9sIG;?;Gyc$vvSs`q?se
> z3!kvx_yq15oO)pQ0fPR+E?!mSNu2T)ONly6AgG5)2tl>qhags+FsQ5Qpjl~0jULWv
> zIjmM=bucy^#`NNhyL8%-Vi!ySL9N&ADtvf?sF$b!pC7-UgtebcAl0fg;;^es;s2AZ
> zqCfSP`^n)-Mi`!7qm!*oXHg-{z2Usiwk^r7KRx4g&RqXHWQ=5=!7ROqpc|9y*uP7P
> zMXoJ<eCUf;vTR87%m>sn4s@;v;(GWV?I7mHL<ru~fOI=$!_l>+I$ey6^9EcSdZvPF
> zOMT9&zho{2`&0e&db;)35p!cA1ltDuo)n8*+X~KQ<=;2?xlfdz-VHbZK4KJ;>qJr0
> z6O-sI1BP>c=8q81^8egiW_}+z4Er(v&G_>?^S!ahfYHfby*B=?`5gmnj$F0G>u2~S
> z3R(SomeD`@I<Y_ZmDb_rrr6I6*fI;H?*qNhu0QRGd-o86zYTbi6pOfT3{l)H|97VT
> KKiJT4*Z)8NIl2)5
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
> index aea57c32..5372b3fe 100644
> --- a/tests/test-abidiff-exit.cc
> +++ b/tests/test-abidiff-exit.cc
> @@ -120,6 +120,15 @@ InOutSpec in_out_specs[] =
>      "data/test-abidiff-exit/test-no-stray-comma-report.txt",
>      "output/test-abidiff-exit/test-no-stray-comma-report.txt"
>    },
> +  {
> +    "data/test-abidiff-exit/test-leaf0-v0.o",
> +    "data/test-abidiff-exit/test-leaf0-v1.o",
> +    "",
> +    "--no-show-locs --leaf-changes-only",
> +    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
> +    "data/test-abidiff-exit/test-leaf0-report.txt",
> +    "output/test-abidiff-exit/test-leaf0-report.txt"
> +  },
>    {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
>  };
>
> --
> 2.25.1.481.gfbce0eb801-goog
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5] Fix the reporting of leaf change statistics.
  2020-03-09 15:10       ` [PATCH v5] " Giuliano Procida
  2020-03-09 15:12         ` Giuliano Procida
@ 2020-03-10 16:47         ` Dodji Seketeli
  1 sibling, 0 replies; 14+ messages in thread
From: Dodji Seketeli @ 2020-03-10 16:47 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: libabigail, kernel-team, Matthias Maennich

Hello Giuliano,

Giuliano Procida <gprocida@google.com> a écrit:

[...]

> diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
> index 5371e843..ef753e6d 100644
> --- a/src/abg-comparison.cc
> +++ b/src/abg-comparison.cc
> @@ -9767,8 +9767,8 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat)
>  	      (stat.num_leaf_var_changes_filtered_out() + 1);
>  	}
>        if ((*i)->has_local_changes())
> -	stat.num_leaf_func_changes
> -	  (stat.num_leaf_func_changes() + 1);
> +	stat.num_leaf_var_changes
> +	  (stat.num_leaf_var_changes() + 1);
>      }
>  

Whoah, good catch!

> Leaf changes (as reported with --leaf-changes-only) to variables were
> miscounted as changes to functions.
>
> 	* src/abg-comparison.cc
> 	(apply_filters_and_compute_diff_stats):	Increment the correct
> 	counter for leaf variable changes.
> 	* tests/data/Makefile.am: Add new test case files.
> 	* tests/data/test-abidiff-exit/test-leaf0-report.txt: New test
> 	case.
> 	* tests/data/test-abidiff-exit/test-leaf0-v0.cc: Ditto.
> 	* tests/data/test-abidiff-exit/test-leaf0-v0.o: Ditto.
> 	* tests/data/test-abidiff-exit/test-leaf0-v1.cc: Ditto.
> 	* tests/data/test-abidiff-exit/test-leaf0-v1.o: Ditto.
> 	* tests/test-abidiff-exit.cc: Run new test case.

This looks great to me and has been pushed to master.

Thanks!

-- 
		Dodji

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-03-10 16:47 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-01  0:00 [PATCH] Fix the reporting of leaf change statistics Giuliano Procida via libabigail
2020-01-01  0:00 ` [PATCH v2] " Giuliano Procida via libabigail
2020-01-01  0:00   ` Matthias Maennich via libabigail
2020-01-01  0:00     ` Matthias Maennich via libabigail
2020-01-01  0:00       ` Dodji Seketeli
2020-01-01  0:00   ` [PATCH v3] " Giuliano Procida via libabigail
2020-01-01  0:00     ` Matthias Maennich via libabigail
2020-01-01  0:00       ` Giuliano Procida via libabigail
2020-01-01  0:00         ` Dodji Seketeli
2020-01-01  0:00           ` Giuliano Procida via libabigail
2020-01-01  0:00     ` [PATCH v4] " Giuliano Procida via libabigail
2020-03-09 15:10       ` [PATCH v5] " Giuliano Procida
2020-03-09 15:12         ` Giuliano Procida
2020-03-10 16:47         ` Dodji Seketeli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).