[U-Boot] dlmalloc: remove manual reloc alias warning
diff mbox

Message ID 1344848523-20359-1-git-send-email-andreas.devel@googlemail.com
State Superseded
Headers show

Commit Message

Andreas Bießmann Aug. 13, 2012, 9:02 a.m. UTC
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(-)

Comments

Mike Frysinger Aug. 13, 2012, 2:54 p.m. UTC | #1
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
Andreas Bießmann Aug. 13, 2012, 3:14 p.m. UTC | #2
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
Andreas Bießmann Aug. 13, 2012, 3:31 p.m. UTC | #3
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
> 
>
Andreas Bießmann Aug. 14, 2012, 8:50 a.m. UTC | #4
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
Simon Glass Aug. 15, 2012, 12:15 a.m. UTC | #5
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
Andreas Bießmann Aug. 15, 2012, 6:29 a.m. UTC | #6
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
Wolfgang Denk Sept. 1, 2012, 10:10 a.m. UTC | #7
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
Wolfgang Denk Sept. 1, 2012, 12:02 p.m. UTC | #8
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
Andreas Bießmann Sept. 1, 2012, 1:20 p.m. UTC | #9
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
>

Patch
diff mbox

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;