Re: Strip 2.13 corrupts exe file

[Nick Clifton <nickc at redhat dot com>]

Hi Richard,

  I think that I have now traced down the cause of this problem.
  Strip is zeroing the $idata2 and $idata5 Data Dictionary entries
  (Import Directory and Import Address Table).  It is doing this
  because the code expects these entries to be reconstructed during a
  call to bfd_coff_final_link().

  Since, however a final link is not being performed, they are never
  reconstructed and so you end with a broken binary.  Please try
  applying the patch below to your sources and let me know if it fixes
  the problem.

Cheers
        Nick

2003-03-18  Nick Clifton  <nickc at redhat dot com>

	* peXXigen.c (_bfd_XXi_swap_aouthdr_out): Initialse $idata2
	and $idata5 even though they will be recreated during a call
	to bfd_coff_final_link, since it cannot be relied upon that
	this function will be called.

Index: bfd/peXXigen.c
===================================================================
RCS file: /cvs/src/src/bfd/peXXigen.c,v
retrieving revision 1.12
diff -c -3 -p -w -r1.12 peXXigen.c
*** bfd/peXXigen.c	30 Nov 2002 08:39:40 -0000	1.12
--- bfd/peXXigen.c	18 Mar 2003 17:08:10 -0000
*************** _bfd_XXi_swap_aouthdr_out (abfd, in, out
*** 570,575 ****
--- 571,578 ----
    struct internal_extra_pe_aouthdr *extra = &pe->pe_opthdr;
    PEAOUTHDR *aouthdr_out = (PEAOUTHDR *) out;
    bfd_vma sa, fa, ib;
+   IMAGE_DATA_DIRECTORY idata2, idata5;
+ 
    
    if (pe->force_minimum_alignment)
      {
*************** _bfd_XXi_swap_aouthdr_out (abfd, in, out
*** 586,591 ****
--- 589,597 ----
    fa = extra->FileAlignment;
    ib = extra->ImageBase;
  
+   idata2 = pe->pe_opthdr.DataDirectory[1];
+   idata5 = pe->pe_opthdr.DataDirectory[12];
+   
    if (aouthdr_in->tsize)
      {
        aouthdr_in->text_start -= ib;
*************** _bfd_XXi_swap_aouthdr_out (abfd, in, out
*** 614,641 ****
  #define SA(x) (((x) + sa -1 ) & (- sa))
  
    /* We like to have the sizes aligned.  */
- 
    aouthdr_in->bsize = FA (aouthdr_in->bsize);
  
    extra->NumberOfRvaAndSizes = IMAGE_NUMBEROF_DIRECTORY_ENTRIES;
  
!   /* first null out all data directory entries ..  */
    memset (extra->DataDirectory, 0, sizeof (extra->DataDirectory));
  
    add_data_entry (abfd, extra, 0, ".edata", ib);
- 
-   /* Don't call add_data_entry for .idata$2 or .idata$5.  It's done in
-      bfd_coff_final_link where all the required information is
-      available.  */
- 
-   /* However, until other .idata fixes are made (pending patch), the
-      entry for .idata is needed for backwards compatability.  FIXME.  */
-   add_data_entry (abfd, extra, 1, ".idata", ib);
- 
    add_data_entry (abfd, extra, 2, ".rsrc", ib);
- 
    add_data_entry (abfd, extra, 3, ".pdata", ib);
  
    /* For some reason, the virtual size (which is what's set by
       add_data_entry) for .reloc is not the same as the size recorded
       in this slot by MSVC; it doesn't seem to cause problems (so far),
--- 620,653 ----
  #define SA(x) (((x) + sa -1 ) & (- sa))
  
    /* We like to have the sizes aligned.  */
    aouthdr_in->bsize = FA (aouthdr_in->bsize);
  
    extra->NumberOfRvaAndSizes = IMAGE_NUMBEROF_DIRECTORY_ENTRIES;
  
!   /* First null out all data directory entries.  */
    memset (extra->DataDirectory, 0, sizeof (extra->DataDirectory));
  
    add_data_entry (abfd, extra, 0, ".edata", ib);
    add_data_entry (abfd, extra, 2, ".rsrc", ib);
    add_data_entry (abfd, extra, 3, ".pdata", ib);
  
+   /* In theory we do not need to call add_data_entry for .idata$2 or
+      .idata$5.  It will be done in bfd_coff_final_link where all the
+      required information is available.  If however, we are not going
+      to perform a final link, eg because we have been invoked by objcopy
+      or strip, then we need to make sure that these Data Directory
+      entries are initialised properly.
+ 
+      So - we copy the input values into the output values, and then, if
+      a final link is going to be performed, it can overwrite them.  */
+   extra->DataDirectory[1]  = idata2;
+   extra->DataDirectory[12] = idata5;
+ 
+   if (extra->DataDirectory[1].VirtualAddress == 0)
+     /* Until other .idata fixes are made (pending patch), the entry for
+        .idata is needed for backwards compatability.  FIXME.  */
+     add_data_entry (abfd, extra, 1, ".idata", ib);
+     
    /* For some reason, the virtual size (which is what's set by
       add_data_entry) for .reloc is not the same as the size recorded
       in this slot by MSVC; it doesn't seem to cause problems (so far),