Re: pgsql: Remove "fmgr.h" include in cube contrib --- caused crash on a Ge

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Jeremy Drake <pgsql(at)jdrake(dot)com>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-committers <pgsql-committers(at)postgresql(dot)org>
Subject: Re: pgsql: Remove "fmgr.h" include in cube contrib --- caused crash on a Ge
Date: 2011-09-02 16:11:00
Message-ID: 18388.1314979860@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> I wonder if this would be the right time to start using the
> FLEXIBLE_ARRAY_MEMBER stuff in contrib/cube. Note pg_config.h.in says

> /* Define to nothing if C supports flexible array members, and to 1 if it does
> not. That way, with a declaration like `struct s { int n; double
> d[FLEXIBLE_ARRAY_MEMBER]; };', the struct hack can be used with pre-C99
> compilers. When computing the size of such an object, don't use 'sizeof
> (struct s)' as it overestimates the size. Use 'offsetof (struct s, d)'
> instead. Don't use 'offsetof (struct s, d[0])', as this doesn't work with
> MSVC and with C++ compilers. */

D'oh ... I bet that last sentence is pointing us at the problem. cube
is using exactly that construct, and for some reason it's crashing.
The most likely explanation for why it's crashing is that the compiler
is trying to dereference NULL instead of successfully reducing
offsetof() to a compile-time constant. It's still not too clear to me
why the inclusion changes cause that, but certainly walsender.h is
pulling in a crapload of other stuff that was not previously included
there ... which connects back to my previous complaints that I think
Bruce was way too aggressive in adding #includes to headers.

Jeremy, could you look at the preprocessor output for cube.c (ie,
use -E instead of -c in the gcc call) and see how the relevant line
of cube_f8_f8 looks in both broken and non-broken cases? What I see
on a Fedora box is

size = __builtin_offsetof (NDBOX, x[0]) +sizeof(double) * 2;

but I'm thinking you might be getting something different.

regards, tom lane

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Bruce Momjian 2011-09-02 16:44:28 Re: pgsql: Remove "fmgr.h" include in cube contrib --- caused crash on a Ge
Previous Message Alvaro Herrera 2011-09-02 15:38:28 Re: pgsql: Remove "fmgr.h" include in cube contrib --- caused crash on a Ge

Browse pgsql-hackers by date

  From Date Subject
Next Message Kohei Kaigai 2011-09-02 16:38:51 Re: [v9.1] sepgsql - userspace access vector cache
Previous Message Alvaro Herrera 2011-09-02 15:38:28 Re: pgsql: Remove "fmgr.h" include in cube contrib --- caused crash on a Ge