Back branches vs. gcc 4.8.0

Started by Tom Lanealmost 13 years ago18 messages
#1Tom Lane
tgl@sss.pgh.pa.us

After quite a bit of hair-pulling trying to install Fedora 19 Alpha,
I've been able to reproduce the initdb-time failure that's currently
being seen on buildfarm member anchovy, and was also complained of
recently by an end user:
/messages/by-id/CAOD=oQ-kq3Eg5SOvRYOVxDuqibVWC8R0wEivPsMGcyzZY-nfzA@mail.gmail.com

It is exactly what I suspected, namely that gcc 4.8.0 is applying an
optimization that breaks our code; the odd thing though is that it's
not breaking 9.2 or HEAD, just the older branches.

It turns out that what's happening is that with
-faggressive-loop-optimizations turned on (as it is by default),
gcc decides that loops that iterate over the elements of an int2vector
can iterate at most once, because int2vector is declared with a fixed
size values[] array:

int16 values[1]; /* VARIABLE LENGTH ARRAY */
} int2vector; /* VARIABLE LENGTH STRUCT */

Now, gcc does know better than to make such an assumption
unconditionally, but what I discovered is that it *will* assume this if
the int2vector is declared as a non-last element of a larger struct,
so that (in gcc's little mind anyway) it couldn't possibly really be
a variable-length array.

In other words, the reason 9.2 and up don't break is commit
8137f2c32322c624e0431fac1621e8e9315202f9, which arranged to hide
non-fixed-offset catalog columns from the compiler. Without that,
gcc decides that for instance pg_index.indkey cannot have more than one
member. That breaks the loop in BuildIndexInfo() that copies the key
column numbers into an IndexInfo, leading to the observed failure.

Since gcc 4.8 is going to be on a lot of people's machines pretty soon,
I think we need to do something to prevent it from breaking 8.4.x and
9.0.x. 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'm a bit leaning towards (1), mainly because I'm not excited about
fighting a compiler arms race in the back branches.

It also strikes me that we ought to take this as a warning sign
that we need to work on getting rid of coding like the above in favor
of genuine "flexible arrays", before the gcc boys think of some other
overly-cute optimization based on the assumption that an array declared
with a fixed size really is fixed.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Peter Geoghegan
pg@heroku.com
In reply to: Tom Lane (#1)
Re: Back branches vs. gcc 4.8.0

On Fri, Apr 5, 2013 at 11:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

It also strikes me that we ought to take this as a warning sign
that we need to work on getting rid of coding like the above in favor
of genuine "flexible arrays", before the gcc boys think of some other
overly-cute optimization based on the assumption that an array declared
with a fixed size really is fixed.

The traditional argument against that has been that that's a C99
feature. However, since it appears that even MSVC supports flexible
arrays (which are described as a "Microsoft extension", so may not
have identical semantics), it might be possible to do this across the
board without contorting the code with preprocessor hacks. That's
something that I'd certainly be in favor of pursuing.

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Gavin Flower
GavinFlower@archidevsys.co.nz
In reply to: Tom Lane (#1)
Re: Back branches vs. gcc 4.8.0

On 06/04/13 11:14, Tom Lane wrote:

After quite a bit of hair-pulling trying to install Fedora 19 Alpha,
I've been able to reproduce the initdb-time failure that's currently
being seen on buildfarm member anchovy, and was also complained of
recently by an end user:
/messages/by-id/CAOD=oQ-kq3Eg5SOvRYOVxDuqibVWC8R0wEivPsMGcyzZY-nfzA@mail.gmail.com

It is exactly what I suspected, namely that gcc 4.8.0 is applying an
optimization that breaks our code; the odd thing though is that it's
not breaking 9.2 or HEAD, just the older branches.

It turns out that what's happening is that with
-faggressive-loop-optimizations turned on (as it is by default),
gcc decides that loops that iterate over the elements of an int2vector
can iterate at most once, because int2vector is declared with a fixed
size values[] array:

int16 values[1]; /* VARIABLE LENGTH ARRAY */
} int2vector; /* VARIABLE LENGTH STRUCT */

Now, gcc does know better than to make such an assumption
unconditionally, but what I discovered is that it *will* assume this if
the int2vector is declared as a non-last element of a larger struct,
so that (in gcc's little mind anyway) it couldn't possibly really be
a variable-length array.

In other words, the reason 9.2 and up don't break is commit
8137f2c32322c624e0431fac1621e8e9315202f9, which arranged to hide
non-fixed-offset catalog columns from the compiler. Without that,
gcc decides that for instance pg_index.indkey cannot have more than one
member. That breaks the loop in BuildIndexInfo() that copies the key
column numbers into an IndexInfo, leading to the observed failure.

Since gcc 4.8 is going to be on a lot of people's machines pretty soon,
I think we need to do something to prevent it from breaking 8.4.x and
9.0.x. 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'm a bit leaning towards (1), mainly because I'm not excited about
fighting a compiler arms race in the back branches.

It also strikes me that we ought to take this as a warning sign
that we need to work on getting rid of coding like the above in favor
of genuine "flexible arrays", before the gcc boys think of some other
overly-cute optimization based on the assumption that an array declared
with a fixed size really is fixed.

regards, tom lane

I am probably missing something here!

I would have thought it reasonable for a compiler assume 'an array
declared with a fixed size really is fixed.'!

Seems dangerous to play tricks like that (though I admit to doing nasty
things like that in COBOL many many years ago!).

Cheers,
Gavin

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Geoghegan (#2)
Re: Back branches vs. gcc 4.8.0

Peter Geoghegan <pg@heroku.com> writes:

On Fri, Apr 5, 2013 at 11:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

It also strikes me that we ought to take this as a warning sign
that we need to work on getting rid of coding like the above in favor
of genuine "flexible arrays", before the gcc boys think of some other
overly-cute optimization based on the assumption that an array declared
with a fixed size really is fixed.

The traditional argument against that has been that that's a C99
feature.

Well, we already have a solution for that, see FLEXIBLE_ARRAY_MEMBER.
But up to now we've just supposed that that was a code beautification
thing and there was no particular urgency to convert all applicable
places to use that notation.

Since there's a potential to break code with such changes (we'd have to
fix any uses of sizeof on the struct type), it's been very far down the
to-do list. But now it appears that we're taking risks if we *don't*
change it.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Andres Freund
andres@2ndquadrant.com
In reply to: Peter Geoghegan (#2)
Re: Back branches vs. gcc 4.8.0

On 2013-04-05 23:28:03 +0100, Peter Geoghegan wrote:

On Fri, Apr 5, 2013 at 11:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

It also strikes me that we ought to take this as a warning sign
that we need to work on getting rid of coding like the above in favor
of genuine "flexible arrays", before the gcc boys think of some other
overly-cute optimization based on the assumption that an array declared
with a fixed size really is fixed.

The traditional argument against that has been that that's a C99
feature. However, since it appears that even MSVC supports flexible
arrays (which are described as a "Microsoft extension", so may not
have identical semantics), it might be possible to do this across the
board without contorting the code with preprocessor hacks. That's
something that I'd certainly be in favor of pursuing.

The respective macro magic is already in place, its just not used in all
places. The problem is more that we can't easily use it in all places
because e.g. in the one case mentioned here the array isn't in the last
place *in the back branches*.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Peter Geoghegan
pg@heroku.com
In reply to: Andres Freund (#5)
Re: Back branches vs. gcc 4.8.0

On Fri, Apr 5, 2013 at 11:39 PM, Andres Freund <andres@2ndquadrant.com> wrote:

The respective macro magic is already in place, its just not used in all
places. The problem is more that we can't easily use it in all places
because e.g. in the one case mentioned here the array isn't in the last
place *in the back branches*.

Are you proposing that we use the FLEXIBLE_ARRAY_MEMBER macro in every
single place where we currently use the one element array pattern? I
count one place where we currently use FLEXIBLE_ARRAY_MEMBER. It'd be
pretty ugly to have that everywhere, in my opinion.

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#5)
Re: Back branches vs. gcc 4.8.0

Andres Freund <andres@2ndquadrant.com> writes:

The respective macro magic is already in place, its just not used in all
places. The problem is more that we can't easily use it in all places
because e.g. in the one case mentioned here the array isn't in the last
place *in the back branches*.

I don't think we should try to back-patch such changes; there seems too
much risk of breaking third-party code because of the sizeof() issue.
But it'd be a good idea to have it in place before we find ourselves
having to do -fno-aggressive-loop-optimizations or some such even in
up-to-date branches.

(I'm actually even more worried about gcc bugs that make this type of
assumption than about intentional changes on their part.)

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Geoghegan (#6)
Re: Back branches vs. gcc 4.8.0

Peter Geoghegan <pg@heroku.com> writes:

Are you proposing that we use the FLEXIBLE_ARRAY_MEMBER macro in every
single place where we currently use the one element array pattern?

Yup, exactly.

I count one place where we currently use FLEXIBLE_ARRAY_MEMBER. It'd be
pretty ugly to have that everywhere, in my opinion.

Hm, I see 4 places in HEAD. But in any case, is

int16 values[1]; /* VARIABLE LENGTH ARRAY */
} int2vector; /* VARIABLE LENGTH STRUCT */

really better than

int16 values[FLEXIBLE_ARRAY_MEMBER];
} int2vector;

? I don't think so. Relying on comments to tell about critical
semantics of a data structure isn't really nice if you can do it
in a way that is standards-blessed and (some) compilers understand.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Peter Geoghegan
pg@heroku.com
In reply to: Tom Lane (#8)
Re: Back branches vs. gcc 4.8.0

On Fri, Apr 5, 2013 at 11:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Hm, I see 4 places in HEAD. But in any case, is

My mistake. I had REL9_2_STABLE checked out.

int16 values[1]; /* VARIABLE LENGTH ARRAY */
} int2vector; /* VARIABLE LENGTH STRUCT */

really better than

int16 values[FLEXIBLE_ARRAY_MEMBER];
} int2vector;

? I don't think so.

I can see your point. Now that I look at the comments beside
FLEXIBLE_ARRAY_MEMBER, I see that indeed, as I suspected, the
Microsoft flexible array members are not completely compatible with
C99 style flexible arrays, so this may be the least-worst option.

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#1)
Re: Back branches vs. gcc 4.8.0

On Fri, 2013-04-05 at 18:14 -0400, Tom Lane wrote:

Since gcc 4.8 is going to be on a lot of people's machines pretty
soon,
I think we need to do something to prevent it from breaking 8.4.x and
9.0.x. 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.

Using a fixed-size struct member as a flexible one has always been a
violation of the C standard, although a widely tolerated one. Doing
that in the middle of a struct, however, is totally wrong, and the
compiler is perfectly in its right to make a mess of it. Even flexible
array members are not allowed in the middle of a struct.

So I think this is not a compiler bug or an arms race. We just need to
fix the code. So I'm in favor of backporting.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11Peter Eisentraut
peter_e@gmx.net
In reply to: Peter Geoghegan (#6)
Re: Back branches vs. gcc 4.8.0

On Fri, 2013-04-05 at 23:44 +0100, Peter Geoghegan wrote:

On Fri, Apr 5, 2013 at 11:39 PM, Andres Freund <andres@2ndquadrant.com> wrote:

The respective macro magic is already in place, its just not used in all
places. The problem is more that we can't easily use it in all places
because e.g. in the one case mentioned here the array isn't in the last
place *in the back branches*.

Are you proposing that we use the FLEXIBLE_ARRAY_MEMBER macro in every
single place where we currently use the one element array pattern? I
count one place where we currently use FLEXIBLE_ARRAY_MEMBER. It'd be
pretty ugly to have that everywhere, in my opinion.

Background: The reason I put in that one use of FLEXIBLE_ARRAY_MEMBER is
that at one point clang threw a warning about the old coding. There
were no warnings about the other sites that use array[1].

The reason that the whole code wasn't converted right away was (besides
a lot of legwork with sizeof and offsetoff) that flexible array members
aren't allowed in the middle of structs. Which eventually led to the
mentioned commit 8137f2c32322c624e0431fac1621e8e9315202f9.

If someone wants to go through and change the rest of the code to use
FLEXIBLE_ARRAY_MEMBER, I won't mind. But I think it actually has
nothing to do with the current bug or future-proofing anything. All
compilers tolerate the current coding.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#11)
Re: Back branches vs. gcc 4.8.0

Peter Eisentraut <peter_e@gmx.net> writes:

The reason that the whole code wasn't converted right away was (besides
a lot of legwork with sizeof and offsetoff) that flexible array members
aren't allowed in the middle of structs. Which eventually led to the
mentioned commit 8137f2c32322c624e0431fac1621e8e9315202f9.

If someone wants to go through and change the rest of the code to use
FLEXIBLE_ARRAY_MEMBER, I won't mind. But I think it actually has
nothing to do with the current bug or future-proofing anything. All
compilers tolerate the current coding.

The reason I'm thinking it's a good idea is that it would expose any
remaining places where we have nominally var-length arrays embedded in
larger structs. Now that I've seen the failures with gcc 4.8.0, I'm
quite worried that there might be some more declarations like that
which we've not identified yet, but that by chance aren't causing
obvious failures today. (This is also why I'm not that excited about
trying to fix things "properly" in the back branches compared to
selecting -fno-aggressive-loop-optimizations: I'm afraid there might
be more to it than just the one commit.)

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#13Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#10)
Re: Back branches vs. gcc 4.8.0

On Fri, Apr 5, 2013 at 9:45 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

So I think this is not a compiler bug or an arms race. We just need to
fix the code. So I'm in favor of backporting.

I can certainly see this argument. I understand Tom's point about an
arms race, but back-porting this doesn't feel terribly risky to me.
The thing is, if the arms race is escalating faster than we're
comfortable with, we can always opt opt at a later time; it's not as
if back-porting this fix now commits us irrevocably.

Then, too, I tend to think this is more our fault than gcc's - for a change.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#1)
Re: Back branches vs. gcc 4.8.0

On 4/5/13 6:14 PM, Tom Lane wrote:

Since gcc 4.8 is going to be on a lot of people's machines pretty soon,
I think we need to do something to prevent it from breaking 8.4.x and
9.0.x. 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'm a bit leaning towards (1), mainly because I'm not excited about
fighting a compiler arms race in the back branches.

At the moment, I wouldn't do anything. At least until we have converted
master to use flexible array members completely, and we have learned the
extent of the issue.

The problem manifests itself easily through the regression tests, so
there is no guessing about whether a particular combination of versions
will work. Someone who uses a cutting edge compiler with a somewhat old
PG release is doing something special anyway, so they should have the
required skills to put in the workaround.

I would rather avoid patching in specific compiler options for specific
versions. These things come and go, but releases live a long time. How
do we know -fno-aggressive-loop-optimizations is the only option we need
in the long run? I'd rather see a direct crash or a known code fix.

As an aside, we already have -fno-strict-aliasing and -fwrapv. Add more
and it will begin to read like

-fmy-code -fis-broken -fhelp-me

;-)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#15Gavin Flower
GavinFlower@archidevsys.co.nz
In reply to: Peter Eisentraut (#14)
Re: Back branches vs. gcc 4.8.0

On 09/04/13 08:41, Peter Eisentraut wrote:

On 4/5/13 6:14 PM, Tom Lane wrote:

Since gcc 4.8 is going to be on a lot of people's machines pretty soon,
I think we need to do something to prevent it from breaking 8.4.x and
9.0.x. 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'm a bit leaning towards (1), mainly because I'm not excited about
fighting a compiler arms race in the back branches.

At the moment, I wouldn't do anything. At least until we have converted
master to use flexible array members completely, and we have learned the
extent of the issue.

The problem manifests itself easily through the regression tests, so
there is no guessing about whether a particular combination of versions
will work. Someone who uses a cutting edge compiler with a somewhat old
PG release is doing something special anyway, so they should have the
required skills to put in the workaround.

I would rather avoid patching in specific compiler options for specific
versions. These things come and go, but releases live a long time. How
do we know -fno-aggressive-loop-optimizations is the only option we need
in the long run? I'd rather see a direct crash or a known code fix.

As an aside, we already have -fno-strict-aliasing and -fwrapv. Add more
and it will begin to read like

-fmy-code -fis-broken -fhelp-me

;-)

-fno-break-my-code

#16Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#12)
1 attachment(s)
Re: Back branches vs. gcc 4.8.0

On Sat, 2013-04-06 at 12:59 -0400, Tom Lane wrote:

The reason I'm thinking it's a good idea is that it would expose any
remaining places where we have nominally var-length arrays embedded in
larger structs. Now that I've seen the failures with gcc 4.8.0, I'm
quite worried that there might be some more declarations like that
which we've not identified yet, but that by chance aren't causing
obvious failures today.

Here is a rough patch that replaces almost all occurrences of
something[1] in a struct with FLEXIBLE_ARRAY_MEMBER. It crashes left
and right (because of sizeof issues, probably), but at least so far the
compiler hasn't complained about any flexible-array members not at the
end of the struct, which is what it did last time. So the answer to
your concern so far is negative.

Completing this patch will be quite a bit more debugging work. Some
kind of electric fence for palloc would be helpful.

Attachments:

flexible-array-member.patchtext/x-patch; charset=UTF-8; name=flexible-array-member.patchDownload
diff --git a/contrib/cube/cubedata.h b/contrib/cube/cubedata.h
index fd0c26a..01e0818 100644
--- a/contrib/cube/cubedata.h
+++ b/contrib/cube/cubedata.h
@@ -6,7 +6,7 @@ typedef struct NDBOX
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	unsigned int dim;
-	double		x[1];
+	double		x[FLEXIBLE_ARRAY_MEMBER];
 } NDBOX;
 
 #define DatumGetNDBOX(x)	((NDBOX*)DatumGetPointer(x))
