strip,unstrip: Use and set shdrstrndx consistently.

In various places in strip we used e_shstrndx instead of shdrstrndx and we
didn't setup the shdrstrndx for the debug file. In unstrip we forgot to copy
the shdrstrndx in case the -o output option was used.

Added a new testcase that adds many sections to a testfile and runs strip, elflint,
unstrip and elfcmp.

Signed-off-by: Mark Wielaard <[email protected]>

src/ChangeLog

@@ -1,3 +1,10 @@
+2018-09-13  Mark Wielaard  <[email protected]>
+
+	* strip.c (handle_elf): Check against shstrndx, not e_shstrndx.
+	Explicitly set shdrstrndx for debug file.
+	* unstrip.c (copy_elf): Explicitly copy shstrndx.
+	(find_alloc_sections_prelink): Document shnum usage.
+
 2018-09-13  Mark Wielaard  <[email protected]>
 
 	* elflint.c (check_elf_header): Use shnum instead of e_shnum for all

src/strip.c

@@ -820,7 +820,7 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,
 	: (ebl_section_strip_p (ebl, &shdr_info[cnt].shdr,
 				shdr_info[cnt].name, remove_comment,
 				remove_debug)
-	   || cnt == ehdr->e_shstrndx
+	   || cnt == shstrndx
 	   || section_name_matches (remove_secs, shdr_info[cnt].name)))
       {
 	/* The user might want to explicitly keep this one.  */
@@ -1081,7 +1081,7 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,
 				  && shdr_info[cnt].debug_data == NULL
 				  && shdr_info[cnt].shdr.sh_type != SHT_NOTE
 				  && shdr_info[cnt].shdr.sh_type != SHT_GROUP
-				  && cnt != ehdr->e_shstrndx);
+				  && cnt != shstrndx);
 
 	  /* Set the section header in the new file.  */
 	  GElf_Shdr debugshdr = shdr_info[cnt].shdr;
@@ -1134,7 +1134,42 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,
       debugehdr->e_version = ehdr->e_version;
       debugehdr->e_entry = ehdr->e_entry;
       debugehdr->e_flags = ehdr->e_flags;
-      debugehdr->e_shstrndx = ehdr->e_shstrndx;
+
+      size_t shdrstrndx;
+      if (elf_getshdrstrndx (elf, &shdrstrndx) < 0)
+	{
+	  error (0, 0, gettext ("%s: error while getting shdrstrndx: %s"),
+		 fname, elf_errmsg (-1));
+	  result = 1;
+	  goto fail_close;
+	}
+
+      if (shstrndx < SHN_LORESERVE)
+	debugehdr->e_shstrndx = shdrstrndx;
+      else
+	{
+	  debugehdr->e_shstrndx = SHN_XINDEX;
+	  Elf_Scn *scn0 = elf_getscn (debugelf, 0);
+	  GElf_Shdr shdr0_mem;
+	  GElf_Shdr *shdr0 = gelf_getshdr (scn0, &shdr0_mem);
+	  if (shdr0 == NULL)
+	    {
+	      error (0, 0, gettext ("%s: error getting zero section: %s"),
+		     debug_fname, elf_errmsg (-1));
+	      result = 1;
+	      goto fail_close;
+	    }
+
+	  shdr0->sh_link = shdrstrndx;
+	  if (gelf_update_shdr (scn0, shdr0) == 0)
+	    {
+	      error (0, 0, gettext ("%s: error while updating zero section: %s"),
+		     debug_fname, elf_errmsg (-1));
+	      result = 1;
+	      goto fail_close;
+	    }
+
+	}
 
       if (unlikely (gelf_update_ehdr (debugelf, debugehdr) == 0))
 	{
@@ -1187,7 +1222,7 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,
      the .shstrtab section which we would add again.  */
   bool removing_sections = !(cnt == idx
 			     || (cnt == idx + 1
-				 && shdr_info[ehdr->e_shstrndx].idx == 0));
+				 && shdr_info[shstrndx].idx == 0));
   if (output_fname == NULL && !removing_sections)
       goto fail_close;
 

src/unstrip.c

@@ -239,8 +239,27 @@ copy_elf (Elf *outelf, Elf *inelf)
   ELF_CHECK (gelf_newehdr (outelf, gelf_getclass (inelf)),
 	     _("cannot create ELF header: %s"));
 
+  size_t shstrndx;
+  ELF_CHECK (elf_getshdrstrndx (inelf, &shstrndx) == 0,
+	     _("cannot get shdrstrndx:%s"));
+
   GElf_Ehdr ehdr_mem;
   GElf_Ehdr *ehdr = gelf_getehdr (inelf, &ehdr_mem);
