FAQ
gcc 4.6 has now arrived as the default compiler on my desktop, and as
previously reported, it throws a bunch of warnings, foiling my life-long
plan of compiling PostgreSQL with -Werror.

So looking more aggressively into fixing some of these, let's look at
this case:

gistutil.c: In function ‘gistMakeUnionKey’:
gistutil.c:263:16: warning: array subscript is above array bounds [-Warray-bounds]
gistutil.c:268:16: warning: array subscript is above array bounds [-Warray-bounds]
gistutil.c:273:16: warning: array subscript is above array bounds [-Warray-bounds]

The code in question is this:

typedef struct
{
int32 n; /* number of elements */
GISTENTRY vector[1]; /* variable-length array */
} GistEntryVector;

Not sure why the new gcc is confused about this when -Warray-bounds has
existed for a while. But thinking a bit further, the "proper" fix for
this would be to use flexible array members like this:

typedef struct
{
int32 n; /* number of elements */
GISTENTRY vector[];
} GistEntryVector;

This is C99, but with some gentle standard autoconf seasoning, it can be
made transparent. See attached patch.

Is this a route we want to go down?

It looks as though other compilers could also benefit from this. clang
throws even more warnings of this kind, and the clang static analyzer
even more.

One thing that is a bit concerning is that throwing more flexible array
members around the code wherever variable-length arrays are used results
in crash and burn. Probably some places are using sizeof or offsetof on
these structures in incompatible ways. So each place would have to be
examined separately.

Search Discussions

  • Tom Lane at Jun 15, 2011 at 10:19 pm

    Peter Eisentraut writes:
    Is this a route we want to go down?
    - GISTENTRY vector[1]; /* variable-length array */
    + GISTENTRY vector[FLEXIBLE_ARRAY_MEMBER];
    Yes, I was thinking about the same trick after noting these warnings on
    Fedora 15, although personally I'd name the macro VARIABLE_LENGTH_ARRAY.
    One thing that is a bit concerning is that throwing more flexible array
    members around the code wherever variable-length arrays are used results
    in crash and burn. Probably some places are using sizeof or offsetof on
    these structures in incompatible ways. So each place would have to be
    examined separately.
    Hmm, that's nasty. But from a code-documentation standpoint I think
    this is a useful improvement, so it seems worth doing the work to clean
    things up.

    (I do recall a number of places that assume that sizeof() includes a
    single array element ...)

    regards, tom lane
  • Peter Eisentraut at Jun 16, 2011 at 7:50 pm

    On ons, 2011-06-15 at 18:19 -0400, Tom Lane wrote:
    Peter Eisentraut <peter_e@gmx.net> writes:
    Is this a route we want to go down?
    - GISTENTRY vector[1]; /* variable-length array */
    + GISTENTRY vector[FLEXIBLE_ARRAY_MEMBER];
    Yes, I was thinking about the same trick after noting these warnings
    on Fedora 15, although personally I'd name the macro
    VARIABLE_LENGTH_ARRAY.
    This macro is provided by Autoconf and it appears to be using the
    standard's terminology.

    Actually, the term "variable-length array" appears to refer to another
    C99 feature, namely this one:

    void
    foo(int n)
    {
    bar int[n];

    do_something();
    }
  • Brar Piening at Jun 16, 2011 at 11:04 pm

    On Thu, 16 Jun 2011 22:49:45 +0300, Peter Eisentraut wrote:
    This macro is provided by Autoconf and it appears to be using the
    standard's terminology.
    commit dbbba5279f66f95805c1e084e6f646d174931e56 refs/heads/master
    Author: Peter Eisentraut <peter_e@gmx.net>
    Date: Thu Jun 16 22:39:09 2011 +0300

    Periodically checking my VS2010 patch I noticed that this one broke
    Visual Studio builds.

    At least mine and "chough
    <http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=chough&br=HEAD>"
    in the build farm - I expect others to follow once they rebuild.

    error C2065: 'FLEXIBLE_ARRAY_MEMBER' : undeclared identifier
    error C2057: expected constant expression

    Regards,

    Brar

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedJun 15, '11 at 8:14p
activeJun 16, '11 at 11:04p
posts4
users3
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2021 Grokbase