This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: [qboosh@pld-linux.org: binutils 2.14.90.0.8 - ld crash whencross-linking PE DLL on 64-bit archs]
- From: Nick Clifton <nickc at redhat dot com>
- To: hjl at lucon dot org, qboosh at pld-linux dot org
- Cc: binutils at sources dot redhat dot com
- Date: Mon, 08 Mar 2004 10:45:34 +0000
- Subject: Re: [qboosh@pld-linux.org: binutils 2.14.90.0.8 - ld crash whencross-linking PE DLL on 64-bit archs]
- References: <20040225001543.GB16109@lucon.org>
Hi H.J. Hi Jakub,
> SEGV appears inside malloc(), after corruption of some malloc structs.
> Using ElectricFence I found that ld writes past allocated area in
> fill_edata() function (ld/pe-dll.c).
>
> edata_d is pointer to allocated edata_sz bytes, where edata_sz is
> calculated by:
>
> | /* OK, now we can allocate some memory. */
> | edata_sz = (40 /* directory */
> | + 4 * export_table_size /* addresses */
> | + 4 * count_exported_byname /* name ptrs */
> | + 2 * count_exported_byname /* ordinals */
> | + name_table_size + strlen (dll_name) + 1);
>
> but then pointers to edata areas are calculated inconsequently:
>
> | unsigned char *edirectory;
> | unsigned long *eaddresses;
> | unsigned long *enameptrs;
> | unsigned short *eordinals;
> | unsigned char *enamestr;
> [...]
> | edata_d = xmalloc (edata_sz);
> |
> | /* Note use of array pointer math here. */
> | edirectory = edata_d;
> | eaddresses = (unsigned long *) (edata_d + 40);
> | enameptrs = eaddresses + export_table_size;
> | eordinals = (unsigned short *) (enameptrs + count_exported_byname);
> | enamestr = (char *) (eordinals + count_exported_byname);
>
> These pointers are wrong if sizeof(unsigned long)!=4 or
> sizeof(unsigned short)!=2.
> On alpha-linux and amd64-linux it's the first case (long is 8-byte long).
>
> I'm attaching the patch I've used - it changes unsinged long to uint32_t
> in above pointers. It works for me on alpha-linux and amd64-linux, doesn't
> break anything on x86-linux and sparc-linux.
> I don't know what is binutils policy on <stdint.h> - is using uint32_t
> OK, or binutils has some other name for exactly 32-bit long type.
Sorry, no it does not.
> unsigned shorts above probably should be changed to uint16_t (or
> equivalent), but I didn't touch it as all archs I'm using have 16-bit
> unsigned short.
The patch is a good start, but it does not go quite far enough. The
16-bit types really should be forced to be exactly 16-bits as well.
As for using stdint.h - that is OK, provided that it is checked for by
configure and then only used if HAVE_STDINT_H is defined.
If you would care to resubmit the patch with these changes made then I
would be happy to accept it.
Cheers
Nick