[PATCH] unstrip: Also check sh_size in compare_unalloc_sections.

[PATCH] unstrip: Also check sh_size in compare_unalloc_sections.

[Mark Wielaard]

compare_unalloc_sections only checked sh_flags and the section names.
This would cause stripped/debug section mismatches when there were
multiple sections with the same name and flags. Fix this by also checking
the size of the section matches.

Add a testcase that has two ".group" sections created on i386 with the
gcc annobin plugin.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 src/ChangeLog                          |   4 +++
 src/unstrip.c                          |   6 ++++
 tests/ChangeLog                        |   6 ++++
 tests/Makefile.am                      |   1 +
 tests/run-annobingroup.sh              |  55 +++++++++++++++++++++++++++++++++
 tests/testfile-annobingroup-i386.o.bz2 | Bin 0 -> 1387 bytes
 6 files changed, 72 insertions(+)
 create mode 100644 tests/testfile-annobingroup-i386.o.bz2

diff --git a/src/ChangeLog b/src/ChangeLog
index 37e7b62..eee144d 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2018-07-24  Mark Wielaard  <mark@klomp.org>
+
+	* unstrip.c (compare_unalloc_sections): Also compare sh_size.
+
 2018-07-21  Mark Wielaard  <mark@klomp.org>
 
 	* unstrip.c (adjust_all_relocs): Skip SHT_GROUP sections.
diff --git a/src/unstrip.c b/src/unstrip.c
index cb1f7dc..ec46c95 100644
--- a/src/unstrip.c
+++ b/src/unstrip.c
@@ -709,6 +709,12 @@ compare_unalloc_sections (const GElf_Shdr *shdr1, const GElf_Shdr *shdr2,
   if (shdr1->sh_flags > shdr2->sh_flags)
     return 1;
 
+  /* Sizes should be the same.  */
+  if (shdr1->sh_size < shdr2->sh_size)
+    return -1;
+  if (shdr1->sh_size > shdr2->sh_size)
+    return 1;
+
   /* Sort by name as last resort.  */
   return strcmp (name1, name2);
 }
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 45844b1..2a20ffa 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,9 @@
+2018-07-24  Mark Wielaard  <mark@klomp.org>
+
+	* run-annobingroup.sh: Add testfile-annobingroup-i386.o tests.
+	* testfile-annobingroup-i386.o.bz2: New test file.
+	* Makefile.am (EXTRA_DIST): Add testfile-annobingroup-i386.o.bz2.
+
 2018-07-21  Mark Wielaard  <mark@klomp.org>
 
 	* run-annobingroup.sh: New test.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b7a0e17..2946083 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -195,6 +195,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
 	     run-strip-test12.sh \
 	     run-strip-nothing.sh run-strip-remove-keep.sh run-strip-g.sh \
 	     run-annobingroup.sh testfile-annobingroup.o.bz2 \
+	     testfile-annobingroup-i386.o.bz2 \
 	     run-strip-strmerge.sh run-strip-nobitsalign.sh \
 	     testfile-nobitsalign.bz2 testfile-nobitsalign.strip.bz2 \
 	     run-strip-reloc.sh hello_i386.ko.bz2 hello_x86_64.ko.bz2 \
diff --git a/tests/run-annobingroup.sh b/tests/run-annobingroup.sh
index 5f08b35..700df32 100755
--- a/tests/run-annobingroup.sh
+++ b/tests/run-annobingroup.sh
@@ -65,4 +65,59 @@ EOF
 
 testrun ${abs_top_builddir}/src/elfcmp testfile-annobingroup.o remerged.elf
 
