(fixed patch) pe-dll fix for exporting common symbols

(fixed patch) pe-dll fix for exporting common symbols

[Mumit Khan]

Ooops! Looks like I sent the wrong file. Please ignore the last one.

The following change allows i386-pe ``ld --shared'' to export common 
symbols correctly.

Thu Jun 24 12:41:50 1999  Mumit Khan  <khan@xraylith.wisc.edu>

	* pe-dll.c (process_def_file): Export common symbols correctly.

Index: pe-dll.c
===================================================================
RCS file: /cvs/binutils/binutils/ld/pe-dll.c,v
retrieving revision 1.2
diff -u -3 -p -u -p -r1.2 pe-dll.c
--- pe-dll.c	1999/05/11 21:06:48	1.2
+++ pe-dll.c	1999/06/24 20:48:17
@@ -383,13 +383,23 @@ process_def_file (abfd, info)
 				   name,
 				   false, false, true);
 
-      if (blhe && (blhe->type == bfd_link_hash_defined))
+      if (blhe 
+          && (blhe->type == bfd_link_hash_defined
+	      || (blhe->type == bfd_link_hash_common)))
 	{
 	  count_exported++;
 	  if (!pe_def_file->exports[i].flag_noname)
 	    count_exported_byname++;
-	  exported_symbol_offsets[i] = blhe->u.def.value;
-	  exported_symbol_sections[i] = blhe->u.def.section;
+          if (blhe->type == bfd_link_hash_defined)
+	    {
+	      exported_symbol_offsets[i] = blhe->u.def.value;
+	      exported_symbol_sections[i] = blhe->u.def.section;
+	    }
+	  else
+	    {
+	      exported_symbol_offsets[i] = 0;
+	      exported_symbol_sections[i] = blhe->u.c.p->section;
+	    }
 	  if (pe_def_file->exports[i].ordinal != -1)
 	    {
 	      if (max_ordinal < pe_def_file->exports[i].ordinal)

Regards,
Mumit

Re: (fixed patch) pe-dll fix for exporting common symbols

[Ian Lance Taylor]

   Date: Thu, 24 Jun 1999 14:43:05 -0500
   From: Mumit Khan <khan@xraylith.wisc.EDU>

   +          if (blhe->type == bfd_link_hash_defined)
   +	    {
   +	      exported_symbol_offsets[i] = blhe->u.def.value;
   +	      exported_symbol_sections[i] = blhe->u.def.section;
   +	    }
   +	  else
   +	    {
   +	      exported_symbol_offsets[i] = 0;
   +	      exported_symbol_sections[i] = blhe->u.c.p->section;
   +	    }

I don't know how PE handles common symbols in DLLs, but I don't see
how this could be right.  This appears to assume that the common
symbol is at the start of the section, but that is not correct.  You
can have multiple common symbols in a single section.  Try putting
several common symbols in a single input file, and see what happens.

Ian

Re: (fixed patch) pe-dll fix for exporting common symbols

[Mumit Khan]

Ian Lance Taylor <ian@zembu.com> writes:
>    Date: Thu, 24 Jun 1999 14:43:05 -0500
>    From: Mumit Khan <khan@xraylith.wisc.EDU>
> 
>    +          if (blhe->type == bfd_link_hash_defined)
>    +	    {
>    +	      exported_symbol_offsets[i] = blhe->u.def.value;
>    +	      exported_symbol_sections[i] = blhe->u.def.section;
>    +	    }
>    +	  else
>    +	    {
>    +	      exported_symbol_offsets[i] = 0;
>    +	      exported_symbol_sections[i] = blhe->u.c.p->section;
>    +	    }
> 
> I don't know how PE handles common symbols in DLLs, but I don't see
> how this could be right.  This appears to assume that the common
> symbol is at the start of the section, but that is not correct.  You
> can have multiple common symbols in a single section.  Try putting
> several common symbols in a single input file, and see what happens.

Yeah, it's completely bogus. Please pretend I never sent it in ;-)

Turns out that the offset value for common variables is *always* 16
no matter what the type is and how many there. So either way, I was
overwriting the values by unintended means.

I have yet to figure out how bfd is laying out common symbols, and how
I can access the info to write out the offsets into bss/common section.

Regards,
Mumit

Re: (fixed patch) pe-dll fix for exporting common symbols

[Ian Lance Taylor]

   Date: Thu, 24 Jun 1999 17:04:19 -0500
   From: Mumit Khan <khan@xraylith.wisc.EDU>

   I have yet to figure out how bfd is laying out common symbols, and how
   I can access the info to write out the offsets into bss/common section.

The linker lays out common symbols in lang_common in ld/ldlang.c.

I believe the code you are patching is run in the after_open emulation
routine, which is before the linker has laid out common symbols.  By
the time of the before_allocation emulation routine, the linker will
have laid out the common symbols (see lang_process or ldint.texinfo).
Assuming you are not doing a relocateable link, there should be no
more bfd_link_hash_common symbols after that point.

Ian

Re: (fixed patch) pe-dll fix for exporting common symbols

[Mumit Khan]

Ian Lance Taylor <ian@zembu.com> writes:
> 
> The linker lays out common symbols in lang_common in ld/ldlang.c.
> 
> I believe the code you are patching is run in the after_open emulation
> routine, which is before the linker has laid out common symbols.  By
> the time of the before_allocation emulation routine, the linker will
> have laid out the common symbols (see lang_process or ldint.texinfo).
> Assuming you are not doing a relocateable link, there should be no
> more bfd_link_hash_common symbols after that point.

Thank you! I believe you just pointed me to the correct fix. The problem 
is that currently the i386-pe ld is trying to write out the export 
table in after_open, hence there are actually symbols with 
bfd_link_hash_common, and things are going haywire. I'll redo it so that 
it's done in ldemul_finish instead.

Regards,
Mumit

Re: (fixed patch) pe-dll fix for exporting common symbols

[DJ Delorie]

> Thank you! I believe you just pointed me to the correct fix. The problem 
> is that currently the i386-pe ld is trying to write out the export 
> table in after_open, hence there are actually symbols with 
> bfd_link_hash_common, and things are going haywire. I'll redo it so that 
> it's done in ldemul_finish instead.

You can't build the export table in ldemul_finish because building the
export table produces yet another section that must be linked in (the
reloc section is a mess because of this, but at least it's one of the
last sections in the dll so has less ramifications if its size
changes).  Perhaps just the part about filling in the final values can
be shifted to the later stage?

take 2: pe-dll fix for exporting common symbols

[Mumit Khan]

DJ Delorie <dj@delorie.com> writes:
> 
> You can't build the export table in ldemul_finish because building the
> export table produces yet another section that must be linked in (the
> reloc section is a mess because of this, but at least it's one of the
> last sections in the dll so has less ramifications if its size
> changes).  Perhaps just the part about filling in the final values can
> be shifted to the later stage?

Ok, I finally have the right offsets in the test cases (and this time
real ones with lots of commons, both exported and local variety), and
all the addresses agree with dllwrap (multiple gcc/ld passes) created
DLLs.

The code is essentially the same as before, except that we lookup the
just exported symbol offsets after the commons are laid out. The only
offsets to change are the ones that were previous bfd_hash_link_common
type.

Fri Jun 25 10:07:41 1999  Mumit Khan  <khan@xraylith.wisc.edu>

	* pe-dll.c (process_def_file): Move the offset lookup from here to
	(fill_exported_offsets): here. New static function.
	(fill_edata): Use.

Index: pe-dll.c
===================================================================
RCS file: /cvs/binutils/binutils/ld/pe-dll.c,v
retrieving revision 1.2
diff -u -3 -p -r1.2 pe-dll.c
--- pe-dll.c	1999/05/11 21:06:48	1.2
+++ pe-dll.c	1999/06/25 16:14:14
@@ -383,13 +383,21 @@ process_def_file (abfd, info)
 				   name,
 				   false, false, true);
 
-      if (blhe && (blhe->type == bfd_link_hash_defined))
+      if (blhe 
+          && (blhe->type == bfd_link_hash_defined
+	      || (blhe->type == bfd_link_hash_common)))
 	{
 	  count_exported++;
 	  if (!pe_def_file->exports[i].flag_noname)
 	    count_exported_byname++;
-	  exported_symbol_offsets[i] = blhe->u.def.value;
-	  exported_symbol_sections[i] = blhe->u.def.section;
+
+	  /* Only fill in the sections. The actual offsets are computed
+	     in fill_exported_offsets() after common symbols are laid
+	     out.  */
+          if (blhe->type == bfd_link_hash_defined)
+	    exported_symbol_sections[i] = blhe->u.def.section;
+	  else
+	    exported_symbol_sections[i] = blhe->u.c.p->section;
 	  if (pe_def_file->exports[i].ordinal != -1)
 	    {
 	      if (max_ordinal < pe_def_file->exports[i].ordinal)
@@ -571,7 +579,44 @@ generate_edata (abfd, info)
 	      + name_table_size + strlen (dll_name) + 1);
 }
 
+/* Fill the exported symbol offsets. The preliminary work has already 
+   been done in process_def_file().  */
+
 static void
+fill_exported_offsets (abfd, info)
+     bfd *abfd;
+     struct bfd_link_info *info;
+{
+  int i, j;
+  struct bfd_link_hash_entry *blhe;
+  bfd *b;
+  struct sec *s;
+  def_file_export *e=0;
+
+  for (i = 0; i < pe_def_file->num_exports; i++)
+    {
+      char *name = (char *) xmalloc (strlen (pe_def_file->exports[i].internal_name) + 2);
+      if (pe_details->underscored)
+	{
+	  *name = '_';
+	  strcpy (name + 1, pe_def_file->exports[i].internal_name);
+	}
+      else
+	strcpy (name, pe_def_file->exports[i].internal_name);
+
+      blhe = bfd_link_hash_lookup (info->hash,
+				   name,
+				   false, false, true);
+
+      if (blhe && (blhe->type == bfd_link_hash_defined))
+	{
+	  exported_symbol_offsets[i] = blhe->u.def.value;
+        }
+      free (name);
+    }
+}
+
+static void
 fill_edata (abfd, info)
      bfd *abfd;
      struct bfd_link_info *info;
@@ -613,6 +658,8 @@ fill_edata (abfd, info)
   bfd_put_32 (abfd, ERVA (eaddresses), edata_d + 28);
   bfd_put_32 (abfd, ERVA (enameptrs), edata_d + 32);
   bfd_put_32 (abfd, ERVA (eordinals), edata_d + 36);
+
+  fill_exported_offsets (abfd, info);
 
   /* Ok, now for the filling in part */
   hint = 0;