On 2013-07-29 08:02:49 -0400, Tom Lane wrote: > Andres Freund <and...@2ndquadrant.com> writes: > > On 2013-07-29 07:11:13 -0400, Stephen Frost wrote: > >> * Tom Lane (t...@sss.pgh.pa.us) wrote: > >>> The bottom line was: > >>> It looks like our choices are (1) teach configure to enable > >>> -fno-aggressive-loop-optimizations if the compiler recognizes it, > >>> or (2) back-port commit 8137f2c32322c624e0431fac1621e8e9315202f9. > >>> > >>> I am in favor of fixing the back branches via (1), because it's less > >>> work and much less likely to break third-party extensions. Some other > >>> people argued for (2), but I've not seen any patch emerge from them, > >>> and you can bet I'm not going to do it. > > >> Yea, just passing -fno-aggressive-loop-optimizations seems like the > >> safest and best option to me also.. > > > I think we need to do both. There very well might be other optimizations > > made based on the unreachability information. > > If we turn off the optimization, that will fix any other cases as well, > no? So why would we risk breaking third-party code by back-porting the > struct declaration changes?
This seems to be the agreed upon course of action, so I've prepared a patch including a preliminary commit message. I confirmed that it fixes the issue with gcc 4.8 and 9.1 for me. The patch needs to be applied to 9.1, 9.0 and 8.4. Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
>From 1d9f13fd9245c66de02478875c9f6c3eea3bc978 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Wed, 21 Aug 2013 13:10:23 +0200 Subject: [PATCH] Amend CFLAGS for gcc 4.8 to prevent optimization problems due to variable length structs gcc 4.8 concludes it can terminate some loops prematurely, causing e.g. initdb to fail in optimized builds, because we embed variable length structs inside catalog structs. If those embedded variable length structs are not the trailing field gcc concludes that they an area of memory has to be of a certain size and uses that information to infer that some loops can only iterate a limited number of times. In reality, the actual struct layout isn't used for any of the elements beyond the first variable length element, it's just there for the benefit genbki.pl. Later fields are only accessed indirectly via heap_getattr() and friends. But the compiler doesn't know that. Commits 8137f2c3 / 8a3f745f, which are only included in 9.2 and onwards, thus hide all variable length fields after the first one via #ifdefs. For the benefit of earlier releases, let configure check for -fno-aggressive-loop-optimizations, which disables this kind of optimization. It was concluded that this way of fixing problems arising from the optimization is less likely to cause problems than backporting the aforementioned commits. Per discussion on the hackers mailinglist. --- configure.in | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configure.in b/configure.in index cf1afeb..76cd5eb 100644 --- a/configure.in +++ b/configure.in @@ -438,6 +438,9 @@ if test "$GCC" = yes -a "$ICC" = no; then PGAC_PROG_CC_CFLAGS_OPT([-fwrapv]) # Disable FP optimizations that cause various errors on gcc 4.5+ or maybe 4.6+ PGAC_PROG_CC_CFLAGS_OPT([-fexcess-precision=standard]) + # Disable some loop optimizations, causes error due to variable length structs + # needed for < 9.2 and gcc 4.8+ + PGAC_PROG_CC_CFLAGS_OPT([-fno-aggressive-loop-optimizations]) elif test "$ICC" = yes; then # Intel's compiler has a bug/misoptimization in checking for # division by NAN (NaN == 0), -mp1 fixes it, so add it to the CFLAGS. -- 1.8.2.rc2.4.g7799588.dirty
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
