| Message ID | 1503396068-26205-2-git-send-email-kevin@darbyshire-bryant.me.uk |
|---|---|
| State | Rejected |
| Headers | show |
> On Aug 22, 2017, at 4:01 AM, Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> wrote: > > Drop 300-mips_Os_cpu_rtx_cost_model.patch for gcc 7.2 > > This was causing mis-compilation of dropbear with the default '-Os' size > optimization as reported in FS#814 > > Tested on ar71xx, archer C7 v2. For size comparison of my whole build: > > 12058628 O2-withoutpatch-dropbearworks.bin > 12058628 O2-withpatch-dropbearworks.bin > 11468804 Os-withoutpatch-dropbearworks.bin > 11468804 Os-withpatch-dropbearfails.bin > > Signed-off-by: Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> > --- > .../7.2.0/300-mips_Os_cpu_rtx_cost_model.patch | 21 --------------------- > 1 file changed, 21 deletions(-) > delete mode 100644 toolchain/gcc/patches/7.2.0/300-mips_Os_cpu_rtx_cost_model.patch > > diff --git a/toolchain/gcc/patches/7.2.0/300-mips_Os_cpu_rtx_cost_model.patch b/toolchain/gcc/patches/7.2.0/300-mips_Os_cpu_rtx_cost_model.patch > deleted file mode 100644 > index 84c0fda..0000000 > --- a/toolchain/gcc/patches/7.2.0/300-mips_Os_cpu_rtx_cost_model.patch > +++ /dev/null > @@ -1,21 +0,0 @@ > -commit ecf7671b769fe96f7b5134be442089f8bdba55d2 > -Author: Felix Fietkau <nbd@nbd.name> > -Date: Thu Aug 4 20:29:45 2016 +0200 > - > -gcc: add a patch to generate better code with Os on mips > - > -Also happens to reduce compressed code size a bit > - > -Signed-off-by: Felix Fietkau <nbd@nbd.name> > - > ---- a/gcc/config/mips/mips.c > -+++ b/gcc/config/mips/mips.c > -@@ -19784,7 +19784,7 @@ mips_option_override (void) > - flag_pcc_struct_return = 0; > - > - /* Decide which rtx_costs structure to use. */ > -- if (optimize_size) > -+ if (0 && optimize_size) > - mips_cost = &mips_rtx_cost_optimize_size; > - else > - mips_cost = &mips_rtx_cost_data[mips_tune]; > -- > 2.7.4 LGTM
On 2017-08-22 12:01, Kevin Darbyshire-Bryant wrote: > Drop 300-mips_Os_cpu_rtx_cost_model.patch for gcc 7.2 > > This was causing mis-compilation of dropbear with the default '-Os' size > optimization as reported in FS#814 > > Tested on ar71xx, archer C7 v2. For size comparison of my whole build: > > 12058628 O2-withoutpatch-dropbearworks.bin > 12058628 O2-withpatch-dropbearworks.bin > 11468804 Os-withoutpatch-dropbearworks.bin > 11468804 Os-withpatch-dropbearfails.bin > > Signed-off-by: Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> I strongly suspect that this change is hiding the real bug instead of fixing it. Please double-check that the mis-compilation also does not happen with -O2 instead of -Os. - Felix
On 23/08/17 09:20, Felix Fietkau wrote: > On 2017-08-22 12:01, Kevin Darbyshire-Bryant wrote: >> Drop 300-mips_Os_cpu_rtx_cost_model.patch for gcc 7.2 >> >> This was causing mis-compilation of dropbear with the default '-Os' size >> optimization as reported in FS#814 >> >> Tested on ar71xx, archer C7 v2. For size comparison of my whole build: >> >> 12058628 O2-withoutpatch-dropbearworks.bin >> 12058628 O2-withpatch-dropbearworks.bin >> 11468804 Os-withoutpatch-dropbearworks.bin >> 11468804 Os-withpatch-dropbearfails.bin >> >> Signed-off-by: Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> > I strongly suspect that this change is hiding the real bug instead of > fixing it. Please double-check that the mis-compilation also does not > happen with -O2 instead of -Os. Hi Felix, The symptom of dropbear not responding (it goes into a tight loop) *only* occurs for me with the patch installed and with '-Os'. As documented in the FS report, starting with gcc 7.1, dropbear (and for some uhttpd) go AWOL when built with '-Os'. Initially I did not experience that issue because I always build with '-O2', however by switching to '-Os' I was able to reproduce the behaviour. As part of my bump to gcc 7.2 and 'cargo culting/refreshing' the 7.1 patches across, I thought I would investigate if the same erroneous behaviour existed - it did. So questions: Why MIPS only, why only with '-Os' and not '-O2', why is no one else screaming about this? Many experiments and 'make dirclean' (to ensure gcc and the whole router image rebuilt) later I reached my conclusions with 300-mips_Os_cpu_rtx_cost_model.patch. What I have not done is check to see if removal of 300-mips_Os_cpu_rtx_cost_model.patch on gcc 7.1 solves the problem there, it could be that gcc 7.1 also had a bug. Cheers, Kevin > > - Felix >
Hi, Based on my test, it works well even if you keep this mips patch, upstream already update libtommath and libtomcrypt to latest, and I can confirm the update to these two libs fixes dropbear misbehavior on mips. https://github.com/mkj/dropbear/commit/a79b61517bc7123250d0e2dc21dc18deccf0bb64 https://github.com/mkj/dropbear/commit/364fb6019c1931de3d181f21ea491ec112161577 Best Regards, Syrone Wong On Wed, Aug 23, 2017 at 4:50 PM, Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> wrote: > > > On 23/08/17 09:20, Felix Fietkau wrote: >> >> On 2017-08-22 12:01, Kevin Darbyshire-Bryant wrote: >>> >>> Drop 300-mips_Os_cpu_rtx_cost_model.patch for gcc 7.2 >>> >>> This was causing mis-compilation of dropbear with the default '-Os' size >>> optimization as reported in FS#814 >>> >>> Tested on ar71xx, archer C7 v2. For size comparison of my whole build: >>> >>> 12058628 O2-withoutpatch-dropbearworks.bin >>> 12058628 O2-withpatch-dropbearworks.bin >>> 11468804 Os-withoutpatch-dropbearworks.bin >>> 11468804 Os-withpatch-dropbearfails.bin >>> >>> Signed-off-by: Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> >> >> I strongly suspect that this change is hiding the real bug instead of >> fixing it. Please double-check that the mis-compilation also does not >> happen with -O2 instead of -Os. > > > Hi Felix, > > The symptom of dropbear not responding (it goes into a tight loop) *only* > occurs for me with the patch installed and with '-Os'. As documented in the > FS report, starting with gcc 7.1, dropbear (and for some uhttpd) go AWOL > when built with '-Os'. Initially I did not experience that issue because I > always build with '-O2', however by switching to '-Os' I was able to > reproduce the behaviour. > > As part of my bump to gcc 7.2 and 'cargo culting/refreshing' the 7.1 patches > across, I thought I would investigate if the same erroneous behaviour > existed - it did. So questions: Why MIPS only, why only with '-Os' and not > '-O2', why is no one else screaming about this? Many experiments and 'make > dirclean' (to ensure gcc and the whole router image rebuilt) later I reached > my conclusions with 300-mips_Os_cpu_rtx_cost_model.patch. > > What I have not done is check to see if removal of > 300-mips_Os_cpu_rtx_cost_model.patch on gcc 7.1 solves the problem there, it > could be that gcc 7.1 also had a bug. > > Cheers, > > Kevin > > > > >> >> - Felix >> > > _______________________________________________ > Lede-dev mailing list > Lede-dev@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/lede-dev
On 23/08/17 09:20, Felix Fietkau wrote: > On 2017-08-22 12:01, Kevin Darbyshire-Bryant wrote: >> Drop 300-mips_Os_cpu_rtx_cost_model.patch for gcc 7.2 >> >> This was causing mis-compilation of dropbear with the default '-Os' size >> optimization as reported in FS#814 >> >> Tested on ar71xx, archer C7 v2. For size comparison of my whole build: >> >> 12058628 O2-withoutpatch-dropbearworks.bin >> 12058628 O2-withpatch-dropbearworks.bin >> 11468804 Os-withoutpatch-dropbearworks.bin >> 11468804 Os-withpatch-dropbearfails.bin >> >> Signed-off-by: Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> > I strongly suspect that this change is hiding the real bug instead of > fixing it. Please double-check that the mis-compilation also does not > happen with -O2 instead of -Os. > > - Felix > To further expand on this: I switched from the 74kc cpu target to 24kc (which is the lede default) 11993092 24O2-withoutpatch-works.bin 12058628 24O2-withpatch-works.bin 11403268 24Os-withoutpatch-works.bin 11468804 24Os-withpatch-fails.bin 2 observations: The working/not working follows the same pattern as found with 74kc. Sizes on 24kc are always larger with the patch. Kevin
Citeren Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk>: > On 23/08/17 09:20, Felix Fietkau wrote: >> On 2017-08-22 12:01, Kevin Darbyshire-Bryant wrote: >>> Drop 300-mips_Os_cpu_rtx_cost_model.patch for gcc 7.2 >>> >>> This was causing mis-compilation of dropbear with the default '-Os' size >>> optimization as reported in FS#814 >>> >>> Tested on ar71xx, archer C7 v2. For size comparison of my whole build: >>> >>> 12058628 O2-withoutpatch-dropbearworks.bin >>> 12058628 O2-withpatch-dropbearworks.bin >>> 11468804 Os-withoutpatch-dropbearworks.bin >>> 11468804 Os-withpatch-dropbearfails.bin >>> >>> Signed-off-by: Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> >> I strongly suspect that this change is hiding the real bug instead of >> fixing it. Please double-check that the mis-compilation also does not >> happen with -O2 instead of -Os. >> >> - Felix >> > > To further expand on this: I switched from the 74kc cpu target to > 24kc (which is the lede default) > > 11993092 24O2-withoutpatch-works.bin > 12058628 24O2-withpatch-works.bin > 11403268 24Os-withoutpatch-works.bin > 11468804 24Os-withpatch-fails.bin > > 2 observations: The working/not working follows the same pattern as > found with 74kc. Sizes on 24kc are always larger with the patch. While removing the MIPS patch fixes dropbear when using gcc 7.1.0 and '-Os', uhttpd goes tits up with a segfault in liblua. With '-O2' it's fine for both, so I'll probably stick with that for now.
On 23/08/17 18:25, Arjen de Korte wrote: > While removing the MIPS patch fixes dropbear when using gcc 7.1.0 and > '-Os', uhttpd goes tits up with a segfault in liblua. With '-O2' it's > fine for both, so I'll probably stick with that for now. What about with gcc 7.2.0?
Citeren Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk>: > On 23/08/17 18:25, Arjen de Korte wrote: > >> While removing the MIPS patch fixes dropbear when using gcc 7.1.0 >> and '-Os', uhttpd goes tits up with a segfault in liblua. With >> '-O2' it's fine for both, so I'll probably stick with that for now. > > What about with gcc 7.2.0? Good question. I'll try that later today.
Citeren Arjen de Korte <arjen+lede@de-korte.org>: > Citeren Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk>: > >> On 23/08/17 18:25, Arjen de Korte wrote: >> >>> While removing the MIPS patch fixes dropbear when using gcc 7.1.0 >>> and '-Os', uhttpd goes tits up with a segfault in liblua. With >>> '-O2' it's fine for both, so I'll probably stick with that for now. >> >> What about with gcc 7.2.0? > > Good question. I'll try that later today. No joy, uhttpd still fails to start with exactly the same errormessage as with gcc 7.1.0: Thu Aug 24 19:44:23 2017 kern.info kernel: [ 49.227713] Thu Aug 24 19:44:23 2017 kern.info kernel: [ 49.227713] do_page_fault(): sending SIGSEGV to uhttpd for invalid read access from 778c50d8 Thu Aug 24 19:44:23 2017 kern.info kernel: [ 49.227734] epc = 778cc277 in liblua.so.5.1.5[778c6000+2e000] Thu Aug 24 19:44:23 2017 kern.info kernel: [ 49.230672] ra = 778cc271 in liblua.so.5.1.5[778c6000+2e000] Thu Aug 24 19:44:23 2017 kern.info kernel: [ 49.233703] Thu Aug 24 19:44:23 2017 daemon.info procd: Instance uhttpd::instance1 s in a crash loop 6 crashes, 0 seconds since last crash
Citeren Arjen de Korte <arjen+lede@de-korte.org>: > Citeren Arjen de Korte <arjen+lede@de-korte.org>: > >> Citeren Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk>: >> >>> On 23/08/17 18:25, Arjen de Korte wrote: >>> >>>> While removing the MIPS patch fixes dropbear when using gcc 7.1.0 >>>> and '-Os', uhttpd goes tits up with a segfault in liblua. With >>>> '-O2' it's fine for both, so I'll probably stick with that for now. >>> >>> What about with gcc 7.2.0? >> >> Good question. I'll try that later today. > > No joy, uhttpd still fails to start with exactly the same > errormessage as with gcc 7.1.0: > > Thu Aug 24 19:44:23 2017 kern.info kernel: [ 49.227713] > Thu Aug 24 19:44:23 2017 kern.info kernel: [ 49.227713] > do_page_fault(): sending SIGSEGV to uhttpd for invalid read access > from 778c50d8 > Thu Aug 24 19:44:23 2017 kern.info kernel: [ 49.227734] epc = > 778cc277 in liblua.so.5.1.5[778c6000+2e000] > Thu Aug 24 19:44:23 2017 kern.info kernel: [ 49.230672] ra = > 778cc271 in liblua.so.5.1.5[778c6000+2e000] > Thu Aug 24 19:44:23 2017 kern.info kernel: [ 49.233703] > Thu Aug 24 19:44:23 2017 daemon.info procd: Instance > uhttpd::instance1 s in a crash loop 6 crashes, 0 seconds since last > crash And like with gcc 7.1.0, all is well when compiled with '-O2' (OK) instead of '-Os' (not OK).
diff --git a/toolchain/gcc/patches/7.2.0/300-mips_Os_cpu_rtx_cost_model.patch b/toolchain/gcc/patches/7.2.0/300-mips_Os_cpu_rtx_cost_model.patch deleted file mode 100644 index 84c0fda..0000000 --- a/toolchain/gcc/patches/7.2.0/300-mips_Os_cpu_rtx_cost_model.patch +++ /dev/null @@ -1,21 +0,0 @@ -commit ecf7671b769fe96f7b5134be442089f8bdba55d2 -Author: Felix Fietkau <nbd@nbd.name> -Date: Thu Aug 4 20:29:45 2016 +0200 - -gcc: add a patch to generate better code with Os on mips - -Also happens to reduce compressed code size a bit - -Signed-off-by: Felix Fietkau <nbd@nbd.name> - ---- a/gcc/config/mips/mips.c -+++ b/gcc/config/mips/mips.c -@@ -19784,7 +19784,7 @@ mips_option_override (void) - flag_pcc_struct_return = 0; - - /* Decide which rtx_costs structure to use. */ -- if (optimize_size) -+ if (0 && optimize_size) - mips_cost = &mips_rtx_cost_optimize_size; - else - mips_cost = &mips_rtx_cost_data[mips_tune];
Drop 300-mips_Os_cpu_rtx_cost_model.patch for gcc 7.2 This was causing mis-compilation of dropbear with the default '-Os' size optimization as reported in FS#814 Tested on ar71xx, archer C7 v2. For size comparison of my whole build: 12058628 O2-withoutpatch-dropbearworks.bin 12058628 O2-withpatch-dropbearworks.bin 11468804 Os-withoutpatch-dropbearworks.bin 11468804 Os-withpatch-dropbearfails.bin Signed-off-by: Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> --- .../7.2.0/300-mips_Os_cpu_rtx_cost_model.patch | 21 --------------------- 1 file changed, 21 deletions(-) delete mode 100644 toolchain/gcc/patches/7.2.0/300-mips_Os_cpu_rtx_cost_model.patch