[Patch] Add --identify-strict option to dlltool; Handle ms-style implibs [Was: --identify-ms]

[Charles Wilson <cygwin at cwilson dot fastmail dot fm>]

This is an improved implementation. According to Danny Smith, ld can, in
fact, link against import libraries generated by the MS toolchain.
Therefore, it is a useful thing if dlltool's --identify option can be
made to work with those import libraries, as well as those created by
ld.  However, the only way I could get that to work properly (and
automatically, without the need for a special command-line switch) was
to search the symbol list of the implib for the DLL-structure symbol
that MS (but not ld) inserts: __NULL_IMPORT_DESCRIPTOR.

Once dlltool "knows" whether it is dealing with an ms-style or
binutils-style import library, it can search for the dllname in the
proper section for that style.

I also added the capability to accumulate -- and report -- multiple
dllnames, for those import libs that specify more than one. I did find
one example of this "in the wild": Vfw32.Lib from the Windows SDK
imports symbols directly from three different DLLs.

To preserve the (quite useful, for libtool) behavior where dlltool exits
with error when there are multiple imported DLLs, I added the
--identify-strict option.

--
Chuck

 NEWS              |    7 +
 dlltool.c         |  325 ++++++++++++++++++++++++++++++++++++++++++++----------
 doc/binutils.texi |   19 +--
 3 files changed, 286 insertions(+), 65 deletions(-)

? dlltool.c.debug
Index: NEWS
===================================================================
RCS file: /cvs/src/src/binutils/NEWS,v
retrieving revision 1.72
diff -u -p -r1.72 NEWS
--- NEWS	4 Dec 2008 10:29:15 -0000	1.72
+++ NEWS	10 Jan 2009 06:12:50 -0000
@@ -1,5 +1,12 @@
 -*- text -*-
 
+* Added --identify-strict switch to cause --identify <implib> to 
+  report an error when the import library is associated with
+  multiple DLLs.
+
+* Added --identify <implib> option to dlltool, which determines the
+  name of the DLL associated with the specified <implib>.
+
 * Support for PowerPC booke64 instructions has been removed.  The assembler no
   longer accepts -mbooke32 or -mbooke64 and the disassembler no longer accepts
   -Mbooke32 or -Mbooke64.  Instead, -mbooke and -Mbooke should be used.
Index: dlltool.c
===================================================================
RCS file: /cvs/src/src/binutils/dlltool.c,v
retrieving revision 1.86
diff -u -p -r1.86 dlltool.c
--- dlltool.c	25 Nov 2008 08:48:49 -0000	1.86
+++ dlltool.c	10 Jan 2009 06:12:52 -0000
@@ -353,7 +353,27 @@ static int no_idata5;
 static char *exp_name;
 static char *imp_name;
 static char *identify_imp_name;
-static char *identify_dll_name;
+static bfd_boolean identify_ms;
+static bfd_boolean identify_strict;
+
+/* Holds a linked list of dllnames associated with the
+   specified import lib. Used by the identify_* code.
+   The _head entry is always empty (_head->dllname is
+   NULL).
+ */
+typedef struct dll_name_list_t {
+  char                    *dllname;
+  struct dll_name_list_t  *next;
+} dll_name_list_type;
+static dll_name_list_type *identify_dll_name_list_head;
+static dll_name_list_type *identify_dll_name_list_tail;
+/* dll_name_list management functions */
+static void identify_append_dll_name_to_list (const char *data);
+static int  identify_count_dll_name_list (void);
+static void identify_print_dll_name_list (void);
+static void identify_free_dll_name_list (dll_name_list_type* entry);
+static bfd_boolean identify_member_contains_symname_result = FALSE;
+
 static char *head_label;
 static char *imp_name_lab;
 static char *dll_name;
@@ -727,10 +747,14 @@ static bfd *make_head (void);
 static bfd *make_tail (void);
 static void gen_lib_file (void);
 static void identify_dll_for_implib (void);
-static void identify_search_archive (bfd*);
-static void identify_search_member (bfd*, bfd*);
+static void identify_search_archive (bfd *,
+   void (*operation) (bfd *, bfd *, void *),
+   void *);
+static void identify_search_member (bfd *, bfd *, void *);
 static bfd_boolean identify_process_section_p (asection *);
 static void identify_search_section (bfd *, asection *, void *);