diff --git a/contrib/intarray/_int.h b/contrib/intarray/_int.h
index 56aa23c..c35e862 100644
--- a/contrib/intarray/_int.h
+++ b/contrib/intarray/_int.h
@@ -72,7 +72,7 @@ typedef struct
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	int32		flag;
-	char		data[1];
+	char		data[FLEXIBLE_ARRAY_MEMBER];
 } GISTTYPE;
 
 #define ALLISTRUE		0x04
@@ -132,7 +132,7 @@ typedef struct QUERYTYPE
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	int32		size;			/* number of ITEMs */
-	ITEM		items[1];		/* variable length array */
+	ITEM		items[FLEXIBLE_ARRAY_MEMBER];
 } QUERYTYPE;
 
 #define HDRSIZEQT	offsetof(QUERYTYPE, items)
diff --git a/contrib/ltree/ltree.h b/contrib/ltree/ltree.h
index 563970a..bf25f4c 100644
--- a/contrib/ltree/ltree.h
+++ b/contrib/ltree/ltree.h
@@ -9,7 +9,7 @@
 typedef struct
 {
 	uint16		len;
-	char		name[1];
+	char		name[FLEXIBLE_ARRAY_MEMBER];
 } ltree_level;
 
 #define LEVEL_HDRSIZE	(offsetof(ltree_level,name))
@@ -19,7 +19,7 @@ typedef struct
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	uint16		numlevel;
-	char		data[1];
+	char		data[FLEXIBLE_ARRAY_MEMBER];
 } ltree;
 
 #define LTREE_HDRSIZE	MAXALIGN( offsetof(ltree, data) )
@@ -33,7 +33,7 @@ typedef struct
 	int32		val;
 	uint16		len;
 	uint8		flag;
-	char		name[1];
+	char		name[FLEXIBLE_ARRAY_MEMBER];
 } lquery_variant;
 
 #define LVAR_HDRSIZE   MAXALIGN(offsetof(lquery_variant, name))
@@ -50,7 +50,7 @@ typedef struct
 	uint16		numvar;
 	uint16		low;
 	uint16		high;
-	char		variants[1];
+	char		variants[FLEXIBLE_ARRAY_MEMBER];
 } lquery_level;
 
 #define LQL_HDRSIZE MAXALIGN( offsetof(lquery_level,variants) )
@@ -71,7 +71,7 @@ typedef struct
 	uint16		numlevel;
 	uint16		firstgood;
 	uint16		flag;
-	char		data[1];
+	char		data[FLEXIBLE_ARRAY_MEMBER];
 } lquery;
 
 #define LQUERY_HDRSIZE	 MAXALIGN( offsetof(lquery, data) )
@@ -106,7 +106,7 @@ typedef struct
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	int32		size;
-	char		data[1];
+	char		data[FLEXIBLE_ARRAY_MEMBER];
 } ltxtquery;
 
 #define HDRSIZEQT		MAXALIGN(VARHDRSZ + sizeof(int32))
@@ -205,7 +205,7 @@ typedef struct
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	uint32		flag;
-	char		data[1];
+	char		data[FLEXIBLE_ARRAY_MEMBER];
 } ltree_gist;
 
 #define LTG_ONENODE 0x01
diff --git a/contrib/pg_trgm/trgm.h b/contrib/pg_trgm/trgm.h
index ed649b8..f030558 100644
--- a/contrib/pg_trgm/trgm.h
+++ b/contrib/pg_trgm/trgm.h
@@ -63,7 +63,7 @@ typedef struct
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	uint8		flag;
-	char		data[1];
+	char		data[FLEXIBLE_ARRAY_MEMBER];
 } TRGM;
 
 #define TRGMHDRSIZE		  (VARHDRSZ + sizeof(uint8))
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index f48c0bc..4323184 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -1066,7 +1066,7 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
 		 */
 		effective_nargs = Max(pronargs, nargs);
 		newResult = (FuncCandidateList)
-			palloc(sizeof(struct _FuncCandidateList) - sizeof(Oid)
+			palloc(offsetof(struct _FuncCandidateList, args)
 				   + effective_nargs * sizeof(Oid));
 		newResult->pathpos = pathpos;
 		newResult->oid = HeapTupleGetOid(proctup);
@@ -1580,7 +1580,7 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
 	 * separate palloc for each operator, but profiling revealed that the
 	 * pallocs used an unreasonably large fraction of parsing time.
 	 */
-#define SPACE_PER_OP MAXALIGN(sizeof(struct _FuncCandidateList) + sizeof(Oid))
+#define SPACE_PER_OP MAXALIGN(offsetof(struct _FuncCandidateList, args) + sizeof(Oid)) // XXX
 
 	if (catlist->n_members > 0)
 		resultSpace = palloc(catlist->n_members * SPACE_PER_OP);
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 62208eb..ca16a4d 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -383,10 +383,9 @@ static ParamListInfo EvaluateParams(PreparedStatement *pstmt, List *params,
 	/* Prepare the expressions for execution */
 	exprstates = (List *) ExecPrepareExpr((Expr *) params, estate);
 
-	/* sizeof(ParamListInfoData) includes the first array element */
 	paramLI = (ParamListInfo)
-		palloc(sizeof(ParamListInfoData) +
-			   (num_params - 1) * sizeof(ParamExternData));
+		palloc(offsetof(ParamListInfoData, params) +
+			   num_params * sizeof(ParamExternData));
 	/* we have static list of params, so no hooks needed */
 	paramLI->paramFetch = NULL;
 	paramLI->paramFetchArg = NULL;
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index dbb4805..034a17e 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -888,9 +888,8 @@ static Datum postquel_get_single_result(TupleTableSlot *slot,
 
 		if (fcache->paramLI == NULL)
 		{
-			/* sizeof(ParamListInfoData) includes the first array element */
-			paramLI = (ParamListInfo) palloc(sizeof(ParamListInfoData) +
-									  (nargs - 1) * sizeof(ParamExternData));
+			paramLI = (ParamListInfo) palloc(offsetof(ParamListInfoData, params) +
+											 nargs * sizeof(ParamExternData));
 			/* we have static list of params, so no hooks needed */
 			paramLI->paramFetch = NULL;
 			paramLI->paramFetchArg = NULL;
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index ca0d05d..df052d7 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2216,9 +2216,8 @@ static void _SPI_cursor_operation(Portal portal,
 	{
 		int			i;
 
-		/* sizeof(ParamListInfoData) includes the first array element */
-		paramLI = (ParamListInfo) palloc(sizeof(ParamListInfoData) +
-									  (nargs - 1) * sizeof(ParamExternData));
+		paramLI = (ParamListInfo) palloc(offsetof(ParamListInfoData, params) +
+										 nargs * sizeof(ParamExternData));
 		/* we have static list of params, so no hooks needed */
 		paramLI->paramFetch = NULL;
 		paramLI->paramFetchArg = NULL;
diff --git a/src/backend/nodes/params.c b/src/backend/nodes/params.c
index 601659b..6fe3744 100644
--- a/src/backend/nodes/params.c
+++ b/src/backend/nodes/params.c
@@ -40,9 +40,8 @@
 	if (from == NULL || from->numParams <= 0)
 		return NULL;
 
-	/* sizeof(ParamListInfoData) includes the first array element */
-	size = sizeof(ParamListInfoData) +
-		(from->numParams - 1) * sizeof(ParamExternData);
+	size = offsetof(ParamListInfoData, params) +
+		from->numParams * sizeof(ParamExternData);
 
 	retval = (ParamListInfo) palloc(size);
 	retval->paramFetch = NULL;
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index f078303..47b1a80 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -1562,9 +1562,8 @@
 	{
 		int			paramno;
 
-		/* sizeof(ParamListInfoData) includes the first array element */
-		params = (ParamListInfo) palloc(sizeof(ParamListInfoData) +
-								  (numParams - 1) * sizeof(ParamExternData));
+		params = (ParamListInfo) palloc(offsetof(ParamListInfoData, params) +
+								  numParams * sizeof(ParamExternData));
 		/* we have static list of params, so no hooks needed */
 		params->paramFetch = NULL;
 		params->paramFetchArg = NULL;
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index 356e8eb..74f83c2 100644
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -22,7 +22,7 @@
 typedef struct SimpleStringListCell
 {
 	struct SimpleStringListCell *next;
-	char		val[1];			/* VARIABLE LENGTH FIELD */
+	char		val[FLEXIBLE_ARRAY_MEMBER];
 } SimpleStringListCell;
 
 typedef struct SimpleStringList
diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h
index cae6dbc..72106ab 100644
--- a/src/include/access/gist_private.h
+++ b/src/include/access/gist_private.h
@@ -31,7 +31,7 @@ typedef struct
 {
 	BlockNumber prev;
 	uint32		freespace;
-	char		tupledata[1];
+	char		tupledata[FLEXIBLE_ARRAY_MEMBER];
 } GISTNodeBufferPage;
 
 #define BUFFER_PAGE_DATA_OFFSET MAXALIGN(offsetof(GISTNodeBufferPage, tupledata))
@@ -129,7 +129,7 @@ typedef struct GISTSearchTreeItem
 	RBNode		rbnode;			/* this is an RBTree item */
 	GISTSearchItem *head;		/* first chain member */
 	GISTSearchItem *lastHeap;	/* last heap-tuple member, if any */
-	double		distances[1];	/* array with numberOfOrderBys entries */
+	double		distances[FLEXIBLE_ARRAY_MEMBER];	/* array with numberOfOrderBys entries */
 } GISTSearchTreeItem;
 
 #define GSTIHDRSZ offsetof(GISTSearchTreeItem, distances)
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index e58eae5..2e2ab05 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -122,7 +122,7 @@ typedef struct xl_heap_multi_insert
 	BlockNumber blkno;
 	bool		all_visible_cleared;
 	uint16		ntuples;
-	OffsetNumber offsets[1];
+	OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
 
 	/* TUPLE DATA (xl_multi_insert_tuples) FOLLOW AT END OF STRUCT */
 } xl_heap_multi_insert;
diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index cd01ecd..2856986 100644
--- a/src/include/access/htup_details.h
+++ b/src/include/access/htup_details.h
@@ -149,7 +149,7 @@ struct HeapTupleHeaderData
 
 	/* ^ - 23 bytes - ^ */
 
-	bits8		t_bits[1];		/* bitmap of NULLs -- VARIABLE LENGTH */
+	bits8		t_bits[FLEXIBLE_ARRAY_MEMBER];		/* bitmap of NULLs */
 
 	/* MORE DATA FOLLOWS AT END OF STRUCT */
 };
@@ -523,7 +523,7 @@ struct MinimalTupleData
 
 	/* ^ - 23 bytes - ^ */
 
-	bits8		t_bits[1];		/* bitmap of NULLs -- VARIABLE LENGTH */
+	bits8		t_bits[FLEXIBLE_ARRAY_MEMBER];		/* bitmap of NULLs */
 
 	/* MORE DATA FOLLOWS AT END OF STRUCT */
 };
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
index 835f6ac..3b39ee6 100644
--- a/src/include/access/xact.h
+++ b/src/include/access/xact.h
@@ -116,7 +116,7 @@ typedef struct xl_xact_assignment
 {
 	TransactionId xtop;			/* assigned XID's top-level XID */
 	int			nsubxacts;		/* number of subtransaction XIDs */
-	TransactionId xsub[1];		/* assigned subxids */
+	TransactionId xsub[FLEXIBLE_ARRAY_MEMBER];		/* assigned subxids */
 } xl_xact_assignment;
 
 #define MinSizeOfXactAssignment offsetof(xl_xact_assignment, xsub)
@@ -126,7 +126,7 @@ typedef struct xl_xact_commit_compact
 	TimestampTz xact_time;		/* time of commit */
 	int			nsubxacts;		/* number of subtransaction XIDs */
 	/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
-	TransactionId subxacts[1];	/* VARIABLE LENGTH ARRAY */
+	TransactionId subxacts[FLEXIBLE_ARRAY_MEMBER];
 } xl_xact_commit_compact;
 
 #define MinSizeOfXactCommitCompact offsetof(xl_xact_commit_compact, subxacts)
@@ -141,7 +141,7 @@ typedef struct xl_xact_commit
 	Oid			dbId;			/* MyDatabaseId */
 	Oid			tsId;			/* MyDatabaseTableSpace */
 	/* Array of RelFileNode(s) to drop at commit */
-	RelFileNode xnodes[1];		/* VARIABLE LENGTH ARRAY */
+	RelFileNode xnodes[FLEXIBLE_ARRAY_MEMBER];
 	/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
 	/* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */
 } xl_xact_commit;
@@ -169,7 +169,7 @@ typedef struct xl_xact_abort
 	int			nrels;			/* number of RelFileNodes */
 	int			nsubxacts;		/* number of subtransaction XIDs */
 	/* Array of RelFileNode(s) to drop at abort */
-	RelFileNode xnodes[1];		/* VARIABLE LENGTH ARRAY */
+	RelFileNode xnodes[FLEXIBLE_ARRAY_MEMBER];
 	/* ARRAY OF ABORTED SUBTRANSACTION XIDs FOLLOWS */
 } xl_xact_abort;
 
diff --git a/src/include/c.h b/src/include/c.h
index ca7ff86..f533074 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -394,7 +394,7 @@ typedef struct
 struct varlena
 {
 	char		vl_len_[4];		/* Do not touch this field directly! */
-	char		vl_dat[1];
+	char		vl_dat[FLEXIBLE_ARRAY_MEMBER];
 };
 
 #define VARHDRSZ		((int32) sizeof(int32))
@@ -427,8 +427,8 @@ typedef struct
 	Oid			elemtype;
 	int			dim1;
 	int			lbound1;
-	int16		values[1];		/* VARIABLE LENGTH ARRAY */
-} int2vector;					/* VARIABLE LENGTH STRUCT */
+	int16		values[FLEXIBLE_ARRAY_MEMBER];
+} int2vector;
 
 typedef struct
 {
@@ -438,8 +438,8 @@ typedef struct
 	Oid			elemtype;
 	int			dim1;
 	int			lbound1;
-	Oid			values[1];		/* VARIABLE LENGTH ARRAY */
-} oidvector;					/* VARIABLE LENGTH STRUCT */
+	Oid			values[FLEXIBLE_ARRAY_MEMBER];
+} oidvector;
 
 /*
  * Representation of a Name: effectively just a C string, but null-padded to
diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h
index 4fadc74..4db3d42 100644
--- a/src/include/catalog/namespace.h
+++ b/src/include/catalog/namespace.h
@@ -34,8 +34,8 @@ typedef struct _FuncCandidateList
 	int			nvargs;			/* number of args to become variadic array */
 	int			ndargs;			/* number of defaulted args */
 	int		   *argnumbers;		/* args' positional indexes, if named call */
