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

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

[Andreas Bießmann]

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;
-- 
1.7.10.4

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

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120813/41eee298/attachment.pgp>

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

[Andreas Bießmann]

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

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

[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
> 
> 

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

[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

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

[Simon Glass]

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

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

[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

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

[Wolfgang Denk]

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

-- 
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: wd at 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] [PATCH] dlmalloc: remove manual reloc alias warning

[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

-- 
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: wd at denx.de
All your people must learn before you can reach for the stars.
	-- Kirk, "The Gamesters of Triskelion", stardate 3259.2

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

[Andreas Bießmann]

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
>