[RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Hans-Peter Nilsson]

(CC to ARM maintainer approving the original patch.)

I'm listing this under "caveats" rather than "improvements" and
before the current top ARM-related caveat (as this one is more
important :) because I don't see performance figures in the
context of the original patch (r178852) backing up this as an
improvement, and it caused sort-of a wild goose hunt for
applications causing unaligned accesses, until it was found to be
deliberately emitted by gcc-4.7 when configured for
arm-unknown-linux-gnueabi and passing "-marmv6".  Hence caveat.
Nomenclature taken from that patch; if prefer a different
spelling of the ARM variants, please tell how.  The "some source
codes" was in the analyzed case a strcpy of a five-byte string
(busybox/libbb/procps.c:365 'strcpy(filename_tail, "stat");' of
some unknown busybox-version).

An OS configured with unaligned accesses turned off (IIUC the
default for Linux/ARM), has the nice property that you easily
spot a certain class of bad codes.

Ok to commit?

Alternatively and IMHO preferably, there's reason enough to
revert the (new) default, including the gcc-4.7 branch.

Index: changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
retrieving revision 1.113
diff -p -u -r1.113 changes.html
--- changes.html	5 Jun 2012 11:03:53 -0000	1.113
+++ changes.html	8 Jun 2012 00:01:09 -0000
@@ -43,6 +43,16 @@
 
     </li>
 
+    <li>On ARM, when compiling for ARMv6 (but not ARMv6-M), ARMv7-A,
+    ARMv7-R, or ARMv7-M, the default of the new option
+    <code>-munaligned-accesses</code> is on, which for some source
+    codes generates code that accesses memory on unaligned adresses.
+    This will require the OS of those systems to enable such accesses
+    (controlled by CP15 register c1, refer to ARM documentation).
+    Alternatively or for compatibility with OS versions that do not
+    enable unaligned accesses, all codes has to be compiled with
+    <code>-mno-unaligned-accesses</code>.</li>
+
     <li>Support on ARM for the legacy floating-point accelerator (FPA) and
     the mixed-endian floating-point format that it used has been obsoleted.
     The ports that still use this format have been obsoleted as well.

brgds, H-P

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Michael Hope]

On 8 June 2012 12:15, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote:
> (CC to ARM maintainer approving the original patch.)
>
> I'm listing this under "caveats" rather than "improvements" and
> before the current top ARM-related caveat (as this one is more
> important :) because I don't see performance figures in the
> context of the original patch (r178852) backing up this as an
> improvement, and it caused sort-of a wild goose hunt for
> applications causing unaligned accesses, until it was found to be
> deliberately emitted by gcc-4.7 when configured for
> arm-unknown-linux-gnueabi and passing "-marmv6".  Hence caveat.
> Nomenclature taken from that patch; if prefer a different
> spelling of the ARM variants, please tell how.  The "some source
> codes" was in the analyzed case a strcpy of a five-byte string
> (busybox/libbb/procps.c:365 'strcpy(filename_tail, "stat");' of
> some unknown busybox-version).
>
> An OS configured with unaligned accesses turned off (IIUC the
> default for Linux/ARM), has the nice property that you easily
> spot a certain class of bad codes.
>
> Ok to commit?

The wording is scarier than I'd like.  Userspace and the Linux kernel
post 3.1 are fine.  Earlier kernels fail to start due to the kernel
turning off the hardware support and an interaction of
-fconserve-stack and -munaligned-access cause a char array to be
unaligned on the stack.

I don't agree that unaligned access is a sign of wrong code.  It's
very common when poking about in protocol buffers.  Using the hardware
support is better than the multi-byte read plus OR that GCC used to
do.

Where else have you seen this?

> Alternatively and IMHO preferably, there's reason enough to
> revert the (new) default, including the gcc-4.7 branch.
>
> Index: changes.html
> ===================================================================
> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
> retrieving revision 1.113
> diff -p -u -r1.113 changes.html
> --- changes.html        5 Jun 2012 11:03:53 -0000       1.113
> +++ changes.html        8 Jun 2012 00:01:09 -0000
> @@ -43,6 +43,16 @@
>
>     </li>
>
> +    <li>On ARM, when compiling for ARMv6 (but not ARMv6-M), ARMv7-A,
> +    ARMv7-R, or ARMv7-M

How about "ARMv6K and above"? or "ARMv6 and above, except for ARMv6-M"

> the default of the new option
> +    <code>-munaligned-accesses</code> is on, which for some source
> +    codes generates code that accesses memory on unaligned adresses.
> +    This will require the OS of those systems to enable such accesses
> +    (controlled by CP15 register c1, refer to ARM documentation).

The CPU has an unaligned access trap which is off by default.  The
kernel has to turn the trap on to cause the fault.

> +    Alternatively or for compatibility with OS versions that do not
> +    enable unaligned accesses, all codes has to be compiled with
> +    <code>-mno-unaligned-accesses</code>.</li>
> +
>     <li>Support on ARM for the legacy floating-point accelerator (FPA) and
>     the mixed-endian floating-point format that it used has been obsoleted.
>     The ports that still use this format have been obsoleted as well.
>
> brgds, H-P

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Hans-Peter Nilsson]

> From: Michael Hope <michael.hope@linaro.org>
> Date: Fri, 8 Jun 2012 04:42:30 +0200

> On 8 June 2012 12:15, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote:
> > The "some source
> > codes" was in the analyzed case a strcpy of a five-byte string
> > (busybox/libbb/procps.c:365 'strcpy(filename_tail, "stat");' of
> > some unknown busybox-version).
> >
> > An OS configured with unaligned accesses turned off (IIUC the
> > default for Linux/ARM), has the nice property that you easily
> > spot a certain class of bad codes.
> >
> > Ok to commit?
> 
> The wording is scarier than I'd like.  Userspace and the Linux kernel
> post 3.1 are fine.

So the default for ALIGNMENT_TRAP changed in >3.1?

>  Earlier kernels fail to start due to the kernel
> turning off the hardware support and an interaction of
> -fconserve-stack and -munaligned-access cause a char array to be
> unaligned on the stack.
> 
> I don't agree that unaligned access is a sign of wrong code.  It's
> very common when poking about in protocol buffers.  Using the hardware
> support is better than the multi-byte read plus OR that GCC used to
> do.

Totally disagree.  Code that e.g. casts unaligned char * to int *
and dereferences that, is crappy and unportable and fails
without OS-configury choice on some platforms, and is also
avoidable.  But hopefully that wasn't what you meant.

> Where else have you seen this?

I don't get the "else".  I've seen the unaligned accesses from
userland code as quoted above.  There were other (userland)
places in that build-tree that I'm not going to check, as this
was (again) GCC-generated code.  There were no other misaligned
accesses on that system; not from the kernel, not from userspace.

> > +    <li>On ARM, when compiling for ARMv6 (but not ARMv6-M), ARMv7-A,
> > +    ARMv7-R, or ARMv7-M
> 
> How about "ARMv6K and above"? or "ARMv6 and above, except for ARMv6-M"

I'll let an ARM maintainer sort out the names and color for the
toolshed; ARMv6K isn't an obvious improvement to me over the
names used in the patch introducing -munaligned-access.

> > the default of the new option
> > +    <code>-munaligned-accesses</code> is on, which for some source
> > +    codes generates code that accesses memory on unaligned adresses.
> > +    This will require the OS of those systems to enable such accesses
> > +    (controlled by CP15 register c1, refer to ARM documentation).
> 
> The CPU has an unaligned access trap which is off by default.  The
> kernel has to turn the trap on to cause the fault.

In arch/arm/Kconfig it looks for all purposes default "on" to me
(config ALIGNMENT_TRAP, 2.6.35): "default y if !ARCH_EBSA110"
where ARCH_EBSA110 seems to be for some "evaluation board from Digital".

brgds, H-P

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Michael Hope]

On 8 June 2012 15:20, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote:
>> From: Michael Hope <michael.hope@linaro.org>
>> Date: Fri, 8 Jun 2012 04:42:30 +0200
>
>> On 8 June 2012 12:15, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote:
>> > The "some source
>> > codes" was in the analyzed case a strcpy of a five-byte string
>> > (busybox/libbb/procps.c:365 'strcpy(filename_tail, "stat");' of
>> > some unknown busybox-version).
>> >
>> > An OS configured with unaligned accesses turned off (IIUC the
>> > default for Linux/ARM), has the nice property that you easily
>> > spot a certain class of bad codes.
>> >
>> > Ok to commit?
>>
>> The wording is scarier than I'd like.  Userspace and the Linux kernel
>> post 3.1 are fine.
>
> So the default for ALIGNMENT_TRAP changed in >3.1?

ALIGNMENT_TRAP is on by default but the early boot time trap is now
conditional on the architecture level:

__enable_mmu:
#if defined(CONFIG_ALIGNMENT_TRAP) && __LINUX_ARM_ARCH__ < 6
	orr	r0, r0, #CR_A
#else
	bic	r0, r0, #CR_A
#endif

>>  Earlier kernels fail to start due to the kernel
>> turning off the hardware support and an interaction of
>> -fconserve-stack and -munaligned-access cause a char array to be
>> unaligned on the stack.
>>
>> I don't agree that unaligned access is a sign of wrong code.  It's
>> very common when poking about in protocol buffers.  Using the hardware
>> support is better than the multi-byte read plus OR that GCC used to
>> do.
>
> Totally disagree.  Code that e.g. casts unaligned char * to int *
> and dereferences that, is crappy and unportable and fails
> without OS-configury choice on some platforms, and is also
> avoidable.  But hopefully that wasn't what you meant.

I mean direct access via helpers:

inline u16 get_u16(const char *p) {
#ifdef __ARM_FEATURE_UNALIGNED
  return ntohs(*(const u16 *)p);
#else
...
}
...
  u16 checksum = get_u16(buffer + CHECKSUM_OFFSET);

