Re: [U-Boot] [PATCH] dlmalloc: remove manual reloc alias warning
Dear =?UTF-8?q?Andreas=20Bie=C3=9Fmann?=, In message 1344848523-20359-1-git-send-email-andreas.de...@googlemail.com you wrote: This patch fixes following warning: ---8--- dlmalloc.c: In function 'malloc_bin_reloc': dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1490: note: initialized from here dlmalloc.c:1493: note: initialized from here ... +typedef unsigned long __attribute__((__may_alias__)) ulong_aliased; void malloc_bin_reloc (void) { - unsigned long *p = (unsigned long *)(av_[2]); + ulong_aliased *p = (ulong_aliased *)(av_[2]); I dislike this typedef, especially as it's used only a single time. But even so - why cannot we avoid the __may_alias__ thing alltogether? You wrote in http://article.gmane.org/gmane.comp.boot-loaders.u-boot/137770 that Simon's patch breaks avr32 boards at runtime. The implementation is indeed broken. I wonder if it ever worked: mbinptr *p = av_[2]; int i; for (i = 2; i ARRAY_SIZE(av_); ++i) *p = (mbinptr)((ulong)*p + gd-reloc_off); i. e. this has the offset 2 added twice, so it is missing the first two real entries, env writing beyond the end of the array. Can you please test this code instead: mbinptr *p = av_; int i; for (i = 2; i ARRAY_SIZE(av_); ++i) *p = (mbinptr)((ulong)*p + gd-reloc_off); ? 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 Don't try to outweird me, three-eyes. I get stranger things than you free with my breakfast cereal. - Zaphod Beeblebrox in Hitchhiker's Guide to the Galaxy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dlmalloc: remove manual reloc alias warning
Dear Andreas, In message 20120901101031.2da15205...@gemini.denx.de I wrote: i. e. this has the offset 2 added twice, so it is missing the first two real entries, env writing beyond the end of the array. Please ignore me. Seems my brain is still in vacation mode. However, I really wonder why you say this code would be causing runtime errors for you: mbinptr *p = av_[2]; int i; for (i = 2; i ARRAY_SIZE(av_); ++i) *p = (mbinptr)((ulong)*p + gd-reloc_off); It looks good to me. Can you please add some debug printf()s to find out how the old code and this one differ in their results? 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 All your people must learn before you can reach for the stars. -- Kirk, The Gamesters of Triskelion, stardate 3259.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dlmalloc: remove manual reloc alias warning
Dera Wolfgang Denk, On 01.09.2012 14:02, Wolfgang Denk wrote: Dear Andreas, In message 20120901101031.2da15205...@gemini.denx.de I wrote: i. e. this has the offset 2 added twice, so it is missing the first two real entries, env writing beyond the end of the array. Please ignore me. Seems my brain is still in vacation mode. However, I really wonder why you say this code would be causing runtime errors for you: mbinptr *p = av_[2]; int i; for (i = 2; i ARRAY_SIZE(av_); ++i) *p = (mbinptr)((ulong)*p + gd-reloc_off); It looks good to me. Well, I did not investigate it further, but I can do this weekend (after merging/testing/pull request the atmel arm stuff). Hopefully we get a solution for the annoying alias warning in upcoming release. Best regards Andreas Bießmann Can you please add some debug printf()s to find out how the old code and this one differ in their results? Best regards, Wolfgang Denk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dlmalloc: remove manual reloc alias warning
Dear Simon Glass, On 15.08.12 02:15, Simon Glass wrote: Hi, On Tue, Aug 14, 2012 at 1:50 AM, Andreas Bießmann andreas.de...@googlemail.com wrote: Dear all, On 13.08.2012 11:02, Andreas Bießmann wrote: From: Andreas Bießmann biessm...@corscience.de The avr32 architecture (and some others) require manual relocation. Due to the previous error all avr32 boards gave warnings in MAKEALL wich makes it hard to find new warnings. This patch fixes following warning: ---8--- dlmalloc.c: In function 'malloc_bin_reloc': dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1490: note: initialized from here dlmalloc.c:1493: note: initialized from here ---8--- Signed-off-by: Andreas Bießmann biessm...@corscience.de --- A question to all the other related arches, namely m68k, mips, nds32 and sparc: Do you encounter the same warnings or is this warning due to my outdated compiler (4.4.3 currently, unfortunately atmel do not bother to send their patches mainline)? common/dlmalloc.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index c645d73..78b1885 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -1485,9 +1485,10 @@ static mbinptr av_[NAV * 2 + 2] = { }; #ifdef CONFIG_NEEDS_MANUAL_RELOC +typedef unsigned long __attribute__((__may_alias__)) ulong_aliased; void malloc_bin_reloc (void) { - unsigned long *p = (unsigned long *)(av_[2]); + ulong_aliased *p = (ulong_aliased *)(av_[2]); int i; for (i=2; i(sizeof(av_)/sizeof(mbinptr)); ++i) { *p++ += gd-reloc_off; this seems to be related to http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/121848/focus=121849 can you Wolfgang and/or Simon please comment. My only comment is that I created that patch due to errors I saw at the time, and still see with the toolchain I use for avr32. So you also want to remove this annoying warning. Sorry that I didn't test your patch from January earlier, but it is broken at runtime. I would really like to remove the alias warning in 2012.10. My patch is straight forward, maybe there are better solutions. However I would like to see tested by from other affected arches (m68k, mips, nds32 and sparc). Best regards Andreas Bießmann ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dlmalloc: remove manual reloc alias warning
Dear all, On 13.08.2012 11:02, Andreas Bießmann wrote: From: Andreas Bießmann biessm...@corscience.de The avr32 architecture (and some others) require manual relocation. Due to the previous error all avr32 boards gave warnings in MAKEALL wich makes it hard to find new warnings. This patch fixes following warning: ---8--- dlmalloc.c: In function 'malloc_bin_reloc': dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1490: note: initialized from here dlmalloc.c:1493: note: initialized from here ---8--- Signed-off-by: Andreas Bießmann biessm...@corscience.de --- A question to all the other related arches, namely m68k, mips, nds32 and sparc: Do you encounter the same warnings or is this warning due to my outdated compiler (4.4.3 currently, unfortunately atmel do not bother to send their patches mainline)? common/dlmalloc.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index c645d73..78b1885 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -1485,9 +1485,10 @@ static mbinptr av_[NAV * 2 + 2] = { }; #ifdef CONFIG_NEEDS_MANUAL_RELOC +typedef unsigned long __attribute__((__may_alias__)) ulong_aliased; void malloc_bin_reloc (void) { - unsigned long *p = (unsigned long *)(av_[2]); + ulong_aliased *p = (ulong_aliased *)(av_[2]); int i; for (i=2; i(sizeof(av_)/sizeof(mbinptr)); ++i) { *p++ += gd-reloc_off; this seems to be related to http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/121848/focus=121849 can you Wolfgang and/or Simon please comment. best regards Andreas Bießmann ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dlmalloc: remove manual reloc alias warning
Hi, On Tue, Aug 14, 2012 at 1:50 AM, Andreas Bießmann andreas.de...@googlemail.com wrote: Dear all, On 13.08.2012 11:02, Andreas Bießmann wrote: From: Andreas Bießmann biessm...@corscience.de The avr32 architecture (and some others) require manual relocation. Due to the previous error all avr32 boards gave warnings in MAKEALL wich makes it hard to find new warnings. This patch fixes following warning: ---8--- dlmalloc.c: In function 'malloc_bin_reloc': dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1490: note: initialized from here dlmalloc.c:1493: note: initialized from here ---8--- Signed-off-by: Andreas Bießmann biessm...@corscience.de --- A question to all the other related arches, namely m68k, mips, nds32 and sparc: Do you encounter the same warnings or is this warning due to my outdated compiler (4.4.3 currently, unfortunately atmel do not bother to send their patches mainline)? common/dlmalloc.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index c645d73..78b1885 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -1485,9 +1485,10 @@ static mbinptr av_[NAV * 2 + 2] = { }; #ifdef CONFIG_NEEDS_MANUAL_RELOC +typedef unsigned long __attribute__((__may_alias__)) ulong_aliased; void malloc_bin_reloc (void) { - unsigned long *p = (unsigned long *)(av_[2]); + ulong_aliased *p = (ulong_aliased *)(av_[2]); int i; for (i=2; i(sizeof(av_)/sizeof(mbinptr)); ++i) { *p++ += gd-reloc_off; this seems to be related to http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/121848/focus=121849 can you Wolfgang and/or Simon please comment. My only comment is that I created that patch due to errors I saw at the time, and still see with the toolchain I use for avr32. Regards, Simon best regards Andreas Bießmann ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dlmalloc: remove manual reloc alias warning
On Monday 13 August 2012 05:02:03 Andreas Bießmann wrote: From: Andreas Bießmann biessm...@corscience.de The avr32 architecture (and some others) require manual relocation. Due to the previous error all avr32 boards gave warnings in MAKEALL wich makes it hard to find new warnings. This patch fixes following warning: ---8--- dlmalloc.c: In function 'malloc_bin_reloc': dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1490: note: initialized from here dlmalloc.c:1493: note: initialized from here ---8--- Signed-off-by: Andreas Bießmann biessm...@corscience.de --- A question to all the other related arches, namely m68k, mips, nds32 and sparc: Do you encounter the same warnings or is this warning due to my outdated compiler (4.4.3 currently, unfortunately atmel do not bother to send their patches mainline)? previous thread (and i think there was one before this): http://patchwork.ozlabs.org/patch/134597/ -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dlmalloc: remove manual reloc alias warning
Dear Mike Frysinger, On 13.08.2012 16:54, Mike Frysinger wrote: On Monday 13 August 2012 05:02:03 Andreas Bießmann wrote: From: Andreas Bießmann biessm...@corscience.de The avr32 architecture (and some others) require manual relocation. Due to the previous error all avr32 boards gave warnings in MAKEALL wich makes it hard to find new warnings. This patch fixes following warning: ---8--- dlmalloc.c: In function 'malloc_bin_reloc': dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1490: note: initialized from here dlmalloc.c:1493: note: initialized from here ---8--- Signed-off-by: Andreas Bießmann biessm...@corscience.de --- A question to all the other related arches, namely m68k, mips, nds32 and sparc: Do you encounter the same warnings or is this warning due to my outdated compiler (4.4.3 currently, unfortunately atmel do not bother to send their patches mainline)? previous thread (and i think there was one before this): http://patchwork.ozlabs.org/patch/134597/ thank you for the hint. I think this is a better solution so I will test it and try to force the other patch as the correct solution. Best regards Andreas Bießmann ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dlmalloc: remove manual reloc alias warning
On 13.08.2012 17:14, Andreas Bießmann wrote: Dear Mike Frysinger, On 13.08.2012 16:54, Mike Frysinger wrote: On Monday 13 August 2012 05:02:03 Andreas Bießmann wrote: From: Andreas Bießmann biessm...@corscience.de The avr32 architecture (and some others) require manual relocation. Due to the previous error all avr32 boards gave warnings in MAKEALL wich makes it hard to find new warnings. This patch fixes following warning: ---8--- dlmalloc.c: In function 'malloc_bin_reloc': dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1493: warning: dereferencing pointer 'p' does break strict-aliasing rules dlmalloc.c:1490: note: initialized from here dlmalloc.c:1493: note: initialized from here ---8--- Signed-off-by: Andreas Bießmann biessm...@corscience.de --- A question to all the other related arches, namely m68k, mips, nds32 and sparc: Do you encounter the same warnings or is this warning due to my outdated compiler (4.4.3 currently, unfortunately atmel do not bother to send their patches mainline)? previous thread (and i think there was one before this): http://patchwork.ozlabs.org/patch/134597/ thank you for the hint. I think this is a better solution so I will test it and try to force the other patch as the correct solution. well, looks nice but is broken at runtime ;( Best regards Andreas Bießmann ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot