| Message ID | 1344848523-20359-1-git-send-email-andreas.devel@googlemail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Monday 13 August 2012 05:02:03 Andreas Bießmann wrote: > From: Andreas Bießmann <biessmann@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 <biessmann@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
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 <biessmann@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 <biessmann@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
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 <biessmann@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 <biessmann@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 > >
Dear all, On 13.08.2012 11:02, Andreas Bießmann wrote: > From: Andreas Bießmann <biessmann@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 <biessmann@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
Hi, On Tue, Aug 14, 2012 at 1:50 AM, Andreas Bießmann <andreas.devel@googlemail.com> wrote: > Dear all, > > On 13.08.2012 11:02, Andreas Bießmann wrote: >> From: Andreas Bießmann <biessmann@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 <biessmann@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
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.devel@googlemail.com> wrote: >> Dear all, >> >> On 13.08.2012 11:02, Andreas Bießmann wrote: >>> From: Andreas Bießmann <biessmann@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 <biessmann@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
Dear =?UTF-8?q?Andreas=20Bie=C3=9Fmann?=, In message <1344848523-20359-1-git-send-email-andreas.devel@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
Dear Andreas, In message <20120901101031.2DA15205615@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
Dera Wolfgang Denk, On 01.09.2012 14:02, Wolfgang Denk wrote: > Dear Andreas, > > In message <20120901101031.2DA15205615@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 >
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;