-	Oid			args[1];		/* arg types --- VARIABLE LENGTH ARRAY */
-}	*FuncCandidateList;	/* VARIABLE LENGTH STRUCT */
+	Oid			args[FLEXIBLE_ARRAY_MEMBER];		/* arg types */
+}	*FuncCandidateList;
 
 /*
  *	Structure for xxxOverrideSearchPath functions
diff --git a/src/include/commands/dbcommands.h b/src/include/commands/dbcommands.h
index 942b883..ab7a56c 100644
--- a/src/include/commands/dbcommands.h
+++ b/src/include/commands/dbcommands.h
@@ -25,7 +25,7 @@ typedef struct xl_dbase_create_rec_old
 {
 	/* Records copying of a single subdirectory incl. contents */
 	Oid			db_id;
-	char		src_path[1];	/* VARIABLE LENGTH STRING */
+	char		src_path[FLEXIBLE_ARRAY_MEMBER];
 	/* dst_path follows src_path */
 }	xl_dbase_create_rec_old;
 
@@ -33,7 +33,7 @@ typedef struct xl_dbase_drop_rec_old
 {
 	/* Records dropping of a single subdirectory incl. contents */
 	Oid			db_id;
-	char		dir_path[1];	/* VARIABLE LENGTH STRING */
+	char		dir_path[FLEXIBLE_ARRAY_MEMBER];
 }	xl_dbase_drop_rec_old;
 
 typedef struct xl_dbase_create_rec
diff --git a/src/include/commands/tablespace.h b/src/include/commands/tablespace.h
index 1e396a8..0ec1869 100644
--- a/src/include/commands/tablespace.h
+++ b/src/include/commands/tablespace.h
@@ -24,7 +24,7 @@
 typedef struct xl_tblspc_create_rec
 {
 	Oid			ts_id;
-	char		ts_path[1];		/* VARIABLE LENGTH STRING */
+	char		ts_path[FLEXIBLE_ARRAY_MEMBER];
 } xl_tblspc_create_rec;
 
 typedef struct xl_tblspc_drop_rec
diff --git a/src/include/nodes/bitmapset.h b/src/include/nodes/bitmapset.h
index 2a4b41d..6d51fa8 100644
--- a/src/include/nodes/bitmapset.h
+++ b/src/include/nodes/bitmapset.h
@@ -32,8 +32,8 @@ typedef int32 signedbitmapword; /* must be the matching signed type */
 typedef struct Bitmapset
 {
 	int			nwords;			/* number of words in array */
-	bitmapword	words[1];		/* really [nwords] */
-} Bitmapset;					/* VARIABLE LENGTH STRUCT */
+	bitmapword	words[FLEXIBLE_ARRAY_MEMBER];		/* really [nwords] */
+} Bitmapset;
 
 
 /* result of bms_subset_compare */
diff --git a/src/include/nodes/params.h b/src/include/nodes/params.h
index 61bdff0..7a71e45 100644
--- a/src/include/nodes/params.h
+++ b/src/include/nodes/params.h
@@ -71,7 +71,7 @@ typedef struct ParamListInfoData
 	ParserSetupHook parserSetup;	/* parser setup hook */
 	void	   *parserSetupArg;
 	int			numParams;		/* number of ParamExternDatas following */
-	ParamExternData params[1];	/* VARIABLE LENGTH ARRAY */
+	ParamExternData params[FLEXIBLE_ARRAY_MEMBER];
 }	ParamListInfoData;
 
 
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 70c0025..071b35b 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -41,8 +41,8 @@ typedef struct
 	int			ntuples;		/* -1 indicates lossy result */
 	bool		recheck;		/* should the tuples be rechecked? */
 	/* Note: recheck is always true if ntuples < 0 */
-	OffsetNumber offsets[1];	/* VARIABLE LENGTH ARRAY */
-} TBMIterateResult;				/* VARIABLE LENGTH STRUCT */
+	OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+} TBMIterateResult;
 
 /* function prototypes in nodes/tidbitmap.c */
 
diff --git a/src/include/postgres.h b/src/include/postgres.h
index f9c5527..340c8ef 100644
--- a/src/include/postgres.h
+++ b/src/include/postgres.h
@@ -86,27 +86,27 @@ typedef union
 	struct						/* Normal varlena (4-byte length) */
 	{
 		uint32		va_header;
-		char		va_data[1];
+		char		va_data[FLEXIBLE_ARRAY_MEMBER];
 	}			va_4byte;
 	struct						/* Compressed-in-line format */
 	{
 		uint32		va_header;
 		uint32		va_rawsize; /* Original data size (excludes header) */
-		char		va_data[1]; /* Compressed data */
+		char		va_data[FLEXIBLE_ARRAY_MEMBER]; /* Compressed data */
 	}			va_compressed;
 } varattrib_4b;
 
 typedef struct
 {
 	uint8		va_header;
-	char		va_data[1];		/* Data begins here */
+	char		va_data[FLEXIBLE_ARRAY_MEMBER];		/* Data begins here */
 } varattrib_1b;
 
 typedef struct
 {
 	uint8		va_header;		/* Always 0x80 or 0x01 */
 	uint8		va_len_1be;		/* Physical length of datum */
-	char		va_data[1];		/* Data (for now always a TOAST pointer) */
+	char		va_data[FLEXIBLE_ARRAY_MEMBER];		/* Data (for now always a TOAST pointer) */
 } varattrib_1b_e;
 
 /*
diff --git a/src/include/postmaster/syslogger.h b/src/include/postmaster/syslogger.h
index 5a3ecde..17b8970 100644
--- a/src/include/postmaster/syslogger.h
+++ b/src/include/postmaster/syslogger.h
@@ -48,7 +48,7 @@ typedef struct
 	int32		pid;			/* writer's pid */
 	char		is_last;		/* last chunk of message? 't' or 'f' ('T' or
 								 * 'F' for CSV case) */
-	char		data[1];		/* data payload starts here */
+	char		data[FLEXIBLE_ARRAY_MEMBER];		/* data payload starts here */
 } PipeProtoHeader;
 
 typedef union
diff --git a/src/include/replication/walsender_private.h b/src/include/replication/walsender_private.h
index 7eaa21b..67c8ccd 100644
--- a/src/include/replication/walsender_private.h
+++ b/src/include/replication/walsender_private.h
@@ -88,7 +88,7 @@ typedef struct
 	 */
 	bool		sync_standbys_defined;
 
-	WalSnd		walsnds[1];		/* VARIABLE LENGTH ARRAY */
+	WalSnd		walsnds[FLEXIBLE_ARRAY_MEMBER];
 } WalSndCtlData;
 
 extern WalSndCtlData *WalSndCtl;
