[PATCH] Eliminate redundant jump stubs on windows/cygwin/MinGW

["Murali Vemulapati" <murali dot vemulapati at gmail dot com>]

Hi,

The attached patch excludes unnecessary jump stub sections in the PE import
libraries from the final link. When a function symbol <sym> is imported
explicitly using a __declspec(dllimport) declaration, the compiler generates
the symbol reference as __imp_<sym> which directly resolves to the
symbol in the .idata$5 section (which will be the slot corresponding to
this symbol in the .idata section of the output image). The jump stub
in the .text section of the form jmp *__imp_<sym> is needed only when
the symbol is imported implicitly (without a
__declspec(dllimport) declaration). When the symbol is imported explicitly,
not only is the jump stub dead code, but also it generates an extra
base relocation in the .text section because the address bytes following the
jmp need to be patched at load time in the case of a DLL (if it is not mapped
at the preferred base address).

The fix needs to be done in two cases:
(a) In the case of direct import (without import libraries), the function
make_one() in pe-dll.c generates the jump stub and other .idata$ sections
for each imported symbol. In this case, we generate the .text section
containing jump stub only if it is needed.
(b) In the case of import through import libraries, in
gld_${EMULATION_NAME}_after_open(), we go through all the import libraries
and identify .text sections containing the jump stubs which are not
referenced by anyone and exclude them from being copied to the output image.

Thanks
murali

===================================================================
ld/ChangeLog:
2007-01-12  Murali Vemulapati  <murali.vemulapati@gmail.com>

	* pe-dll.c: (make_one) Conditionally include jump stubs.
	* emultempl/pe.em (gld_${EMULATION_NAME}_after_open): Identify
	redundant jump stubs from import libraries and exclude them from
	link.

ld/
Index: ld/pe-dll.c
===================================================================
RCS file: /cvs/src/src/ld/pe-dll.c,v
retrieving revision 1.94
diff -u -p -r1.94 pe-dll.c
--- ld/pe-dll.c	2 Jan 2007 07:43:13 -0000	1.94
+++ ld/pe-dll.c	13 Jan 2007 01:32:47 -0000
@@ -1873,7 +1873,7 @@ static const unsigned char jmp_arm_bytes


