From 1b513401599fc5c35a3a8ad0321e0b00a0bdb0f8 Mon Sep 17 00:00:00 2001 From: Nick Clifton Date: Wed, 29 Apr 2020 16:01:40 +0100 Subject: [PATCH] Add a warning if an emtpty SHT_REL, SHT_RELA or SHT_PROGBITS section is detected. Disable all warnings unless the (new) lint mode is enabled. * readelf.c (warn): New function - like elfcomm.c version but only produces output if warnings are enabled. (struct options): Add --lint and --enable-checks. (usage): Add entry for --lint. (parse_args): Handle -L. If checks are enabled but no dumps have been selected then enable all dumps. (process_section_headers): Replace long if-then-else sequence with a switch. Add warning messages for empty SHT_REL, SHT_RELA and SHT_PROGBITS sections. (process_file): Do not complain if the file is an archive and lint mode has been enabled. * elfcomm.c (error): Make the function weak. (warn): Likewise. * NEWS: Mention the new feature. * doc/binutils.texi: Document the new feature. * dwarf.h (report_leb_status): Add file name and line number parameters. Include them in the diagnostic output. (READ_ULEB): Pass file and line number to report_leb_status. (READ_SLEB): Likewise. * dwarf.c (read_and_print_leb128): Pass file and line number to report_leb_status. * testsuite/binutils-all/readelf.exp: Add test of new feature. * testsuite/binutils-all/zero-sec.s: New test source file. * testsuite/binutils-all/zero-sec.r: Expected output from new test. --- binutils/ChangeLog | 28 ++++ binutils/NEWS | 6 + binutils/doc/binutils.texi | 10 ++ binutils/dwarf.c | 2 +- binutils/dwarf.h | 22 +-- binutils/elfcomm.c | 9 ++ binutils/readelf.c | 162 ++++++++++++++------ binutils/testsuite/binutils-all/readelf.exp | 11 ++ binutils/testsuite/binutils-all/zero-sec.r | 3 + binutils/testsuite/binutils-all/zero-sec.s | 1 + 10 files changed, 201 insertions(+), 53 deletions(-) create mode 100644 binutils/testsuite/binutils-all/zero-sec.r create mode 100644 binutils/testsuite/binutils-all/zero-sec.s diff --git a/binutils/ChangeLog b/binutils/ChangeLog index f23950d8873..048a9721a25 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,3 +1,31 @@ +2020-04-29 Nick Clifton + + * readelf.c (warn): New function - like elfcomm.c version but only + produces output if warnings are enabled. + (struct options): Add --lint and --enable-checks. + (usage): Add entry for --lint. + (parse_args): Handle -L. If checks are enabled but no dumps have + been selected then enable all dumps. + (process_section_headers): Replace long if-then-else sequence with + a switch. Add warning messages for empty SHT_REL, SHT_RELA and + SHT_PROGBITS sections. + (process_file): Do not complain if the file is an archive and lint + mode has been enabled. + * elfcomm.c (error): Make the function weak. + (warn): Likewise. + * NEWS: Mention the new feature. + * doc/binutils.texi: Document the new feature. + * dwarf.h (report_leb_status): Add file name and line number + parameters. Include them in the diagnostic output. + (READ_ULEB): Pass file and line number to report_leb_status. + (READ_SLEB): Likewise. + * dwarf.c (read_and_print_leb128): Pass file and line number to + report_leb_status. + * testsuite/binutils-all/readelf.exp: Add test of new feature. + * testsuite/binutils-all/zero-sec.s: New test source file. + * testsuite/binutils-all/zero-sec.r: Expected output from new + test. + 2020-04-29 Nick Clifton * po/sr.po: Updated Serbian translation. diff --git a/binutils/NEWS b/binutils/NEWS index 1650a3ac934..e1aaf996e89 100644 --- a/binutils/NEWS +++ b/binutils/NEWS @@ -1,5 +1,11 @@ -*- text -*- +* The readelf tool now has a -L or --lint or --enable-checks option which turns + on warning messages about possible problems with the file(s) being examined. + These checks include things like zero-sized sections, which are allowed by + the ELF standard but which nevertheless might be of concern if the user + was expecting them to actually contain something. + Changes in 2.34: * Binutils now supports debuginfod, an HTTP server for distributing diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi index 0da04929618..9a3a40f1476 100644 --- a/binutils/doc/binutils.texi +++ b/binutils/doc/binutils.texi @@ -4706,6 +4706,7 @@ readelf [@option{-a}|@option{--all}] [@option{-V}|@option{--version-info}] [@option{-A}|@option{--arch-specific}] [@option{-D}|@option{--use-dynamic}] + [@option{-L}|@option{--lint}|@option{--enable-checks}] [@option{-x} |@option{--hex-dump=}] [@option{-p} |@option{--string-dump=}] [@option{-R} |@option{--relocated-dump=}] @@ -4862,6 +4863,15 @@ symbol table sections. When displaying relocations, this option makes @command{readelf} display the dynamic relocations rather than the static relocations. +@item -L +@itemx --lint +@itemx --enable-checks +Displays warning messages about possible problems with the file(s) +being examined. If used on its own then all of the contents of the +file(s) will be examined. If used with one of the dumping options +then the warning messages will only be produced for the things being +displayed. + @item -x @itemx --hex-dump= Displays the contents of the indicated section as a hexadecimal bytes. diff --git a/binutils/dwarf.c b/binutils/dwarf.c index c75059bd93a..675b4d016fd 100644 --- a/binutils/dwarf.c +++ b/binutils/dwarf.c @@ -2008,7 +2008,7 @@ read_and_print_leb128 (unsigned char * data, int status; dwarf_vma val = read_leb128 (data, end, is_signed, bytes_read, &status); if (status != 0) - report_leb_status (status); + report_leb_status (status, __FILE__, __LINE__); else printf ("%s", dwarf_vmatoa (is_signed ? "d" : "u", val)); } diff --git a/binutils/dwarf.h b/binutils/dwarf.h index 2249750f87a..27f8a51521e 100644 --- a/binutils/dwarf.h +++ b/binutils/dwarf.h @@ -263,12 +263,12 @@ extern unsigned char * get_build_id (void *); #endif static inline void -report_leb_status (int status) +report_leb_status (int status, const char *file, unsigned long lnum) { if ((status & 1) != 0) - error (_("LEB end of data\n")); + error (_("%s:%lu: end of data encountered whilst reading LEB\n"), file, lnum); else if ((status & 2) != 0) - error (_("LEB value too large\n")); + error (_("%s:%lu: read LEB value is too large to store in destination variable\n"), file, lnum); } #define SKIP_ULEB(start, end) \ @@ -277,7 +277,8 @@ report_leb_status (int status) unsigned int _len; \ read_leb128 (start, end, FALSE, &_len, NULL); \ start += _len; \ - } while (0) + } \ + while (0) #define SKIP_SLEB(start, end) \ do \ @@ -285,7 +286,8 @@ report_leb_status (int status) unsigned int _len; \ read_leb128 (start, end, TRUE, &_len, NULL); \ start += _len; \ - } while (0) + } \ + while (0) #define READ_ULEB(var, start, end) \ do \ @@ -299,8 +301,9 @@ report_leb_status (int status) (var) = _val; \ if ((var) != _val) \ _status |= 2; \ - report_leb_status (_status); \ - } while (0) + report_leb_status (_status, __FILE__, __LINE__); \ + } \ + while (0) #define READ_SLEB(var, start, end) \ do \ @@ -314,5 +317,6 @@ report_leb_status (int status) (var) = _val; \ if ((var) != _val) \ _status |= 2; \ - report_leb_status (_status); \ - } while (0) + report_leb_status (_status, __FILE__, __LINE__); \ + } \ + while (0) diff --git a/binutils/elfcomm.c b/binutils/elfcomm.c index 5ec4690e132..fc47b40757f 100644 --- a/binutils/elfcomm.c +++ b/binutils/elfcomm.c @@ -34,6 +34,15 @@ extern char *program_name; +/* FIXME: This definition really ought to be in ansidecl.h. */ +#ifndef ATTRIBUTE_WEAK +#define ATTRIBUTE_WEAK __attribute__((weak)) +#endif + +/* Allow the following two functions to be overridden if desired. */ +void error (const char *, ...) ATTRIBUTE_WEAK; +void warn (const char *, ...) ATTRIBUTE_WEAK; + void error (const char *message, ...) { diff --git a/binutils/readelf.c b/binutils/readelf.c index 68dfa32f1fa..4ceb6b2151f 100644 --- a/binutils/readelf.c +++ b/binutils/readelf.c @@ -199,7 +199,8 @@ struct dump_list_entry /* A dynamic array of flags indicating for which sections a dump has been requested via command line switches. */ -struct dump_data { +struct dump_data +{ dump_type * dump_sects; unsigned int num_dump_sects; }; @@ -230,6 +231,8 @@ static bfd_boolean do_ctf = FALSE; static bfd_boolean do_arch = FALSE; static bfd_boolean do_notes = FALSE; static bfd_boolean do_archive_index = FALSE; +static bfd_boolean do_checks = FALSE; +static bfd_boolean check_all = FALSE; static bfd_boolean is_32bit_elf = FALSE; static bfd_boolean decompress_dumps = FALSE; @@ -384,6 +387,25 @@ bfd_vmatoa (char *fmtch, bfd_vma value) return ret; } +/* A version of the warn() function that is disabled if do_checks is not active. */ + +void +warn (const char *message, ...) +{ + va_list args; + + if (!do_checks) + return; + + /* Try to keep warning messages in sync with the program's normal output. */ + fflush (stdout); + + va_start (args, message); + fprintf (stderr, _("%s: Warning: "), program_name); + vfprintf (stderr, message, args); + va_end (args); +} + /* Retrieve NMEMB structures, each SIZE bytes long from FILEDATA starting at OFFSET + the offset of the current archive member, if we are examining an archive. Put the retrieved data into VAR, if it is not NULL. Otherwise @@ -4478,6 +4500,8 @@ static struct option options[] = {"relocs", no_argument, 0, 'r'}, {"notes", no_argument, 0, 'n'}, {"dynamic", no_argument, 0, 'd'}, + {"lint", no_argument, 0, 'L'}, + {"enable-checks", no_argument, 0, 'L'}, {"arch-specific", no_argument, 0, 'A'}, {"version-info", no_argument, 0, 'V'}, {"use-dynamic", no_argument, 0, 'D'}, @@ -4525,7 +4549,7 @@ usage (FILE * stream) -e --headers Equivalent to: -h -l -S\n\ -s --syms Display the symbol table\n\ --symbols An alias for --syms\n\ - --dyn-syms Display the dynamic symbol table\n\ + --dyn-syms Display the dynamic symbol table\n\ -n --notes Display the core notes (if present)\n\ -r --relocs Display the relocations (if present)\n\ -u --unwind Display the unwind info (if present)\n\ @@ -4534,6 +4558,7 @@ usage (FILE * stream) -A --arch-specific Display architecture specific information (if any)\n\ -c --archive-index Display the symbol/file index in an archive\n\ -D --use-dynamic Use the dynamic section info when displaying symbols\n\ + -L --lint|--enable-checks Display warning messages for possible problems\n\ -x --hex-dump=\n\ Dump the contents of section as bytes\n\ -p --string-dump=\n\ @@ -4662,7 +4687,7 @@ parse_args (struct dump_data *dumpdata, int argc, char ** argv) usage (stderr); while ((c = getopt_long - (argc, argv, "ADHINR:SVWacdeghi:lnp:rstuvw::x:z", options, NULL)) != EOF) + (argc, argv, "ADHILNR:SVWacdeghi:lnp:rstuvw::x:z", options, NULL)) != EOF) { switch (c) { @@ -4736,6 +4761,9 @@ parse_args (struct dump_data *dumpdata, int argc, char ** argv) case 'c': do_archive_index = TRUE; break; + case 'L': + do_checks = TRUE; + break; case 'x': request_dump (dumpdata, HEX_DUMP); break; @@ -4832,7 +4860,18 @@ parse_args (struct dump_data *dumpdata, int argc, char ** argv) && !do_histogram && !do_debugging && !do_arch && !do_notes && !do_section_groups && !do_archive_index && !do_dyn_syms) - usage (stderr); + { + if (do_checks) + { + check_all = TRUE; + do_dynamic = do_syms = do_reloc = do_unwind = do_sections = TRUE; + do_segments = do_header = do_dump = do_version = TRUE; + do_histogram = do_debugging = do_arch = do_notes = TRUE; + do_section_groups = do_archive_index = do_dyn_syms = TRUE; + } + else + usage (stderr); + } } static const char * @@ -6277,7 +6316,7 @@ process_section_headers (Filedata * filedata) while (0) #define CHECK_ENTSIZE(section, i, type) \ - CHECK_ENTSIZE_VALUES (section, i, sizeof (Elf32_External_##type), \ + CHECK_ENTSIZE_VALUES (section, i, sizeof (Elf32_External_##type), \ sizeof (Elf64_External_##type)) for (i = 0, section = filedata->section_headers; @@ -6286,8 +6325,11 @@ process_section_headers (Filedata * filedata) { char * name = SECTION_NAME (section); - if (section->sh_type == SHT_DYNSYM) + /* Run some sanity checks on the headers and + possibly fill in some file data as well. */ + switch (section->sh_type) { + case SHT_DYNSYM: if (filedata->dynamic_symbols != NULL) { error (_("File contains multiple dynamic symbol tables\n")); @@ -6297,45 +6339,74 @@ process_section_headers (Filedata * filedata) CHECK_ENTSIZE (section, i, Sym); filedata->dynamic_symbols = GET_ELF_SYMBOLS (filedata, section, &filedata->num_dynamic_syms); - } - else if (section->sh_type == SHT_STRTAB - && streq (name, ".dynstr")) - { - if (filedata->dynamic_strings != NULL) + break; + + case SHT_STRTAB: + if (streq (name, ".dynstr")) { - error (_("File contains multiple dynamic string tables\n")); - continue; + if (filedata->dynamic_strings != NULL) + { + error (_("File contains multiple dynamic string tables\n")); + continue; + } + + filedata->dynamic_strings + = (char *) get_data (NULL, filedata, section->sh_offset, + 1, section->sh_size, _("dynamic strings")); + filedata->dynamic_strings_length + = filedata->dynamic_strings == NULL ? 0 : section->sh_size; } + break; + + case SHT_SYMTAB_SHNDX: + { + elf_section_list * entry = xmalloc (sizeof * entry); + + entry->hdr = section; + entry->next = filedata->symtab_shndx_list; + filedata->symtab_shndx_list = entry; + } + break; + + case SHT_SYMTAB: + CHECK_ENTSIZE (section, i, Sym); + break; + + case SHT_GROUP: + CHECK_ENTSIZE_VALUES (section, i, GRP_ENTRY_SIZE, GRP_ENTRY_SIZE); + break; - filedata->dynamic_strings - = (char *) get_data (NULL, filedata, section->sh_offset, - 1, section->sh_size, _("dynamic strings")); - filedata->dynamic_strings_length - = filedata->dynamic_strings == NULL ? 0 : section->sh_size; - } - else if (section->sh_type == SHT_SYMTAB_SHNDX) - { - elf_section_list * entry = xmalloc (sizeof * entry); - - entry->hdr = section; - entry->next = filedata->symtab_shndx_list; - filedata->symtab_shndx_list = entry; - } - else if (section->sh_type == SHT_SYMTAB) - CHECK_ENTSIZE (section, i, Sym); - else if (section->sh_type == SHT_GROUP) - CHECK_ENTSIZE_VALUES (section, i, GRP_ENTRY_SIZE, GRP_ENTRY_SIZE); - else if (section->sh_type == SHT_REL) - CHECK_ENTSIZE (section, i, Rel); - else if (section->sh_type == SHT_RELA) - CHECK_ENTSIZE (section, i, Rela); - else if ((do_debugging || do_debug_info || do_debug_abbrevs - || do_debug_lines || do_debug_pubnames || do_debug_pubtypes - || do_debug_aranges || do_debug_frames || do_debug_macinfo - || do_debug_str || do_debug_loc || do_debug_ranges - || do_debug_addr || do_debug_cu_index || do_debug_links) - && (const_strneq (name, ".debug_") - || const_strneq (name, ".zdebug_"))) + case SHT_REL: + CHECK_ENTSIZE (section, i, Rel); + if (section->sh_size == 0) + warn (_("Section '%s': zero-sized relocation section\n"), name); + break; + + case SHT_RELA: + CHECK_ENTSIZE (section, i, Rela); + if (section->sh_size == 0) + warn (_("Section '%s': zero-sized relocation section\n"), name); + break; + + case SHT_NOTE: + case SHT_PROGBITS: + if (section->sh_size == 0) + /* This is not illegal according to the ELF standard, but + it might be an indication that something is wrong. */ + warn (_("Section '%s': has a size of zero - is this intended ?\n"), name); + break; + + default: + break; + } + + if ((do_debugging || do_debug_info || do_debug_abbrevs + || do_debug_lines || do_debug_pubnames || do_debug_pubtypes + || do_debug_aranges || do_debug_frames || do_debug_macinfo + || do_debug_str || do_debug_loc || do_debug_ranges + || do_debug_addr || do_debug_cu_index || do_debug_links) + && (const_strneq (name, ".debug_") + || const_strneq (name, ".zdebug_"))) { if (name[1] == 'z') name += sizeof (".zdebug_") - 1; @@ -20646,7 +20717,7 @@ process_file (char * file_name) } else { - if (do_archive_index) + if (do_archive_index && !check_all) error (_("File %s is not an archive so its index cannot be displayed.\n"), file_name); @@ -20712,9 +20783,14 @@ main (int argc, char ** argv) parse_args (& cmdline, argc, argv); if (optind < (argc - 1)) + /* When displaying information for more than one file, + prefix the information with the file name. */ show_name = TRUE; else if (optind >= argc) { + /* Ensure that the warning is always displayed. */ + do_checks = TRUE; + warn (_("Nothing to do.\n")); usage (stderr); } diff --git a/binutils/testsuite/binutils-all/readelf.exp b/binutils/testsuite/binutils-all/readelf.exp index cc78e66ea3c..6b385fda2d6 100644 --- a/binutils/testsuite/binutils-all/readelf.exp +++ b/binutils/testsuite/binutils-all/readelf.exp @@ -506,4 +506,15 @@ if {![binutils_assemble $srcdir/$subdir/dwo.s tmpdir/dwo.o]} then { readelf_test {--debug-dump=links} $tempfile readelf.k2 {} } +if {![binutils_assemble $srcdir/$subdir/zero-sec.s tmpdir/zero-sec.o]} then { + unresolved "readelf --enable-checks (failed to assemble zero-sec.s)" +} else { + if ![is_remote host] { + set tempfile tmpdir/zero-sec.o + } else { + set tempfile [remote_download host tmpdir/zero-sec.o] + } + + readelf_test {--enable-checks --sections --wide} $tempfile zero-sec.r {} +} diff --git a/binutils/testsuite/binutils-all/zero-sec.r b/binutils/testsuite/binutils-all/zero-sec.r new file mode 100644 index 00000000000..98dfdc36869 --- /dev/null +++ b/binutils/testsuite/binutils-all/zero-sec.r @@ -0,0 +1,3 @@ +#... +.*Warning: Section '.*': has a size of zero - is this intended.* +#... diff --git a/binutils/testsuite/binutils-all/zero-sec.s b/binutils/testsuite/binutils-all/zero-sec.s new file mode 100644 index 00000000000..ba8de231cbd --- /dev/null +++ b/binutils/testsuite/binutils-all/zero-sec.s @@ -0,0 +1 @@ + .data -- 2.30.2