commit b065d4a401e2120c11c92357b213a85e6d70f300

Author: Mark Wielaard <mark@klomp.org>

Date:   Wed Jun 7 20:32:38 2017 +0200

    strip: Don't generate empty output file when nothing to do.

    If there was nothing to do strip would skip generating a separate

    debug file if one was requested, but it would also not finish the

    creation of a new output file (with the non-stripped sections).

    Also if there was an error any partially created output would be kept.

    Make sure that when the -o output file option is given we always generate

    a complete output file (except on error). Also make sure that when the -f

    debug file option is given it is only generated when it is not empty.

    Add testcase run-strip-nothing.sh that tests the various combinations.

    https://sourceware.org/bugzilla/show_bug.cgi?id=21522

    Signed-off-by: Mark Wielaard <mark@klomp.org>

diff --git a/src/ChangeLog b/src/ChangeLog

index 6ac0ef2..e19122e 100644

--- a/src/ChangeLog

+++ b/src/ChangeLog

@@ -1,3 +1,9 @@

+2017-06-07  Mark Wielaard  <mark@klomp.org>

+

+	* strip.c (handle_elf): Introduce new handle_elf boolean. Use it to

+	determine whether to create an output and/or debug file. Remove new

+	output file on error.

+

 2017-06-06  Mark Wielaard  <mark@klomp.org>

 	* strip.c (handle_elf): Assume e_shstrndx section can be removed.

diff --git a/src/strip.c b/src/strip.c

index 11b2a37..2bf95f9 100644

--- a/src/strip.c

+++ b/src/strip.c

@@ -1063,15 +1063,17 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,

 	shdr_info[cnt].se = dwelf_strtab_add (shst, shdr_info[cnt].name);

       }

-  /* Test whether we are doing anything at all.  */

-  if (cnt == idx

-      || (cnt == idx + 1 && shdr_info[ehdr->e_shstrndx].idx == 0))

-    /* Nope, all removable sections are already gone.  Or the only section

-       we would remove is the .shstrtab section which we will add again.  */

-    goto fail_close;

-

-  /* Create the reference to the file with the debug info.  */

-  if (debug_fname != NULL && !remove_shdrs)

+  /* Test whether we are doing anything at all.  Either all removable

+     sections are already gone.  Or the only section we would remove is

+     the .shstrtab section which we would add again.  */

+  bool removing_sections = !(cnt == idx

+			     || (cnt == idx + 1

+				 && shdr_info[ehdr->e_shstrndx].idx == 0));

+  if (output_fname == NULL && !removing_sections)

+      goto fail_close;

+

+  /* Create the reference to the file with the debug info (if any).  */