static bfd *
-make_one (def_file_export *exp, bfd *parent)
+make_one (def_file_export *exp, bfd *parent, bfd_boolean include_jmp_stub)
{
  asection *tx, *id7, *id5, *id4, *id6;
  unsigned char *td = NULL, *d7, *d5, *d4, *d6 = NULL;
@@ -1883,28 +1883,37 @@ make_one (def_file_export *exp, bfd *par
  const unsigned char *jmp_bytes = NULL;
  int jmp_byte_count = 0;

-  switch (pe_details->pe_arch)
+  /* Include the jump stub section only if it is needed. A jump
+     stub is needed if the symbol being imported <sym> is a function
+     symbol and there is at least one undefined reference to that
+     symbol. In other words, if all the import references to <sym> are
+     explicitly through _declspec(dllimport) then the jump stub is not
+     needed. */
+  if (include_jmp_stub)
    {
-    case PE_ARCH_i386:
-      jmp_bytes = jmp_ix86_bytes;
-      jmp_byte_count = sizeof (jmp_ix86_bytes);
-      break;
-    case PE_ARCH_sh:
-      jmp_bytes = jmp_sh_bytes;
-      jmp_byte_count = sizeof (jmp_sh_bytes);
-      break;
-    case PE_ARCH_mips:
-      jmp_bytes = jmp_mips_bytes;
-      jmp_byte_count = sizeof (jmp_mips_bytes);
-      break;
-    case PE_ARCH_arm:
-    case PE_ARCH_arm_epoc:
-    case PE_ARCH_arm_wince:
-      jmp_bytes = jmp_arm_bytes;
-      jmp_byte_count = sizeof (jmp_arm_bytes);
-      break;
-    default:
-      abort ();
+      switch (pe_details->pe_arch)
+	{
+	case PE_ARCH_i386:
+	  jmp_bytes = jmp_ix86_bytes;
+	  jmp_byte_count = sizeof (jmp_ix86_bytes);
+	  break;
+	case PE_ARCH_sh:
+	  jmp_bytes = jmp_sh_bytes;
+	  jmp_byte_count = sizeof (jmp_sh_bytes);
+	  break;
+	case PE_ARCH_mips:
+	  jmp_bytes = jmp_mips_bytes;
+	  jmp_byte_count = sizeof (jmp_mips_bytes);
+	  break;
+	case PE_ARCH_arm:
+	case PE_ARCH_arm_epoc:
+	case PE_ARCH_arm_wince:
+	  jmp_bytes = jmp_arm_bytes;
+	  jmp_byte_count = sizeof (jmp_arm_bytes);
+	  break;
+	default:
+	  abort ();
+	}
    }

  oname = xmalloc (20);
@@ -1930,7 +1939,7 @@ make_one (def_file_export *exp, bfd *par
    {
      quick_symbol (abfd, U ("_head_"), dll_symname, "", UNDSEC,
		    BSF_GLOBAL, 0);
-      if (! exp->flag_data)
+      if (include_jmp_stub)
	quick_symbol (abfd, "", exp->internal_name, "", tx, BSF_GLOBAL, 0);
      quick_symbol (abfd, "__imp_", exp->internal_name, "", id5,
		    BSF_GLOBAL, 0);
@@ -1941,7 +1950,7 @@ make_one (def_file_export *exp, bfd *par
    {
      quick_symbol (abfd, U ("_head_"), dll_symname, "", UNDSEC,
		    BSF_GLOBAL, 0);
-      if (! exp->flag_data)
+      if (include_jmp_stub)
	quick_symbol (abfd, U (""), exp->internal_name, "", tx,
		      BSF_GLOBAL, 0);
      quick_symbol (abfd, "__imp_", U (""), exp->internal_name, id5,
@@ -1956,7 +1965,7 @@ make_one (def_file_export *exp, bfd *par
    quick_symbol (abfd, U ("__imp_"), exp->internal_name, "", id5,
		  BSF_GLOBAL, 0);

-  if (! exp->flag_data)
+  if (include_jmp_stub)
    {
      bfd_set_section_size (abfd, tx, jmp_byte_count);
      td = xmalloc (jmp_byte_count);
@@ -1986,6 +1995,8 @@ make_one (def_file_export *exp, bfd *par
	}
      save_relocs (tx);
    }
+  else
+    bfd_set_section_size (abfd, tx, 0);

  bfd_set_section_size (abfd, id7, 4);
  d7 = xmalloc (4);
@@ -2050,7 +2061,8 @@ make_one (def_file_export *exp, bfd *par

  bfd_set_symtab (abfd, symtab, symptr);

-  bfd_set_section_contents (abfd, tx, td, 0, jmp_byte_count);
+  if (include_jmp_stub)
+    bfd_set_section_contents (abfd, tx, td, 0, jmp_byte_count);
  bfd_set_section_contents (abfd, id7, d7, 0, 4);
  bfd_set_section_contents (abfd, id5, d5, 0, PE_IDATA5_SIZE);
  bfd_set_section_contents (abfd, id4, d4, 0, PE_IDATA4_SIZE);
@@ -2398,7 +2410,8 @@ pe_dll_generate_implib (def_file *def, c
      if (pe_def_file->exports[i].flag_private)
	continue;
      def->exports[i].internal_name = def->exports[i].name;
-      n = make_one (def->exports + i, outarch);
+      n = make_one (def->exports + i, outarch,
+		    ! (def->exports + i)->flag_data);
      n->next = head;
      head = n;
      def->exports[i].internal_name = internal;
@@ -2474,6 +2487,7 @@ pe_process_import_defs (bfd *output_bfd,
	    /* See if we need this import.  */
	    size_t len = strlen (pe_def_file->imports[i].internal_name);
	    char *name = xmalloc (len + 2 + 6);
+	    bfd_boolean include_jmp_stub = FALSE;

 	    if (lead_at)
	      sprintf (name, "%s",
@@ -2484,7 +2498,8 @@ pe_process_import_defs (bfd *output_bfd,

	    blhe = bfd_link_hash_lookup (link_info->hash, name,
					 FALSE, FALSE, FALSE);
-
+	    /* Include the jump stub for <sym> only if the <sym>
+	       is undefined. */
	    if (!blhe || (blhe && blhe->type != bfd_link_hash_undefined))
	      {
		if (lead_at)
@@ -2497,6 +2512,8 @@ pe_process_import_defs (bfd *output_bfd,
		blhe = bfd_link_hash_lookup (link_info->hash, name,
					     FALSE, FALSE, FALSE);
	      }
+	    else
+	      include_jmp_stub = TRUE;
	    free (name);

	    if (blhe && blhe->type == bfd_link_hash_undefined)
@@ -2517,7 +2534,7 @@ pe_process_import_defs (bfd *output_bfd,
		exp.flag_constant = 0;
		exp.flag_data = pe_def_file->imports[i].data;
		exp.flag_noname = exp.name ? 0 : 1;
-		one = make_one (&exp, output_bfd);
+		one = make_one (&exp, output_bfd, (! exp.flag_data) && include_jmp_stub);
		add_bfd_to_link (one, one->filename, link_info);
	      }
	  }
Index: ld/emultempl/pe.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/pe.em,v
retrieving revision 1.122
diff -u -p -r1.122 pe.em
--- ld/emultempl/pe.em	18 Dec 2006 22:38:53 -0000	1.122
+++ ld/emultempl/pe.em	13 Jan 2007 01:32:48 -0000
@@ -1250,6 +1250,73 @@ gld_${EMULATION_NAME}_after_open (void)
	  }
      }
  }
+  {
+    /* The following chunk of code tries to identify jump stubs in
+       import libraries which are dead code and eliminates them
+       from the final link. For each exported symbol <sym>, there
+       is a object file in the import library with a .text section
+       and several .idata$* sections. The .text section contains the
+       symbol definition for <sym> which is a jump stub of the form
+       jmp *__imp_<sym>. The .idata$5 contains the symbol definition
+       for __imp_<sym> which is the address of the slot for <sym> in
+       the import address table. When a symbol is imported explicitly
+       using __declspec(dllimport) declaration, the compiler generates
+       a reference to __imp_<sym> which directly resolves to the
+       symbol in .idata$5, in which case the jump stub code is not
+       needed. The following code tries to identify jump stub sections
+       in import libraries which are not referred to by anyone and
+       marks them for exclusion from the final link. */
+    LANG_FOR_EACH_INPUT_STATEMENT (is)
+      {
+	if (is->the_bfd->my_archive)
+	  {
+	    int is_imp = 0;
+	    asection *sec, *stub_sec = NULL;
+
+	    /* See if this is an import library thunk.  */
+	    for (sec = is->the_bfd->sections; sec; sec = sec->next)
+	      {
+		if (strncmp (sec->name, ".idata\$", 7) == 0)
+		  is_imp = 1;
+		/* The section containing the jmp stub has code
+		   and has a reloc. */
+		if ((sec->flags & SEC_CODE) && sec->reloc_count)
+		  stub_sec = sec;
+	      }
+	
+	    if (is_imp && stub_sec)
+	      {
+		long symsize;
+		asymbol **symbols;
+		long src_count;
+		struct bfd_link_hash_entry * blhe;
+
+		symsize = bfd_get_symtab_upper_bound (is->the_bfd);
+		symbols = (asymbol **) xmalloc (symsize);
+		symsize = bfd_canonicalize_symtab (is->the_bfd, symbols);
+
+		for (src_count = 0; src_count < symsize; src_count++)
+		  {
+		    if (symbols[src_count]->section->id == stub_sec->id)
+		      {
+			/* This symbol belongs to the section containing
+			   the stub. */
+			blhe = bfd_link_hash_lookup (link_info.hash,
+						     symbols[src_count]->name,
+						     FALSE, FALSE, TRUE);
+			/* If the symbol in the stub section has no other
+			   undefined references, exclude the stub section
+			   from the final link. */
+			if (blhe && (blhe->type == bfd_link_hash_defined)
+			    && (blhe->u.undef.next == NULL))
+			  stub_sec->flags |= SEC_EXCLUDE;
+		      }
+		  }
+		free (symbols);
+	      }
+	  }
+      }
+  }
}

static void