[U-Boot] [PATCH V2] ARM: prevent misaligned array inits

[Albert ARIBAUD]

Hi Andreas,

On Mon, 08 Oct 2012 16:56:41 +0200, "Andreas Bießmann"
<andreas.devel at googlemail.com> wrote:

> Hi Albert,
> 
> On 08.10.2012 16:36, Albert ARIBAUD wrote:
> > Hi Andreas,
> > 
> > On Mon, 08 Oct 2012 13:21:45 +0200, "Andreas Bießmann"
> > <andreas.devel at googlemail.com> wrote:
> > 
> >> 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 at aribaud.net>
> >>> ---
> >>> V2: fix incorrect doc file name in dabort handler message
> >>
> >> well, I can not see any change regarding the doc file ;)
> > 
> > Maybe you though there was a change in the doc file itself? V2 is about
> > correcting the file *name* as printed out by the dabort handler.
> 
> Well, that was I understood by the V2 comment ...
> 
> but V1 has:
> ---8<---
> -	printf ("data abort\n");
> +	printf ("data abort\n\n    MAYBE you should read
> doc/README.unaligned-access\n\n");
> --->8---
> 
> and V2 has:
> ---8<---
> -	printf ("data abort\n");
> +	printf ("data abort\n\n    MAYBE you should read
> doc/README.unaligned-accesses\n\n");
> --->8---
> 
> However the filename is 'doc/README.arm-unaligned-accesses'

Argh!

Will fix in V3... For good. Thanks.

> <snip>
> 
> >>> +
> >>> +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.
> > 
> > s/if/of/, but yes, the sentence could use some rewriting. "Considering
> > that actual occurrences of the issue are rare, contributors should not
> > be required to systematically try and detect the issue in their
> > patches". Would that be ok?
> 
> Sounds way better.

Will put in V3.

> > 
> >>> +
> >>> +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.
> > 
> > Not sure I understand what you mean about "ask[ing] the list instead".
> 
> I meant to discuss this on the list and just install the hook then (at
> least for the denx.de repositories).
> 
> > As for writing it down, I did because it's better written down than
> > forgotten, and at this point there is no proper way to auto install
> > a git hook in a U-Boot developer's tree, at least none that I would
> > feel safe pushing so close to a release. So even if I did provide a
> > script, each developer would have to manually put the hook in place.
> > Therefore, the doc just says how to automate detection.
> 
> But you are right, so forget my comment.

Thanks.

> best regards
> 
> Andreas Bießmann

Amicalement,
-- 
Albert.