[U-Boot,V4] ARM: prevent misaligned array inits

On Mon, Oct 08, 2012 at 09:50:18PM +0200, 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>

We unfortunately have a problem here.  This ends up passing
-mno-unaligned-access globally, for all platforms.  We need something
like the following:

Ensure that we only pass -mno-unaligned-access to ARMv7 platforms (where
we must ensure this flag is passed so no using call-cc-option).

Signed-off-by; Tom Rini <trini@ti.com>

Hi Tom,

On Tue, 9 Oct 2012 11:34:11 -0700, Tom Rini <trini@ti.com> wrote:

> On Mon, Oct 08, 2012 at 09:50:18PM +0200, 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>
> 
> We unfortunately have a problem here.  This ends up passing
> -mno-unaligned-access globally, for all platforms.  We need something
> like the following:

General remark: I'd strongly suggest not to copy-paste in a list reply
somtheing that looks suspiciously like a (new) patch. It leads to this
kind of things: http://patchwork.ozlabs.org/patch/190403/ which may not
be what you wanted.

> Ensure that we only pass -mno-unaligned-access to ARMv7 platforms (where
> we must ensure this flag is passed so no using call-cc-option).
> 
> Signed-off-by; Tom Rini <trini@ti.com>
> 
> diff --git a/arch/arm/cpu/armv7/config.mk b/arch/arm/cpu/armv7/config.mk
> index 5407cb6..3c5ca23 100644
> --- a/arch/arm/cpu/armv7/config.mk
> +++ b/arch/arm/cpu/armv7/config.mk
> @@ -34,6 +34,10 @@ PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_ARMV7)
>  # =========================================================================
>  PF_RELFLAGS_SLB_AT := $(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,))
>  PLATFORM_RELFLAGS += $(PF_RELFLAGS_SLB_AT)
> +
> +# SEE README.arm-unaligned-accesses
> +PLATFORM_NO_UNALIGNED := -mno-unaligned-access

I suspect that you meant

PLATFORM_NO_UNALIGNED := $(call cc-option,-mno-unaligned-access,)

?

> +
>  ifneq ($(CONFIG_IMX_CONFIG),)
>  ALL-y	+= $(obj)u-boot.imx
>  endif
> diff --git a/common/Makefile b/common/Makefile
> index a498367..33c606a 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -233,8 +233,8 @@ $(obj)../tools/envcrc:
>  	$(MAKE) -C ../tools
>  
>  # SEE README.arm-unaligned-accesses
> -$(obj)hush.o: CFLAGS += -mno-unaligned-access
> -$(obj)fdt_support.o: CFLAGS += -mno-unaligned-access
> +$(obj)hush.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
> +$(obj)fdt_support.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
>  
>  #########################################################################
>  
> diff --git a/fs/fat/Makefile b/fs/fat/Makefile
> index 5c4a2aa..02e6881 100644
> --- a/fs/fat/Makefile
> +++ b/fs/fat/Makefile
> @@ -40,7 +40,7 @@ $(LIB):	$(obj).depend $(OBJS)
>  	$(call cmd_link_o_target, $(OBJS))
>  
>  # SEE README.arm-unaligned-accesses
> -$(obj)file.o: CFLAGS += -mno-unaligned-access
> +$(obj)file.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
>  
>  #########################################################################
>  
> diff --git a/fs/ubifs/Makefile b/fs/ubifs/Makefile
> index 71c40f2..bfe6874 100644
> --- a/fs/ubifs/Makefile
> +++ b/fs/ubifs/Makefile
> @@ -43,7 +43,7 @@ $(LIB):	$(obj).depend $(OBJS)
>  	$(call cmd_link_o_target, $(OBJS))
>  
>  # SEE README.arm-unaligned-accesses
> -$(obj)super.o: CFLAGS += -mno-unaligned-access
> +$(obj)super.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
>  
>  #########################################################################
>  
> diff --git a/lib/Makefile b/lib/Makefile
> index decc5ee..e44e045 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -84,7 +84,7 @@ $(LIB):	$(obj).depend $(OBJS)
>  	$(call cmd_link_o_target, $(OBJS))
>  
>  # SEE README.arm-unaligned-accesses
> -$(obj)bzlib.o: CFLAGS += -mno-unaligned-access
> +$(obj)bzlib.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
>  
>  #########################################################################

Will prepare V5 right away.

Amicalement,

On Tue, Oct 09, 2012 at 08:42:26PM +0200, Albert ARIBAUD wrote:
> Hi Tom,
> 
> On Tue, 9 Oct 2012 11:34:11 -0700, Tom Rini <trini@ti.com> wrote:
> 
> > On Mon, Oct 08, 2012 at 09:50:18PM +0200, 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>
> > 
> > We unfortunately have a problem here.  This ends up passing
> > -mno-unaligned-access globally, for all platforms.  We need something
> > like the following:
> 
> General remark: I'd strongly suggest not to copy-paste in a list reply
> somtheing that looks suspiciously like a (new) patch. It leads to this
> kind of things: http://patchwork.ozlabs.org/patch/190403/ which may not
> be what you wanted.
> 
> > Ensure that we only pass -mno-unaligned-access to ARMv7 platforms (where
> > we must ensure this flag is passed so no using call-cc-option).
> > 
> > Signed-off-by; Tom Rini <trini@ti.com>
> > 
> > diff --git a/arch/arm/cpu/armv7/config.mk b/arch/arm/cpu/armv7/config.mk
> > index 5407cb6..3c5ca23 100644
> > --- a/arch/arm/cpu/armv7/config.mk
> > +++ b/arch/arm/cpu/armv7/config.mk
> > @@ -34,6 +34,10 @@ PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_ARMV7)
> >  # =========================================================================
> >  PF_RELFLAGS_SLB_AT := $(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,))
> >  PLATFORM_RELFLAGS += $(PF_RELFLAGS_SLB_AT)
> > +
> > +# SEE README.arm-unaligned-accesses
> > +PLATFORM_NO_UNALIGNED := -mno-unaligned-access
> 
> I suspect that you meant
> 
> PLATFORM_NO_UNALIGNED := $(call cc-option,-mno-unaligned-access,)

This could lead to silently not applying the flag when we want to apply
the flag.  Just like before, I think we need to force the option, until
such time as we can do $(call
cc-option,-mdont-unalign-strings-for-fun,-mno-unaligned-access).