The function maybe_report_data_members_replaced_by_anon_dm has some
minor issues. This commit tidies these up.
The function came to my attention when a broken equality test
triggered an infinite loop.
The issues included:
- anonymous_data_member, assigned but not used
- two iterators i, j used, when one would suffice
- dms_replaced_by_same_anon_dm declared outside loop, gets reused
- self-comparison of first decl, potential infinite loop
The third of these would result incorrect diff reports under the right
circumstances.
* src/abg-reporter-priv.cc
(maybe_report_data_members_replaced_by_anon_dm): Move
declarations of anonymous_data_member and
dms_replaced_by_same_anon_dm into inner loop. Use
anonymous_data_member for testing and reporting, allowing
iterators i and j to be replaced by just iterator i. Push
first decl onto dms_replaced_by_same_anon_dm unconditionally
and move control flow logic into loop condition.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
src/abg-reporter-priv.cc | 39 +++++++-----------
tests/data/Makefile.am | 8 ++++
.../test-PR25661-7-report-1.txt | 3 ++
.../test-PR25661-7-report-2.txt | 5 +++
.../test-PR25661-7-report-3.txt | 14 +++++++
.../test-PR25661-7-report-4.txt | 11 +++++
.../data/test-diff-filter/test-PR25661-7-v0.c | 12 ++++++
.../data/test-diff-filter/test-PR25661-7-v0.o | Bin 0 -> 2680 bytes
.../data/test-diff-filter/test-PR25661-7-v1.c | 26 ++++++++++++
.../data/test-diff-filter/test-PR25661-7-v1.o | Bin 0 -> 2976 bytes
tests/test-diff-filter.cc | 28 +++++++++++++
11 files changed, 123 insertions(+), 23 deletions(-)
create mode 100644 tests/data/test-diff-filter/test-PR25661-7-report-1.txt
create mode 100644 tests/data/test-diff-filter/test-PR25661-7-report-2.txt
create mode 100644 tests/data/test-diff-filter/test-PR25661-7-report-3.txt
create mode 100644 tests/data/test-diff-filter/test-PR25661-7-report-4.txt
create mode 100644 tests/data/test-diff-filter/test-PR25661-7-v0.c
create mode 100644 tests/data/test-diff-filter/test-PR25661-7-v0.o
create mode 100644 tests/data/test-diff-filter/test-PR25661-7-v1.c
create mode 100644 tests/data/test-diff-filter/test-PR25661-7-v1.o
@@ -1437,30 +1437,25 @@ maybe_report_data_members_replaced_by_anon_dm(const class_or_union_diff &d,
// Let's detect all the data members that are replaced by
// members of the same anonymous data member and report them
// in one go.
- changed_var_sptrs_type::const_iterator i, j;
- i = d.ordered_data_members_replaced_by_adms().begin();
- // This contains the data members replaced by the same
- // anonymous data member.
- vector<var_decl_sptr> dms_replaced_by_same_anon_dm;
- // This contains the anonymous data member that replaced the
- // data members in the variable above.
- var_decl_sptr anonymous_data_member;
-
- while (i != d.ordered_data_members_replaced_by_adms().end())
+ for (changed_var_sptrs_type::const_iterator i =
+ d.ordered_data_members_replaced_by_adms().begin();
+ i != d.ordered_data_members_replaced_by_adms().end();)
{
- anonymous_data_member = i->second;
+ // This contains the data members replaced by the same
+ // anonymous data member.
+ vector<var_decl_sptr> dms_replaced_by_same_anon_dm;
+ dms_replaced_by_same_anon_dm.push_back(i->first);
+ // This contains the anonymous data member that replaced the
+ // data members in the variable above.
+ var_decl_sptr anonymous_data_member = i->second;
// Let's look forward to see if the subsequent data
// members were replaced by members of
// anonymous_data_member.
- for (j = i;
- j != d.ordered_data_members_replaced_by_adms().end();
- ++j)
- {
- if (*i->second == *j->second)
- dms_replaced_by_same_anon_dm.push_back(j->first);
- else
- break;
- }
+ for (++i;
+ i != d.ordered_data_members_replaced_by_adms().end()
+ && *i->second == *anonymous_data_member;
+ ++i)
+ dms_replaced_by_same_anon_dm.push_back(i->first);
bool several_data_members_replaced =
dms_replaced_by_same_anon_dm.size() > 1;
@@ -1490,10 +1485,8 @@ maybe_report_data_members_replaced_by_anon_dm(const class_or_union_diff &d,
out << "replaced by anonymous data member:\n"
<< indent + " "
<< "'"
- << i->second->get_pretty_representation()
+ << anonymous_data_member->get_pretty_representation()
<< "'\n";
-
- i = j;
}
}
}
@@ -851,6 +851,14 @@ test-diff-filter/test-PR25661-6-report-1.txt \
test-diff-filter/test-PR25661-6-report-2.txt \
test-diff-filter/test-PR25661-6-report-3.txt \
test-diff-filter/test-PR25661-6-report-4.txt \
+test-diff-filter/test-PR25661-7-v0.c \
+test-diff-filter/test-PR25661-7-v1.c \
+test-diff-filter/test-PR25661-7-v0.o \
+test-diff-filter/test-PR25661-7-v1.o \
+test-diff-filter/test-PR25661-7-report-1.txt \
+test-diff-filter/test-PR25661-7-report-2.txt \
+test-diff-filter/test-PR25661-7-report-3.txt \
+test-diff-filter/test-PR25661-7-report-4.txt \
\
test-diff-suppr/test0-type-suppr-v0.cc \
test-diff-suppr/test0-type-suppr-v1.cc \
new file mode 100644
@@ -0,0 +1,3 @@
+Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
new file mode 100644
@@ -0,0 +1,5 @@
+Leaf changes summary: 0 artifact changed (1 filtered out)
+Changed leaf types summary: 0 (1 filtered out) leaf type changed
+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
+
new file mode 100644
@@ -0,0 +1,14 @@
+Functions changes summary: 0 Removed, 1 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+1 function with some indirect sub-type change:
+
+ [C] 'function void foo(S*)' at test-PR25661-7-v1.c:24:1 has some indirect sub-type changes:
+ parameter 1 of type 'S*' has sub-type changes:
+ in pointed to type 'struct S' at test-PR25661-7-v1.c:1:1:
+ type size hasn't changed
+ data members 'S::a', 'S::b' were replaced by anonymous data member:
+ 'union {int tag1[2]; struct {int a; int b;};}'
+ data members 'S::c', 'S::d' were replaced by anonymous data member:
+ 'union {int tag2[2]; struct {int c; int d;};}'
+
new file mode 100644
@@ -0,0 +1,11 @@
+Leaf changes summary: 1 artifact changed
+Changed leaf types summary: 1 leaf type changed
+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
+
+'struct S at test-PR25661-7-v0.c:1:1' changed:
+ type size hasn't changed
+ data members 'S::a', 'S::b' were replaced by anonymous data member:
+ 'union {int tag1[2]; struct {int a; int b;};}'
+ data members 'S::c', 'S::d' were replaced by anonymous data member:
+ 'union {int tag2[2]; struct {int c; int d;};}'
new file mode 100644
@@ -0,0 +1,12 @@
+struct S
+{
+ int a;
+ int b;
+ int c;
+ int d;
+};
+
+void
+foo(struct S* s __attribute__((unused)))
+{
+}
new file mode 100644
GIT binary patch
literal 2680
zcmbtV&1)1(5U<|Zk4Yw3H(5nP6!yWBs5oQdM@*FH`l9g@35qu*&hAdKi`iM(nP^nR
zgCclS5LA5d4)5a8qj&!dFYhAAyLk(E(5jxUq|?q`v|zide^tM#>h9^<M`te0WrP40
z0ViN%#VEjs+=gD2Y8A>b4A;&*`+V-%!?ou(KE6dbD^0eHFd%hpa5O!06-q+b0Ya7!
z!YYskRt-c}s#&WJA}3YbYJkW~bpW#2Tq}$rx7g``=wGjtLyNy(W0-t|wk@P(UNoU#
zyA)dtiQUDrV--h>Lt?m4+K$$;ux!UU<{WpX9Z|;ogJ9Xo@_cy;gJ%B($2s9xHbBl3
zCr8*iHbOC~J6`=YLcDxV<7IXL6t*DmJX&BsRYCayV`#z^YmijuaI|0_jzWAZ4&o?p
z2X`tU4x-qdzjE-%<m7~V)V(wA)g>n<pfAZm<bLruV<9ii`#{{j4QDT3tISRutxS7|
zyzz?Lj#t89W+@1RZmW(<zgut4tWHh3lZPwrQpH{LqkCb!+3kd#mB?KQ@3z8*8~e58
zAObi$JA16M=X6kO`C$^!o!Xlyvb3#o#NzK?0kK*VqXPx|Ic{br;`AetM{%lHdYYLX
z%+H}i0;i62<5~FnJ<W)lPJ{6{*AHit-9k&6Sn4x2Z5HHBNUkj?^uYUi;OUod!C(*n
zVh{YMCkETlb67iv^-O7B13e-oyt95>vrZ<?KsfDYgg}&`WCF(3QsyU}alU`%(GBUh
zu~!kvU%&TQ;if;=M*dn$f>ICv=^l7R;Z%!W4|vhNHh$bV$K7OZvK9xcF?bC>_Q9(~
z5qRBT+4mYjZDnbp6)twrCUw77>jrl=nf<OGF5!F8Da)-eU<#%Huik0518i<y*ogxV
zU#T0#e*Kmr1<i%UuHTk@QWgIj(<pN3Z23GnR8=YK-;G+EFY6D)2{T{NMdfc!CVvba
z=)QlIKY?5`CgtS(|Ik7c)0f&Z{aIBtA5!h)9}M0poW3i9_}j$ZE@cKsGbS1SmsWtg
zYUauGokxR-k*}}7aVDzS^rd~K)&E}A&wQ$%x|3Od12HouLhwR?7igi0>AQfowEX|6
z{CrN7pZbQG|2AS2lj}rP$|seG>eGeu^ZWpfY5bGwG4ImJ%9kMKKhbo;S*~wt28?d-
z7<q#V#WLM*DqxBo)y(^6_$4Z7=eL$}e$@S$&*z$!n_|CK`RCO}sZZ0tipi%tG2b3S
d@I`^#uL&dJb4yl9|4-wWR8#(!9o1~|e*g&qr{4eo
literal 0
HcmV?d00001
new file mode 100644
@@ -0,0 +1,26 @@
+struct S
+{
+ union
+ {
+ int tag1[2];
+ struct
+ {
+ int a;
+ int b;
+ };
+ };
+ union
+ {
+ int tag2[2];
+ struct
+ {
+ int c;
+ int d;
+ };
+ };
+};
+
+void
+foo(struct S* s __attribute__((unused)))
+{
+}
new file mode 100644
GIT binary patch
literal 2976
zcmbtVL2nyX5T55}C+n=ec5F}+1*9zk+5)>x3Zza^T0<ahC{$5P1s90c_Qtk|*Ohl&
zQV=QvRH?TjAt5AAAaUj|pkBCggBu)>IKmBy1D)BOXK$XD3p{D}&3rT8%$wQw-rj!Y
zwe6e`poW0+Fx3<a@XOp(ZpLC0>QIC0m+t;{>F$Sr-Fy4KfxLWdA_<@r%c+-{l5>Wb
zE4+??^gtLUNg!Jwim{l7e7?{RA}AO_$|2g~^rBECdEOy!gE+VGDbh>8f-#RXVBkvO
zTNGp)#3BdofT(d`j98N2qdkdeB@yos)w~8(A&j3<EWJTMiQ*Vg-X9J@EKS;?>JVQQ
z5VJzGRY=Nx)`N;cu`#oF!m69nTCo;IZN7R8t#x6V=gb$(4O7%{d1a7BqVCii7&8Aj
znC5v?8UTf3M_lL7)l#llei?Q!fFZ<Aogx*FqkbAAEN}!1OQ{QKq|=p%L}|*dxUj4;
zpID~VHJ844X_+Efh18mIO8}J$u8EStGx+4FGVDiss9vNolDF}Z@D#p<_}ePuJKkj2
zB>|BiM$YclGtaNDuQ_L(n`>^HD1bdICOJ@%OX8E9MBc$L=-T5T>~{mdW7DR{;zwTh
z44jSzH{jxx>-N^#S$o5M)?KxoK{O8h&8{E#qkbDzZ`AH>9yZpU_2+D-Ydia1cq?f4
zM#Eq@4xMrEUO(tKk=Hu#L%^VG02jBm&e<nl_FH`~NTNE8Q;7iZP`q-i>iw%A4y$6N
zG;iF)Djz|d9vX3vUbL!ra$DuOZFETB%r{c;5dHa2mJz#=1!JElT{~izXvrFqkEw2x
z$(hh$j1r%$47@ml5g?O_XKWJ6Gg#t}J2qp;*((GdM!#CB50m|uwJGS_@`z76$_w}?
znM~@EBAlvOB@oAAGR6Lc*I1|gG?f3-i3-a+(-%sAjZ=)1fBL(>lfo(f2dsar;jF@U
z8vZ2fzw)f-RW(xH0{hTZWm)&%_!rz}oN7wf1#Wn2fX4*qXq3!7B}M*W1a8NRJaAiK
z2=2%~@Z65y8h7{l!Tu0!vFx>4Bmd?Rvp4dBE}pzJ<)9z<ih?P?Z4U<nAMXJ-7)HK}
z=iLb-ul+8Q{NCRF$Q$^)@c%ijAfI}r`l-S-S(xlU*2y<Lj-x`DeiARIol>&SKY=#d
zH~*E7_gCt)OcSa6B`u+*#`iw`)SFXkEd8GR>ERE?X<ref_Bj4L;<1TuJti66Wbnt>
zYx>fCchRI{<b9Gi<LZ<eOS&)BnYI3(_)sW5t)Ff=ef=H8^q2_2Tb%z@Qc^?r-9uYe
zeuFoZ>J#OsdqB^B12KxJ)`?;~zUKTNv92bSU(Ix{Wbq&44fUuZ@~n7j{*?Ke8(h#g
zq{dhNUoxMUt6*p{zcE1)LDjF|&r!(gpPkV^`im&O>MPZu=cd?)oPU=KrTd=#PCB2y
gNmi*p(4ugtdq2@;_X@?$;x~Bx-zXx_s_zPZ081>tR{#J2
literal 0
HcmV?d00001
@@ -738,6 +738,34 @@ InOutSpec in_out_specs[] =
"data/test-diff-filter/test-PR25661-6-report-4.txt",
"output/test-diff-filter/test-PR25661-6-report-4.txt",
},
+ {
+ "data/test-diff-filter/test-PR25661-7-v0.o",
+ "data/test-diff-filter/test-PR25661-7-v1.o",
+ "--no-default-suppression",
+ "data/test-diff-filter/test-PR25661-7-report-1.txt",
+ "output/test-diff-filter/test-PR25661-7-report-1.txt",
+ },
+ {
+ "data/test-diff-filter/test-PR25661-7-v0.o",
+ "data/test-diff-filter/test-PR25661-7-v1.o",
+ "--no-default-suppression --leaf-changes-only",
+ "data/test-diff-filter/test-PR25661-7-report-2.txt",
+ "output/test-diff-filter/test-PR25661-7-report-2.txt",
+ },
+ {
+ "data/test-diff-filter/test-PR25661-7-v0.o",
+ "data/test-diff-filter/test-PR25661-7-v1.o",
+ "--no-default-suppression --harmless",
+ "data/test-diff-filter/test-PR25661-7-report-3.txt",
+ "output/test-diff-filter/test-PR25661-7-report-3.txt",
+ },
+ {
+ "data/test-diff-filter/test-PR25661-7-v0.o",
+ "data/test-diff-filter/test-PR25661-7-v1.o",
+ "--no-default-suppression --harmless --leaf-changes-only",
+ "data/test-diff-filter/test-PR25661-7-report-4.txt",
+ "output/test-diff-filter/test-PR25661-7-report-4.txt",
+ },
// This should be the last entry
{NULL, NULL, NULL, NULL, NULL}
};