Message ID | 20141023131039.59826380415@gemini.denx.de |
---|---|
State | Not Applicable |
Delegated to: | Wolfgang Denk |
Headers | show |
Hello Wolfgang, 2014-10-23 15:10 GMT+02:00 Wolfgang Denk <wd@denx.de>: > Given that GCC 4.9.1 apparently solves this issue I wonder which > approach we should take? > > Should we blacklist GCC 4.8.x (and 4.9.0) like the kernel folks are > doing [1] ? > > [1] https://lkml.org/lkml/2014/10/10/272 question is: what are the gcc folks going to do with the relro stuff? It seems the sections are not used in 4.9.1, but will this be true for future releases? If we are not absolutely sure they will not be used anymore, adding them to the linker scripts is probably the best option. Anybody with deeper gcc insights following this thread? Cheers Dirk
On 23 Oct 2014, wd@denx.de wrote: > Given that GCC 4.9.1 apparently solves this issue I wonder which > approach we should take? > > Should we blacklist GCC 4.8.x (and 4.9.0) like the kernel folks are > doing [1] ? > > [1] https://lkml.org/lkml/2014/10/10/272 My understanding is this is for a completely different issue. The proposed patch is, +# if ((GCC_VERSION >= 40800 && GCC_VERSION <= 40802) || GCC_VERSION == 40900) \ + && defined(__arm__) && defined(CONFIG_FRAME_POINTER) +# error Your version of gcc miscompiles stack frames +# endif So, the proposed patch was blacklist gcc v4.8.[012] and 4.9.0 on the *ARM* when frame pointer tracing is enabled. This thread refers to a different bug where 'sp' is adjusted for a return while the 'fp' will still be references (for instance in a return calculation) above the 'sp'. It is a completely different issue than the '.data.rel.ro' which is on all architectures. Also, if you read further in the thread, it seems that 4.9.0 does not exhibit this problem. The 4.9.0 was only suggested as the 'Known to fail' of bug 58854 shows '4.9.0'. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58854 Apparently this was added during the 4.9.0 release process, but didn't make it to the official release. In any case, unless u-boot uses signals and/or an interrupt stack common to the mainline, this bug would not even affect the ARM cpus with U-boot. On 23 Oct 2014, wd@denx.de wrote: > 4) For the problemativ 4.8.x versions of GCC, the following patch > apparently solves the problem for my (MPC5200 based) board - guess > this would have to be applied to all .lds files... >diff --git a/arch/powerpc/cpu/mpc5xxx/u-boot.lds b/arch/powerpc/cpu/mpc5xxx/u-boot.lds >index cd9e23f..82c86d7 100644 >--- a/arch/powerpc/cpu/mpc5xxx/u-boot.lds >+++ b/arch/powerpc/cpu/mpc5xxx/u-boot.lds >@@ -27,6 +27,8 @@ SECTIONS > { > _GOT2_TABLE_ = .; > KEEP(*(.got2)) >+ KEEP(*(.data.rel.ro)) >+ KEEP(*(.data.rel.ro.local)) > KEEP(*(.got)) > PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); > _FIXUP_TABLE_ = .; > Given that GCC 4.9.1 apparently solves this issue I wonder which > approach we should take? From my perspective and reading the blog below this patch seems sensible. http://www.airs.com/blog/archives/189 I assume u-boot has no MMU enabled, then all the relocations should be similar. This would be for all architectures though? Fwiw, Bill Pringlemeir.
On Thu, Oct 23, 2014 at 8:10 AM, Wolfgang Denk <wd@denx.de> wrote: > Dear Joakim, dear Dirk, > > In message <OF14C3D470.864842B6-ONC1257D7A.002471AC-C1257D7A.0024DEC4@transmode.se> you wrote: >> >> Ouch, that was a nasty surprise. > > Indeed. > >> > In my original mail I referenced this potential solution, at least it >> > worked for me: >> > https://gcc.gnu.org/ml/gcc-help/2014-02/msg00054.html >> >> That looks like the correct fix but I presume both .data.rel.ro and >> data.rel.ro.local should be added? > > I can confirm: > > 1) The problem was observed with gcc 4.8.1 [as in Yocto 1.5.x / ELDK > 5.5.x]. > > 2) Switching back to gcc 4.7.2 [as in Yocto 1.4 / ELDK 5.4] makes the > problem go away. > > 3) Switching forward to gcc 4.9.1 [as in Yocto 1.7 / ELDK 5.7] makes > the problem go away. > > 4) For the problemativ 4.8.x versions of GCC, the following patch > apparently solves the problem for my (MPC5200 based) board - guess > this would have to be applied to all .lds files... > > diff --git a/arch/powerpc/cpu/mpc5xxx/u-boot.lds b/arch/powerpc/cpu/mpc5xxx/u-boot.lds > index cd9e23f..82c86d7 100644 > --- a/arch/powerpc/cpu/mpc5xxx/u-boot.lds > +++ b/arch/powerpc/cpu/mpc5xxx/u-boot.lds > @@ -27,6 +27,8 @@ SECTIONS > { > _GOT2_TABLE_ = .; > KEEP(*(.got2)) > + KEEP(*(.data.rel.ro)) > + KEEP(*(.data.rel.ro.local)) > KEEP(*(.got)) > PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); > _FIXUP_TABLE_ = .; > > Given that GCC 4.9.1 apparently solves this issue I wonder which > approach we should take? > > Should we blacklist GCC 4.8.x (and 4.9.0) like the kernel folks are > doing [1] ? > > [1] https://lkml.org/lkml/2014/10/10/272 Was there a resolution to this thread? 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. 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...
On Wednesday, September 30, 2015 at 08:24:10 PM, Andy Fleming wrote: Hi! > On Thu, Oct 23, 2014 at 8:10 AM, Wolfgang Denk <wd@denx.de> wrote: > > Dear Joakim, dear Dirk, > > > > In message <OF14C3D470.864842B6-ONC1257D7A.002471AC- C1257D7A.0024DEC4@transmode.se> you wrote: > >> Ouch, that was a nasty surprise. > > > > Indeed. > > > >> > In my original mail I referenced this potential solution, at least it > >> > worked for me: > >> > https://gcc.gnu.org/ml/gcc-help/2014-02/msg00054.html > >> > >> That looks like the correct fix but I presume both .data.rel.ro and > >> data.rel.ro.local should be added? > > > > I can confirm: > > > > 1) The problem was observed with gcc 4.8.1 [as in Yocto 1.5.x / ELDK > > > > 5.5.x]. > > > > 2) Switching back to gcc 4.7.2 [as in Yocto 1.4 / ELDK 5.4] makes the > > > > problem go away. > > > > 3) Switching forward to gcc 4.9.1 [as in Yocto 1.7 / ELDK 5.7] makes > > > > the problem go away. > > > > 4) For the problemativ 4.8.x versions of GCC, the following patch > > > > apparently solves the problem for my (MPC5200 based) board - guess > > this would have to be applied to all .lds files... > > > > diff --git a/arch/powerpc/cpu/mpc5xxx/u-boot.lds > > b/arch/powerpc/cpu/mpc5xxx/u-boot.lds index cd9e23f..82c86d7 100644 > > --- a/arch/powerpc/cpu/mpc5xxx/u-boot.lds > > +++ b/arch/powerpc/cpu/mpc5xxx/u-boot.lds > > @@ -27,6 +27,8 @@ SECTIONS > > > > { > > > > _GOT2_TABLE_ = .; > > KEEP(*(.got2)) > > > > + KEEP(*(.data.rel.ro)) > > + KEEP(*(.data.rel.ro.local)) > > > > KEEP(*(.got)) > > PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); > > _FIXUP_TABLE_ = .; > > > > Given that GCC 4.9.1 apparently solves this issue I wonder which > > approach we should take? > > > > Should we blacklist GCC 4.8.x (and 4.9.0) like the kernel folks are > > doing [1] ? > > > > [1] https://lkml.org/lkml/2014/10/10/272 > > Was there a resolution to this thread? 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. > 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... Blacklist patch please, thank you! Best regards, Marek Vasut
On Wed, 2015-09-30 at 21:35 +0200, Marek Vasut wrote: > On Wednesday, September 30, 2015 at 08:24:10 PM, Andy Fleming wrote: > > Hi! > > > On Thu, Oct 23, 2014 at 8:10 AM, Wolfgang Denk <wd@denx.de> wrote: > > > Dear Joakim, dear Dirk, > > > > > > In message <OF14C3D470.864842B6-ONC1257D7A.002471AC- > C1257D7A.0024DEC4@transmode.se> you wrote: > > > > Ouch, that was a nasty surprise. > > > > > > Indeed. > > > > > > > > In my original mail I referenced this potential solution, at least it > > > > > worked for me: > > > > > https://gcc.gnu.org/ml/gcc-help/2014-02/msg00054.html > > > > > > > > That looks like the correct fix but I presume both .data.rel.ro and > > > > data.rel.ro.local should be added? > > > > > > I can confirm: > > > > > > 1) The problem was observed with gcc 4.8.1 [as in Yocto 1.5.x / ELDK > > > > > > 5.5.x]. > > > > > > 2) Switching back to gcc 4.7.2 [as in Yocto 1.4 / ELDK 5.4] makes the > > > > > > problem go away. > > > > > > 3) Switching forward to gcc 4.9.1 [as in Yocto 1.7 / ELDK 5.7] makes > > > > > > the problem go away. > > > > > > 4) For the problemativ 4.8.x versions of GCC, the following patch > > > > > > apparently solves the problem for my (MPC5200 based) board - guess > > > this would have to be applied to all .lds files... > > > > > > diff --git a/arch/powerpc/cpu/mpc5xxx/u-boot.lds > > > b/arch/powerpc/cpu/mpc5xxx/u-boot.lds index cd9e23f..82c86d7 100644 > > > --- a/arch/powerpc/cpu/mpc5xxx/u-boot.lds > > > +++ b/arch/powerpc/cpu/mpc5xxx/u-boot.lds > > > @@ -27,6 +27,8 @@ SECTIONS > > > > > > { > > > > > > _GOT2_TABLE_ = .; > > > KEEP(*(.got2)) > > > > > > + KEEP(*(.data.rel.ro)) > > > + KEEP(*(.data.rel.ro.local)) > > > > > > KEEP(*(.got)) > > > PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); > > > _FIXUP_TABLE_ = .; > > > > > > Given that GCC 4.9.1 apparently solves this issue I wonder which > > > approach we should take? > > > > > > Should we blacklist GCC 4.8.x (and 4.9.0) like the kernel folks are > > > doing [1] ? > > > > > > [1] https://lkml.org/lkml/2014/10/10/272 > > > > Was there a resolution to this thread? 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. > > 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... > > Blacklist patch please, thank you! Yes, but all gcc 4.8.x versions? There is a fix here https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01679.html but I don't know if it got committed or not or which version. I am using gcc 4.8.4 and it works but I have one problem, if I erase uboot after relocation, u-boot misbehavex or crashes so there is something off still. Does it work for all but me to erase u-boot after relocation? Using T1040(mpc85xx family) Jocke
Dear Andy, In message <CAKWjMd71hXW9-ahCUQm_rWS6z5Srd3My4TJ4x3KDRTdsHXHb=A@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... > 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. 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? Best regards, Wolfgang Denk
On Thu, 2015-10-01 at 08:57 +0000, Joakim Tjernlund wrote: > On Wed, 2015-09-30 at 21:35 +0200, Marek Vasut wrote: > > On Wednesday, September 30, 2015 at 08:24:10 PM, Andy Fleming wrote: > > > > Hi! > > > > > On Thu, Oct 23, 2014 at 8:10 AM, Wolfgang Denk <wd@denx.de> wrote: > > > > Dear Joakim, dear Dirk, > > > > > > > > In message <OF14C3D470.864842B6-ONC1257D7A.002471AC- > > C1257D7A.0024DEC4@transmode.se> you wrote: > > > > > Ouch, that was a nasty surprise. > > > > > > > > Indeed. > > > > > > > > > > In my original mail I referenced this potential solution, at least it > > > > > > worked for me: > > > > > > https://gcc.gnu.org/ml/gcc-help/2014-02/msg00054.html > > > > > > > > > > That looks like the correct fix but I presume both .data.rel.ro and > > > > > data.rel.ro.local should be added? > > > > > > > > I can confirm: > > > > > > > > 1) The problem was observed with gcc 4.8.1 [as in Yocto 1.5.x / ELDK > > > > > > > > 5.5.x]. > > > > > > > > 2) Switching back to gcc 4.7.2 [as in Yocto 1.4 / ELDK 5.4] makes the > > > > > > > > problem go away. > > > > > > > > 3) Switching forward to gcc 4.9.1 [as in Yocto 1.7 / ELDK 5.7] makes > > > > > > > > the problem go away. > > > > > > > > 4) For the problemativ 4.8.x versions of GCC, the following patch > > > > > > > > apparently solves the problem for my (MPC5200 based) board - guess > > > > this would have to be applied to all .lds files... > > > > > > > > diff --git a/arch/powerpc/cpu/mpc5xxx/u-boot.lds > > > > b/arch/powerpc/cpu/mpc5xxx/u-boot.lds index cd9e23f..82c86d7 100644 > > > > --- a/arch/powerpc/cpu/mpc5xxx/u-boot.lds > > > > +++ b/arch/powerpc/cpu/mpc5xxx/u-boot.lds > > > > @@ -27,6 +27,8 @@ SECTIONS > > > > > > > > { > > > > > > > > _GOT2_TABLE_ = .; > > > > KEEP(*(.got2)) > > > > > > > > + KEEP(*(.data.rel.ro)) > > > > + KEEP(*(.data.rel.ro.local)) > > > > > > > > KEEP(*(.got)) > > > > PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); > > > > _FIXUP_TABLE_ = .; > > > > > > > > Given that GCC 4.9.1 apparently solves this issue I wonder which > > > > approach we should take? > > > > > > > > Should we blacklist GCC 4.8.x (and 4.9.0) like the kernel folks are > > > > doing [1] ? > > > > > > > > [1] https://lkml.org/lkml/2014/10/10/272 > > > > > > Was there a resolution to this thread? 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. > > > 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... > > > > Blacklist patch please, thank you! > > Yes, but all gcc 4.8.x versions? > > There is a fix here > https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01679.html > but I don't know if it got committed or not or which version. > > I am using gcc 4.8.4 and it works but I have one problem, if I erase uboot > after relocation, u-boot misbehavex or crashes so there is something off still. > > Does it work for all but me to erase u-boot after relocation? > Using T1040(mpc85xx family) Here is a better URL: http://patchwork.ozlabs.org/patch/342888/ From what I can tell the above bug has been fixed in gcc 4.8.5(4.8.4 has the error) and 4.9.3 (by looking at varasm.c). Adding KEEP(*(.data.rel.ro.local)) i u-boot.lds does not seem to be the correct fix as it is not an .fixup entry? Jocke
On Tue, 2015-10-06 at 11:17 +0000, Joakim Tjernlund wrote: > On Thu, 2015-10-01 at 08:57 +0000, Joakim Tjernlund wrote: > > On Wed, 2015-09-30 at 21:35 +0200, Marek Vasut wrote: > > > On Wednesday, September 30, 2015 at 08:24:10 PM, Andy Fleming wrote: > > > > > > Hi! > > > > > > > On Thu, Oct 23, 2014 at 8:10 AM, Wolfgang Denk <wd@denx.de> wrote: > > > > > Dear Joakim, dear Dirk, > > > > > > > > > > In message <OF14C3D470.864842B6-ONC1257D7A.002471AC- > > > C1257D7A.0024DEC4@transmode.se> you wrote: > > > > > > Ouch, that was a nasty surprise. > > > > > > > > > > Indeed. > > > > > > > > > > > > In my original mail I referenced this potential solution, at least it > > > > > > > worked for me: > > > > > > > https://gcc.gnu.org/ml/gcc-help/2014-02/msg00054.html > > > > > > > > > > > > That looks like the correct fix but I presume both .data.rel.ro and > > > > > > data.rel.ro.local should be added? > > > > > > > > > > I can confirm: > > > > > > > > > > 1) The problem was observed with gcc 4.8.1 [as in Yocto 1.5.x / ELDK > > > > > > > > > > 5.5.x]. > > > > > > > > > > 2) Switching back to gcc 4.7.2 [as in Yocto 1.4 / ELDK 5.4] makes the > > > > > > > > > > problem go away. > > > > > > > > > > 3) Switching forward to gcc 4.9.1 [as in Yocto 1.7 / ELDK 5.7] makes > > > > > > > > > > the problem go away. > > > > > > > > > > 4) For the problemativ 4.8.x versions of GCC, the following patch > > > > > > > > > > apparently solves the problem for my (MPC5200 based) board - guess > > > > > this would have to be applied to all .lds files... > > > > > > > > > > diff --git a/arch/powerpc/cpu/mpc5xxx/u-boot.lds > > > > > b/arch/powerpc/cpu/mpc5xxx/u-boot.lds index cd9e23f..82c86d7 100644 > > > > > --- a/arch/powerpc/cpu/mpc5xxx/u-boot.lds > > > > > +++ b/arch/powerpc/cpu/mpc5xxx/u-boot.lds > > > > > @@ -27,6 +27,8 @@ SECTIONS > > > > > > > > > > { > > > > > > > > > > _GOT2_TABLE_ = .; > > > > > KEEP(*(.got2)) > > > > > > > > > > + KEEP(*(.data.rel.ro)) > > > > > + KEEP(*(.data.rel.ro.local)) > > > > > > > > > > KEEP(*(.got)) > > > > > PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); > > > > > _FIXUP_TABLE_ = .; > > > > > > > > > > Given that GCC 4.9.1 apparently solves this issue I wonder which > > > > > approach we should take? > > > > > > > > > > Should we blacklist GCC 4.8.x (and 4.9.0) like the kernel folks are > > > > > doing [1] ? > > > > > > > > > > [1] https://lkml.org/lkml/2014/10/10/272 > > > > > > > > Was there a resolution to this thread? 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. > > > > 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... > > > > > > Blacklist patch please, thank you! > > > > Yes, but all gcc 4.8.x versions? > > > > There is a fix here > > https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01679.html > > but I don't know if it got committed or not or which version. > > > > I am using gcc 4.8.4 and it works but I have one problem, if I erase uboot > > after relocation, u-boot misbehavex or crashes so there is something off still. > > > > Does it work for all but me to erase u-boot after relocation? > > Using T1040(mpc85xx family) > > Here is a better URL: > http://patchwork.ozlabs.org/patch/342888/ > > From what I can tell the above bug has been fixed in gcc 4.8.5(4.8.4 has the error) > and 4.9.3 (by looking at varasm.c). > > Adding KEEP(*(.data.rel.ro.local)) i u-boot.lds does not seem to be the > correct fix as it is not an .fixup entry? After upgrading to gcc 4.9.3 I still see this bug(there is no .fixup entry) The bug can be avoided with -fno-ira-hoist-pressure and while you are it, throw in -mbss-plt to reduce size Jocke
diff --git a/arch/powerpc/cpu/mpc5xxx/u-boot.lds b/arch/powerpc/cpu/mpc5xxx/u-boot.lds index cd9e23f..82c86d7 100644 --- a/arch/powerpc/cpu/mpc5xxx/u-boot.lds +++ b/arch/powerpc/cpu/mpc5xxx/u-boot.lds @@ -27,6 +27,8 @@ SECTIONS { _GOT2_TABLE_ = .; KEEP(*(.got2)) + KEEP(*(.data.rel.ro)) + KEEP(*(.data.rel.ro.local)) KEEP(*(.got)) PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); _FIXUP_TABLE_ = .;