+  if (shstrndx < SHN_LORESERVE)
+    ehdr->e_shstrndx = shstrndx;
+  else
+    {
+      ehdr->e_shstrndx = SHN_XINDEX;
+      Elf_Scn *scn0 = elf_getscn (outelf, 0);
+      GElf_Shdr shdr0_mem;
+      GElf_Shdr *shdr0 = gelf_getshdr (scn0, &shdr0_mem);
+      ELF_CHECK (shdr0 != NULL,
+		 _("cannot get new zero section: %s"));
+      shdr0->sh_link = shstrndx;
+      ELF_CHECK (gelf_update_shdr (scn0, shdr0),
+		 _("cannot update new zero section: %s"));
+    }
+
   ELF_CHECK (gelf_update_ehdr (outelf, ehdr),
 	     _("cannot copy ELF header: %s"));
 
@@ -1025,7 +1044,7 @@ find_alloc_sections_prelink (Elf *debug, Elf_Data *debug_shstrtab,
 		 _("cannot read '.gnu.prelink_undo' section: %s"));
 
       uint_fast16_t phnum;
-      uint_fast16_t shnum;
+      uint_fast16_t shnum;  /* prelink doesn't handle > SHN_LORESERVE.  */
       if (ehdr.e32.e_ident[EI_CLASS] == ELFCLASS32)
 	{
 	  phnum = ehdr.e32.e_phnum;

tests/ChangeLog

@@ -1,3 +1,9 @@
+2018-09-13  Mark Wielaard  <[email protected]>
+
+	* run-strip-test-many.sh: New test.
+	* Makefile.am (TESTS): Add run-strip-test-many.sh.
+	(EXTRA_DIST): Likewise.
+
 2018-09-13  Mark Wielaard  <[email protected]>
 
 	* run-typeiter-many.sh: New test.

tests/Makefile.am

@@ -156,7 +156,7 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh newfile test-nlist \
 	run-reloc-bpf.sh \
 	run-next-cfi.sh run-next-cfi-self.sh \
 	run-copyadd-sections.sh run-copymany-sections.sh \
-	run-typeiter-many.sh
+	run-typeiter-many.sh run-strip-test-many.sh
 
 if !BIARCH
 export ELFUTILS_DISABLE_BIARCH = 1
@@ -408,7 +408,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
 	     testfile-riscv64.bz2 testfile-riscv64-s.bz2 \
 	     testfile-riscv64-core.bz2 \
 	     run-copyadd-sections.sh run-copymany-sections.sh \
-	     run-typeiter-many.sh
+	     run-typeiter-many.sh run-strip-test-many.sh
 
 if USE_VALGRIND
 valgrind_cmd='valgrind -q --leak-check=full --error-exitcode=1'

tests/run-strip-test-many.sh

@@ -0,0 +1,66 @@
+#! /bin/sh
+# Copyright (C) 2018 Red Hat, Inc.
+# This file is part of elfutils.
+#
+# This file is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# elfutils is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. $srcdir/test-subr.sh
+
+# Use the original file from run-strip-test.sh but with many sections
+testfiles testfile
+tempfiles testfile.strip testfile.debug testfile.unstrip
+
+echo "Adding sections to testfile"
+testrun ${abs_builddir}/addsections 65535 testfile
+
+echo "Testing strip -o"
+testrun ${abs_top_builddir}/src/strip -o testfile.strip -f testfile.debug testfile
+
+# Do the parts check out?
+echo "elflint testfile.strip"
+testrun ${abs_top_builddir}/src/elflint --gnu -q testfile.strip
+echo "elflint testfile.debug"
+testrun ${abs_top_builddir}/src/elflint --gnu -q -d testfile.debug
+
+# Now test unstrip recombining those files.
+echo "unstrip"
+testrun ${abs_top_builddir}/src/unstrip -o testfile.unstrip testfile.strip testfile.debug
+echo "elfcmp"
+testrun ${abs_top_builddir}/src/elfcmp testfile testfile.unstrip
+
+# test strip -g
+echo "Testing strip -g"
+testrun ${abs_top_builddir}/src/strip -g -o testfile.strip -f testfile.debug testfile
+
+# Do the parts check out?
+echo "elflint testfile.strip"
+testrun ${abs_top_builddir}/src/elflint --gnu -q testfile.strip
+echo "elflint testfile.debug"
+testrun ${abs_top_builddir}/src/elflint --gnu -q -d testfile.debug
+
+# Now strip "in-place" and make sure it is smaller.
+echo "TEsting strip in-place"
+SIZE_original=$(stat -c%s testfile)
+echo "original size $SIZE_original"
+
+testrun ${abs_top_builddir}/src/strip testfile
+SIZE_stripped=$(stat -c%s testfile)
+echo "stripped size $SIZE_stripped"
+test $SIZE_stripped -lt $SIZE_original ||
+  { echo "*** failure in-place strip file not smaller $original"; exit 1; }
+
+echo "elflint in-place"
+testrun ${abs_top_builddir}/src/elflint --gnu -q testfile
+
+exit 0