[U-Boot] [PATCH] Align linebuf to avoid misaligned aliases of it
Commit 64419e47518bbba059c80b77558f93ad4804145c aliases the uint16_t usp and uint32_t uip variables in print_buffer() to uint8_t variable linebuf without aligning it to an uint32_t address, thus causing data aborts on ARM when doing md.l on 32-bit wide area (and probably 16-bit wide as well).
Aligning linebuf fixes the issue.
Signed-off-by: Albert Aribaud albert.aribaud@free.fr --- lib/display_options.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/lib/display_options.c b/lib/display_options.c index 20319e6..c544777 100644 --- a/lib/display_options.c +++ b/lib/display_options.c @@ -101,7 +101,8 @@ void print_size(unsigned long long size, const char *s) #define DEFAULT_LINE_LENGTH_BYTES (16) int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen) { - uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1]; + uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1] + __attribute__((__aligned__(sizeof(uint32_t)))); uint32_t *uip = (void*)linebuf; uint16_t *usp = (void*)linebuf; uint8_t *ucp = (void*)linebuf;
On Sat, Aug 14, 2010 at 4:11 AM, Albert Aribaud wrote:
Commit 64419e47518bbba059c80b77558f93ad4804145c aliases the uint16_t usp and uint32_t uip variables in print_buffer() to uint8_t variable linebuf without aligning it to an uint32_t address, thus causing data aborts on ARM when doing md.l on 32-bit wide area (and probably 16-bit wide as well).
hmm, i guess i overlooked that issue. usually i'm pretty good at this stuff. sorry about that.
Aligning linebuf fixes the issue.
i dont believe there are issues with gcc stack aligning for native sizes ... just above that
int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen) {
- uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1];
- uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1]
- __attribute__((__aligned__(sizeof(uint32_t))));
__aligned(sizeof(uint32_t)) -mike
Le 14/08/2010 10:25, Mike Frysinger a écrit :
int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen) {
uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1];
uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1]__attribute__((__aligned__(sizeof(uint32_t))));__aligned(sizeof(uint32_t)) -mike
I based the __aligned__ on what's in post/cpu/ppc4xx/cache.c. What is the difference between __aligned and __aligned__?
Amicalement,
On Sat, Aug 14, 2010 at 4:33 AM, Albert ARIBAUD wrote:
Le 14/08/2010 10:25, Mike Frysinger a écrit :
int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen) {
- uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1];
- uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1]
- __attribute__((__aligned__(sizeof(uint32_t))));
__aligned(sizeof(uint32_t))
I based the __aligned__ on what's in post/cpu/ppc4xx/cache.c. What is the difference between __aligned and __aligned__?
i dont mean "__attribute__((__aligned(sizeof(uint32_t))))", i mean "__aligned(sizeof(uint32_t))". the linux compiler headers take care of expanding it to an attribute. -mike
Le 14/08/2010 19:42, Mike Frysinger a écrit :
On Sat, Aug 14, 2010 at 4:33 AM, Albert ARIBAUD wrote:
Le 14/08/2010 10:25, Mike Frysinger a écrit :
int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen) {
uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1];
uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1]__attribute__((__aligned__(sizeof(uint32_t))));__aligned(sizeof(uint32_t))
I based the __aligned__ on what's in post/cpu/ppc4xx/cache.c. What is the difference between __aligned and __aligned__?
i dont mean "__attribute__((__aligned(sizeof(uint32_t))))", i mean "__aligned(sizeof(uint32_t))". the linux compiler headers take care of expanding it to an attribute. -mike
The __aligned(...) syntax fails to compile with the ELDK 4.2 arm toolchain.
Amicalement,
On Wed, Aug 18, 2010 at 8:49 AM, Albert ARIBAUD wrote:
Le 14/08/2010 19:42, Mike Frysinger a écrit :
On Sat, Aug 14, 2010 at 4:33 AM, Albert ARIBAUD wrote:
Le 14/08/2010 10:25, Mike Frysinger a écrit :
int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen) {
- uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1];
- uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1]
- __attribute__((__aligned__(sizeof(uint32_t))));
__aligned(sizeof(uint32_t))
I based the __aligned__ on what's in post/cpu/ppc4xx/cache.c. What is the difference between __aligned and __aligned__?
i dont mean "__attribute__((__aligned(sizeof(uint32_t))))", i mean "__aligned(sizeof(uint32_t))". the linux compiler headers take care of expanding it to an attribute.
The __aligned(...) syntax fails to compile with the ELDK 4.2 arm toolchain.
you need to include linux/compiler.h first ... but i would have thought this be a header already included globally. maybe that's a new topic to start. -mike
Le 18/08/2010 18:46, Mike Frysinger a écrit :
On Wed, Aug 18, 2010 at 8:49 AM, Albert ARIBAUD wrote:
Le 14/08/2010 19:42, Mike Frysinger a écrit :
On Sat, Aug 14, 2010 at 4:33 AM, Albert ARIBAUD wrote:
Le 14/08/2010 10:25, Mike Frysinger a écrit :
int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen) {
uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1];
uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1]__attribute__((__aligned__(sizeof(uint32_t))));__aligned(sizeof(uint32_t))
I based the __aligned__ on what's in post/cpu/ppc4xx/cache.c. What is the difference between __aligned and __aligned__?
i dont mean "__attribute__((__aligned(sizeof(uint32_t))))", i mean "__aligned(sizeof(uint32_t))". the linux compiler headers take care of expanding it to an attribute.
The __aligned(...) syntax fails to compile with the ELDK 4.2 arm toolchain.
you need to include linux/compiler.h first ... but i would have thought this be a header already included globally. maybe that's a new topic to start. -mike
I don't understand why I should introduce a dependency on linux. What is the benefit?
Amicalement,
On Wed, Aug 18, 2010 at 1:46 PM, Albert ARIBAUD wrote:
Le 18/08/2010 18:46, Mike Frysinger a écrit :
you need to include linux/compiler.h first ... but i would have thought this be a header already included globally. maybe that's a new topic to start.
I don't understand why I should introduce a dependency on linux. What is the benefit?
u-boot already includes the header in the source tree, not to mention the fact that this file is already including linux/ctype.h -mike
Le 18/08/2010 19:54, Mike Frysinger a écrit :
On Wed, Aug 18, 2010 at 1:46 PM, Albert ARIBAUD wrote:
Le 18/08/2010 18:46, Mike Frysinger a écrit :
you need to include linux/compiler.h first ... but i would have thought this be a header already included globally. maybe that's a new topic to start.
I don't understand why I should introduce a dependency on linux. What is the benefit?
u-boot already includes the header in the source tree, not to mention the fact that this file is already including linux/ctype.h -mike
But absolutely no .h or .c file uses __aligned whereas __attribute__((__aligned__())) is used in several places, right?
Amicalement,
On Wednesday, August 18, 2010 16:36:39 Albert ARIBAUD wrote:
Le 18/08/2010 19:54, Mike Frysinger a écrit :
On Wed, Aug 18, 2010 at 1:46 PM, Albert ARIBAUD wrote:
Le 18/08/2010 18:46, Mike Frysinger a écrit :
you need to include linux/compiler.h first ... but i would have thought this be a header already included globally. maybe that's a new topic to start.
I don't understand why I should introduce a dependency on linux. What is the benefit?
u-boot already includes the header in the source tree, not to mention the fact that this file is already including linux/ctype.h
But absolutely no .h or .c file uses __aligned whereas __attribute__((__aligned__())) is used in several places, right?
that would seem to be the case currently -mike
Le 19/08/2010 07:58, Mike Frysinger a écrit :
On Wednesday, August 18, 2010 16:36:39 Albert ARIBAUD wrote:
Le 18/08/2010 19:54, Mike Frysinger a écrit :
On Wed, Aug 18, 2010 at 1:46 PM, Albert ARIBAUD wrote:
Le 18/08/2010 18:46, Mike Frysinger a écrit :
you need to include linux/compiler.h first ... but i would have thought this be a header already included globally. maybe that's a new topic to start.
I don't understand why I should introduce a dependency on linux. What is the benefit?
u-boot already includes the header in the source tree, not to mention the fact that this file is already including linux/ctype.h
But absolutely no .h or .c file uses __aligned whereas __attribute__((__aligned__())) is used in several places, right?
that would seem to be the case currently -mike
In that case, I'll stick with the currently used syntax for this patch.
If someone feels the need to switch over to __aligned, I think issuing a separate global patch which changes all occurrences makes more sense than using both forms in the source code.
Amicalement,
participants (3)
-
Albert ARIBAUD -
Albert Aribaud
-
Mike Frysinger