Re: GIST code doesn't build on strict 64-bit machines

Tom Lane wrote:
> I've just found out the hard way that Postgres doesn't even build on
> recent gcc releases for 64-bit HPPA. The reason is that the compiler
> now notices and complains about alignment errors that will lead to
> core dump at runtime, and GIST has got some. The particular code that
> fails to compile is in gist.c:
> gistentryinit(((GISTENTRY *) VARDATA(evec))[1],
> ((GISTENTRY *) VARDATA(evec))[0].key, r, NULL,
> (OffsetNumber) 0, ((GISTENTRY *) VARDATA(evec))[0].bytes, FALSE);
>
> Since VARDATA() is at a 4-byte offset from the start of the datum, this
> is trying to overlay a GISTENTRY struct at a 4-byte boundary. When
> compiling in 64-bit mode, Datum is 8 bytes, and so the GISTENTRY struct
> is not sufficiently well aligned. Unlike Intel machines, the HP chips
> *require* 8-byte loads and stores to be 8-byte-aligned.

Hm.

evec is defined as
storage = (char *) palloc(n * sizeof(GISTENTRY) + MAXALIGN(VARHDRSZ));
evec = (bytea *) (storage + MAXALIGN(VARHDRSZ) - VARHDRSZ);

VARDATA is defined as:
#define VARDATA(__PTR) VARATT_DATA(__PTR)
#define VARATT_DATA(PTR) (((varattrib *)(PTR))->va_content.va_data)

and VARHDRSZ is
#define VARHDRSZ ((int32) sizeof(int32))

Look follow:
VARATT_SIZEP(evec) = 2 * sizeof(GISTENTRY) + VARHDRSZ;
#define VARATT_SIZEP(_PTR) (((varattrib *)(_PTR))->va_header)

So, if ((varattrib *)evec)->va_content.va_data - (char*)evec == 4 then
((GISTENTRY *) VARDATA(evec))[i] is 8-byte aligned, but evec - no.

But if ((varattrib *)evec)->va_content.va_data - (char*)evec == 8 then
both evec and ((GISTENTRY *) VARDATA(evec))[i] isn't 8-byte aligned.

I don't afraid to say some rubbish :)
I wrote simple test:
#include <stdio.h>
#include "c.h"
typedef struct {
int32 len;
char data[1];
} TST;

int main(int argn, char *argv[]) {
TST t;
TST *ptr = &t;
printf("%d\n", (ptr->data - (char*)ptr));
return 0;
}

It prints 4 for my Intel systems and for Alpha system, but I havn't access to
HPUX. If result is equal to 8 on HPUX, then I suppose that replacing to
evec = (bytea *) storage
will resolve our problem (but VARHDRSZ has inconsistent definition).
But if result is 4 then we should use
evec = (bytea *) storage
and replace all VARDATA macro to something like
#define MY_VARDATA(PTR) ( ((char*)PTR) + MAXALIGN(VARHDRSZ) )

But all of this is strage for me, because we already faced to problem with
8-bytes strict aliasing in GiST code, and we had resolved problem on Sun and
Alpha boxes. What was it changed?

> I suppose that a correct fix involves doing MAXALIGN(VARDATA(evec)),
> but I do not know what places need to change to support this.

Its only union and picksplit user-defined methods in contrib modules.

--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru