Re: [U-Boot] Bug introduced in x86 cleanup patches

2011-11-15 Thread Wolfgang Denk
Dear Graeme,

In message calbutcjfyqwb4ttf_nwjgk6haceeb1pntqzufk_npxwts3c...@mail.gmail.com 
you wrote:
 
 My recent x86 cleanup added a small, but very nasty, bug at line 231 of
 arch/x86/lib/board.c:
 
   offset_ptr_ram = offset_ptr_rom + gd-reloc_off
 
 Because offset_ptr_rom is a pointer, when gd-reloc_off gets added, there
 is a silent 4x multiplication. The solution is (tested):
 
   offset_ptr_ram = (Elf32_Rel *)((ulong)offset_ptr_rom
   + gd-reloc_off);
 
 Or (haven't tested - will test tonight):
 
   offset_ptr_ram = offset_ptr_rom + (Elf32_Rel *)gd-reloc_off;
 
 I have two options
   - Fix it in the existing commit. As it has not been pulled into
 u-boot/master yet, distribution is likely limited to yourself only
   - Add a fixup patch

As far as I'm concerned, I'm OK with a fix in the existing commit and
a rebase of your tree.

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
Success in marriage is not so much finding the right person as it  is
being the right person.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] Bug introduced in x86 cleanup patches

2011-11-14 Thread Graeme Russ
Hi Wolfgang, Gabe,

My recent x86 cleanup added a small, but very nasty, bug at line 231 of
arch/x86/lib/board.c:

offset_ptr_ram = offset_ptr_rom + gd-reloc_off

Because offset_ptr_rom is a pointer, when gd-reloc_off gets added, there
is a silent 4x multiplication. The solution is (tested):

offset_ptr_ram = (Elf32_Rel *)((ulong)offset_ptr_rom
+ gd-reloc_off);

Or (haven't tested - will test tonight):

offset_ptr_ram = offset_ptr_rom + (Elf32_Rel *)gd-reloc_off;

I have two options
  - Fix it in the existing commit. As it has not been pulled into
u-boot/master yet, distribution is likely limited to yourself only
  - Add a fixup patch

Thoughts?

Regards,

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


Re: [U-Boot] Bug introduced in x86 cleanup patches

2011-11-14 Thread Gabe Black
On Mon, Nov 14, 2011 at 2:10 PM, Graeme Russ graeme.r...@gmail.com wrote:

 Hi Wolfgang, Gabe,

 My recent x86 cleanup added a small, but very nasty, bug at line 231 of
 arch/x86/lib/board.c:

offset_ptr_ram = offset_ptr_rom + gd-reloc_off

 Because offset_ptr_rom is a pointer, when gd-reloc_off gets added, there
 is a silent 4x multiplication. The solution is (tested):

offset_ptr_ram = (Elf32_Rel *)((ulong)offset_ptr_rom
+ gd-reloc_off);

 Or (haven't tested - will test tonight):

offset_ptr_ram = offset_ptr_rom + (Elf32_Rel *)gd-reloc_off;

 I have two options
  - Fix it in the existing commit. As it has not been pulled into
u-boot/master yet, distribution is likely limited to yourself only
  - Add a fixup patch

 Thoughts?

 Regards,

 Graeme



I think the second one is either illegal or depends on undefined behavior.
I don't think you can add two pointers like that. The first should work,
though. Ironically I introduced this same bug in our tree a while ago and
fixed it in a separate patch. They were folded together when I sent them
upstream.

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