+# echo "void * __attribute__((cold)) foo (void) { return foo; }"
+#      > testfile-annobingroup-i386.c
+# gcc -fpic -g -O2 -fplugin=annobin -c testfile-annobingroup-i386.c
+testfiles testfile-annobingroup-i386.o
+
+testrun_compare ${abs_top_builddir}/src/readelf -g testfile-annobingroup-i386.o << EOF
+
+Section group [ 1] '.group' with signature '.text.unlikely.group' contains 3 entries:
+  [ 8] .gnu.build.attributes..text.unlikely
+  [ 9] .rel.gnu.build.attributes..text.unlikely
+  [10] .text.unlikely
+
+COMDAT section group [ 2] '.group' with signature '__x86.get_pc_thunk.ax' contains 1 entry:
+  [13] .text.__x86.get_pc_thunk.ax
+EOF
+
+testrun ${abs_top_builddir}/src/strip -o stripped.elf -f debugfile.elf testfile-annobingroup-i386.o
+
+testrun_compare ${abs_top_builddir}/src/readelf -g stripped.elf << EOF
+
+Section group [ 1] '.group' with signature '.text.unlikely.group' contains 3 entries:
+  [ 8] .gnu.build.attributes..text.unlikely
+  [ 9] .rel.gnu.build.attributes..text.unlikely
+  [10] .text.unlikely
+
+COMDAT section group [ 2] '.group' with signature '__x86.get_pc_thunk.ax' contains 1 entry:
+  [13] .text.__x86.get_pc_thunk.ax
+EOF
+
+testrun_compare ${abs_top_builddir}/src/readelf -g debugfile.elf << EOF
+
+Section group [ 1] '.group' with signature '.text.unlikely.group' contains 3 entries:
+  [ 8] .gnu.build.attributes..text.unlikely
+  [ 9] .rel.gnu.build.attributes..text.unlikely
+  [10] .text.unlikely
+
+COMDAT section group [ 2] '.group' with signature '__x86.get_pc_thunk.ax' contains 1 entry:
+  [13] .text.__x86.get_pc_thunk.ax
+EOF
+
+testrun ${abs_top_builddir}/src/unstrip -o remerged.elf stripped.elf debugfile.elf
+
+testrun_compare ${abs_top_builddir}/src/readelf -g remerged.elf << EOF
+
+Section group [ 1] '.group' with signature '.text.unlikely.group' contains 3 entries:
+  [ 8] .gnu.build.attributes..text.unlikely
+  [ 9] .rel.gnu.build.attributes..text.unlikely
+  [10] .text.unlikely
+
+COMDAT section group [ 2] '.group' with signature '__x86.get_pc_thunk.ax' contains 1 entry:
+  [13] .text.__x86.get_pc_thunk.ax
+EOF
+
+testrun ${abs_top_builddir}/src/elfcmp testfile-annobingroup-i386.o remerged.elf
+
 exit 0