+static void identify_member_contains_symname (bfd *abfd, bfd  *, void *);
+
 static int pfunc (const void *, const void *);
 static int nfunc (const void *, const void *);
 static void remove_null_names (export_type **);
@@ -2926,24 +2950,147 @@ gen_lib_file (void)
   inform (_("Created lib file"));
 }
 
+/* management of the identify_dll_name_list */
+static void
+identify_append_dll_name_to_list (const char *data)
+{
+  /* allocate new node */
+  dll_name_list_type *entry =
+    (dll_name_list_type*) xmalloc(sizeof(dll_name_list_type));
+  /* initialize its values */
+  entry->dllname = xstrdup ((char*) data);
+  entry->next = NULL;
+  /* add to tail, and move tail */
+  identify_dll_name_list_tail->next = entry;
+  identify_dll_name_list_tail = entry;
+}
+
+static int 
+identify_count_dll_name_list (void)
+{
+  int count = 0;
+  dll_name_list_type* p = identify_dll_name_list_head;
+  while (p && p->next)
+    {
+      count++;
+      p = p->next;
+    }
+  return count;
+}
+
+static void 
+identify_print_dll_name_list (void)
+{
+  dll_name_list_type* p = identify_dll_name_list_head;
+  while (p && p->next && p->next->dllname && *(p->next->dllname))
+    {
+      printf ("%s\n", p->next->dllname);
+      p = p->next;
+    }
+}
+
+static void 
+identify_free_dll_name_list (dll_name_list_type* entry)
+{
+  if (entry)
+    {
+      if (entry->next)
+        {
+          identify_free_dll_name_list (entry->next);
+          entry->next = NULL;
+        }
+      if (entry->dllname)
+        {
+          free (entry->dllname);
+          entry->dllname = NULL;
+        }
+      free (entry);
+    }
+}
+
+/* identify_member_contains_symname
+
+   Search the symbol table of the suppled bfd* for a 
+   symbol whose name matches obj (where obj is
+   cast to const char *). If found, set global 
+   variable identify_member_contains_symname_result
+   TRUE. It is the caller's responsibility to set
+   the result variable FALSE before iterating with
+   this function.
+*/   
+static void 
+identify_member_contains_symname
+   (bfd  *abfd,
+    bfd  *archive_bfd ATTRIBUTE_UNUSED,
+    void *obj)
+{
+  long storage_needed;
+  asymbol **symbol_table;
+  long number_of_symbols;
+  long i;
+
+  const char *name = (const char *)obj;
+  /* if we already found the symbol in a different member,
+     short circuit */
+  if (identify_member_contains_symname_result)
+    return;
+
+  storage_needed = bfd_get_symtab_upper_bound (abfd);
+  if (storage_needed <= 0)
+    return;
+
+  symbol_table = xmalloc (storage_needed);
+  number_of_symbols =
+    bfd_canonicalize_symtab (abfd, symbol_table);
+  if (number_of_symbols < 0)
+    {
+      free (symbol_table);
+      return;
+    }
+
+  for (i = 0; i < number_of_symbols; i++)
+    {
+      if (strncmp(symbol_table[i]->name, name, strlen(name)) == 0)
+      {
+        identify_member_contains_symname_result = TRUE;
+        break;
+      }
+    }
+  free (symbol_table);
+}
+
 /* identify_dll_for_implib
 
    This is the main implementation for the --identify option.
    Given the name of an import library in identify_imp_name,
-   search all archive members for an .idata$7 section
-   (.idata$6 on PPC). This section will consist of a single
-   char* constant, indicating the name of the DLL represented
-   by the import library.
-
-   It is possible to construct an import library that has
-   two members with a non-empty .idata$7 section, but these
-   are not often seen in normal operation.  In this case,
-   an error is flagged.
+   first determine if the import library is a GNU binutils-style
+   one (where the DLL name is stored in an .idata$7 (.idata$6
+   on PPC) section, or if it is a MS-style one (where the DLL
+   name, along with much other data, is stored in the .idata$6
+   section). We determine the style of import library by 
+   searching for the DLL-structure symbol inserted by MS tools:
+   __NULL_IMPORT_DESCRIPTOR.
+
+   Once we know which section to search, evaluate each section
+   for the appropriate properties that indicate it may contain
+   the name of the associated DLL (this differs depending on the
+   style).  Add the contents of all sections which meet the
+   criteria to a linked list of dll names.
+
+   Finally, print them all to stdout. (If --identify-strict,
+   an error is reported if more than one match was found).
 */   
 static void 
 identify_dll_for_implib (void)
 {
   bfd* abfd = NULL;
+  int count = 0;
+
+  /* initialize identify_dll_name_list */
+  identify_dll_name_list_head = (dll_name_list_type*) xmalloc (sizeof(dll_name_list_type));
+  identify_dll_name_list_head->dllname = NULL;
+  identify_dll_name_list_head->next = NULL;
+  identify_dll_name_list_tail = identify_dll_name_list_head;
 
   bfd_init ();
 
@@ -2960,31 +3107,72 @@ identify_dll_for_implib (void)
       fatal ("%s is not a library", identify_imp_name);
     }
 
