Re: Bogus code in coffgen.c?

[Ian Lance Taylor <ian at airs dot com>]

"H . J . Lu" <hjl@lucon.org> writes:

> In coffgen.c, there is
> 
>     290   if (internal_f.f_opthdr)
>     291     {
>     292       PTR opthdr;
>     293 
>     294       opthdr = bfd_alloc (abfd, aoutsz);
> 					^^^^^^^^
>     295       if (opthdr == NULL)
>     296         return 0;;
>     297       if (bfd_bread (opthdr, (bfd_size_type) internal_f.f_opthdr, abfd)
> 						     ^^^^^^^^^^^^^^^^^^^
>     298           != internal_f.f_opthdr)
>     299         {
>     300           return 0;
>     301         }
>     302       bfd_coff_swap_aouthdr_in (abfd, opthdr, (PTR) &internal_a);
>     303     }
>     304 
> 
> We allocate a buffer of `aoutsz' bytes. Then we read `internal_f.f_opthdr'
> bytes into it. Why?

For a normal COFF target, f_opthdr should be either 0 or aoutsz.
XCOFF is an irritating exception: XCOFF defines a large and a small
aout header (I believe the small header is used for an object file
while the large header is used for an executable), so for XCOFF you
have to pay attention to f_opthdr, and not read more than that.  But
you still want to allocate aoutsz bytes.  because that is what
swap_aouthdr_in and friends expect, even for a small XCOFF header.

The code does the right thing for a correct object, but it's obviously
risky for a bad object.  I think your proposed patch is appropriate;
we may want to consider a call bfd_error_handler describing the
problem.

Ian