+  if (debug_fname != NULL && !remove_shdrs && removing_sections)

     {

       /* Add the section header string table section name.  */

       shdr_info[cnt].se = dwelf_strtab_add_len (shst, ".gnu_debuglink", 15);

@@ -1759,7 +1761,8 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,

   /* Remove any relocations between debug sections in ET_REL

      for the debug file when requested.  These relocations are always

      zero based between the unallocated sections.  */

-  if (debug_fname != NULL && reloc_debug && ehdr->e_type == ET_REL)

+  if (debug_fname != NULL && removing_sections

+      && reloc_debug && ehdr->e_type == ET_REL)

     {

       scn = NULL;

       cnt = 0;

@@ -1997,7 +2000,7 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,

   /* Now that we have done all adjustments to the data,

      we can actually write out the debug file.  */

-  if (debug_fname != NULL)

+  if (debug_fname != NULL && removing_sections)

     {

       /* Finally write the file.  */

       if (unlikely (elf_update (debugelf, ELF_C_WRITE) == -1))

@@ -2230,7 +2233,11 @@ cannot set access and modification date of '%s'"),

   /* Close the file descriptor if we created a new file.  */

   if (output_fname != NULL)

-    close (fd);

+    {

+      close (fd);

+      if (result != 0)

+       unlink (output_fname);

+    }

   return result;

 }

diff --git a/tests/ChangeLog b/tests/ChangeLog

index 5800946..5550eac 100644

--- a/tests/ChangeLog

+++ b/tests/ChangeLog

@@ -1,3 +1,9 @@

+2017-06-07  Mark Wielaard  <mark@klomp.org>

+

+	* run-strip-nothing.sh: New test.

+	* Makefile.am (TESTS): Add run-strip-nothing.sh.

+	(EXTRA_DIST): Likewise.

+

 2017-06-06  Mark Wielaard  <mark@klomp.org>

 	* run-strip-test.sh: Test strip -g doesn't introduce extra .shstrtab.

diff --git a/tests/Makefile.am b/tests/Makefile.am

index 3a12fe3..28e997c 100644

--- a/tests/Makefile.am

+++ b/tests/Makefile.am

@@ -81,6 +81,7 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile test-nlist \

 	run-strip-test3.sh run-strip-test4.sh run-strip-test5.sh \

 	run-strip-test6.sh run-strip-test7.sh run-strip-test8.sh \

 	run-strip-test9.sh run-strip-test10.sh run-strip-test11.sh \

+	run-strip-nothing.sh \

 	run-strip-groups.sh run-strip-reloc.sh run-strip-strmerge.sh \

 	run-strip-nobitsalign.sh \

 	run-unstrip-test.sh run-unstrip-test2.sh run-unstrip-test3.sh \

@@ -174,6 +175,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh \

 	     run-strip-test4.sh run-strip-test5.sh run-strip-test6.sh \

 	     run-strip-test7.sh run-strip-test8.sh run-strip-groups.sh \

 	     run-strip-test9.sh run-strip-test10.sh run-strip-test11.sh \

+	     run-strip-nothing.sh \

 	     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-strip-nothing.sh b/tests/run-strip-nothing.sh

new file mode 100755

index 0000000..e80bd90

--- /dev/null

+++ b/tests/run-strip-nothing.sh

@@ -0,0 +1,62 @@

+#! /bin/sh

+# Copyright (C) 2017 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

+

+# If there is nothing to strip then -o output should be identical to input.

+# And there should not be an (empty) -f debug file.

+

+tempfiles a.out strip.out debug.out

+

+# Create no-debug a.out.

+echo "int main() { return 1; }" | gcc -xc -

+

+# strip to file

+testrun ${abs_top_builddir}/src/strip -g -o strip.out ||

+  { echo "*** failed to strip -g -o strip.out a.out"; exit -1; }

+

+testrun ${abs_top_builddir}/src/elfcmp a.out strip.out ||

+  { echo "*** failed strip.out different from a.out"; exit -1; }

+

+# strip original

+testrun ${abs_top_builddir}/src/strip -g ||

+  { echo "*** failed to strip -g a.out"; exit -1; }

+

+testrun ${abs_top_builddir}/src/elfcmp strip.out a.out ||

+  { echo "*** failed a.out different from strip.out"; exit -1; }

+

+# strip to file with debug file

+testrun ${abs_top_builddir}/src/strip -g -o strip.out -f debug.out ||

+  { echo "*** failed to strip -g -o strip.out -f debug.out a.out"; exit -1; }

+

+testrun ${abs_top_builddir}/src/elfcmp a.out strip.out ||

+  { echo "*** failed strip.out different from a.out (with debug)"; exit -1; }

+

+test ! -f debug.out ||

+  { echo "*** failed strip.out and debug.out exist"; exit -1; }

+

+# strip original with debug file

+testrun ${abs_top_builddir}/src/strip -g -f debug.out ||

+  { echo "*** failed to strip -g -f debug.out a.out"; exit -1; }

+

+testrun ${abs_top_builddir}/src/elfcmp strip.out a.out ||

+  { echo "*** failed a.out different from strip.out (with debug)"; exit -1; }

+

+test ! -f debug.out ||

+  { echo "*** failed a.out and debug.out exist"; exit -1; }

+

+exit 0

--- elfutils-0.169/tests/Makefile.in.orig	2017-06-07 21:48:55.475994222 +0200

+++ elfutils-0.169/tests/Makefile.in	2017-06-07 21:49:20.441430261 +0200

@@ -138,8 +138,8 @@

 	run-strip-test.sh run-strip-test2.sh run-strip-test3.sh \

 	run-strip-test4.sh run-strip-test5.sh run-strip-test6.sh \

 	run-strip-test7.sh run-strip-test8.sh run-strip-test9.sh \

-	run-strip-test10.sh run-strip-test11.sh run-strip-groups.sh \

-	run-strip-reloc.sh run-strip-strmerge.sh \

+	run-strip-test10.sh run-strip-test11.sh run-strip-nothing.sh \

+	run-strip-groups.sh run-strip-reloc.sh run-strip-strmerge.sh \

 	run-strip-nobitsalign.sh run-unstrip-test.sh \

 	run-unstrip-test2.sh run-unstrip-test3.sh run-unstrip-test4.sh \

 	run-unstrip-M.sh run-elfstrmerge-test.sh run-ecp-test.sh \

@@ -1046,6 +1046,7 @@

 	     run-strip-test4.sh run-strip-test5.sh run-strip-test6.sh \

 	     run-strip-test7.sh run-strip-test8.sh run-strip-groups.sh \

 	     run-strip-test9.sh run-strip-test10.sh run-strip-test11.sh \

+	     run-strip-nothing.sh \

 	     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 \

@@ -2332,6 +2333,13 @@

 	$(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \

 	--log-file $$b.log --trs-file $$b.trs \

 	$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \

+	"$$tst" $(AM_TESTS_FD_REDIRECT)

+run-strip-nothing.sh.log: run-strip-nothing.sh

+	@p='run-strip-nothing.sh'; \

+	b='run-strip-nothing.sh'; \

+	$(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \

+	--log-file $$b.log --trs-file $$b.trs \

+	$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \

 	"$$tst" $(AM_TESTS_FD_REDIRECT)

 run-strip-groups.sh.log: run-strip-groups.sh

 	@p='run-strip-groups.sh'; \