diff --git a/src/include/storage/bufpage.h b/src/include/storage/bufpage.h
index b9ee7c2..ccb1c90 100644
--- a/src/include/storage/bufpage.h
+++ b/src/include/storage/bufpage.h
@@ -156,7 +156,7 @@ typedef struct PageHeaderData
 	LocationIndex pd_special;	/* offset to start of special space */
 	uint16		pd_pagesize_version;
 	TransactionId pd_prune_xid; /* oldest prunable XID, or zero if none */
-	ItemIdData	pd_linp[1];		/* beginning of line pointer array */
+	ItemIdData	pd_linp[FLEXIBLE_ARRAY_MEMBER];		/* beginning of line pointer array */
 } PageHeaderData;
 
 typedef PageHeaderData *PageHeader;
diff --git a/src/include/storage/fsm_internals.h b/src/include/storage/fsm_internals.h
index 0d86e1d..75ba781 100644
--- a/src/include/storage/fsm_internals.h
+++ b/src/include/storage/fsm_internals.h
@@ -39,7 +39,7 @@ typedef struct
 	 * NonLeafNodesPerPage elements are upper nodes, and the following
 	 * LeafNodesPerPage elements are leaf nodes. Unused nodes are zero.
 	 */
-	uint8		fp_nodes[1];
+	uint8		fp_nodes[FLEXIBLE_ARRAY_MEMBER];
 } FSMPageData;
 
 typedef FSMPageData *FSMPage;
diff --git a/src/include/storage/standby.h b/src/include/storage/standby.h
index 168c14c..f8f2e71 100644
--- a/src/include/storage/standby.h
+++ b/src/include/storage/standby.h
@@ -59,7 +59,7 @@ extern void StandbyReleaseOldLocks(int nxids, TransactionId *xids);
 typedef struct xl_standby_locks
 {
 	int			nlocks;			/* number of entries in locks array */
-	xl_standby_lock locks[1];	/* VARIABLE LENGTH ARRAY */
+	xl_standby_lock locks[FLEXIBLE_ARRAY_MEMBER];
 } xl_standby_locks;
 
 /*
@@ -74,7 +74,7 @@ typedef struct xl_running_xacts
 	TransactionId oldestRunningXid;		/* *not* oldestXmin */
 	TransactionId latestCompletedXid;	/* so we can set xmax */
 
-	TransactionId xids[1];		/* VARIABLE LENGTH ARRAY */
+	TransactionId xids[FLEXIBLE_ARRAY_MEMBER];
 } xl_running_xacts;
 
 #define MinSizeOfXactRunningXacts offsetof(xl_running_xacts, xids)
diff --git a/src/include/tsearch/dicts/regis.h b/src/include/tsearch/dicts/regis.h
index 0f3ef3f..89cc72f 100644
--- a/src/include/tsearch/dicts/regis.h
+++ b/src/include/tsearch/dicts/regis.h
@@ -21,7 +21,7 @@ typedef struct RegisNode
 				len:16,
 				unused:14;
 	struct RegisNode *next;
-	unsigned char data[1];
+	unsigned char data[FLEXIBLE_ARRAY_MEMBER];
 } RegisNode;
 
 #define  RNHDRSZ	(offsetof(RegisNode,data))
diff --git a/src/include/tsearch/dicts/spell.h b/src/include/tsearch/dicts/spell.h
index c8e70b2..9b65386 100644
--- a/src/include/tsearch/dicts/spell.h
+++ b/src/include/tsearch/dicts/spell.h
@@ -49,7 +49,7 @@ typedef struct
 typedef struct SPNode
 {
 	uint32		length;
-	SPNodeData	data[1];
+	SPNodeData	data[FLEXIBLE_ARRAY_MEMBER];
 } SPNode;
 
 #define SPNHDRSZ	(offsetof(SPNode,data))
@@ -70,7 +70,7 @@ typedef struct spell_struct
 			int			len;
 		}			d;
 	}			p;
-	char		word[1];		/* variable length, null-terminated */
+	char		word[FLEXIBLE_ARRAY_MEMBER];		/* variable length, null-terminated */
 } SPELL;
 
 #define SPELLHDRSZ	(offsetof(SPELL, word))
@@ -120,7 +120,7 @@ typedef struct AffixNode
 {
 	uint32		isvoid:1,
 				length:31;
-	AffixNodeData data[1];
+	AffixNodeData data[FLEXIBLE_ARRAY_MEMBER];
 } AffixNode;
 
 #define ANHRDSZ		   (offsetof(AffixNode, data))
diff --git a/src/include/tsearch/ts_type.h b/src/include/tsearch/ts_type.h
index 7e0737c..14cd8dd 100644
--- a/src/include/tsearch/ts_type.h
+++ b/src/include/tsearch/ts_type.h
@@ -63,7 +63,7 @@ typedef uint16 WordEntryPos;
 typedef struct
 {
 	uint16		npos;
-	WordEntryPos pos[1];		/* variable length */
+	WordEntryPos pos[FLEXIBLE_ARRAY_MEMBER];
 } WordEntryPosVector;
 
 
@@ -82,7 +82,7 @@ typedef struct
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	int32		size;
-	WordEntry	entries[1];		/* variable length */
+	WordEntry	entries[FLEXIBLE_ARRAY_MEMBER];
 	/* lexemes follow the entries[] array */
 } TSVectorData;
 
@@ -233,7 +233,7 @@ typedef struct
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	int32		size;			/* number of QueryItems */
-	char		data[1];		/* data starts here */
+	char		data[FLEXIBLE_ARRAY_MEMBER];		/* data starts here */
 } TSQueryData;
 
 typedef TSQueryData *TSQuery;
diff --git a/src/include/utils/catcache.h b/src/include/utils/catcache.h
index b6e1c97..c992ed8 100644
--- a/src/include/utils/catcache.h
+++ b/src/include/utils/catcache.h
@@ -66,8 +66,8 @@ typedef struct catcache
 	long		cc_lsearches;	/* total # list-searches */
 	long		cc_lhits;		/* # of matches against existing lists */
 #endif
-	dlist_head	cc_bucket[1];	/* hash buckets --- VARIABLE LENGTH ARRAY */
-} CatCache;						/* VARIABLE LENGTH STRUCT */
+	dlist_head	cc_bucket[FLEXIBLE_ARRAY_MEMBER];	/* hash buckets */
+} CatCache;
 
 
 typedef struct catctup
@@ -147,8 +147,8 @@ typedef struct catclist
 	uint32		hash_value;		/* hash value for lookup keys */
 	HeapTupleData tuple;		/* header for tuple holding keys */
 	int			n_members;		/* number of member tuples */
-	CatCTup    *members[1];		/* members --- VARIABLE LENGTH ARRAY */
-} CatCList;						/* VARIABLE LENGTH STRUCT */
+	CatCTup    *members[FLEXIBLE_ARRAY_MEMBER];		/* members */
+} CatCList;
 
 
 typedef struct catcacheheader
diff --git a/src/include/utils/datetime.h b/src/include/utils/datetime.h
index 3cd921a..7bfb729 100644
--- a/src/include/utils/datetime.h
+++ b/src/include/utils/datetime.h
@@ -207,7 +207,7 @@ typedef struct
 typedef struct TimeZoneAbbrevTable
 {
 	int			numabbrevs;
-	datetkn		abbrevs[1];		/* VARIABLE LENGTH ARRAY */
+	datetkn		abbrevs[FLEXIBLE_ARRAY_MEMBER];
 } TimeZoneAbbrevTable;
 
 
diff --git a/src/include/utils/geo_decls.h b/src/include/utils/geo_decls.h
index 5c83a71..32f6a7b 100644
--- a/src/include/utils/geo_decls.h
+++ b/src/include/utils/geo_decls.h
@@ -82,7 +82,7 @@ typedef struct
 	int32		npts;
 	int32		closed;			/* is this a closed polygon? */
 	int32		dummy;			/* padding to make it double align */
-	Point		p[1];			/* variable length array of POINTs */
+	Point		p[FLEXIBLE_ARRAY_MEMBER];			/* variable length array of POINTs */
 } PATH;
 
 
@@ -124,7 +124,7 @@ typedef struct
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	int32		npts;
 	BOX			boundbox;
-	Point		p[1];			/* variable length array of POINTs */
+	Point		p[FLEXIBLE_ARRAY_MEMBER];			/* variable length array of POINTs */
 } POLYGON;
 
 /*---------------------------------------------------------------------
diff --git a/src/include/utils/relmapper.h b/src/include/utils/relmapper.h
index 8f0b438..27edffa 100644
--- a/src/include/utils/relmapper.h
+++ b/src/include/utils/relmapper.h
@@ -28,7 +28,7 @@ typedef struct xl_relmap_update
 	Oid			dbid;			/* database ID, or 0 for shared map */
 	Oid			tsid;			/* database's tablespace, or pg_global */
 	int32		nbytes;			/* size of relmap data */
-	char		data[1];		/* VARIABLE LENGTH ARRAY */
+	char		data[FLEXIBLE_ARRAY_MEMBER];
 } xl_relmap_update;
 
 #define MinSizeOfRelmapUpdate offsetof(xl_relmap_update, data)
diff --git a/src/include/utils/varbit.h b/src/include/utils/varbit.h
index 4879a2e..ee1263d 100644
--- a/src/include/utils/varbit.h
+++ b/src/include/utils/varbit.h
@@ -24,7 +24,7 @@ typedef struct
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	int32		bit_len;		/* number of valid bits */
-	bits8		bit_dat[1];		/* bit string, most sig. byte first */
+	bits8		bit_dat[FLEXIBLE_ARRAY_MEMBER];		/* bit string, most sig. byte first */
 } VarBit;
 
 /*
diff --git a/src/interfaces/ecpg/ecpglib/extern.h b/src/interfaces/ecpg/ecpglib/extern.h
index 835e70c..a9f2de2 100644
--- a/src/interfaces/ecpg/ecpglib/extern.h
+++ b/src/interfaces/ecpg/ecpglib/extern.h
@@ -33,7 +33,7 @@ enum ARRAY_TYPE
 struct ECPGgeneric_varchar
 {
 	int			len;
-	char		arr[1];
+	char		arr[FLEXIBLE_ARRAY_MEMBER];
 };
 
 /*
diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c
index 9d561c2..f1cd30c 100644
--- a/src/pl/plpgsql/src/pl_funcs.c
+++ b/src/pl/plpgsql/src/pl_funcs.c
@@ -97,7 +97,7 @@
 	/* first item added must be a label */
 	Assert(ns_top != NULL || itemtype == PLPGSQL_NSTYPE_LABEL);
 
-	nse = palloc(sizeof(PLpgSQL_nsitem) + strlen(name));
+	nse = palloc(offsetof(PLpgSQL_nsitem, name) + strlen(name) + 1);
 	nse->itemtype = itemtype;
 	nse->itemno = itemno;
 	nse->prev = ns_top;
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index 8996501..346953b 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -318,7 +318,7 @@ typedef struct PLpgSQL_nsitem
 	int			itemtype;
 	int			itemno;
 	struct PLpgSQL_nsitem *prev;
-	char		name[1];		/* actually, as long as needed */
+	char		name[FLEXIBLE_ARRAY_MEMBER]; // FIXME
 } PLpgSQL_nsitem;
 
 
#17Greg Stark
stark@mit.edu
In reply to: Tom Lane (#1)
Re: Back branches vs. gcc 4.8.0

On Fri, Apr 5, 2013 at 11:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Since gcc 4.8 is going to be on a lot of people's machines pretty soon,
I think we need to do something to prevent it from breaking 8.4.x and
9.0.x. 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'm a bit leaning towards (1), mainly because I'm not excited about

I'm confused. I would have described (1) as entering an arms race.
Each new optimization related to arrays and structs would need a new
flag. Whereas (2) makes the code pretty common traditional code that
gcc is going to need to tolerate for the foreseeable future

--
greg

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#18Andres Freund
andres@2ndquadrant.com
In reply to: Peter Eisentraut (#16)
Re: Back branches vs. gcc 4.8.0

On 2013-04-29 23:37:43 -0400, Peter Eisentraut wrote:

On Sat, 2013-04-06 at 12:59 -0400, Tom Lane wrote:

The reason I'm thinking it's a good idea is that it would expose any
remaining places where we have nominally var-length arrays embedded in
larger structs. Now that I've seen the failures with gcc 4.8.0, I'm
quite worried that there might be some more declarations like that
which we've not identified yet, but that by chance aren't causing
obvious failures today.

Here is a rough patch that replaces almost all occurrences of
something[1] in a struct with FLEXIBLE_ARRAY_MEMBER. It crashes left
and right (because of sizeof issues, probably), but at least so far the
compiler hasn't complained about any flexible-array members not at the
end of the struct, which is what it did last time. So the answer to
your concern so far is negative.

I think this point in the cycle would be a good one to apply something
like this.

Completing this patch will be quite a bit more debugging work. Some
kind of electric fence for palloc would be helpful.

Noah's recently added valgrind mode should provide this.

Do you have an updated version of this patch already? I'd be willing to
make a pass over it to check whether I find any missed updates...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers