[U-Boot] [PATCH] input: fix unaligned access in key_matrix_decode_fdt()
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;
On Wed, May 22, 2013 at 11:48 AM, Stephen Warren swarren@wwwdotorg.orgwrote:
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
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
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@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@denx.de Der Horizont vieler Menschen ist ein Kreis mit Radius Null -- und das nennen sie ihren Standpunkt.
That was worth translating :-)
Regards, Simon
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@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@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:-)
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.
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!
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.
participants (4)
-
Simon Glass -
Stephen Warren -
Tom Rini -
Wolfgang Denk