[U-Boot] [PATCH RFC 3/3] arm920t: do not relocate NULL pointer

[Joakim Tjernlund]

"Andreas Bießmann" <andreas.devel at googlemail.com> wrote on 2010/11/30 12:48:41:
>
> Dear Joakim Tjernlund,
> Dear Albert ARIBAUD,
>
> Am 30.11.2010 10:41, schrieb Joakim Tjernlund:
> > Albert ARIBAUD <albert at aribaud.net> wrote on 2010/11/30 10:02:45:
> >>
> >> Le 30/11/2010 09:47, Joakim Tjernlund a écrit :
> >>>>
> >>>> Le 30/11/2010 08:06, Andreas Bießmann a écrit :
> >>>>> Signed-off-by: Andreas Bießmann<andreas.devel at googlemail.com>
> >>>>
> >>>>> +   cmp   r1, #0         /* symbol == NULL ? */
> >>>>> +   beq   fixnext
> >>>>
> >>>> Nak. Don't hide a null pointer. NULL pointers are *not* relocated, since
> >>>> they are a constant. If a NULL ends up in relocation tables, that is
> >>>> because of a corruption *or* because it was to be relocated, and should
> >>>> thus never be ignored.
> >>>
> >>> Depends, if the same routine is used for relocating fixups you need
> >>> this test. Undefined weaks will generate a NULL fixup entry.
> >>
> >> Understood.
> >
> > note that I don't know how this routine is used so if just
> > relocates the GOT you don't need to test for NULL.
> >
> >>
> >> Weren't there an effort to not use weak symbols any more?
> >
> > ehh, not what I am aware of. Just that we should not use(ATM)
> > undefined weaks.
>
> Ok, I did forget to investigate that as mentioned in
> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88024/focus=88324
>
> Here are the results:
>
> Currently arm920/at91 devices have one undefined weak symbol in .dynsym:
>
> ---8<---
> Symbol table '.dynsym' contains 11 entries:
>    Num:    Value  Size Type    Bind   Vis      Ndx Name
>      0: 00000000     0 NOTYPE  LOCAL  DEFAULT  UND <corrupt>
>      1: 10000000     0 SECTION LOCAL  DEFAULT    1 <corrupt>
>      2: 10028a28     0 SECTION LOCAL  DEFAULT    6 <corrupt>
>      3: 10029ff8     0 NOTYPE  GLOBAL DEFAULT    9 <corrupt>
>      4: 100299f4     0 NOTYPE  GLOBAL DEFAULT  ABS <corrupt>
>      5: 00000000     0 NOTYPE  WEAK   DEFAULT  UND <corrupt>
>      6: 10029ff8     0 NOTYPE  GLOBAL DEFAULT  ABS <corrupt>
>      7: 10029ff8     0 NOTYPE  GLOBAL DEFAULT   11 <corrupt>
>      8: 1002edf0     0 NOTYPE  GLOBAL DEFAULT   10 <corrupt>
>      9: 100701c0     0 NOTYPE  GLOBAL DEFAULT   11 <corrupt>
>     10: 1002edf0     0 NOTYPE  GLOBAL DEFAULT    9 <corrupt>
> --->8---
>
> ---8<---
> Hex dump of section '.dynsym':
>   0x1002edf0 00000000 00000000 00000000 00000000 ................
>   0x1002ee00 00000000 00000010 00000000 03000100 ................
>   0x1002ee10 00000000 288a0210 00000000 03000600 ....(...........
>   0x1002ee20 25000000 f89f0210 00000000 10000900 %...............
>   0x1002ee30 01000000 f4990210 00000000 1000f1ff ................
>   0x1002ee40 5e000000 00000000 00000000 20000000 ^........... ...
>   0x1002ee50 14000000 f89f0210 00000000 1000f1ff ................
>   0x1002ee60 52000000 f89f0210 00000000 10000b00 R...............
>   0x1002ee70 43000000 f0ed0210 00000000 10000a00 C...............
>   0x1002ee80 20000000 c0010710 00000000 10000b00  ...............
>   0x1002ee90 35000000 f0ed0210 00000000 10000900 5...............
> --->8---
>
> So there are two options here.
>
> First is to apply '[PATCH v2 1/2] arm920t/at91/reset.c: fix weak
> reset_board()' second (and safer) is to apply (maybe modified) '[PATCH
> RFC 3/3] arm920t: do not relocate NULL pointer'.
>
> which one is preferred? Or should we do both?

Both, you don't know what the future will be and perhaps someone
wants to use undef weaks in board code.

   Jocke