-  identify_search_archive (abfd);
+  /* Detect if this a Microsoft import library */
+  identify_member_contains_symname_result = FALSE;
+  identify_search_archive (
+    abfd,
+    identify_member_contains_symname,
+    (void*)"__NULL_IMPORT_DESCRIPTOR");
+  if (identify_member_contains_symname_result)
+    {
+      identify_ms = TRUE;
+    }
+  
+  /* rewind abfd */
+  if (!bfd_close (abfd))
+    bfd_fatal (identify_imp_name);
+  abfd = bfd_openr (identify_imp_name, 0);
+  if (abfd == NULL)
+    {
+      bfd_fatal (identify_imp_name);
+    }
+  if (!bfd_check_format (abfd, bfd_archive))
+    {
+      if (!bfd_close (abfd))
+        bfd_fatal (identify_imp_name);
+
+      fatal ("%s is not a library", identify_imp_name);
+    }
+ 
+  /* Now search for the dll name */
+  identify_search_archive (abfd, identify_search_member, NULL);
 
   if (!bfd_close (abfd))
     bfd_fatal (identify_imp_name);
 
-  if (identify_dll_name && *identify_dll_name)
+  count = identify_count_dll_name_list();
+  if (count > 0)
     {
-      printf ("%s\n",identify_dll_name);
-      free (identify_dll_name);
-      identify_dll_name = NULL;
+      if (identify_strict && count > 1)
+        {
+          identify_free_dll_name_list (identify_dll_name_list_head);
+          identify_dll_name_list_head = NULL;
+          fatal ("Import library `%s' specifies two or more dlls", identify_imp_name);
+        }
+      identify_print_dll_name_list();
+      identify_free_dll_name_list (identify_dll_name_list_head);
+      identify_dll_name_list_head = NULL;
     }
   else
     {
-      fatal ("Unable to determine dll name for %s (not an import library?)", identify_imp_name);
+      identify_free_dll_name_list (identify_dll_name_list_head);
+      identify_dll_name_list_head = NULL;
+      fatal ("Unable to determine dll name for `%s' (not an import library?)", identify_imp_name);
     }
 }
 
 /* identify_search_archive
 
-   Loop over all members of the archive, inspecting 
-   each for the presence of an .idata$7 (.idata$6 on PPC)
-   section with non-empty contents.
+   Loop over all members of the archive, applying the
+   supplied function to each member that is a bfd_object.
+   The function will be called as if:
+      func (member_bfd, abfd, user_storage)
 */   
 static void
