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

Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.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.

I see 35 instances of this coding, and only 12 are in contrib/cube;
examples attached.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +
/usr/var/local/src/gen/pgsql/postgresql/contrib/cube/cubeparse.y:178:  int size = offsetof(NDBOX, x[0]) +
sizeof(double)* dim * 2; 
/usr/var/local/src/gen/pgsql/postgresql/contrib/cube/cubeparse.y:210:  size = offsetof(NDBOX, x[0]) + sizeof(double) *
dim* 2; 
/usr/var/local/src/gen/pgsql/postgresql/contrib/cube/cube.c:202:    size = offsetof(NDBOX, x[0]) +sizeof(double) * 2 *
dim;
/usr/var/local/src/gen/pgsql/postgresql/contrib/cube/cube.c:238:    size = offsetof(NDBOX, x[0]) +sizeof(double) * 2 *
dim;
/usr/var/local/src/gen/pgsql/postgresql/contrib/cube/cube.c:271:    size = offsetof(NDBOX, x[0]) +sizeof(double) * 2 *
dim;
/usr/var/local/src/gen/pgsql/postgresql/contrib/cube/cube.c:1376:    size = offsetof(NDBOX, x[0]) +sizeof(double) * dim
*2; 
/usr/var/local/src/gen/pgsql/postgresql/contrib/cube/cube.c:1417:    size = offsetof(NDBOX, x[0]) +sizeof(double) * 2;
/usr/var/local/src/gen/pgsql/postgresql/contrib/cube/cube.c:1435:    size = offsetof(NDBOX, x[0]) +sizeof(double) * 2;
/usr/var/local/src/gen/pgsql/postgresql/contrib/cube/cube.c:1456:    size = offsetof(NDBOX, x[0]) +sizeof(double) *
(c->dim+ 1) *2; 
/usr/var/local/src/gen/pgsql/postgresql/contrib/cube/cube.c:1483:    size = offsetof(NDBOX, x[0]) +sizeof(double) *
(c->dim+ 1) *2; 
/usr/var/local/src/gen/pgsql/postgresql/contrib/cube/cubeparse.c:1369:  int size = offsetof(NDBOX, x[0]) +
sizeof(double)* dim * 2; 
/usr/var/local/src/gen/pgsql/postgresql/contrib/cube/cubeparse.c:1401:  size = offsetof(NDBOX, x[0]) + sizeof(double) *
dim* 2; 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/postmaster/pgstat.c:794:    len = offsetof(PgStat_MsgTabstat,
m_entry[0])+ 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/postmaster/pgstat.c:840:            pgstat_send(&msg,
offsetof(PgStat_MsgFuncstat,m_entry[0]) + 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/postmaster/pgstat.c:850:        pgstat_send(&msg,
offsetof(PgStat_MsgFuncstat,m_entry[0]) + 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/postmaster/pgstat.c:951:            len =
offsetof(PgStat_MsgTabpurge,m_tableid[0]) 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/postmaster/pgstat.c:967:        len = offsetof(PgStat_MsgTabpurge,
m_tableid[0])
/usr/var/local/src/gen/pgsql/postgresql/src/backend/postmaster/pgstat.c:1011:                len =
offsetof(PgStat_MsgFuncpurge,m_functionid[0]) 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/postmaster/pgstat.c:1025:            len =
offsetof(PgStat_MsgFuncpurge,m_functionid[0]) 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/postmaster/pgstat.c:1127:    len = offsetof(PgStat_MsgTabpurge,
m_tableid[0])+sizeof(Oid); 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/access/heap/syncscan.c:109:#define SizeOfScanLocations(N)
offsetof(ss_scan_locations_t,items[N]) 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/access/nbtree/nbtree.c:535:
offsetof(BTScanPosData,items[1]) + 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/access/nbtree/nbtsearch.c:1120:
offsetof(BTScanPosData,items[1]) + 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/access/nbtree/nbtutils.c:1450:    size = offsetof(BTVacInfo,
vacuums[0]);
/usr/var/local/src/gen/pgsql/postgresql/src/backend/utils/adt/geo_ops.c:1424:    size = offsetof(PATH, p[0])
+sizeof(path->p[0])* npts; 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/utils/adt/geo_ops.c:1470:    if (npts <= 0 || npts >= (int32)
((INT_MAX- offsetof(PATH, p[0])) / sizeof(Point))) 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/utils/adt/geo_ops.c:1475:    size = offsetof(PATH, p[0])
+sizeof(path->p[0])* npts; 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/utils/adt/geo_ops.c:3476:    size = offsetof(POLYGON, p[0])
+sizeof(poly->p[0])* npts; 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/utils/adt/geo_ops.c:3523:    if (npts <= 0 || npts >= (int32)
((INT_MAX- offsetof(POLYGON, p[0])) / sizeof(Point))) 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/utils/adt/geo_ops.c:3528:    size = offsetof(POLYGON, p[0])
+sizeof(poly->p[0])* npts; 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/utils/adt/geo_ops.c:4244:    size = offsetof(PATH, p[0])
+base_size;
/usr/var/local/src/gen/pgsql/postgresql/src/backend/utils/adt/geo_ops.c:4382:    size = offsetof(POLYGON, p[0])
+sizeof(poly->p[0])* path->npts; 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/utils/adt/geo_ops.c:4457:    size = offsetof(POLYGON, p[0])
+sizeof(poly->p[0])* 4; 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/utils/adt/geo_ops.c:4487:    size = offsetof(PATH, p[0])
+sizeof(path->p[0])* poly->npts; 
/usr/var/local/src/gen/pgsql/postgresql/src/backend/utils/adt/geo_ops.c:5166:    size = offsetof(POLYGON, p[0])
+base_size;