>> Where else have you seen this?
>
> I don't get the "else".  I've seen the unaligned accesses from
> userland code as quoted above.  There were other (userland)
> places in that build-tree that I'm not going to check, as this
> was (again) GCC-generated code.  There were no other misaligned
> accesses on that system; not from the kernel, not from userspace.

You're suggesting we disable an optimisation.  The combination of
older Linux ARM kernels and GCC 4.7 gives a faulty kernel.  In all
other cases it should be an improvement.  Have you seen other
combinations where there's a fault when this feature is turned on?

-- Michael

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Hans-Peter Nilsson]

> From: Michael Hope <michael.hope@linaro.org>
> Date: Fri, 8 Jun 2012 05:50:52 +0200

> On 8 June 2012 15:20, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote:
> > So the default for ALIGNMENT_TRAP changed in >3.1?
> 
> ALIGNMENT_TRAP is on by default but the early boot time trap is now
> conditional on the architecture level:
> 
> __enable_mmu:
> #if defined(CONFIG_ALIGNMENT_TRAP) && __LINUX_ARM_ARCH__ < 6
> 	orr	r0, r0, #CR_A
> #else
> 	bic	r0, r0, #CR_A
> #endif

Ah, interesting.

> >> I don't agree that unaligned access is a sign of wrong code.  It's
> >> very common when poking about in protocol buffers.  Using the hardware
> >> support is better than the multi-byte read plus OR that GCC used to
> >> do.
> >
> > Totally disagree.  Code that e.g. casts unaligned char * to int *
> > and dereferences that, is crappy and unportable and fails
> > without OS-configury choice on some platforms, and is also
> > avoidable.  But hopefully that wasn't what you meant.
> 
> I mean direct access via helpers:
> 
> inline u16 get_u16(const char *p) {
> #ifdef __ARM_FEATURE_UNALIGNED
>   return ntohs(*(const u16 *)p);
> #else
> ...
> }
> ...
>   u16 checksum = get_u16(buffer + CHECKSUM_OFFSET);

*shrug* I see no relation to the "certain classes of bad codes"
I allude to; i.e. brute unaligned accesses without portability
checks.  The above code IIUC is in Linux, that also has control
over, and enabled the unaligned access mode in the code you
quoted, so it certainly knows it's safe!

> >> Where else have you seen this?
> >
> > I don't get the "else".  I've seen the unaligned accesses from
> > userland code as quoted above.  There were other (userland)
> > places in that build-tree that I'm not going to check, as this
> > was (again) GCC-generated code.  There were no other misaligned
> > accesses on that system; not from the kernel, not from userspace.
> 
> You're suggesting we disable an optimisation.

Tongue-in-cheek: I'm not really sure it is an "optimization";
for that claim we still have to see numbers. ;)

>  The combination of
> older Linux ARM kernels and GCC 4.7 gives a faulty kernel.

We're in agreement!

>  In all
> other cases it should be an improvement.  Have you seen other
> combinations where there's a fault when this feature is turned on?

Nope, I think this one is bad enough.

It doesn't seem far-fetched that other non-Linux-kernels have
the trap on (maybe even knowingly and deliberately, for debug
reasons) and whose users are surprised by the new behavior.
Maybe they want to change their kernels, maybe not.  With the
updated changes.html they can at least see that this is expected
behavior without digging in too deeply.  The decision to
document it (as per the patch) or revert it (my humble
preference) might well be helped by performance numbers.

You're implying this change is an optimization that helps more
than it hurts; have you seen any such numbers?  1/2 :-)

brgds, H-P

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Fri, 8 Jun 2012 06:29:04 +0200

> > From: Michael Hope <michael.hope@linaro.org>
> > Date: Fri, 8 Jun 2012 05:50:52 +0200
> >  The combination of
> > older Linux ARM kernels and GCC 4.7 gives a faulty kernel.
> 
> We're in agreement!

Oh wait sorry, my bad, I misread.  Instead of "gives a faulty
kernel", I'd say "for ARMv6 and later (not -M), gives faulty
user-space code".  Maybe the kernel too, I can't say; there was
IIRC no sign of it.

brgds, H-P

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Michael Hope]

On 8 June 2012 16:53, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote:
>> From: Hans-Peter Nilsson <hp@axis.com>
>> Date: Fri, 8 Jun 2012 06:29:04 +0200
>
>> > From: Michael Hope <michael.hope@linaro.org>
>> > Date: Fri, 8 Jun 2012 05:50:52 +0200
>> >  The combination of
>> > older Linux ARM kernels and GCC 4.7 gives a faulty kernel.
>>
>> We're in agreement!
>
> Oh wait sorry, my bad, I misread.  Instead of "gives a faulty
> kernel", I'd say "for ARMv6 and later (not -M), gives faulty
> user-space code".  Maybe the kernel too, I can't say; there was
> IIRC no sign of it.

Is there a bugzilla ticket logged for this?  I'd like to try to reproduce it.

It's interesting as we backported the patch into the Linaro GCC that
was used to build Ubuntu Precise and didn't find any faults.

