input: fix unaligned access in key_matrix_decode_fdt()
PATCH 1/1 input: fix unaligned access in key_matrix_decode_fdt()
From: Stephen Warren <[email protected]>
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.
--- drivers/input/key_matrix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/input/key_matrix.c +1 -1
@@ -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;
Signature · 1 line
1.7.10.4
PATCH 1/1 input: fix unaligned access in key_matrix_decode_fdt()
Quoted 5 lines · Stephen Warren
From: Stephen Warren <[email protected]>
Initialized character arrays on the stack can cause gcc to emit code that performs unaligned accessess. Make the data static to avoid this.
Quoted 7 lines
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 <[email protected]>
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
PATCH 1/1 input: fix unaligned access in key_matrix_decode_fdt()
Dear Simon Glass,
Quoted 3 lines · In message <CAPnjgZ1_M-7EgGade-p-TLrYnPNNXGyh8zh32w8a2+2qGVYuow@mail.gmail.com>…
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
Signature · 5 lines
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.
PATCH 1/1 input: fix unaligned access in key_matrix_decode_fdt()
Hi Wolfgang,
Quoted 6 lines · Wolfgang Denk
Dear Simon Glass,
In message < CAPnjgZ1_M-7EgGade-p-TLrYnPNNXGyh8zh32w8a2+2qGVYuow at mail.gmail.com> you
Quoted 1 line · wrote:
Quoted 8 lines
Quoted 1 line
Another problem I have is that the 'linux' in 'linux,keymap' in the
device
Quoted 1 line
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.
Quoted 12 lines
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
PATCH 1/1 input: fix unaligned access in key_matrix_decode_fdt()
In reply to Simon GlassQuoted 22 lines · Simon Glass
On Wed, May 22, 2013 at 11:48 AM, Stephen Warren <[email protected] <mailto:[email protected]>> wrote:
From: Stephen Warren <swarren at nvidia.com <mailto:[email protected]>>
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 <[email protected]<mailto:[email protected]>>
Quoted 6 lines
Acked-by Simon Glass <sjg at chromium.org <mailto:[email protected]>>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.
Quoted 4 lines
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:-)
PATCH 1/1 input: fix unaligned access in key_matrix_decode_fdt()
In reply to Stephen WarrenQuoted 9 lines · Stephen Warren
From: Stephen Warren <[email protected]>
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.
Quoted 5 lines · Stephen Warren
From: Stephen Warren <[email protected]>
Initialized character arrays on the stack can cause gcc to emit code that performs unaligned accessess. Make the data static to avoid this.
Quoted 7 lines
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 <[email protected]>Acked-by Simon Glass <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
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>
Quoted 12 lines · Tom Rini
Quoted 10 lines · Stephen Warren
From: Stephen Warren <[email protected]>
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.
Quoted 4 lines
Quoted 2 lines
Signed-off-by Stephen Warren <[email protected]>Acked-by Simon Glass <[email protected]>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.
Signature · 8 lines
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>