commit b065d4a401e2120c11c92357b213a85e6d70f300 Author: Mark Wielaard 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 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 + + * 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 * 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 + + * run-strip-nothing.sh: New test. + * Makefile.am (TESTS): Add run-strip-nothing.sh. + (EXTRA_DIST): Likewise. + 2017-06-06 Mark Wielaard * 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 . + +. $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'; \