-- Michael

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Gerald Pfeifer]

This is only a review wearing my web hat; it is orthogonal to the
discussion with the ARM guys. ;-)

On Fri, 8 Jun 2012, Hans-Peter Nilsson wrote:
> +    <li>On ARM, when compiling for ARMv6 (but not ARMv6-M), ARMv7-A,
> +    ARMv7-R, or ARMv7-M, the default of the new option
> +    <code>-munaligned-accesses</code> is on, which for some source

How about "the new option...is active by default"?

> +    This will require the OS of those systems to enable such accesses

Omit "of those systems" and spell out "operating system"?

> +    (controlled by CP15 register c1, refer to ARM documentation).

<code>c1</cdoe>

> +    Alternatively or for compatibility with OS versions that do not
> +    enable unaligned accesses, all codes has to be compiled with

"enable" -> "support"

"all code has"

Gerald

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Hans-Peter Nilsson]

> From: Michael Hope <michael.hope@linaro.org>
> Date: Mon, 11 Jun 2012 00:04:19 +0200

> On 8 June 2012 16:53, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote:
> >> From: Hans-Peter Nilsson <hp@axis.com>
> >> Date: Fri, 8 Jun 2012 06:29:04 +0200
> >
> >> > From: Michael Hope <michael.hope@linaro.org>
> >> > Date: Fri, 8 Jun 2012 05:50:52 +0200
> >> >  The combination of
> >> > older Linux ARM kernels and GCC 4.7 gives a faulty kernel.
> >>
> >> We're in agreement!
> >
> > Oh wait sorry, my bad, I misread.  Instead of "gives a faulty
> > kernel", I'd say "for ARMv6 and later (not -M), gives faulty
> > user-space code".  Maybe the kernel too, I can't say; there was
> > IIRC no sign of it.
> 
> Is there a bugzilla ticket logged for this?  I'd like to try to reproduce it.

No, I think it was clear that it was a deliberate change.  I'm
waiting for an ARM maintainer to review the gcc-4.7/changes.html
patch or reconsider the mentioned change.

> It's interesting as we backported the patch into the Linaro GCC that
> was used to build Ubuntu Precise and didn't find any faults.

You don't think unaligned accesses are generated or what's your
point?   Did you use -marmv6 when compiling the user code?

As already mentioned, for example anything that executes
busybox/libbb/procps.c:365 (procps_scan), but that's just one
random location.  I think you'd have seen it assuming your
kernel was configured as mentioned and your usercode was
configured for armv6 or higher (or gcc configured to default
generate for armv6 or higher).

brgds, H-P

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Hans-Peter Nilsson]

> From: Gerald Pfeifer <gerald@pfeifer.com>
> Date: Mon, 11 Jun 2012 00:27:21 +0200

> This is only a review wearing my web hat; it is orthogonal to the
> discussion with the ARM guys. ;-)
> 
> On Fri, 8 Jun 2012, Hans-Peter Nilsson wrote:
> > +    <li>On ARM, when compiling for ARMv6 (but not ARMv6-M), ARMv7-A,
> > +    ARMv7-R, or ARMv7-M, the default of the new option
> > +    <code>-munaligned-accesses</code> is on, which for some source
> 
> How about "the new option...is active by default"?
> 
> > +    This will require the OS of those systems to enable such accesses
> 
> Omit "of those systems" and spell out "operating system"?

I see the tautology to which you allude. 8]  Maybe "the kernel of
those systems" is better?

> > +    (controlled by CP15 register c1, refer to ARM documentation).
> 
> <code>c1</cdoe>
> 
> > +    Alternatively or for compatibility with OS versions that do not
> > +    enable unaligned accesses, all codes has to be compiled with
> 
> "enable" -> "support"

I wanted to emphasize that it's something you can configure to
be enabled or disabled (with the kernel catching an exception),
but maybe that doesn't really matter.  Clarity matters and that
part doesn't really have to explain in detail.

> "all code has"

I thought "codes" was a valid common reference to source code,
but I guess not.

Good suggestions, I hope I got them all below in one form or
another:

Index: changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
retrieving revision 1.113
diff -p -u -r1.113 changes.html
--- changes.html	5 Jun 2012 11:03:53 -0000	1.113
+++ changes.html	10 Jun 2012 23:12:02 -0000
@@ -43,6 +43,16 @@
 
     </li>
 
+    <li>On ARM, when compiling for ARMv6 (but not ARMv6-M), ARMv7-A,
+    ARMv7-R, or ARMv7-M, the new option
+    <code>-munaligned-accesses</code> is active by default, which for
+    some source codes generates code that accesses memory on unaligned
+    adresses.  This will require the kernel of those systems to enable
+    such accesses (controlled by CP15 register <code>c1</code>, refer
+    to ARM documentation).  Alternatively or for compatibility with
+    kernels where unaligned accesses are not supported, all code has
+    to be compiled with <code>-mno-unaligned-accesses</code>.</li>
+
     <li>Support on ARM for the legacy floating-point accelerator (FPA) and
     the mixed-endian floating-point format that it used has been obsoleted.
     The ports that still use this format have been obsoleted as well.

