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 <mark@klomp.org>

diff --git a/src/ChangeLog b/src/ChangeLog
index a093a73..524c81a 100644 (file)
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,12 @@
 2018-09-13  Mark Wielaard  <mark@klomp.org>
 
+       * 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  <mark@klomp.org>
+
        * elflint.c (check_elf_header): Use shnum instead of e_shnum for all
        checks.
        (check_symtab): Use shstrndx instead of e_shstrndx to get section

diff --git a/src/strip.c b/src/strip.c
index dc71236..4a3db1b 100644 (file)
--- a/src/strip.c
+++ b/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;
 

diff --git a/src/unstrip.c b/src/unstrip.c
index ec46c95..e6f0947 100644 (file)
--- a/src/unstrip.c
+++ b/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;

diff --git a/tests/ChangeLog b/tests/ChangeLog
index 33fee35..4e8b814 100644 (file)
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,5 +1,11 @@
 2018-09-13  Mark Wielaard  <mark@klomp.org>
 
+       * run-strip-test-many.sh: New test.
+       * Makefile.am (TESTS): Add run-strip-test-many.sh.
+       (EXTRA_DIST): Likewise.
+
+2018-09-13  Mark Wielaard  <mark@klomp.org>
+
        * run-typeiter-many.sh: New test.
        * Makefile.am (TESTS): Add run-typeiter-many.sh.
        (EXTRA_DIST): Likewise.

diff --git a/tests/Makefile.am b/tests/Makefile.am
index d8ebd03..74c477e 100644 (file)
--- a/tests/Makefile.am
+++ b/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'

diff --git a/tests/run-strip-test-many.sh b/tests/run-strip-test-many.sh
new file mode 100755 (executable)
index 0000000..9a9657c
--- /dev/null
+++ b/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