[U-Boot] [PATCH] input: fix unaligned access in key_matrix_decode_fdt()

[U-Boot] [PATCH] input: fix unaligned access in key_matrix_decode_fdt()

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Initialized character arrays on the stack can cause gcc to emit code that
performs unaligned accessess. Make the data static to avoid this.

Note that the unaligned accesses are made when copying data to prefix[] on
the stack from .rodata. By making the data static, the copy is completely
avoided. All explicitly written code treats the data as u8[], so will never
cause any unaligned accesses.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/input/key_matrix.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/key_matrix.c b/drivers/input/key_matrix.c
index 946a186..c8b048e 100644
--- a/drivers/input/key_matrix.c
+++ b/drivers/input/key_matrix.c
@@ -158,7 +158,7 @@ int key_matrix_decode_fdt(struct key_matrix *config, const void *blob,
 			  int node)
 {
 	const struct fdt_property *prop;
-	const char prefix[] = "linux,";
+	static const char prefix[] = "linux,";
 	int plen = sizeof(prefix) - 1;
 	int offset;
 
-- 
1.7.10.4

[U-Boot] [PATCH] input: fix unaligned access in key_matrix_decode_fdt()

[Simon Glass]

On Wed, May 22, 2013 at 11:48 AM, Stephen Warren <swarren@wwwdotorg.org>wrote:

> From: Stephen Warren <swarren@nvidia.com>
>
> Initialized character arrays on the stack can cause gcc to emit code that
> performs unaligned accessess. Make the data static to avoid this.
>
> Note that the unaligned accesses are made when copying data to prefix[] on
> the stack from .rodata. By making the data static, the copy is completely
> avoided. All explicitly written code treats the data as u8[], so will never
> cause any unaligned accesses.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>

Acked-by: Simon Glass <sjg@chromium.org>

Thanks for fixing.

I hit this with gcc 4.7. I wonder if previous revisions would not make this
assumption?

Another problem I have is that the 'linux' in 'linux,keymap' in the device
compile turns into '1' since gcc predefines 'linux' to 1:

I think I'm going to add a -Ulinux to dts/Makefile.

Regards.
Simon

[U-Boot] [PATCH] input: fix unaligned access in key_matrix_decode_fdt()

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ1_M-7EgGade-p-TLrYnPNNXGyh8zh32w8a2+2qGVYuow@mail.gmail.com> you wrote:
>
> Another problem I have is that the 'linux' in 'linux,keymap' in the device
> compile turns into '1' since gcc predefines 'linux' to 1:

Should this not be considered a GCC bug? After all, "linux" is not a
reserved identifier.  [Defining __linux, or __linux__ would be probab-
ly OK, but "linux" or "arm" are not - IMO.]

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Der Horizont vieler Menschen ist ein Kreis mit Radius Null --
und das nennen sie ihren Standpunkt.

[U-Boot] [PATCH] input: fix unaligned access in key_matrix_decode_fdt()

[Simon Glass]

Hi Wolfgang,

On Sun, May 26, 2013 at 1:55 PM, Wolfgang Denk <wd@denx.de> wrote:

> Dear Simon Glass,
>
> In message <
> CAPnjgZ1_M-7EgGade-p-TLrYnPNNXGyh8zh32w8a2+2qGVYuow at mail.gmail.com> you
> wrote:
> >
> > Another problem I have is that the 'linux' in 'linux,keymap' in the
> device
> > compile turns into '1' since gcc predefines 'linux' to 1:
>
> Should this not be considered a GCC bug? After all, "linux" is not a
> reserved identifier.  [Defining __linux, or __linux__ would be probab-
> ly OK, but "linux" or "arm" are not - IMO.]
>

It certainly surprised me, but if it is a bug, then it might be too late to
fix it, since that release is widespread.


>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> Der Horizont vieler Menschen ist ein Kreis mit Radius Null --
> und das nennen sie ihren Standpunkt.
>

That was worth translating :-)

Regards,
Simon

[U-Boot] [PATCH] input: fix unaligned access in key_matrix_decode_fdt()

[Stephen Warren]

