This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PE/COFF _raw_size 2003-04-15 BFD patch breaks my project


I spent a few hours looking at this, but I don't have any more time to
devote right now.  Please see my comments below.

On Fri, 17 Oct 2003, Nick Clifton wrote:
> Aaron <AWLaFramboise@aol.com> wrote:
> > After doing some misc. upgrades, I noticed my project suddenly and
> > inexplicably failed to link.  I traced it to this patch:
> > 2003-04-15  Brian Ford  <ford@vss.fsi.com>
> >
> >         * peicode.h (coff_swap_scnhdr_in): If a section holds
> >         uninitialized data and is from an object file or from an
> >         executable image that has not initialized the s_size field, or if
> >         the physical size is padded, use the virtual size (stored in
> >         s_paddr) instead.
> >
> > Thread: http://sources.redhat.com/ml/binutils/2003-04/msg00272.html
> >
> > Specifically, one of the object files (generated by a Microsoft
> > compiler) contains a .text section with a VirtualSize 0x40 but a
> > SizeOfRawData 0x676.  There are relocations into this section with
> > addresses >0x40.  Since this patch causes _raw_size to be set to
> > VirtualSize instead of SizeOfRawData, BFD generates an error at
> > relocation time and the link fails:
> > ld: myobj.obj: bad reloc address 0x50 in section `.text'
> > ld: final link failed: Invalid operation
> >
> > In all honesty, I do not know why the MS compiler generates this
> > kind of object.  There is nothing about it in "Microsoft Portable
> > Executable and Common Object File Format Specification Revision 6.0
> > - February 1999," at any rate.  (It does not mention the case where
> > SizeOfRawData > VirtualSize.)
> >
In fact, it does, but only in the case of FileAlignment rounding for
executables.

| 4. Section Table (Section Headers)
|  VirtualSize
|    Total size of the section when loaded into memory.
|    If this value is greater than Size of Raw Data, the
|    section is zero-padded. This field is valid only for
|    executable images and should be set to 0 for object
|    files.
|
|  SizeOfRawData
|    Size of the section (object file) or size of the
|    initialized data on disk (image files). For executable
|    image, this must be a multiple of FileAlignment from
|    the optional header. If this is less than VirtualSize
|    the remainder of the section is zero filled. Because
|    this field is rounded while the VirtualSize field is not
|    it is possible for this to be greater than VirtualSize as
|    well. When a section contains only uninitialized data,
|    this field should be 0.
|

So, that clearly states that object files "should" have a value of 0
there.  Leave it to Microsoft to violate their own specs. ;)

I guess it also clearly states that this field is just plain invalid for
object files, but I don't know how to tell which is which in bfd.
Suggestions please?

> > However, since this object is part of a library in a Microsoft SDK,
> > which is probably produced by a new version of Microsoft's compiler,
> > it seems reasonable to me that the 2003-04-15 patch should be
> > altered or reversed.
> >
Altered: agreed.

> > In any case, lieing about the actual size of the section to all of
> > BFD does not seem like a good thing, and may cause problems in
> > addition to those that I am having.  Perhaps a better way to
> > implement Brian Ford's patch might be to compare these two section
> > sizes locally to the parts of BFD that might take advantage from
> > doing so, eg when doing section merging.
> >
It wasn't meant to lie.  In fact, it was meant to correct a lie.

Since _raw_size is supposed to represent the size of the data (notice I
said data, not section, even though it is not really clear this is the
case in the bfd.h header), this patch was meant to cover the case where
the section size (SizeOfRawData) had been rounded up because of FileAlignment
padding.  In that case, VirtualSize *is* the correct size of the data
(but apparently, only for executables).

Without this patch, we *were* lieing to bfd in that case by telling it
the padding was data.  This caused programs using bfd to try and read the
padding as data (ie. gdb when parsing DWARF 2 info, bfd when outputting
the section, etc.).  That was bad too.

The padding will be reallocated when/if bfd outputs the section back to
disk, and SizeofRawData will then be adjusted accordingly.

> > Any comments?  If this patch is not critical to maintaining
> > functionality somewhere, I'd recommend it be reversed until a better
> > solution is found.
>
I think a better solution should be relatively easy to find.

> Well Brian was trying to fix a real problem:
>
>   http://sources.redhat.com/ml/binutils/2003-04/msg00240.html
>
> So just reverting the patch will probably not help.
>
Unfortunately, that is *really* still somewhat of a fictitious problem
since I have yet to finish my DWARF 2 support for Cygwin.

Sorry for the tangent here, but:
<TANGENT>The section relative relocation (IMAGE_REL_I386_SECREL) in bfd,
and the pseudo op syntax to generate such in gas, has me stuck.  I am
sure this is a simple thing for someone who knows what they are doing in
binutils.  (AFAIK, relocations are stored section relative in bfd anyway.)

Incidentally, I was under the impression that this relocation was also
needed to support thread local storage (TLS).  From the PE spec: "This is
used to support debugging information as well as static thread local
storage."  I guess Jonathan Wilson hasn't run into the need yet?  Maybe
we could get together on this one.

It is becoming a "hot topic" here again, so maybe I'll get back to it
soon.  Anyone else want to help? :D </TANGENT>

That said, I'd really prefer not to loose the intent of the patch.

> Which target are you using ?  You may find that defining
> COFF_NO_HACK_SCNHDR_SIZE in a similar way to bfd/pe-mips.c will help
> your target.
>
I am almost sure he is on pei-i386, so this would defeat the purpose, I
think.

> Alternatively what may be needed is a better heuristic to determine
> why the raw size has been set to larger than the virtual size.
> Brian's patch was trying to overcome issues with padding and section
> alignment.  Maybe the code could check to see if the difference
> between two sizes does amount to an alignment padding operation.  If
> not, then the sizes could be left alone.
>
Agreed.  How about simply "only remove the padding when the bfd
represents an executable"?

I just don't know a good way to tell that right now.  I thought that
s_paddr (VirtualSize) != 0 was a way, but obviously not.  Maybe use
something like IMAGE_FILE_EXECUTABLE_IMAGE in the filehdr.f_flags, or
sectionhdr.reloc_done?  I'm not sure the former is easy to get to there,
and I'm not sure if either are reliable indicators.

I think this would still preserve the spirit of my patch and solve my
particular problem, but I don't have time to test a patch right now.  I'll
put it on my list to get to soon if no one beats me to it.

-- 
Brian Ford
Senior Realtime Software Engineer
VITAL - Visual Simulation Systems
FlightSafety International
Phone: 314-551-8460
Fax:   314-551-8444


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]