[U-Boot] Relocation issue - need help!

[Andy Fleming]

On Thu, Oct 1, 2015 at 9:18 AM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Andy,
>
> In message <CAKWjMd71hXW9-ahCUQm_rWS6z5Srd3My4TJ4x3KDRTdsHXHb=A at mail.gmail.com> you wrote:
>>
>> > Given that GCC 4.9.1 apparently solves this issue I wonder which
>> > approach we should take?
> ...
>> Was there a resolution to this thread?
>
> No, there was no real resolution.
>
> My understanding was that there was an (unwritten) agreement that we
> should avoid the broken GCC versions and switch to GCC 4.9.1 or later.
>
>> I just spent a bunch of time
>> trying to figure out why u-boot was crashing, and eventually
>> determined that switching from 4.9.0 to 4.6.3 solved the problem.
>
> Switching to GCC 4.9.1 is also supposed to work...

I haven't tried GCC 4.9.1, but I can now report that 4.6.3 didn't
totally work. There was a constant string whose address was not being
relocated. The result was that when I tried to boot, an sprintf
crashed while trying to find its "%s:%d" at the unrelocated address.
My familiarity with the relocation code is pretty weak, so I'm not
sure if there's a bug in the code or in the compiler.

However, I can note that adding the extra KEEP lines to the lds made
the crash go away:

--- a/arch/powerpc/cpu/mpc85xx/u-boot.lds
+++ b/arch/powerpc/cpu/mpc85xx/u-boot.lds
@@ -49,6 +49,8 @@ SECTIONS
   {
     _GOT2_TABLE_ = .;
     KEEP(*(.got2))
+    KEEP(*(.data.rel.ro))
+    KEEP(*(.data.rel.ro.local))
     KEEP(*(.got))
     PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4);
     _FIXUP_TABLE_ = .;


>
>> Should I submit a patch to do what was suggested above? Or add the
>> "blacklist" patch? If so, it should be noted that 4.9.0 is the current
>> default installed when you ask buildman to install a powerpc cross
>> compiler...
>
> In my opinion, we should blacklist the versions.
>
> But I am well aware that this would upset quite a lot of people as
> their favorite toolchains would stop working, and not everybody is
> really flexible here.

I'd be fine with blacklisting, but if we blacklist 4.9.0, it shouldn't
be the compiler that gets installed when you run buildman --fetch-arch
powerpc.

>
> So perhaps a patch would be better - but how to test this? It is
> probably not sufficient to fix the problem in
> arch/powerpc/cpu/mpc5xxx/u-boot.lds - all the other linker sripts in
> arch/powerpc/ look the same, so they probably need fixing, too.  And
> what about the other architectures?  Should we fix ALL linker
> scripts, even if we have no indication of breakage?

At the very least, I would want the fix in
arch/powerpc/cpu/mpc85xx/u-boot.lds. :)

That said, because the problem also exists in 4.6.3 (though it does
happen in a different place for me with that compiler), I worry that
there's a bug in powerpc relocation. It occurs to me that many systems
would not notice if some read-only values were read from flash instead
of DDR. The system I'm working on keeps its image on an SD card. Also,
the image size for this system is 768K instead of 512K, and I believe
that's mostly because of the data section.

I'm going to dig into it a bit more, but I'm confident the
blacklisting isn't enough.

Andy