brgds, H-P

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Mon, 11 Jun 2012 00:59:57 +0200

> > From: Michael Hope <michael.hope@linaro.org>
> > Date: Mon, 11 Jun 2012 00:04:19 +0200
> 
> > On 8 June 2012 16:53, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote:
> > >> From: Hans-Peter Nilsson <hp@axis.com>
> > >> Date: Fri, 8 Jun 2012 06:29:04 +0200
> > >
> > >> > From: Michael Hope <michael.hope@linaro.org>
> > >> > Date: Fri, 8 Jun 2012 05:50:52 +0200
> > >> >  The combination of
> > >> > older Linux ARM kernels and GCC 4.7 gives a faulty kernel.
> > >>
> > >> We're in agreement!
> > >
> > > Oh wait sorry, my bad, I misread.  Instead of "gives a faulty
> > > kernel", I'd say "for ARMv6 and later (not -M), gives faulty
> > > user-space code".  Maybe the kernel too, I can't say; there was
> > > IIRC no sign of it.

But (at least) after removing some local changed defaults,
there's at boot-time a lot of:

[    0.950000] Unhandled fault: alignment exception (0x801) at 0xc821ddee

> > Is there a bugzilla ticket logged for this?  I'd like to try to reproduce it.

Here's a shorter case I'll attach to a PR for this unless it
gets resolved one way or another soonish.  Remember, you'll have
to run this on a pre-3.2 kernel with CONFIG_ALIGNMENT_TRAP on
(the default) and you have to compile for ARM v6 or later (as in
-march=armv6).  Using gcc-4.7.1-rc1 should do, most likely
earlier revisions too.

__attribute__ ((__noinline__, __noclone__))
void doit(char *x)
{
  asm ("");
  __builtin_strcpy (x, "stat");
}

int main(void)
{
  char x[30];
  doit(x + 1);
  doit(x);
  __builtin_exit (0);
}

> > It's interesting as we backported the patch into the Linaro GCC that
> > was used to build Ubuntu Precise and didn't find any faults.

I have no idea why you didn't run into this, unless it was one
of the obvious reasons: not building for ARM v6 or the kernel
was 3.2 or later, or configured with CONFIG_ALIGNMENT_TRAP off.
Or other local patches of yours.

brgds, H-P

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Michael Hope]

On 13 June 2012 02:32, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote:
>> From: Hans-Peter Nilsson <hp@axis.com>
>> Date: Mon, 11 Jun 2012 00:59:57 +0200
>
>> > From: Michael Hope <michael.hope@linaro.org>
>> > Date: Mon, 11 Jun 2012 00:04:19 +0200
>>
>> > On 8 June 2012 16:53, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote:
>> > >> From: Hans-Peter Nilsson <hp@axis.com>
>> > >> Date: Fri, 8 Jun 2012 06:29:04 +0200
>> > >
>> > >> > From: Michael Hope <michael.hope@linaro.org>
>> > >> > Date: Fri, 8 Jun 2012 05:50:52 +0200
>> > >> >  The combination of
>> > >> > older Linux ARM kernels and GCC 4.7 gives a faulty kernel.
>> > >>
>> > >> We're in agreement!
>> > >
>> > > Oh wait sorry, my bad, I misread.  Instead of "gives a faulty
>> > > kernel", I'd say "for ARMv6 and later (not -M), gives faulty
>> > > user-space code".  Maybe the kernel too, I can't say; there was
>> > > IIRC no sign of it.
>
> But (at least) after removing some local changed defaults,
> there's at boot-time a lot of:
>
> [    0.950000] Unhandled fault: alignment exception (0x801) at 0xc821ddee

That's a kernel address.  What does /proc/kallsyms say is there?

For reference, the message comes from
arch/arm/mm/alignment.c:alignment_init() from the default trap
handler.  The lines just before this disable the unaligned trap for
usermode:

	if (cpu_is_v6_unaligned()) {
		cr_alignment &= ~CR_A;
		cr_no_alignment &= ~CR_A;
		set_cr(cr_alignment);
		ai_usermode = safe_usermode(ai_usermode, false);
	}

Support was added by Russell King in 2008-12 and updated by Dave
Martin on 2011-07.

Out of interest, does your CPU report support for unaligned access via
CP15 CR1?  It's bit 22 and shows during boot.  My board shows:

CPU: ARMv7 Processor [411fc092] revision 2 (ARMv7), cr=50c5387d

>> > Is there a bugzilla ticket logged for this?  I'd like to try to reproduce it.
>
> Here's a shorter case I'll attach to a PR for this unless it
> gets resolved one way or another soonish.  Remember, you'll have
> to run this on a pre-3.2 kernel with CONFIG_ALIGNMENT_TRAP on
> (the default) and you have to compile for ARM v6 or later (as in
> -march=armv6).  Using gcc-4.7.1-rc1 should do, most likely
> earlier revisions too.
>
> __attribute__ ((__noinline__, __noclone__))
> void doit(char *x)
> {
>  asm ("");
>  __builtin_strcpy (x, "stat");
> }
>
> int main(void)
> {
>  char x[30];
>  doit(x + 1);
>  doit(x);
>  __builtin_exit (0);
> }

