Dear Albert Aribaud,
On 05.10.2012 21:15, Albert ARIBAUD wrote:
Under option -munaligned-access, gcc can perform local char or 16-bit array initializations using misaligned native accesses which will throw a data abort exception. Fix files where these array initializations were unneeded, and for files known to contain such initializations, enforce gcc option -mno-unaligned-access.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
V2: fix incorrect doc file name in dabort handler message
well, I can not see any change regarding the doc file ;)
arch/arm/cpu/arm926ejs/orion5x/cpu.c | 4 +- arch/arm/lib/interrupts.c | 2 +- board/ti/omap2420h4/sys_info.c | 28 ++++----- common/Makefile | 3 + common/cmd_dfu.c | 2 +- doc/README.arm-unaligned-accesses | 110 ++++++++++++++++++++++++++++++++++
--------------^^
fs/fat/Makefile | 2 + fs/ubifs/Makefile | 3 + lib/Makefile | 3 + 9 files changed, 139 insertions(+), 18 deletions(-) create mode 100644 doc/README.arm-unaligned-accesses
<snip>
diff --git a/arch/arm/lib/interrupts.c b/arch/arm/lib/interrupts.c index 74ff5ce..02ccb22 100644 --- a/arch/arm/lib/interrupts.c +++ b/arch/arm/lib/interrupts.c @@ -169,7 +169,7 @@ void do_prefetch_abort (struct pt_regs *pt_regs)
void do_data_abort (struct pt_regs *pt_regs) {
- printf ("data abort\n");
- printf ("data abort\n\n MAYBE you should read doc/README.unaligned-accesses\n\n");
--------------------------------------------------------------------^^
show_regs (pt_regs); bad_mode (); }
<snip>
diff --git a/doc/README.arm-unaligned-accesses b/doc/README.arm-unaligned-accesses new file mode 100644 index 0000000..6f07436 --- /dev/null +++ b/doc/README.arm-unaligned-accesses @@ -0,0 +1,110 @@ +If you are reading this because of a data abort: the following MIGHT +be relevant to your abort, if it was caused by an alignment violation. +In order to determine this, use the PC from the abort dump along with +an objdump -s -S of the u-boot ELF binary to locate the function where +the abort happened; then compare this function with the exemples below.
examples
+If they match, then you've been hit with a compiler generated unaligned +access, and you should rewrite your code or add -mno-unaligned-access +to the command line of the offending file.
+*
+Since U-Boot runs on a variety of hardware, some only able to perform +unaligned accesses with a strong penalty, some unable to perform them +at all, the policy regarding unaligned accesses is to not perform any, +unless absolutely necessary because of hardware or standards.
+Also, on hardware which permits it, the core is configured to throw +data abort exceptions on unaligned accesses in order to catch these
----------------------------------------------^^
+unallowed accesses as early as possible.
+Until version 4.7, the gcc default for performing unaligned accesses +(-mno-unaligned-access) is to emulate unaligned accesses using aligned +loads and stores plus shifts and masks. Emulated unaligned accesses +will not be caught by hardware. These accesses may be costly and may +be actually unnecessary. In order to catch these accesses and remove
-----^^
+or optimize them, option -munaligned-access is explicitly set for all +versions of gcc which support it.
+From gcc 4.7 onward, the default for performing unaligned accesses +is to use unaligned native loads and stores (-munaligned-access), +because the cost of unaligned accesses has dropped. This should not +affect U-Boot's policy of controlling unaligned accesses, however the +compiler may generate uncontrolled unaligned on its own in at least
-----------------------------------------------^ access?
+one known case: when declaring a local initialized char array, e.g.
+function foo() +{
- char buffer[] = "initial value";
+/* or */
- char buffer[] = { 'i', 'n', 'i', 't', 0 };
- ...
+}
+Under -munaligned-accesses with optimizations on, this declaration +causes the compiler to generate native loads from the literal string +and native stores to the buffer, and the literal string alignment +cannot be controlled. If it is misaligned, then the core will throw +a data abort exception.
+Quite probably the same might happen for 16-bit array initializations +where the constant is aligned on a boundary which is a multiple of 2 +but not of 4:
+function foo() +{
- u16 buffer[] = { 1, 2, 3 };
- ...
+}
+The long term solution to this issue is to add an option to gcc to +allow controlling the general alignment of data, including constant +initialization values.
+However this will only apply to the version of gcc which will have such +an option. For other versions, there are four workarounds:
+a) Enforce as a rule that array initializations as described above
- are forbidden. This is generally not acceptable as they are valid,
- and usual, C constructs. The only case where they could be rejected
- is when they actually equate to a const char* declaration, i.e. the
- array is initialized and never modified in the function's scope.
+b) Drop the requirement on unaligned accesses at least for ARMv7,
- i.e. do not throw a data abort exception upon unaligned accesses.
- But that will allow adding badly aligned code to U-Boot, only for
- it to fail when re-used with another, more strict, target, possibly
-------------------------------------------------------^
- once the bad code is already in mainline.
+c) Relax the -munaligned-access rule globally. This will prevent native
- unaligned accesses of course, but that will also hide any bug caused
- by a bad unaligned access, making it much harder to diagnose it. It
- is actually what already happens when building ARM targets with a
- pre-4.7 gcc, and it may actually already hide some bugs yet unseen
- until the target gets compiled with -munaligned-access.
+d) Relax the -munaligned-access rule only for for files susceptible to
- the local initialized array issue. This minimizes the quantity of
- code which can hide unwanted misaligned accesses.
+The option retained is d).
+Considering the rarity of actual occurrences (as of this writing, 5 +files out of 7840 in U-Boot, or .3%, contain an initialized local char +array which cannot actually be replaced with a const char*), detection +if the issue in patches should not be asked from contributors.
---8<--- Considering the rarity of actual occurrences, detection if the issue in patches should not be asked from contributors. --->8---
I think there is something wrong wih this sentence ... albeit I can not definitely say what it is.
+Detecting files susceptible to the issue can be automated through a +filter installed as a hook in .git which recognizes local char array +initializations. Automation should err on the false positive side, for +instance flagging non-local arrays as if they were local if they cannot +be told apart.
Isn't that a suggestion that should be asked to the list instead? I wonder why it is written down in this README document.
+In any case, detection shall not prevent committing the patch, but +shall pre-populate the commit message with a note to the effect that +this patch contains an initialized local char or 16-bit array and thus +should be protected from the gcc 4.7 issue.
+Upon a positive detection, either option -mno-unaligned-access should +be applied (if not already) to the affected file(s), or if the array +is a pseudo const char*, it should be replaced by one.
Best regards
Andreas Bießmann