mirror of
https://github.com/Fishwaldo/u-boot.git
synced 2025-06-27 17:12:03 +00:00
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 <albert.u.boot@aribaud.net> [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 <trini@ti.com>
This commit is contained in:
parent
6528ff0109
commit
b823fd9ba5
10 changed files with 156 additions and 19 deletions
|
@ -194,8 +194,8 @@ u32 orion5x_device_rev(void)
|
||||||
*/
|
*/
|
||||||
int print_cpuinfo(void)
|
int print_cpuinfo(void)
|
||||||
{
|
{
|
||||||
char dev_str[] = "0x0000";
|
char dev_str[7]; /* room enough for 0x0000 plus null byte */
|
||||||
char rev_str[] = "0x00";
|
char rev_str[5]; /* room enough for 0x00 plus null byte */
|
||||||
char *dev_name = NULL;
|
char *dev_name = NULL;
|
||||||
char *rev_name = NULL;
|
char *rev_name = NULL;
|
||||||
|
|
||||||
|
|
|
@ -26,8 +26,6 @@ PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float
|
||||||
# supported by more tool-chains
|
# supported by more tool-chains
|
||||||
PF_CPPFLAGS_ARMV7 := $(call cc-option, -march=armv7-a, -march=armv5)
|
PF_CPPFLAGS_ARMV7 := $(call cc-option, -march=armv7-a, -march=armv5)
|
||||||
PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_ARMV7)
|
PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_ARMV7)
|
||||||
PF_CPPFLAGS_NO_UNALIGNED := $(call cc-option, -mno-unaligned-access,)
|
|
||||||
PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_NO_UNALIGNED)
|
|
||||||
|
|
||||||
# =========================================================================
|
# =========================================================================
|
||||||
#
|
#
|
||||||
|
@ -36,6 +34,11 @@ PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_NO_UNALIGNED)
|
||||||
# =========================================================================
|
# =========================================================================
|
||||||
PF_RELFLAGS_SLB_AT := $(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,))
|
PF_RELFLAGS_SLB_AT := $(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,))
|
||||||
PLATFORM_RELFLAGS += $(PF_RELFLAGS_SLB_AT)
|
PLATFORM_RELFLAGS += $(PF_RELFLAGS_SLB_AT)
|
||||||
|
|
||||||
|
# SEE README.arm-unaligned-accesses
|
||||||
|
PF_NO_UNALIGNED := $(call cc-option, -mno-unaligned-access,)
|
||||||
|
PLATFORM_NO_UNALIGNED := $(PF_NO_UNALIGNED)
|
||||||
|
|
||||||
ifneq ($(CONFIG_IMX_CONFIG),)
|
ifneq ($(CONFIG_IMX_CONFIG),)
|
||||||
ALL-y += $(obj)u-boot.imx
|
ALL-y += $(obj)u-boot.imx
|
||||||
endif
|
endif
|
||||||
|
|
|
@ -169,7 +169,7 @@ void do_prefetch_abort (struct pt_regs *pt_regs)
|
||||||
|
|
||||||
void do_data_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.arm-unaligned-accesses\n\n");
|
||||||
show_regs (pt_regs);
|
show_regs (pt_regs);
|
||||||
bad_mode ();
|
bad_mode ();
|
||||||
}
|
}
|
||||||
|
|
|
@ -237,20 +237,20 @@ u32 wait_on_value(u32 read_bit_mask, u32 match_value, u32 read_addr, u32 bound)
|
||||||
*********************************************************************/
|
*********************************************************************/
|
||||||
void display_board_info(u32 btype)
|
void display_board_info(u32 btype)
|
||||||
{
|
{
|
||||||
char cpu_2420[] = "2420"; /* cpu type */
|
static const char cpu_2420 [] = "2420"; /* cpu type */
|
||||||
char cpu_2422[] = "2422";
|
static const char cpu_2422 [] = "2422";
|
||||||
char cpu_2423[] = "2423";
|
static const char cpu_2423 [] = "2423";
|
||||||
char db_men[] = "Menelaus"; /* board type */
|
static const char db_men [] = "Menelaus"; /* board type */
|
||||||
char db_ip[] = "IP";
|
static const char db_ip [] = "IP";
|
||||||
char mem_sdr[] = "mSDR"; /* memory type */
|
static const char mem_sdr [] = "mSDR"; /* memory type */
|
||||||
char mem_ddr[] = "mDDR";
|
static const char mem_ddr [] = "mDDR";
|
||||||
char t_tst[] = "TST"; /* security level */
|
static const char t_tst [] = "TST"; /* security level */
|
||||||
char t_emu[] = "EMU";
|
static const char t_emu [] = "EMU";
|
||||||
char t_hs[] = "HS";
|
static const char t_hs [] = "HS";
|
||||||
char t_gp[] = "GP";
|
static const char t_gp [] = "GP";
|
||||||
char unk[] = "?";
|
static const char unk [] = "?";
|
||||||
|
|
||||||
char *cpu_s, *db_s, *mem_s, *sec_s;
|
const char *cpu_s, *db_s, *mem_s, *sec_s;
|
||||||
u32 cpu, rev, sec;
|
u32 cpu, rev, sec;
|
||||||
|
|
||||||
rev = get_cpu_rev();
|
rev = get_cpu_rev();
|
||||||
|
|
|
@ -232,6 +232,10 @@ $(obj)env_embedded.o: $(src)env_embedded.c $(obj)../tools/envcrc
|
||||||
$(obj)../tools/envcrc:
|
$(obj)../tools/envcrc:
|
||||||
$(MAKE) -C ../tools
|
$(MAKE) -C ../tools
|
||||||
|
|
||||||
|
# SEE README.arm-unaligned-accesses
|
||||||
|
$(obj)hush.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
|
||||||
|
$(obj)fdt_support.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
|
||||||
|
|
||||||
#########################################################################
|
#########################################################################
|
||||||
|
|
||||||
# defines $(obj).depend target
|
# defines $(obj).depend target
|
||||||
|
|
|
@ -30,7 +30,7 @@
|
||||||
static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
|
static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
|
||||||
{
|
{
|
||||||
const char *str_env;
|
const char *str_env;
|
||||||
char s[] = "dfu";
|
char *s = "dfu";
|
||||||
char *env_bkp;
|
char *env_bkp;
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
|
|
122
doc/README.arm-unaligned-accesses
Normal file
122
doc/README.arm-unaligned-accesses
Normal file
|
@ -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.
|
|
@ -39,6 +39,8 @@ all: $(LIB) $(AOBJS)
|
||||||
$(LIB): $(obj).depend $(OBJS)
|
$(LIB): $(obj).depend $(OBJS)
|
||||||
$(call cmd_link_o_target, $(OBJS))
|
$(call cmd_link_o_target, $(OBJS))
|
||||||
|
|
||||||
|
# SEE README.arm-unaligned-accesses
|
||||||
|
$(obj)file.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
|
||||||
|
|
||||||
#########################################################################
|
#########################################################################
|
||||||
|
|
||||||
|
|
|
@ -42,6 +42,9 @@ all: $(LIB) $(AOBJS)
|
||||||
$(LIB): $(obj).depend $(OBJS)
|
$(LIB): $(obj).depend $(OBJS)
|
||||||
$(call cmd_link_o_target, $(OBJS))
|
$(call cmd_link_o_target, $(OBJS))
|
||||||
|
|
||||||
|
# SEE README.arm-unaligned-accesses
|
||||||
|
$(obj)super.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
|
||||||
|
|
||||||
#########################################################################
|
#########################################################################
|
||||||
|
|
||||||
# defines $(obj).depend target
|
# defines $(obj).depend target
|
||||||
|
|
|
@ -83,6 +83,9 @@ OBJS := $(addprefix $(obj),$(COBJS))
|
||||||
$(LIB): $(obj).depend $(OBJS)
|
$(LIB): $(obj).depend $(OBJS)
|
||||||
$(call cmd_link_o_target, $(OBJS))
|
$(call cmd_link_o_target, $(OBJS))
|
||||||
|
|
||||||
|
# SEE README.arm-unaligned-accesses
|
||||||
|
$(obj)bzlib.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
|
||||||
|
|
||||||
#########################################################################
|
#########################################################################
|
||||||
|
|
||||||
# defines $(obj).depend target
|
# defines $(obj).depend target
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue