Re: [U-Boot] [PATCH 3/3] post/lib_powerpc/multi.c: fix stack overflow error

2011-12-23 Thread Wolfgang Denk
Dear Anatolij Gustschin,

In message <20111223170640.18cfe56f@wker> you wrote:
> 
> > The code and comment disagreed: the comment claimed that r6...r31
> > were copied, and consequently the arrays for "src" and "dst" were
> > declared with 26 entries, but the actual code ("lmw r5,0(r3)" and
> > "stmw r5,0(r4)") copied _27_ words (r5 through r31), which resulted
> > in false "POST cpu Error at multi test" messages.
> 
> Great! Thanks for fixing this bug!

Thanks for testing and reporting it!

> But I wonder why didn't we see it with U-Boot built using older
> GCC versions.

Yes, I was surprised,too, and suspected a compiler problem instead...

> Since only 26 words will be compared after the test, the issue
> only shows up if the destination buffer is placed at lower
> addresses on the stack than the source buffer. In this case the
> first word in the source buffer is overwritten. GCC 4.6.1 generated
> code which changed the order of src[] and dst[] on the stack and
> the hidden bug showed up.

This matches my own analysis.  Actually the code generated by gcc
4.5.1 and 4.6.1 looks _really_ different in a lot of places; it seems
a lot has been changed in GCC again.

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: w...@denx.de
Few people do business well who do nothing else.
   -- Philip Earl of Chesterfield
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] post/lib_powerpc/multi.c: fix stack overflow error

2011-12-23 Thread Wolfgang Denk
Dear Wolfgang Denk,

In message <1324639752-26856-3-git-send-email...@denx.de> you wrote:
> The code and comment disagreed: the comment claimed that r6...r31
> were copied, and consequently the arrays for "src" and "dst" were
> declared with 26 entries, but the actual code ("lmw r5,0(r3)" and
> "stmw r5,0(r4)") copied _27_ words (r5 through r31), which resulted
> in false "POST cpu Error at multi test" messages.
> 
> Fix the comment and the array sizes.
> 
> Signed-off-by: Wolfgang Denk 
> Cc: Anatolij Gustschin 
> Cc: Stefan Roese 
> Cc: Kumar Gala 
> Cc: Kim Phillips 
> Cc: Andy Fleming 
> ---
>  post/lib_powerpc/multi.c |8 
>  1 files changed, 4 insertions(+), 4 deletions(-)

Applied, thanks.

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: w...@denx.de
"Love is an ideal thing, marriage a real thing; a  confusion  of  the
real with the ideal never goes unpunished."  - Goethe
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] post/lib_powerpc/multi.c: fix stack overflow error

2011-12-23 Thread Anatolij Gustschin
Hello Wolfgang,

On Fri, 23 Dec 2011 12:29:12 +0100
Wolfgang Denk  wrote:

> The code and comment disagreed: the comment claimed that r6...r31
> were copied, and consequently the arrays for "src" and "dst" were
> declared with 26 entries, but the actual code ("lmw r5,0(r3)" and
> "stmw r5,0(r4)") copied _27_ words (r5 through r31), which resulted
> in false "POST cpu Error at multi test" messages.

Great! Thanks for fixing this bug!

Acked-by: Anatolij Gustschin 
Tested-by: Anatolij Gustschin 

But I wonder why didn't we see it with U-Boot built using older
GCC versions.

Since only 26 words will be compared after the test, the issue
only shows up if the destination buffer is placed at lower
addresses on the stack than the source buffer. In this case the
first word in the source buffer is overwritten. GCC 4.6.1 generated
code which changed the order of src[] and dst[] on the stack and
the hidden bug showed up.

Here is a partial dump of each buffer and additionally a
dump of the following word. The buffer address is in
parenthesis:

with GCC 4.2.2:

00: src(03e51c74) 0x, dst(03e51cdc) 0x
01: src(03e51c78) 0x0001, dst(03e51ce0) 0x
...
25: src(03e51cd8) 0x0019, dst(03e51d40) 0x
26: src(03e51cdc) 0x, dst(03e51d44) 0x

Test result:

00: src(03e51c74) 0x, dst(03e51cdc) 0x
01: src(03e51c78) 0x0001, dst(03e51ce0) 0x0001
...
25: src(03e51cd8) 0x0019, dst(03e51d40) 0x0019
26: src(03e51cdc) 0x, dst(03e51d44) 0x


with GCC 4.6.1:

00: src(03e57cf4) 0x, dst(03e57c8c) 0x
01: src(03e57cf8) 0x0001, dst(03e57c90) 0x
...
25: src(03e57d58) 0x0019, dst(03e57cf0) 0x
26: src(03e57d5c) 0x03f9c3c0, dst(03e57cf4) 0x

Test result:
Error at multi test !
00: src(03e57cf4) 0x03f9c3c0, dst(03e57c8c) 0x
01: src(03e57cf8) 0x0001, dst(03e57c90) 0x0001
...
25: src(03e57d58) 0x0019, dst(03e57cf0) 0x0019
26: src(03e57d5c) 0x03f9c3c0, dst(03e57cf4) 0x03f9c3c0

Thanks,
Anatolij
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 3/3] post/lib_powerpc/multi.c: fix stack overflow error

2011-12-23 Thread Wolfgang Denk
The code and comment disagreed: the comment claimed that r6...r31
were copied, and consequently the arrays for "src" and "dst" were
declared with 26 entries, but the actual code ("lmw r5,0(r3)" and
"stmw r5,0(r4)") copied _27_ words (r5 through r31), which resulted
in false "POST cpu Error at multi test" messages.

Fix the comment and the array sizes.

Signed-off-by: Wolfgang Denk 
Cc: Anatolij Gustschin 
Cc: Stefan Roese 
Cc: Kumar Gala 
Cc: Kim Phillips 
Cc: Andy Fleming 
---
 post/lib_powerpc/multi.c |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/post/lib_powerpc/multi.c b/post/lib_powerpc/multi.c
index b8619de..6642ee3 100644
--- a/post/lib_powerpc/multi.c
+++ b/post/lib_powerpc/multi.c
@@ -27,9 +27,9 @@
  * CPU test
  * Load/store multiple word instructions:  lmw, stmw
  *
- * 26 consecutive words are loaded from a source memory buffer
- * into GPRs r6 through r31. After that, 26 consecutive words are stored
- * from the GPRs r6 through r31 into a target memory buffer. The contents
+ * 27 consecutive words are loaded from a source memory buffer
+ * into GPRs r5 through r31. After that, 27 consecutive words are stored
+ * from the GPRs r5 through r31 into a target memory buffer. The contents
  * of the source and target buffers are then compared.
  */
 
@@ -44,7 +44,7 @@ int cpu_post_test_multi(void)
 {
int ret = 0;
unsigned int i;
-   ulong src[26], dst[26];
+   ulong src[27], dst[27];
int flag = disable_interrupts();
 
ulong code[] = {
-- 
1.7.6.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot