readelf verdef and verneed

readelf ought to notify when a symbol wrongly has both a version
definition and a needed version.  This patch does that, and removes
the heuristic that only defined symbols in SHT_NOBITS sections have
verneed entries.

	* readelf (process_version_sections): Check DT_VERNEED and
	DT_VERDEF for all symbols.  Report "*both*" should a symbol
	have both a verneed and verdef.
	(get_symbol_version_string): Reduce indentation by early
	exits.  Don't use SHT_NOBITS heuristic to detect case where a
	defined symbol has a verneed entry.

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index e972c7e34dc28768f509acafa012e62d7baae488..8779082c6496e244233dd43594cb4c1f5c3ab43f 100644 (file)
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,12 @@
+2015-11-03  Alan Modra  <amodra@gmail.com>
+
+       * readelf (process_version_sections): Check DT_VERNEED and
+       DT_VERDEF for all symbols.  Report "*both*" should a symbol
+       have both a verneed and verdef.
+       (get_symbol_version_string): Reduce indentation by early
+       exits.  Don't use SHT_NOBITS heuristic to detect case where a
+       defined symbol has a verneed entry.
+
 2015-10-30  Nick Clifton  <nickc@redhat.com>
 
        * po/zh_CN.po: Updated (simplified) Chinese translation.

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 70a84e54e8efd7630ecfef37288868dac1028ccc..d5dd46f03cccd951bb974b50e5d2002ee946d9da 100644 (file)
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -9889,8 +9889,8 @@ process_version_sections (FILE * file)
            for (cnt = 0; cnt < total; cnt += 4)
              {
                int j, nn;
-               int check_def, check_need;
-               char * name;
+               char *name;
+               char *invalid = _("*invalid*");
 
                printf ("  %03x:", cnt);
 
@@ -9917,20 +9917,8 @@ process_version_sections (FILE * file)
                          break;
                        }
 
-                     check_def = 1;
-                     check_need = 1;
-                     if (symbols[cnt + j].st_shndx >= elf_header.e_shnum
-                         || section_headers[symbols[cnt + j].st_shndx].sh_type
-                            != SHT_NOBITS)
-                       {
-                         if (symbols[cnt + j].st_shndx == SHN_UNDEF)
-                           check_def = 0;
-                         else
-                           check_need = 0;
-                       }
-
-                     if (check_need
-                         && version_info[DT_VERSIONTAGIDX (DT_VERNEED)])
+                     name = NULL;
+                     if (version_info[DT_VERSIONTAGIDX (DT_VERNEED)])
                        {
                          Elf_Internal_Verneed ivn;
                          unsigned long offset;
@@ -9979,14 +9967,9 @@ process_version_sections (FILE * file)
                                  ivna.vna_name = BYTE_GET (evna.vna_name);
 
                                  if (ivna.vna_name >= string_sec->sh_size)
-                                   name = _("*invalid*");
+                                   name = invalid;
                                  else
                                    name = strtab + ivna.vna_name;
-                                 nn += printf ("(%s%-*s",
-                                               name,
-                                               12 - (int) strlen (name),
-                                               ")");
-                                 check_def = 0;
                                  break;
                                }
 
@@ -9995,7 +9978,7 @@ process_version_sections (FILE * file)
                          while (ivn.vn_next);
                        }
 
-                     if (check_def && data[cnt + j] != 0x8001
+                     if (data[cnt + j] != 0x8001
                          && version_info[DT_VERSIONTAGIDX (DT_VERDEF)])
                        {
                          Elf_Internal_Verdef ivd;
@@ -10043,15 +10026,18 @@ process_version_sections (FILE * file)
                              ivda.vda_name = BYTE_GET (evda.vda_name);
 
                              if (ivda.vda_name >= string_sec->sh_size)
-                               name = _("*invalid*");
+                               name = invalid;
+                             else if (name != NULL && name != invalid)
+                               name = _("*both*");
                              else
                                name = strtab + ivda.vda_name;
-                             nn += printf ("(%s%-*s",
-                                           name,
-                                           12 - (int) strlen (name),
-                                           ")");
                            }
                        }
+                     if (name != NULL)
+                       nn += printf ("(%s%-*s",
+                                     name,
+                                     12 - (int) strlen (name),
+                                     ")");
 
                      if (nn < 18)
                        printf ("%*c", 18 - nn, ' ');