diff --git a/tests/testfile-annobingroup-i386.o.bz2 b/tests/testfile-annobingroup-i386.o.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..798eefda9c6018d7a2a7e46bfd9f850c67b1dcb6
GIT binary patch
literal 1387
zcmV-x1(f<iT4*^jL0KkKS?+Yv0RRP&|NsC0{{QCx|Nj5=-+=$`-*oUu5XBU!00e9V
zjKE?*AWYB%(iR4Ul#Bs}NQtIY@usJ#sPQs-A(DEGjUJj!G-&jmpwXe~XwjfFX`lm5
zG6$&1>J1*F%6dU4glGwgG{ngoGzNeG!Z82<13)wY27mwn00AOI(I8XBJx@(Eo|-)(
zdY**Lh&1%jOqu`;X^;~}5w$j^nlMjMr<C;!o6?U_1``t|0$@x{8e%X2Oae5>(SVv{
zz$Qij00J;g7zjax#L0k|6B9<5i~th=jWRS~CYdk^k$?aIj1xuzB5YZ0SR6-J2e|lS
ze7Q}**D~xW626;O=qy~jtAhs;zn)_zR}G0+IbCpLRaHeGRH0i%6K7*eX^uIutRqjS
z#$1=_vG6$fdbKxkZ0yTiXVVQ9!U0dA?cZ)QGvS$z!;(<sPN$D8><mXRvN7Vx&@Isc
zk3xX@eSX`2+LdGKRZ9}CZbFP3uK?q)LSTt}iqc@h(Y|q>O0i>)3dUwj3jrVIHo+u}
zw;sNJ9STj-q}Fredl(T_>4>lpEm^1*6TewR6CJm@F!8vIlF=|j*Zb|-xuK@n1YS;1
zdOC>aFs*{d->VdGxfNhFh4rQu6j-!oW2Ysc3$YO_3j-U7Lm#%mLj)NN12u^R?ZN=5
zgNt%EY*T3gl*EW&E`vdng$T87DT`v0bxh{S{Dx`O6L_+U*adiVzvNUlV>MjjqEMD2
zoq`T*iZaF%ZQoZ2CfgGCMz8W~kd+!)Z7SH4oCp*&B-!gRC}{$01U3!eHT1BPY#Tyt
z4TEisgBooM8Rj+dGfZN}v*TOj!`kC%dplbr=7F3+6%`d=%tAb92%3P9+*Mb_+hff4
z*}l8=B(1P5w`S_1#}MzBr}xv5U>TGk8$)Yy{eB!NTh`JxF7HO&ZW!Aec|I&Fs!*sQ
zsLql`lSW$x>uVO1m)$E?rb=2wy10Y}Z93k0Nb_rc7c4(_fSY_amODlg*tvr`irNAM
zT;=4+BpqXRVfr&0l%h-$meeLm8NbGs9SA&#GtjhPWn`iecQHgs&QaKVPNpXYy~t#U
zp$P*}PEijaBH9ChlJZnY0cb=Jpgo3oH8mtxWGea7ku9A|$&VruBBK%{7te&`H&%uG
zDcKN<&3fcbjb6J_S^umehE#SupJanM-rd5yEkH*t0|-OA$^z1L>nCT7RI3^SMR4tq
z1xV|^d23e)*_w_vDMA3MBs@Fv%>>q3Wva92j@m20v=|0~7Y(8zTn6*xL1gq<MuLnq
zX%|VO1;0&Rta`+4mcO*HWT3Sq#I{7)ONpL43v>X|oQP)Kh(iWm!5!h}!l@xq29)nI
z!56(NK!I|jH)e0|V?%`J=l$=Xj(8d*4OGFON1h!TFt<iC_TGH}qVua5aPG$!kes^$
z7i0z%5?qDlUxcPIr-}4~BN%A~JXlAS7iCz1s)VO?J5_?}LY+73&*_Tr%kKs#hGVM;
zro>QEyDb;EMdXN7Lm6aJk*JNX)UGvzMM606k8l>!M$aUbnnsAEOhZdZ#39h!l(s7D
z#KyYvf!2l~)G`UIvT{U}l7gkOgywAvmJ}Tm5Rjg*p-NB#1|nY$N6!jjQw^*9hfrE^
zFp-O0jEqzog9IlKvOyAOBCx3lg;EX+*9w6s^-MJ{2;NN6!&z8Sz+S*ndX%C)!;;#K
z9{bFoYzRoq^3{oXAi#LXTUmstE0&UCR3Jfxp_K|qNhIk|mlS;@saX-l5)+I;@g&)@
ti+v?ww`My|UIXC9R=~v+RrNVJIB^jS9Z|EZ*EN6fcO+AV2@dB?5D>@JXc7Pb

literal 0
HcmV?d00001

-- 
1.8.3.1

Re: [PATCH] unstrip: Also check sh_size in compare_unalloc_sections.

[Mark Wielaard]

On Wed, 2018-07-25 at 00:15 +0200, Mark Wielaard wrote:
> compare_unalloc_sections only checked sh_flags and the section names.
> This would cause stripped/debug section mismatches when there were
> multiple sections with the same name and flags. Fix this by also
> checking the size of the section matches.

Pushed to master.