Hello world,
the attached patch fixes an 8/9/10 regression where, to fix PR 84487 by not putting the initializers and vtabs into the read-only section (for reasons of size, which could grow enormously) led to a regression on POWER9 and other non-x86 architectures, where the initializer was sometimes optimized away, depending on optimization levels. This was a strange beast to hunt down. This only showed up on the testresults for gcc 8, so I tried to find out what commit had fixed this on trunk, in order to backport. However, bisecting this I found that the test case actually segfaults all the way up to current trunk when run by hand. By running the testsuite, I didn't see it. This is strange, and raises some issues about the testsuite and the possibility of a latent issue, but I lack the knowledge to hunt this down. In the meantime, here is this patch, which puts the vtabs and the initializers where the user actually specified something into the read-only section again. Test case: Well, theoretically it is already there, so it makes little sense to add a new one. Regression-tested on powerpc64le-unknown-linux-gnu, also verified by hand that pr51434.f90 now passes with -O2 there. OK for trunk/9/8? Regards Thomas 2019-11-02 Thomas Koenig <[hidden email]> PR fortran/92133 * trans-decl.c (gfc_get_symbol_decl): If __def_init actually contains a value, put it into the read-only section. |
On Sat, Nov 02, 2019 at 10:38:32AM +0100, Thomas Koenig wrote:
> > the attached patch fixes an 8/9/10 regression where, to fix PR 84487 > by not putting the initializers and vtabs into the read-only section > (for reasons of size, which could grow enormously) led to a regression > on POWER9 and other non-x86 architectures, where the initializer was > sometimes optimized away, depending on optimization levels. > > This was a strange beast to hunt down. This only showed up on > the testresults for gcc 8, so I tried to find out what commit > had fixed this on trunk, in order to backport. > > However, bisecting this I found that the test case actually > segfaults all the way up to current trunk when run by hand. > By running the testsuite, I didn't see it. This is strange, > and raises some issues about the testsuite and the possibility > of a latent issue, but I lack the knowledge to hunt this down. > > In the meantime, here is this patch, which puts the vtabs and > the initializers where the user actually specified something > into the read-only section again. > > Test case: Well, theoretically it is already there, so it makes > little sense to add a new one. > > Regression-tested on powerpc64le-unknown-linux-gnu, also > verified by hand that pr51434.f90 now passes with -O2 there. > > OK for trunk/9/8? OK for all three. It is, as you have indicated, troublesome that a segfaulting testcase isn't caught by the testsuite. -- Steve |
Hi Steve,
>> OK for trunk/9/8? > > OK for all three. Thanks, committed to trunk as r277760. I'll be AFK for a few days, so I will have to wait before committing this to gcc-9. Given the convoluted history of this bug, this might not be a bad thing. > It is, as you have indicated, troublesome that a segfaulting > testcase isn't caught by the testsuite. It certainly is, but I have no solution for this at the moment. Thanks for the review! Regards Thomas |
Free forum by Nabble | Edit this page |