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

Andreas Bießmann
2010-11-30T11:48:41Z

Dear Joakim Tjernlund,
Dear Albert ARIBAUD,

Am 30.11.2010 10:41, schrieb Joakim Tjernlund:
> Albert ARIBAUD <albert@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@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?

regards

Andreas Bie?mann