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.

10 files changed, 201 insertions, 53 deletions

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index f23950d..048a972 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,5 +1,33 @@
 2020-04-29  Nick Clifton  <nickc@redhat.com>
 
+	* 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  <nickc@redhat.com>
+
 	* po/sr.po: Updated Serbian translation.
 
 2020-04-26  Alan Modra  <amodra@gmail.com>
diff --git a/binutils/NEWS b/binutils/NEWS
index 1650a3a..e1aaf99 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 0da0492..9a3a40f 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} <number or name>|@option{--hex-dump=}<number or name>]
         [@option{-p} <number or name>|@option{--string-dump=}<number or name>]
         [@option{-R} <number or name>|@option{--relocated-dump=}<number or name>]
@@ -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 <number or name>
 @itemx --hex-dump=<number or name>
 Displays the contents of the indicated section as a hexadecimal bytes.
diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index c75059b..675b4d0 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 2249750..27f8a51 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 5ec4690..fc47b40 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 68dfa32..4ceb6b2 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=<number|name>\n\
                          Dump the contents of section <number|name> as bytes\n\
   -p --string-dump=<number|name>\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 cc78e66..6b385fd 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 0000000..98dfdc3
--- /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 0000000..ba8de23
--- /dev/null
+++ b/binutils/testsuite/binutils-all/zero-sec.s
@@ -0,0 +1 @@
+	.data