This compiles into a five byte unaligned memcpy:

doit:
	mov	r2, r0
	movw	r3, #:lower16:.LC0
	movt	r3, #:upper16:.LC0
	ldr	r0, [r3, #0]	@ unaligned
	ldrb	r3, [r3, #4]	@ zero_extendqisi2
	str	r0, [r2, #0]	@ unaligned
	strb	r3, [r2, #4]
	bx	lr

which is correct.  The test case runs on my boards and kernels as
noted below.  /proc/cpu/alignment doesn't change so the loads and
stores were handled by the hardware.

I added:

__attribute__ ((__noinline__, __noclone__))
long long doit2(char *x)
{
 asm ("");
 return *(long long *)x;
}

which becomes:

doit2:
	ldmia	r0, {r0, r1}
	bx	lr

ldm must be aligned.  The program runs to completion but this time the
kernel traps and handles the unaligned load:

cbuild@ursa1:~/bugs$ cat /proc/cpu/alignment  > before
cbuild@ursa1:~/bugs$ ./a.out
cbuild@ursa1:~/bugs$ cat /proc/cpu/alignment  > after
cbuild@ursa1:~/bugs$ diff -u before after
--- before	2012-06-12 22:29:20.428268001 +0000
+++ after	2012-06-12 22:29:26.107955560 +0000
@@ -1,8 +1,8 @@
-User:		3
+User:		4
 System:		7
 Skipped:	0
 Half:		0
 Word:		0
 DWord:		0
-Multi:		10
+Multi:		11
 User faults:	2 (fixup)

>> > It's interesting as we backported the patch into the Linaro GCC that
>> > was used to build Ubuntu Precise and didn't find any faults.
>
> I have no idea why you didn't run into this, unless it was one
> of the obvious reasons: not building for ARM v6 or the kernel
> was 3.2 or later, or configured with CONFIG_ALIGNMENT_TRAP off.
> Or other local patches of yours.

Linaro's stock configuration is -march=armv7-a -mtune=cortex-a9
-mthumb.  Ubuntu is the same.  I can't reproduce the fault on a
PandaBoard with omapzoom 2.6.35, Ubuntu 3.2.14, Ubuntu Precise 4.6.3
GCC, or plain gcc-4.7.1-RC-20120606.  The configurations for the
kernels are at:
 * http://bazaar.launchpad.net/~linaro-toolchain-dev/cbuild/hardware/view/head:/ursa/r2/config
 * http://bazaar.launchpad.net/~linaro-toolchain-dev/cbuild/hardware/view/head:/distro/precise/r1/config

and have CONFIG_ALIGNMENT_TRAP on.

-- Michael

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Hans-Peter Nilsson]

> From: Michael Hope <michael.hope@linaro.org>
> Date: Wed, 13 Jun 2012 00:43:47 +0200

> On 13 June 2012 02:32, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote:
> >> From: Hans-Peter Nilsson <hp@axis.com>
> >> Date: Mon, 11 Jun 2012 00:59:57 +0200
> >> > > user-space code".  Maybe the kernel too, I can't say; there was
> >> > > IIRC no sign of it.
> > But (at least) after removing some local changed defaults,
> > there's at boot-time a lot of:
> >
> > [    0.950000] Unhandled fault: alignment exception (0x801) at 0xc821ddee
> 
> That's a kernel address.

Yes I know, there's no mystery there.  My point was that
misalignment traps for this configuration happen at boot time
too, not just from userspace, hence correcting my earlier
statement.

> For reference, the message comes from
> arch/arm/mm/alignment.c:alignment_init() from the default trap
> handler.

No news there.

> The lines just before this disable the unaligned trap for
> usermode:
> 
> 	if (cpu_is_v6_unaligned()) {
> 		cr_alignment &= ~CR_A;
> 		cr_no_alignment &= ~CR_A;
> 		set_cr(cr_alignment);
> 		ai_usermode = safe_usermode(ai_usermode, false);
> 	}
> 
> Support was added by Russell King in 2008-12 and updated by Dave
> Martin on 2011-07.

I see it, for example in stock 2.6.35.9.

> Out of interest, does your CPU report support for unaligned access via
> CP15 CR1?  It's bit 22 and shows during boot.  My board shows:
> 
> CPU: ARMv7 Processor [411fc092] revision 2 (ARMv7), cr=50c5387d

The log that my colleague sent me contains:
[    0.000000] CPU: ARMv6-compatible processor [4117b365] revision 5 (ARMv6TEJ), cr=00c5387f

Bit 22 all set...

> This compiles into a five byte unaligned memcpy:

Yes, that was the point. :)

> Linaro's stock configuration is -march=armv7-a -mtune=cortex-a9
> -mthumb.  Ubuntu is the same.  I can't reproduce the fault on a
> PandaBoard with omapzoom 2.6.35, Ubuntu 3.2.14, Ubuntu Precise 4.6.3
> GCC, or plain gcc-4.7.1-RC-20120606.

Looking at arch/arm/mm/alignment.c:alignment_init(), I see we
have local patches always forcing a warning.  Bah.  Is your
point that having an OS kernel that traps for unaligned accesses
for ARMv6 rare enough that there's no reason whatsoever for a
release note that gcc-4.7 now emits such accesses "by itself"?

Even though this was caused by non-stock Linux, I think there's
reason enough for a caveat entry in gcc-4.7/changes.html (just
as worded; the caveat wasn't directed just at Linux, mind you),
but I retract my suggestion to change back the default.

Maybe we should add a line to the suggested entry mentioning the
first stock Linux version that automatically adjusts this, so
stock Linux-users can be calmed. ;-P  Do you agree?

Something like "Linux/ARM in official releases has automatically
and unconditionally supported unaligned accesses as emitted by
GCC due to this option being active since Linux version 2.X.Y".
I guess X is 6.  What is Y?

brgds, H-P

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Mon, 11 Jun 2012 01:16:09 +0200

> +    to be compiled with <code>-mno-unaligned-accesses</code>.</li>

Better spelled as "-mno-unaligned-access".  Bah.

brgds, H-P

[RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses, take 2

[Hans-Peter Nilsson]

Y is 28 for introduction of the quoted code in
arch/arm/mm/alignment.c, AFAICT, so how about this one, ok now?

Index: changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
retrieving revision 1.113
diff -p -u -r1.113 changes.html
--- changes.html	5 Jun 2012 11:03:53 -0000	1.113
+++ changes.html	15 Jun 2012 02:04:46 -0000
@@ -43,6 +43,19 @@
 
     </li>
 
+    <li>On ARM, when compiling for ARMv6 (but not ARMv6-M), ARMv7-A,
+    ARMv7-R, or ARMv7-M, the new option
+    <code>-munaligned-access</code> is active by default, which for
+    some source codes generates code that accesses memory on unaligned
+    adresses.  This will require the kernel of those systems to enable
+    such accesses (controlled by CP15 register <code>c1</code>, refer
+    to ARM documentation).  Alternatively or for compatibility with
+    kernels where unaligned accesses are not supported, all code has
+    to be compiled with <code>-mno-unaligned-access</code>.
+    Linux/ARM in official releases has automatically and
+    unconditionally supported unaligned accesses as emitted by GCC due
+    to this option being active since Linux version 2.6.28.</li>
+
     <li>Support on ARM for the legacy floating-point accelerator (FPA) and
     the mixed-endian floating-point format that it used has been obsoleted.
     The ports that still use this format have been obsoleted as well.

brgds, H-P

Ping: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses, take 2

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Fri, 15 Jun 2012 04:07:23 +0200

A ping.

> Y is 28 for introduction of the quoted code in
> arch/arm/mm/alignment.c, AFAICT, so how about this one, ok now?
> 
> Index: changes.html
> ===================================================================
> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
> retrieving revision 1.113
> diff -p -u -r1.113 changes.html
> --- changes.html	5 Jun 2012 11:03:53 -0000	1.113
> +++ changes.html	15 Jun 2012 02:04:46 -0000
> @@ -43,6 +43,19 @@
>  
>      </li>
>  
> +    <li>On ARM, when compiling for ARMv6 (but not ARMv6-M), ARMv7-A,
> +    ARMv7-R, or ARMv7-M, the new option
> +    <code>-munaligned-access</code> is active by default, which for
> +    some source codes generates code that accesses memory on unaligned
> +    adresses.  This will require the kernel of those systems to enable
> +    such accesses (controlled by CP15 register <code>c1</code>, refer
> +    to ARM documentation).  Alternatively or for compatibility with
> +    kernels where unaligned accesses are not supported, all code has
> +    to be compiled with <code>-mno-unaligned-access</code>.
> +    Linux/ARM in official releases has automatically and
> +    unconditionally supported unaligned accesses as emitted by GCC due
> +    to this option being active since Linux version 2.6.28.</li>
> +
>      <li>Support on ARM for the legacy floating-point accelerator (FPA) and
>      the mixed-endian floating-point format that it used has been obsoleted.
>      The ports that still use this format have been obsoleted as well.
> 

Re: Ping: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses, take 2

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Fri, 22 Jun 2012 04:24:01 +0200

> > From: Hans-Peter Nilsson <hp@axis.com>
> > Date: Fri, 15 Jun 2012 04:07:23 +0200
> 
> A ping.

And another ping, now CCing ARM maintainers,
<http://gcc.gnu.org/ml/gcc-patches/2012-06/msg00983.html>.

> > Y is 28 for introduction of the quoted code in
> > arch/arm/mm/alignment.c, AFAICT, so how about this one, ok now?
> > 
> > Index: changes.html
> > ===================================================================
> > RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
> > retrieving revision 1.113
> > diff -p -u -r1.113 changes.html
> > --- changes.html	5 Jun 2012 11:03:53 -0000	1.113
> > +++ changes.html	15 Jun 2012 02:04:46 -0000
> > @@ -43,6 +43,19 @@
> >  
> >      </li>
> >  
> > +    <li>On ARM, when compiling for ARMv6 (but not ARMv6-M), ARMv7-A,
> > +    ARMv7-R, or ARMv7-M, the new option
> > +    <code>-munaligned-access</code> is active by default, which for
> > +    some source codes generates code that accesses memory on unaligned
> > +    adresses.  This will require the kernel of those systems to enable
> > +    such accesses (controlled by CP15 register <code>c1</code>, refer
> > +    to ARM documentation).  Alternatively or for compatibility with
> > +    kernels where unaligned accesses are not supported, all code has
> > +    to be compiled with <code>-mno-unaligned-access</code>.
> > +    Linux/ARM in official releases has automatically and
> > +    unconditionally supported unaligned accesses as emitted by GCC due
> > +    to this option being active since Linux version 2.6.28.</li>
> > +
> >      <li>Support on ARM for the legacy floating-point accelerator (FPA) and
> >      the mixed-endian floating-point format that it used has been obsoleted.
> >      The ports that still use this format have been obsoleted as well.
> > 
> 

Ping again: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses, take 2

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Fri, 29 Jun 2012 15:06:44 +0200

> > From: Hans-Peter Nilsson <hp@axis.com>
> > Date: Fri, 22 Jun 2012 04:24:01 +0200
> 
> > > From: Hans-Peter Nilsson <hp@axis.com>
> > > Date: Fri, 15 Jun 2012 04:07:23 +0200
> > 
> > A ping.
> 
> And another ping, now CCing ARM maintainers,
> <http://gcc.gnu.org/ml/gcc-patches/2012-06/msg00983.html>.

And another ping...

> > > Index: changes.html
> > > ===================================================================
> > > RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
> > > retrieving revision 1.113
> > > diff -p -u -r1.113 changes.html
> > > --- changes.html	5 Jun 2012 11:03:53 -0000	1.113
> > > +++ changes.html	15 Jun 2012 02:04:46 -0000
> > > @@ -43,6 +43,19 @@
> > >  
> > >      </li>
> > >  
> > > +    <li>On ARM, when compiling for ARMv6 (but not ARMv6-M), ARMv7-A,
> > > +    ARMv7-R, or ARMv7-M, the new option
> > > +    <code>-munaligned-access</code> is active by default, which for
> > > +    some source codes generates code that accesses memory on unaligned
> > > +    adresses.  This will require the kernel of those systems to enable
> > > +    such accesses (controlled by CP15 register <code>c1</code>, refer
> > > +    to ARM documentation).  Alternatively or for compatibility with
> > > +    kernels where unaligned accesses are not supported, all code has
> > > +    to be compiled with <code>-mno-unaligned-access</code>.
> > > +    Linux/ARM in official releases has automatically and
> > > +    unconditionally supported unaligned accesses as emitted by GCC due
> > > +    to this option being active since Linux version 2.6.28.</li>
> > > +
> > >      <li>Support on ARM for the legacy floating-point accelerator (FPA) and
> > >      the mixed-endian floating-point format that it used has been obsoleted.
> > >      The ports that still use this format have been obsoleted as well.

Re: [RFA:] Caveat for ARM in gcc-4.7/changes.html: unaligned accesses, take 2

[Gerald Pfeifer]

This is a small follow-up change that I had meant to commit for
a while (ahem); done now.

Thanks for taking the time to write this up!

Gerald

Index: changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
retrieving revision 1.133
diff -u -3 -p -r1.133 changes.html
--- changes.html	3 Dec 2012 18:39:27 -0000	1.133
+++ changes.html	29 Dec 2012 17:34:01 -0000
@@ -53,15 +53,15 @@
     <li>On ARM, when compiling for ARMv6 (but not ARMv6-M), ARMv7-A,
     ARMv7-R, or ARMv7-M, the new option
     <code>-munaligned-access</code> is active by default, which for
-    some source codes generates code that accesses memory on unaligned
-    addresses.  This will require the kernel of those systems to enable
+    some sources generates code that accesses memory on unaligned
+    addresses.  This requires the kernel of those systems to enable
     such accesses (controlled by CP15 register <code>c1</code>, refer
-    to ARM documentation).  Alternatively or for compatibility with
+    to ARM documentation).  Alternatively, or for compatibility with
     kernels where unaligned accesses are not supported, all code has
     to be compiled with <code>-mno-unaligned-access</code>.
-    Linux/ARM in official releases has automatically and
+    Upstream Linux kernel releases have automatically and
     unconditionally supported unaligned accesses as emitted by GCC due
-    to this option being active, since Linux version 2.6.28.</li>
+    to this option being active since version 2.6.28.</li>
 
     <li>Support on ARM for the legacy floating-point accelerator (FPA) and
     the mixed-endian floating-point format that it used has been obsoleted.