Re: GIST code doesn't build on strict 64-bit machines - Mailing list pgsql-hackers

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@sigaev.ru