On 8 June 2012 15:20, Hans-Peter Nilsson <hans-peter.nils...@axis.com> wrote:
>> From: Michael Hope <michael.h...@linaro.org>
>> Date: Fri, 8 Jun 2012 04:42:30 +0200
>
>> On 8 June 2012 12:15, Hans-Peter Nilsson <hans-peter.nils...@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
