2010-01-01 01:26:50

by Jun Sun

[permalink] [raw]

2010-01-01 06:23:54

by Mike Frysinger

[permalink] [raw]
Subject: Re: [uClinux-dev] [PATCH] Valid relocation symbol for FLAT format on ARM, v2

On Thursday 31 December 2009 20:26:45 Jun Sun wrote:

when posting updates, you should put the "v2" and such inside of the "[patch]"
marker like so: [PATCH v2]. git will remove this automatically when applying.

> #define flat_old_ram_flag(flags) (flags)
> -#define flat_reloc_valid(reloc, size) ((reloc) <= (size))
> +#define flat_reloc_valid(reloc, size) ((reloc) <= (size) + 0x1000)
> +/* [jsun] new gcc 4.x generates ANCHOR symbols in order to reduce the size

comments go above the thing they're documenting, not below. the "[jsun]" is
also not really useful -- no one who reads this after the fact will have any
idea what "[jsun]" means.
-mike


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part.

2010-01-02 23:07:12

by Jamie Lokier

[permalink] [raw]
Subject: Re: [uClinux-dev] [PATCH] Valid relocation symbol for FLAT format on ARM, v2

Mike Frysinger wrote:
> when posting updates, you should put the "v2" and such inside of the
> "[patch]" marker like so: [PATCH v2]. git will remove this
> automatically when applying.

Thanks; I didn't know this.

> On Thursday 31 December 2009 20:26:45 Jun Sun wrote:
> > #define flat_old_ram_flag(flags) (flags)
> > -#define flat_reloc_valid(reloc, size) ((reloc) <= (size))
> > +#define flat_reloc_valid(reloc, size) ((reloc) <= (size) + 0x1000)
> > +/* [jsun] new gcc 4.x generates ANCHOR symbols in order to reduce the size
>
> comments go above the thing they're documenting, not below. the "[jsun]" is
> also not really useful -- no one who reads this after the fact will have any
> idea what "[jsun]" means.

I agree with everything Mike said. Put the comment before the
#define, and lose the "[jsun]". If someone wants to know who wrote
the comment they can use "git blame" and get your full email address.

Actually I'd go a bit further: I think it's be tidier to put the
comment and #define in their own block, separated by a blank line from
the other #defines.

I don't see how GCC can make the GOT smaller with relocs pointing
_after_ the end of the BSS (middle yes, but after?), but given that it
does, the patch looks good to me.

-- Jamie

2010-01-02 23:09:09

by Jamie Lokier

[permalink] [raw]
Subject: Re: [uClinux-dev] [PATCH] Valid relocation symbol for FLAT format on ARM, v2

Jun Sun wrote:
> +/* [jsun] new gcc 4.x generates ANCHOR symbols in order to reduce the size
> + * of GOT table for PIC code. It is possible the ANCHOR is placed beyond
> + * the end of data/bss segment up to 4K bytes(12 bits), because ARM allows
> + * negative 12-bit offset. Thus we allow 0x1000 extra in reloc address range.
> + */

Can you provide a small test program and *exact* GCC version and
Binutils version which triggers this? People have reported using GCC
4.x on ARM uClinux for some time, and I haven't seen complaints
about this problem before.

Thanks,
-- Jamie

2010-01-04 18:58:45

by Jun Sun

[permalink] [raw]
Subject: Re: [uClinux-dev] [PATCH] Valid relocation symbol for FLAT format on ARM, v2

On Sat, Jan 02, 2010 at 11:09:07PM +0000, Jamie Lokier wrote:
> Jun Sun wrote:
> > +/* [jsun] new gcc 4.x generates ANCHOR symbols in order to reduce the size
> > + * of GOT table for PIC code. It is possible the ANCHOR is placed beyond
> > + * the end of data/bss segment up to 4K bytes(12 bits), because ARM allows
> > + * negative 12-bit offset. Thus we allow 0x1000 extra in reloc address range.
> > + */
>
> Can you provide a small test program and *exact* GCC version and
> Binutils version which triggers this? People have reported using GCC
> 4.x on ARM uClinux for some time, and I haven't seen complaints
> about this problem before.
>

gcc is derived from code saucery's 2008Q2 release, I believe. It is
gcc 4.3.3. So there is minor chance that this ANCHOR feature
is a code sourcery feature rather than a generic gcc 4.3.3. Please help
verify.

I attached the sample code and the assembly output. The command line used
to generate this is:

arm-uclinuxeabi-gcc -c -Os -g -fomit-frame-pointer -pipe -msoft-float -fno-common -fno-builtin -Wall -DEMBED -D__PIC__ -fpic -msingle-pic-base -Dlinux -D__linux__ -Dunix -D__uClinux__ -S helloworld.c

You can see from the assembly output that .LANCHOR1 is set to 8184, well
beyond the real end of bss/data segment. And it is easy to see why, because
variable x2 is then referred as a negative offset to LANCHOR1.

Cheers.

Jun


Attachments:
(No filename) (1.37 kB)
helloworld.c (279.00 B)
helloworld.s (6.88 kB)
Download all attachments