| Message ID | 4D22DFBE.8090503@samsung.com |
|---|---|
| State | Changes Requested |
| Headers | show |
> > Need to check that location is valid on RAM before the fixup. Why this change? The [PATCH RFC] armv7: fixloop: don't fixup if location is NULL should be what you need. You could mention what causes these NULL fixups, here is commit entry for powerpc: powerpc: do not fixup NULL ptrs The fixup routine must not fixup NULL pointers. Problem can be seen by char *testfun(void) __attribute__((weak)); char *(*myfun)(void) = testfun; Then add printf("myfun:%p, &myfun:%p\n", myfun, &myfun); before relocation and after relocation. myfun should be NULL in both cases but it is not.
Dear Joakim Tjernlund, On 4 January 2011 18:49, Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: >> >> Need to check that location is valid on RAM before the fixup. > > Why this change? The [PATCH RFC] armv7: fixloop: don't fixup if location is NULL > should be what you need. Some bss variables are initialized to 0xffffffff in my test case. In this case we should skip the fixup too. So, I make v2 patch for check it. Thanks Minkyu Kang
Minkyu Kang <promsoft@gmail.com> wrote on 2011/01/04 11:04:57: > > Dear Joakim Tjernlund, > > On 4 January 2011 18:49, Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > >> > >> Need to check that location is valid on RAM before the fixup. > > > > Why this change? The [PATCH RFC] armv7: fixloop: don't fixup if location is NULL > > should be what you need. > > Some bss variables are initialized to 0xffffffff in my test case. > In this case we should skip the fixup too. > So, I make v2 patch for check it. 0xffffffff? What variables? Feels like a some other bug to me.
Dear Joakim Tjernlund, On 4 January 2011 19:31, Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > Minkyu Kang <promsoft@gmail.com> wrote on 2011/01/04 11:04:57: >> >> Dear Joakim Tjernlund, >> >> On 4 January 2011 18:49, Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: >> >> >> >> Need to check that location is valid on RAM before the fixup. >> > >> > Why this change? The [PATCH RFC] armv7: fixloop: don't fixup if location is NULL >> > should be what you need. >> >> Some bss variables are initialized to 0xffffffff in my test case. >> In this case we should skip the fixup too. >> So, I make v2 patch for check it. > > 0xffffffff? What variables? Feels like a some other bug to me. It's DRAM initial variable. Thanks Minkyu Kang
Minkyu Kang <promsoft@gmail.com> wrote on 2011/01/04 12:02:29: > > Dear Joakim Tjernlund, > > On 4 January 2011 19:31, Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > > Minkyu Kang <promsoft@gmail.com> wrote on 2011/01/04 11:04:57: > >> > >> Dear Joakim Tjernlund, > >> > >> On 4 January 2011 18:49, Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > >> >> > >> >> Need to check that location is valid on RAM before the fixup. > >> > > >> > Why this change? The [PATCH RFC] armv7: fixloop: don't fixup if location is NULL > >> > should be what you need. > >> > >> Some bss variables are initialized to 0xffffffff in my test case. > >> In this case we should skip the fixup too. > >> So, I make v2 patch for check it. > > > > 0xffffffff? What variables? Feels like a some other bug to me. > > It's DRAM initial variable. what does that mean? initial value of the whole DRAM? With such low level of detail one cannot do much and now I am travelling a few days so no help from me. In either case it feels like another bug and playing games with fixup isn't the right solution. Jocke
(sending again, this time to the list) Le 04/01/2011 12:02, Minkyu Kang a écrit : >>> Some bss variables are initialized to 0xffffffff in my test case. >>> In this case we should skip the fixup too. >>> So, I make v2 patch for check it. >> >> 0xffffffff? What variables? Feels like a some other bug to me. > > It's DRAM initial variable. Yes, seems like an unrelated issue to me, or at least an unexplained one so far. For instance, there is no fixup to BSS variables (they are to be initialized to 0 whatever the load address is, and this initialization is not in the binary, it is done in the startup code). So which fixup should be skipped exactly and how is is related to the BSS variable? > Thanks > Minkyu Kang Amicalement,
On 5 January 2011 02:02, Albert ARIBAUD <albert.aribaud@free.fr> wrote: > (sending again, this time to the list) > > Le 04/01/2011 12:02, Minkyu Kang a écrit : > >>>> Some bss variables are initialized to 0xffffffff in my test case. >>>> In this case we should skip the fixup too. >>>> So, I make v2 patch for check it. >>> >>> 0xffffffff? What variables? Feels like a some other bug to me. >> >> It's DRAM initial variable. > > Yes, seems like an unrelated issue to me, or at least an unexplained one > so far. > > For instance, there is no fixup to BSS variables (they are to be > initialized to 0 whatever the load address is, and this initialization > is not in the binary, it is done in the startup code). So which fixup > should be skipped exactly and how is is related to the BSS variable? > Yes, you right. Seems to another bug. It sometimes happens at arrays on BSS area. OK, I'll investigate this problem. How about v1 patch for NULL pointer. Do you agreed with this patch? http://patchwork.ozlabs.org/patch/76784/ If so, I'm going to make a patch for all of ARM CPUs. Thanks Minkyu Kang
Hi, Le 05/01/2011 06:27, Minkyu Kang a écrit : > How about v1 patch for NULL pointer. > Do you agreed with this patch? > http://patchwork.ozlabs.org/patch/76784/ This one I'm okay with; please repost as a non-RFC, simple, [PATCH]. > If so, I'm going to make a patch for all of ARM CPUs. That would be great. :) > Thanks > Minkyu Kang Amicalement,
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 684f2d2..a052378 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -195,6 +195,11 @@ copy_loop: add r3, r3, r0 /* r3 <- rel dyn end in FLASH */ fixloop: ldr r0, [r2] /* r0 <- location to fix up, IN FLASH! */ + ldr r7, _TEXT_BASE + cmp r0, r7 + blo fixskip + cmp r0, r6 + bhs fixskip add r0, r0, r9 /* r0 <- location to fix up in RAM */ ldr r1, [r2, #4] and r7, r1, #0xff @@ -217,6 +222,7 @@ fixrel: add r1, r1, r9 fixnext: str r1, [r0] +fixskip: add r2, r2, #8 /* each rel.dyn entry is 8 bytes */ cmp r2, r3 blo fixloop
Need to check that location is valid on RAM before the fixup. Signed-off-by: Minkyu Kang <mk7.kang@samsung.com> --- arch/arm/cpu/armv7/start.S | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)