-identify_search_archive (bfd* abfd)
+identify_search_archive
+  (bfd* abfd, 
+   void (*operation) (bfd *, bfd *, void *),
+   void *user_storage)
 {
   bfd *arfile = NULL;
   bfd *last_arfile = NULL;
@@ -3002,7 +3190,7 @@ identify_search_archive (bfd* abfd)
         }
       if (bfd_check_format_matches (arfile, bfd_object, &matching))
         {
-          identify_search_member (arfile, abfd);
+          (*operation) (arfile, abfd, user_storage);
         }
       else
         {
@@ -3024,20 +3212,23 @@ identify_search_archive (bfd* abfd)
 
 /* identify_search_member
 
-   Search all sections of an archive member for the 
-   one with section name of .idata$7 (.idata$6 on PPC)
-   and non-empty contents.
+   Call the identify_search_section() function for
+   each section of this archive member.
 */   
 static void
-identify_search_member (bfd* abfd, bfd* archive_bfd ATTRIBUTE_UNUSED)
+identify_search_member
+   (bfd  *abfd,
+    bfd  *archive_bfd ATTRIBUTE_UNUSED,
+    void *obj)
 {
-  bfd_map_over_sections (abfd, identify_search_section, NULL);
+  bfd_map_over_sections (abfd, identify_search_section, obj);
 }
 
 /* identify_process_section_p
 
    This predicate returns true if section->name
-   is .idata$7 (.idata$6 on PPC).
+   matches the desired value. By default, this is
+   .idata$7 (.idata$6 on PPC, or when --identify-ms)
 */   
 static bfd_boolean
 identify_process_section_p (asection * section)
@@ -3049,8 +3240,12 @@ identify_process_section_p (asection * s
 #else
   ".idata$7";
 #endif
+  static const char * MS_SECTION_NAME = ".idata$6";
 
-  if (strcmp (SECTION_NAME, section->name) == 0)
+  const char * section_name =
+    (identify_ms ? MS_SECTION_NAME : SECTION_NAME);
+  
+  if (strcmp (section_name, section->name) == 0)
     return TRUE;
   return FALSE;
 }
@@ -3058,15 +3253,10 @@ identify_process_section_p (asection * s
 /* identify_search_section
 
    If *section has contents and its name is .idata$7
-   (.data$6 on PPC) then store the contents in 
-   identify_dll_name as an xmalloc'ed array.
-
-   However, if identify_dll_name already has
-   a value, flag an error. We don't know how to handle
-   import libraries that directly reference more than
-   one DLL. (This is different than forwarded symbols.
-   Such import libraries are not seen in normal operation,
-   and must be specifically constructed.)
+   (.data$6 on PPC or if import lib ms-generated) -- and
+   it satisfies several other constraints -- then store
+   the contents in the list pointed to by
+   identify_dll_name_list_head.
 */   
 static void
 identify_search_section (bfd *abfd, asection *section, void *dummy ATTRIBUTE_UNUSED)
@@ -3080,6 +3270,20 @@ identify_search_section (bfd *abfd, asec
   if (! identify_process_section_p (section))
     return;
 
+  /* binutils import libs seem distinguish the .idata$7 section
+     that contains the DLL name from other .idata$7 sections by
+     the absence of the SEC_RELOC flag
+  */
+  if (!identify_ms && ((section->flags & SEC_RELOC) == SEC_RELOC))
+    return;
+
+  /* MS import libs seem to distinguish the .idata$6 section
+     that contains the DLL name from other .idata$6 sections
+     by the presence of the SEC_DATA flag
+  */
+  if (identify_ms && ((section->flags & SEC_DATA) == 0))
+    return;
+
   if ((datasize = bfd_section_size (abfd, section)) == 0)
     return;
 
@@ -3089,27 +3293,26 @@ identify_search_section (bfd *abfd, asec
   bfd_get_section_contents (abfd, section, data, 0, datasize);
   data[datasize] = '\0';
 
-  if (data[0] != '\0')
+  /* Use a heuristic to determine if data is a dll name.
+     Possible to defeat this if (a) the library has MANY
+     (more than 0x302f) imports, (b) it is an ms-style 
+     import library, but (c) it is buggy, in that the SEC_DATA
+     flag is set on the "wrong" sections.  This heuristic might
+     also fail to record a valid dll name if the dllname is
+     uses a multibyte or unicode character set (is that valid?).
+
+     This heuristic is based on the fact that symbols names in
+     the chosen section -- as opposed to the dll name -- begin
+     at offset 2 in the data. The first two bytes are a 16bit
+     little-endian count, and start at 0x0000. However, the dll
+     name begins at offset 0 in the data. We assume that the
+     dll name does not contain unprintable characters.
+   */
+  if (data[0] != '\0' && ISPRINT (data[0]) &&
+      ((datasize < 2) || ISPRINT (data[1])) )
     {
-      if (identify_dll_name != NULL)
-        {
-          if (*identify_dll_name != '\0')
-            {
-              /* The import library specifies two different DLLs.
-                 Treat this as an error. */
-              fatal ("Import library `%s' specifies two or more dlls: `%s' and `%s'",
-                     identify_imp_name, identify_dll_name, data);
-            }
-          else
-            {
-              /* For some reason memory was allocated, but the
-                 contents were empty. Free the memory and continue. */
-              free (identify_dll_name);
-            }
-        }
-      identify_dll_name = xstrdup ((char*) data);
+      identify_append_dll_name_to_list (data);
     }
-
   free (data);
 }
 
@@ -3367,6 +3570,7 @@ usage (FILE *file, int status)
   fprintf (file, _("   -n --no-delete            Keep temp files (repeat for extra preservation).\n"));
   fprintf (file, _("   -t --temp-prefix <prefix> Use <prefix> to construct temp file names.\n"));
   fprintf (file, _("   -I --identify <implib>    Report the name of the DLL associated with <implib>.\n"));
+  fprintf (file, _("      --identify-strict      Causes --identify to report error when multiple DLLs.\n"));
   fprintf (file, _("   -v --verbose              Be verbose.\n"));
   fprintf (file, _("   -V --version              Display the program version.\n"));
   fprintf (file, _("   -h --help                 Display this information.\n"));
@@ -3386,6 +3590,7 @@ usage (FILE *file, int status)
 #define OPTION_EXCLUDE_SYMS		(OPTION_NO_EXPORT_ALL_SYMS + 1)
 #define OPTION_NO_DEFAULT_EXCLUDES	(OPTION_EXCLUDE_SYMS + 1)
 #define OPTION_ADD_STDCALL_UNDERSCORE	(OPTION_NO_DEFAULT_EXCLUDES + 1)
+#define OPTION_IDENTIFY_STRICT		(OPTION_ADD_STDCALL_UNDERSCORE + 1)
 
 static const struct option long_options[] =
 {
@@ -3408,6 +3613,7 @@ static const struct option long_options[
   {"add-stdcall-alias", no_argument, NULL, 'A'},
   {"ext-prefix-alias", required_argument, NULL, 'p'},
   {"identify", required_argument, NULL, 'I'},
+  {"identify-strict", no_argument, NULL, OPTION_IDENTIFY_STRICT},
   {"verbose", no_argument, NULL, 'v'},
   {"version", no_argument, NULL, 'V'},
   {"help", no_argument, NULL, 'h'},
@@ -3470,6 +3676,9 @@ main (int ac, char **av)
 	case OPTION_ADD_STDCALL_UNDERSCORE:
 	  add_stdcall_underscore = 1;
 	  break;
+	case OPTION_IDENTIFY_STRICT:
+	  identify_strict = 1;
+	  break;
 	case 'x':
 	  no_idata4 = 1;
 	  break;
Index: doc/binutils.texi
===================================================================
RCS file: /cvs/src/src/binutils/doc/binutils.texi,v
retrieving revision 1.135
diff -u -p -r1.135 binutils.texi
--- doc/binutils.texi	23 Dec 2008 09:01:46 -0000	1.135
+++ doc/binutils.texi	10 Jan 2009 06:12:55 -0000
@@ -3370,7 +3370,8 @@ dlltool [@option{-d}|@option{--input-def
         [@option{-k}|@option{--kill-at}] [@option{-A}|@option{--add-stdcall-alias}]
         [@option{-p}|@option{--ext-prefix-alias} @var{prefix}]
         [@option{-x}|@option{--no-idata4}] [@option{-c}|@option{--no-idata5}]
-        [@option{-I}|@option{--identify} @var{library-file-name}] [@option{-i}|@option{--interwork}]
+        [@option{-I}|@option{--identify} @var{library-file-name}] [@option{--identify-strict}]
+        [@option{-I}[@option{-i}|@option{--interwork}]
         [@option{-n}|@option{--nodelete}] [@option{-t}|@option{--temp-prefix} @var{prefix}]
         [@option{-v}|@option{--verbose}]
         [@option{-h}|@option{--help}] [@option{-V}|@option{--version}]
@@ -3592,12 +3593,16 @@ with certain operating systems.
 @item -I @var{filename}
 @itemx --identify @var{filename}
 Specifies that @command{dlltool} should inspect the import library
-indicated by @var{filename} and report, on @code{stdout}, the name of
-the associated DLL.  This can be performed in addition to any other
-operations indicated by the other options and arguments.  @command{dlltool}
-@option{--identify} fails if the import library does not exist, is not
-actually an import library, or (rarely) if the import library somehow
-specifies more than one associated DLL.
+indicated by @var{filename} and report, on @code{stdout}, the name(s)
+of the associated DLL(s).  This can be performed in addition to any
+other operations indicated by the other options and arguments.
+@command{dlltool} fails if the import library does not exist or is not
+actually an import library. See also @option{--identify-strict}.
+
+@item --identify-strict
+Modifies the behavior of the @option{--identify} option, such
+that an error is reported if @var{filename} is associated with
+more than one DLL.
 
 @item -i
 @itemx --interwork