RFA: broken strip/objcopy

[H.J. Lu]

On Fri, Apr 24, 2009 at 2:58 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> 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.
>

We can't calll bfd_copy_private_bfd_data first. This patch only
copies pe_opthdr.



-- 
H.J.
--
bfd/

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

	* peXXigen.c (_bfd_XX_bfd_copy_private_bfd_data_common): Don't
	copy pe_opthdr.

binutils/

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

	* objcopy.c (copy_object): Copy pe_opthdr before changing PE
	parameters.
-------------- next part --------------
bfd/

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

	* peXXigen.c (_bfd_XX_bfd_copy_private_bfd_data_common): Don't
	copy pe_opthdr.

binutils/

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

	* objcopy.c (copy_object): Copy pe_opthdr before changing PE
	parameters.

Index: binutils/objcopy.c
===================================================================
--- binutils/objcopy.c	(revision 5816)
+++ binutils/objcopy.c	(working copy)
@@ -1491,6 +1491,11 @@ copy_object (bfd *ibfd, bfd *obfd)
       /* Set up PE parameters.  */
       pe_data_type *pe = pe_data (obfd);
 
+      /* Copy PE parameters before changing them.  */
+      if (ibfd->xvec->flavour == bfd_target_coff_flavour
+	  && bfd_pei_p (ibfd))
+	pe->pe_opthdr = pe_data (ibfd)->pe_opthdr;
+
       if (pe_file_alignment != (bfd_vma) -1)
 	pe->pe_opthdr.FileAlignment = pe_file_alignment;
       else
Index: bfd/peXXigen.c
===================================================================
--- bfd/peXXigen.c	(revision 5816)
+++ bfd/peXXigen.c	(working copy)
@@ -2188,7 +2188,7 @@ _bfd_XX_bfd_copy_private_bfd_data_common
   ipe = pe_data (ibfd);
   ope = pe_data (obfd);
  
-  ope->pe_opthdr = ipe->pe_opthdr;
+  /* pe_opthdr is copied in copy_object.  */
   ope->dll = ipe->dll;
 
   /* Don't copy input subsystem if output is different from input.  */