On 05/26/2013 01:28 PM, Simon Glass wrote:
> 
> On Wed, May 22, 2013 at 11:48 AM, Stephen Warren <swarren@wwwdotorg.org
> <mailto:swarren@wwwdotorg.org>> wrote:
> 
>     From: Stephen Warren <swarren at nvidia.com <mailto:swarren@nvidia.com>>
> 
>     Initialized character arrays on the stack can cause gcc to emit code
>     that
>     performs unaligned accessess. Make the data static to avoid this.
> 
>     Note that the unaligned accesses are made when copying data to
>     prefix[] on
>     the stack from .rodata. By making the data static, the copy is
>     completely
>     avoided. All explicitly written code treats the data as u8[], so
>     will never
>     cause any unaligned accesses.
> 
>     Signed-off-by: Stephen Warren <swarren@nvidia.com
>     <mailto:swarren@nvidia.com>>
> 
> 
> Acked-by: Simon Glass <sjg at chromium.org <mailto:sjg@chromium.org>>
> 
> Thanks for fixing.
> 
> I hit this with gcc 4.7. I wonder if previous revisions would not make
> this assumption?

IIRC, gcc-4.7 introduces the emission of native unaligned accesses, and
it's been back-ported to Linaro gcc-4.6.

> Another problem I have is that the 'linux' in 'linux,keymap' in the
> device compile turns into '1' since gcc predefines 'linux' to 1:
> 
> I think I'm going to add a -Ulinux to dts/Makefile.

I forget the exact details, but if you check the Linux makefiles for
dtc+cpp, they don't suffer from this issue any more; it may have been
due to use of -x assembler-with-cpp. I do also have a bug filed
internally to NVIDIA to fix that, which is assigned to Tom. But, I'm
sure he'd be glad if you fixed it:-)

[U-Boot] [PATCH] input: fix unaligned access in key_matrix_decode_fdt()

[Stephen Warren]

On 05/22/2013 12:48 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Initialized character arrays on the stack can cause gcc to emit code that
> performs unaligned accessess. Make the data static to avoid this.
> 
> Note that the unaligned accesses are made when copying data to prefix[] on
> the stack from .rodata. By making the data static, the copy is completely
> avoided. All explicitly written code treats the data as u8[], so will never
> cause any unaligned accesses.

Tom, does this patch look good?

The discussion following it was unrelated to this patch, but rather
related to pre-processing of device-trees, so I don't think should
prevent this patch being merged.

[U-Boot] input: fix unaligned access in key_matrix_decode_fdt()

[Tom Rini]

On Wed, May 22, 2013 at 08:48:18AM -0000, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> Initialized character arrays on the stack can cause gcc to emit code that
> performs unaligned accessess. Make the data static to avoid this.
> 
> Note that the unaligned accesses are made when copying data to prefix[] on
> the stack from .rodata. By making the data static, the copy is completely
> avoided. All explicitly written code treats the data as u8[], so will never
> cause any unaligned accesses.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130605/c703b9da/attachment.pgp>

[U-Boot] input: fix unaligned access in key_matrix_decode_fdt()

[Tom Rini]

On Wed, Jun 05, 2013 at 08:34:20AM -0400, Tom Rini wrote:
> On Wed, May 22, 2013 at 08:48:18AM -0000, Stephen Warren wrote:
> 
> > From: Stephen Warren <swarren@nvidia.com>
> > 
> > Initialized character arrays on the stack can cause gcc to emit code that
> > performs unaligned accessess. Make the data static to avoid this.
> > 
> > Note that the unaligned accesses are made when copying data to prefix[] on
> > the stack from .rodata. By making the data static, the copy is completely
> > avoided. All explicitly written code treats the data as u8[], so will never
> > cause any unaligned accesses.
> > 
> > Signed-off-by: Stephen Warren <swarren@nvidia.com>
> > Acked-by: Simon Glass <sjg@chromium.org>
> 
> Applied to u-boot/master, thanks!

Bah!  I see I applied v1 and not v2, I shall post and apply the delta
between them momentarily.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130606/de7d41c7/attachment.pgp>