I've just had a quick look at this in passing, but at least some of these changes seem wrong to me. For example, the code in board/ti/omap2420h4/sys_info.c :: display_board_info should be:
void display_board_info(u32 btype) { static const char cpu_2420[] = "2420"; /* cpu type */ static const char cpu_2422[] = "2422"; static const char cpu_2423[] = "2423"; static const char db_men[] = "Menelaus"; /* board type */ static const char db_ip[] = "IP"; static const char mem_sdr[] = "mSDR"; /* memory type */ static const char mem_ddr[] = "mDDR"; static const char t_tst[] = "TST"; /* security level */ static const char t_emu[] = "EMU"; static const char t_hs[] = "HS"; static const char t_gp[] = "GP"; static const char unk[] = "?";
const char *cpu_s, *db_s, *mem_s, *sec_s; u32 cpu, rev, sec;
This produces smaller code and is probably what the original author wanted the compiler to do. I've only compile tested this, not actually run it.
Original file:
Sections: Idx Name Size VMA LMA File off Algn 0 .text 000004b4 00000000 00000000 00000034 2**2 CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE 1 .data 00000000 00000000 00000000 000004e8 2**0 CONTENTS, ALLOC, LOAD, DATA 2 .bss 00000000 00000000 00000000 000004e8 2**0 ALLOC 3 .rodata.str1.1 00000072 00000000 00000000 000004e8 2**0 CONTENTS, ALLOC, LOAD, READONLY, DATA
After my changes:
Sections: Idx Name Size VMA LMA File off Algn 0 .text 000003ac 00000000 00000000 00000034 2**2 CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE 1 .data 00000000 00000000 00000000 000003e0 2**0 CONTENTS, ALLOC, LOAD, DATA 2 .bss 00000000 00000000 00000000 000003e0 2**0 ALLOC 3 .rodata 00000048 00000000 00000000 000003e0 2**2 CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA 4 .rodata.str1.1 00000047 00000000 00000000 00000428 2**0 CONTENTS, ALLOC, LOAD, READONLY, DATA
Regards,
Mark Marshall.
On 10/02/2012 08:46 PM, 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
Please test this patch with gcc 4.7 on boards which do data aborts or resets due to misaligned accesses and report result to me.
arch/arm/cpu/arm926ejs/orion5x/cpu.c | 4 +- board/ti/omap2420h4/sys_info.c | 24 ++++----- common/Makefile | 3 ++ common/cmd_dfu.c | 2 +- doc/README.arm-unaligned-accesses | 95 ++++++++++++++++++++++++++++++++++ fs/fat/Makefile | 2 + fs/ubifs/Makefile | 3 ++ lib/Makefile | 3 ++ 8 files changed, 121 insertions(+), 15 deletions(-) create mode 100644 doc/README.arm-unaligned-accesses
diff --git a/arch/arm/cpu/arm926ejs/orion5x/cpu.c b/arch/arm/cpu/arm926ejs/orion5x/cpu.c index c3948d3..5a4775a 100644 --- a/arch/arm/cpu/arm926ejs/orion5x/cpu.c +++ b/arch/arm/cpu/arm926ejs/orion5x/cpu.c @@ -194,8 +194,8 @@ u32 orion5x_device_rev(void) */ int print_cpuinfo(void) {
- char dev_str[] = "0x0000";
- char rev_str[] = "0x00";
- char dev_str[7]; /* room enough for 0x0000 plus null byte */
- char rev_str[5]; /* room enough for 0x00 plus null byte */ char *dev_name = NULL; char *rev_name = NULL;
diff --git a/board/ti/omap2420h4/sys_info.c b/board/ti/omap2420h4/sys_info.c index a9f7241..b462aa5 100644 --- a/board/ti/omap2420h4/sys_info.c +++ b/board/ti/omap2420h4/sys_info.c @@ -237,18 +237,18 @@ u32 wait_on_value(u32 read_bit_mask, u32 match_value, u32 read_addr, u32 bound) *********************************************************************/ void display_board_info(u32 btype) {
- char cpu_2420[] = "2420"; /* cpu type */
- char cpu_2422[] = "2422";
- char cpu_2423[] = "2423";
- char db_men[] = "Menelaus"; /* board type */
- char db_ip[] = "IP";
- char mem_sdr[] = "mSDR"; /* memory type */
- char mem_ddr[] = "mDDR";
- char t_tst[] = "TST"; /* security level */
- char t_emu[] = "EMU";
- char t_hs[] = "HS";
- char t_gp[] = "GP";
- char unk[] = "?";
char *cpu_2420 = "2420"; /* cpu type */
char *cpu_2422 = "2422";
char *cpu_2423 = "2423";
char *db_men = "Menelaus"; /* board type */
char *db_ip = "IP";
char *mem_sdr = "mSDR"; /* memory type */
char *mem_ddr = "mDDR";
char *t_tst = "TST"; /* security level */
char *t_emu = "EMU";
char *t_hs = "HS";
char *t_gp = "GP";
char *unk = "?";
char *cpu_s, *db_s, *mem_s, *sec_s; u32 cpu, rev, sec;
diff --git a/common/Makefile b/common/Makefile index 125b2be..19b2130 100644 --- a/common/Makefile +++ b/common/Makefile @@ -227,6 +227,9 @@ $(obj)env_embedded.o: $(src)env_embedded.c $(obj)../tools/envcrc $(obj)../tools/envcrc: $(MAKE) -C ../tools
+$(obj)hush.o: CFLAGS += -mno-unaligned-access +$(obj)fdt_support.o: CFLAGS += -mno-unaligned-access
#########################################################################
# defines $(obj).depend target
diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c index 62fb890..01d6b3a 100644 --- a/common/cmd_dfu.c +++ b/common/cmd_dfu.c @@ -30,7 +30,7 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { const char *str_env;
- char s[] = "dfu";
- char *s = "dfu"; char *env_bkp; int ret;
diff --git a/doc/README.arm-unaligned-accesses b/doc/README.arm-unaligned-accesses new file mode 100644 index 0000000..00fb1c0 --- /dev/null +++ b/doc/README.arm-unaligned-accesses @@ -0,0 +1,95 @@ +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 +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 -munified-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 m-unaligned-access.
+d) Relax the -munified-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.
+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.
+Luckily, detecting files susceptible to the issue can be automated +through a filter/regexp/script which would recognize local char array +initializations. Automated 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 be informative only and shall not prevent +applying the patch.
+Upon a positive detection, either option -mno-unaligned-access is +applied (if not already) to the affected file(s), or if the array is a +hidden const char*, it should be replaced by one. diff --git a/fs/fat/Makefile b/fs/fat/Makefile index 9635d36..5c4a2aa 100644 --- a/fs/fat/Makefile +++ b/fs/fat/Makefile @@ -39,6 +39,8 @@ all: $(LIB) $(AOBJS) $(LIB): $(obj).depend $(OBJS) $(call cmd_link_o_target, $(OBJS))
+# SEE README.arm-unaligned-accesses +$(obj)file.o: CFLAGS += -mno-unaligned-access
#########################################################################
diff --git a/fs/ubifs/Makefile b/fs/ubifs/Makefile index ccffe85..71c40f2 100644 --- a/fs/ubifs/Makefile +++ b/fs/ubifs/Makefile @@ -42,6 +42,9 @@ all: $(LIB) $(AOBJS) $(LIB): $(obj).depend $(OBJS) $(call cmd_link_o_target, $(OBJS))
+# SEE README.arm-unaligned-accesses +$(obj)super.o: CFLAGS += -mno-unaligned-access
#########################################################################
# defines $(obj).depend target
diff --git a/lib/Makefile b/lib/Makefile index 45798de..44393ed 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -78,6 +78,9 @@ OBJS := $(addprefix $(obj),$(COBJS)) $(LIB): $(obj).depend $(OBJS) $(call cmd_link_o_target, $(OBJS))
+# SEE README.arm-unaligned-accesses +$(obj)bzlib.o: CFLAGS += -mno-unaligned-access
#########################################################################
# defines $(obj).depend target