@@ -10460,172 +10446,156 @@ get_symbol_version_string (FILE *file, int is_dynsym,
                           enum versioned_symbol_info *sym_info,
                           unsigned short *vna_other)
 {
-  const char *version_string = NULL;
+  unsigned char data[2];
+  unsigned short vers_data;
+  unsigned long offset;
 
-  if (is_dynsym
-      && version_info[DT_VERSIONTAGIDX (DT_VERSYM)] != 0)
-    {
-      unsigned char data[2];
-      unsigned short vers_data;
-      unsigned long offset;
-      int is_nobits;
-      int check_def;
+  if (!is_dynsym
+      || version_info[DT_VERSIONTAGIDX (DT_VERSYM)] == 0)
+    return NULL;
 
-      offset = offset_from_vma
-       (file, version_info[DT_VERSIONTAGIDX (DT_VERSYM)],
-        sizeof data + si * sizeof (vers_data));
+  offset = offset_from_vma (file, version_info[DT_VERSIONTAGIDX (DT_VERSYM)],
+                           sizeof data + si * sizeof (vers_data));
 
-      if (get_data (&data, file, offset + si * sizeof (vers_data),
-                   sizeof (data), 1, _("version data")) == NULL)
-       return NULL;
+  if (get_data (&data, file, offset + si * sizeof (vers_data),
+               sizeof (data), 1, _("version data")) == NULL)
+    return NULL;
+
+  vers_data = byte_get (data, 2);
 
-      vers_data = byte_get (data, 2);
+  if ((vers_data & VERSYM_HIDDEN) == 0 && vers_data <= 1)
+    return NULL;
 
-      is_nobits = (section_headers != NULL
-                  && psym->st_shndx < elf_header.e_shnum
-                  && section_headers[psym->st_shndx].sh_type
-                  == SHT_NOBITS);
+  /* Usually we'd only see verdef for defined symbols, and verneed for
+     undefined symbols.  However, symbols defined by the linker in
+     .dynbss for variables copied from a shared library in order to
+     avoid text relocations are defined yet have verneed.  We could
+     use a heuristic to detect the special case, for example, check
+     for verneed first on symbols defined in SHT_NOBITS sections, but
+     it is simpler and more reliable to just look for both verdef and
+     verneed.  .dynbss might not be mapped to a SHT_NOBITS section.  */
 
-      check_def = (psym->st_shndx != SHN_UNDEF);
+  if (psym->st_shndx != SHN_UNDEF
+      && vers_data != 0x8001
+      && version_info[DT_VERSIONTAGIDX (DT_VERDEF)])
+    {
+      Elf_Internal_Verdef ivd;
+      Elf_Internal_Verdaux ivda;
+      Elf_External_Verdaux evda;
+      unsigned long off;
 
-      if ((vers_data & VERSYM_HIDDEN) || vers_data > 1)
+      off = offset_from_vma (file,
+                            version_info[DT_VERSIONTAGIDX (DT_VERDEF)],
+                            sizeof (Elf_External_Verdef));
+
+      do
        {
-         if (version_info[DT_VERSIONTAGIDX (DT_VERNEED)]
-             && (is_nobits || ! check_def))
+         Elf_External_Verdef evd;
+
+         if (get_data (&evd, file, off, sizeof (evd), 1,
+                       _("version def")) == NULL)
+           {
+             ivd.vd_ndx = 0;
+             ivd.vd_aux = 0;
+             ivd.vd_next = 0;
+           }
+         else
            {
-             Elf_External_Verneed evn;
-             Elf_Internal_Verneed ivn;
-             Elf_Internal_Vernaux ivna;
+             ivd.vd_ndx = BYTE_GET (evd.vd_ndx);
+             ivd.vd_aux = BYTE_GET (evd.vd_aux);
+             ivd.vd_next = BYTE_GET (evd.vd_next);
+           }
 
-             /* We must test both.  */
-             offset = offset_from_vma
-               (file, version_info[DT_VERSIONTAGIDX (DT_VERNEED)],
-                sizeof evn);
+         off += ivd.vd_next;
+       }
+      while (ivd.vd_ndx != (vers_data & VERSYM_VERSION) && ivd.vd_next != 0);
 
-             do
-               {
-                 unsigned long vna_off;
+      if (ivd.vd_ndx == (vers_data & VERSYM_VERSION))
+       {
+         off -= ivd.vd_next;
+         off += ivd.vd_aux;
 
-                 if (get_data (&evn, file, offset, sizeof (evn), 1,
-                               _("version need")) == NULL)
-                   {
-                     ivna.vna_next = 0;
-                     ivna.vna_other = 0;
-                     ivna.vna_name = 0;
-                     break;
-                   }
+         if (get_data (&evda, file, off, sizeof (evda), 1,
+                       _("version def aux")) != NULL)
+           {
+             ivda.vda_name = BYTE_GET (evda.vda_name);
 
-                 ivn.vn_aux  = BYTE_GET (evn.vn_aux);
-                 ivn.vn_next = BYTE_GET (evn.vn_next);
+             if (psym->st_name != ivda.vda_name)
+               {
+                 *sym_info = ((vers_data & VERSYM_HIDDEN) != 0
+                              ? symbol_hidden : symbol_public);
+                 return (ivda.vda_name < strtab_size
+                         ? strtab + ivda.vda_name : _("<corrupt>"));
+               }
+           }
+       }
+    }
 
-                 vna_off = offset + ivn.vn_aux;
+  if (version_info[DT_VERSIONTAGIDX (DT_VERNEED)])
+    {
+      Elf_External_Verneed evn;
+      Elf_Internal_Verneed ivn;
+      Elf_Internal_Vernaux ivna;
 
-                 do
-                   {
-                     Elf_External_Vernaux evna;
+      offset = offset_from_vma (file,
+                               version_info[DT_VERSIONTAGIDX (DT_VERNEED)],
+                               sizeof evn);
+      do
+       {
+         unsigned long vna_off;
 
-                     if (get_data (&evna, file, vna_off,
-                                   sizeof (evna), 1,
-                                   _("version need aux (3)")) == NULL)
-                       {
-                         ivna.vna_next = 0;
-                         ivna.vna_other = 0;
-                         ivna.vna_name = 0;
-                       }
-                     else
-                       {
-                         ivna.vna_other = BYTE_GET (evna.vna_other);
-                         ivna.vna_next  = BYTE_GET (evna.vna_next);
-                         ivna.vna_name  = BYTE_GET (evna.vna_name);
-                       }
+         if (get_data (&evn, file, offset, sizeof (evn), 1,
+                       _("version need")) == NULL)
+           {
+             ivna.vna_next = 0;
+             ivna.vna_other = 0;
+             ivna.vna_name = 0;
+             break;
+           }
 
-                     vna_off += ivna.vna_next;
-                   }
-                 while (ivna.vna_other != vers_data
-                        && ivna.vna_next != 0);
+         ivn.vn_aux  = BYTE_GET (evn.vn_aux);
+         ivn.vn_next = BYTE_GET (evn.vn_next);
 
-                 if (ivna.vna_other == vers_data)
-                   break;
+         vna_off = offset + ivn.vn_aux;
 
-                 offset += ivn.vn_next;
-               }
-             while (ivn.vn_next != 0);
+         do
+           {
+             Elf_External_Vernaux evna;
 
-             if (ivna.vna_other == vers_data)
+             if (get_data (&evna, file, vna_off, sizeof (evna), 1,
+                           _("version need aux (3)")) == NULL)
                {
-                 *sym_info = symbol_undefined;
-                 *vna_other = ivna.vna_other;
-                 version_string = (ivna.vna_name < strtab_size
-                                   ? strtab + ivna.vna_name
-                                   : _("<corrupt>"));
-                 check_def = 0;
+                 ivna.vna_next = 0;
+                 ivna.vna_other = 0;
+                 ivna.vna_name = 0;
                }
-             else if (! is_nobits)
-               error (_("bad dynamic symbol\n"));
              else
-               check_def = 1;
-           }
-
-         if (check_def)
-           {
-             if (vers_data != 0x8001
-                 && version_info[DT_VERSIONTAGIDX (DT_VERDEF)])
                {
-                 Elf_Internal_Verdef ivd;
-                 Elf_Internal_Verdaux ivda;
-                 Elf_External_Verdaux evda;
-                 unsigned long off;
-
-                 off = offset_from_vma
-                   (file,
-                    version_info[DT_VERSIONTAGIDX (DT_VERDEF)],
-                    sizeof (Elf_External_Verdef));
-
-                 do
-                   {
-                     Elf_External_Verdef evd;
-
-                     if (get_data (&evd, file, off, sizeof (evd),
-                                   1, _("version def")) == NULL)
-                       {
-                         ivd.vd_ndx = 0;
-                         ivd.vd_aux = 0;
-                         ivd.vd_next = 0;
-                       }
-                     else
-                       {
-                         ivd.vd_ndx = BYTE_GET (evd.vd_ndx);
-                         ivd.vd_aux = BYTE_GET (evd.vd_aux);
-                         ivd.vd_next = BYTE_GET (evd.vd_next);
-                       }
-
-                     off += ivd.vd_next;
-                   }
-                 while (ivd.vd_ndx != (vers_data & VERSYM_VERSION)
-                        && ivd.vd_next != 0);
+                 ivna.vna_other = BYTE_GET (evna.vna_other);
+                 ivna.vna_next  = BYTE_GET (evna.vna_next);
+                 ivna.vna_name  = BYTE_GET (evna.vna_name);
+               }
 
-                 off -= ivd.vd_next;
-                 off += ivd.vd_aux;
+             vna_off += ivna.vna_next;
+           }
+         while (ivna.vna_other != vers_data && ivna.vna_next != 0);
 
-                 if (get_data (&evda, file, off, sizeof (evda),
-                               1, _("version def aux")) == NULL)
-                   return version_string;
+         if (ivna.vna_other == vers_data)
+           break;
 
-                 ivda.vda_name = BYTE_GET (evda.vda_name);
+         offset += ivn.vn_next;
+       }
+      while (ivn.vn_next != 0);
 
-                 if (psym->st_name != ivda.vda_name)
-                   {
-                     *sym_info = ((vers_data & VERSYM_HIDDEN) != 0
-                                  ? symbol_hidden : symbol_public);
-                     version_string = (ivda.vda_name < strtab_size
-                                       ? strtab + ivda.vda_name
-                                       : _("<corrupt>"));
-                   }
-               }
-           }
+      if (ivna.vna_other == vers_data)
+       {
+         *sym_info = symbol_undefined;
+         *vna_other = ivna.vna_other;
+         return (ivna.vna_name < strtab_size
+                 ? strtab + ivna.vna_name : _("<corrupt>"));
        }
     }
-  return version_string;
+  return NULL;
 }
 
 /* Dump the symbol table.  */