From e0b3232e9a0bc8215ca1be29b1ec6df43811eb87 Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Wed, 7 Jun 2017 14:05:36 +0200 Subject: strip: Make sure old .shstrab is removed when eu-strip recreates it. Although we always recreate the .shstrtab section for the new output file we never explicitly assumed it could be removed. It might not be possible to remove it when the section string table is shared with a symbol table. But if it is removable we should (and recreate it for the new section list). Regression introduced in commit elfutils-0.163-33-gdf7dfab. "Handle merged strtab/shstrtab string tables in strip and unstrip." Add extra testcase to explicitly check for this case. https://sourceware.org/bugzilla/show_bug.cgi?id=21525 Signed-off-by: Mark Wielaard --- src/strip.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'src/strip.c') diff --git a/src/strip.c b/src/strip.c index f7474418..11b2a379 100644 --- a/src/strip.c +++ b/src/strip.c @@ -711,11 +711,13 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname, in the sh_link or sh_info element it cannot be removed either */ for (cnt = 1; cnt < shnum; ++cnt) - /* Check whether the section can be removed. */ + /* Check whether the section can be removed. Since we will create + a new .shstrtab assume it will be removed too. */ if (remove_shdrs ? !(shdr_info[cnt].shdr.sh_flags & SHF_ALLOC) - : ebl_section_strip_p (ebl, ehdr, &shdr_info[cnt].shdr, - shdr_info[cnt].name, remove_comment, - remove_debug)) + : (ebl_section_strip_p (ebl, ehdr, &shdr_info[cnt].shdr, + shdr_info[cnt].name, remove_comment, + remove_debug) + || cnt == ehdr->e_shstrndx)) { /* For now assume this section will be removed. */ shdr_info[cnt].idx = 0; @@ -1062,8 +1064,10 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname, } /* Test whether we are doing anything at all. */ - if (cnt == idx) - /* Nope, all removable sections are already gone. */ + 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. */ -- cgit v1.2.3 From b58aebe71e0b4863db1b7fd3e942e36303257f3a Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Wed, 7 Jun 2017 20:32:38 +0200 Subject: 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 --- src/strip.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) (limited to 'src/strip.c') diff --git a/src/strip.c b/src/strip.c index 11b2a379..2bf95f90 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; } -- cgit v1.2.3 From d03be4f70c688a8c675935973663014c3c4bba76 Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Fri, 14 Jul 2017 17:09:40 +0200 Subject: strip: Add --keep-section=SECTION and --remove-section=SECTION. Adds two new output options: --keep-section=SECTION Keep the named section. SECTION is an extended wildcard pattern. May be given more than once. --remove-section=SECTION Remove the named section. SECTION is an extended wildcard pattern. May be given more than once. Only non-allocated sections can be removed. The --remove-section was already partially implemented, but only for the .comment section. The short option -R is to be compatible with binutils. The new testcase makes sure that various combinations of kept/removed sections pull the correct dependencies into the output and/or debug files. https://bugzilla.redhat.com/show_bug.cgi?id=1465997 Signed-off-by: Mark Wielaard --- src/strip.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 110 insertions(+), 17 deletions(-) (limited to 'src/strip.c') diff --git a/src/strip.c b/src/strip.c index 2bf95f90..4a35ea1d 100644 --- a/src/strip.c +++ b/src/strip.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -60,6 +61,7 @@ ARGP_PROGRAM_BUG_ADDRESS_DEF = PACKAGE_BUGREPORT; #define OPT_PERMISSIVE 0x101 #define OPT_STRIP_SECTIONS 0x102 #define OPT_RELOC_DEBUG 0x103 +#define OPT_KEEP_SECTION 0x104 /* Definitions of arguments for argp functions. */ @@ -83,7 +85,8 @@ static const struct argp_option options[] = N_("Resolve all trivial relocations between debug sections if the removed sections are placed in a debug file (only relevant for ET_REL files, operation is not reversable, needs -f)"), 0 }, { "remove-comment", OPT_REMOVE_COMMENT, NULL, 0, N_("Remove .comment section"), 0 }, - { "remove-section", 'R', "SECTION", OPTION_HIDDEN, NULL, 0 }, + { "remove-section", 'R', "SECTION", 0, N_("Remove the named section. SECTION is an extended wildcard pattern. May be given more than once. Only non-allocated sections can be removed."), 0 }, + { "keep-section", OPT_KEEP_SECTION, "SECTION", 0, N_("Keep the named section. SECTION is an extended wildcard pattern. May be given more than once."), 0 }, { "permissive", OPT_PERMISSIVE, NULL, 0, N_("Relax a few rules to handle slightly broken ELF files"), 0 }, { NULL, 0, NULL, 0, NULL, 0 } @@ -157,6 +160,58 @@ static bool permissive; /* If true perform relocations between debug sections. */ static bool reloc_debug; +/* Sections the user explicitly wants to keep or remove. */ +struct section_pattern +{ + char *pattern; + struct section_pattern *next; +}; + +static struct section_pattern *keep_secs = NULL; +static struct section_pattern *remove_secs = NULL; + +static void +add_pattern (struct section_pattern **patterns, const char *pattern) +{ + struct section_pattern *p = xmalloc (sizeof *p); + p->pattern = xstrdup (pattern); + p->next = *patterns; + *patterns = p; +} + +static void +free_sec_patterns (struct section_pattern *patterns) +{ + struct section_pattern *pattern = patterns; + while (pattern != NULL) + { + struct section_pattern *p = pattern; + pattern = p->next; + free (p->pattern); + free (p); + } +} + +static void +free_patterns (void) +{ + free_sec_patterns (keep_secs); + free_sec_patterns (remove_secs); +} + +static bool +section_name_matches (struct section_pattern *patterns, const char *name) +{ + struct section_pattern *pattern = patterns; + while (pattern != NULL) + { + if (fnmatch (pattern->pattern, name, FNM_EXTMATCH) == 0) + return true; + pattern = pattern->next; + } + return false; +} + int main (int argc, char *argv[]) @@ -207,6 +262,7 @@ Only one input file allowed together with '-o' and '-f'")); while (++remaining < argc); } + free_patterns (); return result; } @@ -257,14 +313,13 @@ parse_opt (int key, char *arg, struct argp_state *state) break; case 'R': - if (!strcmp (arg, ".comment")) + if (fnmatch (arg, ".comment", FNM_EXTMATCH) == 0) remove_comment = true; - else - { - argp_error (state, - gettext ("-R option supports only .comment section")); - return EINVAL; - } + add_pattern (&remove_secs, arg); + break; + + case OPT_KEEP_SECTION: + add_pattern (&keep_secs, arg); break; case 'g': @@ -284,6 +339,16 @@ parse_opt (int key, char *arg, struct argp_state *state) case 's': /* Ignored for compatibility. */ break; + case ARGP_KEY_SUCCESS: + if (remove_comment == true + && section_name_matches (keep_secs, ".comment")) + { + argp_error (state, + gettext ("cannot both keep and remove .comment section")); + return EINVAL; + } + break; + default: return ARGP_ERR_UNKNOWN; } @@ -622,6 +687,28 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname, goto fail_close; } + /* Sanity check the user. */ + if (section_name_matches (remove_secs, shdr_info[cnt].name)) + { + if ((shdr_info[cnt].shdr.sh_flags & SHF_ALLOC) != 0) + { + error (0, 0, + gettext ("Cannot remove allocated section '%s'"), + shdr_info[cnt].name); + result = 1; + goto fail_close; + } + + if (section_name_matches (keep_secs, shdr_info[cnt].name)) + { + error (0, 0, + gettext ("Cannot both keep and remove section '%s'"), + shdr_info[cnt].name); + result = 1; + goto fail_close; + } + } + /* Mark them as present but not yet investigated. */ shdr_info[cnt].idx = 1; @@ -709,6 +796,7 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname, know how to handle them - if a section is referred to from a section which is not removed in the sh_link or sh_info element it cannot be removed either + - the user might have explicitly said to remove or keep a section */ for (cnt = 1; cnt < shnum; ++cnt) /* Check whether the section can be removed. Since we will create @@ -717,8 +805,13 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname, : (ebl_section_strip_p (ebl, ehdr, &shdr_info[cnt].shdr, shdr_info[cnt].name, remove_comment, remove_debug) - || cnt == ehdr->e_shstrndx)) + || cnt == ehdr->e_shstrndx + || section_name_matches (remove_secs, shdr_info[cnt].name))) { + /* The user might want to explicitly keep this one. */ + if (section_name_matches (keep_secs, shdr_info[cnt].name)) + continue; + /* For now assume this section will be removed. */ shdr_info[cnt].idx = 0; @@ -2174,14 +2267,14 @@ while computing checksum for debug information")); if (shdr_info != NULL) { /* For some sections we might have created an table to map symbol - table indices. */ - if (any_symtab_changes) - for (cnt = 1; cnt <= shdridx; ++cnt) - { - free (shdr_info[cnt].newsymidx); - if (shdr_info[cnt].debug_data != NULL) - free (shdr_info[cnt].debug_data->d_buf); - } + table indices. Or we might kept (original) data around to put + into the .debug file. */ + for (cnt = 1; cnt <= shdridx; ++cnt) + { + free (shdr_info[cnt].newsymidx); + if (shdr_info[cnt].debug_data != NULL) + free (shdr_info[cnt].debug_data->d_buf); + } /* Free data we allocated for the .gnu_debuglink section. */ free (debuglink_buf); -- cgit v1.2.3 From 55cb7dfa7e9afb3660b21e51434641c7287baf11 Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Thu, 20 Jul 2017 22:34:29 +0200 Subject: strip: Deal with ARM data marker symbols pointing to debug sections. ARM data marker symbols "$d" indicate the start of a sequence of data items in a section. For data only sections no data marker symbol is necessary, but may be put pointing to the start of the section. binutils however has a bug which places a data marker symbol somewhere inside the section (at least for .debug_frame). https://sourceware.org/bugzilla/show_bug.cgi?id=21809 When strip finds a symbol pointing to a debug section that would be put into the .debug file then it will copy over the whole symbol table. This isn't necessary because the symbol is redundant. Add an ebl hook to recognize data marker symbols with implementations for arm and aarch64. Use it in strip to strip such symbols from the symbol table if they point to a debug section. Signed-off-by: Mark Wielaard --- src/strip.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) (limited to 'src/strip.c') diff --git a/src/strip.c b/src/strip.c index 4a35ea1d..773ed548 100644 --- a/src/strip.c +++ b/src/strip.c @@ -1,5 +1,5 @@ /* Discard section not used at runtime from object files. - Copyright (C) 2000-2012, 2014, 2015, 2016 Red Hat, Inc. + Copyright (C) 2000-2012, 2014, 2015, 2016, 2017 Red Hat, Inc. This file is part of elfutils. Written by Ulrich Drepper , 2000. @@ -960,8 +960,19 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname, if (shdr_info[scnidx].idx == 0) /* This symbol table has a real symbol in a discarded section. So preserve the - original table in the debug file. */ - shdr_info[cnt].debug_data = symdata; + original table in the debug file. Unless + it is a redundant data marker to a debug + (data only) section. */ + if (! (ebl_section_strip_p (ebl, ehdr, + &shdr_info[scnidx].shdr, + shdr_info[scnidx].name, + remove_comment, + remove_debug) + && ebl_data_marker_symbol (ebl, sym, + elf_strptr (elf, + shdr_info[cnt].shdr.sh_link, + sym->st_name)))) + shdr_info[cnt].debug_data = symdata; } } @@ -1293,7 +1304,10 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname, shdr_info[cnt].shdr.sh_name = dwelf_strent_off (shdr_info[cnt].se); /* Update the section header from the input file. Some fields - might be section indeces which now have to be adjusted. */ + might be section indeces which now have to be adjusted. Keep + the index to the "current" sh_link in case we need it to lookup + symbol table names. */ + size_t sh_link = shdr_info[cnt].shdr.sh_link; if (shdr_info[cnt].shdr.sh_link != 0) shdr_info[cnt].shdr.sh_link = shdr_info[shdr_info[cnt].shdr.sh_link].idx; @@ -1492,13 +1506,17 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname, /* The symbol points to a section that is discarded but isn't preserved in the debug file. Check that this is a section or group signature symbol - for a section which has been removed. */ + for a section which has been removed. Or a special + data marker symbol to a debug section. */ { elf_assert (GELF_ST_TYPE (sym->st_info) == STT_SECTION || ((shdr_info[sidx].shdr.sh_type == SHT_GROUP) && (shdr_info[sidx].shdr.sh_info - == inner))); + == inner)) + || ebl_data_marker_symbol (ebl, sym, + elf_strptr (elf, sh_link, + sym->st_name))); } } -- cgit v1.2.3