From b823fd9ba56d56e3cbb5b05e7a4815fb0914204a Mon Sep 17 00:00:00 2001 From: Albert ARIBAUD Date: Tue, 9 Oct 2012 09:28:15 +0000 Subject: ARM: prevent misaligned array inits 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 [trini: Switch to usign call cc-option for -mno-unaligned-access as Albert had done previously as that's really correct] Signed-off-by: Tom Rini --- doc/README.arm-unaligned-accesses | 122 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 doc/README.arm-unaligned-accesses (limited to 'doc') diff --git a/doc/README.arm-unaligned-accesses b/doc/README.arm-unaligned-accesses new file mode 100644 index 0000000..c37d135 --- /dev/null +++ b/doc/README.arm-unaligned-accesses @@ -0,0 +1,122 @@ +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 examples below. +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. + +Note that the PC shown in the abort message is relocated. In order to +be able to match it to an address in the ELF binary dump, you will need +to know the relocation offset. If your target defines CONFIG_CMD_BDI +and if you can get to the prompt and enter commands before the abort +happens, then command "bdinfo" will give you the offset. Otherwise you +will need to try a build with DEBUG set, which will display the offset, +or use a debugger and set a breakpoint at relocate_code() to see the +offset (passed as an argument). + +* + +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 starting at armv7 architectures, the default for +performing unaligned accesses is to use unaligned native loads and +stores (-munaligned-access), because the cost of unaligned accesses +has dropped on armv7 and beyond. This should not affect U-Boot's +policy of controlling unaligned accesses, however the compiler may +generate uncontrolled unaligned accesses on its own in at least 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 a stricter 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 and for armv7 architectures and + beyond. This minimizes the quantity of code which can hide unwanted + misaligned accesses. + +The option retained is d). + +Considering that actual occurrences of the issue are rare (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*), +contributors should not be required to systematically try and detect +the issue in their patches. + +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. + +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 $(PLATFORM_NO_UNALIGNED) should be +added to CFLAGS for the affected file(s), or if the array is a pseudo +const char*, it should be replaced by an actual one. -- cgit v1.1