RFA: broken strip/objcopy

[H.J. Lu]

On Fri, Apr 24, 2009 at 6:41 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Apr 24, 2009 at 6:00 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> Hello,
>>
>> 2009/4/24 Kai Tietz <ktietz70@googlemail.com>:
>>> Hello,
>>>
>>> The current implementation of objcopy/strip in binutils has problems,
>>> because for pe-coff targets. It assumes at the moment that it is an
>>> i386 version. This leads to issues (especially on strip) for PE+
>>> target that binaries are getting corrupted. So I think we have here to
>>> move those target specific pe-coff parts into seperate files. (I
>>> assume that possibly for othe pe-coff targets then i386 the same
>>> problem exists). Of course we could use here target specific macros to
>>> change internal structures, but for a mulilib version of them this
>>> isn't suitable anymore.
>>> So possible somebody of you have a strong opinion about how this issue
>>> could be solved. I'll prepare a patch for it and post it soon.
>>
>> Ah, I found the major issue here
>>
>> 2009-04-17  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>        PR binutils/10074
>>
>> This patch made objcopy and strip for PE-Coff target 32-bit PE only
>
> I am trying to understand where the problem is. pe->pe_opthdr is
> supposed to target independent. Why doesn't it work for w64?

The problem is

#ifdef COFF_PAGE_SIZE
    /* Clear D_PAGED if section alignment is smaller than
       COFF_PAGE_SIZE.  */
   if (pe_data (abfd)->pe_opthdr.SectionAlignment < COFF_PAGE_SIZE)
     abfd->flags &= ~D_PAGED;
#endif

in coff_compute_section_file_positions. For objcopy,
pe_data (abfd)->pe_opthdr.SectionAlignment may not be set. To
change PE parameters, we should call bfd_copy_private_bfd_data
before changing PE parameters. This patch works for me on
both w32 and w64.


-- 
H.J.
--
2009-04-24  H.J. Lu  <hongjiu.lu@intel.com>

	* objcopy.c (copy_object): Call bfd_copy_private_bfd_data
	before changing PE parameters.
-------------- next part --------------
2009-04-24  H.J. Lu  <hongjiu.lu@intel.com>

	* objcopy.c (copy_object): Call bfd_copy_private_bfd_data
	before changing PE parameters.

Index: binutils/objcopy.c
===================================================================
--- binutils/objcopy.c	(revision 5816)
+++ binutils/objcopy.c	(working copy)
@@ -1412,6 +1412,7 @@ copy_object (bfd *ibfd, bfd *obfd)
   void *dhandle;
   enum bfd_architecture iarch;
   unsigned int imach;
+  bfd_boolean copy_private_bfd_data = TRUE;
 
   if (ibfd->xvec->byteorder != obfd->xvec->byteorder
       && ibfd->xvec->byteorder != BFD_ENDIAN_UNKNOWN
@@ -1491,6 +1492,15 @@ copy_object (bfd *ibfd, bfd *obfd)
       /* Set up PE parameters.  */
       pe_data_type *pe = pe_data (obfd);
 
+      /* Copy PE parameters before changing them.  */
+      copy_private_bfd_data = FALSE;
+      if (! bfd_copy_private_bfd_data (ibfd, obfd))
+	{
+	  bfd_nonfatal_message (NULL, obfd, NULL,
+				_("error copying private BFD data"));
+	  return FALSE;
+	}
+
       if (pe_file_alignment != (bfd_vma) -1)
 	pe->pe_opthdr.FileAlignment = pe_file_alignment;
       else
@@ -1913,7 +1923,8 @@ copy_object (bfd *ibfd, bfd *obfd)
      from the input BFD to the output BFD.  This is done last to
      permit the routine to look at the filtered symbol table, which is
      important for the ECOFF code at least.  */
-  if (! bfd_copy_private_bfd_data (ibfd, obfd))
+  if (copy_private_bfd_data
+      && ! bfd_copy_private_bfd_data (ibfd, obfd))
     {
       bfd_nonfatal_message (NULL, obfd, NULL,
 			    _("error copying private BFD data"));